Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 7369c6a

Browse files
committedDec 19, 2024··
[grid] Improve SlotMatcher and SlotSelector on request browserVersion
Signed-off-by: Viet Nguyen Duc <[email protected]>
1 parent 825b040 commit 7369c6a

File tree

7 files changed

+187
-1
lines changed

7 files changed

+187
-1
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
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+
public class CapabilitiesComparator {
21+
22+
private CapabilitiesComparator() {
23+
// Utility methods
24+
}
25+
26+
public static int browserVersionComparator(String v1, String v2) {
27+
// Custom semver comparator with empty strings first
28+
if (v1.isEmpty() && v2.isEmpty()) return 0;
29+
if (v1.isEmpty()) return -1; // Empty string comes first
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(num2, num1); // Descending order
49+
}
50+
} else if (!isPart1Numeric && !isPart2Numeric) {
51+
// Compare lexicographically, case-insensitive
52+
int result = part2.compareToIgnoreCase(part1); // Descending 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+
62+
return 0; // Versions are equal
63+
}
64+
65+
private static boolean isNumber(String str) {
66+
if (str == null || str.isEmpty()) {
67+
return false;
68+
}
69+
for (char c : str.toCharArray()) {
70+
if (!Character.isDigit(c)) {
71+
return false;
72+
}
73+
}
74+
return true;
75+
}
76+
}

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

+6-1
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 CapabilitiesComparator.browserVersionComparator(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

+8
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.parallelStream()
209+
.map(slot -> slot.getStereotype().getBrowserVersion())
210+
.filter(Objects::nonNull)
211+
.min(CapabilitiesComparator::browserVersionComparator)
212+
.orElse("");
213+
}
214+
207215
@Override
208216
public boolean equals(Object o) {
209217
if (!(o instanceof NodeStatus)) {

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

+7
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Set;
2525
import org.openqa.selenium.Capabilities;
2626
import org.openqa.selenium.grid.config.Config;
27+
import org.openqa.selenium.grid.data.CapabilitiesComparator;
2728
import org.openqa.selenium.grid.data.NodeStatus;
2829
import org.openqa.selenium.grid.data.Slot;
2930
import org.openqa.selenium.grid.data.SlotId;
@@ -53,6 +54,12 @@ public Set<SlotId> selectSlot(
5354
.thenComparingDouble(NodeStatus::getLoad)
5455
// Then last session created (oldest first), so natural ordering again
5556
.thenComparingLong(NodeStatus::getLastSessionCreated)
57+
// Then sort by stereotype browserVersion (descending order). SemVer comparison with
58+
// considering empty value at first.
59+
.thenComparing(
60+
Comparator.comparing(
61+
NodeStatus::getBrowserVersion,
62+
CapabilitiesComparator::browserVersionComparator))
5663
// And use the node id as a tie-breaker.
5764
.thenComparing(NodeStatus::getNodeId))
5865
.flatMap(

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

+22
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.openqa.selenium.grid.data;
1919

2020
import static org.assertj.core.api.Assertions.assertThat;
21+
import static org.openqa.selenium.grid.data.CapabilitiesComparator.browserVersionComparator;
2122

2223
import org.junit.jupiter.api.Test;
2324
import org.openqa.selenium.Capabilities;
@@ -29,6 +30,27 @@ class DefaultSlotMatcherTest {
2930

3031
private final DefaultSlotMatcher slotMatcher = new DefaultSlotMatcher();
3132

33+
@Test
34+
void testBrowserVersionMatch() {
35+
assertThat(browserVersionComparator("", "")).isEqualTo(0);
36+
assertThat(browserVersionComparator("", "130.0")).isEqualTo(-1);
37+
assertThat(browserVersionComparator("131.0.6778.85", "131")).isEqualTo(0);
38+
assertThat(browserVersionComparator("131.0.6778.85", "131.0")).isEqualTo(0);
39+
assertThat(browserVersionComparator("131.0.6778.85", "131.0.6778")).isEqualTo(0);
40+
assertThat(browserVersionComparator("131.0.6778.85", "131.0.6778.95")).isEqualTo(1);
41+
assertThat(browserVersionComparator("130.0", "130.0")).isEqualTo(0);
42+
assertThat(browserVersionComparator("130.0", "130")).isEqualTo(0);
43+
assertThat(browserVersionComparator("130.0.1", "130")).isEqualTo(0);
44+
assertThat(browserVersionComparator("130.0.1", "130.0.1")).isEqualTo(0);
45+
assertThat(browserVersionComparator("133.0a1", "133")).isEqualTo(0);
46+
assertThat(browserVersionComparator("dev", "Dev")).isEqualTo(0);
47+
assertThat(browserVersionComparator("Beta", "beta")).isEqualTo(0);
48+
assertThat(browserVersionComparator("130.0.1", "130.0.2")).isEqualTo(1);
49+
assertThat(browserVersionComparator("130.1", "130.0")).isEqualTo(-1);
50+
assertThat(browserVersionComparator("131.0", "130.0")).isEqualTo(-1);
51+
assertThat(browserVersionComparator("130.0", "131")).isEqualTo(1);
52+
}
53+
3254
@Test
3355
void fullMatch() {
3456
Capabilities stereotype =

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

+51
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());

‎rust/src/lock.rs

+17
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
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+
118
use crate::logger::Logger;
219
use anyhow::Error;
320
use std::fs::File;

0 commit comments

Comments
 (0)
Please sign in to comment.