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 9067a2a9..d7d28550 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 @@ -77,7 +77,6 @@ private void validateDom(DOMDocument domDocument, List diagnosticsLi List tempDiagnosticsList = new ArrayList(); includedFeatures = new HashSet<>(); LibertyWorkspace workspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(domDocument.getDocumentURI()); - // TODO: Consider adding a cached feature list onto repo to optimize FeatureListGraph featureGraph = (workspace == null) ? new FeatureListGraph() : workspace.getFeatureListGraph(); for (DOMNode node : nodes) { String nodeName = node.getNodeName(); 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 38aae88b..da28a90a 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 @@ -12,6 +12,8 @@ *******************************************************************************/ package io.openliberty.tools.langserver.lemminx.codeactions; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Set; import java.util.logging.Logger; @@ -30,7 +32,11 @@ 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.FeatureListNode; +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.util.LibertyConstants; public class AddFeature implements ICodeActionParticipant { @@ -57,14 +63,30 @@ public void doCodeAction(ICodeActionRequest request, List codeAction Diagnostic diagnostic = request.getDiagnostic(); DOMDocument document = request.getDocument(); TextDocument textDocument = document.getTextDocument(); + + LibertyWorkspace ws = LibertyProjectsManager.getInstance().getWorkspaceFolder(document.getDocumentURI()); + if (ws == null) { + LOGGER.warning("Could not add quick fix for missing feature because could not find Liberty workspace for document: "+document.getDocumentURI()); + return; + } + + FeatureListNode flNode = ws.getFeatureListGraph().get(diagnostic.getSource()); + if (flNode == null) { + LOGGER.warning("Could not add quick fix for missing feature for config element due to missing information in the feature list: "+diagnostic.getSource()); + return; + } + // getAllEnabledBy would return all transitive features but typically offers too much - Set featureCandidates = LibertyProjectsManager.getInstance() - .getWorkspaceFolder(document.getDocumentURI()) - .getFeatureListGraph().get(diagnostic.getSource()).getEnabledBy(); + Set featureCandidates = flNode.getEnabledBy(); if (featureCandidates.isEmpty()) { return; } + // Need to sort the collection of features so that they are in a reliable order for tests. + ArrayList sortedFeatures = new ArrayList(); + sortedFeatures.addAll(featureCandidates); + Collections.sort(sortedFeatures); + String insertText = ""; int referenceRangeStart = 0; int referenceRangeEnd = 0; @@ -111,7 +133,7 @@ public void doCodeAction(ICodeActionRequest request, List codeAction } insertText = IndentUtil.formatText(insertText, indent, referenceRange.getStart().getCharacter()); - for (String feature : featureCandidates) { + for (String feature : sortedFeatures) { 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/services/FeatureService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java index b6df15d3..9999f4f6 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 @@ -31,6 +31,8 @@ import java.util.logging.Logger; import org.eclipse.lemminx.dom.DOMNode; +import org.eclipse.lemminx.uriresolver.CacheResourcesManager; +import org.eclipse.lemminx.uriresolver.CacheResourcesManager.ResourceToDeploy; import java.util.concurrent.TimeUnit; @@ -60,6 +62,24 @@ public class FeatureService { private static String olFeatureEndpoint = "https://repo1.maven.org/maven2/io/openliberty/features/features/%1$s/features-%1$s.json"; private static String wlpFeatureEndpoint = "https://repo1.maven.org/maven2/com/ibm/websphere/appserver/features/features/%1$s/features-%1$s.json"; + // This file is copied to the local .lemminx cache. + // This is how we ensure the latest default featurelist xml gets used in each developer environment. + private static final String FEATURELIST_XML_RESOURCE_URL = "https://github.com/OpenLiberty/liberty-language-server/blob/master/lemminx-liberty/src/main/resources/featurelist-cached-23.0.0.9.xml"; + private static final String FEATURELIST_XML_CLASSPATH_LOCATION = "/featurelist-cached-23.0.0.9.xml"; + + /** + * FEATURELIST_XML_RESOURCE is the featurelist xml that is located at FEATURELIST_XML_CLASSPATH_LOCATION + * that gets deployed (copied) to the .lemminx cache. The FEATURELIST_XML_RESOURCE_URL is + * used by lemmix to determine the path to store the file in the cache. So for the + * featurelist xml it takes the resource located at FEATURELIST_XML_CLASSPATH_LOCATION and deploys + * it to: + * ~/.lemminx/cache/https/github.com/OpenLiberty/liberty-language-server/master/lemminx-liberty/featurelist-cached-.xml + * + * Declared public to be used by tests + */ + public static final ResourceToDeploy FEATURELIST_XML_RESOURCE = new ResourceToDeploy(FEATURELIST_XML_RESOURCE_URL, + FEATURELIST_XML_CLASSPATH_LOCATION); + public static FeatureService getInstance() { if (instance == null) { instance = new FeatureService(); @@ -69,7 +89,8 @@ public static FeatureService getInstance() { // Cache of Liberty version -> list of supported features private Map> featureCache; // the key consists of runtime-version, where runtime is 'ol' or 'wlp' - private List defaultFeatureList; + private List defaultFeatures; + private FeatureListGraph defaultFeatureList; private long featureUpdateTime; private FeatureService() { @@ -103,28 +124,28 @@ private List fetchFeaturesForVersion(String libertyVersion, String libe } /** - * Returns the default feature list + * Returns the default list of features * * @return list of features supported by the default version of Liberty */ - private List getDefaultFeatureList() { + private List getDefaultFeatures() { try { - if (defaultFeatureList == null) { + if (defaultFeatures == null) { // Changing this to contain the version in the file name since the file is copied to the local .lemminx cache. // This is how we ensure the latest default features json gets used in each developer environment. InputStream is = getClass().getClassLoader().getResourceAsStream("features-cached-23.0.0.9.json"); InputStreamReader reader = new InputStreamReader(is, StandardCharsets.UTF_8); // Only need the public features - defaultFeatureList = readPublicFeatures(reader); + defaultFeatures = readPublicFeatures(reader); } - LOGGER.info("Returning default feature list"); - return defaultFeatureList; + LOGGER.info("Returning default list of features"); + return defaultFeatures; } catch (JsonParseException e) { // unable to read json in resources file, return empty list LOGGER.severe("Error: Unable to get default features."); - return defaultFeatureList; + return defaultFeatures; } } @@ -147,7 +168,7 @@ private ArrayList readPublicFeatures(InputStreamReader reader) throws J /** * Returns the Liberty features corresponding to the Liberty version. First - * attempts to fetch the feature list from Maven, otherwise falls back to the + * attempts to fetch the feature json from Maven, otherwise falls back to the * list of installed features. If the installed features list cannot be * gathered, falls back to the default cached features json file. * @@ -159,8 +180,9 @@ private ArrayList readPublicFeatures(InputStreamReader reader) throws J */ public List getFeatures(String libertyVersion, String libertyRuntime, int requestDelay, String documentURI) { if (libertyRuntime == null || libertyVersion == null) { - // return default feature list - List defaultFeatures = getDefaultFeatureList(); + // return default list of features + List defaultFeatures = getDefaultFeatures(); + getDefaultFeatureList(); return defaultFeatures; } @@ -201,8 +223,8 @@ public List getFeatures(String libertyVersion, String libertyRuntime, i return installedFeatures; } - // return default feature list - List defaultFeatures = getDefaultFeatureList(); + // return default list of features + List defaultFeatures = getDefaultFeatures(); return defaultFeatures; } @@ -345,6 +367,37 @@ public List getInstalledFeaturesList(String documentURI, String liberty return getInstalledFeaturesList(libertyWorkspace, libertyRuntime, libertyVersion); } + public FeatureListGraph getDefaultFeatureList() { + if (defaultFeatureList != null) { + return defaultFeatureList; + } + + try { + Path featurelistXmlFile = CacheResourcesManager.getResourceCachePath(FEATURELIST_XML_RESOURCE); + LOGGER.info("Using cached Liberty featurelist xml file located at: " + featurelistXmlFile.toString()); + + File featureListFile = featurelistXmlFile.toFile(); + + if (featureListFile != null && featureListFile.exists()) { + try { + readFeaturesFromFeatureListFile(null, null, featureListFile, true); + } catch (JAXBException e) { + LOGGER.severe("Error: Unable to load the default cached featurelist file due to exception: "+e.getMessage()); + } + } else { + LOGGER.warning("Unable to find the default cached featurelist at location: "+featurelistXmlFile.toString()); + } + } catch (Exception e) { + LOGGER.severe("Error: Unable to retrieve default cached featurelist file due to exception: "+e.getMessage()); + } + + if (defaultFeatureList == null) { + defaultFeatureList = new FeatureListGraph(); + } + + return defaultFeatureList; + } + /** * Generate the featurelist file for a LibertyWorkspace using the ws-featurelist.jar in the corresponding Liberty installation * @param libertyWorkspace @@ -397,13 +450,20 @@ private File generateFeatureListXml(LibertyWorkspace libertyWorkspace, Path feat public List readFeaturesFromFeatureListFile(List installedFeatures, LibertyWorkspace libertyWorkspace, File featureListFile) throws JAXBException { + return readFeaturesFromFeatureListFile(installedFeatures, libertyWorkspace, featureListFile, false); + } + + // If the graphOnly boolean is true, the libertyWorkspace parameter may be null. Also, the defaultFeatureList should be initialized + // after calling this method with graphOnly set to true. + public List readFeaturesFromFeatureListFile(List installedFeatures, LibertyWorkspace libertyWorkspace, + File featureListFile, boolean graphOnly) throws JAXBException { JAXBContext jaxbContext = JAXBContext.newInstance(FeatureInfo.class); Unmarshaller jaxbUnmarshaller = jaxbContext.createUnmarshaller(); FeatureInfo featureInfo = (FeatureInfo) jaxbUnmarshaller.unmarshal(featureListFile); + FeatureListGraph featureListGraph = new FeatureListGraph(); // 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 @@ -431,12 +491,21 @@ public List readFeaturesFromFeatureListFile(List installedFeat } } } - installedFeatures = featureInfo.getFeatures(); - libertyWorkspace.setFeatureListGraph(featureListGraph); - libertyWorkspace.setInstalledFeatureList(installedFeatures); + + if (!graphOnly) { + installedFeatures = featureInfo.getFeatures(); + libertyWorkspace.setInstalledFeatureList(installedFeatures); + libertyWorkspace.setFeatureListGraph(featureListGraph); + } else { + defaultFeatureList = featureListGraph; + } } else { - LOGGER.warning("Unable to get installed features for current Liberty workspace: " + libertyWorkspace.getWorkspaceString()); - libertyWorkspace.setFeatureListGraph(new FeatureListGraph()); + if (!graphOnly) { + LOGGER.warning("Unable to get installed features for current Liberty workspace: " + libertyWorkspace.getWorkspaceString()); + libertyWorkspace.setFeatureListGraph(new FeatureListGraph()); + } else { + defaultFeatureList = featureListGraph; + } } return installedFeatures; } 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 14fc3e99..a2f0f043 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 @@ -27,10 +27,8 @@ 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 { @@ -221,24 +219,39 @@ public String toString() { return workspaceFolderURI; } + public String getWorkspaceRuntime() { + if (libertyRuntime == null || libertyVersion == null) { + return ""; + } + return libertyRuntime + "-" + libertyVersion; + } + public void setFeatureListGraph(FeatureListGraph featureListGraph) { this.featureListGraph = featureListGraph; - if (isLibertyInstalled) { + if (isLibertyInstalled || isContainerAlive()) { this.featureListGraph.setRuntime(libertyRuntime + "-" + libertyVersion); + } else { + this.featureListGraph.setRuntime(""); } } public FeatureListGraph getFeatureListGraph() { - 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, libertyRuntime, libertyVersion); - if (!this.featureListGraph.isEmpty()) { + FeatureListGraph useFeatureListGraph = this.featureListGraph; + boolean generateGraph = featureListGraph.isEmpty() || !featureListGraph.getRuntime().equals(getWorkspaceRuntime()); + if (generateGraph) { + if (isLibertyInstalled || isContainerAlive()) { + LOGGER.info("Generating installed features list and storing to cache for workspace " + workspaceFolderURI); + FeatureService.getInstance().getInstalledFeaturesList(this, libertyRuntime, libertyVersion); + useFeatureListGraph = this.featureListGraph; + } else { + LOGGER.info("Retrieving default cached feature list for workspace " + workspaceFolderURI); + useFeatureListGraph = FeatureService.getInstance().getDefaultFeatureList(); + } + if (!useFeatureListGraph.isEmpty()) { LOGGER.info("Config element validation enabled for workspace: " + workspaceFolderURI); } } - return this.featureListGraph; + return useFeatureListGraph; } } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index 6fffeb3f..6b2467a5 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -4,12 +4,16 @@ import org.eclipse.lemminx.commons.BadLocationException; import org.eclipse.lsp4j.CodeAction; import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4j.TextDocumentEdit; import org.eclipse.lsp4j.TextEdit; +import org.eclipse.lsp4j.WorkspaceEdit; import org.eclipse.lsp4j.WorkspaceFolder; +import org.eclipse.lsp4j.jsonrpc.messages.Either; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import io.openliberty.tools.langserver.lemminx.LibertyDiagnosticParticipant; +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; @@ -19,6 +23,7 @@ import static org.eclipse.lemminx.XMLAssert.r; import static org.eclipse.lemminx.XMLAssert.ca; import static org.eclipse.lemminx.XMLAssert.te; +import static org.eclipse.lemminx.XMLAssert.tde; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -32,9 +37,10 @@ public class LibertyDiagnosticTest { static String newLine = System.lineSeparator(); - static File srcResourcesDir = new File("src/test/resources"); + static File srcResourcesDir = new File("src/test/resources/sample"); 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 String sampleserverXMLURI = new File(srcResourcesDir, "sample-server.xml").toURI().toString(); static List initList = new ArrayList(); LibertyProjectsManager libPM; LibertyWorkspace libWorkspace; @@ -208,6 +214,62 @@ public void testConfigElementMissingFeatureManager() throws JAXBException { XMLAssert.testDiagnosticsFor(serverXml, null, null, serverXMLURI, config_for_missing_feature); } + @Test + public void testConfigElementMissingFeatureUsingCachedFeaturelist() throws JAXBException, BadLocationException { + LibertyWorkspace ws = libPM.getWorkspaceFolder(sampleserverXMLURI); + ws.setFeatureListGraph(new FeatureListGraph()); // need to clear out the already loaded featureList from other test methods + FeatureService.getInstance().getDefaultFeatureList(); + + String correctFeature = " %s"; + String incorrectFeature = " jaxrs-2.0"; + String configElement = " "; + int diagnosticStart = configElement.indexOf("<"); + int diagnosticLength = configElement.trim().length(); + + String serverXML = 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(serverXML, null, null, sampleserverXMLURI, config_for_missing_feature); + + // TODO: Add code to check the CodeActions also. + config_for_missing_feature.setSource("springBootApplication"); + + List featuresToAdd = new ArrayList(); + featuresToAdd.add("springBoot-1.5"); + featuresToAdd.add("springBoot-2.0"); + featuresToAdd.add("springBoot-3.0"); + Collections.sort(featuresToAdd); + + List codeActions = new ArrayList(); + for (String nextFeature: featuresToAdd) { + String addFeature = System.lineSeparator()+String.format(correctFeature, nextFeature); + TextEdit texted = te(2, 36, 2, 36, addFeature); + CodeAction invalidCodeAction = ca(config_for_missing_feature, texted); + + TextDocumentEdit textDoc = tde(sampleserverXMLURI, 0, texted); + WorkspaceEdit workspaceEdit = new WorkspaceEdit(Collections.singletonList(Either.forLeft(textDoc))); + + invalidCodeAction.setEdit(workspaceEdit); + codeActions.add(invalidCodeAction); + } + + XMLAssert.testCodeActionsFor(serverXML, sampleserverXMLURI, config_for_missing_feature, (String) null, + codeActions.get(0), codeActions.get(1), codeActions.get(2)); + + } + + @Test public void testConfigElementDirect() throws JAXBException { assertTrue(featureList.exists()); diff --git a/lemminx-liberty/src/test/resources/sample/sample-server.xml b/lemminx-liberty/src/test/resources/sample/sample-server.xml new file mode 100644 index 00000000..c69e6c73 --- /dev/null +++ b/lemminx-liberty/src/test/resources/sample/sample-server.xml @@ -0,0 +1,6 @@ + + + jaxrs-2.0 + + + \ No newline at end of file