From dd11fecfa2db7fd3a6126293cbf36120e85e111d Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Wed, 5 Jul 2023 13:17:30 -0500 Subject: [PATCH 1/5] Add late prop resolution for Liberty configuration specified by Maven properties --- .../it/server-param-pom-override-it/pom.xml | 4 +- .../wlp/maven/test/app/PluginConfigXmlIT.java | 37 +++++++++++++++++-- .../maven/server/StartDebugMojoSupport.java | 22 +++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) 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 833581ad9..6bca7e75a 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 @@ -13,8 +13,10 @@ war - + -javaagent:/path/to/some/jar.jar + @{myArgLine} -Xms512m + -Xmx1024m pom.xml /opt/ibm/java 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 2a3ef0dd2..b4b37ed09 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 @@ -15,7 +15,7 @@ import org.w3c.dom.NodeList; import org.w3c.dom.Element; -import static junit.framework.Assert.*; +import static org.junit.Assert.*; public class PluginConfigXmlIT { @@ -126,8 +126,39 @@ public void testJvmOptionsFileElements() throws Exception { String fileContents = FileUtils.fileRead(TARGET_JVM_OPTIONS).replaceAll("\r",""); - // verify that -Xmx768m is last in the jvm.options file, and that -Xms512m and -Xmx1024m appear before it. - assertTrue("verify target server jvm.options", fileContents.equals( "# Generated by liberty-maven-plugin\n-Xms512m\n-Xmx1024m\n-Xmx768m\n") || fileContents.equals("# Generated by liberty-maven-plugin\n-Xmx1024m\n-Xms512m\n-Xmx768m\n")); + String[] fileContentsArray = fileContents.split("\\n"); + assertTrue("fileContents", fileContentsArray.length == 5); + + boolean myArgLineKeyFound = false; + boolean myArgLineValueFound = false; + boolean myXms512mFound = false; + boolean myXmx1024mFound = false; + + 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. + if (i == 0) { + assertTrue("comment not found on first line", nextLine.equals("# Generated by liberty-maven-plugin")); + } else if (i == 4) { + assertTrue("-Xmx768m not found on last line", nextLine.equals("-Xmx768m")); + } else { + if (nextLine.equals("@{myArgLine}")) { + myArgLineKeyFound = true; + } else if (nextLine.equals("-Xms512m")) { + myXms512mFound = true; + } else if (nextLine.equals("-Xmx1024m")) { + myXmx1024mFound = true; + } else if (nextLine.equals("-javaagent:/path/to/some/jar.jar")) { + myArgLineValueFound = true; + } + } + } + + assertFalse("@{myArgLine} found", myArgLineKeyFound); + assertTrue("-javaagent:/path/to/some/jar.jar not found", myArgLineValueFound); + assertTrue("-Xms512m not found", myXms512mFound); + assertTrue("-Xmx1024m not found", myXmx1024mFound); + } @Test 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 b297e0615..a13b76fc7 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 @@ -82,6 +82,8 @@ public abstract class StartDebugMojoSupport extends ServerFeatureSupport { protected static final String HEADER = "# Generated by liberty-maven-plugin"; private static final String LIBERTY_CONFIG_MAVEN_PROPS = "(^liberty\\.(env|jvm|bootstrap|var|defaultVar)\\.).+"; private static final Pattern pattern = Pattern.compile(LIBERTY_CONFIG_MAVEN_PROPS); + private static final String LATE_PROP_RESOLUTION_SYNTAX = "@\\{(.+?)\\}"; + private static final Pattern LATE_PROP_PATTERN = Pattern.compile(LATE_PROP_RESOLUTION_SYNTAX); private static boolean configFilesCopied = false; @@ -796,6 +798,9 @@ private void loadLibertyConfigFromProperties(Properties props) { if (propType != null) { String suffix = key.substring(propType.getPrefix().length()); String value = (String) entry.getValue(); + // Check the value for late property resolution with @{xxx} syntax. + value = handleLatePropertyResolution(value); + getLog().debug("Processing Liberty configuration from property with key "+key+" and value "+value); switch (propType) { case ENV: envMavenProps.put(suffix, value); @@ -813,6 +818,23 @@ private void loadLibertyConfigFromProperties(Properties props) { } } + // Search the value parameter for any properties referenced with @{xxx} syntax and replace those with their property value if defined. + private String handleLatePropertyResolution(String value) { + String returnValue = value; + + Matcher m = LATE_PROP_PATTERN.matcher(value); + while (m.find()) { + String varName = m.group(1); + if (project.getProperties().containsKey(varName)) { + String replacementValue = project.getProperties().getProperty(varName,""); + returnValue = returnValue.replace("@{"+varName+"}", replacementValue); + getLog().debug("Replaced Liberty configuration property value @{"+varName+"} with value "+replacementValue); + } + } + + return returnValue; + } + // The properties parameter comes from the configuration in pom.xml and takes precedence over // the mavenProperties parameter, which comes from generic maven configuration. // One of the passed in Maps must be not null and not empty From a85860934d8801c0524c1215b1b1827540b75abd Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Wed, 5 Jul 2023 13:33:29 -0500 Subject: [PATCH 2/5] Change behavior for unresolved property --- .../src/it/server-param-pom-override-it/pom.xml | 1 + .../wlp/maven/test/app/PluginConfigXmlIT.java | 16 ++++++++-------- .../maven/server/StartDebugMojoSupport.java | 8 +++++--- 3 files changed, 14 insertions(+), 11 deletions(-) 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 6bca7e75a..970839cc4 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 @@ -15,6 +15,7 @@ -javaagent:/path/to/some/jar.jar @{myArgLine} + @{undefined} -Xms512m -Xmx1024m 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 b4b37ed09..cb6e0f7a4 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 @@ -127,37 +127,37 @@ public void testJvmOptionsFileElements() throws Exception { String fileContents = FileUtils.fileRead(TARGET_JVM_OPTIONS).replaceAll("\r",""); String[] fileContentsArray = fileContents.split("\\n"); - assertTrue("fileContents", fileContentsArray.length == 5); + assertTrue("fileContents", fileContentsArray.length == 6); - boolean myArgLineKeyFound = false; boolean myArgLineValueFound = false; boolean myXms512mFound = false; boolean myXmx1024mFound = false; + boolean myUndefinedVarFound = false; 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. if (i == 0) { assertTrue("comment not found on first line", nextLine.equals("# Generated by liberty-maven-plugin")); - } else if (i == 4) { + } else if (i == 5) { assertTrue("-Xmx768m not found on last line", nextLine.equals("-Xmx768m")); } else { - if (nextLine.equals("@{myArgLine}")) { - myArgLineKeyFound = true; - } else if (nextLine.equals("-Xms512m")) { + if (nextLine.equals("-Xms512m")) { myXms512mFound = true; } else if (nextLine.equals("-Xmx1024m")) { myXmx1024mFound = true; } else if (nextLine.equals("-javaagent:/path/to/some/jar.jar")) { myArgLineValueFound = true; + } else if (nextLine.equals("@{undefined}")) { + myUndefinedVarFound = true; } } } - assertFalse("@{myArgLine} found", myArgLineKeyFound); - assertTrue("-javaagent:/path/to/some/jar.jar not found", myArgLineValueFound); assertTrue("-Xms512m not found", myXms512mFound); assertTrue("-Xmx1024m not found", myXmx1024mFound); + assertTrue("-javaagent:/path/to/some/jar.jar not found", myArgLineValueFound); + assertTrue("@{undefined} not found", myUndefinedVarFound); } 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 a13b76fc7..a75d3f566 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 @@ -826,9 +826,11 @@ private String handleLatePropertyResolution(String value) { while (m.find()) { String varName = m.group(1); if (project.getProperties().containsKey(varName)) { - String replacementValue = project.getProperties().getProperty(varName,""); - returnValue = returnValue.replace("@{"+varName+"}", replacementValue); - getLog().debug("Replaced Liberty configuration property value @{"+varName+"} with value "+replacementValue); + String replacementValue = project.getProperties().getProperty(varName); + if (replacementValue != null) { + returnValue = returnValue.replace("@{"+varName+"}", replacementValue); + getLog().debug("Replaced Liberty configuration property value @{"+varName+"} with value "+replacementValue); + } } } From 01e60902302939de6067044d51cbb0276a25d62d Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Wed, 12 Jul 2023 08:31:11 -0500 Subject: [PATCH 3/5] Add prop resolution for config parameters --- .../it/server-param-pom-override-it/pom.xml | 5 +++ .../wlp/maven/test/app/PluginConfigXmlIT.java | 29 ++++++++++++++-- .../maven/server/PluginConfigSupport.java | 10 ++++-- .../maven/server/StartDebugMojoSupport.java | 34 +++++++++++++++++-- 4 files changed, 72 insertions(+), 6 deletions(-) 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 970839cc4..949225f7c 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 @@ -13,6 +13,7 @@ war + someBootstrapValue -javaagent:/path/to/some/jar.jar @{myArgLine} @{undefined} @@ -70,6 +71,10 @@ -Xmx768m + + @{myBootstrapArg} + @{undefinedValue} + test src/test/resources/server.xml src/main/liberty/config/bootstrap.properties 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 cb6e0f7a4..172ce7a03 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 @@ -96,8 +96,33 @@ public void testBootstrapPropertiesFileElements() throws Exception { nodes = (NodeList) xPath.compile(expression).evaluate(inputDoc, XPathConstants.NODESET); assertEquals("Number of bootstrapProperties element ==>", 1, nodes.getLength()); - assertEquals("verify target server bootstrap.properties", "# Generated by liberty-maven-plugin\nlocation=pom.xml\n", - FileUtils.fileRead(TARGET_BOOTSTRAP_PROPERTIES).replaceAll("\r","")); + String fileContents = FileUtils.fileRead(TARGET_BOOTSTRAP_PROPERTIES).replaceAll("\r",""); + + String[] fileContentsArray = fileContents.split("\\n"); + assertTrue("fileContents", fileContentsArray.length == 4); + + boolean someBootstrapVarFound = false; + boolean locationFound = false; + boolean someUndefinedBootstrapVarFound = false; + + for (int i=0; i < fileContentsArray.length; i++) { + String nextLine = fileContentsArray[i]; + if (i == 0) { + assertTrue("comment not found on first line", nextLine.equals("# Generated by liberty-maven-plugin")); + } else { + if (nextLine.equals("someBootstrapVar=someBootstrapValue")) { + someBootstrapVarFound = true; + } else if (nextLine.equals("location=pom.xml")) { + locationFound = true; + } else if (nextLine.equals("someUndefinedBootstrapVar=@{undefinedValue}")) { + someUndefinedBootstrapVarFound = true; + } + } + } + + assertTrue("someBootstrapVar=someBootstrapValue not found", someBootstrapVarFound); + assertTrue("location=pom.xml not found", locationFound); + assertTrue("someUndefinedBootstrapVar=@{undefinedValue} not found", someUndefinedBootstrapVarFound); } @Test 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 1b16fe83a..becefe411 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 @@ -120,7 +120,10 @@ protected File exportParametersToXml() throws IOException, ParserConfigurationEx if (combinedBootstrapProperties != null) { configDocument.createElement("bootstrapProperties", combinedBootstrapProperties); } else if (bootstrapProperties != null) { - configDocument.createElement("bootstrapProperties", bootstrapProperties); + if (bootstrapPropertiesResolved == null) { + bootstrapPropertiesResolved = handleLatePropertyResolution(bootstrapProperties); + } + configDocument.createElement("bootstrapProperties", bootstrapPropertiesResolved); } else { configFile = findConfigFile("bootstrap.properties", bootstrapPropertiesFile); if (configFile != null) { @@ -131,7 +134,10 @@ protected File exportParametersToXml() throws IOException, ParserConfigurationEx if (combinedJvmOptions != null) { configDocument.createElement("jvmOptions", combinedJvmOptions); } else if (jvmOptions != null) { - configDocument.createElement("jvmOptions", jvmOptions); + if (jvmOptionsResolved == null) { + jvmOptionsResolved = handleLatePropertyResolution(jvmOptions); + } + configDocument.createElement("jvmOptions", jvmOptionsResolved); } 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 a75d3f566..851b24505 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 @@ -99,6 +99,10 @@ public abstract class StartDebugMojoSupport extends ServerFeatureSupport { protected Map combinedBootstrapProperties = null; protected List combinedJvmOptions = null; + // the following collections are copies of the originals with any @{xxx} references resolved in the values + protected Map bootstrapPropertiesResolved = null; // original collection is bootstrapProperties + protected List jvmOptionsResolved = null; // original collection is jvmOptions + @Component protected BuildPluginManager pluginManager; @@ -555,7 +559,8 @@ protected void copyConfigFiles() throws IOException, MojoExecutionException { if (jvmOptionsPath != null) { getLog().warn("The " + jvmOptionsPath + " file is overwritten by inlined configuration."); } - writeJvmOptions(optionsFile, jvmOptions, jvmMavenProps); + jvmOptionsResolved = handleLatePropertyResolution(jvmOptions); + writeJvmOptions(optionsFile, jvmOptionsResolved, jvmMavenProps); jvmOptionsPath = "inlined configuration"; } else if (jvmOptionsFile != null && jvmOptionsFile.exists()) { if (jvmOptionsPath != null) { @@ -582,7 +587,8 @@ protected void copyConfigFiles() throws IOException, MojoExecutionException { if (bootStrapPropertiesPath != null) { getLog().warn("The " + bootStrapPropertiesPath + " file is overwritten by inlined configuration."); } - writeBootstrapProperties(bootstrapFile, bootstrapProperties, bootstrapMavenProps); + bootstrapPropertiesResolved = handleLatePropertyResolution(bootstrapProperties); + writeBootstrapProperties(bootstrapFile, bootstrapPropertiesResolved, bootstrapMavenProps); bootStrapPropertiesPath = "inlined configuration"; } else if (bootstrapPropertiesFile != null && bootstrapPropertiesFile.exists()) { if (bootStrapPropertiesPath != null) { @@ -837,6 +843,30 @@ private String handleLatePropertyResolution(String value) { return returnValue; } + protected Map handleLatePropertyResolution(Map properties) { + Map propertiesResolved = null; + if (properties != null) { + propertiesResolved = new HashMap (); + for (Map.Entry entry : properties.entrySet()) { + String value = handleLatePropertyResolution(entry.getValue()); + propertiesResolved.put(entry.getKey(), value); + } + } + return propertiesResolved; + } + + protected List handleLatePropertyResolution(List properties) { + List propertiesResolved = null; + if (properties != null) { + propertiesResolved = new ArrayList (); + for (String nextOption : properties) { + String value = handleLatePropertyResolution(nextOption); + propertiesResolved.add(value); + } + } + return propertiesResolved; + } + // The properties parameter comes from the configuration in pom.xml and takes precedence over // the mavenProperties parameter, which comes from generic maven configuration. // One of the passed in Maps must be not null and not empty From 11c2fdf0325263b6ae955666942753461e799f44 Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Wed, 12 Jul 2023 11:22:55 -0500 Subject: [PATCH 4/5] Handle null values --- .../maven/server/StartDebugMojoSupport.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) 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 851b24505..4199d251f 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 @@ -805,7 +805,7 @@ private void loadLibertyConfigFromProperties(Properties props) { String suffix = key.substring(propType.getPrefix().length()); String value = (String) entry.getValue(); // Check the value for late property resolution with @{xxx} syntax. - value = handleLatePropertyResolution(value); + value = resolveLatePropertyReferences(value); getLog().debug("Processing Liberty configuration from property with key "+key+" and value "+value); switch (propType) { @@ -825,17 +825,19 @@ private void loadLibertyConfigFromProperties(Properties props) { } // Search the value parameter for any properties referenced with @{xxx} syntax and replace those with their property value if defined. - private String handleLatePropertyResolution(String value) { + private String resolveLatePropertyReferences(String value) { String returnValue = value; - Matcher m = LATE_PROP_PATTERN.matcher(value); - while (m.find()) { - String varName = m.group(1); - if (project.getProperties().containsKey(varName)) { - String replacementValue = project.getProperties().getProperty(varName); - if (replacementValue != null) { - returnValue = returnValue.replace("@{"+varName+"}", replacementValue); - getLog().debug("Replaced Liberty configuration property value @{"+varName+"} with value "+replacementValue); + if (value != null) { + Matcher m = LATE_PROP_PATTERN.matcher(value); + while (m.find()) { + String varName = m.group(1); + if (project.getProperties().containsKey(varName)) { + String replacementValue = project.getProperties().getProperty(varName); + if (replacementValue != null) { + returnValue = returnValue.replace("@{"+varName+"}", replacementValue); + getLog().debug("Replaced Liberty configuration property value @{"+varName+"} with value "+replacementValue); + } } } } @@ -848,7 +850,7 @@ protected Map handleLatePropertyResolution(Map pro if (properties != null) { propertiesResolved = new HashMap (); for (Map.Entry entry : properties.entrySet()) { - String value = handleLatePropertyResolution(entry.getValue()); + String value = resolveLatePropertyReferences(entry.getValue()); propertiesResolved.put(entry.getKey(), value); } } @@ -860,7 +862,7 @@ protected List handleLatePropertyResolution(List properties) { if (properties != null) { propertiesResolved = new ArrayList (); for (String nextOption : properties) { - String value = handleLatePropertyResolution(nextOption); + String value = resolveLatePropertyReferences(nextOption); propertiesResolved.add(value); } } From 10ca3c99b196992580ae75f9d2f875dfbe5de20b Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Wed, 12 Jul 2023 15:14:12 -0500 Subject: [PATCH 5/5] add info to doc about late prop resolution --- docs/common-server-parameters.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/common-server-parameters.md b/docs/common-server-parameters.md index e3de81402..454403c96 100644 --- a/docs/common-server-parameters.md +++ b/docs/common-server-parameters.md @@ -170,3 +170,9 @@ Example of Liberty configuration with parameters: ``` + +#### Late property replacement + +Maven does property replacement for `${...}` values in pom.xml before any plugin is run. Starting with the 3.8.3 release of the liberty-maven-plugin, an alternate syntax `@{...}` is supported which allows late replacement of properties when the plugin is executed. This enables properties that are modified by other plugins to be picked up correctly. + +The alternate syntax is supported for Liberty configuration specified by Maven properties, as well as the `jvmOptions` and `bootstrapProperties` parameters.