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

Completion from local xsd seems cached too agressively #194

Closed
fbricon opened this issue Nov 5, 2018 · 8 comments
Closed

Completion from local xsd seems cached too agressively #194

fbricon opened this issue Nov 5, 2018 · 8 comments
Assignees
Labels
bug Something isn't working completion This issue or enhancement is related to completion support XSD
Milestone

Comments

@fbricon
Copy link
Contributor

fbricon commented Nov 5, 2018

Given an xml document:

<?xml version="1.0" encoding="UTF-8"?>
<resources variant="foo" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="resources.xsd">
    <resource name="res00" >
        <property name="propA" value="..." />
        <property name="propB" value="..." />
    </resource>
    <resource name="" >
        <property name="" value="..." />
        <property name="" value="..." />
    </resource> 
</resources>

and its resources.xsd:

<?xml version="1.0"?>
<xs:schema elementFormDefault="qualified" xmlns:xs="http://www.w3.org/2001/XMLSchema">
    <xs:complexType name="property">
        <xs:attribute name="name" type="xs:string" use="required"/>
        <xs:attribute name="value" type="xs:string" />
    </xs:complexType>
    <xs:complexType name="resource">
        <xs:sequence>
            <xs:element name="property" type="property" minOccurs="0" maxOccurs="unbounded" />
        </xs:sequence>
        <xs:attribute name="name" type="xs:string" use="required" />
    </xs:complexType>

    <xs:element name="resources" type="resources">
        
    </xs:element>
    <xs:complexType name="resources">
            <xs:sequence>
                <xs:element name="resource" type="resource" minOccurs="0" maxOccurs="unbounded" />
            </xs:sequence>
            <xs:attribute name="variant" type="xs:string" use="required"/>
        </xs:complexType>
</xs:schema>

If you remove variant="foo" from the xml doc, a validation error pops up, as expected. Completion within the resources node shows you can add variant.
Now if you modify resources.xsd, rename variant into something, save the file, then make a change in the xml document, you'll see 2 errors, as expected:

  • cvc-complex-type.4: Attribute 'something' must appear on element 'resources'.
  • cvc-complex-type.3.2.2: Attribute 'variant' is not allowed to appear in element 'resources'.

Remove the variant attribute from <resources and autocomplete to get the new attribute: variant is still shown, not something. Closing/reopening all documents don't change anything, the old attribute name is definitely cached in memory. Only way to get past it is to restart the server.

@fbricon fbricon added bug Something isn't working completion This issue or enhancement is related to completion support XSD labels Nov 5, 2018
@fbricon fbricon changed the title Local completion from xsd seems cached too agressively Completion from local xsd seems cached too agressively Nov 5, 2018
@angelozerr
Copy link
Contributor

Indeed @fbricon I know this problem. It's because cache is done for completion in a very very basic mean at https://github.com/angelozerr/lsp4xml/blob/master/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/ContentModelManager.java#L138

To fix this issue, we should check if file last modified has changed (the first step), but XML Schema can be too an URL (we could considerer to cache without checking last modified).

Check last modified of the XML Schema to disable cache, will work for your resources.xsd, but if the XML Schema include a B.xsd XML Shema, we must check too that B.xsd last modified has changed.

An another very simple solution is not to cache XML Schema.

@fbricon
Copy link
Contributor Author

fbricon commented Nov 5, 2018

Depends if not caching local schemas has a big impact on perf

@angelozerr
Copy link
Contributor

Depends if not caching local schemas has a big impact on perf

It's like validation (which doesn't use cache now). If XML Schema is on file system, it's fast.

@fbricon
Copy link
Contributor Author

fbricon commented Nov 5, 2018

so +1 to not caching local schema files. Remote URLs should be cached though

@fbricon
Copy link
Contributor Author

fbricon commented Nov 5, 2018

A more robust solution could be implemented later, that'd support remote schemas

@angelozerr
Copy link
Contributor

@fbricon I have removed cache for completion. Your usecase should work now. We need to write tests for that.

@angelozerr angelozerr added this to the v0.0.2 milestone Nov 6, 2018
@angelozerr angelozerr self-assigned this Nov 6, 2018
@fbricon
Copy link
Contributor Author

fbricon commented Nov 6, 2018

Yes I confirm it works. Please add the tests so we can close it

angelozerr added a commit that referenced this issue Nov 6, 2018
@angelozerr
Copy link
Contributor

Yes I confirm it works. Please add the tests so we can close it

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working completion This issue or enhancement is related to completion support XSD
Projects
None yet
Development

No branches or pull requests

2 participants