selenium [grid] Replace Guava list,set,map and sorted set with Java equivalent

User description

🔗 Related Issues

Starting PR for replacing Guava's Immutable Set, List, Map, SortSet and Primitives usage with Java 11 or other equivalent in the Grid.

💥 What does this PR do?

PR replaces Guava's Immutable Set, List, Map, SortSet and Primitives usage with Java 11 or other equivalent in the Grid's packages, commands and config. The rest of the change will be done as follow up PRs.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

Cleanup (formatting, renaming)

PR Type

Enhancement

Description

Replace Guava collections with Java 11 equivalents
Remove dependencies on ImmutableList, ImmutableSet, ImmutableMap
Update sorted collections to use TreeSet
Modernize collection creation patterns

Diagram Walkthrough

flowchart LR
  A["Guava Collections"] -- "Replace with" --> B["Java 11 Collections"]
  B --> C["Map.of()"]
  B --> D["Set.of()"]
  B --> E["List.of()"]
  B --> F["TreeSet + Collections.unmodifiable*"]

File Walkthrough

Relevant files

Enhancement

14 files

DefaultHubConfig.java `Replace ImmutableMap with Map.of`	+5/-5
DefaultStandaloneConfig.java `Replace ImmutableMap with Map.of`	+4/-5
EventBusCommand.java `Replace ImmutableSet and ImmutableMap with Java equivalents`	+7/-8
Hub.java `Replace ImmutableSet with Set.of`	+1/-3
Standalone.java `Replace ImmutableSet with Set.of`	+1/-3
AnnotatedConfig.java `Replace Guava collections with TreeSet and List.copyOf`	+5/-6
CompoundConfig.java `Replace Guava collectors with Java stream collectors`	+14/-9
ConcatenatingConfig.java `Replace ImmutableMap and ImmutableSortedSet with Java equivalents`	+11/-9
Config.java `Replace ImmutableList with List.copyOf`	+1/-2
ConfigFlags.java `Replace ImmutableSet and ImmutableSortedSet with Java equivalents`	+11/-12
DescribedOption.java `Replace Guava Primitives and collections with Java equivalents`	+26/-10
EnvConfig.java `Replace ImmutableList and ImmutableSortedSet with Java equivalents`	+10/-7
MapConfig.java `Replace Guava collections with Java unmodifiable collections`	+16/-15
TomlConfig.java `Replace ImmutableList and ImmutableSortedSet with Java equivalents`	+7/-7

Aug 19 '25 07:08 pujagani

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1234 - Partially compliant

Compliant requirements:

Non-compliant requirements:

Fix click() to trigger JavaScript in href for Firefox.
Add tests demonstrating the regression and the fix.

Requires further human verification:

Manual/browser verification on affected Firefox versions.
End-to-end test behavior across supported browsers and versions.

5678 - Partially compliant

Compliant requirements:

Non-compliant requirements:

Resolve ConnectFailure on multiple ChromeDriver instantiations.
Add diagnostics and/or retries as needed; include tests or docs.

Requires further human verification:

Reproduction on target environment and versions.
Runtime logs and concurrency behavior under load.

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Possible Issue

The replacement for Guava Primitives.wrap uses a custom map PRIMITIVE_TO_WRAPPER. Ensure it fully mirrors Guava behavior for all primitives and is used consistently; also confirm no null case for non-primitive types.

Map.ofEntries(
    Map.entry(boolean.class, Boolean.class),
    Map.entry(byte.class, Byte.class),
    Map.entry(char.class, Character.class),
    Map.entry(double.class, Double.class),
    Map.entry(float.class, Float.class),
    Map.entry(int.class, Integer.class),
    Map.entry(long.class, Long.class),
    Map.entry(short.class, Short.class),
    Map.entry(void.class, Void.class));

Behavior Change

getSectionNames() and getOptions() now return sorted/unmodifiable sets; verify callers did not rely on previous insertion order or specific set implementations.

@Override
public Set<String> getSectionNames() {
  return Set.copyOf(raw.keySet());
}

@Override
public Set<String> getOptions(String section) {
  Require.nonNull("Section name to get options for", section);

  Map<String, Object> values = raw.getOrDefault(section, Map.of());
  return Collections.unmodifiableSortedSet(new TreeSet<>(values.keySet()));
}

API Compatibility

JSON response map construction changed to Map.of(...). Confirm no null values can be passed and that map size constraints (<=10 entries) are respected.

private HttpResponse httpResponse(boolean ready, String message) {
  return new HttpResponse()
      .addHeader("Content-Type", JSON_UTF_8)
      .setContent(
          asJson(
              Map.of(
                  "value",
                  Map.of(
                      "ready", ready,
                      "message", message))));
}

Aug 19 '25 11:08 qodo-code-review[bot]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Avoid unmodifiable sets during collection Using Collections.unmodifiableSortedSet as the map value supplier prevents later additions when keys collide, forcing copies on each merge. Supply a mutable SortedSet during collection and wrap only once at the end. This reduces copying and avoids potential UnsupportedOperationException during merges. java/src/org/openqa/selenium/grid/config/ConfigFlags.java [103-113] Map<String, Set<DescribedOption>> allOptions = DescribedOption.findAllMatchingOptions(currentRoles).stream() .collect( Collectors.toMap( DescribedOption::section, - option -> Collections.unmodifiableSortedSet(new TreeSet<>(Set.of(option))), + option -> { + SortedSet<DescribedOption> set = new TreeSet<>(); + set.add(option); + return set; + }, (l, r) -> { - SortedSet<DescribedOption> merged = new TreeSet<>(l); - merged.addAll(r); - return Collections.unmodifiableSortedSet(merged); - })); + l.addAll(r); + return l; + })) + .entrySet() + .stream() + .collect(Collectors.toMap( + Map.Entry::getKey, + e -> Collections.unmodifiableSortedSet(new TreeSet<>(e.getValue())) + )); `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that creating an unmodifiable set within the `valueMapper` of `Collectors.toMap` is inefficient, as it forces the `mergeFunction` to create a new copy for every merge operation. The proposed change improves performance by collecting to mutable sets first and then wrapping them as unmodifiable in a final step.	Low
[ ] Update <!-- /improve_multi --more_suggestions=true -->

Aug 19 '25 11:08 qodo-code-review[bot]

Found an issue in a related PR. Trying to sort it out first, then accordingly update here if required.

Aug 22 '25 08:08 pujagani

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

5678 - Partially compliant

Compliant requirements:

None

Non-compliant requirements:

Investigate and resolve "Error: ConnectFailure (Connection refused)" when instantiating multiple ChromeDriver instances.
Ensure subsequent ChromeDriver instantiations do not log connection refusal errors.
Provide reproducible steps and validate behavior across runs.

Requires further human verification:

N/A

1234 - Partially compliant

Compliant requirements:

None

Non-compliant requirements:

Ensure click() triggers JavaScript in href in Firefox.
Provide/validate a regression fix.

Requires further human verification:

N/A

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Behavioral Change

The default events implementation remains "GuavaEventBus" while the PR aims to remove Guava usages. Confirm that this class is still available and intended, or update to a non-Guava implementation to avoid runtime dependency on Guava.

          "events", Map.of("implementation", "org.openqa.selenium.events.local.GuavaEventBus"),
          "sessions",
              Map.of(
                  "implementation",
                  "org.openqa.selenium.grid.sessionmap.local.LocalSessionMap")));
}

Map Copy Semantics

The constructor builds an unmodifiable map of section->values without deep-copying nested mutable maps or collections. If the input map is later mutated externally, inner objects might change. Validate callers never mutate inputs or consider defensive copying.

  Require.nonNull("Underlying map", raw);

  Map<String, Map<String, Object>> validated = new HashMap<>();

  for (Map.Entry<String, Object> entry : raw.entrySet()) {
    if (!(entry.getValue() instanceof Map)) {
      continue;
    }

    Map<String, Object> values =
        ((Map<?, ?>) entry.getValue())
            .entrySet().stream()
                .filter(e -> e.getKey() instanceof String)
                .collect(
                    Collectors.toUnmodifiableMap(
                        e -> String.valueOf(e.getKey()), Map.Entry::getValue));

    validated.put(entry.getKey(), values);
  }

  this.raw = Collections.unmodifiableMap(validated);
}

Set Ordering/Equality

Replaced Guava set operations with TreeSet and Collections.disjoint. Ensure natural ordering and equals/hashCode behavior of DescribedOption remains consistent, and that Locale/flags ordering does not affect output stability across platforms.

  Objects.requireNonNull(roles);

  Set<Role> minimized = Set.copyOf(roles);

  return StreamSupport.stream(ServiceLoader.load(HasRoles.class).spliterator(), false)
      .filter(hasRoles -> !Collections.disjoint(hasRoles.getRoles(), minimized))
      .flatMap(DescribedOption::getAllFields)
      .collect(
          Collectors.collectingAndThen(
              Collectors.toCollection(TreeSet::new), Collections::unmodifiableSortedSet));
}

Aug 22 '25 09:08 qodo-code-review[bot]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Preserve original flag order Preserve flag ordering defined by JCommander for predictable help output. Using TreeSet reorders flags lexicographically; instead use an unmodifiable List to retain the original order. java/src/org/openqa/selenium/grid/config/DescribedOption.java [83] `-this.flags = Collections.unmodifiableSortedSet(new TreeSet<>(Arrays.asList(parameter.names()))); +this.flags = Collections.unmodifiableSet( + new java.util.LinkedHashSet<>(Arrays.asList(parameter.names())));` [ ] Apply / Chat <!-- /improve --apply_suggestion=0 --> Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that using `TreeSet` changes the order of flags, and proposes using `LinkedHashSet` to preserve the original insertion order, which improves consistency in help messages.	Medium
Possible issue	Prevent unmodifiable set merge failures Avoid returning unmodifiable views during collection to prevent UnsupportedOperationException in the merge function. Build a mutable set in the collector and wrap it as unmodifiable only after collection completes. java/src/org/openqa/selenium/grid/config/ConfigFlags.java [103-113] Map<String, Set<DescribedOption>> allOptions = DescribedOption.findAllMatchingOptions(currentRoles).stream() .collect( Collectors.toMap( DescribedOption::section, - option -> Collections.unmodifiableSortedSet(new TreeSet<>(Set.of(option))), + option -> { + SortedSet<DescribedOption> set = new TreeSet<>(); + set.add(option); + return set; + }, (l, r) -> { - SortedSet<DescribedOption> merged = new TreeSet<>(l); - merged.addAll(r); - return Collections.unmodifiableSortedSet(merged); - })); + l.addAll(r); + return l; + })) + .entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, + e -> Collections.unmodifiableSortedSet(new TreeSet<>(e.getValue())) + )); `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that the merge function can be more efficient, but the justification about preventing an `UnsupportedOperationException` is incorrect as the original code creates new sets instead of modifying existing ones.	Low
[ ] More <!-- /improve --more_suggestions=true -->

Aug 22 '25 09:08 qodo-code-review[bot]

@pujagani What is the status of this PR? Can we merge it?

Dec 05 '25 13:12 asolntsev