chore(schemas): sync with release700 blob artifacts#1239
Open
divyagovindaiah wants to merge 2 commits into
Open
chore(schemas): sync with release700 blob artifacts#1239divyagovindaiah wants to merge 2 commits into
divyagovindaiah wants to merge 2 commits into
Conversation
- add movie schema (new object type) - update category, categoryinstance, collection, dialcode, objectcategory, question, questionset schemas
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThis PR extends the schema registry with a new Movie schema and refines validation, data shapes, and metadata handling across existing schemas. Key updates include introducing Movie definition with versioning and external table mappings, reshaping CategoryInstance field structures, adjusting Question/QuestionSet validation controls and removing properties, cleaning up ObjectCategory metadata fields, and formatting consistency improvements. ChangesSchema Definitions and Validation Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/objectcategory/1.0/schema.json (1)
9-41:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPotential breaking contract: ObjectCategory schema will now reject legacy metadata fields
schemas/objectcategory/1.0/schema.jsonhasadditionalProperties: falseand no longer definescreatedOn,lastUpdatedOn,lastStatusChangedOn, orversionKey, so any request still sending those keys will fail validation. The create/update path validates the incoming payload’s metadata against the JSON schema viaDefinitionNode.validate→SchemaValidator.validate(node.getMetadata)(backed byplatform-core/schema-validator), and it does not strip unknown properties.Ensure all producers are updated (or server-side strips/compat is added), and add/extend tests to cover requests containing those legacy fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/objectcategory/1.0/schema.json` around lines 9 - 41, The schema in schemas/objectcategory/1.0/schema.json currently sets additionalProperties: false and omits legacy metadata fields (createdOn, lastUpdatedOn, lastStatusChangedOn, versionKey), which will break Validation via DefinitionNode.validate → SchemaValidator.validate when passing node.getMetadata; fix by adding those legacy fields back into the "properties" block (define types for createdOn, lastUpdatedOn, lastStatusChangedOn, versionKey) or, if intended, switch additionalProperties to true to accept unknown metadata, and update/extend tests for create/update flows to include payloads with those legacy keys so DefinitionNode.validate/SchemaValidator.validate behavior is covered; ensure whichever approach you choose is applied consistently across producers or add server-side stripping/compatibility prior to calling SchemaValidator.validate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@schemas/categoryinstance/1.0/schema.json`:
- Around line 65-67: The schema changed the "index" field to type "string",
which is inconsistent with downstream code (CategoryInstanceActor, TermActor
produce Integer/Number and NodeUtil expects asInstanceOf[Integer]); restore type
safety by changing "index" back to a numeric type (use "type": "integer" if you
only allow integers or "number" for floats) in schema.json and ensure any JSON
validators reflect that, or if strings are intentional update the producers and
consumers instead: modify CategoryInstanceActor and TermActor to emit string
values for "index" and update NodeUtil to read it as String then safely parse to
Int (with validation/fallback) and adjust any ordering logic to use numeric
comparison; run/adjust tests covering index serialization/deserialization and
ordering to prevent regressions.
In `@schemas/movie/1.0/schema.json`:
- Around line 13-99: The schema in schemas/movie/1.0/schema.json currently sets
"additionalProperties": false and does not include the external-mapped fields
summaries, synopsis, awards, and story in the "properties" object, causing
validation to reject those mapped fields; update the "properties" section to add
string (or appropriate type) entries for "summaries", "synopsis", "awards", and
"story" (or set their types/arrays to match what config.json expects) or allow
them via "additionalProperties": true if intended, ensuring the schema and the
external.properties mapping align (refer to the "properties" object and the
"additionalProperties" flag to implement the fix).
In `@schemas/question/1.1/config.json`:
- Line 76: The config currently disables API-level shape rejection by setting
"schema_restrict_api": false; restore strict request validation by setting
"schema_restrict_api": true in schemas/question/1.1/config.json or, if you
intentionally need API flexibility, add equivalent validator-side enforcement in
the schema-validator (platform-core/schema-validator) and update the Question
schema.json to disallow additional properties at the root (so
DefinitionDTO.validateRequest behavior is preserved); ensure tests cover
rejection of unknown request fields so DefinitionDTO.validateRequest semantics
remain enforced.
In `@schemas/questionset/1.1/config.json`:
- Line 24: Re-enable the QuestionSet write-safety and request-guardrails by
changing the config values in schemas/questionset/1.1/config.json: set
"versionCheckMode" back to a mode that enforces VersionKeyValidator (not "OFF")
so the VersionKeyValidator concurrency check runs, and set "schema_restrict_api"
to true (or add equivalent logic) so DefinitionDTO.validateRequest's
allowlist/invalid-prop checks run for incoming requests; if you prefer
service-level enforcement, implement equivalent validation in the
request/validator layer (the VersionKeyValidator and
DefinitionDTO.validateRequest code paths) and add regression tests covering
update concurrency and unknown-top-level-field rejection, and ensure schema
validation uses platform-core/schema-validator with schemas loaded from the repo
by configuring schema.base_path to the local schemas/ directory rather than the
remote blob path.
In `@schemas/questionset/1.1/schema.json`:
- Around line 714-715: The root JSON Schema's top-level object is missing
additionalProperties, making the schema permissive; reinstate
additionalProperties: false on the root schema object (the top-level JSON object
definition in this file) so undeclared fields are rejected, and ensure this
change doesn't conflict with any intended extensibility or existing $ref'd
subschemas—if extensibility is required, document and explicitly allow specific
properties instead of leaving additionalProperties undefined.
---
Outside diff comments:
In `@schemas/objectcategory/1.0/schema.json`:
- Around line 9-41: The schema in schemas/objectcategory/1.0/schema.json
currently sets additionalProperties: false and omits legacy metadata fields
(createdOn, lastUpdatedOn, lastStatusChangedOn, versionKey), which will break
Validation via DefinitionNode.validate → SchemaValidator.validate when passing
node.getMetadata; fix by adding those legacy fields back into the "properties"
block (define types for createdOn, lastUpdatedOn, lastStatusChangedOn,
versionKey) or, if intended, switch additionalProperties to true to accept
unknown metadata, and update/extend tests for create/update flows to include
payloads with those legacy keys so
DefinitionNode.validate/SchemaValidator.validate behavior is covered; ensure
whichever approach you choose is applied consistently across producers or add
server-side stripping/compatibility prior to calling SchemaValidator.validate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 042ca4e9-d137-40fb-879f-f04ba066da73
📒 Files selected for processing (12)
schemas/category/1.0/schema.jsonschemas/categoryinstance/1.0/schema.jsonschemas/collection/1.0/config.jsonschemas/dialcode/1.0/config.jsonschemas/dialcode/1.0/schema.jsonschemas/movie/1.0/config.jsonschemas/movie/1.0/schema.jsonschemas/objectcategory/1.0/schema.jsonschemas/question/1.1/config.jsonschemas/question/1.1/schema.jsonschemas/questionset/1.1/config.jsonschemas/questionset/1.1/schema.json
💤 Files with no reviewable changes (1)
- schemas/collection/1.0/config.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Type of change
Please choose appropriate options.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes in the below checkboxes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Schema Updates