-
Notifications
You must be signed in to change notification settings - Fork 54
Restore 4 uniqueness removed in #998 where no xsd:key covers it (+ must-fail guards) #1032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
thbar
wants to merge
15
commits into
v2_remove_unique
Choose a base branch
from
v2_remove_unique_regression_fix
base: v2_remove_unique
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
05f525b
Add reproduction for uniqueness regression
thbar 23a70f1
Add quick readme on should-fail folder
thbar 4ae5c3b
Create minimalistic "should fail" harness on CI
thbar cc4327e
Merge branch 'v2_remove_unique' into v2_remove_unique_regression_fix
thbar a2bc91f
Restore GroupOfLinkSequences uniqueness
thbar 6b77671
Make "should-fail" checks assert behaviour, not the constraint name
thbar 4d8f7d0
Add failing example for duplicate "OperatingDay by CalendarDate"
thbar 7c8f516
We do not want duplicate ValidBetween (same id + version)
thbar 0884659
Add test-case for duplicate ValidityPeriod
thbar 6819d84
Restore OperatingDay CalendarDate uniqueness
thbar 139a643
Restore ValidBetween uniqueness
thbar 39c5883
Restore ValidityPeriod uniqueness
thbar 70f4d93
Lint and update documentation tables
github-actions[bot] 430e8af
Batch verifications (much shorter time) like for regular validation s…
thbar 2e84f08
Merge branch 'v2_remove_unique' into v2_remove_unique_regression_fix
thbar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #!/bin/bash | ||
| # Negative examples: documents that MUST be rejected by the schema. | ||
| # Each case greps the EXACT expected constraint name, so dropping that constraint | ||
| # (document wrongly accepted) fails the build and can't be masked by another error. | ||
|
|
||
| set -u | ||
| SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) | ||
| ROOT_DIR=$( cd -- "${SCRIPT_DIR}/../.." &> /dev/null && pwd ) | ||
| # CI uses the vendored 2025 Linux x86-64 xmllint ("Temporary xmllint master", | ||
| # https://github.com/TransmodelEcosystem/NeTEx/pull/915); set XMLLINT_BIN to a local | ||
| # xmllint to run this on any other OS or CPU architecture (macOS, Windows, ARM, ...). | ||
| XMLLINT="${XMLLINT_BIN:-${SCRIPT_DIR}/xmllint}" | ||
| cd "${ROOT_DIR}" | ||
|
|
||
| fail=0 | ||
| assert_rejected() { # <file> <schema> <error-pattern> | ||
| if "${XMLLINT}" --noout --schema "$2" "$1" 2>&1 | grep -q "$3"; then | ||
| echo "OK $1" | ||
| else | ||
| echo "SHOULD HAVE FAILED $1 — not rejected by '$3' (constraint missing?)" | ||
| fail=1 | ||
| fi | ||
| } | ||
|
|
||
| echo "Checking NeTEx 'should-fail' negative examples ..." | ||
| assert_rejected examples/should-fail/duplicate-GroupOfLinkSequences.xml xsd/NeTEx_publication.xsd GroupOfLinkSequences_UniqueBy_Id_Version | ||
|
|
||
| exit "${fail}" |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Negative examples (must-fail) | ||
|
|
||
| Each file carries one deliberate defect and must therefore be **rejected** by the | ||
| schema for a specific, expected reason. The cases — file, schema and expected | ||
| constraint — are declared in `.github/scripts/validate-should-fail.sh` (run in CI). | ||
|
|
||
| Run locally: `XMLLINT_BIN=$(which xmllint) ./.github/scripts/validate-should-fail.sh` |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <PublicationDelivery xmlns="http://www.netex.org.uk/netex" xmlns:gml="http://www.opengis.net/gml/3.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.0"> | ||
| <PublicationTimestamp>2020-01-01T12:00:00Z</PublicationTimestamp> | ||
| <ParticipantRef>TEST</ParticipantRef> | ||
| <dataObjects> | ||
| <ResourceFrame version="1.0" id="TEST:ResourceFrame:1"> | ||
| <groupsOfEntities> | ||
| <GroupOfLinkSequences version="1.0" id="TEST:GroupOfLinkSequences:1"/> | ||
| <GroupOfLinkSequences version="1.0" id="TEST:GroupOfLinkSequences:1"/> | ||
| </groupsOfEntities> | ||
| </ResourceFrame> | ||
| </dataObjects> | ||
| </PublicationDelivery> |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you think this is correct? For example, why isn't this just a key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See PR description (just added) ; I think it's more correct that what is in #998 on that specific bit, which was effectively removal of any sort of uniqueness check.
Why not key, well, key has been removed before
v2.0.0(see description). I don't know if adding it back in a patch release is a good idea or not, we'll have to discuss that collectively.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thbar and @skinkie,
I confirm that in v1.3, this part was written as follow:
Direct link is this one.
Considering that
GroupOfLinkSequencesRefis based on the substitution groupVersionOfObjectRefand notTypeOfEntityRef, I am uncertain that a key will work.However, let's make it simple and roll back to the original state of this part of the XSD.