changes based on review comments

Signed-off-by: Arun Venmany <[email protected]>
OpenLiberty · Feb 19, 2025 · 775ea6a · 775ea6a
1 parent 30977f4
commit 775ea6a
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 42 deletions.
diff --git a/...y/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java b/...y/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java
@@ -56,6 +56,7 @@ public void onAttributeValue(String valuePrefix, ICompletionRequest request, ICo
             return;
         Properties variableProps = SettingsService.getInstance()
                 .getVariablesForServerXml(request.getXMLDocument());
+        LibertyUtils.checkAndAddNewVariables(request.getXMLDocument(), variableProps);
         //getting all existing variables in current completion prefix string
         List<VariableLoc> variables = LibertyUtils.getVariablesFromTextContent(valuePrefix);
         String variablePrefix = "";

diff --git a/...y/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/...y/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java
@@ -126,6 +126,7 @@ private void validateVariables(DOMDocument domDocument, List<Diagnostic> diagnos
             diagnosticsList.add(diag);
             return;
         }
+        LibertyUtils.checkAndAddNewVariables(domDocument, variablesMap);
         validateVariableExists(domDocument, diagnosticsList, variables, variablesMap);
         validateVariableDataTypeValues(domDocument,diagnosticsList,variablesMap);
     }

diff --git a/...iberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java b/...iberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java
@@ -50,6 +50,7 @@ public Hover onAttributeValue(IHoverRequest request, CancelChecker cancelChecker
         List<VariableLoc> variables = LibertyUtils.getVariablesFromTextContent(request.getNode().getTextContent());
         Properties variableMap = SettingsService.getInstance()
                 .getVariablesForServerXml(request.getXMLDocument());
+        LibertyUtils.checkAndAddNewVariables(request.getXMLDocument(), variableMap);
         StringBuilder stringBuilder = new StringBuilder();
         Iterator<VariableLoc> varIter = variables.iterator();
         while (varIter.hasNext()) {

diff --git a/...ty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java b/...ty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceVariable.java
@@ -54,6 +54,7 @@ public void doCodeAction(ICodeActionRequest request, List<CodeAction> codeAction
 
             if (replaceVariable) {
                 Properties existingVariables = SettingsService.getInstance().getVariablesForServerXml(document);
+                LibertyUtils.checkAndAddNewVariables(document, existingVariables);
                 // filter with entered word -> may not be required
                 String finalInvalidVariable = invalidVariable;
                 Set<Map.Entry<Object, Object>> filteredVariables = existingVariables

diff --git a/...berty/src/main/java/io/openliberty/tools/langserver/lemminx/services/SettingsService.java b/...berty/src/main/java/io/openliberty/tools/langserver/lemminx/services/SettingsService.java
@@ -13,21 +13,17 @@
 package io.openliberty.tools.langserver.lemminx.services;
 
 import io.openliberty.tools.common.plugins.config.ServerConfigDocument;
-import io.openliberty.tools.common.plugins.util.VariableUtility;
 import io.openliberty.tools.langserver.lemminx.util.CommonLogger;
 import io.openliberty.tools.langserver.lemminx.util.LibertyUtils;
 import org.eclipse.lemminx.dom.DOMDocument;
 import org.eclipse.lemminx.utils.JSONUtility;
 import io.openliberty.tools.langserver.lemminx.models.settings.*;
 
-import javax.xml.xpath.XPathExpressionException;
 import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.logging.Logger;
@@ -146,44 +142,6 @@ public Properties getVariablesForServerXml(DOMDocument document) {
         } else {
             LOGGER.warning("Could not find variable mapping for workspace URI %s. Variable resolution cannot be performed.".formatted(workspace.getWorkspaceString()));
         }
-        if (!variableProps.isEmpty()) {
-            checkAndAddNewVariables(document, variableProps);
-        }
         return variableProps;
     }
-
-    /**
-     * Add new variables to variableProps
-     *
-     * @param document      xml document
-     * @param variableProps current variable properties map
-     */
-    private static void checkAndAddNewVariables(DOMDocument document, Properties variableProps) {
-        List<Properties> existingVars = new ArrayList<>();
-        try {
-            existingVars = VariableUtility.parseVariables(document, false, false, true);
-        } catch (XPathExpressionException e) {
-            LOGGER.warning("unable to parse variables for %s. Error message is %s ".formatted(document.getDocumentURI(), e.getMessage()));
-            return;
-        }
-        // a dirty check, verifies whether all variables in server.xml is present in variable map
-        // if not, we consider this variable is added recently with code action or manually
-        Map<String, String> additionalVarMap = new HashMap<>();
-        Map<String, String> combined = new HashMap<>();
-        //put defaultValue first
-        for (final String name : existingVars.get(1).stringPropertyNames()) {
-            additionalVarMap.put(name, existingVars.get(1).getProperty(name));
-        }
-        for (final String name : existingVars.get(0).stringPropertyNames()) {
-            additionalVarMap.put(name, existingVars.get(0).getProperty(name));
-        }
-        for (Map.Entry<String, String> entry : additionalVarMap.entrySet()) {
-            if (!variableProps.containsKey(entry.getKey())) {
-                combined.put(entry.getKey(), entry.getValue());
-            }
-        }
-        if (!combined.isEmpty()) {
-            variableProps.putAll(combined);
-        }
-    }
 }
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
@@ -23,14 +23,18 @@
 import java.nio.file.WatchKey;
 import java.nio.file.WatchService;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
+import io.openliberty.tools.common.plugins.util.VariableUtility;
 import io.openliberty.tools.langserver.lemminx.models.feature.VariableLoc;
 import org.eclipse.lemminx.dom.DOMDocument;
 
@@ -42,6 +46,8 @@
 import io.openliberty.tools.langserver.lemminx.services.LibertyWorkspace;
 import io.openliberty.tools.langserver.lemminx.services.SettingsService;
 
+import javax.xml.xpath.XPathExpressionException;
+
 public class LibertyUtils {
 
     private static final Logger LOGGER = Logger.getLogger(LibertyUtils.class.getName());
@@ -641,4 +647,42 @@ public static boolean containsEachOther(String string, String otherString, boole
         }
         return string.contains(otherString) || otherString.contains(string);
     }
+
+    /**
+     * Add new variables to variableProps
+     * Used to update local variable map with latest data
+     * Checks the xml document for any new variables added,
+     * if any new variable is found, its added to local variable map
+     *
+     * @param document      xml document
+     * @param variableProps current variable properties map
+     */
+    public static void checkAndAddNewVariables(DOMDocument document, Properties variableProps) {
+        List<Properties> existingVars;
+        try {
+            existingVars = VariableUtility.parseVariables(document, false, false, true);
+        } catch (XPathExpressionException e) {
+            LOGGER.warning("unable to parse variables for %s. Error message is %s ".formatted(document.getDocumentURI(), e.getMessage()));
+            return;
+        }
+        // a dirty check, verifies whether all variables in server.xml is present in variable map
+        // if not, we consider this variable is added recently with code action or manually
+        Map<String, String> additionalVarMap = new HashMap<>();
+        Map<String, String> combined = new HashMap<>();
+        //put defaultValue first
+        for (final String name : existingVars.get(1).stringPropertyNames()) {
+            additionalVarMap.put(name, existingVars.get(1).getProperty(name));
+        }
+        for (final String name : existingVars.get(0).stringPropertyNames()) {
+            additionalVarMap.put(name, existingVars.get(0).getProperty(name));
+        }
+        for (Map.Entry<String, String> entry : additionalVarMap.entrySet()) {
+            if (!variableProps.containsKey(entry.getKey())) {
+                combined.put(entry.getKey(), entry.getValue());
+            }
+        }
+        if (!combined.isEmpty()) {
+            variableProps.putAll(combined);
+        }
+    }
 }