Skip to content

Commit e602675

Browse files
VietND96VietND96
committed
[grid] Improve SlotMatcher and SlotSelector on request browserVersion
1 parent cd138fc commit e602675

File tree

6 files changed

+156
-1
lines changed

6 files changed

+156
-1
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ 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+
|| NodeStatus.semVerComparator(
86+
stereotype.getBrowserVersion(), capabilities.getBrowserVersion())
87+
== 0;
8688
boolean platformNameMatch =
8789
capabilities.getPlatformName() == null
8890
|| Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,65 @@ 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(NodeStatus::semVerComparator)
212+
.orElse("");
213+
}
214+
215+
public static int semVerComparator(String v1, String v2) {
216+
// Custom semver comparator with empty strings first
217+
if (v1.isEmpty() && v2.isEmpty()) return 0;
218+
if (v1.isEmpty()) return -1; // Empty string comes first
219+
if (v2.isEmpty()) return 1;
220+
221+
String[] parts1 = v1.split("\\.");
222+
String[] parts2 = v2.split("\\.");
223+
224+
int maxLength = Math.max(parts1.length, parts2.length);
225+
for (int i = 0; i < maxLength; i++) {
226+
String part1 = i < parts1.length ? parts1[i] : "0";
227+
String part2 = i < parts2.length ? parts2[i] : part1;
228+
229+
boolean isPart1Numeric = isNumber(part1);
230+
boolean isPart2Numeric = isNumber(part2);
231+
232+
if (isPart1Numeric && isPart2Numeric) {
233+
// Compare numerically
234+
int num1 = Integer.parseInt(part1);
235+
int num2 = Integer.parseInt(part2);
236+
if (num1 != num2) {
237+
return Integer.compare(num2, num1); // Descending order
238+
}
239+
} else if (!isPart1Numeric && !isPart2Numeric) {
240+
// Compare lexicographically, case-insensitive
241+
int result = part2.compareToIgnoreCase(part1); // Descending order
242+
if (result != 0) {
243+
return result;
244+
}
245+
} else {
246+
// Numbers take precedence over strings
247+
return isPart1Numeric ? -1 : 1;
248+
}
249+
}
250+
251+
return 0; // Versions are equal
252+
}
253+
254+
private static boolean isNumber(String str) {
255+
if (str == null || str.isEmpty()) {
256+
return false;
257+
}
258+
for (char c : str.toCharArray()) {
259+
if (!Character.isDigit(c)) {
260+
return false;
261+
}
262+
}
263+
return true;
264+
}
265+
207266
@Override
208267
public boolean equals(Object o) {
209268
if (!(o instanceof NodeStatus)) {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ public Set<SlotId> selectSlot(
5353
.thenComparingDouble(NodeStatus::getLoad)
5454
// Then last session created (oldest first), so natural ordering again
5555
.thenComparingLong(NodeStatus::getLastSessionCreated)
56+
// Then sort by stereotype browserVersion (descending order). SemVer comparison with
57+
// considering empty value at first.
58+
.thenComparing(
59+
Comparator.comparing(
60+
NodeStatus::getBrowserVersion, NodeStatus::semVerComparator))
5661
// And use the node id as a tie-breaker.
5762
.thenComparing(NodeStatus::getNodeId))
5863
.flatMap(

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,27 @@
3434

3535
class NodeStatusTest {
3636

37+
@Test
38+
void testBrowserVersionMatch() {
39+
assertThat(NodeStatus.semVerComparator("", "")).isEqualTo(0);
40+
assertThat(NodeStatus.semVerComparator("", "130.0")).isEqualTo(-1);
41+
assertThat(NodeStatus.semVerComparator("131.0.6778.85", "131")).isEqualTo(0);
42+
assertThat(NodeStatus.semVerComparator("131.0.6778.85", "131.0")).isEqualTo(0);
43+
assertThat(NodeStatus.semVerComparator("131.0.6778.85", "131.0.6778")).isEqualTo(0);
44+
assertThat(NodeStatus.semVerComparator("131.0.6778.85", "131.0.6778.95")).isEqualTo(1);
45+
assertThat(NodeStatus.semVerComparator("130.0", "130.0")).isEqualTo(0);
46+
assertThat(NodeStatus.semVerComparator("130.0", "130")).isEqualTo(0);
47+
assertThat(NodeStatus.semVerComparator("130.0.1", "130")).isEqualTo(0);
48+
assertThat(NodeStatus.semVerComparator("130.0.1", "130.0.1")).isEqualTo(0);
49+
assertThat(NodeStatus.semVerComparator("133.0a1", "133")).isEqualTo(0);
50+
assertThat(NodeStatus.semVerComparator("dev", "Dev")).isEqualTo(0);
51+
assertThat(NodeStatus.semVerComparator("Beta", "beta")).isEqualTo(0);
52+
assertThat(NodeStatus.semVerComparator("130.0.1", "130.0.2")).isEqualTo(1);
53+
assertThat(NodeStatus.semVerComparator("130.1", "130.0")).isEqualTo(-1);
54+
assertThat(NodeStatus.semVerComparator("131.0", "130.0")).isEqualTo(-1);
55+
assertThat(NodeStatus.semVerComparator("130.0", "131")).isEqualTo(1);
56+
}
57+
3758
@Test
3859
void ensureRoundTripWorks() throws URISyntaxException {
3960
ImmutableCapabilities stereotype = new ImmutableCapabilities("cheese", "brie");

‎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());

‎rust/src/lock.rs‎

Lines changed: 17 additions & 0 deletions
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)