Parameter order is not preserved

[dilipkrish]

looks still broken

Originally posted by @schrek1 in #2705 (comment)

[dilipkrish]

Also previous fix that doesn't work anymore #2721

[schrek1]

only for information I mean ordering property inside response/request model

[dilipkrish]

There is another issue for that too 👍

[roookeee]

I would propose removing the implicit sorting done in Operation.java's constructor and leave them "as-is". Sorting of parameters should be done through the plugin system you already provide. This way people could opt-out of some sorting by disabling specific plugin (e.g. through a property). The current implicit sorting done in the constructor is inflexible and forces an undesireable unflexibility. The current behaviour can be emulated by adding a new plugin that sorts by name (to be backwards compatible). I could work on a PR if you want as the parameter order is quite crucial for a project I am currently working on which is blocked by this issue

[dilipkrish]

That would be awesome @roookeee. As of today, the parameter does preserve the as-is sorting, it sorts on parameter index if Im not mistaken. That order is disturbed only via plugins changing that.

[roookeee]

Okay I will have to recheck which one is "breaking" the default order then.

[roookeee]

Found the issue: you are correct that the order is honored but it's actually not set to the parameter index but left as the default which is the highest precedence for all parameters which leads the current code to only sort alphabetically by name again. I will do a PR to fix this and set the order to the parameter index. The comment in the linked non-working PR is fixing this issue by the way:

import org.springframework.core.Ordered;
import springfox.documentation.service.ResolvedMethodParameter;
import springfox.documentation.spi.DocumentationType;
import springfox.documentation.spi.service.ParameterBuilderPlugin;
import springfox.documentation.spi.service.contexts.ParameterContext;

public class ParameterIndexOrderReader implements ParameterBuilderPlugin {

  private static final int PARAMETER_INITIAL_ORDER = Ordered.HIGHEST_PRECEDENCE;

  @Override
  public boolean supports(DocumentationType delimiter) {
    return true;
  }

  @Override
  public void apply(ParameterContext context) {
    ResolvedMethodParameter methodParameter = context.resolvedMethodParameter();
    context.parameterBuilder().order(PARAMETER_INITIAL_ORDER + methodParameter.getParameterIndex());
  }
}

Should I implement it like this or leave the order of the parameters untouched if I detect that another plugin customized the parameter order already (e.g. checking if all parameters still have PARAMETER_INITIAL_ORDER set) ? It's probably best to set the initial order by parameter index as soon as possible and allow other plugins to override it later.

[m7stock]

I agree that this is the right approach, but I would like to point out that the requestParameterBuilder also needs an update of the parameterIndex at the moment, because it is normally used since Springfox 3.

context.requestParameterBuilder().parameterIndex(PARAMETER_INITIAL_ORDER + methodParameter.getParameterIndex());

Although the data structure Compatibility<Parameter, RequestParameter> in this context gives the impression to me that a migration is done by Springfox, every custom ParameterBuilderPlugin seems to have to handle this by itself.

EDIT: The OperationBuilder later sorts its requestParameters by name, so the parameterIndex has no effect yet. So far this can only be prevented with springfox.documentation.swagger.use-model-v3 = false (uses legacy parameters instead).

[henryso]

My current workaround for this is to make a copy OperationBuilder with the following definition for defaultRequestParameterComparator:

    private Comparator<RequestParameter> defaultRequestParameterComparator() {
        return Comparator.comparingInt(RequestParameter::getParameterIndex)
                .thenComparing(p -> nullToEmpty(p.getName()));
    }

... and then to have this ahead of the springfox library in the CLASSPATH. There's no way that I could find to override or customize the Comparator used by OperationBuilder to sort parameters.

A pretty fragile workaround, but at least it gets me past this hump.

[EugeneGoroschenyaOld]

Came up with an idea of how to sort parameters by using VendorExtension in custom plugins

import io.swagger.models.Swagger;
import io.swagger.models.parameters.Parameter;
import lombok.RequiredArgsConstructor;
import lombok.Value;
import org.springframework.core.annotation.Order;
import org.springframework.stereotype.Component;
import springfox.documentation.service.VendorExtension;
import springfox.documentation.spi.DocumentationType;
import springfox.documentation.spi.service.ParameterBuilderPlugin;
import springfox.documentation.spi.service.contexts.ParameterContext;
import springfox.documentation.swagger2.web.SwaggerTransformationContext;
import springfox.documentation.swagger2.web.WebMvcSwaggerTransformationFilter;

import javax.servlet.http.HttpServletRequest;
import java.util.Comparator;
import java.util.List;

import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
import static java.util.Comparator.comparingInt;
import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
import static springfox.documentation.swagger.common.SwaggerPluginSupport.SWAGGER_PLUGIN_ORDER;

@Component
@Order(SWAGGER_PLUGIN_ORDER + 1000)
@RequiredArgsConstructor
public class OrderParametersSwaggerPlugin implements WebMvcSwaggerTransformationFilter, ParameterBuilderPlugin {

    private static final Comparator<Parameter> COMPARATOR = comparingInt(parameter ->
        (int) defaultIfNull(parameter.getVendorExtensions(), emptyMap()).getOrDefault(OrderExtension.NAME, 0));

    @Override
    public boolean supports(DocumentationType delimiter) {
        return delimiter == DocumentationType.SWAGGER_2;
    }

    @Override
    public Swagger transform(SwaggerTransformationContext<HttpServletRequest> context) {
        Swagger swagger = context.getSpecification();
        swagger.getPaths().forEach((key, value) -> {
            sort(value.getParameters());
            value.getOperations().forEach(operation -> sort(operation.getParameters()));
        });
        return swagger;
    }

    private void sort(List<Parameter> parameters) {
        if (parameters != null) {
            parameters.sort(COMPARATOR);
        }
    }

    @Override
    public void apply(ParameterContext parameterContext) {
        int parameterIndex = parameterContext.resolvedMethodParameter().getParameterIndex();
        parameterContext.requestParameterBuilder()
            .extensions(singletonList(new OrderExtension(parameterIndex)));
    }

    @Value
    static class OrderExtension implements VendorExtension<Integer> {
        static final String NAME = "order";
        Integer value;
        String name = NAME;
    }
}

[16188722]

Please update on this issue.

[okohub]

We finally moved to springdoc.

Everyone, please consider moving to springdoc, kindy suggesting :)