From d5a0fc662b91782189d91caa216969143ea5dea6 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Thu, 19 Dec 2024 02:16:21 +0700 Subject: [PATCH 1/2] [grid] Improve SlotMatcher and SlotSelector on request browserVersion Signed-off-by: Viet Nguyen Duc --- .../grid/data/DefaultSlotMatcher.java | 7 +- .../openqa/selenium/grid/data/NodeStatus.java | 8 ++ .../grid/data/SemanticVersionComparator.java | 74 +++++++++++++++++++ .../selector/DefaultSlotSelector.java | 6 ++ .../grid/data/DefaultSlotMatcherTest.java | 23 ++++++ .../selector/DefaultSlotSelectorTest.java | 51 +++++++++++++ rust/src/lock.rs | 17 +++++ 7 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java diff --git a/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java b/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java index 594f1061dd6c6..3db617bbd5335 100644 --- a/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java +++ b/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java @@ -82,7 +82,8 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) { (capabilities.getBrowserVersion() == null || capabilities.getBrowserVersion().isEmpty() || Objects.equals(capabilities.getBrowserVersion(), "stable")) - || Objects.equals(stereotype.getBrowserVersion(), capabilities.getBrowserVersion()); + || browserVersionMatch( + stereotype.getBrowserVersion(), capabilities.getBrowserVersion()); boolean platformNameMatch = capabilities.getPlatformName() == null || Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName()) @@ -91,6 +92,10 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) { return browserNameMatch && browserVersionMatch && platformNameMatch; } + private boolean browserVersionMatch(String stereotype, String capabilities) { + return new SemanticVersionComparator().compare(stereotype, capabilities) == 0; + } + private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities) { return stereotype.getCapabilityNames().stream() // Matching of extension capabilities is implementation independent. Skip them diff --git a/java/src/org/openqa/selenium/grid/data/NodeStatus.java b/java/src/org/openqa/selenium/grid/data/NodeStatus.java index c9ce38614fcea..ec969ef4163e4 100644 --- a/java/src/org/openqa/selenium/grid/data/NodeStatus.java +++ b/java/src/org/openqa/selenium/grid/data/NodeStatus.java @@ -204,6 +204,14 @@ public long getLastSessionCreated() { .orElse(0); } + public String getBrowserVersion() { + return slots.stream() + .map(slot -> slot.getStereotype().getBrowserVersion()) + .filter(Objects::nonNull) + .max(new SemanticVersionComparator()) + .orElse(""); + } + @Override public boolean equals(Object o) { if (!(o instanceof NodeStatus)) { diff --git a/java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java b/java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java new file mode 100644 index 0000000000000..bc1b344b47ffd --- /dev/null +++ b/java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java @@ -0,0 +1,74 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.openqa.selenium.grid.data; + +import java.util.Comparator; + +public class SemanticVersionComparator implements Comparator { + + @Override + public int compare(String v1, String v2) { + // Custom semver comparator with empty strings last + if (v1.isEmpty() && v2.isEmpty()) return 0; + if (v1.isEmpty()) return 1; // Empty string comes last + if (v2.isEmpty()) return -1; + + String[] parts1 = v1.split("\\."); + String[] parts2 = v2.split("\\."); + + int maxLength = Math.max(parts1.length, parts2.length); + for (int i = 0; i < maxLength; i++) { + String part1 = i < parts1.length ? parts1[i] : "0"; + String part2 = i < parts2.length ? parts2[i] : part1; + + boolean isPart1Numeric = isNumber(part1); + boolean isPart2Numeric = isNumber(part2); + + if (isPart1Numeric && isPart2Numeric) { + // Compare numerically + int num1 = Integer.parseInt(part1); + int num2 = Integer.parseInt(part2); + if (num1 != num2) { + return Integer.compare(num1, num2); // Ascending order + } + } else if (!isPart1Numeric && !isPart2Numeric) { + // Compare lexicographically, case-insensitive + int result = part1.compareToIgnoreCase(part2); // Ascending order + if (result != 0) { + return result; + } + } else { + // Numbers take precedence over strings + return isPart1Numeric ? 1 : -1; + } + } + return 0; // Versions are equal + } + + private boolean isNumber(String str) { + if (str == null || str.isEmpty()) { + return false; + } + for (char c : str.toCharArray()) { + if (c < '\u0030' || c > '\u0039') { + return false; + } + } + return true; + } +} diff --git a/java/src/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelector.java b/java/src/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelector.java index 37c583a259dc2..9622b1d6af710 100644 --- a/java/src/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelector.java +++ b/java/src/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelector.java @@ -26,6 +26,7 @@ import org.openqa.selenium.Capabilities; import org.openqa.selenium.grid.config.Config; import org.openqa.selenium.grid.data.NodeStatus; +import org.openqa.selenium.grid.data.SemanticVersionComparator; import org.openqa.selenium.grid.data.Slot; import org.openqa.selenium.grid.data.SlotId; import org.openqa.selenium.grid.data.SlotMatcher; @@ -54,6 +55,11 @@ public Set selectSlot( .thenComparingDouble(NodeStatus::getLoad) // Then last session created (oldest first), so natural ordering again .thenComparingLong(NodeStatus::getLastSessionCreated) + // Then sort by stereotype browserVersion (descending order). SemVer comparison with + // considering empty value at first. + .thenComparing( + Comparator.comparing( + NodeStatus::getBrowserVersion, new SemanticVersionComparator().reversed())) // And use the node id as a tie-breaker. .thenComparing(NodeStatus::getNodeId)) .flatMap( diff --git a/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java b/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java index 42140dca9ef7e..ef6156d9cde99 100644 --- a/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java +++ b/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java @@ -28,6 +28,29 @@ class DefaultSlotMatcherTest { private final DefaultSlotMatcher slotMatcher = new DefaultSlotMatcher(); + private final SemanticVersionComparator comparator = new SemanticVersionComparator(); + + @Test + void testBrowserVersionMatch() { + assertThat(comparator.compare("", "")).isEqualTo(0); + assertThat(comparator.compare("", "130.0")).isEqualTo(1); + assertThat(comparator.compare("131.0.6778.85", "131")).isEqualTo(0); + assertThat(comparator.compare("131.0.6778.85", "131.0")).isEqualTo(0); + assertThat(comparator.compare("131.0.6778.85", "131.0.6778")).isEqualTo(0); + assertThat(comparator.compare("131.0.6778.85", "131.0.6778.95")).isEqualTo(-1); + assertThat(comparator.compare("130.0", "130.0")).isEqualTo(0); + assertThat(comparator.compare("130.0", "130")).isEqualTo(0); + assertThat(comparator.compare("130.0.1", "130")).isEqualTo(0); + assertThat(comparator.compare("130.0.1", "130.0.1")).isEqualTo(0); + assertThat(comparator.compare("133.0a1", "133")).isEqualTo(0); + assertThat(comparator.compare("133", "133.0a1")).isEqualTo(1); + assertThat(comparator.compare("dev", "Dev")).isEqualTo(0); + assertThat(comparator.compare("Beta", "beta")).isEqualTo(0); + assertThat(comparator.compare("130.0.1", "130.0.2")).isEqualTo(-1); + assertThat(comparator.compare("130.1", "130.0")).isEqualTo(1); + assertThat(comparator.compare("131.0", "130.0")).isEqualTo(1); + assertThat(comparator.compare("130.0", "131")).isEqualTo(-1); + } @Test void fullMatch() { diff --git a/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java b/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java index d05e9d1c5f81b..df3eecbd5d53a 100644 --- a/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java +++ b/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java @@ -87,6 +87,43 @@ void numberOfSupportedBrowsersByNodeIsCorrect() { assertThat(supportedBrowsersByNode).isEqualTo(1); } + @Test + void nodesAreOrderedNodesByBrowserVersion() { + Capabilities caps = new ImmutableCapabilities("browserName", "chrome"); + + NodeStatus node1 = + createNodeWithStereotypes( + Arrays.asList( + ImmutableMap.of("browserName", "chrome", "browserVersion", "131.0"), + ImmutableMap.of("browserName", "chrome", "browserVersion", "132.0"))); + NodeStatus node2 = + createNodeWithStereotypes( + Arrays.asList(ImmutableMap.of("browserName", "chrome", "browserVersion", "131.0"))); + NodeStatus node3 = + createNodeWithStereotypes( + Arrays.asList(ImmutableMap.of("browserName", "chrome", "browserVersion", ""))); + NodeStatus node4 = + createNodeWithStereotypes( + Arrays.asList(ImmutableMap.of("browserName", "chrome", "browserVersion", "131.1"))); + NodeStatus node5 = + createNodeWithStereotypes( + Arrays.asList(ImmutableMap.of("browserName", "chrome", "browserVersion", "beta"))); + Set nodes = ImmutableSet.of(node1, node2, node3, node4, node5); + + Set slots = selector.selectSlot(caps, nodes, new DefaultSlotMatcher()); + + ImmutableSet nodeIds = + slots.stream().map(SlotId::getOwningNodeId).distinct().collect(toImmutableSet()); + + assertThat(nodeIds) + .containsSequence( + node3.getNodeId(), + node1.getNodeId(), + node4.getNodeId(), + node2.getNodeId(), + node5.getNodeId()); + } + @Test void nodesAreOrderedNodesByNumberOfSupportedBrowsers() { Set nodes = new HashSet<>(); @@ -254,6 +291,20 @@ private NodeStatus createNode(String... browsers) { return myNode.getStatus(); } + private NodeStatus createNodeWithStereotypes(List stereotypes) { + URI uri = createUri(); + LocalNode.Builder nodeBuilder = + LocalNode.builder(tracer, bus, uri, uri, new Secret("cornish yarg")); + nodeBuilder.maximumConcurrentSessions(stereotypes.size()); + stereotypes.forEach( + stereotype -> { + Capabilities caps = new ImmutableCapabilities(stereotype); + nodeBuilder.add(caps, new TestSessionFactory((id, c) -> new Handler(c))); + }); + Node myNode = nodeBuilder.build(); + return myNode.getStatus(); + } + private URI createUri() { try { return new URI("http://localhost:" + random.nextInt()); diff --git a/rust/src/lock.rs b/rust/src/lock.rs index dfb987acbf82b..cd074b94ceab0 100644 --- a/rust/src/lock.rs +++ b/rust/src/lock.rs @@ -1,3 +1,20 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + use crate::logger::Logger; use anyhow::Error; use std::fs::File; From 32752726791de7efda65bf2047c7504a6503b19c Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Thu, 26 Dec 2024 21:18:03 +0700 Subject: [PATCH 2/2] Fix tests Signed-off-by: Viet Nguyen Duc --- .../openqa/selenium/grid/data/SemanticVersionComparator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java b/java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java index bc1b344b47ffd..794e356896753 100644 --- a/java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java +++ b/java/src/org/openqa/selenium/grid/data/SemanticVersionComparator.java @@ -17,9 +17,10 @@ package org.openqa.selenium.grid.data; +import java.io.Serializable; import java.util.Comparator; -public class SemanticVersionComparator implements Comparator { +public class SemanticVersionComparator implements Comparator, Serializable { @Override public int compare(String v1, String v2) {