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/05 06:49:42 UTC
[PR] fix(validation): report a duplicated name once, not once per copy [ossie]
ayushtkn opened a new pull request, #517:
URL: https://github.com/apache/ossie/pull/517
## Summary
`find_duplicates` appended an item for every occurrence after the first, so a name
repeated three times produced the same message twice and counted as two failures:
# three datasets named "orders"
Validation FAILED with 2 error(s):
[Unique] Duplicate dataset name 'orders' in model 'm'
[Unique] Duplicate dataset name 'orders' in model 'm'
Now:
Validation FAILED with 1 error(s):
[Unique] Duplicate dataset name 'orders' in model 'm'
Each duplicated name is collected once, in first-appearance order so a document's
diagnostics stay stable. This is the shared path for duplicate dataset, field, metric
and relationship names, so all four read correctly.
Exit status is unchanged — a duplicate is still an error (exit 1).
## Related Issues
NA
## 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(validation): report a duplicated name once, not once per copy [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #517:
URL: https://github.com/apache/ossie/pull/517
--
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(validation): report a duplicated name once, not once per copy [ossie]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #517:
URL: https://github.com/apache/ossie/pull/517#discussion_r4181390694
##########
validation/validate.py:
##########
@@ -188,14 +188,19 @@ def validate_schema(data: dict, schema: dict) -> list[str]:
def find_duplicates(items: list[str]) -> list[str]:
- """Find duplicate items in a list."""
- seen = set()
- duplicates = []
+ """Return the items that appear more than once, each reported once.
+
+ A name repeated three times is one problem, not two: appending per extra
+ occurrence would emit the same message twice and inflate the error count.
+ Order follows first appearance, so a document's diagnostics are stable.
+ """
+ seen: set[str] = set()
+ duplicates: dict[str, None] = {}
for item in items:
if item in seen:
- duplicates.append(item)
+ duplicates[item] = None
Review Comment:
This orders names by when their second occurrence is encountered, not by first appearance as documented. For example, `["a", "b", "b", "a"]` returns `["b", "a"]`, although first-appearance order is `["a", "b"]`. Count occurrences in a dict keyed on the first encounter, then filter that dict for counts greater than one.
##########
validation/tests/test_validate.py:
##########
@@ -190,6 +190,28 @@ def test_semantic_checks_skip_non_model_payloads(data: object) -> None:
assert _VALIDATE.validate_sql(data) == []
+@pytest.mark.parametrize(
+ ("items", "expected"),
+ [
+ (["a", "a"], ["a"]),
+ (["a", "a", "a"], ["a"]),
+ (["a", "b", "a", "b", "a"], ["a", "b"]),
Review Comment:
The order-sensitive case here inserts duplicates in the same order as both their first and second appearances, so it does not verify the promised first-appearance ordering. Add a case such as `(["a", "b", "b", "a"], ["a", "b"])`; it exposes the current implementation returning `["b", "a"]`.
--
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