You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by en...@apache.org on 2021/05/11 21:26:29 UTC

[sling-org-apache-sling-feature-launcher] branch master updated: SLING-10186: Fixes in cli options

This is an automated email from the ASF dual-hosted git repository.

enorman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-launcher.git


The following commit(s) were added to refs/heads/master by this push:
     new 0190cf0  SLING-10186: Fixes in cli options
0190cf0 is described below

commit 0190cf08f5be7b5af84cf237c9439c5170d86063
Author: Stefan Bischof <st...@bipolis.org>
AuthorDate: Tue May 11 23:26:24 2021 +0200

    SLING-10186: Fixes in cli options
---
 src/main/docker/Dockerfile                         |  3 +-
 .../apache/sling/feature/launcher/impl/Main.java   | 93 ++++++++++++++--------
 .../apache/sling/feature/launcher/impl/MainIT.java | 82 +++++++++++--------
 3 files changed, 113 insertions(+), 65 deletions(-)

diff --git a/src/main/docker/Dockerfile b/src/main/docker/Dockerfile
index 0a1e947..ef897bf 100644
--- a/src/main/docker/Dockerfile
+++ b/src/main/docker/Dockerfile
@@ -33,7 +33,8 @@ ENV VARIABLE_VALUES=
 ENV EXTENSION_CONFIGURATION=
 ENV FELIX_FRAMEWORK_VERSION=
 ENV OSGI_FRAMEWORK_ARTIFACT=
-ENV VERBOSE=warn
+ENV VERBOSE=info
+
 
 #ENV for java
 ENV JAVA_OPTS=
diff --git a/src/main/java/org/apache/sling/feature/launcher/impl/Main.java b/src/main/java/org/apache/sling/feature/launcher/impl/Main.java
index e8386bf..0a5d12b 100644
--- a/src/main/java/org/apache/sling/feature/launcher/impl/Main.java
+++ b/src/main/java/org/apache/sling/feature/launcher/impl/Main.java
@@ -20,13 +20,13 @@ import java.io.File;
 import java.io.PrintWriter;
 import java.util.AbstractMap;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Properties;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -77,6 +77,8 @@ public class Main {
 
     private static Options options;
 
+    private static final List<String> logLevels = Arrays.asList("trace", "debug", "info", "warn",
+            "error", "off");
     private static Logger LOG() {
 
         if (LOGGER == null) {
@@ -95,7 +97,7 @@ public class Main {
         return new String[] { keyVal.substring(0, pos), keyVal.substring(pos + 1) };
     }
 
-    static Map.Entry<String, Map<String, String>> splitMap(final String val) {
+    static Map.Entry<String, Map<String, String>> splitMap2(final String val) {
 
         String[] split1 = val.split(":");
 
@@ -103,13 +105,18 @@ public class Main {
             return new AbstractMap.SimpleEntry<>(split1[0], Collections.emptyMap());
         }
 
+        Map<String, String> m = splitMap(split1[1]);
+
+        return new AbstractMap.SimpleEntry<>(split1[0], m);
+    }
+
+    private static Map<String, String> splitMap(String value) {
         Map<String, String> m = new HashMap<>();
-        for (String kv : split1[1].split(",")) {
+        for (String kv : value.split(",")) {
             String[] keyval = splitKeyVal(kv);
             m.put(keyval[0], keyval[1]);
         }
-
-        return new AbstractMap.SimpleEntry<>(split1[0], m);
+        return m;
     }
 
     private static Optional<String> extractValueFromOption(CommandLine cl, String opt) {
@@ -172,15 +179,15 @@ public class Main {
                 .build();
 
         final Option fwkProperties = Option.builder(OPT_FRAMEWORK_PROPERTIES)
-                .longOpt("framework-properties")
-                .desc("Set framework properties")
+                .longOpt("framework-property")
+                .desc("Set framework property, format: -D key1=val1 -D key2=val2")
+                .hasArg()
                 .optionalArg(true)
-                .numberOfArgs(Option.UNLIMITED_VALUES)
                 .build();
 
         final Option varValue = Option.builder(OPT_VARIABLE_VALUES)
-                .longOpt("variable-values")
-                .desc("Set variable values")
+                .longOpt("variable-value")
+                .desc("Set variable value, format: -V key1=val1 -V key2=val2")
                 .optionalArg(true)
                 .numberOfArgs(Option.UNLIMITED_VALUES)
                 .build();
@@ -259,25 +266,32 @@ public class Main {
             extractValuesFromOption(cl, OPT_ARTICACT_CLASH).ifPresent(values -> values
                     .forEach(v -> config.getArtifactClashOverrides().add(ArtifactId.parse(v))));
 
-            Properties cfgCProps = cl.getOptionProperties(OPT_CONFIG_CLASH);
-            for (final String name : cfgCProps.stringPropertyNames()) {
-                config.getConfigClashOverrides().put(name, cfgCProps.getProperty(name));
-            }
-
-            Properties fwProps = cl.getOptionProperties(OPT_FRAMEWORK_PROPERTIES);
-            for (final String name : fwProps.stringPropertyNames()) {
-                config.getInstallation()
-                        .getFrameworkProperties()
-                        .put(name, fwProps.getProperty(name));
-            }
-
-            Properties varProps = cl.getOptionProperties(OPT_VARIABLE_VALUES);
-            for (final String name : varProps.stringPropertyNames()) {
-                config.getVariables().put(name, varProps.getProperty(name));
-            }
-
-            extractValueFromOption(cl, OPT_VERBOSE, "debug").ifPresent(
-                    value -> System.setProperty("org.slf4j.simpleLogger.defaultLogLevel", value));
+            extractValuesFromOption(cl, OPT_CONFIG_CLASH).orElseGet(ArrayList::new)
+            .forEach(value -> {
+                final String[] keyVal = split(value);
+                config.getConfigClashOverrides().put(keyVal[0], keyVal[1]);
+            });
+
+            extractValuesFromOption(cl, OPT_FRAMEWORK_PROPERTIES).orElseGet(ArrayList::new)
+            .forEach(value -> {
+                final String[] keyVal = split(value);
+                config.getInstallation().getFrameworkProperties().put(keyVal[0], keyVal[1]);
+            });
+
+            extractValuesFromOption(cl, OPT_VARIABLE_VALUES).orElseGet(ArrayList::new)
+            .forEach(value -> {
+                final String[] keyVal = split(value);
+                config.getVariables().put(keyVal[0], keyVal[1]);
+            });
+
+            if (cl.hasOption(OPT_VERBOSE)) {
+                extractValueFromOption(cl, OPT_VERBOSE, "debug").ifPresent(value -> {
+
+                    if (isLoglevel(value)) {
+                        System.setProperty("org.slf4j.simpleLogger.defaultLogLevel", value);
+                    }
+                });
+            } // if not the `org.slf4j.simpleLogger.defaultLogLevel` is by default `info`
 
             extractValuesFromOption(cl, OPT_FEATURE_FILES).orElseGet(ArrayList::new)
                     .forEach(config::addFeatureFiles);
@@ -290,7 +304,7 @@ public class Main {
 
             extractValuesFromOption(cl, OPT_EXTENSION_CONFIGURATION)
                     .ifPresent(values -> values.forEach(v -> {
-                        Map.Entry<String, Map<String, String>> xc = splitMap(v);
+                        Map.Entry<String, Map<String, String>> xc = splitMap2(v);
                         Map<String, Map<String, String>> ec = config.getExtensionConfiguration();
                         Map<String, String> c = ec.get(xc.getKey());
                         if (c == null) {
@@ -315,6 +329,11 @@ public class Main {
         }
     }
 
+    private static boolean isLoglevel(String value) {
+
+        return logLevels.contains(value);
+    }
+
     static void printHelp() {
 
         if (options == null) {
@@ -336,18 +355,18 @@ public class Main {
                 writer.println(" -" + OPT_ARTICACT_CLASH + "       -  ARTIFACT_CLASH");
                 writer.println(" -" + OPT_CONFIG_CLASH + "      -  CONFIG_CLASH");
                 writer.println(" -" + OPT_CACHE_DIR + "       -  CACHE_DIR");
-                writer.println(" -" + OPT_FRAMEWORK_PROPERTIES + "       -  FRAMEWORK_PROPERTIES");
+                writer.println(" -" + OPT_FRAMEWORK_PROPERTIES + "       -  FRAMEWORK_PROPERTIES format: `key1=val1` for more `key1=val1 -D key2=val2`");
                 writer.println(" -" + OPT_FEATURE_FILES + "       -  FEATURE_FILES");
                 writer.println(" -" + OPT_HOME_DIR + "       -  HOME_DIR");
                 writer.println(" -" + OPT_REPOSITORY_URLS + "       -  REPOSITORY_URLS");
-                writer.println(" -" + OPT_VARIABLE_VALUES + "       -  VARIABLE_VALUES");
+                writer.println(" -" + OPT_VARIABLE_VALUES + "       -  VARIABLE_VALUES format: `variable1=value1` for more `variable1=val1 -V variable2=val2`");
                 writer.println(
                         " -" + OPT_EXTENSION_CONFIGURATION + "      -  EXTENSION_CONFIGURATION");
                 writer.println(
                         " -" + OPT_FELIX_FRAMEWORK_VERSION + "      -  FELIX_FRAMEWORK_VERSION");
                 writer.println(
                         " -" + OPT_OSGI_FRAMEWORK_ARTIFACT + "      -  OSGI_FRAMEWORK_ARTIFACT");
-                writer.println(" -" + OPT_VERBOSE + "       -  VERBOSE");
+                writer.println(" -" + OPT_VERBOSE + "       -  VERBOSE {trace, debug, info, warn, error, off}");
 
                 writer.println("");
                 writer.println("Java options could be set using the env var 'JAVA_OPTS'");
@@ -359,6 +378,14 @@ public class Main {
 
     }
 
+    /** Split a string into key and value */
+    private static String[] split(final String val) {
+        final int pos = val.indexOf('=');
+        if ( pos == -1 ) {
+            return new String[] {val, "true"};
+        }
+        return new String[] {val.substring(0, pos), val.substring(pos + 1)};
+    }
     public static void main(final String[] args) {
 
         // setup logging
diff --git a/src/test/java/org/apache/sling/feature/launcher/impl/MainIT.java b/src/test/java/org/apache/sling/feature/launcher/impl/MainIT.java
index 3a270bc..a3e6450 100644
--- a/src/test/java/org/apache/sling/feature/launcher/impl/MainIT.java
+++ b/src/test/java/org/apache/sling/feature/launcher/impl/MainIT.java
@@ -18,6 +18,7 @@ package org.apache.sling.feature.launcher.impl;
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -35,9 +36,10 @@ import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-/** Run this as part of integration tests to help make sure we
- *  embed the right Apache commons classes.
-*/
+/**
+ * Run this as part of integration tests to help make sure we embed the right
+ * Apache commons classes.
+ */
 public class MainIT {
 
     protected static class SystemExitException extends SecurityException {
@@ -98,16 +100,16 @@ public class MainIT {
     public void testSplitMapCommandlineArgs() {
 
         assertEquals(new AbstractMap.SimpleEntry<>("foo", Collections.singletonMap("bar", "tar")),
-                Main.splitMap("foo:bar=tar"));
+                Main.splitMap2("foo:bar=tar"));
 
         assertEquals(new AbstractMap.SimpleEntry<>("hello", Collections.emptyMap()),
-                Main.splitMap("hello"));
+                Main.splitMap2("hello"));
 
         Map<String, String> em = new HashMap<>();
         em.put("a.b.c", "d.e.f");
         em.put("h.i.j", "k.l.m");
         Map.Entry<String, Map<String, String>> e = new AbstractMap.SimpleEntry<>("ding.dong", em);
-        assertEquals(e, Main.splitMap("ding.dong:a.b.c=d.e.f,h.i.j=k.l.m"));
+        assertEquals(e, Main.splitMap2("ding.dong:a.b.c=d.e.f,h.i.j=k.l.m"));
     }
 
     LauncherConfig noActionAllowesConfig = mock(LauncherConfig.class, invocationOnMock -> {
@@ -132,12 +134,15 @@ public class MainIT {
     @Test
     public void testParseVerbose() {
 
-        Main.parseArgs(noActionAllowesConfig, new String[] { "-" + Main.OPT_VERBOSE, "debug" });
-        assertEquals("debug", System.getProperty("org.slf4j.simpleLogger.defaultLogLevel"));
+        Main.parseArgs(noActionAllowesConfig, new String[] {  });
+        assertNull(System.getProperty("org.slf4j.simpleLogger.defaultLogLevel"));
 
         Main.parseArgs(noActionAllowesConfig, new String[] { "-" + Main.OPT_VERBOSE });
         assertEquals("debug", System.getProperty("org.slf4j.simpleLogger.defaultLogLevel"));
 
+        Main.parseArgs(noActionAllowesConfig, new String[] { "-" + Main.OPT_VERBOSE, "debug" });
+        assertEquals("debug", System.getProperty("org.slf4j.simpleLogger.defaultLogLevel"));
+
         Main.parseArgs(noActionAllowesConfig, new String[] { "-" + Main.OPT_VERBOSE, "warn" });
         assertEquals("warn", System.getProperty("org.slf4j.simpleLogger.defaultLogLevel"));
 
@@ -194,19 +199,17 @@ public class MainIT {
 
         Main.parseArgs(noActionAllowesConfig, new String[] { "-" + Main.OPT_CONFIG_CLASH });
 
+        assertEquals( Main.OPT_CONFIG_CLASH,"CC");
+
         LauncherConfig config = new LauncherConfig();
-        // -C and -CC may have a conflict
-        // Main.parseArgs(config, new String[] { "-" + Main.OPT_CONFIG_CLASH + "a=1",
-        //         "-" + Main.OPT_CONFIG_CLASH + "b=2" });
-        //
-        // assertTrue(config.getConfigClashOverrides().containsKey("a"));
-        // assertEquals("1", config.getConfigClashOverrides().get("a"));
-        // assertTrue(config.getConfigClashOverrides().containsKey("b"));
-        // assertEquals("2", config.getConfigClashOverrides().get("b"));
+        Main.parseArgs(config,args("-CC a=1"));
+
+        assertTrue(config.getConfigClashOverrides().containsKey("a"));
+        assertEquals("1", config.getConfigClashOverrides().get("a"));
 
+        
         config = new LauncherConfig();
-        Main.parseArgs(config, new String[] { "-" + Main.OPT_CONFIG_CLASH, "a", "1",
-                "-" + Main.OPT_CONFIG_CLASH, "b", "2" });
+        Main.parseArgs(config, args("-CC a=1 -CC b=2"));
 
         assertTrue(config.getConfigClashOverrides().containsKey("a"));
         assertEquals("1", config.getConfigClashOverrides().get("a"));
@@ -220,18 +223,17 @@ public class MainIT {
 
         Main.parseArgs(noActionAllowesConfig, new String[] { "-" + Main.OPT_VARIABLE_VALUES });
 
+        assertEquals( Main.OPT_VARIABLE_VALUES,"V");
+
         LauncherConfig config = new LauncherConfig();
-        Main.parseArgs(config, new String[] { "-" + Main.OPT_VARIABLE_VALUES + "a=1",
-                "-" + Main.OPT_VARIABLE_VALUES + "b=2" });
+        Main.parseArgs(config,args("-V a=1"));
 
         assertTrue(config.getVariables().containsKey("a"));
         assertEquals("1", config.getVariables().get("a"));
-        assertTrue(config.getVariables().containsKey("b"));
-        assertEquals("2", config.getVariables().get("b"));
+
 
         config = new LauncherConfig();
-        Main.parseArgs(config, new String[] { "-" + Main.OPT_VARIABLE_VALUES, "a", "1",
-                "-" + Main.OPT_VARIABLE_VALUES, "b", "2" });
+        Main.parseArgs(config,args("-V a=1 -V b=2"));
 
         assertTrue(config.getVariables().containsKey("a"));
         assertEquals("1", config.getVariables().get("a"));
@@ -245,15 +247,26 @@ public class MainIT {
 
         Main.parseArgs(noActionAllowesConfig, new String[] { "-" + Main.OPT_FRAMEWORK_PROPERTIES });
 
-        LauncherConfig config = new LauncherConfig();
-        Main.parseArgs(config, new String[] { "-" + Main.OPT_FRAMEWORK_PROPERTIES + "a=1",
-                "-" + Main.OPT_FRAMEWORK_PROPERTIES + "b=2" });
+        assertEquals( Main.OPT_FRAMEWORK_PROPERTIES,"D");
 
-        assertTrue(config.getInstallation().getFrameworkProperties().containsKey("a"));
-        assertEquals("1", config.getInstallation().getFrameworkProperties().get("a"));
+        LauncherConfig configSpaceSingle = new LauncherConfig();
+        Main.parseArgs(configSpaceSingle, args("-D a=1"));
 
-        assertTrue(config.getInstallation().getFrameworkProperties().containsKey("b"));
-        assertEquals("2", config.getInstallation().getFrameworkProperties().get("b"));
+        assertTrue(configSpaceSingle.getInstallation().getFrameworkProperties().containsKey("a"));
+        assertEquals("1", configSpaceSingle.getInstallation().getFrameworkProperties().get("a"));
+
+        LauncherConfig configSpaceNoSeparator = new LauncherConfig();
+        Main.parseArgs(configSpaceNoSeparator, args("-D a=1,b=2"));
+       
+        assertTrue(configSpaceNoSeparator.getInstallation().getFrameworkProperties().containsKey("a"));
+        assertEquals("1,b=2", configSpaceNoSeparator.getInstallation().getFrameworkProperties().get("a"));
+
+        LauncherConfig configSpaceMultiple = new LauncherConfig();
+        Main.parseArgs(configSpaceMultiple, args("-D a=1 -D b=2"));
+        assertTrue(configSpaceMultiple.getInstallation().getFrameworkProperties().containsKey("a"));
+        assertEquals("1", configSpaceMultiple.getInstallation().getFrameworkProperties().get("a"));
+        assertTrue(configSpaceMultiple.getInstallation().getFrameworkProperties().containsKey("b"));
+        assertEquals("2", configSpaceMultiple.getInstallation().getFrameworkProperties().get("b"));
     }
 
     @Test
@@ -321,6 +334,13 @@ public class MainIT {
 
     }
 
+    public static String[] args(String value) {
+        if (value == null) {
+            return new String[] {};
+        }
+        return value.split(" ");
+    }
+
     @Test
     public void testMain_main() {