From 629777c92ff79c4a87677d14c256ddb70d86082f Mon Sep 17 00:00:00 2001 From: dshimo Date: Thu, 5 Oct 2023 13:36:47 -0500 Subject: [PATCH 01/14] init push --- lemminx-liberty/pom.xml | 2 +- .../lemminx/LibertyCodeActionParticipant.java | 2 + .../lemminx/LibertyDiagnosticParticipant.java | 77 ++++++++-- .../lemminx/codeactions/AddFeature.java | 79 ++++++++++ .../lemminx/data/FeatureListGraph.java | 144 ++++++++++++++++++ .../lemminx/data/FeatureListNode.java | 48 ++++++ .../lemminx/models/feature/Feature.java | 20 +++ .../lemminx/services/FeatureService.java | 48 ++++-- .../io/openliberty/LibertyDiagnosticTest.java | 74 ++++++++- .../io/openliberty/LibertyFeatureTest.java | 13 ++ liberty-ls/pom.xml | 2 +- 11 files changed, 478 insertions(+), 31 deletions(-) create mode 100644 lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java create mode 100644 lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java create mode 100644 lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java diff --git a/lemminx-liberty/pom.xml b/lemminx-liberty/pom.xml index 28698ab8..263026b8 100644 --- a/lemminx-liberty/pom.xml +++ b/lemminx-liberty/pom.xml @@ -4,7 +4,7 @@ io.openliberty.tools liberty-langserver-lemminx jar - 2.0.2-SNAPSHOT + 2.1-SNAPSHOT lemminx-liberty https://github.com/OpenLiberty/liberty-language-server diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java index 0d8a93fa..a2092722 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java @@ -24,6 +24,7 @@ import org.eclipse.lsp4j.jsonrpc.CancelChecker; import io.openliberty.tools.langserver.lemminx.codeactions.AddAttribute; +import io.openliberty.tools.langserver.lemminx.codeactions.AddFeature; import io.openliberty.tools.langserver.lemminx.codeactions.CreateFile; import io.openliberty.tools.langserver.lemminx.codeactions.EditAttribute; import io.openliberty.tools.langserver.lemminx.codeactions.ReplaceFeature; @@ -55,6 +56,7 @@ private void registerCodeActions() { codeActionParticipants.put(LibertyDiagnosticParticipant.NOT_OPTIONAL_CODE, new EditAttribute()); codeActionParticipants.put(LibertyDiagnosticParticipant.IMPLICIT_NOT_OPTIONAL_CODE, new AddAttribute()); codeActionParticipants.put(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE, new ReplaceFeature()); + codeActionParticipants.put(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_CODE, new AddFeature()); } } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java index b61941a0..aeb1b13e 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java @@ -23,6 +23,7 @@ import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.jsonrpc.CancelChecker; +import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph; import io.openliberty.tools.langserver.lemminx.data.LibertyRuntime; import io.openliberty.tools.langserver.lemminx.services.FeatureService; import io.openliberty.tools.langserver.lemminx.services.SettingsService; @@ -30,20 +31,31 @@ import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.logging.Logger; public class LibertyDiagnosticParticipant implements IDiagnosticsParticipant { + private static final Logger LOGGER = Logger.getLogger(LibertyDiagnosticParticipant.class.getName()); + + public static final String LIBERTY_LEMMINX_SOURCE = "liberty-lemminx"; + public static final String MISSING_FILE_MESSAGE = "The resource at the specified location could not be found."; public static final String MISSING_FILE_CODE = "missing_file"; + public static final String MISSING_CONFIGURED_FEATURE_MESSAGE = "This config element does not configure a feature in the featureManager. Remove this element or add a relevant feature."; + public static final String MISSING_CONFIGURED_FEATURE_CODE = "lost_config_element"; + public static final String NOT_OPTIONAL_MESSAGE = "The specified resource cannot be skipped. Check location value or set optional to true."; public static final String NOT_OPTIONAL_CODE = "not_optional"; public static final String IMPLICIT_NOT_OPTIONAL_MESSAGE = "The specified resource cannot be skipped. Check location value or add optional attribute."; public static final String IMPLICIT_NOT_OPTIONAL_CODE = "implicit_not_optional"; public static final String INCORRECT_FEATURE_CODE = "incorrect_feature"; + + private Set includedFeatures = null; @Override public void doDiagnostics(DOMDocument domDocument, List diagnostics, @@ -53,22 +65,33 @@ public void doDiagnostics(DOMDocument domDocument, List diagnostics, try { validateDom(domDocument, diagnostics); } catch (IOException e) { + // LOGGER.severe("Error validating document " + domDocument.getDocumentURI()); + // LOGGER.severe(e.getMessage()); System.err.println("Error validating document " + domDocument.getDocumentURI()); System.err.println(e.getMessage()); } } - private void validateDom(DOMDocument domDocument, List list) throws IOException { + private void validateDom(DOMDocument domDocument, List diagnosticsList) throws IOException { List nodes = domDocument.getDocumentElement().getChildren(); + List tempDiagnosticsList = new ArrayList(); + + // TODO: consider initiallizing FeatureService even when features aren't detected + FeatureListGraph featureGraph = FeatureService.getInstance().getFeatureListGraph(); for (DOMNode node : nodes) { - if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) { - validateFeature(domDocument, list, node); - } else if (LibertyConstants.INCLUDE_ELEMENT.equals(node.getNodeName())) { - validateIncludeLocation(domDocument, list, node); + String nodeName = node.getNodeName(); + if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(nodeName)) { + validateFeature(domDocument, diagnosticsList, node); + } else if (LibertyConstants.INCLUDE_ELEMENT.equals(nodeName)) { + validateIncludeLocation(domDocument, diagnosticsList, node); + } else if (featureGraph.isConfigElement(nodeName)) { + LOGGER.warning("Detected a config element!"); + holdConfigElement(domDocument, diagnosticsList, node, tempDiagnosticsList); } } - + LOGGER.warning("Entering validateConfigElements"); + validateConfigElements(diagnosticsList, tempDiagnosticsList, featureGraph); } private void validateFeature(DOMDocument domDocument, List list, DOMNode featureManager) { @@ -93,19 +116,20 @@ private void validateFeature(DOMDocument domDocument, List list, DOM Range range = XMLPositionUtility.createRange(featureTextNode.getStart(), featureTextNode.getEnd(), domDocument); String message = "ERROR: The feature \"" + featureName + "\" does not exist."; - list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, "liberty-lemminx", INCORRECT_FEATURE_CODE)); + list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, INCORRECT_FEATURE_CODE)); } else { if (includedFeatures.contains(featureName)) { Range range = XMLPositionUtility.createRange(featureTextNode.getStart(), featureTextNode.getEnd(), domDocument); String message = "ERROR: " + featureName + " is already included."; - list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, "liberty-lemminx")); + list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE)); } else { includedFeatures.add(featureName); } } } } + this.includedFeatures = includedFeatures; } /** @@ -117,7 +141,7 @@ private void validateFeature(DOMDocument domDocument, List list, DOM * 2) performed in isConfigXMLFile * 4) not yet implemented/determined */ - private void validateIncludeLocation(DOMDocument domDocument, List list, DOMNode node) { + private void validateIncludeLocation(DOMDocument domDocument, List diagnosticsList, DOMNode node) { String locAttribute = node.getAttribute("location"); if (locAttribute == null) { return; @@ -131,7 +155,7 @@ private void validateIncludeLocation(DOMDocument domDocument, List l Range range = XMLPositionUtility.createRange(locNode.getStart(), locNode.getEnd(), domDocument); if (!locAttribute.endsWith(".xml")) { String message = "The specified resource is not an XML file."; - list.add(new Diagnostic(range, message, DiagnosticSeverity.Warning, "liberty-lemminx")); + diagnosticsList.add(new Diagnostic(range, message, DiagnosticSeverity.Warning, LIBERTY_LEMMINX_SOURCE)); return; } @@ -144,15 +168,38 @@ private void validateIncludeLocation(DOMDocument domDocument, List l if (!configFile.exists()) { DOMAttr optNode = node.getAttributeNode("optional"); if (optNode == null) { - list.add(new Diagnostic(range, IMPLICIT_NOT_OPTIONAL_MESSAGE, DiagnosticSeverity.Error, "liberty-lemminx", IMPLICIT_NOT_OPTIONAL_CODE)); + diagnosticsList.add(new Diagnostic(range, IMPLICIT_NOT_OPTIONAL_MESSAGE, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, IMPLICIT_NOT_OPTIONAL_CODE)); } else if (optNode.getValue().equals("false")) { Range optRange = XMLPositionUtility.createRange(optNode.getStart(), optNode.getEnd(), domDocument); - list.add(new Diagnostic(optRange, NOT_OPTIONAL_MESSAGE, DiagnosticSeverity.Error, "liberty-lemminx", NOT_OPTIONAL_CODE)); + diagnosticsList.add(new Diagnostic(optRange, NOT_OPTIONAL_MESSAGE, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, NOT_OPTIONAL_CODE)); } - list.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, "liberty-lemminx", MISSING_FILE_CODE)); + diagnosticsList.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, LIBERTY_LEMMINX_SOURCE, MISSING_FILE_CODE)); } } catch (IllegalArgumentException e) { - list.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, "liberty-lemminx-exception", MISSING_FILE_CODE)); + diagnosticsList.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, "liberty-lemminx-exception", MISSING_FILE_CODE)); + } + } + + private void holdConfigElement(DOMDocument domDocument, List diagnosticsList, DOMNode configElementNode, + List tempDiagnosticsList) { + String configElementName = configElementNode.getNodeName(); + Range range = XMLPositionUtility.createRange(configElementNode.getStart(), configElementNode.getEnd(), domDocument); + Diagnostic tempDiagnostic = new Diagnostic(range, MISSING_CONFIGURED_FEATURE_MESSAGE, null, + LIBERTY_LEMMINX_SOURCE, MISSING_CONFIGURED_FEATURE_CODE); + tempDiagnostic.setSource(configElementName); + tempDiagnosticsList.add(tempDiagnostic); + } + + private void validateConfigElements(List diagnosticsList, List tempDiagnosticsList, + FeatureListGraph featureGraph) { + for (Diagnostic tempDiagnostic : tempDiagnosticsList) { + String configElement = tempDiagnostic.getSource(); + Set includedFeaturesCopy = new HashSet(includedFeatures); + Set compatibleFeaturesList = featureGraph.getAllEnablers(configElement); + includedFeaturesCopy.retainAll(compatibleFeaturesList); + if (includedFeaturesCopy.isEmpty()) { + diagnosticsList.add(tempDiagnostic); + } } } -} +} \ No newline at end of file diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java new file mode 100644 index 00000000..8a1650ae --- /dev/null +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -0,0 +1,79 @@ +/******************************************************************************* +* Copyright (c) 2023 IBM Corporation and others. +* +* This program and the accompanying materials are made available under the +* terms of the Eclipse Public License v. 2.0 which is available at +* http://www.eclipse.org/legal/epl-2.0. +* +* SPDX-License-Identifier: EPL-2.0 +* +* Contributors: +* IBM Corporation - initial API and implementation +*******************************************************************************/ +package io.openliberty.tools.langserver.lemminx.codeactions; + +import java.util.List; +import java.util.Set; + +import org.eclipse.lemminx.commons.CodeActionFactory; +import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lemminx.dom.DOMNode; +import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant; +import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest; +import org.eclipse.lemminx.utils.XMLPositionUtility; +import org.eclipse.lsp4j.CodeAction; +import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4j.Position; +import org.eclipse.lsp4j.jsonrpc.CancelChecker; + +import io.openliberty.tools.langserver.lemminx.services.FeatureService; +import io.openliberty.tools.langserver.lemminx.util.LibertyConstants; + +public class AddFeature implements ICodeActionParticipant { + + // This method is still a work in progress. + @Override + public void doCodeAction(ICodeActionRequest request, List codeActions, CancelChecker cancelChecker) { + Diagnostic diagnostic = request.getDiagnostic(); + DOMDocument document = request.getDocument(); + try { + List nodes = document.getDocumentElement().getChildren(); + DOMNode featureManagerNode = null; + + for (DOMNode node : nodes) { + if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) { + featureManagerNode = node; + break; + } + } + + Position insertPosition; + String insertText; + DOMNode locationNode; + if (featureManagerNode == null) { + locationNode = document.getDocumentElement().getFirstChild(); + insertPosition = XMLPositionUtility.createRange(locationNode.getStart(), locationNode.getEnd(), document).getStart(); + insertText = "\nasdf\n"; + + } else { + locationNode = featureManagerNode.getLastChild(); + insertPosition = XMLPositionUtility.createRange(locationNode.getStart(), locationNode.getEnd(), document).getEnd(); + insertText = "\n\tmicroProfile-5.0"; + } + + String configElement = diagnostic.getSource(); + if (configElement.equals("ssl")) { + FeatureService.getInstance().getFeatureListGraph().get("ssl").getEnablers(); + } + Set featureCandidates = FeatureService.getInstance().getFeatureListGraph().get(configElement).getEnablers(); + codeActions.add(CodeActionFactory.insert("This is a test: " + featureCandidates, insertPosition, insertText, document.getTextDocument(), diagnostic)); + for (String feature : featureCandidates) { + String title = "Add feature " + feature; + codeActions.add(CodeActionFactory.insert(title, insertPosition, feature, + document.getTextDocument(), diagnostic)); + } + } catch (Exception e) { + + } + } +} diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java new file mode 100644 index 00000000..a4fab68e --- /dev/null +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java @@ -0,0 +1,144 @@ +/******************************************************************************* +* Copyright (c) 2023 IBM Corporation and others. +* +* This program and the accompanying materials are made available under the +* terms of the Eclipse Public License v. 2.0 which is available at +* http://www.eclipse.org/legal/epl-2.0. +* +* SPDX-License-Identifier: EPL-2.0 +* +* Contributors: +* IBM Corporation - initial API and implementation +*******************************************************************************/ +package io.openliberty.tools.langserver.lemminx.data; + +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +public class FeatureListGraph { + private Map nodes; + // private Map> enablersCache; + // private Map> enablesCache; + + public FeatureListGraph() { + nodes = new HashMap(); + // enablersCache = new HashMap>(); + // enablesCache = new HashMap>(); + } + + public FeatureListNode addFeature(String nodeName) { + if (nodes.containsKey(nodeName)) { + return nodes.get(nodeName); + } + FeatureListNode node = new FeatureListNode(nodeName); + nodes.put(nodeName, node); + return node; + } + + public FeatureListNode addConfigElement(String nodeName) { + if (nodes.containsKey(nodeName)) { + return nodes.get(nodeName); + } + FeatureListNode node = new FeatureListNode(nodeName); + nodes.put(nodeName, node); + return node; + } + + public FeatureListNode get(String nodeName) { + return nodes.get(nodeName); + } + + public boolean isEmpty() { + return nodes.isEmpty(); + } + + public boolean isConfigElement(String featureListNode) { + if (!nodes.containsKey(featureListNode)) { + return false; + } + return nodes.get(featureListNode).isConfigElement(); + } + + /** + * Returns a superset of 'owning' features that enable a given config element or feature. + * @param elementName + * @return + */ + public Set getAllEnablers(String elementName) { + // if (enablersCache.containsKey(elementName)) { + // return enablersCache.get(elementName); + // } + if (!nodes.containsKey(elementName)) { + return null; + } + Set allEnablers = nodes.get(elementName).getEnablers(); + Deque queue = new ArrayDeque(allEnablers); + Set visited = new HashSet(); + while (!queue.isEmpty()) { + String node = queue.getFirst(); + queue.removeFirst(); + if (visited.contains(node)) { + continue; + } + Set enablers = nodes.get(node).getEnablers(); + visited.add(node); + allEnablers.addAll(enablers); + queue.addAll(enablers); + } + return allEnablers; + } + + /** + * Returns the set of supported features or config elements for a given feature. + * @param feature + * @return + */ + public Set getAllEnables(String feature) { + // if (enablesCache.containsKey(feature)) { + // return enablesCache.get(feature); + // } + if (!nodes.containsKey(feature)) { + return null; + } + Set allEnables = nodes.get(feature).getEnables(); + Deque queue = new ArrayDeque(allEnables); + Set visited = new HashSet(); + while (!queue.isEmpty()) { + String node = queue.getFirst(); + queue.removeFirst(); + if (visited.contains(node)) { + continue; + } + Set enablers = nodes.get(node).getEnables(); + visited.add(node); + allEnables.addAll(enablers); + queue.addAll(enablers); + } + // enablesCache.put(feature, allEnables); + return allEnables; + } + + // public Set getAllConfigElements(String feature) { + // Set configElements = new HashSet(); + // for (String node : getAllEnables(feature)) { + // if (isConfigElement(node)) { + // configElements.add(node); + // } + // } + // return configElements; + // } + + // public Set getAllEnabledFeatures(String feature) { + // Set enabledFeatures = new HashSet(); + // for (String node : getAllEnables(feature)) { + // if (!isConfigElement(node)) { + // enabledFeatures.add(node); + // } + // } + // return enabledFeatures; + // } +} \ No newline at end of file diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java new file mode 100644 index 00000000..56c32513 --- /dev/null +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java @@ -0,0 +1,48 @@ +/******************************************************************************* +* Copyright (c) 2023 IBM Corporation and others. +* +* This program and the accompanying materials are made available under the +* terms of the Eclipse Public License v. 2.0 which is available at +* http://www.eclipse.org/legal/epl-2.0. +* +* SPDX-License-Identifier: EPL-2.0 +* +* Contributors: +* IBM Corporation - initial API and implementation +*******************************************************************************/ +package io.openliberty.tools.langserver.lemminx.data; + +import java.util.HashSet; +import java.util.Set; + +public class FeatureListNode { + protected String nodeName; + protected Set enabledBy; + protected Set enables; + + public FeatureListNode(String nodeName) { + enabledBy = new HashSet(); + enables = new HashSet(); + this.nodeName = nodeName; + } + + public void addEnabler(String nodeName) { + enabledBy.add(nodeName); + } + + public void addEnables(String nodeName) { + enables.add(nodeName); + } + + public Set getEnablers() { + return enabledBy; + } + + public Set getEnables() { + return enables; + } + + public boolean isConfigElement() { + return this.nodeName.indexOf('.') == -1; + } +} \ No newline at end of file diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/models/feature/Feature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/models/feature/Feature.java index 1b1d798e..13c3943c 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/models/feature/Feature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/models/feature/Feature.java @@ -16,6 +16,7 @@ import jakarta.xml.bind.annotation.XmlAttribute; import jakarta.xml.bind.annotation.XmlAccessType; import jakarta.xml.bind.annotation.XmlRootElement; +import java.util.List; @XmlRootElement(name = "feature") @XmlAccessorType(XmlAccessType.FIELD) @@ -33,6 +34,9 @@ public class Feature { private String version; WlpInformation wlpInformation; + private List configElement; + private List enables; + // Getter Methods public String getDescription() { @@ -67,6 +71,14 @@ public WlpInformation getWlpInformation() { return wlpInformation; } + public List getConfigElements() { + return configElement; + } + + public List getEnables() { + return enables; + } + // Setter Methods public void setDescription(String description) { @@ -100,4 +112,12 @@ public void setVersion(String version) { public void setWlpInformation(WlpInformation wlpInformation) { this.wlpInformation = wlpInformation; } + + public void setConfigElements(List configElement) { + this.configElement = configElement; + } + + public void setEnables(List enables) { + this.enables = enables; + } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java index afe29631..548562ef 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java @@ -42,7 +42,8 @@ import com.google.gson.Gson; import com.google.gson.JsonParseException; - +import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph; +import io.openliberty.tools.langserver.lemminx.data.FeatureListNode; import io.openliberty.tools.langserver.lemminx.models.feature.Feature; import io.openliberty.tools.langserver.lemminx.models.feature.FeatureInfo; import io.openliberty.tools.langserver.lemminx.models.feature.WlpInformation; @@ -72,8 +73,12 @@ public static FeatureService getInstance() { private List defaultFeatureList; private long featureUpdateTime; + // Contains the mapping a configElement to a set of features that use said configElement + private FeatureListGraph featureListGraph; + private FeatureService() { featureCache = new HashMap<>(); + featureListGraph = new FeatureListGraph(); featureUpdateTime = -1; } @@ -167,13 +172,18 @@ public List getFeatures(String libertyVersion, String libertyRuntime, i String featureCacheKey = libertyRuntime + "-" + libertyVersion; // if the features are already cached in the feature cache - if (featureCache.containsKey(featureCacheKey)) { + if (featureCache.containsKey(featureCacheKey) || !featureListGraph.isEmpty()) { LOGGER.info("Getting cached features for: " + featureCacheKey); return featureCache.get(featureCacheKey); } LOGGER.info("Getting features for: " + featureCacheKey); + List installedFeatures = getInstalledFeaturesList(documentURI, libertyRuntime, libertyVersion); + if (installedFeatures.size() != 0) { + return installedFeatures; + } + // if not a beta runtime, fetch features from maven central // - beta runtimes do not have a published features.json in mc if (!libertyVersion.endsWith("-beta")) { @@ -194,13 +204,6 @@ public List getFeatures(String libertyVersion, String libertyRuntime, i } } - // fetch installed features list - this would only happen if a features.json was not able to be downloaded from Maven Central - // This is the case for beta runtimes and for very old runtimes pre 18.0.0.2 (or if within the requestDelay window of 120 seconds). - List installedFeatures = getInstalledFeaturesList(documentURI, libertyRuntime, libertyVersion); - if (installedFeatures.size() != 0) { - return installedFeatures; - } - // return default feature list List defaultFeatures = getDefaultFeatureList(); return defaultFeatures; @@ -271,7 +274,6 @@ public List collectExistingFeatures(DOMNode featureManager, String curre */ private List getInstalledFeaturesList(String documentURI, String libertyRuntime, String libertyVersion) { List installedFeatures = new ArrayList(); - LibertyWorkspace libertyWorkspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(documentURI); if (libertyWorkspace == null || libertyWorkspace.getWorkspaceString() == null) { return installedFeatures; @@ -287,7 +289,6 @@ private List getInstalledFeaturesList(String documentURI, String libert try { // Need to handle both local installation and container File featureListFile = null; - if (libertyWorkspace.isLibertyInstalled()) { Path featureListJAR = LibertyUtils.findLibertyFileForWorkspace(libertyWorkspace, Paths.get("bin", "tools", "ws-featurelist.jar")); if (featureListJAR != null && featureListJAR.toFile().exists()) { @@ -374,14 +375,32 @@ public List readFeaturesFromFeatureListFile(List installedFeat // Note: Only the public features are loaded when unmarshalling the passed featureListFile. if ((featureInfo.getFeatures() != null) && (featureInfo.getFeatures().size() > 0)) { - for (int i = 0; i < featureInfo.getFeatures().size(); i++) { - Feature f = featureInfo.getFeatures().get(i); + for (Feature f : featureInfo.getFeatures()) { f.setShortDescription(f.getDescription()); // The xml featureListFile does not have a wlpInformation element like the json does, but our code depends on looking up // features by the shortName found in wlpInformation. So create a WlpInformation object and initialize the shortName to // the feature name. WlpInformation wlpInfo = new WlpInformation(f.getName()); f.setWlpInformation(wlpInfo); + + String currentFeature = f.getName(); + List enables = f.getEnables(); + List configElements = f.getConfigElements(); + FeatureListNode currentFeatureNode = featureListGraph.addFeature(currentFeature); + if (enables != null) { + for (String enabledFeature : enables) { + FeatureListNode feature = featureListGraph.addFeature(enabledFeature); + feature.addEnabler(currentFeature); + currentFeatureNode.addEnables(enabledFeature); + } + } + if (configElements != null) { + for (String configElement : configElements) { + FeatureListNode configNode = featureListGraph.addConfigElement(configElement); + configNode.addEnabler(currentFeature); + currentFeatureNode.addEnables(configElement); + } + } } installedFeatures = featureInfo.getFeatures(); libertyWorkspace.setInstalledFeatureList(installedFeatures); @@ -391,4 +410,7 @@ public List readFeaturesFromFeatureListFile(List installedFeat return installedFeatures; } + public FeatureListGraph getFeatureListGraph() { + return this.featureListGraph; + } } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index 2ff59d45..c6601d09 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -9,7 +9,11 @@ import org.junit.jupiter.api.Test; import io.openliberty.tools.langserver.lemminx.LibertyDiagnosticParticipant; +import io.openliberty.tools.langserver.lemminx.models.feature.Feature; +import io.openliberty.tools.langserver.lemminx.services.FeatureService; import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager; +import io.openliberty.tools.langserver.lemminx.services.LibertyWorkspace; +import jakarta.xml.bind.JAXBException; import static org.eclipse.lemminx.XMLAssert.r; import static org.eclipse.lemminx.XMLAssert.ca; @@ -21,7 +25,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; -import java.util.Collection; import java.util.Collections; public class LibertyDiagnosticTest { @@ -175,4 +178,73 @@ public void testDiagnosticsForInclude() throws IOException { XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLFile.toURI().toString(), not_xml, multi_liner, not_optional, missing_xml, optional_not_defined, missing_xml2); } + + @Test + public void testConfigElementMissingFeatureManager() throws JAXBException { + File featureList = new File("src/test/resources/featurelist-ol-23.0.0.1-beta.xml"); + assertTrue(featureList.exists()); + FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), + new LibertyWorkspace(""), featureList); + + String serverXml = "ssl id=\"\"/>"; + Diagnostic config_for_missing_feature = new Diagnostic(); + config_for_missing_feature.setRange(r(0, 0, 0, serverXml.length())); + config_for_missing_feature.setCode(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_CODE); + config_for_missing_feature.setMessage(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_MESSAGE); + } + + @Test + public void testConfigElementDirect() throws JAXBException { + File featureList = new File("src/test/resources/featurelist-ol-23.0.0.1-beta.xml"); + assertTrue(featureList.exists()); + FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), + new LibertyWorkspace(""), featureList); + + String correctFeature = " ssl-1.0"; + String incorrectFeature = " jaxrs-2.0"; + String configElement = " "; + int diagnosticStart = configElement.indexOf("<"); + int diagnosticLength = configElement.trim().length(); + + String serverXML1 = String.join(newLine, + "", + " ", + correctFeature, + " ", + configElement, + "" + ); + XMLAssert.testDiagnosticsFor(serverXML1, null, null, serverXMLURI); + + String serverXML2 = String.join(newLine, + "", + " ", + incorrectFeature, + " ", + configElement, + "" + ); + + Diagnostic config_for_missing_feature = new Diagnostic(); + config_for_missing_feature.setRange(r(4, diagnosticStart, 4, diagnosticStart + diagnosticLength)); + config_for_missing_feature.setCode(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_CODE); + config_for_missing_feature.setMessage(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_MESSAGE); + + XMLAssert.testDiagnosticsFor(serverXML2, null, null, serverXMLURI, config_for_missing_feature); + } + + @Test + public void testConfigElementTransitive() { + File featureList = new File("src/test/resources/featurelist-ol-23.0.0.1-beta.xml"); + assertTrue(featureList.exists()); + String serverXML1 = String.join(newLine, + "", + " ", + " microProfile-5.0", + " ", + " ", + "" + ); + XMLAssert.testDiagnosticsFor(serverXML1, null, null, serverXMLURI); + } } \ No newline at end of file diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java index 74264808..76603973 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java @@ -1,5 +1,6 @@ package io.openliberty; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -11,6 +12,7 @@ import org.eclipse.lsp4j.WorkspaceFolder; import org.junit.jupiter.api.Test; +import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph; import io.openliberty.tools.langserver.lemminx.models.feature.Feature; import io.openliberty.tools.langserver.lemminx.services.FeatureService; import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager; @@ -41,5 +43,16 @@ public void getInstalledFeaturesListTest() throws JAXBException { assertTrue(installedFeatures.equals(libWorkspace.getInstalledFeatureList())); // Check that list contains a beta feature assertTrue(installedFeatures.removeIf(f -> (f.getName().equals("cdi-4.0")))); + + // Check if config map gets built + FeatureListGraph fg = fs.getFeatureListGraph(); + assertEquals(1, fg.get("ssl").getEnablers().size()); + assertTrue(fg.get("ssl").getEnablers().contains("ssl-1.0")); + assertEquals(76, fg.getAllEnablers("ssl-1.0").size()); + assertEquals(235, fg.getAllEnablers("library").size()); + assertTrue(fg.getAllEnablers("ltpa").contains("adminCenter-1.0")); // direct enabler + assertTrue(fg.getAllEnablers("ssl").contains("microProfile-5.0")); // transitive enabler + assertTrue(fg.getAllEnables("microProfile-5.0").contains("ssl")); + assertTrue(fg.getAllEnables("jakartaee-8.0").contains("classloading")); } } diff --git a/liberty-ls/pom.xml b/liberty-ls/pom.xml index 4ab1bf44..31e31b44 100644 --- a/liberty-ls/pom.xml +++ b/liberty-ls/pom.xml @@ -5,7 +5,7 @@ io.openliberty.tools liberty-langserver - 2.0.2-SNAPSHOT + 2.1-SNAPSHOT liberty.langserver https://openliberty.io/ From 2fc562884c7aea3d2dda71901daee4913e8c4d2d Mon Sep 17 00:00:00 2001 From: dshimo Date: Fri, 6 Oct 2023 09:09:52 -0500 Subject: [PATCH 02/14] revert featureservice, refactor, addfeature wip --- .../lemminx/LibertyDiagnosticParticipant.java | 20 +++--- .../lemminx/codeactions/AddFeature.java | 72 ++++++++++++------- .../lemminx/data/FeatureListGraph.java | 37 +++++----- .../lemminx/data/FeatureListNode.java | 4 +- .../lemminx/services/FeatureService.java | 21 +++--- .../io/openliberty/LibertyDiagnosticTest.java | 4 +- .../io/openliberty/LibertyFeatureTest.java | 13 ++-- 7 files changed, 97 insertions(+), 74 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java index aeb1b13e..5d5e590f 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java @@ -76,8 +76,13 @@ private void validateDom(DOMDocument domDocument, List diagnosticsLi List nodes = domDocument.getDocumentElement().getChildren(); List tempDiagnosticsList = new ArrayList(); - // TODO: consider initiallizing FeatureService even when features aren't detected FeatureListGraph featureGraph = FeatureService.getInstance().getFeatureListGraph(); + if (featureGraph.isEmpty()) { + LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(domDocument); + String libertyVersion = runtimeInfo == null ? null : runtimeInfo.getRuntimeVersion(); + String libertyRuntime = runtimeInfo == null ? null : runtimeInfo.getRuntimeType(); + FeatureService.getInstance().getInstalledFeaturesList(domDocument.getDocumentURI(), libertyRuntime, libertyVersion); + } for (DOMNode node : nodes) { String nodeName = node.getNodeName(); @@ -86,11 +91,9 @@ private void validateDom(DOMDocument domDocument, List diagnosticsLi } else if (LibertyConstants.INCLUDE_ELEMENT.equals(nodeName)) { validateIncludeLocation(domDocument, diagnosticsList, node); } else if (featureGraph.isConfigElement(nodeName)) { - LOGGER.warning("Detected a config element!"); holdConfigElement(domDocument, diagnosticsList, node, tempDiagnosticsList); } } - LOGGER.warning("Entering validateConfigElements"); validateConfigElements(diagnosticsList, tempDiagnosticsList, featureGraph); } @@ -180,22 +183,19 @@ private void validateIncludeLocation(DOMDocument domDocument, List d } } - private void holdConfigElement(DOMDocument domDocument, List diagnosticsList, DOMNode configElementNode, - List tempDiagnosticsList) { + private void holdConfigElement(DOMDocument domDocument, List diagnosticsList, DOMNode configElementNode, List tempDiagnosticsList) { String configElementName = configElementNode.getNodeName(); Range range = XMLPositionUtility.createRange(configElementNode.getStart(), configElementNode.getEnd(), domDocument); - Diagnostic tempDiagnostic = new Diagnostic(range, MISSING_CONFIGURED_FEATURE_MESSAGE, null, - LIBERTY_LEMMINX_SOURCE, MISSING_CONFIGURED_FEATURE_CODE); + Diagnostic tempDiagnostic = new Diagnostic(range, MISSING_CONFIGURED_FEATURE_MESSAGE, null, LIBERTY_LEMMINX_SOURCE, MISSING_CONFIGURED_FEATURE_CODE); tempDiagnostic.setSource(configElementName); tempDiagnosticsList.add(tempDiagnostic); } - private void validateConfigElements(List diagnosticsList, List tempDiagnosticsList, - FeatureListGraph featureGraph) { + private void validateConfigElements(List diagnosticsList, List tempDiagnosticsList, FeatureListGraph featureGraph) { for (Diagnostic tempDiagnostic : tempDiagnosticsList) { String configElement = tempDiagnostic.getSource(); Set includedFeaturesCopy = new HashSet(includedFeatures); - Set compatibleFeaturesList = featureGraph.getAllEnablers(configElement); + Set compatibleFeaturesList = featureGraph.getAllEnabledBy(configElement); includedFeaturesCopy.retainAll(compatibleFeaturesList); if (includedFeaturesCopy.isEmpty()) { diagnosticsList.add(tempDiagnostic); diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java index 8a1650ae..fcc8f928 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -14,63 +14,81 @@ import java.util.List; import java.util.Set; +import java.util.logging.Logger; import org.eclipse.lemminx.commons.CodeActionFactory; import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lemminx.dom.DOMElement; import org.eclipse.lemminx.dom.DOMNode; +import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager; +import org.eclipse.lemminx.extensions.contentmodel.utils.XMLGenerator; import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant; import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest; +import org.eclipse.lemminx.utils.XMLBuilder; import org.eclipse.lemminx.utils.XMLPositionUtility; import org.eclipse.lsp4j.CodeAction; import org.eclipse.lsp4j.Diagnostic; import org.eclipse.lsp4j.Position; +import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.jsonrpc.CancelChecker; +import io.openliberty.tools.langserver.lemminx.LibertyDiagnosticParticipant; import io.openliberty.tools.langserver.lemminx.services.FeatureService; import io.openliberty.tools.langserver.lemminx.util.LibertyConstants; public class AddFeature implements ICodeActionParticipant { - // This method is still a work in progress. + // TODO: Support auto indentation + private static final String NEW_LINE = System.lineSeparator(); + public static final String FEATURE_FORMAT = "%s"; + public static final String FEATUREMANAGER_FORMAT = + "" + NEW_LINE + FEATURE_FORMAT + NEW_LINE + ""; + @Override public void doCodeAction(ICodeActionRequest request, List codeActions, CancelChecker cancelChecker) { Diagnostic diagnostic = request.getDiagnostic(); DOMDocument document = request.getDocument(); try { - List nodes = document.getDocumentElement().getChildren(); - DOMNode featureManagerNode = null; - - for (DOMNode node : nodes) { + String insertText = NEW_LINE + FEATUREMANAGER_FORMAT; + DOMNode insertNode = document.getDocumentElement().getFirstChild(); + Position insertPosition = XMLPositionUtility.createRange(insertNode.getStart(), insertNode.getEnd(), document).getStart(); + for (DOMNode node : document.getDocumentElement().getChildren()) { if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) { - featureManagerNode = node; + insertText = NEW_LINE; + insertNode = node.getLastChild(); + insertPosition = XMLPositionUtility.createRange(insertNode.getStart(), insertNode.getEnd(), document).getEnd(); + + // document.getDocumentElement().getEndTagCloseOffset(); + // DOMElement de = (DOMElement) node; + // Position start = document.positionAt(de.getStartTagCloseOffset()+1); + // Position stop = document.positionAt(de.getEndTagCloseOffset()); + // Range targetRange = new Range(start, stop); + // XMLGenerator generator = request.getXMLGenerator(); + String indent = request.getXMLGenerator().getWhitespacesIndent(); + int level = document.positionAt(((DOMElement)node).getStartTagOpenOffset()).getCharacter() / indent.length(); + for (int i = 0; i < level+1; i++) { + insertText += indent; + } + insertText += FEATURE_FORMAT; break; } } - - Position insertPosition; - String insertText; - DOMNode locationNode; - if (featureManagerNode == null) { - locationNode = document.getDocumentElement().getFirstChild(); - insertPosition = XMLPositionUtility.createRange(locationNode.getStart(), locationNode.getEnd(), document).getStart(); - insertText = "\nasdf\n"; - } else { - locationNode = featureManagerNode.getLastChild(); - insertPosition = XMLPositionUtility.createRange(locationNode.getStart(), locationNode.getEnd(), document).getEnd(); - insertText = "\n\tmicroProfile-5.0"; - } + // String insertStrRequired = null; + // String insertStrAll = null; - String configElement = diagnostic.getSource(); - if (configElement.equals("ssl")) { - FeatureService.getInstance().getFeatureListGraph().get("ssl").getEnablers(); - } - Set featureCandidates = FeatureService.getInstance().getFeatureListGraph().get(configElement).getEnablers(); - codeActions.add(CodeActionFactory.insert("This is a test: " + featureCandidates, insertPosition, insertText, document.getTextDocument(), diagnostic)); + // XMLGenerator generator = request.getXMLGenerator(); + // if (!request.canSupportResolve()) { + // insertStrRequired = generator.generateMissingElements(request.getComponent(ContentModelManager.class), document.getDocumentElement(), true); + // insertStrAll = generator.generateMissingElements(request.getComponent(ContentModelManager.class), document.getDocumentElement(), false); + // } + + // getAllEnabledBy would return all transitive features, but is too many to offer without a filter + Set featureCandidates = FeatureService.getInstance().getFeatureListGraph().get(diagnostic.getSource()).getEnabledBy(); for (String feature : featureCandidates) { String title = "Add feature " + feature; - codeActions.add(CodeActionFactory.insert(title, insertPosition, feature, - document.getTextDocument(), diagnostic)); + codeActions.add(CodeActionFactory.insert( + title, insertPosition, String.format(insertText, feature), document.getTextDocument(), diagnostic)); } } catch (Exception e) { diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java index a4fab68e..ba13596a 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java @@ -21,13 +21,13 @@ public class FeatureListGraph { private Map nodes; - // private Map> enablersCache; - // private Map> enablesCache; + private Map> enabledByCache; + private Map> enablesCache; public FeatureListGraph() { nodes = new HashMap(); - // enablersCache = new HashMap>(); - // enablesCache = new HashMap>(); + enabledByCache = new HashMap>(); + enablesCache = new HashMap>(); } public FeatureListNode addFeature(String nodeName) { @@ -68,15 +68,15 @@ public boolean isConfigElement(String featureListNode) { * @param elementName * @return */ - public Set getAllEnablers(String elementName) { - // if (enablersCache.containsKey(elementName)) { - // return enablersCache.get(elementName); - // } + public Set getAllEnabledBy(String elementName) { + if (enabledByCache.containsKey(elementName)) { + return enabledByCache.get(elementName); + } if (!nodes.containsKey(elementName)) { return null; } - Set allEnablers = nodes.get(elementName).getEnablers(); - Deque queue = new ArrayDeque(allEnablers); + Set allEnabledBy = new HashSet(nodes.get(elementName).getEnabledBy()); + Deque queue = new ArrayDeque(allEnabledBy); Set visited = new HashSet(); while (!queue.isEmpty()) { String node = queue.getFirst(); @@ -84,12 +84,13 @@ public Set getAllEnablers(String elementName) { if (visited.contains(node)) { continue; } - Set enablers = nodes.get(node).getEnablers(); + Set enablers = nodes.get(node).getEnabledBy(); visited.add(node); - allEnablers.addAll(enablers); + allEnabledBy.addAll(enablers); queue.addAll(enablers); } - return allEnablers; + enabledByCache.put(elementName, allEnabledBy); + return allEnabledBy; } /** @@ -98,13 +99,13 @@ public Set getAllEnablers(String elementName) { * @return */ public Set getAllEnables(String feature) { - // if (enablesCache.containsKey(feature)) { - // return enablesCache.get(feature); - // } + if (enablesCache.containsKey(feature)) { + return enablesCache.get(feature); + } if (!nodes.containsKey(feature)) { return null; } - Set allEnables = nodes.get(feature).getEnables(); + Set allEnables = new HashSet(nodes.get(feature).getEnables()); Deque queue = new ArrayDeque(allEnables); Set visited = new HashSet(); while (!queue.isEmpty()) { @@ -118,7 +119,7 @@ public Set getAllEnables(String feature) { allEnables.addAll(enablers); queue.addAll(enablers); } - // enablesCache.put(feature, allEnables); + enablesCache.put(feature, allEnables); return allEnables; } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java index 56c32513..639c2d11 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java @@ -26,7 +26,7 @@ public FeatureListNode(String nodeName) { this.nodeName = nodeName; } - public void addEnabler(String nodeName) { + public void addEnabledBy(String nodeName) { enabledBy.add(nodeName); } @@ -34,7 +34,7 @@ public void addEnables(String nodeName) { enables.add(nodeName); } - public Set getEnablers() { + public Set getEnabledBy() { return enabledBy; } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java index 548562ef..e3cda443 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java @@ -73,7 +73,6 @@ public static FeatureService getInstance() { private List defaultFeatureList; private long featureUpdateTime; - // Contains the mapping a configElement to a set of features that use said configElement private FeatureListGraph featureListGraph; private FeatureService() { @@ -172,18 +171,13 @@ public List getFeatures(String libertyVersion, String libertyRuntime, i String featureCacheKey = libertyRuntime + "-" + libertyVersion; // if the features are already cached in the feature cache - if (featureCache.containsKey(featureCacheKey) || !featureListGraph.isEmpty()) { + if (featureCache.containsKey(featureCacheKey)) { LOGGER.info("Getting cached features for: " + featureCacheKey); return featureCache.get(featureCacheKey); } LOGGER.info("Getting features for: " + featureCacheKey); - List installedFeatures = getInstalledFeaturesList(documentURI, libertyRuntime, libertyVersion); - if (installedFeatures.size() != 0) { - return installedFeatures; - } - // if not a beta runtime, fetch features from maven central // - beta runtimes do not have a published features.json in mc if (!libertyVersion.endsWith("-beta")) { @@ -204,6 +198,13 @@ public List getFeatures(String libertyVersion, String libertyRuntime, i } } + // fetch installed features list - this would only happen if a features.json was not able to be downloaded from Maven Central + // This is the case for beta runtimes and for very old runtimes pre 18.0.0.2 (or if within the requestDelay window of 120 seconds). + List installedFeatures = getInstalledFeaturesList(documentURI, libertyRuntime, libertyVersion); + if (installedFeatures.size() != 0) { + return installedFeatures; + } + // return default feature list List defaultFeatures = getDefaultFeatureList(); return defaultFeatures; @@ -272,7 +273,7 @@ public List collectExistingFeatures(DOMNode featureManager, String curre * @param libertyVersion must not be null and should be a valid Liberty version (e.g. 23.0.0.6) * @return list of installed features, or empty list */ - private List getInstalledFeaturesList(String documentURI, String libertyRuntime, String libertyVersion) { + public List getInstalledFeaturesList(String documentURI, String libertyRuntime, String libertyVersion) { List installedFeatures = new ArrayList(); LibertyWorkspace libertyWorkspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(documentURI); if (libertyWorkspace == null || libertyWorkspace.getWorkspaceString() == null) { @@ -390,14 +391,14 @@ public List readFeaturesFromFeatureListFile(List installedFeat if (enables != null) { for (String enabledFeature : enables) { FeatureListNode feature = featureListGraph.addFeature(enabledFeature); - feature.addEnabler(currentFeature); + feature.addEnabledBy(currentFeature); currentFeatureNode.addEnables(enabledFeature); } } if (configElements != null) { for (String configElement : configElements) { FeatureListNode configNode = featureListGraph.addConfigElement(configElement); - configNode.addEnabler(currentFeature); + configNode.addEnabledBy(currentFeature); currentFeatureNode.addEnables(configElement); } } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index c6601d09..1dc26239 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -234,9 +234,11 @@ public void testConfigElementDirect() throws JAXBException { } @Test - public void testConfigElementTransitive() { + public void testConfigElementTransitive() throws JAXBException { File featureList = new File("src/test/resources/featurelist-ol-23.0.0.1-beta.xml"); assertTrue(featureList.exists()); + FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), + new LibertyWorkspace(""), featureList); String serverXML1 = String.join(newLine, "", " ", diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java index 76603973..bc1145cb 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java @@ -46,12 +46,13 @@ public void getInstalledFeaturesListTest() throws JAXBException { // Check if config map gets built FeatureListGraph fg = fs.getFeatureListGraph(); - assertEquals(1, fg.get("ssl").getEnablers().size()); - assertTrue(fg.get("ssl").getEnablers().contains("ssl-1.0")); - assertEquals(76, fg.getAllEnablers("ssl-1.0").size()); - assertEquals(235, fg.getAllEnablers("library").size()); - assertTrue(fg.getAllEnablers("ltpa").contains("adminCenter-1.0")); // direct enabler - assertTrue(fg.getAllEnablers("ssl").contains("microProfile-5.0")); // transitive enabler + assertEquals(76, fg.getAllEnabledBy("ssl-1.0").size()); + assertEquals(1, fg.get("ssl").getEnabledBy().size()); + assertTrue(fg.get("ssl").getEnabledBy().contains("ssl-1.0")); + assertEquals(77, fg.getAllEnabledBy("ssl").size()); + assertEquals(235, fg.getAllEnabledBy("library").size()); + assertTrue(fg.getAllEnabledBy("ltpa").contains("adminCenter-1.0")); // direct enabler + assertTrue(fg.getAllEnabledBy("ssl").contains("microProfile-5.0")); // transitive enabler assertTrue(fg.getAllEnables("microProfile-5.0").contains("ssl")); assertTrue(fg.getAllEnables("jakartaee-8.0").contains("classloading")); } From 588e7a0ed6b67ef751b3f08491c26b6b1823c79a Mon Sep 17 00:00:00 2001 From: dshimo Date: Fri, 6 Oct 2023 13:13:39 -0500 Subject: [PATCH 03/14] AddFeature indentations, clarification --- .../lemminx/LibertyDiagnosticParticipant.java | 24 ++++++-- .../lemminx/codeactions/AddFeature.java | 59 ++++++++----------- .../lemminx/codeactions/IndentUtil.java | 30 ++++++++++ .../lemminx/data/FeatureListGraph.java | 4 ++ .../lemminx/data/FeatureListNode.java | 3 + .../openliberty/CodeActionUtilitiesTest.java | 33 +++++++++++ 6 files changed, 111 insertions(+), 42 deletions(-) create mode 100644 lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/IndentUtil.java create mode 100644 lemminx-liberty/src/test/java/io/openliberty/CodeActionUtilitiesTest.java diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java index 5d5e590f..ceb518fd 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java @@ -65,10 +65,8 @@ public void doDiagnostics(DOMDocument domDocument, List diagnostics, try { validateDom(domDocument, diagnostics); } catch (IOException e) { - // LOGGER.severe("Error validating document " + domDocument.getDocumentURI()); - // LOGGER.severe(e.getMessage()); - System.err.println("Error validating document " + domDocument.getDocumentURI()); - System.err.println(e.getMessage()); + LOGGER.severe("Error validating document " + domDocument.getDocumentURI()); + LOGGER.severe(e.getMessage()); } } @@ -77,6 +75,7 @@ private void validateDom(DOMDocument domDocument, List diagnosticsLi List tempDiagnosticsList = new ArrayList(); FeatureListGraph featureGraph = FeatureService.getInstance().getFeatureListGraph(); + // TODO: Consider adding a cached feature list onto repo to optimize if (featureGraph.isEmpty()) { LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(domDocument); String libertyVersion = runtimeInfo == null ? null : runtimeInfo.getRuntimeVersion(); @@ -90,7 +89,7 @@ private void validateDom(DOMDocument domDocument, List diagnosticsLi validateFeature(domDocument, diagnosticsList, node); } else if (LibertyConstants.INCLUDE_ELEMENT.equals(nodeName)) { validateIncludeLocation(domDocument, diagnosticsList, node); - } else if (featureGraph.isConfigElement(nodeName)) { + } else if (featureGraph.isConfigElement(nodeName)) { // defaults to false holdConfigElement(domDocument, diagnosticsList, node, tempDiagnosticsList); } } @@ -183,6 +182,13 @@ private void validateIncludeLocation(DOMDocument domDocument, List d } } + /** + * Create temporary diagnostics for validation for single pass-through. + * @param domDocument + * @param diagnosticsList + * @param configElementNode + * @param tempDiagnosticsList + */ private void holdConfigElement(DOMDocument domDocument, List diagnosticsList, DOMNode configElementNode, List tempDiagnosticsList) { String configElementName = configElementNode.getNodeName(); Range range = XMLPositionUtility.createRange(configElementNode.getStart(), configElementNode.getEnd(), domDocument); @@ -191,10 +197,16 @@ private void holdConfigElement(DOMDocument domDocument, List diagnos tempDiagnosticsList.add(tempDiagnostic); } + /** + * Compare the required feature set with included feature set for each config element. + * @param diagnosticsList + * @param tempDiagnosticsList + * @param featureGraph + */ private void validateConfigElements(List diagnosticsList, List tempDiagnosticsList, FeatureListGraph featureGraph) { for (Diagnostic tempDiagnostic : tempDiagnosticsList) { String configElement = tempDiagnostic.getSource(); - Set includedFeaturesCopy = new HashSet(includedFeatures); + Set includedFeaturesCopy = (includedFeatures == null) ? new HashSet() : new HashSet(includedFeatures); Set compatibleFeaturesList = featureGraph.getAllEnabledBy(configElement); includedFeaturesCopy.retainAll(compatibleFeaturesList); if (includedFeaturesCopy.isEmpty()) { diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java index fcc8f928..2577965c 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -20,75 +20,62 @@ import org.eclipse.lemminx.dom.DOMDocument; import org.eclipse.lemminx.dom.DOMElement; import org.eclipse.lemminx.dom.DOMNode; -import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager; -import org.eclipse.lemminx.extensions.contentmodel.utils.XMLGenerator; import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant; import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest; -import org.eclipse.lemminx.utils.XMLBuilder; import org.eclipse.lemminx.utils.XMLPositionUtility; import org.eclipse.lsp4j.CodeAction; import org.eclipse.lsp4j.Diagnostic; -import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.jsonrpc.CancelChecker; -import io.openliberty.tools.langserver.lemminx.LibertyDiagnosticParticipant; import io.openliberty.tools.langserver.lemminx.services.FeatureService; import io.openliberty.tools.langserver.lemminx.util.LibertyConstants; public class AddFeature implements ICodeActionParticipant { - - // TODO: Support auto indentation - private static final String NEW_LINE = System.lineSeparator(); - public static final String FEATURE_FORMAT = "%s"; + public static final String FEATURE_FORMAT = "\n%s"; public static final String FEATUREMANAGER_FORMAT = - "" + NEW_LINE + FEATURE_FORMAT + NEW_LINE + ""; + "\n\t" + + "\n\t\t%s" + + "\n\t"; @Override public void doCodeAction(ICodeActionRequest request, List codeActions, CancelChecker cancelChecker) { Diagnostic diagnostic = request.getDiagnostic(); DOMDocument document = request.getDocument(); try { - String insertText = NEW_LINE + FEATUREMANAGER_FORMAT; - DOMNode insertNode = document.getDocumentElement().getFirstChild(); - Position insertPosition = XMLPositionUtility.createRange(insertNode.getStart(), insertNode.getEnd(), document).getStart(); + String insertText = ""; + DOMNode insertNode = null; + String indent = request.getXMLGenerator().getWhitespacesIndent(); + for (DOMNode node : document.getDocumentElement().getChildren()) { if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) { - insertText = NEW_LINE; + // If featureManager is empty, add new feature after featureManager start tag. + // Otherwise, add feature after last feature child. insertNode = node.getLastChild(); - insertPosition = XMLPositionUtility.createRange(insertNode.getStart(), insertNode.getEnd(), document).getEnd(); - - // document.getDocumentElement().getEndTagCloseOffset(); - // DOMElement de = (DOMElement) node; - // Position start = document.positionAt(de.getStartTagCloseOffset()+1); - // Position stop = document.positionAt(de.getEndTagCloseOffset()); - // Range targetRange = new Range(start, stop); - // XMLGenerator generator = request.getXMLGenerator(); - String indent = request.getXMLGenerator().getWhitespacesIndent(); - int level = document.positionAt(((DOMElement)node).getStartTagOpenOffset()).getCharacter() / indent.length(); - for (int i = 0; i < level+1; i++) { - insertText += indent; - } - insertText += FEATURE_FORMAT; + insertText = (insertNode == null) ? FEATURE_FORMAT.replace("\n", "\n\t") : FEATURE_FORMAT; + insertNode = (insertNode == null) ? node : insertNode; break; } } - // String insertStrRequired = null; - // String insertStrAll = null; + int insertEndOffset = (insertNode == null) ? ((DOMElement)document.getDocumentElement()).getStartTagCloseOffset()+1 : insertNode.getEnd(); + // featureManager not found + if (insertNode == null) { + insertNode = document.getDocumentElement(); + insertText = FEATUREMANAGER_FORMAT; + } - // XMLGenerator generator = request.getXMLGenerator(); - // if (!request.canSupportResolve()) { - // insertStrRequired = generator.generateMissingElements(request.getComponent(ContentModelManager.class), document.getDocumentElement(), true); - // insertStrAll = generator.generateMissingElements(request.getComponent(ContentModelManager.class), document.getDocumentElement(), false); - // } + Range nodeRange = XMLPositionUtility.createRange(insertNode.getStart(), insertEndOffset, document); + Logger.getLogger("dshi").warning("startCharacter" + nodeRange.getStart().getCharacter()); + Logger.getLogger("dshi").warning("endCharacter" + nodeRange.getEnd().getCharacter()); + insertText = IndentUtil.formatText(insertText, indent, nodeRange.getStart().getCharacter()); // getAllEnabledBy would return all transitive features, but is too many to offer without a filter Set featureCandidates = FeatureService.getInstance().getFeatureListGraph().get(diagnostic.getSource()).getEnabledBy(); for (String feature : featureCandidates) { String title = "Add feature " + feature; codeActions.add(CodeActionFactory.insert( - title, insertPosition, String.format(insertText, feature), document.getTextDocument(), diagnostic)); + title, nodeRange.getEnd(), String.format(insertText, feature), document.getTextDocument(), diagnostic)); } } catch (Exception e) { diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/IndentUtil.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/IndentUtil.java new file mode 100644 index 00000000..d0a06f39 --- /dev/null +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/IndentUtil.java @@ -0,0 +1,30 @@ +/******************************************************************************* +* Copyright (c) 2023 IBM Corporation and others. +* +* This program and the accompanying materials are made available under the +* terms of the Eclipse Public License v. 2.0 which is available at +* http://www.eclipse.org/legal/epl-2.0. +* +* SPDX-License-Identifier: EPL-2.0 +* +* Contributors: +* IBM Corporation - initial API and implementation +*******************************************************************************/ +package io.openliberty.tools.langserver.lemminx.codeactions; + +public class IndentUtil { + public static final String NEW_LINE = System.lineSeparator(); + + public static String whitespaceBuffer(String indent, int column) { + String whitespaceBuffer = ""; + for (int i = 0; i < column / indent.length(); i++) { + whitespaceBuffer += indent; + } + return whitespaceBuffer; + } + + public static String formatText(String text, String indent, int column) { + return text.replace("\n", System.lineSeparator() + whitespaceBuffer(indent, column)) + .replace("\t", indent); + } +} diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java index ba13596a..0cfce221 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java @@ -75,6 +75,7 @@ public Set getAllEnabledBy(String elementName) { if (!nodes.containsKey(elementName)) { return null; } + // Implements a breadth-first-search on parent nodes Set allEnabledBy = new HashSet(nodes.get(elementName).getEnabledBy()); Deque queue = new ArrayDeque(allEnabledBy); Set visited = new HashSet(); @@ -105,6 +106,7 @@ public Set getAllEnables(String feature) { if (!nodes.containsKey(feature)) { return null; } + // Implements a breadth-first-search on child nodes Set allEnables = new HashSet(nodes.get(feature).getEnables()); Deque queue = new ArrayDeque(allEnables); Set visited = new HashSet(); @@ -123,6 +125,8 @@ public Set getAllEnables(String feature) { return allEnables; } + /** Will be useful for future features **/ + // public Set getAllConfigElements(String feature) { // Set configElements = new HashSet(); // for (String node : getAllEnables(feature)) { diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java index 639c2d11..4c8e1ccb 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java @@ -15,6 +15,8 @@ import java.util.HashSet; import java.util.Set; + +// Class to represent a feature OR config element in a feature list xml public class FeatureListNode { protected String nodeName; protected Set enabledBy; @@ -42,6 +44,7 @@ public Set getEnables() { return enables; } + // based on a heuristic that features use major versions and config elements don't use '.' public boolean isConfigElement() { return this.nodeName.indexOf('.') == -1; } diff --git a/lemminx-liberty/src/test/java/io/openliberty/CodeActionUtilitiesTest.java b/lemminx-liberty/src/test/java/io/openliberty/CodeActionUtilitiesTest.java new file mode 100644 index 00000000..88513389 --- /dev/null +++ b/lemminx-liberty/src/test/java/io/openliberty/CodeActionUtilitiesTest.java @@ -0,0 +1,33 @@ +package io.openliberty; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; + +import io.openliberty.tools.langserver.lemminx.codeactions.IndentUtil; + +public class CodeActionUtilitiesTest { + + @Test + public void indentSpacesTest() { + String indent = " "; // four spaces + String sampleText = "\n\ttest"; + int column = 4; + String expectedText = System.lineSeparator() + " test"; + String formatedText = IndentUtil.formatText(sampleText, indent, column); + assertEquals(expectedText, formatedText, "Expected length of " + expectedText.length() + ". Found " + formatedText.length()); + + int column2 = 3; + String expectedText2 = System.lineSeparator() + " test"; + assertEquals(expectedText2, IndentUtil.formatText(sampleText, indent, column2), "Incorrect detection starting indent."); + } + + @Test + public void indentTabsTest() { + String indent = " "; + String sampleText = "\n\ttest"; + int column = 1; + String expectedText = System.lineSeparator() + indent + indent + "test"; + assertEquals(expectedText, IndentUtil.formatText(sampleText, indent, column), "Incorrect whitespace buffer calculation."); + } +} From a83c26c4c72507696908a9419c9469ba166fce16 Mon Sep 17 00:00:00 2001 From: dshimo Date: Fri, 6 Oct 2023 15:48:46 -0500 Subject: [PATCH 04/14] PR cleanup --- .../lemminx/LibertyDiagnosticParticipant.java | 12 ++++++++---- .../langserver/lemminx/codeactions/AddFeature.java | 3 --- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java index ceb518fd..7d939af0 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java @@ -45,7 +45,7 @@ public class LibertyDiagnosticParticipant implements IDiagnosticsParticipant { public static final String MISSING_FILE_MESSAGE = "The resource at the specified location could not be found."; public static final String MISSING_FILE_CODE = "missing_file"; - public static final String MISSING_CONFIGURED_FEATURE_MESSAGE = "This config element does not configure a feature in the featureManager. Remove this element or add a relevant feature."; + public static final String MISSING_CONFIGURED_FEATURE_MESSAGE = "This config element does not relate to a feature configured in the featureManager. Remove this element or add a relevant feature."; public static final String MISSING_CONFIGURED_FEATURE_CODE = "lost_config_element"; public static final String NOT_OPTIONAL_MESSAGE = "The specified resource cannot be skipped. Check location value or set optional to true."; @@ -90,7 +90,7 @@ private void validateDom(DOMDocument domDocument, List diagnosticsLi } else if (LibertyConstants.INCLUDE_ELEMENT.equals(nodeName)) { validateIncludeLocation(domDocument, diagnosticsList, node); } else if (featureGraph.isConfigElement(nodeName)) { // defaults to false - holdConfigElement(domDocument, diagnosticsList, node, tempDiagnosticsList); + holdConfigElement(domDocument, node, tempDiagnosticsList); } } validateConfigElements(diagnosticsList, tempDiagnosticsList, featureGraph); @@ -189,7 +189,7 @@ private void validateIncludeLocation(DOMDocument domDocument, List d * @param configElementNode * @param tempDiagnosticsList */ - private void holdConfigElement(DOMDocument domDocument, List diagnosticsList, DOMNode configElementNode, List tempDiagnosticsList) { + private void holdConfigElement(DOMDocument domDocument, DOMNode configElementNode, List tempDiagnosticsList) { String configElementName = configElementNode.getNodeName(); Range range = XMLPositionUtility.createRange(configElementNode.getStart(), configElementNode.getEnd(), domDocument); Diagnostic tempDiagnostic = new Diagnostic(range, MISSING_CONFIGURED_FEATURE_MESSAGE, null, LIBERTY_LEMMINX_SOURCE, MISSING_CONFIGURED_FEATURE_CODE); @@ -205,8 +205,12 @@ private void holdConfigElement(DOMDocument domDocument, List diagnos */ private void validateConfigElements(List diagnosticsList, List tempDiagnosticsList, FeatureListGraph featureGraph) { for (Diagnostic tempDiagnostic : tempDiagnosticsList) { + if (includedFeatures == null) { + diagnosticsList.add(tempDiagnostic); + continue; + } String configElement = tempDiagnostic.getSource(); - Set includedFeaturesCopy = (includedFeatures == null) ? new HashSet() : new HashSet(includedFeatures); + Set includedFeaturesCopy = new HashSet(includedFeatures); Set compatibleFeaturesList = featureGraph.getAllEnabledBy(configElement); includedFeaturesCopy.retainAll(compatibleFeaturesList); if (includedFeaturesCopy.isEmpty()) { diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java index 2577965c..d24424d7 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -14,7 +14,6 @@ import java.util.List; import java.util.Set; -import java.util.logging.Logger; import org.eclipse.lemminx.commons.CodeActionFactory; import org.eclipse.lemminx.dom.DOMDocument; @@ -66,8 +65,6 @@ public void doCodeAction(ICodeActionRequest request, List codeAction } Range nodeRange = XMLPositionUtility.createRange(insertNode.getStart(), insertEndOffset, document); - Logger.getLogger("dshi").warning("startCharacter" + nodeRange.getStart().getCharacter()); - Logger.getLogger("dshi").warning("endCharacter" + nodeRange.getEnd().getCharacter()); insertText = IndentUtil.formatText(insertText, indent, nodeRange.getStart().getCharacter()); // getAllEnabledBy would return all transitive features, but is too many to offer without a filter From a7d3ff3802c6b9dfecceba1396e96b7e05525f02 Mon Sep 17 00:00:00 2001 From: dshimo Date: Sun, 8 Oct 2023 16:54:51 -0500 Subject: [PATCH 05/14] move featureListGraph to workspaces --- .../lemminx/LibertyDiagnosticParticipant.java | 23 +++++------- .../lemminx/services/FeatureService.java | 17 +++++---- .../lemminx/services/LibertyWorkspace.java | 23 +++++++++++- .../io/openliberty/LibertyDiagnosticTest.java | 35 ++++++++++++------- .../io/openliberty/LibertyFeatureTest.java | 2 +- 5 files changed, 63 insertions(+), 37 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java index 7d939af0..12838489 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java @@ -26,6 +26,8 @@ import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph; import io.openliberty.tools.langserver.lemminx.data.LibertyRuntime; import io.openliberty.tools.langserver.lemminx.services.FeatureService; +import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager; +import io.openliberty.tools.langserver.lemminx.services.LibertyWorkspace; import io.openliberty.tools.langserver.lemminx.services.SettingsService; import io.openliberty.tools.langserver.lemminx.util.*; @@ -55,7 +57,7 @@ public class LibertyDiagnosticParticipant implements IDiagnosticsParticipant { public static final String INCORRECT_FEATURE_CODE = "incorrect_feature"; - private Set includedFeatures = null; + private Set includedFeatures; @Override public void doDiagnostics(DOMDocument domDocument, List diagnostics, @@ -73,16 +75,10 @@ public void doDiagnostics(DOMDocument domDocument, List diagnostics, private void validateDom(DOMDocument domDocument, List diagnosticsList) throws IOException { List nodes = domDocument.getDocumentElement().getChildren(); List tempDiagnosticsList = new ArrayList(); - - FeatureListGraph featureGraph = FeatureService.getInstance().getFeatureListGraph(); + includedFeatures = new HashSet<>(); + LibertyWorkspace workspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(domDocument.getDocumentURI()); // TODO: Consider adding a cached feature list onto repo to optimize - if (featureGraph.isEmpty()) { - LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(domDocument); - String libertyVersion = runtimeInfo == null ? null : runtimeInfo.getRuntimeVersion(); - String libertyRuntime = runtimeInfo == null ? null : runtimeInfo.getRuntimeType(); - FeatureService.getInstance().getInstalledFeaturesList(domDocument.getDocumentURI(), libertyRuntime, libertyVersion); - } - + FeatureListGraph featureGraph = workspace.getFeatureListGraph(); for (DOMNode node : nodes) { String nodeName = node.getNodeName(); if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(nodeName)) { @@ -105,7 +101,6 @@ private void validateFeature(DOMDocument domDocument, List list, DOM // Search for duplicate features // or features that do not exist - Set includedFeatures = new HashSet<>(); List features = featureManager.getChildren(); for (DOMNode featureNode : features) { DOMNode featureTextNode = (DOMNode) featureNode.getChildNodes().item(0); @@ -131,7 +126,6 @@ private void validateFeature(DOMDocument domDocument, List list, DOM } } } - this.includedFeatures = includedFeatures; } /** @@ -191,7 +185,8 @@ private void validateIncludeLocation(DOMDocument domDocument, List d */ private void holdConfigElement(DOMDocument domDocument, DOMNode configElementNode, List tempDiagnosticsList) { String configElementName = configElementNode.getNodeName(); - Range range = XMLPositionUtility.createRange(configElementNode.getStart(), configElementNode.getEnd(), domDocument); + Range range = XMLPositionUtility + .createRange(configElementNode.getStart(), configElementNode.getEnd(), domDocument); Diagnostic tempDiagnostic = new Diagnostic(range, MISSING_CONFIGURED_FEATURE_MESSAGE, null, LIBERTY_LEMMINX_SOURCE, MISSING_CONFIGURED_FEATURE_CODE); tempDiagnostic.setSource(configElementName); tempDiagnosticsList.add(tempDiagnostic); @@ -205,7 +200,7 @@ private void holdConfigElement(DOMDocument domDocument, DOMNode configElementNod */ private void validateConfigElements(List diagnosticsList, List tempDiagnosticsList, FeatureListGraph featureGraph) { for (Diagnostic tempDiagnostic : tempDiagnosticsList) { - if (includedFeatures == null) { + if (includedFeatures.isEmpty()) { diagnosticsList.add(tempDiagnostic); continue; } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java index e3cda443..113103e7 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java @@ -73,11 +73,8 @@ public static FeatureService getInstance() { private List defaultFeatureList; private long featureUpdateTime; - private FeatureListGraph featureListGraph; - private FeatureService() { featureCache = new HashMap<>(); - featureListGraph = new FeatureListGraph(); featureUpdateTime = -1; } @@ -273,9 +270,8 @@ public List collectExistingFeatures(DOMNode featureManager, String curre * @param libertyVersion must not be null and should be a valid Liberty version (e.g. 23.0.0.6) * @return list of installed features, or empty list */ - public List getInstalledFeaturesList(String documentURI, String libertyRuntime, String libertyVersion) { + public List getInstalledFeaturesList(LibertyWorkspace libertyWorkspace, String libertyRuntime, String libertyVersion) { List installedFeatures = new ArrayList(); - LibertyWorkspace libertyWorkspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(documentURI); if (libertyWorkspace == null || libertyWorkspace.getWorkspaceString() == null) { return installedFeatures; } @@ -317,6 +313,11 @@ public List getInstalledFeaturesList(String documentURI, String liberty LOGGER.info("Returning installed features: " + installedFeatures.size()); return installedFeatures; } + + public List getInstalledFeaturesList(String documentURI, String libertyRuntime, String libertyVersion) { + LibertyWorkspace libertyWorkspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(documentURI); + return getInstalledFeaturesList(libertyWorkspace, libertyRuntime, libertyVersion); + } /** * Generate the featurelist file for a LibertyWorkspace using the ws-featurelist.jar in the corresponding Liberty installation @@ -376,6 +377,7 @@ public List readFeaturesFromFeatureListFile(List installedFeat // Note: Only the public features are loaded when unmarshalling the passed featureListFile. if ((featureInfo.getFeatures() != null) && (featureInfo.getFeatures().size() > 0)) { + FeatureListGraph featureListGraph = new FeatureListGraph(); for (Feature f : featureInfo.getFeatures()) { f.setShortDescription(f.getDescription()); // The xml featureListFile does not have a wlpInformation element like the json does, but our code depends on looking up @@ -404,14 +406,11 @@ public List readFeaturesFromFeatureListFile(List installedFeat } } installedFeatures = featureInfo.getFeatures(); + libertyWorkspace.setFeatureListGraph(featureListGraph); libertyWorkspace.setInstalledFeatureList(installedFeatures); } else { LOGGER.warning("Unable to get installed features for current Liberty workspace: " + libertyWorkspace.getWorkspaceString()); } return installedFeatures; } - - public FeatureListGraph getFeatureListGraph() { - return this.featureListGraph; - } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java index 4022061a..ee6d5cd3 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java @@ -26,9 +26,11 @@ import jakarta.xml.bind.JAXBContext; import jakarta.xml.bind.JAXBException; import jakarta.xml.bind.Unmarshaller; - +import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph; +import io.openliberty.tools.langserver.lemminx.data.LibertyRuntime; import io.openliberty.tools.langserver.lemminx.models.feature.Feature; import io.openliberty.tools.langserver.lemminx.models.settings.DevcMetadata; +import io.openliberty.tools.langserver.lemminx.util.LibertyUtils; public class LibertyWorkspace { @@ -43,6 +45,7 @@ public class LibertyWorkspace { private List installedFeatureList; private String libertyInstallationDir; private boolean isExternalLibertyInstallation; + private FeatureListGraph featureListGraph; // devc vars private String containerName; @@ -65,6 +68,7 @@ public LibertyWorkspace(String workspaceFolderURI) { this.installedFeatureList = new ArrayList(); this.containerName = null; this.containerAlive = false; + this.featureListGraph = new FeatureListGraph(); } public String getWorkspaceString() { @@ -206,4 +210,21 @@ public String toString() { return workspaceFolderURI; } + public void setFeatureListGraph(FeatureListGraph featureListGraph) { + this.featureListGraph = featureListGraph; + } + + public FeatureListGraph getFeatureListGraph() { + if (this.isLibertyInstalled && featureListGraph.isEmpty()) { + LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(this); + LOGGER.info("Generating installed features list and storing to cache for workspace " + workspaceFolderURI); + FeatureService.getInstance().getInstalledFeaturesList(this, + runtimeInfo.getRuntimeType(), runtimeInfo.getRuntimeVersion()); + if (!this.featureListGraph.isEmpty()) { + LOGGER.info("Lost config element detection feature is available."); + } + } + return this.featureListGraph; + } + } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index 1dc26239..3ec292c7 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -6,6 +6,7 @@ import org.eclipse.lsp4j.Diagnostic; import org.eclipse.lsp4j.TextEdit; import org.eclipse.lsp4j.WorkspaceFolder; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import io.openliberty.tools.langserver.lemminx.LibertyDiagnosticParticipant; @@ -30,7 +31,21 @@ public class LibertyDiagnosticTest { static String newLine = System.lineSeparator(); - static String serverXMLURI = "test/server.xml"; + + static File srcResourcesDir = new File("src/test/resources"); + static File featureList = new File("src/test/resources/featurelist-ol-23.0.0.1-beta.xml"); + static String serverXMLURI = new File(srcResourcesDir, "test/server.xml").toURI().toString(); + static List initList = new ArrayList(); + LibertyProjectsManager libPM; + LibertyWorkspace libWorkspace; + + @BeforeEach + public void setupWorkspace() { + initList.add(new WorkspaceFolder(srcResourcesDir.toURI().toString())); + libPM = LibertyProjectsManager.getInstance(); + libPM.setWorkspaceFolders(initList); + libWorkspace = libPM.getLibertyWorkspaceFolders().iterator().next(); + } @Test public void testFeatureDuplicateDiagnostic() { @@ -181,24 +196,22 @@ public void testDiagnosticsForInclude() throws IOException { @Test public void testConfigElementMissingFeatureManager() throws JAXBException { - File featureList = new File("src/test/resources/featurelist-ol-23.0.0.1-beta.xml"); assertTrue(featureList.exists()); - FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), - new LibertyWorkspace(""), featureList); + FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), libWorkspace, featureList); - String serverXml = "ssl id=\"\"/>"; + String serverXml = ""; Diagnostic config_for_missing_feature = new Diagnostic(); - config_for_missing_feature.setRange(r(0, 0, 0, serverXml.length())); + config_for_missing_feature.setRange(r(0, serverXml.indexOf("".length())); config_for_missing_feature.setCode(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_CODE); config_for_missing_feature.setMessage(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_MESSAGE); + + XMLAssert.testDiagnosticsFor(serverXml, null, null, serverXMLURI, config_for_missing_feature); } @Test public void testConfigElementDirect() throws JAXBException { - File featureList = new File("src/test/resources/featurelist-ol-23.0.0.1-beta.xml"); assertTrue(featureList.exists()); - FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), - new LibertyWorkspace(""), featureList); + FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), libWorkspace, featureList); String correctFeature = " ssl-1.0"; String incorrectFeature = " jaxrs-2.0"; @@ -235,10 +248,8 @@ public void testConfigElementDirect() throws JAXBException { @Test public void testConfigElementTransitive() throws JAXBException { - File featureList = new File("src/test/resources/featurelist-ol-23.0.0.1-beta.xml"); assertTrue(featureList.exists()); - FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), - new LibertyWorkspace(""), featureList); + FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList(), libWorkspace, featureList); String serverXML1 = String.join(newLine, "", " ", diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java index bc1145cb..c3971b37 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java @@ -45,7 +45,7 @@ public void getInstalledFeaturesListTest() throws JAXBException { assertTrue(installedFeatures.removeIf(f -> (f.getName().equals("cdi-4.0")))); // Check if config map gets built - FeatureListGraph fg = fs.getFeatureListGraph(); + FeatureListGraph fg = libWorkspace.getFeatureListGraph(); assertEquals(76, fg.getAllEnabledBy("ssl-1.0").size()); assertEquals(1, fg.get("ssl").getEnabledBy().size()); assertTrue(fg.get("ssl").getEnabledBy().contains("ssl-1.0")); From a31e19ca9b2639323b33a50daf3c9bfdf50682b6 Mon Sep 17 00:00:00 2001 From: dshimo Date: Sun, 8 Oct 2023 16:55:29 -0500 Subject: [PATCH 06/14] add feature clarification and cleanup --- .../lemminx/codeactions/AddFeature.java | 95 ++++++++++++------- .../lemminx/codeactions/IndentUtil.java | 17 +++- 2 files changed, 74 insertions(+), 38 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java index d24424d7..1a1e046e 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -14,8 +14,11 @@ import java.util.List; import java.util.Set; +import java.util.logging.Logger; +import org.eclipse.lemminx.commons.BadLocationException; import org.eclipse.lemminx.commons.CodeActionFactory; +import org.eclipse.lemminx.commons.TextDocument; import org.eclipse.lemminx.dom.DOMDocument; import org.eclipse.lemminx.dom.DOMElement; import org.eclipse.lemminx.dom.DOMNode; @@ -27,55 +30,79 @@ import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.jsonrpc.CancelChecker; -import io.openliberty.tools.langserver.lemminx.services.FeatureService; +import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager; import io.openliberty.tools.langserver.lemminx.util.LibertyConstants; public class AddFeature implements ICodeActionParticipant { - public static final String FEATURE_FORMAT = "\n%s"; + + /** This code action adresses 3 main situations: + * 1) Add a feature to an existing empty featureManager + * 2) Add a feature to an existing featureManager with children + * 3) Add a feature and new featureManager + * + * To calculate where to insert, each scenario will use a reference point to calculate range + * 1) The startTag of the featureManager + * 2) The last child of the featureManager + * 3) The startTag of the server.xml + */ + public static final String FEATURE_FORMAT = "%s"; public static final String FEATUREMANAGER_FORMAT = - "\n\t" + - "\n\t\t%s" + + "\n\t"+ + "\n\t\t%s"+ "\n\t"; @Override public void doCodeAction(ICodeActionRequest request, List codeActions, CancelChecker cancelChecker) { Diagnostic diagnostic = request.getDiagnostic(); - DOMDocument document = request.getDocument(); - try { - String insertText = ""; - DOMNode insertNode = null; - String indent = request.getXMLGenerator().getWhitespacesIndent(); + DOMDocument document = request.getDocument(); + TextDocument textDocument = document.getTextDocument(); + String insertText = ""; + int referenceRangeStart = 0; + int referenceRangeEnd = 0; - for (DOMNode node : document.getDocumentElement().getChildren()) { - if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) { - // If featureManager is empty, add new feature after featureManager start tag. - // Otherwise, add feature after last feature child. - insertNode = node.getLastChild(); - insertText = (insertNode == null) ? FEATURE_FORMAT.replace("\n", "\n\t") : FEATURE_FORMAT; - insertNode = (insertNode == null) ? node : insertNode; - break; + for (DOMNode node : document.getDocumentElement().getChildren()) { + if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) { + DOMNode lastChild = node.getLastChild(); + if (lastChild == null) { + // Situation 1 + insertText = "\n\t" + FEATURE_FORMAT; + DOMElement featureManager = (DOMElement) node; + referenceRangeStart = featureManager.getStartTagOpenOffset(); + referenceRangeEnd = featureManager.getStartTagCloseOffset()+1; + } else { + // Situation 2 + insertText = "\n" + FEATURE_FORMAT; + referenceRangeStart = lastChild.getStart(); + referenceRangeEnd = lastChild.getEnd(); } + break; } + } + // Situation 3 + if (insertText.isEmpty()) { + insertText = FEATUREMANAGER_FORMAT; + DOMElement server = document.getDocumentElement(); + referenceRangeStart = server.getStart(); + referenceRangeEnd = server.getStartTagCloseOffset()+1; + } + Range referenceRange = XMLPositionUtility.createRange(referenceRangeStart, referenceRangeEnd, document); - int insertEndOffset = (insertNode == null) ? ((DOMElement)document.getDocumentElement()).getStartTagCloseOffset()+1 : insertNode.getEnd(); - // featureManager not found - if (insertNode == null) { - insertNode = document.getDocumentElement(); - insertText = FEATUREMANAGER_FORMAT; - } - - Range nodeRange = XMLPositionUtility.createRange(insertNode.getStart(), insertEndOffset, document); - insertText = IndentUtil.formatText(insertText, indent, nodeRange.getStart().getCharacter()); + try { + String indent = request.getXMLGenerator().getWhitespacesIndent(); + insertText = IndentUtil.formatText(insertText, indent, referenceRange.getStart().getCharacter()); + } catch (BadLocationException e) { + e.printStackTrace(); + } - // getAllEnabledBy would return all transitive features, but is too many to offer without a filter - Set featureCandidates = FeatureService.getInstance().getFeatureListGraph().get(diagnostic.getSource()).getEnabledBy(); - for (String feature : featureCandidates) { - String title = "Add feature " + feature; - codeActions.add(CodeActionFactory.insert( - title, nodeRange.getEnd(), String.format(insertText, feature), document.getTextDocument(), diagnostic)); - } - } catch (Exception e) { + // getAllEnabledBy would return all transitive features but typically offers too much + Set featureCandidates = LibertyProjectsManager.getInstance() + .getWorkspaceFolder(document.getDocumentURI()) + .getFeatureListGraph().get(diagnostic.getSource()).getEnabledBy(); + for (String feature : featureCandidates) { + String title = "Add feature " + feature; + codeActions.add(CodeActionFactory.insert( + title, referenceRange.getEnd(), String.format(insertText, feature), textDocument, diagnostic)); } } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/IndentUtil.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/IndentUtil.java index d0a06f39..205eecad 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/IndentUtil.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/IndentUtil.java @@ -12,17 +12,26 @@ *******************************************************************************/ package io.openliberty.tools.langserver.lemminx.codeactions; +// Use this helper class to format your strings with indentation public class IndentUtil { public static final String NEW_LINE = System.lineSeparator(); public static String whitespaceBuffer(String indent, int column) { - String whitespaceBuffer = ""; - for (int i = 0; i < column / indent.length(); i++) { - whitespaceBuffer += indent; + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < column / indent.length(); ++i) { + sb.append(indent); } - return whitespaceBuffer; + return sb.toString(); } + /** + * Will return a string where `\n` will be replaced with a proper line separator and match the + * indentation level for the passed in column number. Adding `\t` will add an indent level. + * @param text + * @param indent + * @param column + * @return + */ public static String formatText(String text, String indent, int column) { return text.replace("\n", System.lineSeparator() + whitespaceBuffer(indent, column)) .replace("\t", indent); From 1508c35212971d965335997d0d0f1ae44b78bc6b Mon Sep 17 00:00:00 2001 From: Evie Lau Date: Mon, 9 Oct 2023 13:04:23 -0500 Subject: [PATCH 07/14] Use new surefire with junit 5 support, skip tests in build step GHA --- .github/workflows/build.yml | 2 +- lemminx-liberty/pom.xml | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d885f6f8..549113dc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -39,7 +39,7 @@ jobs: java-version: 17 - name: Build Lemminx Liberty working-directory: ./lemminx-liberty - run: ./mvnw clean package -ntp + run: ./mvnw clean package -ntp -DskipTests - name: Test Lemminx Liberty working-directory: ./lemminx-liberty run: ./mvnw verify -ntp \ No newline at end of file diff --git a/lemminx-liberty/pom.xml b/lemminx-liberty/pom.xml index 263026b8..d026891d 100644 --- a/lemminx-liberty/pom.xml +++ b/lemminx-liberty/pom.xml @@ -55,6 +55,15 @@ 17 + + org.apache.maven.plugins + maven-surefire-plugin + 3.1.2 + + + maven-failsafe-plugin + 3.1.2 + org.apache.maven.plugins maven-shade-plugin From 7379b641b1ffae0f50f97e33c1ed9ef04603eea3 Mon Sep 17 00:00:00 2001 From: Evie Lau Date: Mon, 9 Oct 2023 13:14:11 -0500 Subject: [PATCH 08/14] Fix uri string in tests --- .../java/io/openliberty/XmlReaderTest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lemminx-liberty/src/test/java/io/openliberty/XmlReaderTest.java b/lemminx-liberty/src/test/java/io/openliberty/XmlReaderTest.java index 011e5dec..05c81f8b 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/XmlReaderTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/XmlReaderTest.java @@ -21,26 +21,26 @@ public class XmlReaderTest { @Test public void readEmptyXml() throws IOException { File emptyXml = new File(resourcesDir, "empty_server.xml"); - assertFalse(XmlReader.hasServerRoot(emptyXml.getCanonicalPath())); - assertFalse(LibertyUtils.isServerXMLFile(emptyXml.getCanonicalPath())); - assertFalse(LibertyUtils.isConfigXMLFile(emptyXml.getCanonicalPath())); + assertFalse(XmlReader.hasServerRoot(emptyXml.toURI().toString())); + assertFalse(LibertyUtils.isServerXMLFile(emptyXml.toURI().toString())); + assertFalse(LibertyUtils.isConfigXMLFile(emptyXml.toURI().toString())); } @Test public void readServerXml() throws IOException { File sampleServerXml = new File(resourcesDir, "sample/custom_server.xml"); - assertTrue(XmlReader.hasServerRoot(sampleServerXml.getCanonicalPath())); - assertFalse(LibertyUtils.isServerXMLFile(sampleServerXml.getCanonicalPath())); - assertFalse(LibertyUtils.isConfigDirFile(sampleServerXml.getCanonicalPath())); - assertTrue(LibertyUtils.isConfigXMLFile(sampleServerXml.getCanonicalPath())); + assertTrue(XmlReader.hasServerRoot(sampleServerXml.toURI().toString())); + assertFalse(LibertyUtils.isServerXMLFile(sampleServerXml.toURI().toString())); + assertFalse(LibertyUtils.isConfigDirFile(sampleServerXml.toURI().toString())); + assertTrue(LibertyUtils.isConfigXMLFile(sampleServerXml.toURI().toString())); } @Test public void readLibertyPluginConfigXml() throws IOException { File lpcXml = new File(resourcesDir, "sample/liberty-plugin-config.xml"); Path lpcXmlPath = lpcXml.toPath(); - assertFalse(XmlReader.hasServerRoot(lpcXml.getCanonicalPath())); - assertFalse(LibertyUtils.isConfigXMLFile(lpcXml.getCanonicalPath())); + assertFalse(XmlReader.hasServerRoot(lpcXml.toURI().toString())); + assertFalse(LibertyUtils.isConfigXMLFile(lpcXml.toURI().toString())); Set elementNames = new HashSet (); elementNames.add("configFile"); From 77cbc58a12766b19e80fa58d9e4e437216d688e6 Mon Sep 17 00:00:00 2001 From: dshimo Date: Mon, 9 Oct 2023 12:29:50 -0500 Subject: [PATCH 09/14] graph cache. github actions still fail, pass local --- .../langserver/lemminx/data/FeatureListGraph.java | 11 ++++++++++- .../lemminx/services/LibertyWorkspace.java | 13 ++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java index 0cfce221..3d4a40ff 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java @@ -20,6 +20,7 @@ import java.util.Set; public class FeatureListGraph { + private String runtime = ""; private Map nodes; private Map> enabledByCache; private Map> enablesCache; @@ -61,7 +62,15 @@ public boolean isConfigElement(String featureListNode) { return false; } return nodes.get(featureListNode).isConfigElement(); - } + } + + public void setRuntime(String runtime) { + this.runtime = runtime; + } + + public String getRuntime() { + return this.runtime; + } /** * Returns a superset of 'owning' features that enable a given config element or feature. diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java index ee6d5cd3..43071243 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java @@ -212,16 +212,19 @@ public String toString() { public void setFeatureListGraph(FeatureListGraph featureListGraph) { this.featureListGraph = featureListGraph; + if (isLibertyInstalled) { + this.featureListGraph.setRuntime(libertyRuntime + "-" + libertyVersion); + } } public FeatureListGraph getFeatureListGraph() { - if (this.isLibertyInstalled && featureListGraph.isEmpty()) { - LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(this); + String workspaceRuntime = libertyRuntime + "-" + libertyVersion; + boolean generateGraph = featureListGraph.isEmpty() || !featureListGraph.getRuntime().equals(workspaceRuntime); + if (this.isLibertyInstalled && generateGraph) { LOGGER.info("Generating installed features list and storing to cache for workspace " + workspaceFolderURI); - FeatureService.getInstance().getInstalledFeaturesList(this, - runtimeInfo.getRuntimeType(), runtimeInfo.getRuntimeVersion()); + FeatureService.getInstance().getInstalledFeaturesList(this, libertyRuntime, libertyVersion); if (!this.featureListGraph.isEmpty()) { - LOGGER.info("Lost config element detection feature is available."); + LOGGER.info("Lost config element detection feature is available for workspace: " + workspaceFolderURI); } } return this.featureListGraph; From 763f9904414db4fcfe748db16eca4bb04a2de1bb Mon Sep 17 00:00:00 2001 From: dshimo Date: Mon, 9 Oct 2023 17:23:07 -0500 Subject: [PATCH 10/14] correct trigger for empty featureManager --- .../tools/langserver/lemminx/codeactions/AddFeature.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java index 1a1e046e..4d5e3ca5 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -63,7 +63,7 @@ public void doCodeAction(ICodeActionRequest request, List codeAction for (DOMNode node : document.getDocumentElement().getChildren()) { if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) { DOMNode lastChild = node.getLastChild(); - if (lastChild == null) { + if (lastChild == null || !lastChild.hasChildNodes()) { // Situation 1 insertText = "\n\t" + FEATURE_FORMAT; DOMElement featureManager = (DOMElement) node; From e8c0d692b646499fa7ccfd7634b5799c448a0c85 Mon Sep 17 00:00:00 2001 From: dshimo Date: Tue, 10 Oct 2023 12:21:26 -0500 Subject: [PATCH 11/14] PR comments --- .../lemminx/LibertyDiagnosticParticipant.java | 13 ++++++++----- .../lemminx/services/LibertyWorkspace.java | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java index 12838489..736c359c 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java @@ -78,7 +78,7 @@ private void validateDom(DOMDocument domDocument, List diagnosticsLi includedFeatures = new HashSet<>(); LibertyWorkspace workspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(domDocument.getDocumentURI()); // TODO: Consider adding a cached feature list onto repo to optimize - FeatureListGraph featureGraph = workspace.getFeatureListGraph(); + FeatureListGraph featureGraph = (workspace == null) ? new FeatureListGraph() : workspace.getFeatureListGraph(); for (DOMNode node : nodes) { String nodeName = node.getNodeName(); if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(nodeName)) { @@ -199,11 +199,14 @@ private void holdConfigElement(DOMDocument domDocument, DOMNode configElementNod * @param featureGraph */ private void validateConfigElements(List diagnosticsList, List tempDiagnosticsList, FeatureListGraph featureGraph) { + if (featureGraph.isEmpty()) { + return; + } + if (includedFeatures.isEmpty()) { + diagnosticsList.addAll(tempDiagnosticsList); + return; + } for (Diagnostic tempDiagnostic : tempDiagnosticsList) { - if (includedFeatures.isEmpty()) { - diagnosticsList.add(tempDiagnostic); - continue; - } String configElement = tempDiagnostic.getSource(); Set includedFeaturesCopy = new HashSet(includedFeatures); Set compatibleFeaturesList = featureGraph.getAllEnabledBy(configElement); diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java index 43071243..50d95dbf 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyWorkspace.java @@ -224,7 +224,7 @@ public FeatureListGraph getFeatureListGraph() { LOGGER.info("Generating installed features list and storing to cache for workspace " + workspaceFolderURI); FeatureService.getInstance().getInstalledFeaturesList(this, libertyRuntime, libertyVersion); if (!this.featureListGraph.isEmpty()) { - LOGGER.info("Lost config element detection feature is available for workspace: " + workspaceFolderURI); + LOGGER.info("Config element validation enabled for workspace: " + workspaceFolderURI); } } return this.featureListGraph; From 7e05e6d3aac36249f6588486781e7aa24a0e8bf5 Mon Sep 17 00:00:00 2001 From: dshimo Date: Tue, 10 Oct 2023 12:21:43 -0500 Subject: [PATCH 12/14] reduce request delay 120 -> 10 --- .../tools/langserver/lemminx/services/FeatureService.java | 3 ++- .../tools/langserver/lemminx/services/SettingsService.java | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java index 113103e7..b13a5cca 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java @@ -180,7 +180,7 @@ public List getFeatures(String libertyVersion, String libertyRuntime, i if (!libertyVersion.endsWith("-beta")) { try { // verify that request delay (seconds) has gone by since last fetch request - // Note that the default delay is 120 seconds and can cause us to generate a feature list instead of download from MC when + // Note that the default delay is 10 seconds and can cause us to generate a feature list instead of download from MC when // switching back and forth between projects. long currentTime = System.currentTimeMillis(); if (this.featureUpdateTime == -1 || currentTime >= (this.featureUpdateTime + (requestDelay * 1000))) { @@ -410,6 +410,7 @@ public List readFeaturesFromFeatureListFile(List installedFeat libertyWorkspace.setInstalledFeatureList(installedFeatures); } else { LOGGER.warning("Unable to get installed features for current Liberty workspace: " + libertyWorkspace.getWorkspaceString()); + libertyWorkspace.setFeatureListGraph(new FeatureListGraph()); } return installedFeatures; } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/SettingsService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/SettingsService.java index d65806df..a768155a 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/SettingsService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/SettingsService.java @@ -26,8 +26,8 @@ public static SettingsService getInstance() { return instance; } - // default request delay is 120 seconds - private static int DEFAULT_REQUEST_DELAY = 120; + // default request delay is 10 seconds + private static int DEFAULT_REQUEST_DELAY = 10; private SettingsService() { } From b7ef16b1bf5354699d5bd0f6ca789a1226e0c313 Mon Sep 17 00:00:00 2001 From: dshimo Date: Tue, 10 Oct 2023 12:22:16 -0500 Subject: [PATCH 13/14] working state for "empty featureManager" --- .../lemminx/codeactions/AddFeature.java | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java index 4d5e3ca5..8e2f9b74 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -34,6 +34,7 @@ import io.openliberty.tools.langserver.lemminx.util.LibertyConstants; public class AddFeature implements ICodeActionParticipant { + Logger LOGGER = Logger.getLogger(AddFeature.class.getName()); /** This code action adresses 3 main situations: * 1) Add a feature to an existing empty featureManager @@ -56,6 +57,14 @@ public void doCodeAction(ICodeActionRequest request, List codeAction Diagnostic diagnostic = request.getDiagnostic(); DOMDocument document = request.getDocument(); TextDocument textDocument = document.getTextDocument(); + // getAllEnabledBy would return all transitive features but typically offers too much + Set featureCandidates = LibertyProjectsManager.getInstance() + .getWorkspaceFolder(document.getDocumentURI()) + .getFeatureListGraph().get(diagnostic.getSource()).getEnabledBy(); + if (featureCandidates.isEmpty()) { + return; + } + String insertText = ""; int referenceRangeStart = 0; int referenceRangeEnd = 0; @@ -63,17 +72,24 @@ public void doCodeAction(ICodeActionRequest request, List codeAction for (DOMNode node : document.getDocumentElement().getChildren()) { if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) { DOMNode lastChild = node.getLastChild(); - if (lastChild == null || !lastChild.hasChildNodes()) { - // Situation 1 - insertText = "\n\t" + FEATURE_FORMAT; - DOMElement featureManager = (DOMElement) node; - referenceRangeStart = featureManager.getStartTagOpenOffset(); - referenceRangeEnd = featureManager.getStartTagCloseOffset()+1; - } else { + if (node.getChildren().size() > 1) { // Situation 2 insertText = "\n" + FEATURE_FORMAT; referenceRangeStart = lastChild.getStart(); referenceRangeEnd = lastChild.getEnd(); + } else { + if (lastChild != null && (lastChild.hasChildNodes() || lastChild.isComment())) { + // Situation 2 + insertText = "\n" + FEATURE_FORMAT; + referenceRangeStart = lastChild.getStart(); + referenceRangeEnd = lastChild.getEnd(); + } else { + // Situation 1 + insertText = "\n\t" + FEATURE_FORMAT; + DOMElement featureManager = (DOMElement) node; + referenceRangeStart = featureManager.getStartTagOpenOffset(); + referenceRangeEnd = featureManager.getStartTagCloseOffset()+1; + } } break; } @@ -87,17 +103,13 @@ public void doCodeAction(ICodeActionRequest request, List codeAction } Range referenceRange = XMLPositionUtility.createRange(referenceRangeStart, referenceRangeEnd, document); + String indent = " "; try { - String indent = request.getXMLGenerator().getWhitespacesIndent(); - insertText = IndentUtil.formatText(insertText, indent, referenceRange.getStart().getCharacter()); + indent = request.getXMLGenerator().getWhitespacesIndent(); } catch (BadLocationException e) { - e.printStackTrace(); + Logger.getLogger(AddFeature.class.getName()).info("Defaulting indent to four spaces."); } - - // getAllEnabledBy would return all transitive features but typically offers too much - Set featureCandidates = LibertyProjectsManager.getInstance() - .getWorkspaceFolder(document.getDocumentURI()) - .getFeatureListGraph().get(diagnostic.getSource()).getEnabledBy(); + insertText = IndentUtil.formatText(insertText, indent, referenceRange.getStart().getCharacter()); for (String feature : featureCandidates) { String title = "Add feature " + feature; From 1715c7069da9774f4c2ec4b19a1e7eb8702ccd93 Mon Sep 17 00:00:00 2001 From: dshimo Date: Tue, 10 Oct 2023 12:47:24 -0500 Subject: [PATCH 14/14] add logger to addFeature --- .../tools/langserver/lemminx/codeactions/AddFeature.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java index 8e2f9b74..38aae88b 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -107,7 +107,7 @@ public void doCodeAction(ICodeActionRequest request, List codeAction try { indent = request.getXMLGenerator().getWhitespacesIndent(); } catch (BadLocationException e) { - Logger.getLogger(AddFeature.class.getName()).info("Defaulting indent to four spaces."); + LOGGER.info("Defaulting indent to four spaces."); } insertText = IndentUtil.formatText(insertText, indent, referenceRange.getStart().getCharacter());