[grid] Replace Guava list,set,map and sorted set with Java equivalent by pujagani · Pull Request #16206 · SeleniumHQ/selenium

pujagani · 2025-08-19T07:45:23Z

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

qodo-code-review · 2025-08-19T11:08:00Z

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)))); }

qodo-code-review · 2025-08-19T11:09:08Z

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

diemol

Thank you!

java/src/org/openqa/selenium/grid/commands/EventBusCommand.java

pujagani · 2025-08-22T08:44:15Z

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

qodo-code-review · 2025-08-22T09:31:38Z

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)); }

qodo-code-review · 2025-08-22T09:32:55Z

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 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

asolntsev · 2025-12-05T13:15:49Z

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

asolntsev · 2026-02-01T16:17:50Z

superseded by #16776

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

773b6ca

pujagani added the C-java Java Bindings label Aug 19, 2025

selenium-ci added the B-grid Everything grid and server related label Aug 19, 2025

Fix build

a475dc3

pujagani marked this pull request as ready for review August 19, 2025 11:07

qodo-code-review bot added the Review effort 2/5 label Aug 19, 2025

diemol requested changes Aug 19, 2025

View reviewed changes

java/src/org/openqa/selenium/grid/commands/EventBusCommand.java Outdated Show resolved Hide resolved

Fix import

c923c34

diemol added this to the Selenium 4.36 milestone Aug 19, 2025

pujagani marked this pull request as draft August 22, 2025 08:43

Update the sorted set to immutable

0b4ca83

pujagani marked this pull request as ready for review August 22, 2025 09:30

pujagani and others added 4 commits August 22, 2025 16:12

Fix formatting

9cdaebd

Merge branch 'trunk' into replace-server-guava

ab58628

Merge branch 'trunk' into replace-server-guava

2c6aec4

Merge branch 'trunk' into replace-server-guava

c1fb818

titusfortner modified the milestones: Selenium 4.36, 4.40.0 Dec 14, 2025

iampopovich mentioned this pull request Dec 22, 2025

[java] [refactor] replace most of Guava collections by Java built-in collections #16776

Merged

asolntsev removed this from the 4.40.0 milestone Jan 21, 2026

asolntsev closed this Feb 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

[grid] Replace Guava list,set,map and sorted set with Java equivalent#16206
pujagani wants to merge 8 commits intoSeleniumHQ:trunkfrom
pujagani:replace-server-guava

pujagani commented Aug 19, 2025 •

edited by qodo-code-review bot

Loading

Uh oh!

qodo-code-review bot commented Aug 19, 2025

Uh oh!

qodo-code-review bot commented Aug 19, 2025 •

edited

Loading

Uh oh!

diemol left a comment

Uh oh!

Uh oh!

pujagani commented Aug 22, 2025

Uh oh!

qodo-code-review bot commented Aug 22, 2025

Uh oh!

qodo-code-review bot commented Aug 22, 2025

Uh oh!

asolntsev commented Dec 5, 2025

Uh oh!

asolntsev commented Feb 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

pujagani commented Aug 19, 2025 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-code-review bot commented Aug 19, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review bot commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

diemol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pujagani commented Aug 22, 2025

Uh oh!

qodo-code-review bot commented Aug 22, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review bot commented Aug 22, 2025

PR Code Suggestions ✨

Uh oh!

asolntsev commented Dec 5, 2025

Uh oh!

asolntsev commented Feb 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pujagani commented Aug 19, 2025 •

edited by qodo-code-review bot

Loading

qodo-code-review bot commented Aug 19, 2025 •

edited

Loading