ropensci / emld

:package: JSON-LD representation of EML
https://docs.ropensci.org/emld
Other
13 stars 6 forks source link

Consider changing default `xsi:schemaLocation` value to point to a remote copy of the eml.xsd #44

Closed amoeba closed 4 years ago

amoeba commented 4 years ago

As discussed in https://github.com/ropensci/EML/issues/292, the current default behavior of emld is to set the xsi:schemaLocation to values like https:://ecoinformatics.org/eml-2.2.0/ eml.xsd which matches the examples in https://github.com/NCEAS/eml but isn't useful for validation tools that follow remote URIs to find a copy of the schema. @scelmendorf had to set a custom value to get validation to work which I'd argue isn't desirable.

We should discuss whether it makes sense to set a value like https://eml.ecoinformatics.org/eml-2.2.0/eml.xsd instead of just eml.xsd so, by default and with no tweaking from the user, validation tools like Oxygen or any XML-aware text editors can do validation of documents EML/emld produces.

What do others think about this?

cboettig commented 4 years ago

Thanks Bryce! I'm for moving to https://eml.ecoinformatics.org/eml-2.2.0/eml.xsd. @mbjones? others?

mbjones commented 4 years ago

LGTM. As @amoeba said, it is an optional attribute, so you could also just leave it out altogether. But your proposal seems good too.

amoeba commented 4 years ago

PR sent. @cboettig take a look when you get a chance?

mbjones commented 4 years ago

@cboettig asked a question on this xsi:schemaLocation validation issue over in https://github.com/ropensci/emld/pull/51#issuecomment-579077328. I'm going to answer here to keep the thread together. Sorry this turned out to be a bit longwinded. Hopefully its useful.

I don't think we should be doing any string parsing of xsi:schemaLocation to determine which submodules should be used for validation. XMLSchema already fully handles assigning each element and attribute to a specific namespace with a well-defined (albeit complicated) set of rules. You should never have to guess what namespace somebody is asserting for a root level element via string parsing. Instead, we should 1) use proper XML parsing rules to determine the type of the root element, 2) use the formal namespace xs:import and xs:include statements to track namespace dependencies in schemas, and 3) use a local schema cache based on namespace identifier rather than ever using schemaLocation to resolve a schema. For example, the following root element is explicitly rooted at the EML 2.2.0 eml element, which is matchable via its namespace and only its namespace to its XMLSchema structure:

<?xml version="1.0"?>
<eml:eml
    packageId="doi:10.xxxx/eml.1.1" system="knb"
    xmlns:eml="https://eml.ecoinformatics.org/eml-2.2.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"

    <dataset id="dataset-01">
    ...
    </dataset>
</eml>

Validation of that structure explicitly starts with validation of the root eml:eml element, which resolves to the namespace https://eml.ecoinformatics.org/eml-2.2.0 with local name eml. The XML parser will tell us all of that explicitly. Even if I were to set xsi:schemaLocation="https://eml.ecoinformatics.org/eml-2.2.0 https://hoser.com/xsd/2.1.1/eml.xsd" on the root element, it would be a mistake to use that file to parse the document. It should be ignored as incorrect user data.

In contrast, this structure explicitly is rooted at a different root schema, namely https://eml.ecoinformatics.org/eml-dataset-2.2.0 with local element name dataset:

<?xml version="1.0"?>
<ns2:dataset id="dataset-01" 
    xmlns:ns2="https://eml.ecoinformatics.org/eml-dataset-2.2.0">
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
...
</dataset>

This is everything one needs to know to know its type, and can be resolved as long as the validator has been configured with a canonical set of schemas for each of the referenced namespaces. This is usually done via an XML Catalog file, but each parser provides multiple mechanisms to pass in namespace to schema-file hashes. If a local schema hasn't been provided for a referenced namespace, an error is rightfully generated. And you might note that the namespace of the dataset element in the first example (it's absent/null) is NOT the same as the namespace of the dataset element in the second example, due to EML's use of xs:elementFormDefault=unqualified. See https://stackoverflow.com/questions/1463138/what-does-elementformdefault-do-in-xsd

String parsing for namespaces is highly error prone, because there are a lot of rules about how to determine the namespace for a given XML element or attribute. At a minimum, you have to consider the prefix and namespace string definitions, the xs:import and xs:include statements in the xsd files themselves, and the values of the xs:elementFormDefault and xs:attributeFormDefault values in the schema, and the current value of xs:targetNamespace and the default namespace (set with e.g., xmlns=foo) as you traverse the element tree. These can all change dynamically as you traverse the element tree, inheriting from parents but potentially being overridden in each child node, and then being reset as one steps back up the element tree. And they all have a large impact on namespace interpretation. I think its essentially impossible to do this right without implementing the full XMLSchema grammer, in which case you've reinvented a formal XML Parser.

The only piece of information that can always be ignored by an XML validator is xsi:schemaLocation. So, bottom line, I advocate for not doing any string parsing or logic around xsi:schemaLocation. Let's just ignore it.

cboettig commented 4 years ago

@mbjones Thanks for this, I agree :100: on this, which is why I wanted to flag it again before cutting the next release. I think the key issue here is where you observe:

This is everything one needs to know to know its type, and can be resolved as long as the validator has been configured with a canonical set of schemas for each of the referenced namespaces

The validator in emld has not been configured with "a canonical set of schemas for each of the referenced namespaces". Currently it is guessing these from schemaLocation, which I agree is wrong. So, action items:

  1. should I hold off on cutting a new release until that's fixed? (I believe CRAN still rate-limits the frequency at which they want updates to established packages, though maybe with the automated system they aren't as picky now?)
  2. Do you think you or @amoeba or someone would have a chance to send a PR to fix this?

(I do wish I had time to dive in here more -- its always been a bit frustrating that I haven't been able to secure funding or academic credit that would let me allocate more time to this! But I do really really appreciate all the excellent pull requests here and on EML package too!)