[grid] Fixing cli args parsing

A bug was introduced where cli
args values where being duplicated
in some cases.

Author: diemol

java/server/src/org/openqa/selenium/grid/TemplateGridCommand.java

@@ -98,7 +98,7 @@ public final Executable configure(PrintStream out, PrintStream err, String... ar
         .map(ParameterDescription::getLongestName)
         .collect(Collectors.toSet());
       if (cliArgs.size() > 0) {
-        allFlags.forEach(flags -> allConfigs.add(new AnnotatedConfig(flags, cliArgs)));
+        allFlags.forEach(flags -> allConfigs.add(new AnnotatedConfig(flags, cliArgs, true)));
       }
 
       // 4. Configuration files (config.toml)
@@ -111,7 +111,7 @@ public final Executable configure(PrintStream out, PrintStream err, String... ar
       getFlagObjects().forEach(flagObject -> allConfigs.add(new AnnotatedConfig(flagObject)));
 
       // 7. Default values
-      allFlags.forEach(flags -> allConfigs.add(new AnnotatedConfig(flags)));
+      allFlags.forEach(flags -> allConfigs.add(new AnnotatedConfig(flags, cliArgs, false)));
 
       Config config = new MemoizedConfig(new CompoundConfig(allConfigs.toArray(new Config[0])));
 

java/server/src/org/openqa/selenium/grid/config/AnnotatedConfig.java

@@ -55,10 +55,10 @@ public class AnnotatedConfig implements Config {
   private final Map<String, Map<String, List<String>>> config;
 
   public AnnotatedConfig(Object obj) {
-    this(obj, Collections.emptySet());
+    this(obj, Collections.emptySet(), false);
   }
 
-  public AnnotatedConfig(Object obj, Set<String> cliArgs) {
+  public AnnotatedConfig(Object obj, Set<String> cliArgs, boolean includeCliArgs) {
     Map<String, Map<String, List<String>>> values = new HashMap<>();
 
     Deque<Field> allConfigValues = findConfigFields(obj.getClass());
@@ -80,10 +80,14 @@ public AnnotatedConfig(Object obj, Set<String> cliArgs) {
       Parameter cliAnnotation = field.getAnnotation(Parameter.class);
       boolean containsCliArg = cliAnnotation != null &&
                                Arrays.stream(cliAnnotation.names()).anyMatch(cliArgs::contains);
-      if (cliArgs.size() > 0 && !containsCliArg) {
+      if (cliArgs.size() > 0 && !containsCliArg && includeCliArgs) {
         // Only getting config values for args entered by the user.
         continue;
       }
+      if (cliArgs.size() > 0 && containsCliArg && !includeCliArgs) {
+        // Excluding config values for args entered by the user.
+        continue;
+      }
       Map<String, List<String>> section = values.computeIfAbsent(
         annotation.section(),
         str -> new HashMap<>());

java/server/src/org/openqa/selenium/grid/node/config/NodeOptions.java

@@ -124,15 +124,19 @@ public Map<Capabilities, Collection<SessionFactory>> getSessionFactories(
     Function<Capabilities, Collection<SessionFactory>> factoryFactory) {
 
     LOG.log(Level.INFO, "Detected {0} available processors", DEFAULT_MAX_SESSIONS);
-    int maxSessions = getMaxSessions();
-    if (maxSessions > DEFAULT_MAX_SESSIONS) {
+    boolean overrideMaxSessions = config.getBool(NODE_SECTION, "override-max-sessions")
+      .orElse(OVERRIDE_MAX_SESSIONS);
+    if (overrideMaxSessions) {
       LOG.log(Level.WARNING,
               "Overriding max recommended number of {0} concurrent sessions. "
               + "Session stability and reliability might suffer!",
               DEFAULT_MAX_SESSIONS);
       LOG.warning("One browser session is recommended per available processor. IE and "
                   + "Safari are always limited to 1 session per host.");
       LOG.warning("Double check if enabling 'override-max-sessions' is really needed");
+    }
+    int maxSessions = getMaxSessions();
+    if (maxSessions > DEFAULT_MAX_SESSIONS) {
       LOG.log(Level.WARNING, "Max sessions set to {0} ", maxSessions);
     }
 
@@ -451,13 +455,15 @@ private int getDriverMaxSessions(WebDriverInfo info, int desiredMaxSessions) {
     if (info.getMaximumSimultaneousSessions() == 1) {
       return info.getMaximumSimultaneousSessions();
     }
-    if (desiredMaxSessions > info.getMaximumSimultaneousSessions()) {
+    boolean overrideMaxSessions = config.getBool(NODE_SECTION, "override-max-sessions")
+      .orElse(OVERRIDE_MAX_SESSIONS);
+    if (desiredMaxSessions > info.getMaximumSimultaneousSessions() && overrideMaxSessions) {
       String logMessage = String.format(
         "Overriding max recommended number of %s concurrent sessions for %s, setting it to %s",
         info.getMaximumSimultaneousSessions(),
         info.getDisplayName(),
         desiredMaxSessions);
-      LOG.log(Level.WARNING, logMessage);
+      LOG.log(Level.FINE, logMessage);
       return desiredMaxSessions;
     }
     return Math.min(info.getMaximumSimultaneousSessions(), desiredMaxSessions);

java/server/test/org/openqa/selenium/grid/config/AnnotatedConfigTest.java

@@ -17,10 +17,6 @@
 
 package org.openqa.selenium.grid.config;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 
@@ -33,6 +29,10 @@
 import java.util.Optional;
 import java.util.Set;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 public class AnnotatedConfigTest {
 
   @Test
@@ -172,7 +172,8 @@ class TypesToBeFiltered {
 
     Config config = new AnnotatedConfig(
       new TypesToBeFiltered(),
-      ImmutableSet.of("--string", "--bool"));
+      ImmutableSet.of("--string", "--bool"),
+      true);
     assertEquals(Optional.of(true), config.getBool("types", "boolean"));
     assertEquals(Optional.of("A String"), config.get("types", "string"));
     assertEquals(Optional.empty(), config.getInt("types", "integer"));