-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add examples and enable validation #115
Add examples and enable validation #115
Conversation
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.
Most of my review comments are explanations, the biggest issue to discuss in my opinion is https://github.com/VDVde/OJP/pull/115/files#diff-0e9e7bd8d07a9a105056d18de9859da979d8cc797cdf56ef48f164970d1f4664R2
This branch is based on #113 which should be merged first.
@@ -0,0 +1,137 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
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.
- Add "xml/xml.xsd" referenced by "./siri_utility/siri_types-v2.0.xsd"
using https://www.w3.org/2005/08/xml.xsd
@@ -0,0 +1,23 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> | |||
<OJP xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.siri.org.uk/siri" xmlns:ojp="http://www.vdv.de/ojp" version="1.1-dev" xsi:schemaLocation="http://www.siri.org.uk/siri ../../OJP.xsd"> |
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.
The definition xsi:schemaLocation="http://www.siri.org.uk/siri ../../OJP.xsd"
is kind of a hack and up to now my biggest concern with the pull request.
SIRI is defined by http://www.siri.org.uk/siri
and usually does not know anything about OJP. Only by referencing ../../OJP.xsd
here the root element definition <OJP>
and the other elements from OJP.xsd
are defined.
So OJP is based on SIRI but my XML/XSD knowledge is not good enough yet to propose a valid solution: Do we need an own namespace for this SIRI enhancement of OJP.xsd
(apart from xmlns:ojp="http://www.vdv.de/ojp"
which contains our own definitions not based on SIRI)?
Defining a schemaLocation
is optional, also see https://stackoverflow.com/questions/4998063/one-xml-namespace-equals-one-and-only-one-schema-file:
Some XML processors tolerate multiple for the same namespace, and essentially amalgamate all of the schemaLocation into a single target.
Other processors are stricter, and decide that only one per target namespace is valid. I think this is more correct, when you consider that schemaLocation is optional.
In addition to the VS and xmllint examples you gave, Xerces-J is also super-strict, and ignores subsequent for the same target namespace, giving much the same error as xmllint does. XML Spy, on the other hand, is much more permissive (but then, XML Spy's validation is notoriously flaky)
To be safe, you should not have these multiple imports. A given namespace should have a single "master" document, which in turn has an for each sub-document. This master is often highly artificial, acting only as a container. for these sub-documents.
- Examples are OJP-1.0 files which have been adapted for 1.1-dev - Use "./siri_model/siri_all-v2.0.xsd" for siri namespace everywhere as xmllint and Xerces-J only allow one <import> per target namespace - Import ifopt namespace before siri, otherwise xmllint fails because the first found ifopt import statement in SIRI does not refer to "./ifopt/ifopt_allStopPlace-v0.3.xsd" - Add "xml/xml.xsd" referenced by "./siri_utility/siri_types-v2.0.xsd" using https://www.w3.org/2005/08/xml.xsd
767e054
to
79cea24
Compare
Ready for review. As written before: #115 (comment) I am not sure if it is a valid behaviour to "redefine" the SIRI namespace like that.. |
Fixes: #70