wmo-im / iwxxm

XML schema and Schematron for aviation weather data exchange
https://old.wmo.int/wiswiki/tiki-index.php%3Fpage=TT-AvXML
48 stars 22 forks source link

Schematron rule IWXXM.ExtensionAlwaysLast is not working well #118

Closed blchoy closed 5 years ago

blchoy commented 6 years ago

The intention of having schematron rule IWXXM.ExtensionAlwaysLast is to make sure that <iwxxm:extension> is always the last entity of a sequence of elements in a complex type. However, a complex type extending another complex type will inherit its parent's <iwxxm:extension> and the extension may no longer be at the end of the sequence in the the child complex type. As a result, the rule will fail when there are extensions to these complex types (e.g. TropicalCycloneSIGMET).

This document shows all IWXXM complex types that have an extension and where they get the extension.

As the relative location of <iwxxm:extension> in a complex type is fixed by the schemas, I think there is no need for this schematron rule. Views are most welcome.

mgoberfield commented 6 years ago

Agreed. I think there was a time in the 2.0 era when the element could be placed at the beginning of the XML document. Now that that has been corrected, I don't see a need for this rule any longer.

BTW, nice table.

mark

On Wed, Sep 5, 2018 at 1:49 AM, BL Choy notifications@github.com wrote:

The intention of having schematron rule IWXXM.ExtensionAlwaysLast is to make sure that iwxxm:extension is always the last entity of a sequence of elements in a complex type. However, a complex type extending another complex type will inherit its parent's iwxxm:extension and the extension may not be at the end of the sequence in the the child complex type.

The Wiki page on WIS https://wiswiki.wmo.int/tiki-index.php?page=IWXXM-3.0-ExtensionElementList shows those complex types having an extension.

As the relative location of iwxxm:extension in a complex type is fixed by the schemas, I think there is no need for this schematron rule. Views are most welcome.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/wmo-im/iwxxm/issues/118, or mute the thread https://github.com/notifications/unsubscribe-auth/AHfSTQYLZYpqSY40ZPZ1mMjoUPnDDgEiks5uX2XxgaJpZM4WaOPI .

braeckel commented 6 years ago

Let's consider the SIGMET, VA SIGMET, and TC SIGMET case.

The ExtensionAlwaysLast check was relevant to IWXXM 2 when VA and TC SIGMETs had mandatory contents (iwxxm:volcano and iwxxm:tropicalCyclone) so we had extension blocks on SIGMET, VA SIGMET, and TC SIGMET. Then we had this rule to enforce that the last (the extension blocks on VA and TC SIGMET) were the ones used.

However, as part of 3.0RC1 we made the VA and TC SIGMET contents (iwxxm:volcano and iwxxm:tropicalCyclone) optional which created a validation uncertainty about which iwxxm:extension element was being reported: either SIGMET or VA/TC SIGMET. So we had to remove the extension elements from VA/TC SIGMET.

That's a long way of saying that yes, it's time to remove this rule. ;) AFAIK we no longer allow multiple extension blocks on any element.

blchoy commented 6 years ago

Thank you all for the comments. I found a number of issues here:

However, as part of 3.0RC1 we made the VA and TC SIGMET contents (iwxxm:volcano and iwxxm:tropicalCyclone) optional which created a validation uncertainty about which iwxxm:extension element was being reported: either SIGMET or VA/TC SIGMET. So we had to remove the extension elements from VA/TC SIGMET.

This is not entirely correct. iwxxm:volcano and iwxxm:tropicalCyclone has multiplicity of (1..2) in IWXXM-3 so iwxxm:extension can exists at the end of them without violating XML 1.0 Unique Particle Attribution rule. In other words iwxxm:extension is missing at the end of the element sequence in VA and TC SIGMET in IWXXM-3 is not right (my fault).

But Aaron is still right in the sense that there are possibilities to have optional elements in extending an element, and in that case if iwxxm:extension is appended to the end of the element sequence will definitely trigger a UPA violation. For example, the follow IWXXM-2 XSD fragment will not validate in Oxygen XML. Changing minOccurs to iwxxm:eruptingVolcano from 0 to 1 will do:

<complexType name="VolcanicAshSIGMETType">
    <complexContent>
        <extension base="iwxxm:SIGMETType">
            <sequence>
                <element name="eruptingVolcano" type="metce:VolcanoPropertyType" minOccurs="0">
                    <annotation>
                        <documentation>The volcano that is erupting</documentation>
                    </annotation>
                </element>
                <element name="extension" type="anyType" minOccurs="0" maxOccurs="unbounded">
                    <annotation>
                        <documentation>Extension block for optional and/or additional parameters for element VolcanicAshSIGMET</documentation>
                    </annotation>
                </element>
            </sequence>
        </extension>
    </complexContent>
</complexType>

So there are two things we will have to consider:

  1. Shall we add iwxxm:extension to end of the element sequence of all elements in IWXXM as long as it won't violate UPA? This will introduce multiple iwxxm:extension in VA and TC SIGMET for example. As in previous discussions we want to use only one of them.
  2. How to deal with the possibility that iwxxm:extensions may not necessary be at the end of an element sequence? IWXXM.ExtensionAlwaysLast should not work if iwxxm:extension and the optional element are present at the same time. [A solution is to create element specific schematron to check if the iwxxm:extension used is the last one instead of a generic one. This is possible as we are using XSLT to generate schematron rules and we know which element has an iwxxm:extension added to its end]
braeckel commented 6 years ago

To meet the need to have cancellations that are specific to the SIGMET types (SIGMET, VA SIGMET, TC SIGMET) eruptingVolcano and tropicalCyclone were made optional (0..1) in the early phases of 3.0RC1. This was accidentally removed by the time it was officially released and as you say the cardinality is currently 1..2, which does not allow for VA and TC cancellations as originally intended. #122 was created to fix this issue in 3.0RC2 and set the cardinalities back to 0..2 (optional).

Given the need for optional VA and TC SIGMET elements I don't think it is possible to add iwxxm:extension to the end of the VA and TC SIGMET schemas due to the UPA issue, and therefore it is not possible to keep IWXXM.ExtensionAlwaysLast since this would always fail validation for TC and VA SIGMET extension elements. My recollection of the discussion on the iwxxm:extension element order from 3.0RC1 is that we would simply have to tolerate having a single iwxxm:extension element (even if it isn't the last element) in a few cases, such as SIGMET and its subclasses.

blchoy commented 5 years ago

This is fixed in 3.0.0RC2.