-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
Description
Regression introduced in #21951 (released in 7.16.0)
PR #21951 added @NotNull to all @PathVariable parameters in the Spring generator. This is both a breaking change that should not have landed in a minor release, and it is technically unnecessary.
1. It's a breaking change
Adding @NotNull to method parameters in a generated interface is a breaking change for all existing users that implement those interfaces.
When Bean Validation (javax.validation / jakarta.validation) is active and @Validated is present on the controller, Spring enforces @NotNull constraints at runtime. Any existing implementation that previously compiled and ran fine can now suddenly get a ConstraintViolationException — for example in:
- Unit/integration tests that call the method directly with a
nullargument - Programmatic calls through proxies or AOP interceptors
- Custom binding scenarios where the path variable is resolved differently
This breaks the Semantic Versioning contract this project follows. A change that introduces runtime failures in previously working code belongs in a major version bump (8.0.x), not a minor release (7.16.0).
2. It's unnecessary — Spring MVC already guarantees path variables are non-null
The whole premise of the original bug report (#21921) is based on a misunderstanding of how Spring MVC works.
Spring MVC makes it physically impossible for a @PathVariable to be null at the point the method is called, unless you explicitly declare it with required = false. This is guaranteed by the framework:
- The Spring Framework documentation on
@PathVariablestates: "By default,@PathVariableparameters are required." - If the URI template variable is missing or cannot be bound, Spring throws a
MissingPathVariableException(HTTP 500) before your method is even invoked —@NotNullis never even evaluated. - The template comment that was intentionally removed by PR Marking required path parameters as @NotNull #21951 said exactly this:
This was a deliberate, documented design decision — not an oversight or bug.
{{! PathParam is always required, no @NotNull necessary }}
Adding @NotNull in this context is redundant noise for runtime behavior. The only case where it could add value is for static analysis tools (e.g. NullAway, JSpecify, IntelliJ inspections), but that is a different and separate concern — and one that deserves its own opt-in option.
3. It's inconsistent across generators
This change was only applied to the Spring Java generator, leaving 15+ other Java/Kotlin generators behaving differently. This creates confusion and inconsistency across the ecosystem, as noted by @kolomanschaft in the original PR.
Steps to reproduce the regression
- Generate a Spring server stub with openapi-generator 7.15.x and implement the interface
- Upgrade to 7.16.0
- Regenerate — all
@PathVariableparameters now have@NotNull - If you have
@Validatedon your controller and any test/call passesnullexplicitly to the interface method, you now get aConstraintViolationExceptionat runtime
Expected behavior
@PathVariable parameters should not have @NotNull added automatically, as Spring MVC already guarantees they are non-null before the method is called. The behavior prior to 7.16.0 was correct.
Proposed fix
- Revert the change from
master/7.x(or provide a fallback mechanism) - If adding
@NotNullfor static analysis purposes is desired, expose it as an opt-in generator option (e.g.addNotNullToPathParams=true) rather than making it the silent default - If the community agrees the behavior is desirable for correctness, it should be re-introduced in
8.0.x(the breaking changes branch) with proper release notes
Related
- PR that introduced the regression: Marking required path parameters as @NotNull #21951
- Original issue (incorrectly classified as a bug): [BUG] [Java] [Spring Boot] Required path params not marked NotNull #21921