You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ossie.apache.org by "ayushtkn (via GitHub)" <gi...@apache.org> on 2026/10/14 03:10:58 UTC
[PR] fix(ontology): require a mapping to say how it populates anything [ossie]
ayushtkn opened a new pull request, #582:
URL: https://github.com/apache/ossie/pull/582
## Summary
`ontology.md` marks three pairs of properties required when the other is absent: `object_mappings`/`link_mappings` on a concept mapping at :500, and `expression`/`referent_mappings` on an object mapping at :513 and on a referent mapping at :562. None of the three was expressed in the schema, which has no `anyOf`, `oneOf` or `allOf` anywhere, so a document could name a concept and say nothing about how to populate it.
`ObjectMapping` was the loosest of the three and the only object definition in the file with no `required` at all, so `{}` was a legal object mapping.
Each pair is now an `anyOf` of the two alternatives. At least one, not exactly one: the prose makes each required only when the other is absent, so a mapping carrying both stays valid.
The three flights documents and the converter ontology fixtures validate unchanged. Before this, a concept mapping populating nothing, an empty object mapping and a referent mapping resolving nothing all passed.
## Related Issues
<!-- Link any related GitHub issues: Closes #123, Fixes #456 -->
## Checklist
### Specification
- [ ] Spec changes are included in `core-spec/` and follow the existing structure
- [ ] Spec changes have been discussed on the mailing list or in a linked issue
- [ ] Breaking changes to the spec are clearly called out in the summary
### Ontology
- [ ] Ontology changes in `ontology/` are consistent with spec changes
- [ ] New or modified terms are defined and documented
### Converters
- [ ] Converter logic in `converters/` is updated to reflect spec or ontology changes
- [ ] New converters include tests under the converter's test directory
- [ ] If adding a new converter, `.github/labeler.yml` is updated with the new path
### Validation
- [ ] Validation rules in `validation/` are updated if the spec changed
- [ ] New validation cases are covered by tests
### Documentation
- [ ] `docs/` is updated to reflect any user-facing changes
- [ ] New features or behaviors are documented with examples where appropriate
- [ ] `CONTRIBUTING.md` is updated if the contribution process changed
### Examples
- [ ] `examples/` are added or updated for any new spec constructs or converter support
### Tests
- [ ] All existing tests pass (`pytest` / CI green)
- [ ] New functionality is covered by tests
### Compliance
- [ ] ASF license headers are present on all new source files
- [ ] No third-party dependencies are added without PMC/IPMC approval
- [ ] If third-party source code is vendored/copied (not just declared as a dependency), `NOTICE` and/or `LICENSE` have been updated per [ASF policy](https://infra.apache.org/licensing-howto.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(ontology): require a mapping to say how it populates anything [ossie]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #582:
URL: https://github.com/apache/ossie/pull/582#discussion_r4266455138
##########
ontology/ontology.json:
##########
@@ -193,6 +193,7 @@
}
},
"required": ["concept"],
+ "anyOf": [{ "required": ["object_mappings"] }, { "required": ["link_mappings"] }],
Review Comment:
`anyOf` + `required` only enforces presence of the property, not that it’s meaningful. As written, a concept mapping with `object_mappings: []` or `link_mappings: []` will still validate, even though it still “populates nothing”. If the intent is to reject empty mappings, also constrain these arrays (e.g., add `minItems: 1` on the `object_mappings` and `link_mappings` property schemas).
##########
validation/tests/test_ontology.py:
##########
@@ -341,3 +341,82 @@ def test_mapping_document_rejects_invalid_shapes(offline, mapping_schema, mutate
mutate(document)
assert _VALIDATE.validate_schema(document, mapping_schema)
+
+
+@pytest.mark.parametrize(
+ "mutate",
+ [
+ # A concept mapping populating neither objects nor links.
+ lambda d: d["concept_mappings"].append({"concept": "Customer"}),
+ # An object mapping that computes nothing and references nothing.
+ lambda d: d["concept_mappings"][0]["object_mappings"].append({}),
+ lambda d: d["concept_mappings"][0]["object_mappings"].append({"concept": "Order"}),
+ # A referent mapping that computes nothing and references nothing.
+ lambda d: d["concept_mappings"][0]["object_mappings"].append(
+ {"referent_mappings": [{"relationship": "placed_by"}]}
+ ),
+ ],
+ ids=[
+ "concept-mapping-populates-nothing",
+ "empty-object-mapping",
+ "object-mapping-names-only-a-concept",
+ "referent-mapping-resolves-nothing",
Review Comment:
The new schema constraints can still allow “maps nothing” via empty arrays (e.g., concept mapping with `object_mappings: []`, object mapping with `referent_mappings: []`). Adding explicit test cases for these empty-array variants would prevent regressions and ensure the schema matches the stated intent.
##########
ontology/ontology.json:
##########
@@ -256,6 +258,7 @@
"$ref": "#/$defs/Expression"
}
},
+ "anyOf": [{ "required": ["expression"] }, { "required": ["referent_mappings"] }],
"additionalProperties": false
Review Comment:
Same issue for `referent_mappings`: the schema will accept `referent_mappings: []`, which still resolves nothing. If an empty referent mapping is invalid per the spec intent, consider adding `minItems: 1` to the `referent_mappings` array schema (and similarly ensure `expression` can’t be an empty string if that would also represent “resolves nothing”).
##########
validation/tests/test_ontology.py:
##########
@@ -341,3 +341,82 @@ def test_mapping_document_rejects_invalid_shapes(offline, mapping_schema, mutate
mutate(document)
assert _VALIDATE.validate_schema(document, mapping_schema)
+
+
+@pytest.mark.parametrize(
+ "mutate",
+ [
+ # A concept mapping populating neither objects nor links.
+ lambda d: d["concept_mappings"].append({"concept": "Customer"}),
+ # An object mapping that computes nothing and references nothing.
+ lambda d: d["concept_mappings"][0]["object_mappings"].append({}),
+ lambda d: d["concept_mappings"][0]["object_mappings"].append({"concept": "Order"}),
+ # A referent mapping that computes nothing and references nothing.
+ lambda d: d["concept_mappings"][0]["object_mappings"].append(
+ {"referent_mappings": [{"relationship": "placed_by"}]}
+ ),
+ ],
+ ids=[
+ "concept-mapping-populates-nothing",
+ "empty-object-mapping",
+ "object-mapping-names-only-a-concept",
+ "referent-mapping-resolves-nothing",
+ ],
+)
+def test_mapping_document_rejects_a_mapping_that_maps_nothing(
+ offline, mapping_schema, mutate
+):
+ """`ontology.md` marks each of these pairs required when the other is absent.
+
+ The rule was documented at :500, :513 and :562 but not expressed in the schema, so
+ `{}` was a legal object mapping. Without it a mapping document can declare a
+ concept and say nothing about how to populate it.
Review Comment:
Referencing `ontology.md` by line numbers (`:500`, `:513`, `:562`) is brittle as the document changes. Prefer a stable reference (e.g., a section heading, an anchor/link, or the specific schema terms/property names) so this test docstring stays accurate over time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(ontology): require a mapping to say how it populates anything [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #582:
URL: https://github.com/apache/ossie/pull/582
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org