Rework how Liberty config xml files are determined

lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/LibertyProjectsManager.java

@@ -71,7 +71,7 @@ public void setWorkspaceFolders(List<WorkspaceFolder> workspaceFolders) {
 
             try {
                 serverXmlFiles = LibertyUtils.getXmlFilesWithServerRootInDirectory(workspacePath);
-            } catch (IOException e) {
+            } catch (Exception e) {
                 LOGGER.warning("Received exception while searching for xml files with a <server> root element in: " + workspacePath + ": " + e.getMessage());
             }
 
@@ -80,6 +80,7 @@ public void setWorkspaceFolders(List<WorkspaceFolder> workspaceFolders) {
                 LibertyWorkspace libertyWorkspace = new LibertyWorkspace(normalizedUriString);
                 this.libertyWorkspaceFolders.put(normalizedUriString, libertyWorkspace);
             } else {
+                LOGGER.info("Checking Liberty workspace for sub-modules: " + normalizedUriString);
                 boolean addedSubModule = false;
 
                 List<Path> childrenDirs = null;
@@ -95,9 +96,11 @@ public void setWorkspaceFolders(List<WorkspaceFolder> workspaceFolders) {
                     for (Path nextChildDir : childrenDirs) {
                         lastChildDirPath = nextChildDir.toUri().toString().replace("///", "/");
                         if (nextChildDir.equals(workspacePath)) {
+                            LOGGER.info("Skipping parent module: " + lastChildDirPath);
                             continue; // skip parent module
                         } else if (this.libertyWorkspaceFolders.containsKey(lastChildDirPath)) {
                             // this sub-module was already added but we still don't want to add the parent module
+                            LOGGER.info("Skipping already added sub-module: " + lastChildDirPath);
                             addedSubModule = true; 
                             continue;
                         }
@@ -112,7 +115,7 @@ public void setWorkspaceFolders(List<WorkspaceFolder> workspaceFolders) {
                             }
                         }
                     }
-                } catch (IOException e) {
+                } catch (Exception e) {
                     LOGGER.warning("Received exception while processing workspace folder: " + lastChildDirPath);
                 }
 

lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java

@@ -27,6 +27,8 @@
 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 org.eclipse.lemminx.dom.DOMDocument;
@@ -42,37 +44,63 @@
 public class LibertyUtils {
 
     private static final Logger LOGGER = Logger.getLogger(LibertyUtils.class.getName());
+    private static final String INCLUDE_PATTERN_REGEX = ".*/(?:usr/shared/config/.+|configDropins/(?:defaults|overrides)/.+|server\\.xml)$";
+    private static final Pattern INCLUDE_PATTERN = Pattern.compile(INCLUDE_PATTERN_REGEX);
+
+    private static final String EXCLUDE_PATTERN_REGEX = ".*/(?:target(?!/it/)|build)/.+$";
+    private static final Pattern EXCLUDE_PATTERN = Pattern.compile(EXCLUDE_PATTERN_REGEX);
 
     private static Thread thread;
 
     private LibertyUtils() {
     }
 
-    public static boolean isServerXMLFile(String filePath) {
-        return filePath.endsWith("/" + LibertyConstants.SERVER_XML);
-    }
+    /*
+     * Check the filePath to see if it is a Liberty config file. The following qualify as a Liberty config file:
+     * - an XML file in the /usr/shared/config/, /configDropins/overrides/, or /configDropins/defaults/ directories
+     * - an XML file in any directory ending with /server.xml
+     * - an XML file with a <server> root element
+     * 
+     * If the rootPath is null, all XML files that do not fall into category 1 or 2 will be checked for a <server> root element.
+     * If the rootPath is not null, only XML files that that do not fall into category 1 or 2 and are not located in a target/build 
+     * directory will be checked for a <server> root element. The rootPath is non-null when called from getXmlFilesWithServerRootInDirectory method.
+     * 
+     * @param rootPath - String path of root directory for the filePath - only consider the portion of the filePath after the rootPath
+     * @param filePath - String path of the XML file to check
+     * @return boolean - true if the XML file is a Liberty config file, false otherwise
+     */
+    public static boolean isConfigXMLFile(String rootPath, String filePath) {
+        String pathToCheck = rootPath == null ? filePath : filePath.substring(rootPath.length());
 
-    public static boolean isServerXMLFile(DOMDocument file) {
-        return file.getDocumentURI().endsWith("/" + LibertyConstants.SERVER_XML);
-    }
+        if (File.separator.equals("\\")) {
+            pathToCheck = pathToCheck.replace("\\", "/");
+        }
 
-    public static boolean isConfigDirFile(String filePath) {
-        return filePath.contains(LibertyConstants.WLP_USER_CONFIG_DIR) ||
-                filePath.contains(LibertyConstants.SERVER_CONFIG_DROPINS_DEFAULTS) ||
-                filePath.contains(LibertyConstants.SERVER_CONFIG_DROPINS_OVERRIDES);
-    }
+        // if path contains one of the pre-defined Liberty config dirs or ends with /server.xml, 
+        // just return true without checking for server root
+        Matcher m = INCLUDE_PATTERN.matcher(pathToCheck);
+        if (m.find()) {
+            return true;
+        }
 
-    public static boolean isConfigXMLFile(String filePath) {
-        if (File.separator.equals("\\")) {
-            filePath = filePath.replace("\\", "/");
+        if (rootPath != null) {
+            // if path contains target/build dir (except for target/it for test purposes), just return false
+            m = EXCLUDE_PATTERN.matcher(pathToCheck);
+            if (m.find()) {
+                return false;
+            }
         }
 
-        return isServerXMLFile(filePath) || isConfigDirFile(filePath) || 
-                XmlReader.hasServerRoot(filePath);
+        // need to check if file has a server root element
+        return XmlReader.hasServerRoot(filePath);
+    }
+
+    public static boolean isConfigXMLFile(String filePath) {
+        return isConfigXMLFile(null, filePath);
     }
 
     public static boolean isConfigXMLFile(DOMDocument file) {
-        return isConfigXMLFile(file.getDocumentURI());
+        return isConfigXMLFile(null, file.getDocumentURI());
     }
 
     // Convenience methods
@@ -124,10 +152,11 @@ public static boolean containsFileStartingWithRootPath(Path rootDir, List<Path>
      */
     public static List<Path> getXmlFilesWithServerRootInDirectory(Path dir) throws IOException {
         List<Path> serverRootXmlFiles = new ArrayList<Path>();
+        String rootPath = dir.toString();
 
         List<Path> xmlFiles = findFilesEndsWithInDirectory(dir, ".xml");
         for (Path nextXmlFile : xmlFiles) {
-            if (XmlReader.hasServerRoot(nextXmlFile)) {
+            if (isConfigXMLFile(rootPath, nextXmlFile.toString())) {
                 serverRootXmlFiles.add(nextXmlFile);
             }
         }       

lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/XmlReader.java

@@ -51,31 +51,42 @@ public static boolean hasServerRoot(Path filePath) {
     }
 
     public static boolean hasServerRoot(File xmlFile) {
-        XMLEventReader reader = null;
+        if (!xmlFile.exists() || xmlFile.length() == 0) {
+            return false;
+        }
 
         try {
-            if (!xmlFile.exists() || xmlFile.length() == 0) {
-                return false;
-            }
-
             XMLInputFactory factory = XMLInputFactory.newInstance();
-            reader = factory.createXMLEventReader(new FileInputStream(xmlFile));
-            if (reader.hasNext()) {
-                XMLEvent firstTag = reader.nextTag(); // first start/end element
-                reader.close();
-                return isServerElement(firstTag);
+            try {
+                factory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE);
+                factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE);
+                factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE);
+            } catch (Exception e) {
+                LOGGER.warning("Could not set properties on XMLInputFactory.");
             }
-        } catch (FileNotFoundException e) {
-            LOGGER.severe("Unable to access file "+ xmlFile.getAbsolutePath());
-        } catch (XMLStreamException e) {
-            LOGGER.severe("Error received trying to read XML file: " + xmlFile.getAbsolutePath());
-        } finally {
-            if (reader != null) {
-                try {
-                    reader.close();
-                } catch (Exception ignored) {   
+
+            XMLEventReader reader = null;
+
+            try (FileInputStream fis = new FileInputStream(xmlFile)) {
+                reader = factory.createXMLEventReader(fis);
+                while (reader.hasNext()) {
+                    XMLEvent nextEvent = reader.nextEvent();
+                    if (nextEvent.isStartElement()) {
+                        return isServerElement(nextEvent);
+                    }
                 }
-            }
+            } catch (XMLStreamException | FileNotFoundException e) {
+                LOGGER.severe("Error received trying to read XML file: " + xmlFile.getAbsolutePath()); 
+            } finally {
+                if (reader != null) {
+                    try {
+                        reader.close();
+                    } catch (Exception ignored) {   
+                    }
+                }
+            }            
+        } catch (Exception e) {
+            LOGGER.severe("Unable to access XML file "+ xmlFile.getAbsolutePath());
         }
 
         return false;

lemminx-liberty/src/test/java/io/openliberty/LibertyFeatureTest.java

@@ -24,12 +24,13 @@ public class LibertyFeatureTest {
     @Test
     public void getInstalledFeaturesListTest() throws JAXBException {
         FeatureService fs = FeatureService.getInstance();
-        File srcResourcesDir = new File("src/test/resources");
-        File featureListFile = new File(srcResourcesDir, "featurelist-ol-23.0.0.1-beta.xml");
+        File srcResourcesDir = new File("src/test/resources/sample");
+        File featureListFile = new File(srcResourcesDir.getParentFile(), "featurelist-ol-23.0.0.1-beta.xml");
 
         // LibertyWorkspace must be initialized
         List<WorkspaceFolder> initList = new ArrayList<WorkspaceFolder>();
         initList.add(new WorkspaceFolder(srcResourcesDir.toURI().toString()));
+        LibertyProjectsManager.getInstance().cleanInstance();
         LibertyProjectsManager.getInstance().setWorkspaceFolders(initList);
         Collection<LibertyWorkspace> workspaceFolders = LibertyProjectsManager.getInstance().getLibertyWorkspaceFolders();
         assertTrue(workspaceFolders.size() == 1);

lemminx-liberty/src/test/java/io/openliberty/LibertyWorkspaceTest.java

@@ -43,10 +43,10 @@ public void testBackslashConfigDetection() throws IOException {
         String filePathString = mockXML.getCanonicalPath();
         URI filePathURI = mockXML.toURI();
 
-        assertFalse(LibertyUtils.isConfigDirFile(filePathString));
-        assertTrue(LibertyUtils.isConfigDirFile(filePathURI.toString()));
+        assertTrue(LibertyUtils.isConfigXMLFile(filePathString));
+        assertTrue(LibertyUtils.isConfigXMLFile(filePathURI.toString()));
         // mock replacement
         filePathString = filePathString.replace("\\", "/");
-        assertTrue(LibertyUtils.isConfigDirFile(filePathString));
+        assertTrue(LibertyUtils.isConfigXMLFile(filePathString));
     }
 }

lemminx-liberty/src/test/java/io/openliberty/XmlReaderTest.java

@@ -22,16 +22,13 @@ public class XmlReaderTest {
     public void readEmptyXml() throws IOException {
         File emptyXml = new File(resourcesDir, "empty_server.xml");
         assertFalse(XmlReader.hasServerRoot(emptyXml.toURI().toString()));
-        assertFalse(LibertyUtils.isServerXMLFile(emptyXml.toURI().toString()));
         assertFalse(LibertyUtils.isConfigXMLFile(emptyXml.toURI().toString()));
     }
 
     @Test
     public void readServerXml() throws IOException {
         File sampleServerXml = new File(resourcesDir, "sample/custom_server.xml");
         assertTrue(XmlReader.hasServerRoot(sampleServerXml.toURI().toString()));
-        assertFalse(LibertyUtils.isServerXMLFile(sampleServerXml.toURI().toString()));
-        assertFalse(LibertyUtils.isConfigDirFile(sampleServerXml.toURI().toString()));
         assertTrue(LibertyUtils.isConfigXMLFile(sampleServerXml.toURI().toString()));
     }