From b56be2d59b91b8f9536279e02f0881d6efd99790 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Mon, 9 Dec 2024 13:21:26 +0530 Subject: [PATCH 01/12] multiple variable completion Signed-off-by: Arun Venmany --- .../lemminx/LibertyCompletionParticipant.java | 24 +++- .../io/openliberty/LibertyCompletionTest.java | 135 ++++++++++++++++++ 2 files changed, 153 insertions(+), 6 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java index c2ccc534..57a2386c 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java @@ -53,12 +53,24 @@ public void onAttributeValue(String valuePrefix, ICompletionRequest request, ICo Properties variableProps = SettingsService.getInstance() .getVariablesForServerXml(request.getXMLDocument() .getDocumentURI()); - String variableName = valuePrefix.replace("$", "") - .replace("{", "") - .replace("}", ""); - variableProps.entrySet().stream().filter(it -> it.getKey().toString().toLowerCase().contains(variableName.toLowerCase())) + // value prefix will contain all existing attribute value, till the cursor + String variableName = valuePrefix; + String replacePrefix; + if (valuePrefix.contains("${")) { + variableName = valuePrefix.substring(valuePrefix.lastIndexOf("${")) + .replace("${", ""); + if (variableName.contains("}")) { + variableName = variableName.replace("}", ""); + } + // existing variables + replacePrefix = valuePrefix.substring(0, valuePrefix.lastIndexOf("${")); + } else { + replacePrefix = ""; + } + String finalVariableName = variableName; + variableProps.entrySet().stream().filter(it -> it.getKey().toString().toLowerCase().contains(finalVariableName.toLowerCase())) .forEach(variableProp -> { - String varValue = String.format("${%s}", variableProp.getKey()); + String varValue = String.format("%s${%s}", replacePrefix, variableProp.getKey()); Either edit = Either.forLeft(new TextEdit(request.getReplaceRange(), varValue)); @@ -67,7 +79,7 @@ public void onAttributeValue(String valuePrefix, ICompletionRequest request, ICo completionItem.setTextEdit(edit); completionItem.setFilterText(variableProp.getKey().toString()); completionItem.setKind(CompletionItemKind.Value); - completionItem.setDocumentation(String.format("%s = %s", variableProp.getKey(),variableProp.getValue())); + completionItem.setDocumentation(String.format("%s = %s", variableProp.getKey(), variableProp.getValue())); response.addCompletionItem(completionItem); }); } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java index b863ffee..9107e8ea 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java @@ -370,4 +370,139 @@ public void testVariableCompletionItemWithDefaultXsdValues() throws BadLocationE httpsCompletion); } + // Tests the + // availability of variable completion with ${, ${} and invalid patterns + @Test + public void testVariableCompletionItemWithVariablePatterns() throws BadLocationException { + String serverXML = String.join(newLine, // + "", // + " ", // + " javaee-6.0", // + " acmeCA-2.0", // + " ", // + " ",// + "" // + ); + Map propsMap=new HashMap<>(); + propsMap.put("default.http.port","9080"); + propsMap.put("default.https.port","9443"); + propsMap.put("testVar","false"); + propsMap.put("testVar2","true"); + Properties props = new Properties(); + props.putAll(propsMap); + + when(settingsService.getVariablesForServerXml(any())).thenReturn(props); + CompletionItem httpCompletion = c("${default.http.port}", "${default.http.port}"); + CompletionItem httpsCompletion = c("${default.https.port}", "${default.https.port}"); + final int TOTAL_ITEMS = 2; // total number of available completion items + + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, httpCompletion, + httpsCompletion); + + serverXML = String.join(newLine, // + "", // + " ", // + " javaee-6.0", // + " acmeCA-2.0", // + " ", // + " ",// + "" // + ); + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, httpCompletion, + httpsCompletion); + + serverXML = String.join(newLine, // + "", // + " ", // + " javaee-6.0", // + " acmeCA-2.0", // + " ", // + " ",// + "" // + ); + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, httpCompletion, + httpsCompletion); + + serverXML = String.join(newLine, // + "", // + " ", // + " javaee-6.0", // + " acmeCA-2.0", // + " ", // + " ",// + "" // + ); + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 0); + } + + // Tests the + // availability of multiple variable completion + @Test + public void testVariableCompletionItemWithMultipleVars() throws BadLocationException { + Map propsMap=new HashMap<>(); + propsMap.put("default.http.port","9080"); + propsMap.put("default.https.port","9443"); + propsMap.put("testVar","app-name.war"); + propsMap.put("testVar2","app-name2.war"); + Properties props = new Properties(); + props.putAll(propsMap); + + when(settingsService.getVariablesForServerXml(any())).thenReturn(props); + + String serverXML = String.join(newLine, + "", + " ", + " servlet", + " jakartaee-9.1", + " ", + " ", + " ", + " ", + "" + ); + + CompletionItem testVarCompletion = c("${testVar2}/${testVar}", "${testVar2}/${testVar}"); + CompletionItem testVar2Completion = c("${testVar2}/${testVar2}", "${testVar2}/${testVar2}"); + final int TOTAL_ITEMS = 2; // total number of available completion items + + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, testVarCompletion, + testVar2Completion); + + + serverXML = String.join(newLine, + "", + " ", + " servlet", + " jakartaee-9.1", + " ", + " ", + " ", + " ", + "" + ); + + testVarCompletion = c("${testVar2}/apps/${testVar}", "${testVar2}/apps/${testVar}"); + testVar2Completion = c("${testVar2}/apps/${testVar2}", "${testVar2}/apps/${testVar2}"); + + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, testVarCompletion, + testVar2Completion); + + /* for showing completion of second variable, variable should always prefix with ${ . Hence show no completion here*/ + serverXML = String.join(newLine, + "", + " ", + " servlet", + " jakartaee-9.1", + " ", + " ", + " ", + " ", + "" + ); + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 0); + } } From 1f8b79910f681034308123ceb94ee4e68e1150d8 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Mon, 9 Dec 2024 13:56:19 +0530 Subject: [PATCH 02/12] multiple variable diagnostic Signed-off-by: Arun Venmany --- .../src/test/java/LibertyWorkspaceIT.java | 6 +-- .../lemminx/LibertyDiagnosticParticipant.java | 2 +- .../io/openliberty/LibertyDiagnosticTest.java | 38 +++++++++++++++++-- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java b/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java index b2988a02..c32143ef 100644 --- a/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java +++ b/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java @@ -115,7 +115,7 @@ public void testInvalidVariableDiagnostic() { ); Diagnostic dup1 = new Diagnostic(); - dup1.setRange(r(5, 36, 5, 55)); + dup1.setRange(r(5, 34, 5, 56)); dup1.setCode("incorrect_variable"); dup1.setSource("liberty-lemminx"); dup1.setSeverity(DiagnosticSeverity.Error); @@ -123,7 +123,7 @@ public void testInvalidVariableDiagnostic() { dup1.setData("default.httpsl.port"); Diagnostic dup2 = new Diagnostic(); - dup2.setRange(r(7, 31, 7, 50)); + dup2.setRange(r(7, 29, 7, 51)); dup2.setCode("incorrect_variable"); dup2.setSource("liberty-lemminx"); dup2.setSeverity(DiagnosticSeverity.Error); @@ -156,7 +156,7 @@ public void testInvalidVariableDiagnosticWithCodeAction() throws IOException, Ba ); Diagnostic invalid1 = new Diagnostic(); - invalid1.setRange(r(7, 31, 7, 44)); + invalid1.setRange(r(7, 29, 7, 45)); invalid1.setCode("incorrect_variable"); invalid1.setMessage("ERROR: The variable \"default.https\" does not exist."); invalid1.setData("default.https"); 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 79887574..e388f1d9 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 @@ -129,7 +129,7 @@ private void validateVariables(DOMDocument domDocument, List diagnos for (VariableLoc variable : variables) { if (!variablesMap.containsKey(variable.getValue())) { String variableInDoc = String.format("${%s}", variable.getValue()); - Range range = XMLPositionUtility.createRange(variable.getStartLoc(),variable.getEndLoc(), + Range range = XMLPositionUtility.createRange(variable.getStartLoc()-2,variable.getEndLoc()+1, domDocument); String message = "ERROR: The variable \"" + variable.getValue() + "\" does not exist."; diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index 1486a954..5cf58e9f 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -910,7 +910,7 @@ public void testInvalidVariableDiagnostic() { props.putAll(propsMap); when(settingsService.getVariablesForServerXml(any())).thenReturn(props); Diagnostic dup1 = new Diagnostic(); - dup1.setRange(r(7, 31, 7, 49)); + dup1.setRange(r(7, 29, 7, 50)); dup1.setCode(LibertyDiagnosticParticipant.INCORRECT_VARIABLE_CODE); dup1.setSource("liberty-lemminx"); dup1.setSeverity(DiagnosticSeverity.Error); @@ -939,7 +939,7 @@ public void testInvalidVariableDiagnosticWithCodeAction() throws BadLocationExce props.putAll(propsMap); when(settingsService.getVariablesForServerXml(any())).thenReturn(props); Diagnostic invalid1 = new Diagnostic(); - invalid1.setRange(r(7, 31, 7, 44)); + invalid1.setRange(r(7, 29, 7, 45)); invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_VARIABLE_CODE); invalid1.setMessage("ERROR: The variable \"default.https\" does not exist."); invalid1.setData("default.https"); @@ -1011,7 +1011,7 @@ public void testInvalidVariableRepeatedDiagnostic() { props.putAll(propsMap); when(settingsService.getVariablesForServerXml(any())).thenReturn(props); Diagnostic dup1 = new Diagnostic(); - dup1.setRange(r(5, 36, 5, 54)); + dup1.setRange(r(5, 34, 5, 55)); dup1.setCode(LibertyDiagnosticParticipant.INCORRECT_VARIABLE_CODE); dup1.setSource("liberty-lemminx"); dup1.setSeverity(DiagnosticSeverity.Error); @@ -1019,7 +1019,7 @@ public void testInvalidVariableRepeatedDiagnostic() { dup1.setData("default.https.port"); Diagnostic dup2 = new Diagnostic(); - dup2.setRange(r(7, 31, 7, 49)); + dup2.setRange(r(7, 29, 7, 50)); dup2.setCode(LibertyDiagnosticParticipant.INCORRECT_VARIABLE_CODE); dup2.setSource("liberty-lemminx"); dup2.setSeverity(DiagnosticSeverity.Error); @@ -1028,4 +1028,34 @@ public void testInvalidVariableRepeatedDiagnostic() { XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, false, dup1, dup2); } + + @Test + public void testMultipleVariablesInSameAttributeDiagnostic() { + String serverXML = String.join(newLine, // + "", // + " ", // + " javaee-6.0", // + " acmeCA-2.0", // + " ", // + " ",// + " ", + "" // + ); + Map propsMap = new HashMap<>(); + propsMap.put("default.http.port", "9080"); + propsMap.put("default.https.port", "9443"); + propsMap.put("testVar2", "apps"); + Properties props = new Properties(); + props.putAll(propsMap); + when(settingsService.getVariablesForServerXml(any())).thenReturn(props); + Diagnostic dup1 = new Diagnostic(); + dup1.setRange(r(8, 63, 8, 74)); + dup1.setCode(LibertyDiagnosticParticipant.INCORRECT_VARIABLE_CODE); + dup1.setSource("liberty-lemminx"); + dup1.setSeverity(DiagnosticSeverity.Error); + dup1.setMessage("ERROR: The variable \"testVar1\" does not exist."); + dup1.setData("testVar1"); + XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, false, dup1); + } } \ No newline at end of file From a444e76d28263e57392f7f1925271530b33be724 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Mon, 9 Dec 2024 19:24:29 +0530 Subject: [PATCH 03/12] file watcher for config files alteration for variable re processing Signed-off-by: Arun Venmany --- .../langserver/lemminx/LibertyExtension.java | 29 ++-- .../lemminx/services/FileWatchService.java | 135 ++++++++++++++++++ .../lemminx/util/LibertyConstants.java | 4 + 3 files changed, 156 insertions(+), 12 deletions(-) create mode 100644 lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java index dc65d5e4..a43d4915 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java @@ -12,6 +12,7 @@ *******************************************************************************/ package io.openliberty.tools.langserver.lemminx; +import io.openliberty.tools.langserver.lemminx.services.FileWatchService; import io.openliberty.tools.langserver.lemminx.services.LibertyWorkspace; import org.eclipse.lemminx.services.extensions.IDocumentLinkParticipant; import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant; @@ -26,6 +27,8 @@ import org.eclipse.lsp4j.InitializeParams; import org.eclipse.lsp4j.WorkspaceFolder; +import java.io.File; +import java.nio.file.Path; import java.util.List; import java.util.logging.Logger; @@ -77,6 +80,19 @@ public void start(InitializeParams initializeParams, XMLExtensionsRegistry xmlEx } catch (Exception e) { throw new RuntimeException(e); } + + // for each workspace, a file alteration observer is added + for (LibertyWorkspace workspace : LibertyProjectsManager.getInstance().getLibertyWorkspaceFolders()) { + // checking for any changes in target folder + Path libertyPath = new File(workspace.getWorkspaceURI().getPath(), + "target").toPath(); + try { + FileWatchService.getInstance() + .addFileAlterationObserver(libertyPath.toString(), workspace); + } catch (Exception e) { + LOGGER.warning("unable to add file alteration observer for path " + libertyPath); + } + } } @Override @@ -90,6 +106,7 @@ public void stop(XMLExtensionsRegistry xmlExtensionsRegistry) { xmlExtensionsRegistry.unregisterHoverParticipant(hoverParticipant); xmlExtensionsRegistry.unregisterDiagnosticsParticipant(diagnosticsParticipant); xmlExtensionsRegistry.unregisterCodeActionParticipant(codeActionsParticipant); + FileWatchService.getInstance().cleanFileMonitors(); } // Do save is called on startup with a Settings update @@ -103,17 +120,5 @@ public void doSave(ISaveContext saveContext) { SettingsService.getInstance().updateLibertySettings(xmlSettings); LOGGER.info("Liberty XML settings updated"); } - if (saveContext.getType() == SaveContextType.DOCUMENT) { - try { - LibertyWorkspace workspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(saveContext.getUri()); - if (workspace != null) { - SettingsService.getInstance() - .populateVariablesForWorkspace(workspace); - } - } catch (Exception e) { - throw new RuntimeException(e); - } - LOGGER.info("Liberty XML variables updated"); - } } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java new file mode 100644 index 00000000..7334c7e1 --- /dev/null +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java @@ -0,0 +1,135 @@ +/******************************************************************************* + * Copyright (c) 2024 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.services; + +import io.openliberty.tools.langserver.lemminx.util.LibertyConstants; +import io.openliberty.tools.langserver.lemminx.util.LibertyUtils; +import org.apache.commons.io.monitor.FileAlterationListenerAdaptor; +import org.apache.commons.io.monitor.FileAlterationMonitor; +import org.apache.commons.io.monitor.FileAlterationObserver; + +import java.io.File; +import java.io.IOException; +import java.util.HashSet; +import java.util.Set; +import java.util.logging.Logger; + +public class FileWatchService { + + private final Set fileObservers = new HashSet<>(); + private final Set monitors = new HashSet<>(); + + private static final FileWatchService instance = new FileWatchService(); + + public static FileWatchService getInstance() { + return instance; + } + + private static final Logger LOGGER = Logger.getLogger(FileWatchService.class.getName()); + + private FileWatchService() { + } + + /** + * add observer for file changes + * + * @param parentPath parent location + * @param workspace workspace + */ + public void addFileAlterationObserver(String parentPath, LibertyWorkspace workspace) + throws Exception { + FileAlterationObserver observer = getFileAlterationObserver(parentPath, workspace); + observer.initialize(); + fileObservers.add(observer); + FileAlterationMonitor monitor = new FileAlterationMonitor(); + monitor.addObserver(observer); + monitor.start(); + monitors.add(monitor); + } + + private FileAlterationObserver getFileAlterationObserver(final String parentPath, LibertyWorkspace workspace) { + FileAlterationObserver observer = new FileAlterationObserver(parentPath); + addFileAlterationListener(observer, workspace); + return observer; + } + + private void addFileAlterationListener(FileAlterationObserver observer, LibertyWorkspace workspace) { + observer.addListener(new FileAlterationListenerAdaptor() { + @Override + public void onDirectoryCreate(File file) { + onAlteration(file, workspace); + } + + @Override + public void onDirectoryDelete(File file) { + onAlteration(file, workspace); + } + + @Override + public void onDirectoryChange(File file) { + onAlteration(file, workspace); + } + + @Override + public void onFileCreate(File file) { + onAlteration(file, workspace); + } + + @Override + public void onFileDelete(File file) { + onAlteration(file, workspace); + } + + @Override + public void onFileChange(File file) { + onAlteration(file, workspace); + } + + /** + * update variables on file alteration, if modified file is a config + * + * @param file changed file + * @param workspace current workspace + */ + private void onAlteration(File file, LibertyWorkspace workspace) { + boolean watchedFileChanged = LibertyConstants.filesToWatch.stream().anyMatch(fileName -> file.getName().contains(fileName)); + boolean isConfigXmlFile = false; + try { + isConfigXmlFile = LibertyUtils.isConfigXMLFile(file.getCanonicalPath()); + } catch (IOException e) { + LOGGER.warning("Liberty XML variables cannot be updated for file path %s with error %s" + .formatted(file.getPath(), e.getMessage())); + } + if (watchedFileChanged || isConfigXmlFile) { + SettingsService.getInstance().populateVariablesForWorkspace(workspace); + LOGGER.info("Liberty XML variables updated for workspace URI " + workspace.getWorkspaceString()); + } + } + }); + } + + /** + * clean all monitors for all workspaces + */ + public void cleanFileMonitors() { + fileObservers.clear(); + try { + for (FileAlterationMonitor monitor : monitors) { + monitor.stop(); + } + } catch (Exception e) { + LOGGER.warning("Issue while removing file monitors with error %s" + .formatted(e.getMessage())); + } + } +} diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyConstants.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyConstants.java index dcec096a..6be056ac 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyConstants.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyConstants.java @@ -12,8 +12,10 @@ *******************************************************************************/ package io.openliberty.tools.langserver.lemminx.util; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; public final class LibertyConstants { @@ -74,4 +76,6 @@ private LibertyConstants() { public static String changedFeatureNameDiagMessage="ERROR: The %s feature cannot be configured with the %s feature because they are two different versions of the same feature. " + "The feature name changed from %s to %s for Jakarta EE. Remove one of the features."; + + public static List filesToWatch= Arrays.asList(SERVER_XML,"server.env","bootstrap.properties"); } From 4d037ac1a1046b0e637b64b4241c14460b84c30a Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Tue, 17 Dec 2024 13:15:39 +0530 Subject: [PATCH 04/12] changes for review comments Signed-off-by: Arun Venmany --- .../langserver/lemminx/LibertyExtension.java | 15 ++++++++----- .../lemminx/services/FileWatchService.java | 21 +++++++++++-------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java index a43d4915..b0d107ce 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java @@ -29,6 +29,7 @@ import java.io.File; import java.nio.file.Path; +import java.util.Arrays; import java.util.List; import java.util.logging.Logger; @@ -83,14 +84,18 @@ public void start(InitializeParams initializeParams, XMLExtensionsRegistry xmlEx // for each workspace, a file alteration observer is added for (LibertyWorkspace workspace : LibertyProjectsManager.getInstance().getLibertyWorkspaceFolders()) { - // checking for any changes in target folder - Path libertyPath = new File(workspace.getWorkspaceURI().getPath(), - "target").toPath(); + // checking for any changes in wlp user folder for gradle and maven + Path libertyUsrGradlePath = new File(workspace.getWorkspaceURI().getPath(), + "target/liberty/wlp/usr").toPath(); + Path libertyUsrMavenPath = new File(workspace.getWorkspaceURI().getPath(), + "build/liberty/wlp/usr").toPath(); + List paths = Arrays.asList(libertyUsrMavenPath.toString(), libertyUsrGradlePath.toString()); try { FileWatchService.getInstance() - .addFileAlterationObserver(libertyPath.toString(), workspace); + .addFileAlterationObserver(workspace, paths); } catch (Exception e) { - LOGGER.warning("unable to add file alteration observer for path " + libertyPath); + LOGGER.warning("unable to add file alteration observer for paths " + paths + + " with error message " + e.getMessage()); } } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java index 7334c7e1..209da659 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.logging.Logger; @@ -43,18 +44,20 @@ private FileWatchService() { /** * add observer for file changes * - * @param parentPath parent location * @param workspace workspace + * @param watchLocations parent locations */ - public void addFileAlterationObserver(String parentPath, LibertyWorkspace workspace) + public void addFileAlterationObserver(LibertyWorkspace workspace, List watchLocations) throws Exception { - FileAlterationObserver observer = getFileAlterationObserver(parentPath, workspace); - observer.initialize(); - fileObservers.add(observer); - FileAlterationMonitor monitor = new FileAlterationMonitor(); - monitor.addObserver(observer); - monitor.start(); - monitors.add(monitor); + for (String location:watchLocations) { + FileAlterationObserver observer = getFileAlterationObserver(location, workspace); + observer.initialize(); + fileObservers.add(observer); + FileAlterationMonitor monitor = new FileAlterationMonitor(); + monitor.addObserver(observer); + monitor.start(); + monitors.add(monitor); + } } private FileAlterationObserver getFileAlterationObserver(final String parentPath, LibertyWorkspace workspace) { From b96280f4fc0f4096b0a62ae9c72ec4612d3c76d5 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Tue, 17 Dec 2024 18:17:30 +0530 Subject: [PATCH 05/12] changes for review comments Signed-off-by: Arun Venmany --- .../src/test/java/LibertyWorkspaceIT.java | 12 ++++++++++-- .../langserver/lemminx/LibertyHoverParticipant.java | 2 +- .../lemminx/codeactions/ReplaceVariable.java | 12 ++++++++---- .../java/io/openliberty/LibertyDiagnosticTest.java | 12 ++++++++++-- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java b/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java index c32143ef..af66120d 100644 --- a/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java +++ b/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java @@ -181,7 +181,15 @@ public void testInvalidVariableDiagnosticWithCodeAction() throws IOException, Ba .get(0).getLeft().getTextDocument() .setUri(serverXmlFile.toURI().toString()); } - - XMLAssert.testCodeActionsFor(serverXML, serverXmlFile.toURI().toString(), invalid1, codeActions.get(0)); + TextEdit texted = te(8, 9, + 8, 9, " "); + + CodeAction invalidCodeAction = ca(invalid1, texted); + invalidCodeAction.getEdit() + .getDocumentChanges() + .get(0).getLeft().getTextDocument() + .setUri(serverXmlFile.toURI().toString()); + codeActions.add(invalidCodeAction); + XMLAssert.testCodeActionsFor(serverXML, serverXmlFile.toURI().toString(), invalid1, codeActions.get(0),codeActions.get(1)); } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java index 612fcb4e..6e99c80f 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java @@ -59,7 +59,7 @@ public Hover onAttributeValue(IHoverRequest request, CancelChecker cancelChecker stringBuilder.append(String.format("%s = %s", variable.getValue(), variableMap.get(variable.getValue()))); } if (varIter.hasNext()) { - stringBuilder.append(System.lineSeparator()); + stringBuilder.append("
"); } } if (!stringBuilder.isEmpty()) { diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java index 237d1943..c6c2f638 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java @@ -19,8 +19,10 @@ import org.eclipse.lemminx.dom.DOMDocument; 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.Range; import org.eclipse.lsp4j.jsonrpc.CancelChecker; import java.util.List; @@ -64,10 +66,12 @@ public void doCodeAction(ICodeActionRequest request, List codeAction String variableInDoc = String.format("${%s}", nextVariable.getKey().toString()); codeActions.add(CodeActionFactory.replace(title, diagnostic.getRange(), variableInDoc, document.getTextDocument(), diagnostic)); } - /*for (Map.Entry nextVariable : existingVariables.entrySet()) { - String title = "Replace Variable with " + nextVariable.getKey() + " with value = " + nextVariable.getValue(); - codeActions.add(CodeActionFactory.replace(title, diagnostic.getRange(), nextVariable.getKey().toString(), document.getTextDocument(), diagnostic)); - }*/ + // use special code action to reload variable diagnostics + Range range = XMLPositionUtility.createRange(document.getDocumentElement().getEndTagCloseOffset(), document.getDocumentElement().getEndTagCloseOffset()+1, + document); + String title = "Reload Diagnostics"; + codeActions.add(CodeActionFactory.insert(title, range.getEnd(), " ", + document.getTextDocument(), diagnostic)); } } catch (Exception e) { // BadLocationException not expected diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index 5cf58e9f..262d4ecd 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -966,8 +966,16 @@ public void testInvalidVariableDiagnosticWithCodeAction() throws BadLocationExce .get(0).getLeft().getTextDocument() .setUri(serverXMLURI); } - - XMLAssert.testCodeActionsFor(serverXML, serverXMLURI, invalid1, codeActions.get(0)); + TextEdit texted = te(8, 9, + 8, 9, " "); + + CodeAction invalidCodeAction = ca(invalid1, texted); + invalidCodeAction.getEdit() + .getDocumentChanges() + .get(0).getLeft().getTextDocument() + .setUri(serverXMLURI); + codeActions.add(invalidCodeAction); + XMLAssert.testCodeActionsFor(serverXML, serverXMLURI, invalid1, codeActions.get(0),codeActions.get(1)); } From 264e38c3c5cda45bc123b0eea3ea0c609ccd8457 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Wed, 18 Dec 2024 09:49:33 +0530 Subject: [PATCH 06/12] correcting file watch path Signed-off-by: Arun Venmany --- .../tools/langserver/lemminx/LibertyExtension.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java index b0d107ce..fa50e550 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyExtension.java @@ -86,9 +86,9 @@ public void start(InitializeParams initializeParams, XMLExtensionsRegistry xmlEx for (LibertyWorkspace workspace : LibertyProjectsManager.getInstance().getLibertyWorkspaceFolders()) { // checking for any changes in wlp user folder for gradle and maven Path libertyUsrGradlePath = new File(workspace.getWorkspaceURI().getPath(), - "target/liberty/wlp/usr").toPath(); + "target").toPath(); Path libertyUsrMavenPath = new File(workspace.getWorkspaceURI().getPath(), - "build/liberty/wlp/usr").toPath(); + "build").toPath(); List paths = Arrays.asList(libertyUsrMavenPath.toString(), libertyUsrGradlePath.toString()); try { FileWatchService.getInstance() From 9a85c9198880b01d4a2b2042e2230cebec595a8c Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Wed, 18 Dec 2024 11:03:32 +0530 Subject: [PATCH 07/12] correcting ci commons snapshot version Signed-off-by: Arun Venmany --- lemminx-liberty/pom.xml | 2 +- .../langserver/lemminx/services/FileWatchService.java | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lemminx-liberty/pom.xml b/lemminx-liberty/pom.xml index c5ae7226..b29e8964 100644 --- a/lemminx-liberty/pom.xml +++ b/lemminx-liberty/pom.xml @@ -170,7 +170,7 @@ io.openliberty.tools ci.common - 1.8.36-SNAPSHOT + 1.8.36 junit diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java index 209da659..c4156b7c 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java @@ -14,12 +14,18 @@ import io.openliberty.tools.langserver.lemminx.util.LibertyConstants; import io.openliberty.tools.langserver.lemminx.util.LibertyUtils; +import org.apache.commons.io.IOCase; +import org.apache.commons.io.filefilter.FileFilterUtils; +import org.apache.commons.io.filefilter.IOFileFilter; +import org.apache.commons.io.filefilter.NotFileFilter; +import org.apache.commons.io.filefilter.SuffixFileFilter; import org.apache.commons.io.monitor.FileAlterationListenerAdaptor; import org.apache.commons.io.monitor.FileAlterationMonitor; import org.apache.commons.io.monitor.FileAlterationObserver; import java.io.File; import java.io.IOException; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -61,7 +67,9 @@ public void addFileAlterationObserver(LibertyWorkspace workspace, List w } private FileAlterationObserver getFileAlterationObserver(final String parentPath, LibertyWorkspace workspace) { - FileAlterationObserver observer = new FileAlterationObserver(parentPath); + IOFileFilter notFileFilter = FileFilterUtils.notFileFilter( + new SuffixFileFilter(Arrays.asList(".class", ".lst", ".txt"), IOCase.INSENSITIVE)); + FileAlterationObserver observer = new FileAlterationObserver(parentPath, notFileFilter);zs addFileAlterationListener(observer, workspace); return observer; } From 3ba15487f8b151d0c51fc3f532192f2828be0e43 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Wed, 18 Dec 2024 11:07:06 +0530 Subject: [PATCH 08/12] correcting ci commons snapshot version Signed-off-by: Arun Venmany --- .../tools/langserver/lemminx/services/FileWatchService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java index c4156b7c..e4ac93bb 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java @@ -69,7 +69,7 @@ public void addFileAlterationObserver(LibertyWorkspace workspace, List w private FileAlterationObserver getFileAlterationObserver(final String parentPath, LibertyWorkspace workspace) { IOFileFilter notFileFilter = FileFilterUtils.notFileFilter( new SuffixFileFilter(Arrays.asList(".class", ".lst", ".txt"), IOCase.INSENSITIVE)); - FileAlterationObserver observer = new FileAlterationObserver(parentPath, notFileFilter);zs + FileAlterationObserver observer = new FileAlterationObserver(parentPath, notFileFilter); addFileAlterationListener(observer, workspace); return observer; } From cda976751b4fd5ba5b8d883f7ddecbfe7c62dd43 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Wed, 18 Dec 2024 14:53:11 +0530 Subject: [PATCH 09/12] adding file filters Signed-off-by: Arun Venmany --- .../lemminx/LibertyCompletionParticipant.java | 2 +- .../langserver/lemminx/services/FileWatchService.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java index 57a2386c..51e3cd4b 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java @@ -60,7 +60,7 @@ public void onAttributeValue(String valuePrefix, ICompletionRequest request, ICo variableName = valuePrefix.substring(valuePrefix.lastIndexOf("${")) .replace("${", ""); if (variableName.contains("}")) { - variableName = variableName.replace("}", ""); + variableName = variableName.substring(variableName.lastIndexOf("}") + 1); } // existing variables replacePrefix = valuePrefix.substring(0, valuePrefix.lastIndexOf("${")); diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java index e4ac93bb..e1f87bc3 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java @@ -17,7 +17,7 @@ import org.apache.commons.io.IOCase; import org.apache.commons.io.filefilter.FileFilterUtils; import org.apache.commons.io.filefilter.IOFileFilter; -import org.apache.commons.io.filefilter.NotFileFilter; +import org.apache.commons.io.filefilter.NameFileFilter; import org.apache.commons.io.filefilter.SuffixFileFilter; import org.apache.commons.io.monitor.FileAlterationListenerAdaptor; import org.apache.commons.io.monitor.FileAlterationMonitor; @@ -68,7 +68,10 @@ public void addFileAlterationObserver(LibertyWorkspace workspace, List w private FileAlterationObserver getFileAlterationObserver(final String parentPath, LibertyWorkspace workspace) { IOFileFilter notFileFilter = FileFilterUtils.notFileFilter( - new SuffixFileFilter(Arrays.asList(".class", ".lst", ".txt"), IOCase.INSENSITIVE)); + new SuffixFileFilter(Arrays.asList(".class", ".lst", ".txt",".log",".manager",".libertyls",".sLock"), + IOCase.INSENSITIVE) + .or(new NameFileFilter("workarea")) + .or(new NameFileFilter("plugin-cfg.xml"))); FileAlterationObserver observer = new FileAlterationObserver(parentPath, notFileFilter); addFileAlterationListener(observer, workspace); return observer; @@ -78,17 +81,14 @@ private void addFileAlterationListener(FileAlterationObserver observer, LibertyW observer.addListener(new FileAlterationListenerAdaptor() { @Override public void onDirectoryCreate(File file) { - onAlteration(file, workspace); } @Override public void onDirectoryDelete(File file) { - onAlteration(file, workspace); } @Override public void onDirectoryChange(File file) { - onAlteration(file, workspace); } @Override From 255b45792fc0fe6f075bfaecf4a02925b8a5f011 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Thu, 19 Dec 2024 11:03:57 +0530 Subject: [PATCH 10/12] adding more file filters and changing completion logic for multiple vars Signed-off-by: Arun Venmany --- .../lemminx/LibertyCompletionParticipant.java | 34 ++++++++++++------- .../lemminx/services/FileWatchService.java | 9 +++-- .../io/openliberty/LibertyCompletionTest.java | 19 +++++++++++ 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java index 51e3cd4b..a6579b92 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java @@ -20,6 +20,7 @@ import java.util.Set; import java.util.stream.Collectors; +import io.openliberty.tools.langserver.lemminx.models.feature.VariableLoc; import org.eclipse.lemminx.commons.BadLocationException; import org.eclipse.lemminx.dom.DOMDocument; import org.eclipse.lemminx.dom.DOMElement; @@ -53,24 +54,33 @@ public void onAttributeValue(String valuePrefix, ICompletionRequest request, ICo Properties variableProps = SettingsService.getInstance() .getVariablesForServerXml(request.getXMLDocument() .getDocumentURI()); + //getting all existing variables in current completion prefix string + List variables = LibertyUtils.getVariablesFromTextContent(valuePrefix); // value prefix will contain all existing attribute value, till the cursor - String variableName = valuePrefix; - String replacePrefix; - if (valuePrefix.contains("${")) { - variableName = valuePrefix.substring(valuePrefix.lastIndexOf("${")) - .replace("${", ""); - if (variableName.contains("}")) { - variableName = variableName.substring(variableName.lastIndexOf("}") + 1); + String variableName = valuePrefix.replace("${", "") + .replace("}", ""); + StringBuilder prefixVariables = new StringBuilder(); + String replacePrefix = ""; + for (VariableLoc variableLoc : variables) { + prefixVariables.append(String.format("${%s}", variableLoc.getValue())); + } + if (!variables.isEmpty()) { + // find last variable last character and consider new variable completion from that location + VariableLoc lastVar = variables.get(variables.size() - 1); + variableName = valuePrefix.substring(lastVar.getEndLoc() + 1); + // if last variable is started with ${ + if (variableName.contains("${")) { + variableName = variableName.substring(variableName.lastIndexOf("${")).replace("${", "") + .replace("}", ""); + // add char between last variable and start of completion variable to prefix + replacePrefix = valuePrefix.substring(lastVar.getEndLoc() + 1, valuePrefix.lastIndexOf("${")); } - // existing variables - replacePrefix = valuePrefix.substring(0, valuePrefix.lastIndexOf("${")); - } else { - replacePrefix = ""; } + prefixVariables.append(replacePrefix); String finalVariableName = variableName; variableProps.entrySet().stream().filter(it -> it.getKey().toString().toLowerCase().contains(finalVariableName.toLowerCase())) .forEach(variableProp -> { - String varValue = String.format("%s${%s}", replacePrefix, variableProp.getKey()); + String varValue = String.format("%s${%s}", prefixVariables, variableProp.getKey()); Either edit = Either.forLeft(new TextEdit(request.getReplaceRange(), varValue)); diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java index e1f87bc3..b5b0df6b 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java @@ -67,11 +67,14 @@ public void addFileAlterationObserver(LibertyWorkspace workspace, List w } private FileAlterationObserver getFileAlterationObserver(final String parentPath, LibertyWorkspace workspace) { + //ignore files/directories with below suffixes and names IOFileFilter notFileFilter = FileFilterUtils.notFileFilter( - new SuffixFileFilter(Arrays.asList(".class", ".lst", ".txt",".log",".manager",".libertyls",".sLock"), + new SuffixFileFilter(Arrays.asList(".class", ".lst", ".txt", ".log", ".manager", ".libertyls", + ".sLock", ".jar", ".war", ".ear",".mf"), IOCase.INSENSITIVE) - .or(new NameFileFilter("workarea")) - .or(new NameFileFilter("plugin-cfg.xml"))); + .or(new NameFileFilter(Arrays.asList("workarea", "plugin-cfg.xml", "libs", "tmp", "classes","bin", + "generated-sources", "generated-test-sources", "invoker-reports", + "it", "maven-status", "surefire-reports", "test-classes"), IOCase.INSENSITIVE))); FileAlterationObserver observer = new FileAlterationObserver(parentPath, notFileFilter); addFileAlterationListener(observer, workspace); return observer; diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java index 9107e8ea..e0dc06a3 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java @@ -504,5 +504,24 @@ public void testVariableCompletionItemWithMultipleVars() throws BadLocationExcep "" ); XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 0); + + // in this case, completion will be shown for second variable as there is no special characters in the prefix string + serverXML = String.join(newLine, + "", + " ", + " servlet", + " jakartaee-9.1", + " ", + " ", + " ", + " ", + "" + ); + + testVarCompletion = c("${testVar2}${testVar}", "${testVar2}${testVar}"); + testVar2Completion = c("${testVar2}${testVar2}", "${testVar2}${testVar2}"); + + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, testVarCompletion, + testVar2Completion); } } From b1cf55a6a80022bca55dc09a72788c3bf129cfa3 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Fri, 20 Dec 2024 10:50:47 +0530 Subject: [PATCH 11/12] adding more file filters and changing completion logic for multiple vars Signed-off-by: Arun Venmany --- .../src/test/java/LibertyWorkspaceIT.java | 11 +--- .../lemminx/LibertyCompletionParticipant.java | 50 +++++++++++-------- .../lemminx/LibertyDiagnosticParticipant.java | 4 +- .../lemminx/codeactions/ReplaceVariable.java | 6 --- .../lemminx/services/FileWatchService.java | 5 +- .../langserver/lemminx/util/LibertyUtils.java | 10 ++-- .../io/openliberty/LibertyCompletionTest.java | 30 +++++++++-- .../io/openliberty/LibertyDiagnosticTest.java | 11 +--- 8 files changed, 71 insertions(+), 56 deletions(-) diff --git a/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java b/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java index af66120d..783cce57 100644 --- a/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java +++ b/lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java @@ -181,15 +181,6 @@ public void testInvalidVariableDiagnosticWithCodeAction() throws IOException, Ba .get(0).getLeft().getTextDocument() .setUri(serverXmlFile.toURI().toString()); } - TextEdit texted = te(8, 9, - 8, 9, " "); - - CodeAction invalidCodeAction = ca(invalid1, texted); - invalidCodeAction.getEdit() - .getDocumentChanges() - .get(0).getLeft().getTextDocument() - .setUri(serverXmlFile.toURI().toString()); - codeActions.add(invalidCodeAction); - XMLAssert.testCodeActionsFor(serverXML, serverXmlFile.toURI().toString(), invalid1, codeActions.get(0),codeActions.get(1)); + XMLAssert.testCodeActionsFor(serverXML, serverXmlFile.toURI().toString(), invalid1, codeActions.get(0)); } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java index a6579b92..c91bebe6 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java @@ -56,32 +56,42 @@ public void onAttributeValue(String valuePrefix, ICompletionRequest request, ICo .getDocumentURI()); //getting all existing variables in current completion prefix string List variables = LibertyUtils.getVariablesFromTextContent(valuePrefix); - // value prefix will contain all existing attribute value, till the cursor - String variableName = valuePrefix.replace("${", "") - .replace("}", ""); - StringBuilder prefixVariables = new StringBuilder(); - String replacePrefix = ""; - for (VariableLoc variableLoc : variables) { - prefixVariables.append(String.format("${%s}", variableLoc.getValue())); - } + String variablePrefix = ""; + String completionPrefix; + if (!variables.isEmpty()) { + // for multiple variables // find last variable last character and consider new variable completion from that location VariableLoc lastVar = variables.get(variables.size() - 1); - variableName = valuePrefix.substring(lastVar.getEndLoc() + 1); - // if last variable is started with ${ - if (variableName.contains("${")) { - variableName = variableName.substring(variableName.lastIndexOf("${")).replace("${", "") - .replace("}", ""); - // add char between last variable and start of completion variable to prefix - replacePrefix = valuePrefix.substring(lastVar.getEndLoc() + 1, valuePrefix.lastIndexOf("${")); + // if last index of an existing variable is less than lastIndex of ${, + // this means that completion variable is started with ${ + if (valuePrefix.lastIndexOf("${") > lastVar.getEndLoc()) { + // get whatever is after last ${ as variablePrefix for Completion + variablePrefix = valuePrefix.substring(valuePrefix.lastIndexOf("${") + 2); + completionPrefix = valuePrefix.substring(0, valuePrefix.lastIndexOf("${")); + } else { + variablePrefix = valuePrefix.substring(lastVar.getEndLoc() + 1); + // add char between last variable and start of completion variable to completionPrefix + completionPrefix = valuePrefix.substring(0,lastVar.getEndLoc()+1); + } + } else { + // for single variable,check ${ is specified + if (valuePrefix.contains("${")) { + // extract variable name with whatever is after ${ + variablePrefix = valuePrefix.substring(valuePrefix.lastIndexOf("${") + 2); + // extract completionPrefix with whatever is before ${ + completionPrefix = valuePrefix.substring(0, valuePrefix.lastIndexOf("${")); + } else { + // if no ${ specified, take all data as completion variable + variablePrefix = valuePrefix; + completionPrefix = ""; } } - prefixVariables.append(replacePrefix); - String finalVariableName = variableName; - variableProps.entrySet().stream().filter(it -> it.getKey().toString().toLowerCase().contains(finalVariableName.toLowerCase())) + String finalVariableName = variablePrefix.replace("${","").replace("}",""); + variableProps.entrySet().stream().filter(it -> it.getKey().toString().toLowerCase() + .contains(finalVariableName.toLowerCase())) .forEach(variableProp -> { - String varValue = String.format("%s${%s}", prefixVariables, variableProp.getKey()); - + String varValue = String.format("%s${%s}", completionPrefix, variableProp.getKey()); Either edit = Either.forLeft(new TextEdit(request.getReplaceRange(), varValue)); CompletionItem completionItem = new CompletionItem(); 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 e388f1d9..fa6d4943 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 @@ -129,7 +129,9 @@ private void validateVariables(DOMDocument domDocument, List diagnos for (VariableLoc variable : variables) { if (!variablesMap.containsKey(variable.getValue())) { String variableInDoc = String.format("${%s}", variable.getValue()); - Range range = XMLPositionUtility.createRange(variable.getStartLoc()-2,variable.getEndLoc()+1, + //range is used in ReplaceVariable to provide quick fix. + // we just need the variable value range here as ${} is added in replace variable message + Range range = XMLPositionUtility.createRange(variable.getStartLoc() - 2, variable.getEndLoc() + 1, domDocument); String message = "ERROR: The variable \"" + variable.getValue() + "\" does not exist."; diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java index c6c2f638..2f557f42 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java @@ -66,12 +66,6 @@ public void doCodeAction(ICodeActionRequest request, List codeAction String variableInDoc = String.format("${%s}", nextVariable.getKey().toString()); codeActions.add(CodeActionFactory.replace(title, diagnostic.getRange(), variableInDoc, document.getTextDocument(), diagnostic)); } - // use special code action to reload variable diagnostics - Range range = XMLPositionUtility.createRange(document.getDocumentElement().getEndTagCloseOffset(), document.getDocumentElement().getEndTagCloseOffset()+1, - document); - String title = "Reload Diagnostics"; - codeActions.add(CodeActionFactory.insert(title, range.getEnd(), " ", - document.getTextDocument(), diagnostic)); } } catch (Exception e) { // BadLocationException not expected diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java index b5b0df6b..a6d92497 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FileWatchService.java @@ -72,7 +72,7 @@ private FileAlterationObserver getFileAlterationObserver(final String parentPath new SuffixFileFilter(Arrays.asList(".class", ".lst", ".txt", ".log", ".manager", ".libertyls", ".sLock", ".jar", ".war", ".ear",".mf"), IOCase.INSENSITIVE) - .or(new NameFileFilter(Arrays.asList("workarea", "plugin-cfg.xml", "libs", "tmp", "classes","bin", + .or(new NameFileFilter(Arrays.asList("plugin-cfg.xml", "libs", "tmp", "classes", "generated-sources", "generated-test-sources", "invoker-reports", "it", "maven-status", "surefire-reports", "test-classes"), IOCase.INSENSITIVE))); FileAlterationObserver observer = new FileAlterationObserver(parentPath, notFileFilter); @@ -84,14 +84,17 @@ private void addFileAlterationListener(FileAlterationObserver observer, LibertyW observer.addListener(new FileAlterationListenerAdaptor() { @Override public void onDirectoryCreate(File file) { + // not required } @Override public void onDirectoryDelete(File file) { + // not required } @Override public void onDirectoryChange(File file) { + // not required } @Override diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java index b685ab01..ff5a7f74 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java @@ -54,9 +54,9 @@ public class LibertyUtils { private static Thread thread; //considering ${var} pattern for variable. do we have other representation for variable? - private static final String regex = "\\$\\{(.*?)\\}"; + private static final String VAR_PATTERN_REGEX = "\\$\\{(.*?)\\}"; // Compile the Regex. - private static final Pattern p = Pattern.compile(regex); + private static final Pattern VAR_PATTERN = Pattern.compile(VAR_PATTERN_REGEX); private LibertyUtils() { } @@ -606,15 +606,17 @@ public static File getFileFromLibertyPluginXml(Path pluginConfigFilePath, String /** * read variables from text content + * if the text content is ${testVar1}/${testVar2}/app ,response will contain 2 variable values + * [{testVar1,startIndex,endIndex},{testVar2,startIndex,endIndex}] * @param docContent text content - * @return list of variables + * @return list of variables. */ public static List getVariablesFromTextContent(String docContent) { List variables = new ArrayList<>(); // Find match between given string // and regular expression // using Pattern.matcher() - Matcher m = p.matcher(docContent); + Matcher m = VAR_PATTERN.matcher(docContent); // Get the subsequence // using find() method while (m.find()) { diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java index e0dc06a3..a98d4a0f 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java @@ -389,6 +389,8 @@ public void testVariableCompletionItemWithVariablePatterns() throws BadLocationE propsMap.put("default.https.port","9443"); propsMap.put("testVar","false"); propsMap.put("testVar2","true"); + propsMap.put("appName","app-name.war"); + propsMap.put("appName2","app-name-new.war"); Properties props = new Properties(); props.putAll(propsMap); @@ -437,6 +439,25 @@ public void testVariableCompletionItemWithVariablePatterns() throws BadLocationE "" // ); XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 0); + + // single variable completion with prefix + serverXML = String.join(newLine, + "", + " ", + " servlet", + " jakartaee-9.1", + " ", + " ", + " ", + " ", + "" + ); + + CompletionItem testVarCompletion = c("apps/${appName}", "apps/${appName}"); + CompletionItem testVar2Completion = c("apps/${appName2}", "apps/${appName2}"); + + XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, testVarCompletion, + testVar2Completion); } // Tests the @@ -446,6 +467,7 @@ public void testVariableCompletionItemWithMultipleVars() throws BadLocationExcep Map propsMap=new HashMap<>(); propsMap.put("default.http.port","9080"); propsMap.put("default.https.port","9443"); + propsMap.put("appLocation","root"); propsMap.put("testVar","app-name.war"); propsMap.put("testVar2","app-name2.war"); Properties props = new Properties(); @@ -479,19 +501,19 @@ public void testVariableCompletionItemWithMultipleVars() throws BadLocationExcep " servlet", " jakartaee-9.1", " ", - " ", + " ", " ", " ", "" ); - testVarCompletion = c("${testVar2}/apps/${testVar}", "${testVar2}/apps/${testVar}"); - testVar2Completion = c("${testVar2}/apps/${testVar2}", "${testVar2}/apps/${testVar2}"); + testVarCompletion = c("${testVar2}/apps/${appLocation}/${testVar}", "${testVar2}/apps/${appLocation}/${testVar}"); + testVar2Completion = c("${testVar2}/apps/${appLocation}/${testVar2}", "${testVar2}/apps/${appLocation}/${testVar2}"); XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, testVarCompletion, testVar2Completion); - /* for showing completion of second variable, variable should always prefix with ${ . Hence show no completion here*/ + // for showing completion of second variable, variable should always prefix with ${ . Hence show no completion here serverXML = String.join(newLine, "", " ", diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index 262d4ecd..fa6bf59b 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -966,16 +966,7 @@ public void testInvalidVariableDiagnosticWithCodeAction() throws BadLocationExce .get(0).getLeft().getTextDocument() .setUri(serverXMLURI); } - TextEdit texted = te(8, 9, - 8, 9, " "); - - CodeAction invalidCodeAction = ca(invalid1, texted); - invalidCodeAction.getEdit() - .getDocumentChanges() - .get(0).getLeft().getTextDocument() - .setUri(serverXMLURI); - codeActions.add(invalidCodeAction); - XMLAssert.testCodeActionsFor(serverXML, serverXMLURI, invalid1, codeActions.get(0),codeActions.get(1)); + XMLAssert.testCodeActionsFor(serverXML, serverXMLURI, invalid1, codeActions.get(0)); } From c3b546d8fdf79637665274b5a1182858cd8117b1 Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Tue, 24 Dec 2024 05:02:56 +0530 Subject: [PATCH 12/12] updating one test based on review comments Signed-off-by: Arun Venmany --- .../src/test/java/io/openliberty/LibertyCompletionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java index a98d4a0f..fe1024af 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java @@ -501,7 +501,7 @@ public void testVariableCompletionItemWithMultipleVars() throws BadLocationExcep " servlet", " jakartaee-9.1", " ", - " ", + " ", " ", " ", ""