fix: keep allOf members that have properties alongside a validation keyword - #24551
Open
donald wants to merge 1 commit into
Open
fix: keep allOf members that have properties alongside a validation keyword#24551donald wants to merge 1 commit into
donald wants to merge 1 commit into
Conversation
…eyword
A schema carrying `properties` but omitting `type: object`, next to a
validation keyword, was classified as "validation without a type" and
silently dropped from allOf. Every model inheriting it lost those
properties -- no error, no warning, just a smaller model and a broken JSON
contract:
Parent:
minProperties: 2 # validation keyword, no `type: object`
properties:
alpha: { type: string }
beta: { type: string }
Child:
allOf: [{ $ref: '#/components/schemas/Parent' }]
properties:
gamma: { type: string }
// before: Child has only `gamma`
// after: Child has `alpha`, `beta`, `gamma`
OpenAPINormalizer.removeUnsupportedSchemasFromAllOf() drops any allOf
member for which ModelUtils.isUnsupportedSchema() returns true, and that
check was:
schema.getTypes() == null && hasValidation(schema)
Omitting `type: object` next to `properties` is very common in 3.0 specs,
and getTypes() is only populated for 3.1, so in practice any 3.0 schema
with a validation keyword and no explicit type was judged unsupported --
including ones that are plainly models.
Require that the schema also has no properties (and check getType() as
well, so the 3.0 case is recognised at all). A schema with properties is a
model, not bare validation.
Regenerating all 787 sample configs produces no diff.
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.
Fixes #24550
What
A schema carrying
propertiesbut omittingtype: object, next to a validation keyword, was classified as"validation without a type" and silently dropped from
allOf. Every model inheriting it lost thoseproperties — no error, no warning, just a smaller model and a broken JSON contract.
Removing
minProperties— or adding an explicittype: object— makes the properties reappear, which iswhat makes this easy to hit without noticing.
Why
OpenAPINormalizer.removeUnsupportedSchemasFromAllOf()drops every allOf member for which
ModelUtils.isUnsupportedSchema()returns true, and that check was:getTypes()is only populated for 3.1, so for a 3.0 spec it is always null. In practice that means any3.0 schema with a validation keyword and no explicit type was judged unsupported — including ones that are
plainly models. Omitting
type: objectnext topropertiesis very common in 3.0 specs.How
Also require that the schema has no
properties, and checkgetType()alongsidegetTypes()so the 3.0case is recognised at all. A schema with properties is a model, not bare validation.
The intent of the original check — skipping schemas that are only validation, and
if/thenschemas —is preserved; the existing
isUnsupportedSchemaTestassertions still pass unchanged.Testing
OpenAPINormalizerTest.testAllOfMemberWithValidationAndPropertiesIsKept— asserts the allOf memberreferencing
Parentsurvives normalization. Verified that this test fails without the fix(
the whole allOf was dropped, so Child lost the inherited properties).OpenAPINormalizerTest(64 tests) andModelUtilsTest(68 tests, including the existingisUnsupportedSchemaTest) pass../bin/generate-samples.sh ./bin/configs/*.yaml) and./bin/utils/export_docs_generators.sh— no diff.PR checklist
(Both scripts were run; neither produced any change, so there is nothing to commit beyond the fix and its test.)
masterThis is in the language-agnostic core (
OpenAPINormalizer/ModelUtils), so it is not owned by onelanguage committee. CC @wing328
🤖 Generated with Claude Code
Summary by cubic
Fixes dropped
allOfmembers when a referenced schema haspropertiesalongside a validation keyword but omitstype: object. Child models now correctly inherit those properties, preserving the JSON contract.ModelUtils.isUnsupportedSchema()to only treat a schema as validation-only when it lacks bothgetTypes()(OAS 3.1) andgetType()(OAS 3.0), has validation keywords, and has noproperties.OpenAPINormalizerTest.testAllOfMemberWithValidationAndPropertiesIsKeptwith a 3.0 sample; full tests pass and regenerating samples shows no diffs.Written for commit dd946ff. Summary will update on new commits.