Skip to content

Commit

Permalink
Add explicit delimiters to node configs list (#12444)
Browse files Browse the repository at this point in the history
* Add explicit delimiter to node configs list

* Remove sorting of flattened map entries
  • Loading branch information
sbabcoc committed Aug 23, 2023
1 parent d7e1f84 commit d75d171
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 147 deletions.
23 changes: 16 additions & 7 deletions java/src/org/openqa/selenium/grid/config/Config.java
Expand Up @@ -23,6 +23,8 @@
import java.util.Optional;
import java.util.Set;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import org.openqa.selenium.json.Json;

public interface Config {
Expand Down Expand Up @@ -59,14 +61,21 @@ default <X> X getClass(String section, String option, Class<X> typeOfClass, Stri
}
}

String DELIM_KEY = "\u001E";
String DELIMITER = DELIM_KEY + "=\"record-separator\"";

default List<String> toEntryList(Map<String, Object> mapItem) {
return mapItem.entrySet().stream()
.map(
entry -> {
return String.format("%s=%s", entry.getKey(), toJson(entry.getValue()));
})
.sorted()
.collect(ImmutableList.toImmutableList());
// transform config settings map into list of key/value strings
List<String> entryList = mapItem.entrySet().stream()
.map(
entry -> {
return String.format("%s=%s", entry.getKey(), toJson(entry.getValue()));
})
.collect(Collectors.toList());
// add record separator
entryList.add(DELIMITER);
// return immutable config settings list
return ImmutableList.<String>builder().addAll(entryList).build();
}

default String toJson(Object value) {
Expand Down
243 changes: 112 additions & 131 deletions java/src/org/openqa/selenium/grid/node/config/NodeOptions.java
Expand Up @@ -344,143 +344,124 @@ private SessionFactory createSessionFactory(String clazz, Capabilities stereotyp
private void addDriverConfigs(
Function<ImmutableCapabilities, Collection<SessionFactory>> factoryFactory,
ImmutableMultimap.Builder<Capabilities, SessionFactory> sessionFactories) {
Multimap<WebDriverInfo, SessionFactory> driverConfigs = HashMultimap.create();
config
.getAll(NODE_SECTION, "driver-configuration")
.ifPresent(
drivers -> {
/*
The four accepted keys are: display-name, max-sessions, stereotype, webdriver-executable.
The mandatory keys are display-name and stereotype. When configs are read, they keys always
come alphabetically ordered. This means that we know a new config is present when we find
the "display-name" key again.
*/

if (drivers.size() == 0) {
throw new ConfigException("No driver configs were found!");
}

drivers.stream()
.filter(driver -> !driver.contains("="))
.peek(
driver ->
LOG.warning(
driver
+ " does not have the required 'key=value' "
+ "structure for the configuration"))
.findFirst()
.ifPresent(
ignore -> {
throw new ConfigException(
"One or more driver configs does not have the "
+ "required 'key=value' structure");
});

// Find all indexes where "display-name" is present, as it marks the start of a config
int[] configIndexes =
IntStream.range(0, drivers.size())
.filter(index -> drivers.get(index).startsWith("display-name"))
.toArray();

if (configIndexes.length == 0) {
throw new ConfigException(
"No 'display-name' keyword was found in the provided configs!");
}

List<Map<String, String>> driversMap = new ArrayList<>();
for (int i = 0; i < configIndexes.length; i++) {
int fromIndex = configIndexes[i];
int toIndex =
(i + 1) >= configIndexes.length ? drivers.size() : configIndexes[i + 1];
Map<String, String> configMap = new HashMap<>();
drivers
.subList(fromIndex, toIndex)
.forEach(
keyValue -> {
String[] values = keyValue.split("=", 2);
configMap.put(values[0], unquote(values[1]));
});
driversMap.add(configMap);
}
Multimap<WebDriverInfo, SessionFactory> driverConfigs = HashMultimap.create();

List<DriverService.Builder<?, ?>> builders = new ArrayList<>();
ServiceLoader.load(DriverService.Builder.class).forEach(builders::add);
// get all driver configuration settings
config.getAll(NODE_SECTION, "driver-configuration")
// if settings exist
.ifPresent(drivers -> {
Map<String, String> configMap = new HashMap<>();
List<Map<String, String>> configList = new ArrayList<>();

// iterate over driver settings
for (String setting : drivers) {
// split this setting into key/value pair
String[] values = setting.split("=", 2);
// if format is invalid
if (values.length != 2) {
throw new ConfigException("Driver setting '" + setting
+ "' does not adhere to the required 'key=value' format!");
}
// if this is a record separator
if (values[0].equals(Config.DELIM_KEY)) {
// if config lacks settings
if (configMap.isEmpty()) {
throw new ConfigException("Found config delimiter with no preceding settings!");
}

// if config lacks 'display-name' setting
if (!configMap.containsKey("display-name")) {
throw new ConfigException("Found config with no 'display-name' setting! " + configMap);
}

// if config lacks 'stereotype' setting
if (!configMap.containsKey("stereotype")) {
throw new ConfigException("Found config with no 'stereotype' setting! " + configMap);
}

// add config to list
configList.add(configMap);
// prepare for next config
configMap = new HashMap<>();
} else {
// add setting to config
configMap.put(values[0], unquote(values[1]));
}
}

List<WebDriverInfo> infos = new ArrayList<>();
ServiceLoader.load(WebDriverInfo.class).forEach(infos::add);
// if no configs were found
if (configList.isEmpty()) {
throw new ConfigException("No driver configs were found!");
}

driversMap.forEach(
configMap -> {
if (!configMap.containsKey("stereotype")) {
throw new ConfigException(
"Driver config is missing stereotype value. " + configMap);
}
List<DriverService.Builder<?, ?>> builderList = new ArrayList<>();
ServiceLoader.load(DriverService.Builder.class).forEach(builderList::add);

List<WebDriverInfo> infoList = new ArrayList<>();
ServiceLoader.load(WebDriverInfo.class).forEach(infoList::add);

// iterate over driver configs
configList.forEach(thisConfig -> {
// create Capabilities object from stereotype of this config
Capabilities confStereotype = JSON.toType(thisConfig.get("stereotype"), Capabilities.class);

// extract driver executable path from this config
String webDriverExecutablePath = thisConfig.get("webdriver-executable");
// if executable path is specified
if (null != webDriverExecutablePath) {
// create File object from executable path string
File webDriverExecutable = new File(webDriverExecutablePath);
// if specified path isn't a file
if (!webDriverExecutable.isFile()) {
LOG.warning("Driver executable does not seem to be a file! " + webDriverExecutablePath);
}

// if specified path isn't executable
if (!webDriverExecutable.canExecute()) {
LOG.warning("Driver file exists but does not seem to be a executable! " + webDriverExecutablePath);
}

// add specified driver executable path to capabilities
confStereotype = new PersistentCapabilities(confStereotype)
.setCapability("se:webDriverExecutable", webDriverExecutablePath);
}

Capabilities stereotype = enhanceStereotype(confStereotype);
String configName = thisConfig.getOrDefault("display-name", "Custom Slot Config");

WebDriverInfo info = infoList.stream()
.filter(webDriverInfo -> webDriverInfo.isSupporting(stereotype))
.findFirst()
.orElseThrow(() ->
new ConfigException("Unable to find matching driver for %s", stereotype));

int driverMaxSessions = Integer.parseInt(thisConfig.getOrDefault(
"max-sessions", String.valueOf(info.getMaximumSimultaneousSessions())));
Require.positive("Driver max sessions", driverMaxSessions);

WebDriverInfo driverInfoConfig = createConfiguredDriverInfo(info, stereotype, configName);

builderList.stream()
.filter(builder -> builder.score(stereotype) > 0)
.max(Comparator.comparingInt(builder -> builder.score(stereotype)))
.ifPresent(builder -> {
ImmutableCapabilities immutable = new ImmutableCapabilities(stereotype);
int maxDriverSessions = getDriverMaxSessions(info, driverMaxSessions);
for (int i = 0; i < maxDriverSessions; i++) {
driverConfigs.putAll(driverInfoConfig, factoryFactory.apply(immutable));
}
}
);
}
);
}
);

Capabilities confStereotype =
JSON.toType(configMap.get("stereotype"), Capabilities.class);
if (configMap.containsKey("webdriver-executable")) {
String webDriverExecutablePath =
configMap.getOrDefault("webdriver-executable", "");
File webDriverExecutable = new File(webDriverExecutablePath);
if (!webDriverExecutable.isFile()) {
LOG.warning(
"Driver executable does not seem to be a file! "
+ webDriverExecutablePath);
}
if (!webDriverExecutable.canExecute()) {
LOG.warning(
"Driver file exists but does not seem to be a executable! "
+ webDriverExecutablePath);
}
confStereotype =
new PersistentCapabilities(confStereotype)
.setCapability("se:webDriverExecutable", webDriverExecutablePath);
}
Capabilities stereotype = enhanceStereotype(confStereotype);

String configName =
configMap.getOrDefault("display-name", "Custom Slot Config");

WebDriverInfo info =
infos.stream()
.filter(webDriverInfo -> webDriverInfo.isSupporting(stereotype))
.findFirst()
.orElseThrow(
() ->
new ConfigException(
"Unable to find matching driver for %s", stereotype));

int driverMaxSessions =
Integer.parseInt(
configMap.getOrDefault(
"max-sessions",
String.valueOf(info.getMaximumSimultaneousSessions())));
Require.positive("Driver max sessions", driverMaxSessions);

WebDriverInfo driverInfoConfig =
createConfiguredDriverInfo(info, stereotype, configName);

builders.stream()
.filter(builder -> builder.score(stereotype) > 0)
.max(Comparator.comparingInt(builder -> builder.score(stereotype)))
.ifPresent(
builder -> {
ImmutableCapabilities immutable =
new ImmutableCapabilities(stereotype);
int maxDriverSessions = getDriverMaxSessions(info, driverMaxSessions);
for (int i = 0; i < maxDriverSessions; i++) {
driverConfigs.putAll(
driverInfoConfig, factoryFactory.apply(immutable));
}
});
});
});
driverConfigs.asMap().entrySet().stream()
.peek(this::report)
.forEach(
entry ->
sessionFactories.putAll(
entry.getKey().getCanonicalCapabilities(), entry.getValue()));
.peek(this::report)
.forEach(entry ->
sessionFactories.putAll(entry.getKey().getCanonicalCapabilities(), entry.getValue()));
}

private void addDetectedDrivers(
Expand Down
7 changes: 4 additions & 3 deletions java/test/org/openqa/selenium/grid/config/JsonConfigTest.java
Expand Up @@ -83,8 +83,8 @@ void shouldContainConfigFromArrayOfTables() {

List<String> expected =
Arrays.asList(
"default=\"brie\"", "name=\"soft cheese\"",
"default=\"Emmental\"", "name=\"Medium-hard cheese\"");
"name=\"soft cheese\"", "default=\"brie\"", Config.DELIMITER,
"name=\"Medium-hard cheese\"", "default=\"Emmental\"", Config.DELIMITER);
assertThat(config.getAll("cheeses", "type").orElse(Collections.emptyList()))
.isEqualTo(expected);
assertThat(config.getAll("cheeses", "type").orElse(Collections.emptyList()).subList(0, 2))
Expand Down Expand Up @@ -116,7 +116,8 @@ void ensureCanReadListOfMaps() {
List<String> expected =
Arrays.asList(
"display-name=\"htmlunit\"",
"stereotype={\"browserName\": \"htmlunit\",\"browserVersion\": \"chrome\"}");
"stereotype={\"browserName\": \"htmlunit\",\"browserVersion\": \"chrome\"}",
Config.DELIMITER);
Optional<List<String>> content = config.getAll("node", "driver-configuration");
assertThat(content).isEqualTo(Optional.of(expected));
}
Expand Down
7 changes: 4 additions & 3 deletions java/test/org/openqa/selenium/grid/config/MapConfigTest.java
Expand Up @@ -68,8 +68,8 @@ void shouldContainConfigFromArrayOfTables() {

List<String> expected =
Arrays.asList(
"default=\"brie\"", "name=\"soft cheese\"",
"default=\"Emmental\"", "name=\"Medium-hard cheese\"");
"name=\"soft cheese\"", "default=\"brie\"", Config.DELIMITER,
"name=\"Medium-hard cheese\"", "default=\"Emmental\"", Config.DELIMITER);
assertThat(config.getAll("cheeses", "type").orElse(Collections.emptyList()))
.isEqualTo(expected);
assertThat(config.getAll("cheeses", "type").orElse(Collections.emptyList()).subList(0, 2))
Expand Down Expand Up @@ -123,7 +123,8 @@ void ensureCanReadListOfMaps() {
List<String> expected =
Arrays.asList(
"display-name=\"htmlunit\"",
"stereotype={\"browserName\": \"htmlunit\",\"browserVersion\": \"chrome\"}");
"stereotype={\"browserName\": \"htmlunit\",\"browserVersion\": \"chrome\"}",
Config.DELIMITER);
Optional<List<String>> content = config.getAll("node", "driver-configuration");
assertThat(content).isEqualTo(Optional.of(expected));
}
Expand Down
7 changes: 4 additions & 3 deletions java/test/org/openqa/selenium/grid/config/TomlConfigTest.java
Expand Up @@ -55,8 +55,8 @@ void shouldContainConfigFromArrayOfTables() {

List<String> expected =
Arrays.asList(
"default=\"brie\"", "name=\"soft cheese\"",
"default=\"Emmental\"", "name=\"Medium-hard cheese\"");
"default=\"brie\"", "name=\"soft cheese\"", Config.DELIMITER,
"default=\"Emmental\"", "name=\"Medium-hard cheese\"", Config.DELIMITER);
assertThat(config.getAll("cheeses", "type").orElse(Collections.emptyList()))
.isEqualTo(expected);
assertThat(config.getAll("cheeses", "type").orElse(Collections.emptyList()).subList(0, 2))
Expand Down Expand Up @@ -89,7 +89,8 @@ void ensureCanReadListOfMaps() {
List<String> expected =
Arrays.asList(
"display-name=\"htmlunit\"",
"stereotype={\"browserVersion\": \"chrome\",\"browserName\": \"htmlunit\"}");
"stereotype={\"browserVersion\": \"chrome\",\"browserName\": \"htmlunit\"}",
Config.DELIMITER);
Optional<List<String>> content = config.getAll("node", "driver-configuration");
assertThat(content).isEqualTo(Optional.of(expected));
}
Expand Down

0 comments on commit d75d171

Please sign in to comment.