From b1918b6d9ebc824ffe11c8146095f36c79a14dcd Mon Sep 17 00:00:00 2001 From: Evie Lau <689163+evie-lau@users.noreply.github.com> Date: Wed, 25 Oct 2023 16:18:52 -0500 Subject: [PATCH] Support directory as valid location to include in server configuration (#425) * Parse include dirs in server.xml. Add tests for specifying directory in --- .github/workflows/maven.yml | 4 +- .../plugins/config/ServerConfigDocument.java | 119 +++++++++++++----- .../plugins/util/ServerFeatureUtil.java | 41 ++++-- ...stallFeatureUtilGetServerFeaturesTest.java | 55 +++++++- .../util/ServerConfigDocumentTest.java | 14 ++- .../includeDir/anotherInclude.xml | 3 + .../includeDir/yetAnotherInclude.xml | 3 + .../wlp/usr/servers/defaultServer/server.xml | 4 + .../servers/includeDir/extraFeatures.xml | 6 + .../servers/includeDir/extraFeatures2.xml | 6 + .../servers/includeDir/extraFeatures4.xml | 6 + .../resources/servers/server_dir_ignore.xml | 7 ++ .../resources/servers/server_dir_replace.xml | 7 ++ 13 files changed, 230 insertions(+), 45 deletions(-) create mode 100644 src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/anotherInclude.xml create mode 100644 src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/yetAnotherInclude.xml create mode 100644 src/test/resources/servers/includeDir/extraFeatures.xml create mode 100644 src/test/resources/servers/includeDir/extraFeatures2.xml create mode 100644 src/test/resources/servers/includeDir/extraFeatures4.xml create mode 100644 src/test/resources/servers/server_dir_ignore.xml create mode 100644 src/test/resources/servers/server_dir_replace.xml diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 53266c2b4..16bbbdba9 100644 --- a/.github/workflows/maven.yml +++ b/.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 diff --git a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java b/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java index 00d99b1be..2dbb50e68 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java +++ b/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 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 getIncludeDocs(String loc) throws IOException, SAXException { + ArrayList docs = new ArrayList(); 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 + * @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 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 + * @param directory - directory to parse documents from + * @param docs - ArrayList to store parsed Documents. + * @throws IOException + */ + private static void parseDocumentsInDirectory(File directory, ArrayList docs) throws IOException { + DirectoryStream 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 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); } } } diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/ServerFeatureUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/ServerFeatureUtil.java index 5e241e285..f59e9d4d5 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/ServerFeatureUtil.java +++ b/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 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 parseIncludeNode(Set origResult, File serverDirectory, File serverFile, Properties bootstrapProperties, Element node, List updatedParsedXmls) { @@ -453,13 +458,35 @@ private Set parseIncludeNode(Set origResult, File serverDirector debug("Exception received: "+e.getMessage(), e); return result; } - if (!updatedParsedXmls.contains(includeFile)) { - String onConflict = node.getAttribute("onConflict"); - Set features = getServerXmlFeatures(null, serverDirectory, includeFile, bootstrapProperties, updatedParsedXmls); - if (features != null && !features.isEmpty()) { - info("Features were included for file "+ includeFileName); + + ArrayList includeFiles = new ArrayList(); + if (includeFile.isDirectory()) { + try (DirectoryStream 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 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(); } } diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/InstallFeatureUtilGetServerFeaturesTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/InstallFeatureUtilGetServerFeaturesTest.java index 02850237d..7c0d75aaf 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/InstallFeatureUtilGetServerFeaturesTest.java +++ b/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 expected) throws Exception { Set getServerResult = util.getServerFeatures(serverDirectory, null); assertEquals("The features returned from getServerFeatures do not equal the expected features.", expected, getServerResult); } - - private void verifyServerFeatures(Set expected, Set ignoreFiles) throws Exception { Set 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 expected = new HashSet(); + 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 expected = new HashSet(); + 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 expected = new HashSet(); + 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 diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/ServerConfigDocumentTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/ServerConfigDocumentTest.java index 859b258e8..f03814389 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/ServerConfigDocumentTest.java +++ b/src/test/java/io/openliberty/tools/common/plugins/util/ServerConfigDocumentTest.java @@ -46,6 +46,8 @@ 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 libertyDirectoryPropertyToFile = getLibertyDirectoryPropertyFiles(log, serverDirectory); @@ -53,12 +55,14 @@ public void testAppLocationUsesLibertyProperty() throws Exception { ServerConfigDocument scd = ServerConfigDocument.getInstance(log, serverXML, serverDirectory, null, null, null, true, libertyDirectoryPropertyToFile); Set 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); } diff --git a/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/anotherInclude.xml b/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/anotherInclude.xml new file mode 100644 index 000000000..d17469a22 --- /dev/null +++ b/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/anotherInclude.xml @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/yetAnotherInclude.xml b/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/yetAnotherInclude.xml new file mode 100644 index 000000000..9609976ef --- /dev/null +++ b/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/includeDir/yetAnotherInclude.xml @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/server.xml b/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/server.xml index 72e9f7453..955900861 100644 --- a/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/server.xml +++ b/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/server.xml @@ -17,4 +17,8 @@ + + + + \ No newline at end of file diff --git a/src/test/resources/servers/includeDir/extraFeatures.xml b/src/test/resources/servers/includeDir/extraFeatures.xml new file mode 100644 index 000000000..45fc9c439 --- /dev/null +++ b/src/test/resources/servers/includeDir/extraFeatures.xml @@ -0,0 +1,6 @@ + + + + extra + + \ No newline at end of file diff --git a/src/test/resources/servers/includeDir/extraFeatures2.xml b/src/test/resources/servers/includeDir/extraFeatures2.xml new file mode 100644 index 000000000..c11cdf780 --- /dev/null +++ b/src/test/resources/servers/includeDir/extraFeatures2.xml @@ -0,0 +1,6 @@ + + + + extra2 + + \ No newline at end of file diff --git a/src/test/resources/servers/includeDir/extraFeatures4.xml b/src/test/resources/servers/includeDir/extraFeatures4.xml new file mode 100644 index 000000000..904ae0c0e --- /dev/null +++ b/src/test/resources/servers/includeDir/extraFeatures4.xml @@ -0,0 +1,6 @@ + + + + extra4 + + \ No newline at end of file diff --git a/src/test/resources/servers/server_dir_ignore.xml b/src/test/resources/servers/server_dir_ignore.xml new file mode 100644 index 000000000..c9f70e384 --- /dev/null +++ b/src/test/resources/servers/server_dir_ignore.xml @@ -0,0 +1,7 @@ + + + + orig + + + \ No newline at end of file diff --git a/src/test/resources/servers/server_dir_replace.xml b/src/test/resources/servers/server_dir_replace.xml new file mode 100644 index 000000000..260287281 --- /dev/null +++ b/src/test/resources/servers/server_dir_replace.xml @@ -0,0 +1,7 @@ + + + + orig + + + \ No newline at end of file