From 6577cc10edb4ca033667aed1622f7d109f369a5d Mon Sep 17 00:00:00 2001 From: azerr Date: Wed, 4 Sep 2019 11:42:02 +0200 Subject: [PATCH] Improve performance and memory by caching XML Schema / DTD Fixes #534 Signed-off-by: azerr --- .../contentmodel/ContentModelPlugin.java | 5 + .../model/ContentModelManager.java | 15 +- .../model/FilesChangedTracker.java | 10 +- .../ContentModelDiagnosticsParticipant.java | 3 +- .../diagnostics/LSPXMLGrammarPool.java | 246 ++++++++++++++++++ .../LSPXMLParserConfiguration.java | 9 +- .../diagnostics/XMLValidator.java | 6 +- .../dtd/contentmodel/CMDTDDocument.java | 32 +-- .../extensions/dtd/utils/DTDUtils.java | 29 +++ .../xsd/contentmodel/CMXSDDocument.java | 40 +-- .../extensions/xsd/utils/XSDUtils.java | 50 ++++ .../java/org/eclipse/lsp4xml/XMLAssert.java | 13 +- .../contentmodel/XMLExternalTest.java | 1 + .../XMLSchemaCompletionExtensionsTest.java | 19 +- .../XMLValidationPoolCacheTest.java | 210 +++++++++++++++ 15 files changed, 607 insertions(+), 81 deletions(-) create mode 100644 org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/LSPXMLGrammarPool.java create mode 100644 org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLValidationPoolCacheTest.java diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/ContentModelPlugin.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/ContentModelPlugin.java index 8f24224983..0665591e88 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/ContentModelPlugin.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/ContentModelPlugin.java @@ -153,4 +153,9 @@ public void stop(XMLExtensionsRegistry registry) { public ContentModelSettings getContentModelSettings() { return cmSettings; } + + public ContentModelManager getContentModelManager() { + return contentModelManager; + } + } diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/ContentModelManager.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/ContentModelManager.java index 78064dd424..373a74aa32 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/ContentModelManager.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/ContentModelManager.java @@ -17,8 +17,10 @@ import java.util.List; import java.util.Map; +import org.apache.xerces.xni.grammars.XMLGrammarPool; import org.eclipse.lsp4xml.dom.DOMDocument; import org.eclipse.lsp4xml.dom.DOMElement; +import org.eclipse.lsp4xml.extensions.contentmodel.participants.diagnostics.LSPXMLGrammarPool; import org.eclipse.lsp4xml.extensions.contentmodel.settings.XMLFileAssociation; import org.eclipse.lsp4xml.extensions.contentmodel.uriresolver.XMLCacheResolverExtension; import org.eclipse.lsp4xml.extensions.contentmodel.uriresolver.XMLCatalogResolverExtension; @@ -43,6 +45,9 @@ public class ContentModelManager { private final XMLCatalogResolverExtension catalogResolverExtension; private final XMLFileAssociationResolverExtension fileAssociationResolver; + // the Grammar Pool to be shared similarly + private final XMLGrammarPool grammarPool; + public ContentModelManager(URIResolverExtensionManager resolverManager) { this.resolverManager = resolverManager; modelProviders = new ArrayList<>(); @@ -53,6 +58,7 @@ public ContentModelManager(URIResolverExtensionManager resolverManager) { resolverManager.registerResolver(catalogResolverExtension); cacheResolverExtension = new XMLCacheResolverExtension(); resolverManager.registerResolver(cacheResolverExtension); + grammarPool = new LSPXMLGrammarPool(); // Use cache by default setUseCache(true); } @@ -180,7 +186,7 @@ private void cache(String key, CMDocument cmDocument) { cmDocumentCache.put(key, cmDocument); } } - + public CMElementDeclaration findInternalCMElement(DOMElement element) throws Exception { return findInternalCMElement(element, element.getNamespaceURI()); } @@ -273,6 +279,9 @@ public void setRootURI(String rootUri) { public void setUseCache(boolean useCache) { cacheResolverExtension.setUseCache(useCache); + if (!useCache) { + grammarPool.clear(); + } } public void registerModelProvider(ContentModelProvider modelProvider) { @@ -283,4 +292,8 @@ public void unregisterModelProvider(ContentModelProvider modelProvider) { modelProviders.remove(modelProvider); } + public XMLGrammarPool getGrammarPool() { + return cacheResolverExtension.isUseCache() ? grammarPool : null; + } + } \ No newline at end of file diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/FilesChangedTracker.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/FilesChangedTracker.java index 99471408d2..1f0c716284 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/FilesChangedTracker.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/model/FilesChangedTracker.java @@ -38,10 +38,12 @@ private static class FileChangedTracker { public FileChangedTracker(Path file) { this.file = file; - try { - lastModified = Files.getLastModifiedTime(file); - } catch (IOException e) { - LOGGER.log(Level.SEVERE, "Get last modified time failed", e); + if (Files.exists(file)) { + try { + lastModified = Files.getLastModifiedTime(file); + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Get last modified time failed", e); + } } } diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/ContentModelDiagnosticsParticipant.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/ContentModelDiagnosticsParticipant.java index 8cdd01582f..678b275588 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/ContentModelDiagnosticsParticipant.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/ContentModelDiagnosticsParticipant.java @@ -44,7 +44,8 @@ public void doDiagnostics(DOMDocument xmlDocument, List diagnostics, XMLEntityResolver entityResolver = xmlDocument.getResolverExtensionManager(); // Process validation XMLValidator.doDiagnostics(xmlDocument, entityResolver, diagnostics, - contentModelPlugin.getContentModelSettings(), monitor); + contentModelPlugin.getContentModelSettings(), + contentModelPlugin.getContentModelManager().getGrammarPool(), monitor); } } diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/LSPXMLGrammarPool.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/LSPXMLGrammarPool.java new file mode 100644 index 0000000000..f460cd272b --- /dev/null +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/LSPXMLGrammarPool.java @@ -0,0 +1,246 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.eclipse.lsp4xml.extensions.contentmodel.participants.diagnostics; + +import org.apache.xerces.impl.dtd.DTDGrammar; +import org.apache.xerces.impl.xs.SchemaGrammar; +import org.apache.xerces.xni.grammars.Grammar; +import org.apache.xerces.xni.grammars.XMLGrammarDescription; +import org.apache.xerces.xni.grammars.XMLGrammarPool; +import org.eclipse.lsp4xml.extensions.contentmodel.model.FilesChangedTracker; +import org.eclipse.lsp4xml.extensions.dtd.utils.DTDUtils; +import org.eclipse.lsp4xml.extensions.xsd.utils.XSDUtils; + +import com.google.common.base.Objects; + +/** + * LSP XML grammar pool. + * + *

+ * This class is a copy/paste of + * {@link org.apache.xerces.util.XMLGrammarPoolImpl.XMLGrammarPoolImpl} from + * Xerces adapated to use .lsp4xml cache. + *

+ * + * @author Jeffrey Rodriguez, IBM + * @author Andy Clark, IBM + * @author Neil Graham, IBM + * @author Pavani Mukthipudi, Sun Microsystems + * @author Neeraj Bajaj, SUN Microsystems + * @author Angelo ZERR + * + * + */ +public class LSPXMLGrammarPool implements XMLGrammarPool { + + private static final int TABLE_SIZE = 11; + + /** Grammars. */ + private final Entry[] fGrammars; + + public LSPXMLGrammarPool() { + this(TABLE_SIZE); + } + + public LSPXMLGrammarPool(int initialCapacity) { + fGrammars = new Entry[initialCapacity]; + } + + @Override + public Grammar[] retrieveInitialGrammarSet(String grammarType) { + // To avoid having trouble with xsi:noNamespaceSchemaLocation, we return nothing + // because in the case of xsi:noNamespaceSchemaLocation + // it's the first XML Schema which was registered as + // xs:noNamespaceSchemaLocation which is used. + return null; + } + + @Override + public void cacheGrammars(String grammarType, Grammar[] grammars) { + for (int i = 0; i < grammars.length; i++) { + putGrammar(grammars[i]); + } + } + + @Override + public Grammar retrieveGrammar(XMLGrammarDescription desc) { + return getGrammar(desc); + } + + private void putGrammar(Grammar grammar) { + synchronized (fGrammars) { + XMLGrammarDescription desc = grammar.getGrammarDescription(); + int hash = hashCode(desc); + int index = (hash & 0x7FFFFFFF) % fGrammars.length; + for (Entry entry = fGrammars[index]; entry != null; entry = entry.next) { + if (entry.hash == hash && equals(entry.desc, desc)) { + entry.grammar = grammar; + return; + } + } + // create a new entry + Entry entry = new Entry(hash, desc, grammar, fGrammars[index]); + fGrammars[index] = entry; + } + } + + /** + * Returns the grammar associated to the specified grammar description. + * Currently, the root element name is used as the key for DTD grammars and the + * target namespace is used as the key for Schema grammars. + * + * @param desc The Grammar Description. + */ + private Grammar getGrammar(XMLGrammarDescription desc) { + synchronized (fGrammars) { + int hash = hashCode(desc); + int index = (hash & 0x7FFFFFFF) % fGrammars.length; + for (Entry entry = fGrammars[index]; entry != null; entry = entry.next) { + if ((entry.hash == hash) && equals(entry.desc, desc)) { + if (entry.isDirty()) { + removeGrammar(entry.desc); + return null; + } + return entry.grammar; + } + } + return null; + } + } + + /** + * Removes the grammar associated to the specified grammar description from the + * grammar pool and returns the removed grammar. Currently, the root element + * name is used as the key for DTD grammars and the target namespace is used as + * the key for Schema grammars. + * + * @param desc The Grammar Description. + * @return The removed grammar. + */ + private Grammar removeGrammar(XMLGrammarDescription desc) { + synchronized (fGrammars) { + int hash = hashCode(desc); + int index = (hash & 0x7FFFFFFF) % fGrammars.length; + for (Entry entry = fGrammars[index], prev = null; entry != null; prev = entry, entry = entry.next) { + if ((entry.hash == hash) && equals(entry.desc, desc)) { + if (prev != null) { + prev.next = entry.next; + } else { + fGrammars[index] = entry.next; + } + Grammar tempGrammar = entry.grammar; + entry.grammar = null; + return tempGrammar; + } + } + return null; + } + } + + @Override + public void lockPool() { + // Do nothing + } + + @Override + public void unlockPool() { + // Do nothing + } + + @Override + public void clear() { + for (int i = 0; i < fGrammars.length; i++) { + if (fGrammars[i] != null) { + fGrammars[i].clear(); + fGrammars[i] = null; + } + } + } + + /** + * This method checks whether two grammars are the same. Currently, we compare + * the root element names for DTD grammars and the target namespaces for Schema + * grammars. The application can override this behaviour and add its own logic. + * + * @param desc1 The grammar description + * @param desc2 The grammar description of the grammar to be compared to + * @return True if the grammars are equal, otherwise false + */ + public boolean equals(XMLGrammarDescription desc1, XMLGrammarDescription desc2) { + String systemId1 = desc1.getExpandedSystemId(); + String systemId2 = desc2.getExpandedSystemId(); + if (systemId1 != null && systemId2 != null) { + return Objects.equal(systemId1, systemId2); + } + return false; // desc1.equals(desc2); + } + + /** + * Returns the hash code value for the given grammar description. + * + * @param desc The grammar description + * @return The hash code value + */ + public int hashCode(XMLGrammarDescription desc) { + return desc.hashCode(); + } + + /** + * This class is a grammar pool entry. Each entry acts as a node in a linked + * list. + */ + protected static final class Entry { + public int hash; + public XMLGrammarDescription desc; + public Grammar grammar; + public Entry next; + private final FilesChangedTracker tracker; + + protected Entry(int hash, XMLGrammarDescription desc, Grammar grammar, Entry next) { + this.hash = hash; + this.desc = desc; + this.grammar = grammar; + this.next = next; + this.tracker = create(grammar); + } + + private static FilesChangedTracker create(Grammar grammar) { + if (grammar instanceof SchemaGrammar) { + return XSDUtils.createFilesChangedTracker((SchemaGrammar) grammar); + } + if (grammar instanceof DTDGrammar) { + return DTDUtils.createFilesChangedTracker((DTDGrammar) grammar); + } + return null; + } + + public boolean isDirty() { + return tracker != null ? tracker.isDirty() : true; + } + + // clear this entry; useful to promote garbage collection + // since reduces reference count of objects to be destroyed + protected void clear() { + desc = null; + grammar = null; + if (next != null) { + next.clear(); + next = null; + } + } + } +} diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java index 39184753c4..d14d31d4ac 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java @@ -12,6 +12,7 @@ import org.apache.xerces.impl.dtd.XMLDTDValidator; import org.apache.xerces.parsers.XIncludeAwareParserConfiguration; import org.apache.xerces.xni.XNIException; +import org.apache.xerces.xni.grammars.XMLGrammarPool; import org.apache.xerces.xni.parser.XMLComponentManager; import org.apache.xerces.xni.parser.XMLConfigurationException; import org.eclipse.lsp4xml.extensions.contentmodel.settings.XMLValidationSettings; @@ -21,6 +22,10 @@ * *
    *
  • disable only DTD validation if required
    • + *
  • disable doctype declaration according validation settings
    • + *
  • disable external entities according validation settings
    • + *
  • manage a custom grammar pool to retrieve compiled XML Schema/DTD from a + * given XML file path
    • *
* */ @@ -28,7 +33,9 @@ class LSPXMLParserConfiguration extends XIncludeAwareParserConfiguration { private final boolean disableDTDValidation; - public LSPXMLParserConfiguration(boolean disableDTDValidation, XMLValidationSettings validationSettings) { + public LSPXMLParserConfiguration(XMLGrammarPool grammarPool, boolean disableDTDValidation, + XMLValidationSettings validationSettings) { + super(null, grammarPool); this.disableDTDValidation = disableDTDValidation; // Disable DOCTYPE declaration if settings is set to true. boolean disallowDocTypeDecl = validationSettings != null ? validationSettings.isDisallowDocTypeDecl() : false; diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/XMLValidator.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/XMLValidator.java index 207ef12fd9..d395918c76 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/XMLValidator.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/diagnostics/XMLValidator.java @@ -23,6 +23,7 @@ import org.apache.xerces.impl.XMLEntityManager; import org.apache.xerces.parsers.SAXParser; import org.apache.xerces.xni.XNIException; +import org.apache.xerces.xni.grammars.XMLGrammarPool; import org.apache.xerces.xni.parser.XMLEntityResolver; import org.apache.xerces.xni.parser.XMLInputSource; import org.apache.xerces.xni.parser.XMLParserConfiguration; @@ -58,12 +59,13 @@ public class XMLValidator { private static final String DTD_NOT_FOUND = "Cannot find DTD ''{0}''.\nCreate the DTD file or configure an XML catalog for this DTD."; public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityResolver, - List diagnostics, ContentModelSettings contentModelSettings, CancelChecker monitor) { + List diagnostics, ContentModelSettings contentModelSettings, XMLGrammarPool grammarPool, + CancelChecker monitor) { try { XMLValidationSettings validationSettings = contentModelSettings != null ? contentModelSettings.getValidation() : null; - LSPXMLParserConfiguration configuration = new LSPXMLParserConfiguration( + LSPXMLParserConfiguration configuration = new LSPXMLParserConfiguration(grammarPool, isDisableOnlyDTDValidation(document), validationSettings); if (entityResolver != null) { diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/dtd/contentmodel/CMDTDDocument.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/dtd/contentmodel/CMDTDDocument.java index 2fc2b00cba..2acb45e14d 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/dtd/contentmodel/CMDTDDocument.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/dtd/contentmodel/CMDTDDocument.java @@ -15,7 +15,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -34,7 +33,7 @@ import org.eclipse.lsp4xml.extensions.contentmodel.model.CMDocument; import org.eclipse.lsp4xml.extensions.contentmodel.model.CMElementDeclaration; import org.eclipse.lsp4xml.extensions.contentmodel.model.FilesChangedTracker; -import org.eclipse.lsp4xml.utils.URIUtils; +import org.eclipse.lsp4xml.extensions.dtd.utils.DTDUtils; /** * DTD document. @@ -145,37 +144,10 @@ public void endContentModel(Augmentations augs) throws XNIException { @Override public Grammar loadGrammar(XMLInputSource source) throws IOException, XNIException { grammar = (DTDGrammar) super.loadGrammar(source); - this.tracker = new FilesChangedTracker(); - updateFilesChangedTracker(); + this.tracker = DTDUtils.createFilesChangedTracker(grammar); return grammar; } - /** - * Update files tracker by adding DTD - */ - private void updateFilesChangedTracker() { - Set trackedGrammars = new HashSet<>(); - updateTracker(grammar, trackedGrammars, tracker); - } - - private static void updateTracker(DTDGrammar grammar, Set trackedGrammars, - FilesChangedTracker tracker) { - if (grammar == null || trackedGrammars.contains(grammar)) { - return; - } - trackedGrammars.add(grammar); - // Track the grammar - String dtdURI = getDTDURI(grammar); - if (dtdURI != null && URIUtils.isFileResource(dtdURI)) { - // The DTD is a file, track when file changed - tracker.addFileURI(dtdURI); - } - } - - private static String getDTDURI(DTDGrammar grammar) { - return grammar.getGrammarDescription().getExpandedSystemId(); - } - public void loadInternalDTD(String internalSubset, String baseSystemId, String systemId) throws XNIException, IOException { // Load empty DTD grammar diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/dtd/utils/DTDUtils.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/dtd/utils/DTDUtils.java index 23dfeebb1c..04869f2e78 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/dtd/utils/DTDUtils.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/dtd/utils/DTDUtils.java @@ -10,10 +10,13 @@ package org.eclipse.lsp4xml.extensions.dtd.utils; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Consumer; +import org.apache.xerces.impl.dtd.DTDGrammar; import org.eclipse.lsp4j.jsonrpc.CancelChecker; import org.eclipse.lsp4xml.dom.DOMDocumentType; import org.eclipse.lsp4xml.dom.DOMNode; @@ -21,6 +24,8 @@ import org.eclipse.lsp4xml.dom.DTDDeclNode; import org.eclipse.lsp4xml.dom.DTDDeclParameter; import org.eclipse.lsp4xml.dom.DTDElementDecl; +import org.eclipse.lsp4xml.extensions.contentmodel.model.FilesChangedTracker; +import org.eclipse.lsp4xml.utils.URIUtils; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -181,4 +186,28 @@ private static boolean isValid(DTDElementDecl elementDecl) { return elementDecl.getNameParameter() != null; } + public static FilesChangedTracker createFilesChangedTracker(DTDGrammar grammar) { + FilesChangedTracker tracker = new FilesChangedTracker(); + Set trackedGrammars = new HashSet<>(); + updateTracker(grammar, trackedGrammars, tracker); + return tracker; + } + + private static void updateTracker(DTDGrammar grammar, Set trackedGrammars, + FilesChangedTracker tracker) { + if (grammar == null || trackedGrammars.contains(grammar)) { + return; + } + trackedGrammars.add(grammar); + // Track the grammar + String dtdURI = getDTDURI(grammar); + if (dtdURI != null && URIUtils.isFileResource(dtdURI)) { + // The DTD is a file, track when file changed + tracker.addFileURI(dtdURI); + } + } + + private static String getDTDURI(DTDGrammar grammar) { + return grammar.getGrammarDescription().getExpandedSystemId(); + } } diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/xsd/contentmodel/CMXSDDocument.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/xsd/contentmodel/CMXSDDocument.java index 46299b4289..237a5ac1ee 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/xsd/contentmodel/CMXSDDocument.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/xsd/contentmodel/CMXSDDocument.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.Vector; import java.util.logging.Level; import java.util.logging.Logger; @@ -85,41 +84,24 @@ public CMXSDDocument(XSModel model, String uri) { this.model = model; this.elementMappings = new HashMap<>(); this.uri = uri; - this.tracker = new FilesChangedTracker(); - updateFilesChangedTracker(); + this.tracker = createFilesChangedTracker(model); } /** * Update files tracker by adding all XML Schema (root and imported) + * + * @return */ - private void updateFilesChangedTracker() { - Set trackedGrammars = new HashSet<>(); - XSNamespaceItemList grammars = model.getNamespaceItems(); - for (int i = 0; i < grammars.getLength(); i++) { - SchemaGrammar grammar = getSchemaGrammar(grammars.item(i)); - updateTracker(grammar, trackedGrammars, tracker); - } - } - - private static void updateTracker(SchemaGrammar grammar, Set trackedGrammars, - FilesChangedTracker tracker) { - if (grammar == null || trackedGrammars.contains(grammar)) { - return; - } - trackedGrammars.add(grammar); - // Track the grammar - String schemaURI = getSchemaURI(grammar); - if (schemaURI != null && URIUtils.isFileResource(schemaURI)) { - // The schema is a file, track when file changed - tracker.addFileURI(schemaURI); - } - // Track the imported grammars - Vector importedGrammars = grammar.getImportedGrammars(); - if (importedGrammars != null) { - for (Object importedGrammar : importedGrammars) { - updateTracker((SchemaGrammar) importedGrammar, trackedGrammars, tracker); + private static FilesChangedTracker createFilesChangedTracker(XSModel model) { + Set grammars = new HashSet<>(); + XSNamespaceItemList namespaces = model.getNamespaceItems(); + for (int i = 0; i < namespaces.getLength(); i++) { + SchemaGrammar grammar = getSchemaGrammar(namespaces.item(i)); + if (grammar != null) { + grammars.add(grammar); } } + return XSDUtils.createFilesChangedTracker(grammars); } @Override diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/xsd/utils/XSDUtils.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/xsd/utils/XSDUtils.java index 6e6a180689..86eefdfb6a 100644 --- a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/xsd/utils/XSDUtils.java +++ b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/xsd/utils/XSDUtils.java @@ -10,15 +10,23 @@ package org.eclipse.lsp4xml.extensions.xsd.utils; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import java.util.Vector; import java.util.function.BiConsumer; +import org.apache.xerces.impl.xs.SchemaGrammar; +import org.apache.xerces.xs.StringList; import org.eclipse.lsp4j.jsonrpc.CancelChecker; import org.eclipse.lsp4xml.dom.DOMAttr; import org.eclipse.lsp4xml.dom.DOMDocument; import org.eclipse.lsp4xml.dom.DOMElement; import org.eclipse.lsp4xml.dom.DOMNode; +import org.eclipse.lsp4xml.extensions.contentmodel.model.FilesChangedTracker; import org.eclipse.lsp4xml.utils.StringUtils; +import org.eclipse.lsp4xml.utils.URIUtils; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NamedNodeMap; @@ -345,4 +353,46 @@ public static boolean isXSTargetElement(Element element) { public static boolean isXSAttribute(DOMElement element) { return "attribute".equals(element.getLocalName()); } + + public static FilesChangedTracker createFilesChangedTracker(SchemaGrammar grammar) { + return createFilesChangedTracker(Collections.singleton(grammar)); + } + + public static FilesChangedTracker createFilesChangedTracker(Set grammars) { + FilesChangedTracker tracker = new FilesChangedTracker(); + Set trackedGrammars = new HashSet<>(); + Set trackedURIs = new HashSet<>(); + for (SchemaGrammar grammar : grammars) { + updateTracker(grammar, trackedGrammars, trackedURIs, tracker); + } + return tracker; + } + + private static void updateTracker(SchemaGrammar grammar, Set trackedGrammars, + Set trackedURIs, FilesChangedTracker tracker) { + if (grammar == null || trackedGrammars.contains(grammar)) { + return; + } + trackedGrammars.add(grammar); + // Loop for all XML Schema (root + included) to track it + StringList locations = grammar.getDocumentLocations(); + for (int i = 0; i < locations.getLength(); i++) { + String location = locations.item(i); + if (!trackedURIs.contains(location)) { + trackedURIs.add(location); + } + if (location != null && URIUtils.isFileResource(location)) { + // The schema is a file, track when file changed + tracker.addFileURI(location); + } + } + // Track the imported grammars + Vector importedGrammars = grammar.getImportedGrammars(); + if (importedGrammars != null) { + for (Object importedGrammar : importedGrammars) { + updateTracker((SchemaGrammar) importedGrammar, trackedGrammars, trackedURIs, tracker); + } + } + } + } diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/XMLAssert.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/XMLAssert.java index 0399155d96..f14e62d4c8 100644 --- a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/XMLAssert.java +++ b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/XMLAssert.java @@ -314,9 +314,15 @@ public static void testDiagnosticsFor(String xml, String catalogPath, Consumer configuration, String fileURI, boolean filter, ContentModelSettings settings, Diagnostic... expected) { + testDiagnosticsFor(new XMLLanguageService(), xml, catalogPath, configuration, fileURI, filter, settings, + expected); + } + + public static void testDiagnosticsFor(XMLLanguageService xmlLanguageService, String xml, String catalogPath, + Consumer configuration, String fileURI, boolean filter, ContentModelSettings settings, + Diagnostic... expected) { TextDocument document = new TextDocument(xml, fileURI != null ? fileURI : "test.xml"); - XMLLanguageService xmlLanguageService = new XMLLanguageService(); DOMDocument xmlDocument = DOMParser.getInstance().parse(document, xmlLanguageService.getResolverExtensionManager()); xmlLanguageService.setDocumentProvider((uri) -> xmlDocument); @@ -895,14 +901,15 @@ public static void assertRename(String value, String newText) throws BadLocation assertRename(value, newText, Collections.emptyList()); } - public static void assertRename(String value, String newText, List expectedEdits) throws BadLocationException { + public static void assertRename(String value, String newText, List expectedEdits) + throws BadLocationException { int offset = value.indexOf("|"); value = value.substring(0, offset) + value.substring(offset + 1); DOMDocument document = DOMParser.getInstance().parse(value, "test://test/test.html", null); Position position = document.positionAt(offset); - + XMLLanguageService languageService = new XMLLanguageService(); WorkspaceEdit workspaceEdit = languageService.doRename(document, position, newText); List actualEdits = workspaceEdit.getChanges().get("test://test/test.html"); diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLExternalTest.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLExternalTest.java index 1387dc7e0a..e3a0d9b4e8 100644 --- a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLExternalTest.java +++ b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLExternalTest.java @@ -93,6 +93,7 @@ public void externalDTDTest() throws InterruptedException, IOException { Thread.sleep(threadSleepMs); Assert.assertEquals(2, actualDiagnostics.size()); + Assert.assertFalse(actualDiagnostics.get(1).getDiagnostics().isEmpty()); Assert.assertEquals("MSG_ELEMENT_NOT_DECLARED", actualDiagnostics.get(1).getDiagnostics().get(0).getCode()); } diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLSchemaCompletionExtensionsTest.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLSchemaCompletionExtensionsTest.java index 183f44c560..d39b30571e 100644 --- a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLSchemaCompletionExtensionsTest.java +++ b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLSchemaCompletionExtensionsTest.java @@ -31,11 +31,14 @@ import org.eclipse.lsp4xml.settings.XMLCompletionSettings; import org.junit.Test; +import com.google.common.io.MoreFiles; +import com.google.common.io.RecursiveDeleteOption; + /** * XSD completion tests. * */ -public class XMLSchemaCompletionExtensionsTest { +public class XMLSchemaCompletionExtensionsTest extends BaseFileTempTest { @Test public void completionInRoot() throws BadLocationException { @@ -338,17 +341,12 @@ public void completionWithoutStartBracket() throws BadLocationException { @Test public void completionWithXMLSchemaContentChanged() throws Exception { // This https://github.com/angelozerr/lsp4xml/issues/194 for the test scenario - Path dir = Paths.get("target/xsd/"); - if (!Files.isDirectory(dir)) { - Files.createDirectory(dir); - } - Files.deleteIfExists(Paths.get(dir.toString(), "resources.xsd")); - + String xsdPath = tempDirUri.getPath() + "/resources.xsd"; XMLLanguageService xmlLanguageService = new XMLLanguageService(); String xml = "\r\n" + "\r\n" + // + + "xsi:noNamespaceSchemaLocation=\"" + xsdPath + "\">\r\n" + // " \r\n" + // " \r\n" + // " \r\n" + // @@ -373,7 +371,8 @@ public void completionWithXMLSchemaContentChanged() throws Exception { + " \r\n" + " \r\n" + " \r\n" + " \r\n" + ""; - Files.write(Paths.get("target/xsd/resources.xsd"), schema.getBytes()); + + createFile(xsdPath, schema); XMLAssert.testCompletionFor(xmlLanguageService, xml, null, null, "target/resources.xml", 5, false, c("variant", "variant=\"\"")); @@ -393,7 +392,7 @@ public void completionWithXMLSchemaContentChanged() throws Exception { + " \r\n" // + " \r\n" + " \r\n" + " \r\n" + ""; - Files.write(Paths.get("target/xsd/resources.xsd"), schema.getBytes()); + createFile(xsdPath, schema); XMLAssert.testCompletionFor(xmlLanguageService, xml, null, null, "target/resources.xml", 4, false); } diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLValidationPoolCacheTest.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLValidationPoolCacheTest.java new file mode 100644 index 0000000000..7de6eb2a37 --- /dev/null +++ b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLValidationPoolCacheTest.java @@ -0,0 +1,210 @@ +/** + * Copyright (c) 2019 Red Hat, Inc. and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v2.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v20.html + * + * Contributors: + * Red Hat Inc. - initial API and implementation + */ +package org.eclipse.lsp4xml.extensions.contentmodel; + +import static org.eclipse.lsp4xml.XMLAssert.d; + +import java.io.IOException; + +import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4xml.XMLAssert; +import org.eclipse.lsp4xml.extensions.contentmodel.participants.XMLSchemaErrorCode; +import org.eclipse.lsp4xml.extensions.contentmodel.settings.ContentModelSettings; +import org.eclipse.lsp4xml.services.XMLLanguageService; +import org.junit.Test; + +/** + * Test to check the LSP XML Grammar Pool. + * + * @author Angelo ZERR + * + */ +public class XMLValidationPoolCacheTest extends BaseFileTempTest { + + @Test + public void twoNoNamespaceSchemaLocation() { + XMLLanguageService xmlLanguageService = new XMLLanguageService(); + + // Validate 2 xml bound with noNamespaceSchemaLocation + // By default Xerces XMLGrammarPool can store only the first XML Schema bound + // with noNamespaceSchemaLocation + // This test validate 2 XML file bound with 2 different XML Schema using + // noNamespaceSchemaLocation + + String xml = " "; + Diagnostic d = d(0, 143, 0, 144, XMLSchemaErrorCode.cvc_complex_type_2_1); + testDiagnosticsFor(xmlLanguageService, xml, d); + // validate a second time (to use cached Grammar) + testDiagnosticsFor(xmlLanguageService, xml, d); + + xml = ""; + Diagnostic patternValid = d(3, 6, 3, 9, XMLSchemaErrorCode.cvc_pattern_valid); + Diagnostic cvcAttribute3 = d(3, 6, 3, 9, XMLSchemaErrorCode.cvc_attribute_3); + testDiagnosticsFor(xmlLanguageService, xml, patternValid, cvcAttribute3); + } + + @Test + public void includedSchemaLocation() throws IOException { + XMLLanguageService xmlLanguageService = new XMLLanguageService(); + + String schemaAPath = tempDirUri.getPath() + "/SchemaA.xsd"; + String schemaA = "\r\n" + // + "\r\n" + + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + ""; + createFile(schemaAPath, schemaA); + + String schemaBPath = tempDirUri.getPath() + "/SchemaB.xsd"; + String schemaB = "\r\n" + // + "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + "\r\n" + // + ""; + createFile(schemaBPath, schemaB); + + String xml = "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // <-- error FooBar missing + " \r\n" + // + ""; + Diagnostic d = d(2, 2, 5, XMLSchemaErrorCode.cvc_complex_type_2_4_b); + testDiagnosticsFor(xmlLanguageService, xml, d); + + // Update Schema A -> remove sequence from Bar + schemaA = "\r\n" + // + "\r\n" + + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + // " \r\n" + // <-- remove sequence from Bar + // " \r\n" + // + // " \r\n" + // + // " \r\n" + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + ""; + createFile(schemaAPath, schemaA); + xml = "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // <-- error FooBar must have no character + " \r\n" + // + ""; + d = d(2, 6, 4, 1, XMLSchemaErrorCode.cvc_complex_type_2_1); + testDiagnosticsFor(xmlLanguageService, xml, d); + + // Update Schema A as init + schemaA = "\r\n" + // + "\r\n" + + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + "\r\n" + // + " \r\n" + // + ""; + createFile(schemaAPath, schemaA); + xml = "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // <-- error XMLElement from Schema A is not declared + " \r\n" + // + " \r\n" + // + " "; + d = d(4, 3, 8, XMLSchemaErrorCode.cvc_complex_type_2_4_b); + testDiagnosticsFor(xmlLanguageService, xml, d); + + // Update included Schema B + schemaB = "\r\n" + // + "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + //" \r\n" + // + //" \r\n" + // + //" \r\n" + // + " \r\n" + // + " \r\n" + // + "\r\n" + // + ""; + createFile(schemaBPath, schemaB); + + xml = "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // <-- error AType must have no character + " \r\n" + // + " \r\n" + // + " "; + d = d(4, 9, 6, 2, XMLSchemaErrorCode.cvc_complex_type_2_1); + testDiagnosticsFor(xmlLanguageService, xml, d); + + } + + private static void testDiagnosticsFor(XMLLanguageService xmlLanguageService, String xml, Diagnostic... expected) { + String catalogPath = "src/test/resources/catalogs/catalog.xml"; + ContentModelSettings settings = new ContentModelSettings(); + XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, catalogPath, null, null, true, settings, expected); + } +}