-
Notifications
You must be signed in to change notification settings - Fork 149
Simplify jsonschema example #2504
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
Conversation
WalkthroughThis PR reorganizes JSON Schema validation examples by consolidating separate example configurations, moving schema files into a unified Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
distribution/examples/validation/json-schema/proxies-old.xml (1)
1-39: Remove proxies-old.xml; it is deprecated in favor ofapis.yaml.The codebase has migrated to YAML-based configuration (
apis.yaml), and this file is not referenced anywhere. Theapis.yamlin this same directory contains identical configuration. Per the FilterExamples.java build logic, XML configuration files are removed when a YAML equivalent exists. The README.md also directs users toapis.yaml, not this file. Remove it to avoid confusion.
🧹 Nitpick comments (1)
distribution/examples/validation/json-schema/README.md (1)
31-32: Consider wrapping URLs in angle brackets.The bare URLs trigger a markdown linting warning. While functional, wrapping them in angle brackets improves markdown formatting.
🔎 Proposed fix
-See: -- JSON Schema: https://json-schema.org/ -- Membrane validator reference: https://membrane-api.io/docs/current/validator.html +See: +- JSON Schema: <https://json-schema.org/> +- Membrane validator reference: <https://membrane-api.io/docs/current/validator.html>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
distribution/examples/validation/json-schema/README.mddistribution/examples/validation/json-schema/apis.yamldistribution/examples/validation/json-schema/bad2001.jsondistribution/examples/validation/json-schema/good2001.jsondistribution/examples/validation/json-schema/proxies-old.xmldistribution/examples/validation/json-schema/schema-mappings/README.mddistribution/examples/validation/json-schema/schema-mappings/bad2000.jsondistribution/examples/validation/json-schema/schema-mappings/bad2001.jsondistribution/examples/validation/json-schema/schema-mappings/good2000.jsondistribution/examples/validation/json-schema/schema-mappings/good2001.jsondistribution/examples/validation/json-schema/schema-mappings/membrane.cmddistribution/examples/validation/json-schema/schema-mappings/membrane.shdistribution/examples/validation/json-schema/schema-mappings/proxies.xmldistribution/examples/validation/json-schema/schema-mappings/schemas/base-param.jsondistribution/examples/validation/json-schema/schema-mappings/schemas/schema2000.jsondistribution/examples/validation/json-schema/schema2001.jsondistribution/examples/validation/json-schema/schemas/base.jsondistribution/examples/validation/json-schema/schemas/meta.jsondistribution/examples/validation/json-schema/schemas/schema2000.jsondistribution/examples/validation/json-schema/schemas/schema2001.jsondistribution/src/test/java/com/predic8/membrane/examples/withoutinternet/validation/JSONSchemaMappingsExampleTest.javadistribution/src/test/java/com/predic8/membrane/examples/withoutinternet/validation/JSONSchemaValidationExampleTest.java
💤 Files with no reviewable changes (12)
- distribution/examples/validation/json-schema/schema-mappings/membrane.sh
- distribution/examples/validation/json-schema/schema-mappings/good2001.json
- distribution/examples/validation/json-schema/schema-mappings/good2000.json
- distribution/examples/validation/json-schema/schema-mappings/schemas/schema2000.json
- distribution/examples/validation/json-schema/schema-mappings/schemas/base-param.json
- distribution/examples/validation/json-schema/schema-mappings/README.md
- distribution/examples/validation/json-schema/schema2001.json
- distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/validation/JSONSchemaMappingsExampleTest.java
- distribution/examples/validation/json-schema/schema-mappings/bad2000.json
- distribution/examples/validation/json-schema/schema-mappings/membrane.cmd
- distribution/examples/validation/json-schema/schema-mappings/proxies.xml
- distribution/examples/validation/json-schema/schema-mappings/bad2001.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-18T12:03:09.931Z
Learnt from: rrayst
Repo: membrane/api-gateway PR: 1906
File: core/src/test/java/com/predic8/membrane/core/openapi/validators/JsonSchemaTestSuiteTests.java:0-0
Timestamp: 2025-06-18T12:03:09.931Z
Learning: The mediaType method in the Request class throws jakarta.mail.internet.ParseException, not java.text.ParseException, making the jakarta.mail.internet.ParseException import necessary and correct in JsonSchemaTestSuiteTests.java.
Applied to files:
distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/validation/JSONSchemaValidationExampleTest.java
🪛 markdownlint-cli2 (0.18.1)
distribution/examples/validation/json-schema/README.md
31-31: Bare URL used
(MD034, no-bare-urls)
32-32: Bare URL used
(MD034, no-bare-urls)
⏰ 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 (7)
distribution/examples/validation/json-schema/schemas/schema2001.json (1)
12-15: LGTM! Schema changes align with example restructuring.The increased
minItemsconstraint and updated URN reference tourn:app:base_defalign with the new base.json schema and demonstrate validation requirements effectively. The good2001.json example has 2 params (passes), while bad2001.json has 1 param (fails), which correctly demonstrates the validation behavior.distribution/examples/validation/json-schema/bad2001.json (1)
1-12: LGTM! Multiple validation failures are intentional.This "bad" example correctly demonstrates validation failures:
paramsarray has only 1 item (violatesminItems: 2)meta.sourceis empty string (violatesminLength: 1)meta.unexpectedfield present (violatesadditionalProperties: false)These intentional violations effectively demonstrate schema validation behavior.
distribution/examples/validation/json-schema/schemas/meta.json (1)
7-17: LGTM! Validation relaxation is intentional.Making
requestIdoptional and removing itsminLengthconstraint provides flexibility in the example. The schema still enforces:
- Required
sourcefield withminLength: 1- No additional properties (
additionalProperties: false)This aligns with the example data where good2001.json includes requestId while bad2001.json fails on other violations.
distribution/examples/validation/json-schema/apis.yaml (1)
22-29: LGTM! Schema mappings configuration is correct.The
schemaMappingsconfiguration properly maps URNs to their corresponding schema files:
urn:app:base_def→schemas/base.jsonurn:app:meta_def→schemas/meta.jsonThis enables schema2001.json to resolve
$refURNs correctly during validation.distribution/examples/validation/json-schema/good2001.json (1)
1-16: LGTM! Example data meets all validation requirements.This "good" example correctly satisfies the schema:
paramsarray has 2 items (meetsminItems: 2)meta.sourceis non-empty (meetsminLength: 1)meta.requestIdis provided with valid format- No additional properties at root level
The data should pass validation successfully.
distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/validation/JSONSchemaValidationExampleTest.java (2)
66-85: Asymmetry in test coverage:port2001()tests only the negative case.The
port2001()method only testsbad2001.json, while thegood2001.jsonfile exists but is never used. In contrast,port2000()tests both valid and invalid JSON. This creates an inconsistency in test coverage where port 2001 has no verification that it accepts valid JSON.If this asymmetry is intentional as part of the simplification goal, please clarify in the commit message. Otherwise, consider adding the positive test case to maintain consistent coverage between the two test methods.
79-81: No changes needed. The error pointer paths correctly reference the evaluation path according to JSON Schema draft 2020-12 specification, which includes schema keywords and$reftraversal paths. This is the intended behavior of the networknt/json-schema library when validating against draft 2020-12 schemas.
|
This pull request needs "/ok-to-test" from an authorized committer. |
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.