Merge pull request #870 from cherylking/fixPropertyLoading

Do not add duplicate jvm options

src/main/groovy/io/openliberty/tools/gradle/tasks/AbstractServerTask.groovy

@@ -444,7 +444,8 @@ abstract class AbstractServerTask extends AbstractLibertyTask {
                                           break
             case PropertyType.BOOTSTRAP:  bootstrapProjectProps.setProperty(propName, propValue)
                                           break
-            case PropertyType.JVM:        jvmProjectProps.add(propName)
+            case PropertyType.JVM:        jvmProjectProps.remove(propName)  // avoid exact duplicates
+                                          jvmProjectProps.add(propName)
                                           break
             case PropertyType.VAR:        varProjectProps.setProperty(propName, propValue)
                                           break
@@ -787,18 +788,40 @@ abstract class AbstractServerTask extends AbstractLibertyTask {
         }
     }
 
+    // Remove any duplicate entries in the passed in List
+    protected List<String> getUniqueValues(List<String> values) {
+        List<String> uniqueValues = new ArrayList<String> ();
+        if (values == null) {
+            return uniqueValues
+        }
+
+        for (String nextValue : values) {
+            // by removing a matching existing value, it ensures there will not be a duplicate and that this current one will appear later in the List
+            if (uniqueValues.contains(nextValue)) {
+                getLog().debug("Remove duplicate value: "+nextValue+" at position: "+uniqueValues.indexOf(nextValue))
+            }
+            uniqueValues.remove(nextValue) // has no effect if the value is not present
+            uniqueValues.add(nextValue)
+        }
+        return uniqueValues
+    }
+
     private void writeJvmOptions(File file, List<String> options, List<String> projectProperties) throws IOException {
-        if (! projectProperties.isEmpty()) {
-            if (options == null) {
-                combinedJvmOptions = projectProperties;
+        List<String> uniqueOptions = getUniqueValues(options)
+        List<String> uniqueProps = getUniqueValues(projectProperties)
+
+        if (! uniqueProps.isEmpty()) {
+            if (uniqueOptions.isEmpty()) {
+                combinedJvmOptions = uniqueProps;
             } else {
                 combinedJvmOptions = new ArrayList<String> ()
                 // add the project properties (which come from the command line) last so that they take precedence over the options specified in build.gradle
-                combinedJvmOptions.addAll(options)
-                combinedJvmOptions.addAll(projectProperties)
+                combinedJvmOptions.addAll(uniqueOptions)
+                combinedJvmOptions.removeAll(uniqueProps) // remove any exact duplicates before adding all the project properties
+                combinedJvmOptions.addAll(uniqueProps)
             }
         } else {
-            combinedJvmOptions = options
+            combinedJvmOptions = uniqueOptions
         }
 
         makeParentDirectory(file)

src/test/groovy/io/openliberty/tools/gradle/TestCreateWithInlineProperties.groovy

@@ -39,18 +39,49 @@ public class TestCreateWithInlineProperties extends AbstractIntegrationTest{
         assert libertyConfigVarOverridesFile.exists() : "file not found"
         assert libertyConfigVarDefaultsFile.exists() : "file not found"
 
-        if (OSUtil.isWindows()) {
-            assert jvmOptionsFile.text.startsWith("# Generated by liberty-gradle-plugin\r\n") : "jvm inline options did not copy properly "+jvmOptionsFile.text
-            assert jvmOptionsFile.text.endsWith("-Xmx2048m\r\n") : "jvm inline options did not copy properly "+jvmOptionsFile.text
-            assert jvmOptionsFile.text.contains("-Xmx512m\r\n") : "jvm inline options did not copy properly "+jvmOptionsFile.text
-            assert jvmOptionsFile.text.contains("-Xms128m\r\n") : "jvm inline options did not copy properly "+jvmOptionsFile.text
-        } else {
-            assert jvmOptionsFile.text.startsWith("# Generated by liberty-gradle-plugin\n") : "jvm inline options did not copy properly "+jvmOptionsFile.text
-            assert jvmOptionsFile.text.endsWith("-Xmx2048m\n") : "jvm inline options did not copy properly "+jvmOptionsFile.text
-            assert jvmOptionsFile.text.contains("-Xmx512m\n") : "jvm inline options did not copy properly "+jvmOptionsFile.text
-            assert jvmOptionsFile.text.contains("-Xms128m\n") : "jvm inline options did not copy properly "+jvmOptionsFile.text
+        // Verify file contents appear in this overall order (but order within section is not guaranteed)
+        //
+        // Gradle jvmOptions config
+        // -Xmx512m
+        //
+        // Gradle properties
+        // -Xms128m
+        // -Dmy.dup.jvmoption=This is the value  (should only appear once even though specified in both)
+        // -Xmx2048m
+
+        String fileContents = jvmOptionsFile.text.replaceAll("\r","");
+        String[] fileContentsArray = fileContents.split("\\n");
+        assert fileContentsArray.length == 5 : "fileContentsArray.length="+fileContentsArray.length+" fileContents: "+fileContents
+        assert fileContentsArray[0].equals("# Generated by liberty-gradle-plugin") : "jvm inline options did not copy properly "+fileContentsArray[0]
+        assert fileContentsArray[1].endsWith("-Xmx512m") : "jvm inline option expected -Xmx512m, but found: "+fileContentsArray[4]
+
+        boolean xms128mFound = false
+        boolean xmx2048Found = false
+        boolean myDupJvmOptionFound = false
+        boolean unrecognizedOptionFound = false
+        String unrecognizedOption = ""
+
+        for (int i=2; i < 5; i++) {
+            if (fileContentsArray[i].equals("-Xms128m")) {
+                assert !xms128mFound : "jvm inline option -Xms128m found more than once"
+                xms128mFound = true
+            } else if (fileContentsArray[i].equals("-Xmx2048m")) {
+                assert !xmx2048Found : "jvm inline option -Xmx2048m found more than once"
+                xmx2048Found = true
+            } else if (fileContentsArray[i].equals("-Dmy.dup.jvmoption=This is the value")) {
+                assert !myDupJvmOptionFound : "jvm inline option -Dmy.dup.jvmoption found more than once"
+                myDupJvmOptionFound = true                
+            } else {
+                unrecognizedOptionFound = true
+                unrecognizedOption = fileContentsArray[i]
+            }
         }
 
+        assert !unrecognizedOptionFound : "Unrecognized jvm inline option found: "+unrecognizedOption
+        assert xms128mFound : "Expected jvm inline option -Xms128m not found"
+        assert xmx2048Found : "Expected jvm inline option -Xmx2048m not found"
+        assert myDupJvmOptionFound : "Expected jvm inline option -Dmy.dup.jvmoptio not found"
+
         assert FileUtils.contentEquals(configFile, new File("build/testBuilds/test-create-with-inline-properties/src/main/liberty/config/server.xml")) : "server.xml file did not copy properly"
 
 

src/test/resources/server-config-files/gradle.properties

@@ -1,2 +1,2 @@
-liberty.server.jvmOptions={"-Xms128m","-Xmx2048m"}
+liberty.server.jvmOptions={"-Xms128m","-Dmy.dup.jvmoption=This is the value","-Xmx2048m"}
 liberty.server.env."some.env.var"=someValue

src/test/resources/server-config-files/testCreateLibertyInlineProperties.gradle

@@ -33,7 +33,7 @@ liberty {
         name = wlpServerName
 
         bootstrapProperties = ['default.http.port':'9084','default.https.port':'9474']
-        jvmOptions = ['-Xmx512m']
+        jvmOptions = ['-Xmx512m','-Dmy.dup.jvmoption=This is the value']
     }
 }