-
Notifications
You must be signed in to change notification settings - Fork 424
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
Spring - update nullable and required validation #1308
Conversation
Co-authored-by: frantuma <[email protected]>
Unfortunately (as stated above) this fixes the problem only for Spring, for me ( BTW: I had to search the web to see how to configure for Spring to try it out (the <language>spring</language>
<configOptions>
<library>spring-boot3</library>
</configOptions>
<additionalProperties>
<additionalProperty>jakarta=true</additionalProperty>
<additionalProperty>validationMode=legacy</additionalProperty>
</additionalProperties> |
Hi @frantuma -- we are running into a new issue upgrading from 3.0.61 -> 3.0.62 in our open source Spring Boot project (see linked PR above). Our generated code fails to compile with errors like the following:
To work around we've elected |
Would you be able to share the spec causing the issue along with configuration? |
@pe-st the fix is indeed only for spring at this moment, a similar change for all java generators needs more effort, it is planned with no fixed ETA atm. Re-opening #1295 to track full java update |
Noticed similar exceptions as @okotsopoulos when upgrading from v3.0.61 to v3.0.62: |
This has been fixed in refs #1308 - fix jakarta spring generator by frantuma · Pull Request #1315 · swagger-api/swagger-codegen-generators part of release 3.0.53 of codegen |
Replaces and includes #1305
This PR provides a thorough update of
spring
generator logic and options handling validation scenarios where schema properties are defined asrequired
ornullable
or any combinations of these keywords.This is (also) related to #1295 and #1291
Current status (before this PR, Codegen <=3.0.61 / Generators <=1.0.51)
Before this PR, validation for properties with
nullable
orrequired
keywords or any combination of these was based on the following:@NotNull
. This is not correct for cases where a property is both required and nullable.useNullableForNotNull
(true by default). When set to true all properties not defined withnullable=true
are annotated with@NotNull
. This is not correct for cases where a property is nullable but also required. This is also possibly too strict (even if correct according to OpenAPI / Json Schema specification) for properties without anullable
keyword. This rule is the one applied by default in Codegen 3.0.61+ / Generators 1.0.51+Updated behavior
Both behaviors above are limited and not correct, as there are 4 possible combinations:
nullable + required
nullable + not required
not nullable + required
not nullable + not required
Note that scenario 4.
not nullable + not required
is the one occurring when nonullable
norrequired
keyword is applied to a property; in this case OpenAPI / JSON Schema mandate that the property CANNOT be null if atype
is defined, therefore in astrict
mode this should be considered in validation.Scenario 1.
nullable + required
and 4.nullable + required
are the most "problematic" in context of a deserialization and representation in Java, as they involve distinguishing between "non absent" andnull
values.Specifically for Scenario 1.
nullable + required
, this is only achievable by wrapping the type into a container (JsonNullable
) and applying custom validation. JsonNullable has been chosen instead of e.g.Optional
for various reasons, detailed and discussed in project README, OpenAPI Generator issue among others)A custom validator has been added as validation provided by
jackson-databind-nullable
is not working as expected (see e.g. OpenAPITools/openapi-generator#14766, OpenAPITools/openapi-generator#14765, OpenAPITools/openapi-generator#11969 (comment), OpenAPITools/jackson-databind-nullable#17Scenario 4. is addressed using Jackson (de) serialization annotations while Scenario 3. is addressed simply using
@NotNull
validationMode
optionThis PR introduces option
validationMode
(limited tospring
generator) which allows to choose the validation strategy. Default mode isstrict
.strict
(default)nullable + required
:JsonNullable
+ custom validationnullable + not required
: no validation annotationsnot nullable + required
:@NotNull
not nullable + not required
:@JsonInclude(NON_ABSENT)
+ @JsonSetter(nulls = FAIL)`loose
nullable + required
:JsonNullable
+ custom validationnullable + not required
: no validation annotationsnot nullable + required
:@NotNull
not nullable + not required
: no validation annotationsloose
mode behaves likestrict
except for scenario 4not nullable + not required
where no validation is defined; this is for cases where the correct behavior when nonullable
norrequired
keyword is applied to a property is considered too strict.legacy
nullable + required
:@NotNull
nullable + not required
: no validation annotationsnot nullable + required
:@NotNull
not nullable + not required
: no validation annotationslegacy
mode provides the same validation applied by default before Java - fix @NotNull resolving from required/nullable #1291legacyNullable
nullable + required
: no validation annotationsnullable + not required
: no validation annotationsnot nullable + required
:@NotNull
not nullable + not required
:@NotNull
legacyNullable
mode provides the same validation applied by default after Java - fix @NotNull resolving from required/nullable #1291 and before this PR.We are asking the community to provide feedback to this change and we welcome and ready to further refinement and/or additional options.