Skip to content
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

Can't use XML catalog with XSD files that have <xs:include /> #914

Closed
rnathuji opened this issue Oct 20, 2020 · 2 comments · Fixed by #919
Closed

Can't use XML catalog with XSD files that have <xs:include /> #914

rnathuji opened this issue Oct 20, 2020 · 2 comments · Fixed by #919
Assignees
Labels
bug Something isn't working catalog validation
Milestone

Comments

@rnathuji
Copy link

While using the XML catalog with XSD configuration with the vscode-xml plugin, we encountered an issue where XSD files that utilize <xs:include /> tags don't work. Take, for example, the following test.xml with test.xsd which includes test-include.xsd:

test.xml

<?xml version="1.0" encoding="UTF-8"?>
<document xmlns="http://foobar.com/test">
  <page>
    <title>Page 1</title>
    <content>Content for page 1</content>
  </page>
  <page>
    <title>Page 2</title>
    <content>Content for page 2</content>
  </page>
</document>

test.xsd

<?xml version="1.0" encoding="UTF-8"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" elementFormDefault="qualified" targetNamespace="http://foobar.com/test" xmlns:test="http://foobar.com/test">
  <xs:include schemaLocation="test-include.xsd"/>
</xs:schema>

test-include.xsd

<?xml version="1.0" encoding="UTF-8"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" elementFormDefault="qualified" targetNamespace="http://foobar.com/test" xmlns:test="http://foobar.com/test">
  <xs:element name="document">
    <xs:complexType>
      <xs:sequence>
        <xs:element maxOccurs="unbounded" ref="test:page"/>
      </xs:sequence>
    </xs:complexType>
  </xs:element>
  <xs:element name="page" type="test:page-content"/>
  <xs:complexType name="page-content">
    <xs:sequence>
      <xs:element ref="test:title"/>
      <xs:element ref="test:content"/>
    </xs:sequence>
  </xs:complexType>
  <xs:element name="title" type="xs:string"/>
  <xs:element name="content" type="xs:string"/>
</xs:schema>

With a catalog definition as follows:

test_catalog.xml

<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog">
  <uri
      name="http://foobar.com/test"
      uri="./test.xsd" />
</catalog>

Opening test.xml results in:

cvc-elt.1.a: Cannot find the declaration of element 'document'.

A "merged" test-merged.xsd, however, works just fine:

test-merged.xsd

<?xml version="1.0" encoding="UTF-8"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" elementFormDefault="qualified" targetNamespace="http://foobar.com/test" xmlns:test="http://foobar.com/test">
  <xs:element name="document">
    <xs:complexType>
      <xs:sequence>
        <xs:element maxOccurs="unbounded" ref="test:page"/>
      </xs:sequence>
    </xs:complexType>
  </xs:element>
  <xs:element name="page" type="test:page-content"/>
  <xs:complexType name="page-content">
    <xs:sequence>
      <xs:element ref="test:title"/>
      <xs:element ref="test:content"/>
    </xs:sequence>
  </xs:complexType>
  <xs:element name="title" type="xs:string"/>
  <xs:element name="content" type="xs:string"/>
</xs:schema>

It seems this may be due to the way resolution occurs in XMLCatalogResolver based upon the namespace when invoked here. As a quick test to help confirm the root cause, we did find that making a similar change as the one in this related SO post to resolveEntity in XMLCatalogResolverExtension.java seemed to correct the problem.

@angelozerr
Copy link
Contributor

@rnathuji thank a lot for reporting this issue and give us a very detailed information.

Is there any chance to have a PR and tests with your suggestion fix?

@rnathuji
Copy link
Author

rnathuji commented Oct 21, 2020

@angelozerr - Sure, I'd be happy to help with a PR. I may, however, need some guidance due to my lack of familiarity 😅 . I've created a draft PR for the time being which contains the quick and dirty change that allowed things to work while debugging / testing. I'd definitely welcome feedback as far as:

  1. Whether this is even the proper place to try and hook this logic in
  2. The use of resourceIdentifier.getExpandedSystemId() != null as a conditional gate -- It feels brittle to me in that I don't know what underlying assumptions that makes

I can also start looking at the test additions, but LMK if there are any major course corrections that should be made in terms of the implementation approach.

angelozerr pushed a commit to angelozerr/lemminx that referenced this issue Oct 23, 2020
Fixes eclipse#914

Signed-off-by: azerr <azerr@redhat.com>
angelozerr pushed a commit to angelozerr/lemminx that referenced this issue Oct 23, 2020
Fixes eclipse#914

Signed-off-by: azerr <azerr@redhat.com>
angelozerr pushed a commit to angelozerr/lemminx that referenced this issue Oct 23, 2020
Fixes eclipse#914

Signed-off-by: azerr <azerr@redhat.com>
@angelozerr angelozerr self-assigned this Oct 23, 2020
@angelozerr angelozerr added this to the 0.14.0 milestone Oct 23, 2020
angelozerr pushed a commit to angelozerr/lemminx that referenced this issue Oct 23, 2020
Fixes eclipse#914

Signed-off-by: azerr <azerr@redhat.com>
angelozerr pushed a commit to angelozerr/lemminx that referenced this issue Oct 23, 2020
Fixes eclipse#914

Signed-off-by: azerr <azerr@redhat.com>
angelozerr pushed a commit to angelozerr/lemminx that referenced this issue Oct 23, 2020
Fixes eclipse#914

Signed-off-by: azerr <azerr@redhat.com>
@angelozerr angelozerr added bug Something isn't working catalog validation labels Oct 23, 2020
datho7561 pushed a commit that referenced this issue Oct 26, 2020
Fixes #914

Signed-off-by: azerr <azerr@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working catalog validation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants