Enhance schema generation to always reference flowParser for interceptor list#2815
Enhance schema generation to always reference flowParser for interceptor list#2815
Conversation
📝 WalkthroughWalkthroughRefactors flowParser detection and attachment logic in JSON schema generation. Instead of hard-coded "flow" property names, the code now uses type-based checks to identify when a flowParser should be generated, and dynamically routes flowParser references to the correct property names within list or object schemas. Changes
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
|
/ok-to-test |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java (1)
119-135:⚠️ Potential issue | 🟠 MajorEnsure
flowParserdefinition is always created when referenced from anoEnvelopeelement.When a
noEnvelopeelement with an Interceptor-typed child callsshouldGenerateFlowParserType(), it returns a reference to#/$defs/flowParser(line 132) but never callsprocessMCChilds, which is the only code path that creates the definition viaaddFlowParserRef. While the framework's existing elements (Proxy, Transport) ensure the definition is created before noEnvelope elements are processed, this relies on an implicit assumption about which elements exist in the model. If a minimal schema contains only noEnvelope elements with Interceptor children, the definition would never be created.Suggested fix: Eagerly create the flowParser definition
In the noEnvelope block when
shouldGenerateFlowParserType(child)is true, ensure the definition exists:if (shouldGenerateFlowParserType(child)) { + if (!flowDefCreated) { + var sos = getSchemaObjects(m, main, child); + schema.definition(array("flowParser").items(anyOf(sos))); + flowDefCreated = true; + } return ref(parserName).ref("#/$defs/flowParser"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java` around lines 119 - 135, The noEnvelope branch returns a reference to "#/$defs/flowParser" when shouldGenerateFlowParserType(child) is true but never creates that definition; update the block in JsonSchemaGenerator (around elementInfo.noEnvelope handling) to eagerly ensure the flowParser definition is added before returning the ref—either by invoking the same code path that creates it (call processMCChilds or addFlowParserRef with the child and appropriate SchemaArray) or explicitly calling addFlowParserRef(parserName/main) so flowParser is defined whenever shouldGenerateFlowParserType(child) is true prior to returning ref(parserName).ref("#/$defs/flowParser").
🧹 Nitpick comments (1)
annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java (1)
288-293: Minor:getSchemaObjectsis computed on every Interceptor-typed list child, but only the first result is used.At line 291,
getSchemaObjects(m, main, cei)is called each time, butaddFlowParserRefdiscards it whenflowDefCreatedis alreadytrue(line 396). Consider guarding withflowDefCreated:Suggested optimization
if (shouldGenerateFlowParserType(cei)) { - processList(i, so, cei, getSchemaObjects(m, main, cei)); + var sos = flowDefCreated ? null : getSchemaObjects(m, main, cei); + processList(i, so, cei, sos); continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java` around lines 288 - 293, The call to getSchemaObjects(m, main, cei) is being recomputed for every Interceptor-typed child even after flowDefCreated is true and its result is ignored; to fix, introduce a local variable (e.g., schemaObjects) that is lazily initialized and only assigned by calling getSchemaObjects(m, main, cei) when flowDefCreated is false, then pass that variable into processList(i, so, cei, schemaObjects) instead of calling getSchemaObjects each time; reference functions/classes: getSchemaObjects, shouldGenerateFlowParserType, processList, addFlowParserRef, and the flowDefCreated flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java`:
- Around line 119-135: The noEnvelope branch returns a reference to
"#/$defs/flowParser" when shouldGenerateFlowParserType(child) is true but never
creates that definition; update the block in JsonSchemaGenerator (around
elementInfo.noEnvelope handling) to eagerly ensure the flowParser definition is
added before returning the ref—either by invoking the same code path that
creates it (call processMCChilds or addFlowParserRef with the child and
appropriate SchemaArray) or explicitly calling addFlowParserRef(parserName/main)
so flowParser is defined whenever shouldGenerateFlowParserType(child) is true
prior to returning ref(parserName).ref("#/$defs/flowParser").
---
Nitpick comments:
In
`@annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java`:
- Around line 288-293: The call to getSchemaObjects(m, main, cei) is being
recomputed for every Interceptor-typed child even after flowDefCreated is true
and its result is ignored; to fix, introduce a local variable (e.g.,
schemaObjects) that is lazily initialized and only assigned by calling
getSchemaObjects(m, main, cei) when flowDefCreated is false, then pass that
variable into processList(i, so, cei, schemaObjects) instead of calling
getSchemaObjects each time; reference functions/classes: getSchemaObjects,
shouldGenerateFlowParserType, processList, addFlowParserRef, and the
flowDefCreated flag.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java
|
This pull request needs "/ok-to-test" from an authorized committer. |
2 participants
Summary by CodeRabbit