incorrect definition of PatchType for MPD patch in 5th edition FDIS

[paulhiggs]

The PatchType defined in clause 5.15.3.3 of the 5th edition FDIS includes

        <xs:sequence>
            <xs:element ref="patch:add"/>
            <xs:element ref="patch:remove"/>
            <xs:element ref="patch:replace"/>
            <xs:any namespace="##other" processContents="lax”/>
        </xs:sequence>

whereas the 5th edition schema defines this collection of elements as

        <xs:choice minOccurs="1" maxOccurs="unbounded">
            <xs:element name="add" type="add"/>
            <xs:element name="remove" type="remove"/>
            <xs:element name="replace" type="replace"/>
            <xs:any namespace="##other" processContents="lax"/>
        </xs:choice>

the schema definition seems to be correct since

1. it allows multiple <add>, <remove> and <replace> elements to be in the same patch and
2. it allows the <add>, <remove> and <replace> elements to be in any order, i.e. <remove> <add> <replace> <add>

[paulhiggs]

@technogeek00 - any thoughts?

should be related to Issue #53, pull request #72

[paulhiggs]

example_patch_update.xml in clause G.21 seems to be aligned with the use of <choice> as it includes multiple <replace> and <add> elements.

[paulhiggs]

PatchType definition in schema and example are correct - update schema fragment in FDIS

[technogeek00]

@paulhiggs sorry for the confusion, we did a lot of iteration on the schemas outside the FDIS which needed to be ported back into FDIS. Consider the schema repo as the accurate source.

[mikedo]

@technogeek00

we did a lot of iteration on the schemas outside the FDIS which needed to be ported back into FDIS. Consider the schema repo as the accurate source.

Be careful. The text is what was balloted with the understanding that the schemas will be perfected to best reflect the intent of the text. And, a number of edits to the electronic schema were made against DAM1 and AMD1 which changed along the say to FDIS 5th Ed. So, the electronic schema is not authoritative today either.

Take the **FDIS draft spec text (not schemas) as authoritative** and apply other information carefully.

When we are done with the electronic schemas, Thomas will copy them verbatim into the draft FDIS for ballot.

[paulhiggs]

For clarity, we did a lot of work to create the patch schema prior to the FDIS, i.e. making something that was valid for DAM.1 and is a valid use of IETF RFC 5261.

[mikedo]

Yes. Sorry. This was a general comment to others and not really against this particular issue. I have no concerns about this.

[technogeek00]

A sorry from me as well, my comment was too broad as I was also only referring to patching :smile:

[paulhiggs]

Take the FDIS draft spec text (not schemas) as authoritative and apply other information carefully.

@mikedo Regarding the <PatchLocation> element and PatchLoctionType definition, it is stated in clause 5.15.3 MPD patch document that The Patch element may contain one or multiple add, remove, and replace elements as defined in IETF RFC 5261. As such, the schema fragment currently in the FDIS is incorrect since it only allows one of each of add, remove and replace

<xs:schema elementFormDefault="qualified" xmlns:xs="http://www.w3.org/2001/XMLSchema" 
           targetNamespace="urn:mpeg:dash:schema:mpd-patch:2020" attributeFormDefault="unqualified"
  elementFormDefault="qualified" xmlns:xs="http://www.w3.org/2001/XMLSchema"
  xmlns:patch="urn:ietf:params:xml:schema:patch-ops" xmlns="urn:mpeg:dash:schema:mpd-patch:2020">

    <xs:element name="Patch" type="PatchType"/>

    <!-- MPD Type -->
    <xs:complexType name="PatchType">
        <xs:sequence>
            <xs:element ref="patch:add"/>
            <xs:element ref="patch:remove"/>
            <xs:element ref="patch:replace"/>
            <xs:any namespace="##other" processContents="lax”/>
        </xs:sequence>
        <xs:attribute name="mpdId" type="xs:string"/>
        <xs:attribute name="publishTime" type="xs:dateTime"/>
        <xs:attribute name="originalPublishTime" type="xs:dateTime"/>
        <xs:anyAttribute namespace="##other" processContents="lax"/>
    </xs:complexType>
</xs:schema>

The current schema that was developed in preparation of this topic does (and is currently in this repository) is better aligned with the FDIS text

<?xml version="1.0" encoding="UTF-8"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:patch="urn:ietf:params:xml:schema:patch-ops" xmlns="urn:mpeg:dash:schema:mpd-patch:2020" targetNamespace="urn:mpeg:dash:schema:mpd-patch:2020" elementFormDefault="qualified" attributeFormDefault="unqualified">
    <!-- Include patch operations from RFC5261 -->
    <xs:include schemaLocation="https://www.iana.org/assignments/xml-registry/schema/patch-ops.xsd"/>
    <xs:element name="Patch" type="PatchType"/>
    <!-- Patch -->
    <xs:complexType name="PatchType">
        <xs:choice minOccurs="1" maxOccurs="unbounded">
            <xs:element name="add" type="add"/>
            <xs:element name="remove" type="remove"/>
            <xs:element name="replace" type="replace"/>
            <xs:any namespace="##other" processContents="lax"/>
        </xs:choice>
        <xs:attribute name="mpdId" type="xs:string" use="required"/>
        <xs:attribute name="publishTime" type="xs:dateTime" use="required"/>
        <xs:attribute name="originalPublishTime" type="xs:dateTime" use="required"/>
        <xs:anyAttribute namespace="##other" processContents="lax"/>
    </xs:complexType>
</xs:schema>

The cardinality and use of xs:choice permits there to be 1 or more add, remove and/or replace elements whose structure are defined in the patch-ops.xsd schema and RFC 5261 "An Extensible Markup Language (XML) Patch Operations Framework Utilizing XML Path Language (XPath) Selectors"

[mikedo]

The Patch element may contain one or multiple add, remove, and replace elements as defined in IETF RFC 5261.

This sentence is ambiguous. It can be read either way.

@technogeek00 Can you clarify the intent? Is it like the current schema or the proposed new schema?

[paulhiggs]

I think we can look to the examples provided in IETF RFC 7351 "A Media Type for XML Patch Operations" and particularly section2.1 which seems to define a schema for us anyway (all be it in the urn:ietf:rfc:7351 namespace).

[mikedo]

It would have been better long term to use W3C/IETF schemas unmodified in their namespace and just extend it with DASH namespace pieces as needed, and constrain it in the text. We could have delegated such constraints to Schematron. Copying/amending it will be error prone (like this current situation with the (I assume) unintended cardinality and forced ordinality. But that ship sailed in the 4th Ed.

It would be good to hear from the proponents what the intent was.

[technogeek00]

@mikedo The intent is the ability to have any number of add, remove, and/or replace elements at a time. The schema within this repository is aligned with the intentions.

If I recall from our earlier conversation correctly we attempted to directly use the patch operations without copying but found that the only thing the RFC exported was the types, which required us to specify our own underlying elements (RFC 5261 Section 10.3 reference), but I don't think we had found RFC7351 at the time. If that gives us a better reference we could switch to it.

[mikedo]

The intent is the ability to have any number of add, remove, and/or replace elements at a time. The schema within this repository is aligned with the intentions.

If the intent is to allow, e.g. add then remove, then the current schema does not permit this today as Paul mentioned above. It only permits, e.g. add then add.

[technogeek00]

Hmml, let me take a look at this then. Last I thought ,the example included in this repo validated example_patch_update.xml, but if I understand you correctly, the mixture of replace and add shouldn't be allowed?

[paulhiggs]

The use of <choice maxOccurs=unboundee permits child elements to be in any order whereas '<sequence` requires child elements to be in the specified order. I find <choice to be preferable

[technogeek00]

Okay that aligns with what what I expect then from the validation working. Are we aligned then that the current FDIS schema needs to be replaced by DASH-MPD-PATCH.xsd?

[mikedo]

if I understand you correctly, the mixture of replace and add shouldn't be allowed?

I think it should align with what W3C/IETF did. I assume that was the intent, but?

[mikedo]

Are we aligned then that the current FDIS schema needs to be replaced by DASH-MPD-PATCH.xsd?

Remember that the draft FDIS inline schema is just a rough guess. It is not authoritative either now or when we publish. The electronic files are normative and authoritative (the specs says this somewhere). Once this group is happy with the electronic files, they will all be copied verbatim into the FDIS text.

[technogeek00]

I think it should align with what W3C/IETF did. I assume that was the intent, but?

We are mostly aligned with W3C/IETF, exception being minimum choice which agreed can be a spec text constraint and required attributes on the patch document which we could specify with an import of 7351 and extension of the <patch> element

[ZmGorynych]

I would go and use the schema from RFC 7351 directly, and define PatchType as extension of the patch element defined in that RFC.

[paulhiggs]

Okay that aligns with what what I expect then from the validation working. Are we aligned then that the current FDIS schema needs to be replaced by DASH-MPD-PATCH.xsd?

Yes, we know that DASH-MPD-PATCH.xsd is valid and fit for purpose.

I think it should align with what W3C/IETF did. I assume that was the intent, but?

... we could specify with an import of 7351 and extension of the <patch> element

Can we 'extend' an element definition? It it was defined as a element/complexType association then we could extend the complexType.

[ZmGorynych]

Here is the RFC 7351 definition of patch:

<xs:schema targetNamespace="urn:ietf:rfc:7351"
              xmlns:xs="http://www.w3.org/2001/XMLSchema">
       <xs:import schemaLocation="rfc5261.xsd"/>
       <xs:element name="patch">
           <xs:complexType>
               <xs:choice minOccurs="0" maxOccurs="unbounded">
                   <xs:element name="add" type="add"/>
                   <xs:element name="remove" type="remove"/>
                   <xs:element name="replace" type="replace"/>
               </xs:choice>
           </xs:complexType>
       </xs:element>
   </xs:schema>

So extension should be OK, I guess

[paulhiggs]

Here is the RFC 7351 definition of patch:

So extension should be OK, I guess

Please provide the necessary schema!

XML allows defined types to be redefined using xs:redefine, extended using xs:extension for simple content and complex content, or restricted using xs:restriction on simple types, simple content or complex types, but the patch schema you provided is made with an anonymous datatype definition. The schema to 'replace it' would be convoluted and counter productive.

[mikedo]

Is someone working on integrating the substance of the RFC 7351 schema? It allows any number of add, replace, or remove, assuming that was the intent. We're out of time...

[technogeek00]

Because of the anonymous datatype I'm unsure if we are going to specify a RFC 7351 based schema, while its not ideal to duplicate it will likely be more straight-forward to stick with the RFC 5261 based definition as defined here: DASH-MPD-PATCH.xsd

[mikedo]

Understood.