-
Notifications
You must be signed in to change notification settings - Fork 52
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
base: v2_remove_unique
Are you sure you want to change the base?
Changes from all commits
05f525b
23a70f1
4ae5c3b
cc4327e
a2bc91f
6b77671
4d8f7d0
7c8f516
0884659
6819d84
139a643
39c5883
70f4d93
430e8af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| #!/bin/bash | ||
| # Negative examples: documents that MUST be rejected by the schema. | ||
| # | ||
| # Each case is "<file>|<expected error substring>". We assert BEHAVIOUR (the | ||
| # document is rejected, and its output contains the declared substring) - not a | ||
| # specific constraint name, and each case carries its own clause. | ||
| # | ||
| # All cases share ONE schema and are validated in a SINGLE xmllint call (one | ||
| # compile for many files) to stay fast. Fails closed: accepted, wrong reason, or | ||
| # xmllint not running all count as failures. | ||
| # | ||
| # 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, ...). | ||
|
|
||
| set -u | ||
| SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) | ||
| ROOT_DIR=$( cd -- "${SCRIPT_DIR}/../.." &> /dev/null && pwd ) | ||
| XMLLINT="${XMLLINT_BIN:-${SCRIPT_DIR}/xmllint}" | ||
| cd "${ROOT_DIR}" | ||
|
|
||
| SCHEMA="xsd/NeTEx_publication.xsd" | ||
|
|
||
| # "<file>|<expected error substring>" | ||
| CASES=( | ||
| "examples/should-fail/duplicate-GroupOfLinkSequences.xml|Duplicate key-sequence" | ||
| "examples/should-fail/duplicate-CalendarDate.xml|Duplicate key-sequence" | ||
| "examples/should-fail/duplicate-ValidBetween.xml|Duplicate key-sequence" | ||
| "examples/should-fail/duplicate-ValidityPeriod.xml|Duplicate key-sequence" | ||
| ) | ||
|
|
||
| files=() | ||
| for c in "${CASES[@]}"; do files+=("${c%%|*}"); done | ||
| out=$("${XMLLINT}" --noout --schema "${SCHEMA}" "${files[@]}" 2>&1) | ||
|
|
||
| fail=0 | ||
| echo "Checking NeTEx 'should-fail' negative examples ..." | ||
| for c in "${CASES[@]}"; do | ||
| f="${c%%|*}"; expected="${c#*|}" | ||
| if printf '%s\n' "${out}" | grep -Fqx "${f} validates"; then | ||
| echo "SHOULD HAVE FAILED ${f} — accepted (must be rejected)"; fail=1 | ||
| elif printf '%s\n' "${out}" | grep -F "${f}:" | grep -q "${expected}"; then | ||
| echo "OK ${f}" | ||
| else | ||
| echo "ERROR ${f} — rejected, but not matching '${expected}'"; fail=1 | ||
| fi | ||
| done | ||
|
|
||
| exit "${fail}" |
| 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` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <PublicationDelivery xmlns="http://www.netex.org.uk/netex" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.0"> | ||
| <PublicationTimestamp>2020-01-01T12:00:00Z</PublicationTimestamp> | ||
| <ParticipantRef>TEST</ParticipantRef> | ||
| <dataObjects> | ||
| <ServiceCalendarFrame version="1.0" id="TEST:ServiceCalendarFrame:1"> | ||
| <operatingDays> | ||
| <OperatingDay version="1.0" id="TEST:OperatingDay:1"> | ||
| <CalendarDate>2020-01-01</CalendarDate> | ||
| </OperatingDay> | ||
| <OperatingDay version="1.0" id="TEST:OperatingDay:2"> | ||
| <CalendarDate>2020-01-01</CalendarDate> | ||
| </OperatingDay> | ||
| </operatingDays> | ||
| </ServiceCalendarFrame> | ||
| </dataObjects> | ||
| </PublicationDelivery> |
| 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> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <PublicationDelivery xmlns="http://www.netex.org.uk/netex" 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"> | ||
| <ValidBetween id="TEST:ValidBetween:1" version="1.0"> | ||
| <FromDate>2020-01-01T00:00:00Z</FromDate> | ||
| </ValidBetween> | ||
| </ResourceFrame> | ||
| <ServiceFrame version="1.0" id="TEST:ServiceFrame:1"> | ||
| <ValidBetween id="TEST:ValidBetween:1" version="1.0"> | ||
| <FromDate>2020-01-01T00:00:00Z</FromDate> | ||
| </ValidBetween> | ||
| </ServiceFrame> | ||
| </dataObjects> | ||
| </PublicationDelivery> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <PublicationDelivery xmlns="http://www.netex.org.uk/netex" 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"> | ||
| <organisations> | ||
| <Authority version="1.0" id="TEST:Authority:1"> | ||
| <Name>A</Name> | ||
| <ValidityPeriod id="TEST:ValidityPeriod:1" version="1.0"> | ||
| <FromDate>2020-01-01T00:00:00Z</FromDate> | ||
| </ValidityPeriod> | ||
| </Authority> | ||
| <Operator version="1.0" id="TEST:Operator:1"> | ||
| <Name>B</Name> | ||
| <ValidityPeriod id="TEST:ValidityPeriod:1" version="1.0"> | ||
| <FromDate>2020-01-01T00:00:00Z</FromDate> | ||
| </ValidityPeriod> | ||
| </Operator> | ||
| </organisations> | ||
| </ResourceFrame> | ||
| </dataObjects> | ||
| </PublicationDelivery> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -979,6 +979,15 @@ | |
| <xsd:field xpath="@version"/> | ||
| </xsd:key> | ||
| <!-- =====GroupOfLinkSequences============================== --> | ||
| <!-- =====GroupOfLinkSequences unique========================== --> | ||
| <xsd:unique name="GroupOfLinkSequences_UniqueBy_Id_Version"> | ||
| <xsd:annotation> | ||
| <xsd:documentation>Every [GroupOfLinkSequences Id + Version] must be unique within document.</xsd:documentation> | ||
| </xsd:annotation> | ||
| <xsd:selector xpath=".//netex:GroupOfLinkSequences"/> | ||
| <xsd:field xpath="@id"/> | ||
| <xsd:field xpath="@version"/> | ||
| </xsd:unique> | ||
| <!-- =====SimpleFeature============================== --> | ||
| <!-- =====SimpleFeature Key ========================== --> | ||
| <xsd:keyref name="SimpleFeature_KeyRef" refer="netex:SimpleFeature_AnyVersionedKey"> | ||
|
|
@@ -2191,6 +2200,15 @@ | |
| <xsd:field xpath="@version"/> | ||
| </xsd:key> | ||
| <!-- =====OperatingDay============================== --> | ||
| <!-- =====OperatingDay unique==In Calendar======================== --> | ||
| <xsd:unique name="OperatingDay_UniqueCalendarDateInCalendar"> | ||
| <xsd:annotation> | ||
| <xsd:documentation>Each date is only allowed once per calendar.</xsd:documentation> | ||
| </xsd:annotation> | ||
| <xsd:selector xpath=".//netex:ServiceCalendarFrame/netex:operatingDays/netex:OperatingDay"/> | ||
| <xsd:field xpath="netex:CalendarDate"/> | ||
| <xsd:field xpath="@version"/> | ||
| </xsd:unique> | ||
| <!-- =====OperatingDay Key ========================== --> | ||
| <xsd:keyref name="OperatingDay_KeyRef" refer="netex:OperatingDay_AnyVersionedKey"> | ||
| <xsd:selector xpath=".//netex:OperatingDayRef | .//netex:FromOperatingDayRef | .//netex:ToOperatingDayRef"/> | ||
|
|
@@ -2239,6 +2257,15 @@ | |
| <xsd:field xpath="@version"/> | ||
| </xsd:key> | ||
| <!-- =====ValidityCondition============================== --> | ||
| <!-- =====ValidityCondition unique========================== --> | ||
| <xsd:unique name="ValidityCondition_UniqueBy_Id_Version"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should just be a key.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is worth a dedicated ticket (for this one & all the places deserving a key, either removed erroneously before #1032 is just short rollback of 4 missing uniques.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the opposite is true. It was erronousley added as unique, and the removal script removed id unique, because the expectation was a key must exist for an id.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, yet the key isn't there, so it creates a larger trouble even (no more unicity at all). We'll need to address that, so I created #1034 ; we'll catalogue over there. |
||
| <xsd:annotation> | ||
| <xsd:documentation>Every [ValidityCondition Id + Version] must be unique within document.</xsd:documentation> | ||
| </xsd:annotation> | ||
| <xsd:selector xpath=".//netex:ValidityCondition | .//netex:ValidityTrigger | .//netex:ValidityRuleParameter | .//netex:ValidBetween"/> | ||
| <xsd:field xpath="@id"/> | ||
| <xsd:field xpath="@version"/> | ||
| </xsd:unique> | ||
| <!-- =====ValidityCondition Key ========================== --> | ||
| <xsd:keyref name="ValidityCondition_KeyRef" refer="netex:ValidityCondition_AnyVersionedKey"> | ||
| <xsd:selector xpath=".//netex:ValidityConditionRef | .//netex:ValidityTriggerRef | .//netex:ValidityRuleParameterRef | .//netex:WithConditionRef"/> | ||
|
|
@@ -6488,6 +6515,15 @@ | |
| <xsd:field xpath="@version"/> | ||
| </xsd:key> | ||
| <!-- ===== UsageValidityPeriod Key ========================== --> | ||
| <!-- ===== UsageValidityPeriodting unique========================== --> | ||
| <xsd:unique name="UsageValidityPeriod"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be unique by key.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See PR description, I'm just undoing problematic removal of any uniqueness at all, this is not a PR where I'm changing any unique to a key (and as commented above, at least one was removed just pre- To be discussed, then (but this is not this PR's goal).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A duplicate id is never correct, it is a key. Hence it should not be unique.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #1032 (comment) |
||
| <xsd:annotation> | ||
| <xsd:documentation>Every [UsageValidityPeriod Id + Version] must be unique within document.</xsd:documentation> | ||
| </xsd:annotation> | ||
| <xsd:selector xpath=".//netex:UsageValidityPeriod | .//netex:ValidityPeriod"/> | ||
| <xsd:field xpath="@id"/> | ||
| <xsd:field xpath="@version"/> | ||
| </xsd:unique> | ||
| <xsd:keyref name="UsageValidityPeriod_KeyRef" refer="netex:UsageValidityPeriod_AnyVersionedKey"> | ||
| <xsd:selector xpath=".//netex:UsageValidityPeriodRef"/> | ||
| <xsd:field xpath="@ref"/> | ||
|
|
||
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.