diff --git a/liberty-maven-plugin/src/it/dev-it/resources/basic-dev-project/pom.xml b/liberty-maven-plugin/src/it/dev-it/resources/basic-dev-project/pom.xml index 08c39639d..ffb5356e1 100755 --- a/liberty-maven-plugin/src/it/dev-it/resources/basic-dev-project/pom.xml +++ b/liberty-maven-plugin/src/it/dev-it/resources/basic-dev-project/pom.xml @@ -13,6 +13,7 @@ UTF-8 1.8 1.8 + -Xms512m LibertyProject 9080 @@ -212,6 +213,9 @@ ${testServerHttpsPort} json + + -Xms512m + diff --git a/liberty-maven-plugin/src/it/dev-it/src/test/java/net/wasdev/wlp/test/dev/it/DevCopyTestDependenciesTest.java b/liberty-maven-plugin/src/it/dev-it/src/test/java/net/wasdev/wlp/test/dev/it/DevCopyTestDependenciesTest.java index bd66f4949..c3f1a5040 100644 --- a/liberty-maven-plugin/src/it/dev-it/src/test/java/net/wasdev/wlp/test/dev/it/DevCopyTestDependenciesTest.java +++ b/liberty-maven-plugin/src/it/dev-it/src/test/java/net/wasdev/wlp/test/dev/it/DevCopyTestDependenciesTest.java @@ -42,7 +42,7 @@ public static void setUpBeforeClass() throws Exception { // add new parameter in first argument to skip install features on restart // in this case, it should not skip install feature because Liberty was not previously installed - startProcess("-DskipInstallFeature=true", true); + startProcess("-DskipInstallFeature=true -Dliberty.jvm.minHeapSize=-Xms512m", true); } @AfterClass @@ -57,10 +57,32 @@ public void copyDependenciesTest() throws Exception { File f = new File(targetDir, "liberty/wlp/usr/servers/defaultServer/lib/global/postgresql-42.1.1.jar"); assertFalse(f.exists()); + } + @Test + public void skipInstallFeatureTest() throws Exception { assertTrue("The install-feature goal did not run: "+getLogTail(), verifyLogMessageExists("Running liberty:install-feature", 2000)); assertFalse("The skip install-feature log message was found unexpectedly: "+getLogTail(), verifyLogMessageExists("Skipping liberty:install-feature", 2000)); - } + @Test + public void duplicatJvmOptionsTest() throws Exception { + File jvmOptionsFile = new File(tempProj,"/target/liberty/wlp/usr/servers/defaultServer/jvm.options"); + assertTrue("The jvm.options file not found at path: "+jvmOptionsFile.getAbsolutePath(),jvmOptionsFile.exists()); + + String fileContents = org.codehaus.plexus.util.FileUtils.fileRead(jvmOptionsFile.getCanonicalPath()).replaceAll("\r",""); + + String[] fileContentsArray = fileContents.split("\\n"); + assertTrue("fileContents", fileContentsArray.length == 2); + + for (int i=0; i < fileContentsArray.length; i++) { + String nextLine = fileContentsArray[i]; + // verify that a single -Xms512m is the only entry in the jvm.options file. + if (i == 0) { + assertTrue("comment not found on first line", nextLine.equals("# Generated by liberty-maven-plugin")); + } else if (i == 1) { + assertTrue("-Xms512m not found on last line", nextLine.equals("-Xms512m")); + } + } + } } diff --git a/liberty-maven-plugin/src/it/server-param-pom-override-it/invoker.properties b/liberty-maven-plugin/src/it/server-param-pom-override-it/invoker.properties index c21a4c83d..2b4b8f232 100644 --- a/liberty-maven-plugin/src/it/server-param-pom-override-it/invoker.properties +++ b/liberty-maven-plugin/src/it/server-param-pom-override-it/invoker.properties @@ -1,2 +1,2 @@ # prove that Liberty configuration provided by maven properties can be overridden on cmd line -invoker.goals.1 = clean install -Dliberty.var.someVariable2=myCmdLineValue -Dliberty.var.new.CmdLine.Var=newCmdLineValue \ No newline at end of file +invoker.goals.1 = clean install -Dliberty.var.someVariable2=myCmdLineValue -Dliberty.var.new.CmdLine.Var=newCmdLineValue -Dliberty.jvm.minHeap=-Xms512m -Dliberty.jvm.myprop1="-Dmy.jvm.prop=abc" "-Dliberty.jvm.debug=-agentlib:jdwp=transport=dt_socket;server=y;suspend=n;address=7777" \ No newline at end of file diff --git a/liberty-maven-plugin/src/it/server-param-pom-override-it/pom.xml b/liberty-maven-plugin/src/it/server-param-pom-override-it/pom.xml index 949225f7c..fb603fb60 100644 --- a/liberty-maven-plugin/src/it/server-param-pom-override-it/pom.xml +++ b/liberty-maven-plugin/src/it/server-param-pom-override-it/pom.xml @@ -17,9 +17,11 @@ -javaagent:/path/to/some/jar.jar @{myArgLine} @{undefined} - -Xms512m + -agentlib:jdwp=transport=dt_socket;server=y;suspend=n;address=7777 + -Dmy.jvm.prop=abc + -Xms1024m - -Xmx1024m + -Xmx2056m pom.xml /opt/ibm/java someValue1 @@ -69,7 +71,9 @@ zip - -Xmx768m + -Xmx1024m + -agentlib:jdwp=transport=dt_socket;server=y;suspend=n;address=7777 + -Dmy.jvm.prop2=This is the value @{myBootstrapArg} @@ -104,6 +108,12 @@ io.openliberty.tools liberty-maven-plugin + + + + -Dmy.jvm.prop2=This is the value + + stop-server-before-clean diff --git a/liberty-maven-plugin/src/it/server-param-pom-override-it/src/main/liberty/config/jvm.options b/liberty-maven-plugin/src/it/server-param-pom-override-it/src/main/liberty/config/jvm.options index baa05f119..abc3faa2e 100644 --- a/liberty-maven-plugin/src/it/server-param-pom-override-it/src/main/liberty/config/jvm.options +++ b/liberty-maven-plugin/src/it/server-param-pom-override-it/src/main/liberty/config/jvm.options @@ -1 +1,2 @@ --Xmx1024m +-Xmx2056m +-Xms512m diff --git a/liberty-maven-plugin/src/it/server-param-pom-override-it/src/test/java/net/wasdev/wlp/maven/test/app/PluginConfigXmlIT.java b/liberty-maven-plugin/src/it/server-param-pom-override-it/src/test/java/net/wasdev/wlp/maven/test/app/PluginConfigXmlIT.java index 8b34396ec..0aed9ff65 100644 --- a/liberty-maven-plugin/src/it/server-param-pom-override-it/src/test/java/net/wasdev/wlp/maven/test/app/PluginConfigXmlIT.java +++ b/liberty-maven-plugin/src/it/server-param-pom-override-it/src/test/java/net/wasdev/wlp/maven/test/app/PluginConfigXmlIT.java @@ -154,37 +154,96 @@ public void testJvmOptionsFileElements() throws Exception { String fileContents = FileUtils.fileRead(TARGET_JVM_OPTIONS).replaceAll("\r",""); String[] fileContentsArray = fileContents.split("\\n"); - assertTrue("fileContents", fileContentsArray.length == 6); + assertTrue("fileContentsArray.length="+fileContentsArray.length+" fileContents: "+fileContents, fileContentsArray.length == 9); boolean myArgLineValueFound = false; + boolean myUndefinedVarFound = false; boolean myXms512mFound = false; + boolean myXms1024mFound = false; // should NOT be present boolean myXmx1024mFound = false; - boolean myUndefinedVarFound = false; + boolean myXmx2056mFound = false; // should appear before -Xmx1024m + boolean myJvmPropFoundAbc = false; // should only be found once (had dup values specified) + boolean myJvmProp2 = false; // should only be found once (had dup values specified) + boolean myDebugPropFound = false; // should only be found once, but was specified 3 different ways + + // Verify file contents appear in this overall order (but order within a section is not guaranteed). + // There should be NO duplicates. + + // Maven project properties: + // -javaagent:/path/to/some/jar.jar + // @{undefined} + // -Xmx2056m + + // System properties: + // -Xms512m (overrode project property with same name) + // -Dmy.jvm.prop=abc (overrode project property that had dup value) + + // jvmOptions + // -Xmx1024m (from jvmOptions parameter) + // -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=7777. (from jvmOptions parameter that had dup value of both a System prop and Maven project prop) + // -Dmy.jvm.prop2=This is the value (from jvmOptions parameter specified in both parent and project pom.xml - should only appear once) for (int i=0; i < fileContentsArray.length; i++) { String nextLine = fileContentsArray[i]; - // verify that -Xmx768m is last in the jvm.options file, and that -Xms512m and -Xmx1024m appear before it. + // verify that -Xmx1024m comes after -Xmx2056m and -Xms512m in the jvm.options file, and that -Xms1024m is not included. if (i == 0) { assertTrue("comment not found on first line", nextLine.equals("# Generated by liberty-maven-plugin")); - } else if (i == 5) { - assertTrue("-Xmx768m not found on last line", nextLine.equals("-Xmx768m")); - } else { + } else if (i >= 6) { // verify jvmOptions present + assertTrue("Expected jvmOptions not found: "+nextLine, + nextLine.equals("-Xmx1024m") || + nextLine.equals("-agentlib:jdwp=transport=dt_socket;server=y;suspend=n;address=7777") || + nextLine.equals("-Dmy.jvm.prop2=This is the value")); + + if (nextLine.equals("-Xmx1024m")) { + assertFalse("jvm option found more than once: "+nextLine,myXmx1024mFound); + myXmx1024mFound = true; + } else if (nextLine.equals("-agentlib:jdwp=transport=dt_socket;server=y;suspend=n;address=7777")) { + assertFalse("jvm option found more than once: "+nextLine,myDebugPropFound); + myDebugPropFound = true; + } else { + assertFalse("jvm option found more than once: "+nextLine,myJvmProp2); + myJvmProp2 = true; + } + } else if (i >= 4) { // verify System properties present + assertTrue("Expected System properties not found: "+nextLine, + nextLine.equals("-Xms512m") || + nextLine.equals("-Dmy.jvm.prop=abc")); + if (nextLine.equals("-Xms512m")) { + assertFalse("jvm option found more than once: "+nextLine,myXms512mFound); myXms512mFound = true; - } else if (nextLine.equals("-Xmx1024m")) { - myXmx1024mFound = true; - } else if (nextLine.equals("-javaagent:/path/to/some/jar.jar")) { + } else { + assertFalse("jvm option found more than once: "+nextLine,myJvmPropFoundAbc); + myJvmPropFoundAbc = true; + } + } else { // verify Maven project properties present + assertTrue("Expected jvmOptions not found: "+nextLine, + nextLine.equals("-javaagent:/path/to/some/jar.jar") || + nextLine.equals("@{undefined}") || + nextLine.equals("-Xmx2056m")); + + if (nextLine.equals("-javaagent:/path/to/some/jar.jar")) { + assertFalse("jvm option found more than once: "+nextLine,myArgLineValueFound); myArgLineValueFound = true; } else if (nextLine.equals("@{undefined}")) { + assertFalse("jvm option found more than once: "+nextLine,myUndefinedVarFound); myUndefinedVarFound = true; + } else { + assertFalse("jvm option found more than once: "+nextLine,myXmx2056mFound); + myXmx2056mFound = true; } } } + assertTrue("-Xmx1024m not found", myXmx1024mFound); + assertTrue("-Dmy.jvm.prop2=This is the value - not found", myJvmProp2); + assertTrue("-agentlib:jdwp=transport=dt_socket;server=y;suspend=n;address=7777 not found", myDebugPropFound); assertTrue("-Xms512m not found", myXms512mFound); - assertTrue("-Xmx1024m not found", myXmx1024mFound); + assertTrue("-Dmy.jvm.prop=abc", myJvmPropFoundAbc); + assertTrue("-javaagent:/path/to/some/jar.jar not found", myArgLineValueFound); assertTrue("@{undefined} not found", myUndefinedVarFound); + assertTrue("-Xmx2056m not found", myXmx2056mFound); } } diff --git a/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/BasicSupport.java b/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/BasicSupport.java index 6546e998e..572e7ca25 100644 --- a/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/BasicSupport.java +++ b/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/BasicSupport.java @@ -718,14 +718,16 @@ protected Map getLibertyDirectoryPropertyFiles() { protected void setContainerEngine(AbstractContainerSupportUtil util) throws PluginExecutionException { String LIBERTY_DEV_PODMAN = "liberty.dev.podman"; - Properties mergedProperties = project.getProperties(); - mergedProperties.putAll(System.getProperties()); //System/command line properties will take precedence - if (!mergedProperties.isEmpty() && mergedProperties.containsKey(LIBERTY_DEV_PODMAN)) { - Object isPodman = mergedProperties.get(LIBERTY_DEV_PODMAN); - if (isPodman != null) { - util.setIsDocker(!(Boolean.parseBoolean(isPodman.toString()))); - getLog().debug("liberty.dev.podman was set to: " + (Boolean.parseBoolean(isPodman.toString()))); - } + Object podmanPropValue = null; + if (System.getProperties().containsKey(LIBERTY_DEV_PODMAN)) { + podmanPropValue = System.getProperties().get(LIBERTY_DEV_PODMAN); + } else if (project.getProperties().containsKey(LIBERTY_DEV_PODMAN)) { + podmanPropValue = project.getProperties().get(LIBERTY_DEV_PODMAN); + } + + if (podmanPropValue != null) { + util.setIsDocker(!(Boolean.parseBoolean(podmanPropValue.toString()))); + getLog().debug("liberty.dev.podman was set to: " + (Boolean.parseBoolean(podmanPropValue.toString()))); } } } diff --git a/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/PluginConfigSupport.java b/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/PluginConfigSupport.java index f939bf232..13af4401d 100644 --- a/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/PluginConfigSupport.java +++ b/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/PluginConfigSupport.java @@ -137,7 +137,8 @@ protected File exportParametersToXml() throws IOException, ParserConfigurationEx if (jvmOptionsResolved == null) { jvmOptionsResolved = handleLatePropertyResolution(jvmOptions); } - configDocument.createElement("jvmOptions", jvmOptionsResolved); + List uniqueOptions = getUniqueValues(jvmOptionsResolved); + configDocument.createElement("jvmOptions", uniqueOptions); } else { configFile = findConfigFile("jvm.options", jvmOptionsFile); if (configFile != null) { diff --git a/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/StartDebugMojoSupport.java b/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/StartDebugMojoSupport.java index c8a0a21a5..a909ef918 100644 --- a/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/StartDebugMojoSupport.java +++ b/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/server/StartDebugMojoSupport.java @@ -92,7 +92,8 @@ public abstract class StartDebugMojoSupport extends ServerFeatureSupport { protected Map bootstrapMavenProps = new HashMap(); protected Map envMavenProps = new HashMap(); - protected List jvmMavenProps = new ArrayList(); + protected List jvmMavenPropNames = new ArrayList(); // only used for tracking overriding properties - not included in the generated jvm.options file + protected List jvmMavenPropValues = new ArrayList(); protected Map varMavenProps = new HashMap(); protected Map defaultVarMavenProps = new HashMap(); @@ -566,12 +567,12 @@ protected void copyConfigFiles() throws IOException, MojoExecutionException { optionsFile.delete(); } } - if (jvmOptions != null || !jvmMavenProps.isEmpty()) { + if (jvmOptions != null || !jvmMavenPropValues.isEmpty()) { if (jvmOptionsPath != null) { getLog().warn("The " + jvmOptionsPath + " file is overwritten by inlined configuration."); } jvmOptionsResolved = handleLatePropertyResolution(jvmOptions); - writeJvmOptions(optionsFile, jvmOptionsResolved, jvmMavenProps); + writeJvmOptions(optionsFile, jvmOptionsResolved, jvmMavenPropValues); jvmOptionsPath = "inlined configuration"; } else if (jvmOptionsFile != null && jvmOptionsFile.exists()) { if (jvmOptionsPath != null) { @@ -824,7 +825,14 @@ private void loadLibertyConfigFromProperties(Properties props) { break; case BOOTSTRAP: bootstrapMavenProps.put(suffix, value); break; - case JVM: jvmMavenProps.add(value); + case JVM: if (jvmMavenPropNames.contains(suffix)) { + int index = jvmMavenPropNames.indexOf(suffix); + getLog().debug("Remove duplicate property with name: "+suffix+" at position: "+index); + jvmMavenPropNames.remove(index); + jvmMavenPropValues.remove(index); + } + jvmMavenPropNames.add(suffix); // need to keep track of names so that a system prop can override a project prop + jvmMavenPropValues.add(value); break; case VAR: varMavenProps.put(suffix, value); break; @@ -943,19 +951,42 @@ private void writeServerEnvProperties(File file, Map mavenProper } } + // 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; + } + // One of the passed in Lists must be not null and not empty private void writeJvmOptions(File file, List options, List mavenProperties) throws IOException { - if (!mavenProperties.isEmpty()) { - if (options == null) { - combinedJvmOptions = mavenProperties; + List uniqueOptions = getUniqueValues(options); + List uniqueMavenProps = getUniqueValues(mavenProperties); + + if (!uniqueMavenProps.isEmpty()) { + if (uniqueOptions.isEmpty()) { + combinedJvmOptions = uniqueMavenProps; } else { combinedJvmOptions = new ArrayList (); - // add the maven properties first so that they do not take precedence over the options specified with jvmOptions - combinedJvmOptions.addAll(mavenProperties); - combinedJvmOptions.addAll(options); + // add the maven properties (which consist of both project properties and system properties) first, + // so that they do not take precedence over the options specified with jvmOptions config parameter + combinedJvmOptions.addAll(uniqueMavenProps); + combinedJvmOptions.removeAll(uniqueOptions); // remove any exact duplicates before adding all the jvmOptions + combinedJvmOptions.addAll(uniqueOptions); } } else { - combinedJvmOptions = options; + combinedJvmOptions = uniqueOptions; } makeParentDirectory(file);