Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not add duplicate jvm options #870

Merged
merged 3 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"


Expand Down
2 changes: 1 addition & 1 deletion src/test/resources/server-config-files/gradle.properties
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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']
}
}

Expand Down
Loading