Skip to content

Commit dc952cd

Browse files
VietND96diemol
VietND96
and
diemol
authored
[grid] Improve SlotMatcher and SlotSelector on request browserVersion (SeleniumHQ#14914)
* [grid] Improve SlotMatcher and SlotSelector on request browserVersion Signed-off-by: Viet Nguyen Duc <[email protected]> * Fix tests Signed-off-by: Viet Nguyen Duc <[email protected]> --------- Signed-off-by: Viet Nguyen Duc <[email protected]> Co-authored-by: Diego Molina <[email protected]>
1 parent b49678d commit dc952cd

File tree

6 files changed

+169
-1
lines changed

6 files changed

+169
-1
lines changed

‎java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
8282
(capabilities.getBrowserVersion() == null
8383
|| capabilities.getBrowserVersion().isEmpty()
8484
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
85-
|| Objects.equals(stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
85+
|| browserVersionMatch(
86+
stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
8687
boolean platformNameMatch =
8788
capabilities.getPlatformName() == null
8889
|| Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
@@ -91,6 +92,10 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
9192
return browserNameMatch && browserVersionMatch && platformNameMatch;
9293
}
9394

95+
private boolean browserVersionMatch(String stereotype, String capabilities) {
96+
return new SemanticVersionComparator().compare(stereotype, capabilities) == 0;
97+
}
98+
9499
private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities) {
95100
return stereotype.getCapabilityNames().stream()
96101
// Matching of extension capabilities is implementation independent. Skip them

‎java/src/org/openqa/selenium/grid/data/NodeStatus.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,14 @@ public long getLastSessionCreated() {
204204
.orElse(0);
205205
}
206206

207+
public String getBrowserVersion() {
208+
return slots.stream()
209+
.map(slot -> slot.getStereotype().getBrowserVersion())
210+
.filter(Objects::nonNull)
211+
.max(new SemanticVersionComparator())
212+
.orElse("");
213+
}
214+
207215
@Override
208216
public boolean equals(Object o) {
209217
if (!(o instanceof NodeStatus)) {

‎java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.grid.data;
19+
20+
import java.io.Serializable;
21+
import java.util.Comparator;
22+
23+
public class SemanticVersionComparator implements Comparator<String>, Serializable {
24+
25+
@Override
26+
public int compare(String v1, String v2) {
27+
// Custom semver comparator with empty strings last
28+
if (v1.isEmpty() && v2.isEmpty()) return 0;
29+
if (v1.isEmpty()) return 1; // Empty string comes last
30+
if (v2.isEmpty()) return -1;
31+
32+
String[] parts1 = v1.split("\\.");
33+
String[] parts2 = v2.split("\\.");
34+
35+
int maxLength = Math.max(parts1.length, parts2.length);
36+
for (int i = 0; i < maxLength; i++) {
37+
String part1 = i < parts1.length ? parts1[i] : "0";
38+
String part2 = i < parts2.length ? parts2[i] : part1;
39+
40+
boolean isPart1Numeric = isNumber(part1);
41+
boolean isPart2Numeric = isNumber(part2);
42+
43+
if (isPart1Numeric && isPart2Numeric) {
44+
// Compare numerically
45+
int num1 = Integer.parseInt(part1);
46+
int num2 = Integer.parseInt(part2);
47+
if (num1 != num2) {
48+
return Integer.compare(num1, num2); // Ascending order
49+
}
50+
} else if (!isPart1Numeric && !isPart2Numeric) {
51+
// Compare lexicographically, case-insensitive
52+
int result = part1.compareToIgnoreCase(part2); // Ascending order
53+
if (result != 0) {
54+
return result;
55+
}
56+
} else {
57+
// Numbers take precedence over strings
58+
return isPart1Numeric ? 1 : -1;
59+
}
60+
}
61+
return 0; // Versions are equal
62+
}
63+
64+
private boolean isNumber(String str) {
65+
if (str == null || str.isEmpty()) {
66+
return false;
67+
}
68+
for (char c : str.toCharArray()) {
69+
if (c < '\u0030' || c > '\u0039') {
70+
return false;
71+
}
72+
}
73+
return true;
74+
}
75+
}

‎java/src/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelector.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.openqa.selenium.Capabilities;
2727
import org.openqa.selenium.grid.config.Config;
2828
import org.openqa.selenium.grid.data.NodeStatus;
29+
import org.openqa.selenium.grid.data.SemanticVersionComparator;
2930
import org.openqa.selenium.grid.data.Slot;
3031
import org.openqa.selenium.grid.data.SlotId;
3132
import org.openqa.selenium.grid.data.SlotMatcher;
@@ -54,6 +55,11 @@ public Set<SlotId> selectSlot(
5455
.thenComparingDouble(NodeStatus::getLoad)
5556
// Then last session created (oldest first), so natural ordering again
5657
.thenComparingLong(NodeStatus::getLastSessionCreated)
58+
// Then sort by stereotype browserVersion (descending order). SemVer comparison with
59+
// considering empty value at first.
60+
.thenComparing(
61+
Comparator.comparing(
62+
NodeStatus::getBrowserVersion, new SemanticVersionComparator().reversed()))
5763
// And use the node id as a tie-breaker.
5864
.thenComparing(NodeStatus::getNodeId))
5965
.flatMap(

‎java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,29 @@
2828
class DefaultSlotMatcherTest {
2929

3030
private final DefaultSlotMatcher slotMatcher = new DefaultSlotMatcher();
31+
private final SemanticVersionComparator comparator = new SemanticVersionComparator();
32+
33+
@Test
34+
void testBrowserVersionMatch() {
35+
assertThat(comparator.compare("", "")).isEqualTo(0);
36+
assertThat(comparator.compare("", "130.0")).isEqualTo(1);
37+
assertThat(comparator.compare("131.0.6778.85", "131")).isEqualTo(0);
38+
assertThat(comparator.compare("131.0.6778.85", "131.0")).isEqualTo(0);
39+
assertThat(comparator.compare("131.0.6778.85", "131.0.6778")).isEqualTo(0);
40+
assertThat(comparator.compare("131.0.6778.85", "131.0.6778.95")).isEqualTo(-1);
41+
assertThat(comparator.compare("130.0", "130.0")).isEqualTo(0);
42+
assertThat(comparator.compare("130.0", "130")).isEqualTo(0);
43+
assertThat(comparator.compare("130.0.1", "130")).isEqualTo(0);
44+
assertThat(comparator.compare("130.0.1", "130.0.1")).isEqualTo(0);
45+
assertThat(comparator.compare("133.0a1", "133")).isEqualTo(0);
46+
assertThat(comparator.compare("133", "133.0a1")).isEqualTo(1);
47+
assertThat(comparator.compare("dev", "Dev")).isEqualTo(0);
48+
assertThat(comparator.compare("Beta", "beta")).isEqualTo(0);
49+
assertThat(comparator.compare("130.0.1", "130.0.2")).isEqualTo(-1);
50+
assertThat(comparator.compare("130.1", "130.0")).isEqualTo(1);
51+
assertThat(comparator.compare("131.0", "130.0")).isEqualTo(1);
52+
assertThat(comparator.compare("130.0", "131")).isEqualTo(-1);
53+
}
3154

3255
@Test
3356
void fullMatch() {

‎java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,43 @@ void numberOfSupportedBrowsersByNodeIsCorrect() {
8787
assertThat(supportedBrowsersByNode).isEqualTo(1);
8888
}
8989

90+
@Test
91+
void nodesAreOrderedNodesByBrowserVersion() {
92+
Capabilities caps = new ImmutableCapabilities("browserName", "chrome");
93+
94+
NodeStatus node1 =
95+
createNodeWithStereotypes(
96+
Arrays.asList(
97+
ImmutableMap.of("browserName", "chrome", "browserVersion", "131.0"),
98+
ImmutableMap.of("browserName", "chrome", "browserVersion", "132.0")));
99+
NodeStatus node2 =
100+
createNodeWithStereotypes(
101+
Arrays.asList(ImmutableMap.of("browserName", "chrome", "browserVersion", "131.0")));
102+
NodeStatus node3 =
103+
createNodeWithStereotypes(
104+
Arrays.asList(ImmutableMap.of("browserName", "chrome", "browserVersion", "")));
105+
NodeStatus node4 =
106+
createNodeWithStereotypes(
107+
Arrays.asList(ImmutableMap.of("browserName", "chrome", "browserVersion", "131.1")));
108+
NodeStatus node5 =
109+
createNodeWithStereotypes(
110+
Arrays.asList(ImmutableMap.of("browserName", "chrome", "browserVersion", "beta")));
111+
Set<NodeStatus> nodes = ImmutableSet.of(node1, node2, node3, node4, node5);
112+
113+
Set<SlotId> slots = selector.selectSlot(caps, nodes, new DefaultSlotMatcher());
114+
115+
ImmutableSet<NodeId> nodeIds =
116+
slots.stream().map(SlotId::getOwningNodeId).distinct().collect(toImmutableSet());
117+
118+
assertThat(nodeIds)
119+
.containsSequence(
120+
node3.getNodeId(),
121+
node1.getNodeId(),
122+
node4.getNodeId(),
123+
node2.getNodeId(),
124+
node5.getNodeId());
125+
}
126+
90127
@Test
91128
void nodesAreOrderedNodesByNumberOfSupportedBrowsers() {
92129
Set<NodeStatus> nodes = new HashSet<>();
@@ -254,6 +291,20 @@ private NodeStatus createNode(String... browsers) {
254291
return myNode.getStatus();
255292
}
256293

294+
private NodeStatus createNodeWithStereotypes(List<ImmutableMap> stereotypes) {
295+
URI uri = createUri();
296+
LocalNode.Builder nodeBuilder =
297+
LocalNode.builder(tracer, bus, uri, uri, new Secret("cornish yarg"));
298+
nodeBuilder.maximumConcurrentSessions(stereotypes.size());
299+
stereotypes.forEach(
300+
stereotype -> {
301+
Capabilities caps = new ImmutableCapabilities(stereotype);
302+
nodeBuilder.add(caps, new TestSessionFactory((id, c) -> new Handler(c)));
303+
});
304+
Node myNode = nodeBuilder.build();
305+
return myNode.getStatus();
306+
}
307+
257308
private URI createUri() {
258309
try {
259310
return new URI("http://localhost:" + random.nextInt());

0 commit comments

Comments
 (0)