feat: add MCExcludeFromSchema annotation and migrate statusCode t… #2388
Conversation
…o `status` in YAML tutorials and schemas - Introduced `MCExcludeFromSchema` annotation to allow exclusion of fields during schema generation. - Replaced `statusCode` with `status` across YAML tutorial and schema files for consistency. - Updated `JsonSchemaGenerator` and `AttributeInfo` to support exclusion logic based on the new annotation.
| Caution Review failedThe pull request is closed. WalkthroughAdds MCAttribute.excludeFromJson, exposes AttributeInfo.excludedFromJsonSchema(), makes JsonSchemaGenerator and HelpReference honor the exclusion and deprecation flags, renames ReturnInterceptor's internal statusCode → status with new accessors and deprecates the old setter, and updates many YAML examples to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| This pull request needs "/ok-to-test" from an authorized committer. |
annot/src/main/java/com/predic8/membrane/annot/MCExcludeFromSchema.java Outdated Show resolved Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
annot/src/main/java/com/predic8/membrane/annot/MCExcludeFromSchema.java (1)
22-37: Align Javadoc with@Target(METHOD)(fields vs. methods)The Javadoc states the annotation “can be applied to fields”, but
@Target(METHOD)restricts usage to methods only. Either update the Javadoc to mention methods (e.g., setters) or expand the target to includeFIELDif field usage is intended.core/src/main/java/com/predic8/membrane/core/interceptor/flow/ReturnInterceptor.java (1)
35-43: Update high-level documentation to referencestatusas the primary optionThe class Javadoc still states “The options statusCode and contentType will overwrite the values from the messages.” while the new configuration surface now uses
statusas the primary property (withstatusCodekept only for backward compatibility). To avoid confusing users (and schema-driven tooling), it would be good to:
- Mention
statusas the main option in the Javadoc (e.g., “status (preferred) and legacy statusCode…”), and- Keep
setStatus’s Javadoc as the canonical description/default/example.This keeps the docs aligned with the new schema and the renamed property.
Also applies to: 141-148
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
annot/src/main/java/com/predic8/membrane/annot/MCExcludeFromSchema.java(1 hunks)annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java(1 hunks)annot/src/main/java/com/predic8/membrane/annot/model/AttributeInfo.java(1 hunks)core/src/main/java/com/predic8/membrane/core/interceptor/flow/ReturnInterceptor.java(5 hunks)core/src/test/java/com/predic8/membrane/core/config/spring/k8s/EnvelopeTest.java(1 hunks)distribution/examples/extending-membrane/error-handling/custom-error-messages/apis.yaml(2 hunks)distribution/examples/extending-membrane/if/apis.yaml(1 hunks)distribution/examples/monitoring-tracing/prometheus/apis.yaml(1 hunks)distribution/examples/openapi/validation-security/apis.yaml(1 hunks)distribution/examples/openapi/validation-simple/apis.yaml(1 hunks)distribution/examples/openapi/validation/apis.yaml(2 hunks)distribution/examples/orchestration/call-authentication/apis.yaml(1 hunks)distribution/examples/routing-traffic/shadowing/apis.yaml(3 hunks)distribution/examples/security/ssl-tls/api-with-tls-pem/apis.yaml(1 hunks)distribution/examples/security/ssl-tls/api-with-tls-pkcs12/apis.yaml(1 hunks)distribution/examples/templating/json/apis.yaml(1 hunks)distribution/examples/templating/xml/apis.yaml(2 hunks)distribution/tutorials/advanced/10-PathParameters.yaml(1 hunks)distribution/tutorials/advanced/60-if.yaml(1 hunks)distribution/tutorials/advanced/70-Scripting-Groovy.yaml(1 hunks)distribution/tutorials/getting-started/50-Short-Circuit.yaml(1 hunks)distribution/tutorials/getting-started/60-SetHeader.yaml(1 hunks)distribution/tutorials/getting-started/70-Template.yaml(1 hunks)distribution/tutorials/json/20-JSONPath.yaml(1 hunks)distribution/tutorials/json/60-Json-Transformation.yaml(1 hunks)distribution/tutorials/xml/10-JSON-to-XML.yaml(1 hunks)distribution/tutorials/xml/15-XML-to-JSON.yaml(1 hunks)distribution/tutorials/xml/20-XPath.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-23T13:04:11.388Z
Learnt from: predic8 Repo: membrane/api-gateway PR: 2240 File: annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java:210-215 Timestamp: 2025-10-23T13:04:11.388Z Learning: In JsonSchemaGenerator.java, when allowForeign is true for non-list child elements, use a property named "ref" (not "$ref") with type "string" to provide IDE completion hints for Spring bean references while avoiding JSON Schema keyword collision. Applied to files:
annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java
🔇 Additional comments (28)
distribution/examples/security/ssl-tls/api-with-tls-pem/apis.yaml (1)
32-33: statusCode → status field rename looks good.The return block correctly uses the new
statusfield name with the HTTP 200 status code preserved.distribution/tutorials/xml/20-XPath.yaml (1)
52-53: statusCode → status field rename looks good.The return block correctly uses the new
statusfield name with HTTP 200 status code preserved.distribution/examples/security/ssl-tls/api-with-tls-pkcs12/apis.yaml (1)
33-34: statusCode → status field rename looks good.The return block correctly uses the new
statusfield name with HTTP 200 status code preserved.distribution/examples/monitoring-tracing/prometheus/apis.yaml (1)
13-14: statusCode → status field renames look good.All three return blocks correctly use the new
statusfield name with HTTP status codes (200, 404, 500) preserved.Also applies to: 21-22, 29-30
distribution/tutorials/getting-started/70-Template.yaml (1)
27-28: statusCode → status field rename looks good.The return block correctly uses the new
statusfield name with HTTP 200 status code preserved.distribution/examples/extending-membrane/error-handling/custom-error-messages/apis.yaml (1)
14-14: Verify backward compatibility of ${statusCode} template variable references.This file contains multiple references to
${statusCode}in template variables (lines 21, 36, 56) and conditional expressions (lines 14, 45), but per the PR summary,ReturnInterceptorrenamed the internal field fromstatusCodetostatus. Confirm that the expression evaluation engine can still resolve${statusCode}via deprecated backward-compatible accessors, or these references will break at runtime.Also applies to: 21-21, 36-36, 45-45, 56-56
distribution/tutorials/xml/10-JSON-to-XML.yaml (1)
21-22: statusCode → status field rename looks good.The return block correctly uses the new
statusfield name with HTTP 200 status code preserved.distribution/examples/templating/xml/apis.yaml (1)
14-15: statusCode → status field renames look good.Both return blocks correctly use the new
statusfield name with HTTP 200 status code preserved.Also applies to: 29-30
distribution/tutorials/advanced/60-if.yaml (1)
22-22: Return block status field rename looks correctUsing
status: 400in thereturninterceptor is consistent with the new convention and keeps behavior identical.distribution/examples/openapi/validation-simple/apis.yaml (1)
25-25: Consistent status field in mock backendThe switch to
status: 201in the mock proxy’sreturnblock is consistent with the new response schema and keeps semantics unchanged.distribution/examples/templating/json/apis.yaml (1)
18-18: Short‑circuit status rename is in line with new API
status: 200in thereturnblock matches the updated ReturnInterceptor configuration and preserves the 200 OK behavior.distribution/tutorials/json/60-Json-Transformation.yaml (1)
54-54: Tutorial return status field updated appropriatelyUsing
status: 200in the JSON transformation tutorial aligns the sample with the new configuration model without altering behavior.core/src/test/java/com/predic8/membrane/core/config/spring/k8s/EnvelopeTest.java (1)
85-87: Inline YAML updated to new status fieldThe embedded YAML in
routerConfConfignow usesstatus: 200, consistent with the updated samples and ReturnInterceptor schema; no test logic impact beyond keeping things in sync.distribution/tutorials/advanced/70-Scripting-Groovy.yaml (1)
20-20: Groovy scripting tutorial return status updatedThe
returnblock now correctly usesstatus: 200, matching the new configuration field name while keeping the example behavior identical.distribution/examples/openapi/validation/apis.yaml (1)
33-33: Validation example mocks now use standardized status fieldBoth
returnblocks now specifystatus(201 and 200) instead ofstatusCode, which is consistent with the new ReturnInterceptor schema and other OpenAPI validation examples.Also applies to: 56-56
distribution/tutorials/getting-started/50-Short-Circuit.yaml (1)
31-31: Getting-started short‑circuit uses new status fieldThe tutorial’s
returnblock now usesstatus: 200, which aligns the introductory docs with the current configuration surface.distribution/tutorials/json/20-JSONPath.yaml (1)
54-54: Status field rename is consistent and correct
status: 200is syntactically valid and consistent with the newstatusconvention; no further changes needed here.distribution/tutorials/advanced/10-PathParameters.yaml (1)
25-26: Return status rename looks goodUsing
status: 200under thereturnstep matches the new API and keeps behavior unchanged.distribution/examples/extending-membrane/if/apis.yaml (1)
71-72: Return block migration tostatusis consistentThe final
returnnow usesstatus: 302, which aligns with the new naming while leaving existingstatusCodetests/snippets intact for backward compatibility.distribution/examples/routing-traffic/shadowing/apis.yaml (2)
37-39: Other status renames are correct
status: 201is syntactically valid and consistent with the new convention.
46-47: Third return status rename also looks good
status: 202is correct and keeps the original behavior.distribution/tutorials/xml/15-XML-to-JSON.yaml (1)
20-21: XML-to-JSON tutorial return uses new status field correctly
status: 200under thereturnblock is correct and matches the updated schema.distribution/examples/orchestration/call-authentication/apis.yaml (1)
48-49: Auth error response status rename is fineSwitching to
status: 401preserves semantics and aligns with the new status field naming.annot/src/main/java/com/predic8/membrane/annot/model/AttributeInfo.java (1)
199-222: Exclusion helpers are straightforward and correctBoth
isExcludedFromJsonSchema()andisExcludedFromXsdSchema()correctly consultMCExcludeFromSchemaon the setter element and return the correspondingjson/xsdflags; this integrates cleanly with existinggetE()-based logic.annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java (1)
174-187: JSON schema exclusion guard is correctly placed and non-invasiveUsing
ai.isExcludedFromJsonSchema()as an early return insideprocessMCAttributescleanly ensures excluded attributes never reachcreateProperty, while preserving the existing special-case foridon top-level elements. This integrates well with the newMCExcludeFromSchemasemantics and does not interfere with priorallowForeign/refbehavior.
Based on learnings, existing allowForeign JSON schema conventions remain unchanged.core/src/main/java/com/predic8/membrane/core/interceptor/flow/ReturnInterceptor.java (1)
54-96: Status field rename and usage look consistent and backward compatibleSwitching from
statusCodetostatus(field, response builder, and short description) is internally consistent:getOrCreateResponseandcreateResponseFromRequestboth use the newstatusfield, and the short description now formats that same value. Existing flows that relied on the previousstatusCodefield semantics should continue to behave the same, while the newstatusaccessors provide a clearer public API.Also applies to: 115-116
distribution/examples/openapi/validation-security/apis.yaml (1)
27-33: YAML example now matches the newstatuspropertyRenaming
statusCodetostatusunder thereturnblock is consistent with the updatedReturnInterceptorAPI and schema direction, and keeps the example aligned with the language-server schema (statusbeing the advertised field).distribution/tutorials/getting-started/60-SetHeader.yaml (1)
14-25: Tutorial return block correctly migrated tostatusUsing
status: 200in thereturnstep is in line with the new configuration name and will work better with updated JSON schema and editor tooling, without affecting runtime behavior.
core/src/main/java/com/predic8/membrane/core/interceptor/flow/ReturnInterceptor.java Show resolved Hide resolved
…on logic to `MCAttribute`
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
annot/src/main/java/com/predic8/membrane/annot/MCAttribute.java(1 hunks)annot/src/main/java/com/predic8/membrane/annot/model/AttributeInfo.java(1 hunks)core/src/main/java/com/predic8/membrane/core/interceptor/flow/ReturnInterceptor.java(6 hunks)distribution/examples/routing-traffic/shadowing/apis.yaml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- distribution/examples/routing-traffic/shadowing/apis.yaml
- core/src/main/java/com/predic8/membrane/core/interceptor/flow/ReturnInterceptor.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: predic8 Repo: membrane/api-gateway PR: 2240 File: annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java:210-215 Timestamp: 2025-10-23T13:04:11.388Z Learning: In JsonSchemaGenerator.java, when allowForeign is true for non-list child elements, use a property named "ref" (not "$ref") with type "string" to provide IDE completion hints for Spring bean references while avoiding JSON Schema keyword collision. ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (1)
annot/src/main/java/com/predic8/membrane/annot/MCAttribute.java (1)
22-32: New exclusion flags and Javadoc look consistent with intended behaviorThe added
excludeFromJson()/excludeFromXsd()members and the updated class‑level Javadoc line up with how these attributes will be used in schema generation; no functional issues from this change.Also applies to: 38-39
…andling in `AttributeInfo` and `JsonSchemaGenerator`
| /ok-to-test |
| @coderabbitai review |
✅ Actions performedReview triggered.
|
core/src/main/java/com/predic8/membrane/core/interceptor/flow/ReturnInterceptor.java Show resolved Hide resolved
annot/src/main/java/com/predic8/membrane/annot/model/AttributeInfo.java Outdated Show resolved Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
annot/src/main/java/com/predic8/membrane/annot/model/AttributeInfo.java (1)
197-203: Consider referencing the annotation element in Javadoc.The implementation correctly uses the cached
annotationfield. The Javadoc could optionally referenceMCAttribute#excludeFromJson()for better discoverability.Example enhancement:
/** * Determines whether the current field or method should be excluded from the JSON schema during schema generation. + * This is based on the {@link MCAttribute#excludeFromJson()} annotation element. * @return {@code true} if the field or method is excluded from the JSON schema; {@code false} otherwise. */ public boolean excludedFromJsonSchema() { return annotation != null && annotation.excludeFromJson(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
annot/src/main/java/com/predic8/membrane/annot/MCAttribute.java(1 hunks)annot/src/main/java/com/predic8/membrane/annot/generator/HelpReference.java(3 hunks)annot/src/main/java/com/predic8/membrane/annot/model/AbstractJavadocedInfo.java(3 hunks)annot/src/main/java/com/predic8/membrane/annot/model/AttributeInfo.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-23T13:04:11.388Z
Learnt from: predic8 Repo: membrane/api-gateway PR: 2240 File: annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java:210-215 Timestamp: 2025-10-23T13:04:11.388Z Learning: In JsonSchemaGenerator.java, when allowForeign is true for non-list child elements, use a property named "ref" (not "$ref") with type "string" to provide IDE completion hints for Spring bean references while avoiding JSON Schema keyword collision. Applied to files:
annot/src/main/java/com/predic8/membrane/annot/model/AttributeInfo.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Automated tests
- GitHub Check: Analyze (java)
🔇 Additional comments (6)
annot/src/main/java/com/predic8/membrane/annot/MCAttribute.java (1)
22-37: LGTM! Clean annotation enhancement.The new
excludeFromJsonelement and enhanced Javadoc are well-structured. The default value offalseensures backward compatibility, and the inline comment clarifies the purpose.annot/src/main/java/com/predic8/membrane/annot/generator/HelpReference.java (3)
126-126: LGTM! Consistent metadata exposure.The addition of the
deprecatedattribute for element nodes properly exposes deprecation status in the generated XML reference.
202-202: LGTM! Consistent metadata exposure.The addition of the
deprecatedattribute for child nodes properly exposes deprecation status in the generated XML reference.
233-234: LGTM! Complete metadata exposure for attributes.Both the
excludeFromJsonanddeprecatedattributes are properly added to attribute nodes, providing comprehensive metadata in the generated XML reference.annot/src/main/java/com/predic8/membrane/annot/model/AbstractJavadocedInfo.java (2)
29-29: LGTM! Well-defined constant.The constant follows Java naming conventions and provides a clear, reusable reference to the
@Deprecatedannotation's fully qualified name.
56-61: LGTM! Correct deprecation detection.The implementation correctly iterates through annotation mirrors and checks for the
@Deprecatedannotation using the fully qualified name comparison.
…o
statusin YAML tutorials and schemasMCExcludeFromSchemaannotation to allow exclusion of fields during schema generation.statusCodewithstatusacross YAML tutorial and schema files for consistency.JsonSchemaGeneratorandAttributeInfoto support exclusion logic based on the new annotation.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.