Server: Infer property type from discriminator/oneOf in getProperty#172
Merged
Conversation
d5f435f to
938c52e
Compare
- `getProperty` fills in `type: 'object'` when a $ref-resolved schema has a `discriminator` (JSON Schema implies an object) - Adopts a homogeneous oneOf/anyOf/allOf branches' shared type - Fixes loadDataPath() throwing on assets like `content.article.items[**].file` where `content` is a $ref-based union schema with no explicit top-level `type` - Add unit tests for the new inference behaviour
938c52e to
0ea6bda
Compare
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
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.
Written by Claude.
Problem
Model.getProperty(name)resolves$refschemas but doesn't infer a top-leveltypewhen the resolved definition uses JSON Schema constructs that imply a type without stating one explicitly. Concretely, deleting a row whose model has an asset path likecontent.article.items[**].fileblows up atQueryBuilder.loadDataPath():The check in
loadDataPath()requiresproperty.typeto be'object'or'array'(or a wildcard) to traverse a nested data path. Whencontentis defined as:getProperty()resolves the$refbut the merged property hasdiscriminator + oneOfand notype— so the check fails.Fix
Extend
getProperty()to fill in the missingtypewhen it can be inferred unambiguously:discriminatorimpliestype: 'object'(per JSON Schema / OpenAPI semantics).oneOf/anyOf/allOfadopts its branches' shared type.This is additive — existing properties with explicit
typeare unchanged. Any caller that previously branched onproperty.type === undefinedfor these schemas will now see the inferred type, which is what they would have expected if the schema author had spelled it out.Test plan
pnpm vitest run packages/server/src/models/Model.test.js— 6 new unit tests cover explicit type, discriminator-on-$ref, homogeneous oneOf/anyOf, mixed oneOf, missing property