Modify behavior for include directory to match Liberty

OpenLiberty · Oct 26, 2023 · 959f1fa · 959f1fa
1 parent b1918b6
commit 959f1fa
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 36 deletions.
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
@@ -450,11 +450,8 @@ private static ArrayList<Document> getIncludeDocs(String loc) throws IOException
             // TODO handle ftp protocol
         } else {
             locFile = new File(loc);
-
             // check if absolute file
-            if (locFile.isAbsolute()) {
-                parseDocumentFromFileOrDirectory(locFile, docs);
-            } else {
+            if (!locFile.isAbsolute()) {
                 // check configDirectory first if exists
                 if (configDirectory != null && configDirectory.exists()) {
                     locFile = new File(configDirectory, loc);
@@ -463,8 +460,8 @@ private static ArrayList<Document> getIncludeDocs(String loc) throws IOException
                 if (locFile == null || !locFile.exists()) {
                     locFile = new File(getServerXML().getParentFile(), loc);
                 }
-                parseDocumentFromFileOrDirectory(locFile, docs);
             }
+            parseDocumentFromFileOrDirectory(locFile, loc, docs);
         }
 
         if (docs.isEmpty()) {
@@ -475,24 +472,36 @@ private static ArrayList<Document> getIncludeDocs(String loc) throws IOException
 
     /**
      * Parses file or directory for all xml documents, and adds to ArrayList<Document>
-     * @param file - file or directory to parse documents from
+     * @param f - file or directory to parse documents from
+     * @param isLibertyDirectory - indicates if directory. Liberty bases this off of the presence of trailing File.separator
      * @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 {
+    private static void parseDocumentFromFileOrDirectory(File f, String loc, ArrayList<Document> docs) throws FileNotFoundException, IOException, SAXException {
         Document doc = null;
-        if (file == null || !file.exists()) {
-            log.warn("Unable to parse from file: " + file.getCanonicalPath());
+        boolean isLibertyDirectory = loc.endsWith("/"); // Liberty uses this to determine if directory
+
+        if (f == null || !f.exists()) {
+            log.warn("Unable to parse from file: " + f.getCanonicalPath());
             return;
         }
-        if (file.isFile()) {
-            doc = parseDocument(file);
-            docs.add(doc);
+        // If file mismatches Liberty definition of directory
+        if (f.isFile() && isLibertyDirectory) {
+            log.error("Path specified a directory, but resource exists as a file (path=" + loc + ")");
+            return;
+        } else if (f.isDirectory() && !isLibertyDirectory) {
+            log.error("Path specified a file, but resource exists as a directory (path=" + loc + ")");
+            return;
+        }
+
+        if (f.isDirectory()) {
+            parseDocumentsInDirectory(f, docs);
         }
-        if (file.isDirectory()) {
-            parseDocumentsInDirectory(file, docs);
+        if (f.isFile()) {
+            doc = parseDocument(f);
+            docs.add(doc);
         }
     }
 

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
@@ -423,6 +423,7 @@ private Set<String> parseIncludeNode(Set<String> origResult, File serverDirector
         Set<String> result = origResult;
         // Need to handle more variable substitution for include location.
         String nodeValue = node.getAttribute("location");
+        // NOTE: resolveVariables converts Windows \ into /
         String includeFileName = VariableUtility.resolveVariables(this, nodeValue, null, bootstrapProperties, new Properties(), getLibertyDirectoryPropertyFiles());
 
         if (includeFileName == null || includeFileName.trim().isEmpty()) {
@@ -459,7 +460,32 @@ private Set<String> parseIncludeNode(Set<String> origResult, File serverDirector
             return result;
         }
 
+        ArrayList<File> includeFiles = parseIncludeFileOrDirectory(includeFileName, 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);
+            }
+        }
+        return result;
+    }
+
+    private ArrayList<File> parseIncludeFileOrDirectory(String includeFileName, File includeFile) {
+        boolean isLibertyDirectory = includeFileName.endsWith("/"); // Note, resolveVariables converts Windows File.separator to use forward slash
         ArrayList<File> includeFiles = new ArrayList<File>();
+        if (includeFile.isFile() && isLibertyDirectory) {
+            error("Path specified a directory, but resource exists as a file (path=" + includeFileName + ")");
+            return includeFiles;
+        } else if (includeFile.isDirectory() && !isLibertyDirectory) {
+            error("Path specified a file, but resource exists as a directory (path=" + includeFileName + ")");
+            return includeFiles;
+        }
+
         if (includeFile.isDirectory()) {
             try (DirectoryStream<Path> dstream = Files.newDirectoryStream(includeFile.toPath(), "*.xml")) {
                 StreamSupport.stream(dstream.spliterator(), false)
@@ -477,19 +503,8 @@ private Set<String> parseIncludeNode(Set<String> origResult, File serverDirector
         } 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);
-            }
-        }
-        return result;
-    }
+        return includeFiles;
+    } 
 
     private static boolean isURL(String url) {
         try {

diff --git a/...ava/io/openliberty/tools/common/plugins/util/InstallFeatureUtilGetServerFeaturesTest.java b/...ava/io/openliberty/tools/common/plugins/util/InstallFeatureUtilGetServerFeaturesTest.java
@@ -598,7 +598,8 @@ private void replaceIncludeLocation(String includeLocation) throws Exception {
      */
     @Test
     public void testIncludeDir() throws Exception {
-        replaceIncludeDir("includeDir");
+        // Note: Both the product code and test code end up converting Windows \ into /
+        replaceIncludeLocation("includeDir/"); 
         copy("includeDir");
 
         Set<String> expected = new HashSet<String>();
@@ -633,11 +634,6 @@ public void testIncludeDirIgnore() throws Exception {
 
         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/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/server.xml b/src/test/resources/serverConfig/liberty/wlp/usr/servers/defaultServer/server.xml
@@ -17,7 +17,7 @@
     <include location="${includeLocationNonDefault}/firstInclude.xml"/>    
     <webApplication location="${project.artifactId.four}.ear"/>
 
-    <include location="${server.config.dir}/includeDir"/>
+    <include location="${server.config.dir}/includeDir/"/>
     <webApplication location="${project.artifactId.five}.war"/>
     <webApplication location="${project.artifactId.six}.war"/>
 

diff --git a/src/test/resources/servers/server_dir_ignore.xml b/src/test/resources/servers/server_dir_ignore.xml
@@ -3,5 +3,5 @@
     <featureManager>
         <feature>orig</feature>
     </featureManager>
-    <include location="includeDir" onConflict="IGNORE"/>
+    <include location="includeDir/" onConflict="IGNORE"/>
 </server>
diff --git a/src/test/resources/servers/server_dir_replace.xml b/src/test/resources/servers/server_dir_replace.xml
@@ -3,5 +3,5 @@
     <featureManager>
         <feature>orig</feature>
     </featureManager>
-    <include location="includeDir" onConflict="REPLACE"/>
+    <include location="includeDir/" onConflict="REPLACE"/>
 </server>