Support directory as valid location to include in server configuration (

#425)

* Parse include dirs in server.xml. Add tests for specifying directory in <include>

.github/workflows/maven.yml

@@ -37,6 +37,6 @@ jobs:
         cache: 'maven'
     - name: Install ci.ant
       run: |
-        ./mvnw -V clean install -f ci.ant -DskipTests
+        ./mvnw -V clean install -ntp -f ci.ant -DskipTests
     - name: Build and run tests
-      run: ./mvnw -V clean install
+      run: ./mvnw -V clean install -ntp

src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java

@@ -22,9 +22,15 @@
 import java.io.InputStream;
 import java.net.URL;
 import java.net.URLConnection;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.stream.StreamSupport;
 import java.util.Map;
 import java.util.Properties;
 
@@ -360,17 +366,16 @@ private static void parseInclude(Document doc) throws XPathExpressionException,
 
                 if (includeFileName == null || includeFileName.trim().isEmpty()) {
                     log.warn("Unable to resolve include file location "+nodeValue+". Skipping the included file during application location processing.");
-                    return;
+                    continue;
                 }
 
-                Document docIncl = getIncludeDoc(includeFileName);
-
-                if (docIncl != null) {
-                    parseApplication(docIncl, XPATH_SERVER_APPLICATION);
-                    parseApplication(docIncl, XPATH_SERVER_WEB_APPLICATION);
-                    parseApplication(docIncl, XPATH_SERVER_ENTERPRISE_APPLICATION);
+                ArrayList<Document> inclDocs = getIncludeDocs(includeFileName);
+                for (Document inclDoc : inclDocs) {
+                    parseApplication(inclDoc, XPATH_SERVER_APPLICATION);
+                    parseApplication(inclDoc, XPATH_SERVER_WEB_APPLICATION);
+                    parseApplication(inclDoc, XPATH_SERVER_ENTERPRISE_APPLICATION);
                     // handle nested include elements
-                    parseInclude(docIncl);
+                    parseInclude(inclDoc);
                 }
             }
         }
@@ -422,8 +427,8 @@ private static void parseDropinsFile(File file) throws IOException, XPathExpress
         }
     }
 
-    private static Document getIncludeDoc(String loc) throws IOException, SAXException {
-
+    private static ArrayList<Document> getIncludeDocs(String loc) throws IOException, SAXException {
+        ArrayList<Document> docs = new ArrayList<Document>();
         Document doc = null;
         File locFile = null;
 
@@ -432,14 +437,14 @@ private static Document getIncludeDoc(String loc) throws IOException, SAXExcepti
                 URL url = new URL(loc);
                 URLConnection connection = url.openConnection();
                 doc = parseDocument(connection.getInputStream());
+                docs.add(doc);
             }
         } else if (loc.startsWith("file:")) {
             if (isValidURL(loc)) {
                 locFile = new File(loc);
-                if (locFile.exists()) {
-                    InputStream inputStream = new FileInputStream(locFile.getCanonicalPath());
-                    doc = parseDocument(inputStream);
-                }
+                // While URIs support directories, the Liberty include implementation does not support them yet.
+                doc = parseDocument(locFile);
+                docs.add(doc);
             }
         } else if (loc.startsWith("ftp:")) {
             // TODO handle ftp protocol
@@ -448,10 +453,7 @@ private static Document getIncludeDoc(String loc) throws IOException, SAXExcepti
 
             // check if absolute file
             if (locFile.isAbsolute()) {
-                if (locFile.exists()) {
-                    InputStream inputStream = new FileInputStream(locFile.getCanonicalPath());
-                    doc = parseDocument(inputStream);
-                }
+                parseDocumentFromFileOrDirectory(locFile, docs);
             } else {
                 // check configDirectory first if exists
                 if (configDirectory != null && configDirectory.exists()) {
@@ -461,14 +463,69 @@ private static Document getIncludeDoc(String loc) throws IOException, SAXExcepti
                 if (locFile == null || !locFile.exists()) {
                     locFile = new File(getServerXML().getParentFile(), loc);
                 }
-
-                if (locFile != null && locFile.exists()) {
-                    InputStream inputStream = new FileInputStream(locFile.getCanonicalPath());
-                    doc = parseDocument(inputStream);
-                }
+                parseDocumentFromFileOrDirectory(locFile, docs);
             }
         }
-        return doc;
+
+        if (docs.isEmpty()) {
+            log.warn("Did not parse any file(s) from include location: " + loc);
+        }
+        return docs;
+    }
+
+    /**
+     * Parses file or directory for all xml documents, and adds to ArrayList<Document>
+     * @param file - file or directory to parse documents from
+     * @param docs - ArrayList to store parsed Documents.
+     * @throws FileNotFoundException
+     * @throws IOException
+     * @throws SAXException
+     */
+    private static void parseDocumentFromFileOrDirectory(File file, ArrayList<Document> docs) throws FileNotFoundException, IOException, SAXException {
+        Document doc = null;
+        if (file == null || !file.exists()) {
+            log.warn("Unable to parse from file: " + file.getCanonicalPath());
+            return;
+        }
+        if (file.isFile()) {
+            doc = parseDocument(file);
+            docs.add(doc);
+        }
+        if (file.isDirectory()) {
+            parseDocumentsInDirectory(file, docs);
+        }
+    }
+
+    /**
+     * In a given directory, parse all direct children xml files in alphabetical order by filename, and adds to ArrayList<Document>
+     * @param directory - directory to parse documents from
+     * @param docs - ArrayList to store parsed Documents.
+     * @throws IOException
+     */
+    private static void parseDocumentsInDirectory(File directory, ArrayList<Document> docs) throws IOException {
+        DirectoryStream<Path> dstream = Files.newDirectoryStream(directory.toPath(), "*.xml");
+        StreamSupport.stream(dstream.spliterator(), false)
+            .sorted(Comparator.comparing(Path::toString))
+            .forEach(p -> { 
+                try {
+                    docs.add(parseDocument(p.toFile()));
+                } catch (Exception e) {
+                    log.warn("Unable to parse from file " + p.getFileName() + " from specified include directory: " + directory.getPath());
+                }
+            });
+    }
+
+    /**
+     * Parse Document from XML file
+     * @param file - XML file to parse for Document
+     * @return
+     * @throws FileNotFoundException
+     * @throws IOException
+     * @throws SAXException
+     */
+    private static Document parseDocument(File file) throws FileNotFoundException, IOException, SAXException {
+        InputStream is = new FileInputStream(file.getCanonicalPath());
+        return parseDocument(is);
     }
 
     private static Document parseDocument(InputStream in) throws SAXException, IOException {
@@ -559,17 +616,15 @@ private static void parseIncludeVariables(Document doc) throws XPathExpressionEx
 
                 if (includeFileName == null || includeFileName.trim().isEmpty()) {
                     log.warn("Unable to resolve include file location "+nodeValue+". Skipping the included file during application location processing.");
-                    return;
+                    continue;
                 }
-
-                Document docIncl = getIncludeDoc(includeFileName);
 
-                if (docIncl != null) {
-                    parseVariablesForBothValues(docIncl);
+                ArrayList<Document> inclDocs = getIncludeDocs(includeFileName);
+
+                for (Document inclDoc : inclDocs) {
+                    parseVariablesForBothValues(inclDoc);
                     // handle nested include elements
-                    parseIncludeVariables(docIncl);
-                } else {
-                    log.warn("Unable to parse include file "+includeFileName+". Skipping the included file during application location processing.");
+                    parseIncludeVariables(inclDoc);
                 }
             }
         }

src/main/java/io/openliberty/tools/common/plugins/util/ServerFeatureUtil.java

@@ -24,6 +24,9 @@
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URL;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.net.URLClassLoader;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
@@ -38,6 +41,7 @@
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
+import java.util.stream.StreamSupport;
 import java.util.logging.Level;
 
 import javax.xml.parsers.DocumentBuilder;
@@ -412,6 +416,7 @@ private Set<String> parseFeatureManagerNode(Element node) {
      *         parsed xml files only have featureManager sections but no
      *         features to install, or null if there are no valid xml files or
      *         they have no featureManager section
+     * @throws IOException
      */
     private Set<String> parseIncludeNode(Set<String> origResult, File serverDirectory, File serverFile, Properties bootstrapProperties, Element node,
             List<File> updatedParsedXmls) {
@@ -453,13 +458,35 @@ private Set<String> parseIncludeNode(Set<String> origResult, File serverDirector
             debug("Exception received: "+e.getMessage(), e);
             return result;
         }
-        if (!updatedParsedXmls.contains(includeFile)) {
-            String onConflict = node.getAttribute("onConflict");
-            Set<String> features = getServerXmlFeatures(null, serverDirectory, includeFile, bootstrapProperties, updatedParsedXmls);
-            if (features != null && !features.isEmpty()) {
-                info("Features were included for file "+ includeFileName);
+
+        ArrayList<File> includeFiles = new ArrayList<File>();
+        if (includeFile.isDirectory()) {
+            try (DirectoryStream<Path> dstream = Files.newDirectoryStream(includeFile.toPath(), "*.xml")) {
+                StreamSupport.stream(dstream.spliterator(), false)
+                    .sorted(Comparator.comparing(Path::toString))
+                    .forEach(p -> { 
+                        try {
+                            includeFiles.add(p.toFile());
+                        } catch (Exception e) {
+                            debug("Failed to resolve file from path: " + p);
+                        }
+                    });
+            } catch (IOException e) {
+                debug("Unable to open include directory: " + includeFileName);
+            }
+        } else {
+            includeFiles.add(includeFile);
+        }
+
+        for (File file : includeFiles) {
+            if (!updatedParsedXmls.contains(file)) {
+                String onConflict = node.getAttribute("onConflict");
+                Set<String> features = getServerXmlFeatures(null, serverDirectory, file, bootstrapProperties, updatedParsedXmls);
+                if (features != null && !features.isEmpty()) {
+                    info("Features were included for file "+ file.toString());
+                }
+                result = handleOnConflict(result, onConflict, features);
             }
-            result = handleOnConflict(result, onConflict, features);
         }
         return result;
     }
@@ -534,7 +561,7 @@ private String getRelativeServerFilePath(File serverDirectory, File serverFile)
         } catch (IOException e1) {
             debug("Unable to determine the file path of " + serverFile + " relative to the server directory "
                     + serverDirectory);
-           return serverFile.toString();
+            return serverFile.toString();
         }
     }
 

src/test/java/io/openliberty/tools/common/plugins/util/InstallFeatureUtilGetServerFeaturesTest.java

@@ -59,15 +59,17 @@ private void copyAsName(String origName, String newName) throws IOException {
 
     private void copy(String origName) throws IOException {
         File file = new File(src, origName);
-        FileUtils.copyFileToDirectory(file, serverDirectory);
+        if (file.isFile()) {
+            FileUtils.copyFileToDirectory(file, serverDirectory);
+        } else {
+            FileUtils.copyDirectoryToDirectory(file, serverDirectory);
+        }
     }
 
     private void verifyServerFeatures(Set<String> expected) throws Exception {
         Set<String> getServerResult = util.getServerFeatures(serverDirectory, null);
         assertEquals("The features returned from getServerFeatures do not equal the expected features.", expected, getServerResult);
     }
-
-
     private void verifyServerFeatures(Set<String> expected, Set<String> ignoreFiles) throws Exception {
         Set<String> getServerResult = util.getServerFeatures(serverDirectory, null, ignoreFiles);
         assertEquals("The features returned from getServerFeatures do not equal the expected features.", expected, getServerResult);
@@ -589,6 +591,53 @@ private void replaceIncludeLocation(String includeLocation) throws Exception {
         content = content.replaceAll("@includeReplacementToken@", includeReplacement);
         Files.write(serverXmlPath, content.getBytes(charset));
     }
+
+    /**
+     * Tests server.xml with include dir
+     * @throws Exception
+     */
+    @Test
+    public void testIncludeDir() throws Exception {
+        replaceIncludeDir("includeDir");
+        copy("includeDir");
+
+        Set<String> expected = new HashSet<String>();
+        expected.add("orig");
+        expected.add("extra");
+        expected.add("extra2");
+        expected.add("extra4");
+
+        verifyServerFeatures(expected);
+    }
+
+    @Test
+    public void testIncludeDirReplace() throws Exception {
+        copyAsName("server_dir_replace.xml", "server.xml");
+        copy("includeDir");
+
+        // only the last replace should be kept
+        Set<String> expected = new HashSet<String>();
+        expected.add("extra4");
+
+        verifyServerFeatures(expected);
+    }
+
+    @Test
+    public void testIncludeDirIgnore() throws Exception {
+        copyAsName("server_dir_ignore.xml", "server.xml");
+        copy("includeDir");
+
+        // only the last replace should be kept
+        Set<String> expected = new HashSet<String>();
+        expected.add("orig");
+
+        verifyServerFeatures(expected);
+    }
+
+    private void replaceIncludeDir(String includeDirName) throws Exception {
+        File includeDir = new File(src, includeDirName);
+        replaceIncludeLocation(includeDir.getName());
+    }
 
     /**
      * Tests server.xml with user features

src/test/java/io/openliberty/tools/common/plugins/util/ServerConfigDocumentTest.java

@@ -46,19 +46,23 @@ public void testAppLocationUsesLibertyProperty() throws Exception {
 		String compareAppLocation3 = "test-war-three.war";
 		 // this next one won't resolve because the var referenced in the include location uses a variable with a non-default value in server.xml
 		String compareAppLocation4 = "${project.artifactId.four}.ear"; 
+		String compareAppLocation5 = "test-war-five.war";
+		String compareAppLocation6 = "test-war-six.war";
 
 		Map<String, File> libertyDirectoryPropertyToFile = getLibertyDirectoryPropertyFiles(log, serverDirectory);
 
 		File serverXML = new File(serverDirectory, "server.xml");
 
 		ServerConfigDocument scd = ServerConfigDocument.getInstance(log, serverXML, serverDirectory, null, null, null, true, libertyDirectoryPropertyToFile);
 		Set<String> locations = scd.getLocations();
-		assertTrue("Expected four app locations", locations.size() == 4);
+		assertTrue("Expected six app locations", locations.size() == 6);
 
 		boolean locOneFound = false;
 		boolean locTwoFound = false;
 		boolean locThreeFound = false;
 		boolean locFourFound = false;
+		boolean locFiveFound = false;
+		boolean locSixFound = false;
 
 		for (String loc : locations) {
 			if (loc.contains("-two")) {
@@ -70,6 +74,12 @@ public void testAppLocationUsesLibertyProperty() throws Exception {
 			} else if (loc.endsWith(".ear")) {
 				assertTrue("Unexpected app location found: "+loc, loc.equals(compareAppLocation4));
 				locFourFound = true;
+			} else if (loc.contains("-five")) {
+				assertTrue("Unexpected app location found: "+loc, loc.equals(compareAppLocation5));
+				locFiveFound = true;
+			} else if (loc.contains("-six")) {
+				assertTrue("Unexpected app location found: "+loc, loc.equals(compareAppLocation6));
+				locSixFound = true;
 			} else {
 				assertTrue("Unexpected app location found: "+loc, loc.equals(compareAppLocation1));
 				locOneFound = true;
@@ -80,6 +90,8 @@ public void testAppLocationUsesLibertyProperty() throws Exception {
 		assertTrue("App location two not found.", locTwoFound);
 		assertTrue("App location three not found.", locThreeFound);
 		assertTrue("App location four not found.", locFourFound);
+		assertTrue("App location five not found.", locFiveFound);
+		assertTrue("App location six not found.", locSixFound);
 
 	}
 

src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/anotherInclude.xml

@@ -0,0 +1,3 @@
+<server description="">
+    <variable name="project.artifactId.five" value="test-war-five"/>
+</server>