fix: pass along type_use annotations to swagger(-core)#3300
fix: pass along type_use annotations to swagger(-core)#3300tthornton3-chwy wants to merge 2 commits into
Conversation
| * A parameter object whose {@code clinicId} field is reused as both an optional, nullable | ||
| * query parameter and a (required, non-null) path parameter, depending on the controller. | ||
| */ | ||
| class SearchCriteria { |
There was a problem hiding this comment.
What is the reason for wanting to reuse this object in different contexts where @Nullable is not always correct? For the case where it is used in a path-parameter context, then the annotation misleads the consumer using the object, since the object says nullable but it actually is required?
Isn't the better approach to have an object specifically for the path scenario, so that it both documents the nullable correctly inwards and outwards?
There was a problem hiding this comment.
An example: we follow the CQRS-pattern, and have types that get deserialized for some queries. There's an "admin" path in, something like /admin/vet which can do whatever it wants (add a resource id as a query param, not add an id, put any id it wants). But there is also a "scoped" path, think of something like /resource/{resourceId}/vet that requires it (and authenticates that it is able to access said resourceId). They do the absolute same thing though, and come back to the same type. Sure, we could have an AdminSearchCriteria vs ScopedSearchCriteria, but now we have two methods in our service and gotta map one probably back to the other so we can stay DRY.
So the case where a PathParameter exists, is valid, then its required no matter what, and this helps us do that! Swagger Core didn't seem the right place to think about this, since that's a layer deeper than it probably should be. And with this I get both:
{
"name": "resourceId",
"in": "path",
"description": "Find something affiliated with something id.",
"required": true,
"schema": {
"type": "string",
"description": "Find Something affiliated with this something id.",
"example": 11111
},
"example": 1111
}
for the scoped and
{
"name": "resourceId",
"in": "query",
"description": "Find something affiliated with this something id.",
"required": false,
"schema": {
"type": [
"string",
"null"
],
"description": "Find Something affiliated with this something id.",
"example": 11111
},
"example": 11111
},
which is exactly what we want :)
This use case may be semi-niche, but I don't think the case of forcing a path parameter to be required is niche at all!
There was a problem hiding this comment.
If DRY-ness is of importance in the service, would we not model the service to rely on an interface that states that it takes an optional resourceId, and then that the two DTOs implement that?
So we have DTOs that are very expressive with how the deliver data (one has resourceId as required, the other does not, which aligns exactly to the OAS), but then for the case of this particular service they utilize the interface to abide to the requirements of this particular service?
There was a problem hiding this comment.
I appreciate your time and concern. Maybe we bring this back down to basics -- if there was some way, somehow, outside of anything talked about here, for a Parameter/Schema to come back to springdoc-openapi as nullable, should we fail (swagger-ui fails pretty spectactuarly) or should we make it required? SpringDocUtils already has handleSchemaTypes / fixNullOnlyAdditionalProperties that handle a suspiciously similar case as we have here, so a precedent has already been made for the latter?
Summary
Preserve top-level TYPE_USE annotations when Springdoc flattens @ParameterObject fields into query parameters.
Previously, GenericParameterService only forwarded annotations from parameterized type arguments. That meant annotations like JSpecify-style @nullable on an array type, e.g. Status @nullable [] status, were dropped before schema extraction. Swagger-core therefore never saw the nullable signal and generated only type: "array".
This change forwards both:
That allows OpenAPI 3.1 nullable array query parameters to generate:
"type": ["array", "null"]
Change Plan
Test Plan
Ran the test suite with the new tests.