Repository navigation
fix(python): reject unknown keys on nested models, as ossie validate does - #552
Conversation
…does ossie-schema.json sets additionalProperties: false on every object but AIContext. The SDK set extra="forbid" only on the root OssieDocument, so a mistyped key on a nested object -- "descriptionn" on a field, "expresssion" on a metric -- passed model_validate() and then disappeared from the serialized document, while validation/validate.py reported both. A model the SDK round-tripped cleanly could still fail the validator, which is the one guarantee the SDK exists to give. The nine models whose schema node forbids extras now forbid them too: CustomExtension, DialectExpression, Expression, Dimension, Field, Dataset, Relationship, Metric and SemanticModel. OssieAIContextObject stays extra="allow" because AIContext is the one open node in the schema. The test that asserted only the document rejected extras encoded the bug as intent; it is replaced by one that checks each nesting level and one that pins the forbidding model set against the schema, so a node added to ossie-schema.json later cannot keep Pydantic's extra="ignore" default unnoticed. OssieSemanticModel.model_validate() now rejects a document's "version", which it previously ignored. That matches the schema, where the SemanticModel node has no version property, and the SDK already documents OssieDocument rather than OssieSemanticModel as the type for models embedded in an ontology map. Closes apache#530
|
Thank you for your first contribution to Apache Ossie (incubating)! A committer will review your PR. In the meantime, please ensure:
No ICLA is required to contribute. If you are elected as a committer in the future, you will need an Individual Contributor License Agreement (ICLA) on file with the ASF. Questions? Ask in the PR comments or join us on Slack. We appreciate your contribution! |
jbonofre
left a comment
There was a problem hiding this comment.
LGTM. The nine models that the schema marks additionalProperties: false now forbid extra fields, and AIContext stays open as in the schema. The version deletion in test_document_is_a_semantic_model is needed, since version would now be rejected on the embedded model.
One thing to note: documents with misspelled or unknown nested keys used to load and will now raise ValidationError. That is the intended behavior, but users may notice it.
What
OssieDocumentrejected an unknown key at the document root but silently droppedone on any nested object, while
validation/validate.pyrejected the samedocument. The SDK is the type layer the converters build on, so a mistyped key
could enter a pipeline as a clean
model_validate()and simply disappear atserialization.
Closes #530.
Why the boundary is exactly this
core-spec/ossie-schema.jsonsetsadditionalProperties: falseon nine nodes —CustomExtension,DialectExpression,Expression,Dimension,Field,Dataset,Relationship,Metric,SemanticModel— and on no other node.AIContextis the one open object, soOssieAIContextObjectkeepsextra="allow"; the fix is nineextra="forbid", not a blanket one.Reproduction, before
After: both report the same two locations, e.g.
loc=('datasets', 0, 'fields', 0, 'descriptionn') type=extra_forbidden.One deliberate behavior change
OssieSemanticModel.model_validate()now rejects a document'sversion, whichit previously ignored. That matches the schema — the
SemanticModelnode has noversionproperty — and the SDK already documentsOssieDocument, notOssieSemanticModel, as the type for models embedded in an ontology map(
ontology.jsonembeds via$refto the root schema, which requiresversion).A test so it cannot come back
test_unknown_field_names_match_the_schema_allowlistreadsossie-schema.json,derives the set of nodes that forbid extras, and asserts it equals the set of
models configured to forbid — plus that
AIContextdoes not. A future schema nodethat gains
additionalProperties: falsewithout a matching model config now failsCI instead of silently re-enabling the drop.
The previous
test_only_document_rejects_extra_fieldsasserted the buggybehavior, so it is replaced by a parametrized test covering each nesting level.
Test plan
python: 122 passedvalidation/tests: 181 passed (rebased onto feat(validation): validate several files per run and look up the schema via OSSIE_SCHEMA #545);validation/test_validate.py: OK;generate-spec-types.py --check: cleanapache-ossie: dbt 176, hex 224, sigma 103, wisdom 37 passedconverters/snowflake186,converters/gooddata94 (do not use the SDK; run as a regression check)dev/check-license(Apache RAT): passed