From 92ded7bf22c9c4fa6eb1e4b02a34f4bbd82d1725 Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Fri, 8 Dec 2023 17:07:07 -0600 Subject: [PATCH 1/3] Do not add duplicate jvm options --- .../openliberty/tools/gradle/tasks/AbstractServerTask.groovy | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/io/openliberty/tools/gradle/tasks/AbstractServerTask.groovy b/src/main/groovy/io/openliberty/tools/gradle/tasks/AbstractServerTask.groovy index f3544509..0c18e4f1 100644 --- a/src/main/groovy/io/openliberty/tools/gradle/tasks/AbstractServerTask.groovy +++ b/src/main/groovy/io/openliberty/tools/gradle/tasks/AbstractServerTask.groovy @@ -444,7 +444,7 @@ abstract class AbstractServerTask extends AbstractLibertyTask { break case PropertyType.BOOTSTRAP: bootstrapProjectProps.setProperty(propName, propValue) break - case PropertyType.JVM: jvmProjectProps.add(propName) + case PropertyType.JVM: if (!jvmProjectProps.contains(propName)) { jvmProjectProps.add(propName) } // avoid exact duplicates break case PropertyType.VAR: varProjectProps.setProperty(propName, propValue) break @@ -795,6 +795,7 @@ abstract class AbstractServerTask extends AbstractLibertyTask { combinedJvmOptions = new ArrayList () // 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.removeAll(projectProperties); // remove any exact duplicates before adding all the project properties combinedJvmOptions.addAll(projectProperties) } } else { From 72d733cc11cda4febfd19d794b95082842b26c31 Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Mon, 11 Dec 2023 23:51:50 -0600 Subject: [PATCH 2/3] Additional changes for handling duplicate values for jvm options --- .../gradle/tasks/AbstractServerTask.groovy | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/main/groovy/io/openliberty/tools/gradle/tasks/AbstractServerTask.groovy b/src/main/groovy/io/openliberty/tools/gradle/tasks/AbstractServerTask.groovy index 0c18e4f1..a4e0c6b4 100644 --- a/src/main/groovy/io/openliberty/tools/gradle/tasks/AbstractServerTask.groovy +++ b/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: if (!jvmProjectProps.contains(propName)) { jvmProjectProps.add(propName) } // avoid exact duplicates + case PropertyType.JVM: jvmProjectProps.remove(propName) // avoid exact duplicates + jvmProjectProps.add(propName) break case PropertyType.VAR: varProjectProps.setProperty(propName, propValue) break @@ -787,19 +788,40 @@ abstract class AbstractServerTask extends AbstractLibertyTask { } } + // Remove any duplicate entries in the passed in List + protected List getUniqueValues(List values) { + List uniqueValues = new ArrayList (); + 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 options, List projectProperties) throws IOException { - if (! projectProperties.isEmpty()) { - if (options == null) { - combinedJvmOptions = projectProperties; + List uniqueOptions = getUniqueValues(options) + List uniqueProps = getUniqueValues(projectProperties) + + if (! uniqueProps.isEmpty()) { + if (uniqueOptions.isEmpty()) { + combinedJvmOptions = uniqueProps; } else { combinedJvmOptions = new ArrayList () // 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.removeAll(projectProperties); // remove any exact duplicates before adding all the project properties - 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) From c1ca00bb47e2ebdd2dc7a02fdbb83ac4d5935b03 Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Tue, 12 Dec 2023 10:21:56 -0600 Subject: [PATCH 3/3] Test that dups are eliminated --- .../TestCreateWithInlineProperties.groovy | 51 +++++++++++++++---- .../server-config-files/gradle.properties | 2 +- .../testCreateLibertyInlineProperties.gradle | 2 +- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/test/groovy/io/openliberty/tools/gradle/TestCreateWithInlineProperties.groovy b/src/test/groovy/io/openliberty/tools/gradle/TestCreateWithInlineProperties.groovy index 455c7f6f..e7a7920f 100644 --- a/src/test/groovy/io/openliberty/tools/gradle/TestCreateWithInlineProperties.groovy +++ b/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" diff --git a/src/test/resources/server-config-files/gradle.properties b/src/test/resources/server-config-files/gradle.properties index 62cde100..2208877e 100644 --- a/src/test/resources/server-config-files/gradle.properties +++ b/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 \ No newline at end of file diff --git a/src/test/resources/server-config-files/testCreateLibertyInlineProperties.gradle b/src/test/resources/server-config-files/testCreateLibertyInlineProperties.gradle index 0b158f68..42eb3f30 100644 --- a/src/test/resources/server-config-files/testCreateLibertyInlineProperties.gradle +++ b/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'] } }