Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong Parameter Name with Default Sort #1010

Closed
tkalmar opened this issue Jan 12, 2021 · 3 comments
Closed

Wrong Parameter Name with Default Sort #1010

tkalmar opened this issue Jan 12, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@tkalmar
Copy link

tkalmar commented Jan 12, 2021

Describe the bug
When a controller Pageable method parameter is annotated with @PageableDefault and a sort Parameter is given in the annotation the parameter name for the sort is not sort. Instead it is the value of the sort Parameter.
The cause is:

if (pageableDefault != null && ArrayUtils.isNotEmpty(pageableDefault.sort()))
name = String.join(",", pageableDefault.sort());

To Reproduce
Following Controller Method triggers the bug

  @Operation(description = "SomeDescription")
  @GetMapping("/")
  public String getPatientList(@PageableDefault(size = 100, sort = { "someField" }, direction = Sort.Direction.ASC)
                                            @ParameterObject
                                            Pageable pageable){
return "bla";
}

leads to:

{
            "name": "someField",
            "in": "query",
            "description": "Sorting criteria in the format: property(,asc|desc). Default sort order is ascending. Multiple sort criteria are supported.",
            "required": false,
            "schema": {
              "type": "array",
              "items": {
                "type": "string"
              }
            }
          }

Expected behavior
the field name should stay sort the default value should be someField,asc (notice value + sort direction)
The only time the field name should be changed, is when annotated with @Qualifier which seems also not supported.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

@bnasslahsen
Copy link
Contributor

@tkalmar,

The wrong parameter naming is now fixed.

Note that @Qualifieris only for adding a prefix to properties naming.
For changing properties names, you have 2 additional ways through configuration properties available on SpringDataWebProperties and RepositoryRestConfiguration.

For the default value of the sort Parameter, it actually depends on the default value of the annoatation @ArraySchema which is used internally by springdoc-openapi to map the value.

This is unfortunate, because the @ArraySchema annotation does not contain the default value, even this annotation exists on the @Schema level.

This can be added through OpenApiCustomiser, but it seems too much.
If you need it out of the box, in springdoc-openapi, the straight forward way is to submit an enhancement request to the swagger-core project to add the defaultValue on @ArraySchema annotation.

@tkalmar
Copy link
Author

tkalmar commented Jan 20, 2021

@bnasslahsen Isn't @ArraySchmema arrayschema attribute used to define defaults for the array schema?

@bnasslahsen
Copy link
Contributor

bnasslahsen commented Jan 20, 2021

it is indeed...
These are the details to show you the problem.

This is a first sample to show the problem that defaultValue are not handled correctly on the @ArraySchema annotation.

@SpringBootApplication
public class DemoApplication {

	public static void main(String[] args) {
		SpringApplication.run(DemoApplication.class, args);
		ResolvedSchema resolvedSchema = ModelConverters.getInstance()
				.resolveAsResolvedSchema(new AnnotatedType(Sample.class));
		ArraySchema arraySchema = (ArraySchema) resolvedSchema.schema.getProperties().get("sort");
		Assert.isTrue( !String.class.equals(arraySchema.getDefault().getClass()), "The result should not be of type string" );

	}


	class Sample {

		@io.swagger.v3.oas.annotations.media.ArraySchema(arraySchema = @Schema(defaultValue =  "[\"sample1\", \"sample1\"]"))
		@JsonProperty
		private List<String> sort;

	}
}

I have added a workaround for that. This is not the ideal solution, but should make it work.
If you create the enhancement request on the swagger-core project to handle properly default values on for array types, please link it to this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants