[Spring] Add JsonProperty access=READ_ONLY for readOnly properties - #24929
[Spring] Add JsonProperty access=READ_ONLY for readOnly properties#24929antoineclech-arkea wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
3 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache:259">
P1: When Lombok generates the setter through `lombok.Setter` or `lombok.Data`, this line is not rendered and no Lombok-side annotation sets `READ_ONLY`, so read-only properties remain writable by Jackson. Apply the access setting to the Lombok-generated accessor or field path as well.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache:259">
P1: When `openApiNullable` is enabled for a non-required nullable read-only property, this guard suppresses the new annotation, so Jackson still auto-detects the setter and accepts request values. Emit `READ_ONLY` for this case too while retaining any nullable-specific mapping.</violation>
<violation number="3" location="modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache:259">
P2: When the Spring generator runs with `withXml` enabled, this change silently removes `@JacksonXmlProperty` and `@JacksonXmlElementWrapper` from every generated setter. The original setter used the shared `{{>jackson_annotations}}` partial, which emits those XML annotations under `{{#withXml}}`; the new inline `@JsonProperty(...)` only emits the JSON annotation, so XML deserialization mapping (element name/wrapper) on setters is lost while the getter keeps them. Preserve the partial and add the `access = JsonProperty.Access.READ_ONLY` parameter to `jackson_annotations.mustache` instead of replacing the partial inline.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @Deprecated | ||
| {{/deprecated}} | ||
| {{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{>jackson_annotations}}{{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) { | ||
| {{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}} @JsonProperty(value = "{{baseName}}"{{#isReadOnly}}, access = JsonProperty.Access.READ_ONLY{{/isReadOnly}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) { |
There was a problem hiding this comment.
P1: When Lombok generates the setter through lombok.Setter or lombok.Data, this line is not rendered and no Lombok-side annotation sets READ_ONLY, so read-only properties remain writable by Jackson. Apply the access setting to the Lombok-generated accessor or field path as well.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache, line 259:
<comment>When Lombok generates the setter through `lombok.Setter` or `lombok.Data`, this line is not rendered and no Lombok-side annotation sets `READ_ONLY`, so read-only properties remain writable by Jackson. Apply the access setting to the Lombok-generated accessor or field path as well.</comment>
<file context>
@@ -256,7 +256,7 @@ public {{>sealed}}class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}
@Deprecated
{{/deprecated}}
-{{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{>jackson_annotations}}{{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) {
+{{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}} @JsonProperty(value = "{{baseName}}"{{#isReadOnly}}, access = JsonProperty.Access.READ_ONLY{{/isReadOnly}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) {
this.{{name}} = {{name}};
}
</file context>
| @Deprecated | ||
| {{/deprecated}} | ||
| {{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{>jackson_annotations}}{{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) { | ||
| {{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}} @JsonProperty(value = "{{baseName}}"{{#isReadOnly}}, access = JsonProperty.Access.READ_ONLY{{/isReadOnly}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) { |
There was a problem hiding this comment.
P1: When openApiNullable is enabled for a non-required nullable read-only property, this guard suppresses the new annotation, so Jackson still auto-detects the setter and accepts request values. Emit READ_ONLY for this case too while retaining any nullable-specific mapping.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache, line 259:
<comment>When `openApiNullable` is enabled for a non-required nullable read-only property, this guard suppresses the new annotation, so Jackson still auto-detects the setter and accepts request values. Emit `READ_ONLY` for this case too while retaining any nullable-specific mapping.</comment>
<file context>
@@ -256,7 +256,7 @@ public {{>sealed}}class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}
@Deprecated
{{/deprecated}}
-{{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{>jackson_annotations}}{{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) {
+{{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}} @JsonProperty(value = "{{baseName}}"{{#isReadOnly}}, access = JsonProperty.Access.READ_ONLY{{/isReadOnly}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) {
this.{{name}} = {{name}};
}
</file context>
| @Deprecated | ||
| {{/deprecated}} | ||
| {{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{>jackson_annotations}}{{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) { | ||
| {{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}} @JsonProperty(value = "{{baseName}}"{{#isReadOnly}}, access = JsonProperty.Access.READ_ONLY{{/isReadOnly}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) { |
There was a problem hiding this comment.
P2: When the Spring generator runs with withXml enabled, this change silently removes @JacksonXmlProperty and @JacksonXmlElementWrapper from every generated setter. The original setter used the shared {{>jackson_annotations}} partial, which emits those XML annotations under {{#withXml}}; the new inline @JsonProperty(...) only emits the JSON annotation, so XML deserialization mapping (element name/wrapper) on setters is lost while the getter keeps them. Preserve the partial and add the access = JsonProperty.Access.READ_ONLY parameter to jackson_annotations.mustache instead of replacing the partial inline.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache, line 259:
<comment>When the Spring generator runs with `withXml` enabled, this change silently removes `@JacksonXmlProperty` and `@JacksonXmlElementWrapper` from every generated setter. The original setter used the shared `{{>jackson_annotations}}` partial, which emits those XML annotations under `{{#withXml}}`; the new inline `@JsonProperty(...)` only emits the JSON annotation, so XML deserialization mapping (element name/wrapper) on setters is lost while the getter keeps them. Preserve the partial and add the `access = JsonProperty.Access.READ_ONLY` parameter to `jackson_annotations.mustache` instead of replacing the partial inline.</comment>
<file context>
@@ -256,7 +256,7 @@ public {{>sealed}}class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}
@Deprecated
{{/deprecated}}
-{{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{>jackson_annotations}}{{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) {
+{{#jackson}}{{^vendorExtensions.x-is-jackson-optional-nullable}} @JsonProperty(value = "{{baseName}}"{{#isReadOnly}}, access = JsonProperty.Access.READ_ONLY{{/isReadOnly}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{/jackson}} public void {{setter}}({{>nullableAnnotation}}{{>nullableDataType}} {{name}}) {
this.{{name}} = {{name}};
}
</file context>
bdbded2 to
c7e18e0
Compare
|
@cubic review |
@antoineclech-arkea I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
c7e18e0 to
f9ecd99
Compare
|
@cubic review |
@antoineclech-arkea I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 existing issue remains and 1 new issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/JavaSpring/jackson_annotations.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/JavaSpring/jackson_annotations.mustache:1">
P2: This template change alters generated output for every readOnly property in existing Spring samples, but no samples were regenerated. The committed samples (e.g. samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java, which currently emits `@JsonProperty("bar")` for its readOnly fields) will now produce `@JsonProperty(value = "bar", access = JsonProperty.Access.READ_ONLY)`, so the sample-verification CI will fail. Regenerate the affected Spring samples (./bin/generate-samples.sh for the spring configs) as part of this PR.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
| @@ -1,4 +1,4 @@ | |||
| @JsonProperty("{{baseName}}") | |||
| @JsonProperty({{#isReadOnly}}value = {{/isReadOnly}}"{{baseName}}"{{#isReadOnly}}, access = JsonProperty.Access.READ_ONLY{{/isReadOnly}}) | |||
There was a problem hiding this comment.
P2: This template change alters generated output for every readOnly property in existing Spring samples, but no samples were regenerated. The committed samples (e.g. samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java, which currently emits @JsonProperty("bar") for its readOnly fields) will now produce @JsonProperty(value = "bar", access = JsonProperty.Access.READ_ONLY), so the sample-verification CI will fail. Regenerate the affected Spring samples (./bin/generate-samples.sh for the spring configs) as part of this PR.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/JavaSpring/jackson_annotations.mustache, line 1:
<comment>This template change alters generated output for every readOnly property in existing Spring samples, but no samples were regenerated. The committed samples (e.g. samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java, which currently emits `@JsonProperty("bar")` for its readOnly fields) will now produce `@JsonProperty(value = "bar", access = JsonProperty.Access.READ_ONLY)`, so the sample-verification CI will fail. Regenerate the affected Spring samples (./bin/generate-samples.sh for the spring configs) as part of this PR.</comment>
<file context>
@@ -1,4 +1,4 @@
- @JsonProperty("{{baseName}}")
+ @JsonProperty({{#isReadOnly}}value = {{/isReadOnly}}"{{baseName}}"{{#isReadOnly}}, access = JsonProperty.Access.READ_ONLY{{/isReadOnly}})
{{#withXml}}
@JacksonXmlProperty(localName = "{{items.xmlName}}{{^items.xmlName}}{{xmlName}}{{^xmlName}}{{baseName}}{{/xmlName}}{{/items.xmlName}}"{{#isXmlAttribute}}, isAttribute = true{{/isXmlAttribute}}{{#xmlNamespace}}, namespace = "{{.}}"{{/xmlNamespace}})
</file context>
f9ecd99 to
546d04d
Compare
|
@cubic review |
@antoineclech-arkea I have started the AI code review. It will take a few minutes to complete. |
Fixes OpenAPITools#20612 Fixes generation of read-only properties in Spring models to include @JsonProperty(access = JsonProperty.Access.READ_ONLY) on setters. This prevents Jackson from accepting read-only properties during deserialization, enforcing the OpenAPI contract. Changes: - Modified jackson_annotations.mustache to emit access=READ_ONLY for readOnly properties - Added unit test with readOnly property test spec - Regenerated affected Spring samples
66b33e4 to
f8f08c1
Compare
|
@cubic review |
@antoineclech-arkea I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/petstore/spring-http-interface-noResponseEntity/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java">
<violation number="1" location="samples/client/petstore/spring-http-interface-noResponseEntity/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java:42">
P2: Jackson `READ_ONLY` means serialize but never deserialize. That is the right contract for a server model (don't accept readOnly values in requests), but this shared template is also used by client-side Spring generators — this very sample is `samples/client/petstore/spring-http-interface-noResponseEntity`. For a client, a readOnly value arrives in the server response and must be deserialized; with `READ_ONLY` on the getter/setter Jackson now silently skips it, so `getBar()`/`getFoo()` return null for values the server actually sent. This is a real behavior change for Spring clients that the PR does not account for (it only frames the server benefit). Confirm client deserialization of readOnly values is not needed, or gate the `READ_ONLY` emission to server-side generation.</violation>
</file>
<file name="samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java">
<violation number="1" location="samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java:42">
P2: For these Spring CLIENT models the shared READ_ONLY setter change prevents Jackson from deserializing readOnly fields, so the client silently receives null for server-generated values (e.g. IDs) in API responses. OpenAPI readOnly means 'may be sent in responses but should not be sent in requests', so a client must still be able to read these fields. Gate the READ_ONLY emission to server-side generators only, or apply it only where the model is used for request deserialization.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| */ | ||
|
|
||
| @JsonProperty("bar") | ||
| @JsonProperty(value = "bar", access = JsonProperty.Access.READ_ONLY) |
There was a problem hiding this comment.
P2: Jackson READ_ONLY means serialize but never deserialize. That is the right contract for a server model (don't accept readOnly values in requests), but this shared template is also used by client-side Spring generators — this very sample is samples/client/petstore/spring-http-interface-noResponseEntity. For a client, a readOnly value arrives in the server response and must be deserialized; with READ_ONLY on the getter/setter Jackson now silently skips it, so getBar()/getFoo() return null for values the server actually sent. This is a real behavior change for Spring clients that the PR does not account for (it only frames the server benefit). Confirm client deserialization of readOnly values is not needed, or gate the READ_ONLY emission to server-side generation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/spring-http-interface-noResponseEntity/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java, line 42:
<comment>Jackson `READ_ONLY` means serialize but never deserialize. That is the right contract for a server model (don't accept readOnly values in requests), but this shared template is also used by client-side Spring generators — this very sample is `samples/client/petstore/spring-http-interface-noResponseEntity`. For a client, a readOnly value arrives in the server response and must be deserialized; with `READ_ONLY` on the getter/setter Jackson now silently skips it, so `getBar()`/`getFoo()` return null for values the server actually sent. This is a real behavior change for Spring clients that the PR does not account for (it only frames the server benefit). Confirm client deserialization of readOnly values is not needed, or gate the `READ_ONLY` emission to server-side generation.</comment>
<file context>
@@ -39,12 +39,12 @@ public HasOnlyReadOnlyDto bar(@Nullable String bar) {
*/
- @JsonProperty("bar")
+ @JsonProperty(value = "bar", access = JsonProperty.Access.READ_ONLY)
public @Nullable String getBar() {
return bar;
</file context>
| */ | ||
|
|
||
| @JsonProperty("bar") | ||
| @JsonProperty(value = "bar", access = JsonProperty.Access.READ_ONLY) |
There was a problem hiding this comment.
P2: For these Spring CLIENT models the shared READ_ONLY setter change prevents Jackson from deserializing readOnly fields, so the client silently receives null for server-generated values (e.g. IDs) in API responses. OpenAPI readOnly means 'may be sent in responses but should not be sent in requests', so a client must still be able to read these fields. Gate the READ_ONLY emission to server-side generators only, or apply it only where the model is used for request deserialization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/spring-http-interface/src/main/java/org/openapitools/model/HasOnlyReadOnlyDto.java, line 42:
<comment>For these Spring CLIENT models the shared READ_ONLY setter change prevents Jackson from deserializing readOnly fields, so the client silently receives null for server-generated values (e.g. IDs) in API responses. OpenAPI readOnly means 'may be sent in responses but should not be sent in requests', so a client must still be able to read these fields. Gate the READ_ONLY emission to server-side generators only, or apply it only where the model is used for request deserialization.</comment>
<file context>
@@ -39,12 +39,12 @@ public HasOnlyReadOnlyDto bar(@Nullable String bar) {
*/
- @JsonProperty("bar")
+ @JsonProperty(value = "bar", access = JsonProperty.Access.READ_ONLY)
public @Nullable String getBar() {
return bar;
</file context>
Fixes #20612
Fixes generation of read-only properties in Spring models to include @JsonProperty(access = JsonProperty.Access.READ_ONLY) on setters.
This prevents Jackson from accepting read-only properties during deserialization, enforcing the OpenAPI contract.
Problem
Issue #20612: The OpenAPI Generator doesn't properly propagate readOnly property information. Generated setters for readOnly properties are treated as fully writable by Jackson,
violating the OpenAPI 3.0.0 specification.
Before this fix: