Skip to content

Commit 3bebd42

Browse files
sbabcocsbabcoc
authored
Add supports for list-of-table options (#12363)
1 parent 276240d commit 3bebd42

9 files changed

Lines changed: 331 additions & 27 deletions

File tree

‎java/src/org/openqa/selenium/grid/config/Config.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717

1818
package org.openqa.selenium.grid.config;
1919

20+
import com.google.common.collect.ImmutableList;
21+
import org.openqa.selenium.json.Json;
22+
2023
import java.util.List;
24+
import java.util.Map;
2125
import java.util.Optional;
2226
import java.util.Set;
2327
import java.util.logging.Logger;
@@ -55,4 +59,19 @@ default <X> X getClass(String section, String option, Class<X> typeOfClass, Stri
5559
throw new IllegalArgumentException("Unable to find class: " + clazz, e);
5660
}
5761
}
62+
63+
default List<String> toEntryList(Map<String, Object> mapItem) {
64+
return mapItem.entrySet().stream()
65+
.map(entry -> {
66+
return String.format("%s=%s", entry.getKey(), toJson(entry.getValue()));
67+
})
68+
.sorted()
69+
.collect(ImmutableList.toImmutableList());
70+
}
71+
72+
default String toJson(Object value) {
73+
StringBuilder jsonStr = new StringBuilder();
74+
new Json().newOutput(jsonStr).setPrettyPrint(false).write(value);
75+
return jsonStr.toString();
76+
}
5877
}

‎java/src/org/openqa/selenium/grid/config/MapConfig.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@
2121
import com.google.common.collect.ImmutableMap;
2222
import com.google.common.collect.ImmutableSet;
2323
import com.google.common.collect.ImmutableSortedSet;
24+
import java.util.Collection;
2425
import java.util.List;
2526
import java.util.Map;
2627
import java.util.Optional;
2728
import java.util.Set;
29+
2830
import org.openqa.selenium.internal.Require;
2931

3032
public class MapConfig implements Config {
@@ -65,7 +67,33 @@ public Optional<List<String>> getAll(String section, String option) {
6567
}
6668

6769
Object value = rawSection.get(option);
68-
return value == null ? Optional.empty() : Optional.of(ImmutableList.of(String.valueOf(value)));
70+
if (value == null) {
71+
return Optional.empty();
72+
}
73+
74+
if (value instanceof Collection) {
75+
Collection<?> collection = (Collection<?>) value;
76+
// Case when an array of map is used as config
77+
if (collection.stream().anyMatch(item -> item instanceof Map)) {
78+
return Optional.of(collection.stream()
79+
.map(item -> (Map<String, Object>) item)
80+
.map(this::toEntryList)
81+
.flatMap(Collection::stream)
82+
.collect(ImmutableList.toImmutableList()));
83+
}
84+
85+
return Optional.of(
86+
collection.stream()
87+
.filter(item -> (!(item instanceof Collection)))
88+
.map(String::valueOf)
89+
.collect(ImmutableList.toImmutableList()));
90+
}
91+
92+
if (value instanceof Map) {
93+
return Optional.of(toEntryList((Map<String, Object>) value));
94+
}
95+
96+
return Optional.of(ImmutableList.of(String.valueOf(value)));
6997
}
7098

7199
@Override

‎java/src/org/openqa/selenium/grid/config/TomlConfig.java‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,16 @@
2323
import io.ous.jtoml.ParseException;
2424
import io.ous.jtoml.Toml;
2525
import io.ous.jtoml.TomlTable;
26+
import org.openqa.selenium.internal.Require;
27+
2628
import java.io.IOException;
2729
import java.io.Reader;
2830
import java.nio.file.Files;
2931
import java.nio.file.Path;
30-
import java.util.ArrayList;
3132
import java.util.Collection;
3233
import java.util.List;
3334
import java.util.Optional;
3435
import java.util.Set;
35-
import org.openqa.selenium.internal.Require;
3636

3737
public class TomlConfig implements Config {
3838

@@ -87,24 +87,24 @@ public Optional<List<String>> getAll(String section, String option) {
8787
// Case when an array of tables is used as config
8888
// https://toml.io/en/v1.0.0-rc.3#array-of-tables
8989
if (collection.stream().anyMatch(item -> item instanceof TomlTable)) {
90-
List<String> toReturn = new ArrayList<>();
91-
collection.stream()
90+
return Optional.of(
91+
collection.stream()
9292
.map(item -> (TomlTable) item)
93-
.forEach(
94-
tomlTable ->
95-
tomlTable.toMap().entrySet().stream()
96-
.map(entry -> String.format("%s=%s", entry.getKey(), entry.getValue()))
97-
.sorted()
98-
.forEach(toReturn::add));
99-
return Optional.of(toReturn);
93+
.map(TomlTable::toMap)
94+
.map(this::toEntryList)
95+
.flatMap(Collection::stream)
96+
.collect(ImmutableList.toImmutableList()));
10097
}
101-
List<String> values =
102-
collection.stream()
103-
.filter(item -> (!(item instanceof Collection)))
104-
.map(String::valueOf)
105-
.collect(ImmutableList.toImmutableList());
10698

107-
return Optional.of(values);
99+
return Optional.of(
100+
collection.stream()
101+
.filter(item -> (!(item instanceof Collection)))
102+
.map(String::valueOf)
103+
.collect(ImmutableList.toImmutableList()));
104+
}
105+
106+
if (value instanceof TomlTable) {
107+
return Optional.of(toEntryList(((TomlTable) value).toMap()));
108108
}
109109

110110
return Optional.of(ImmutableList.of(String.valueOf(value)));

‎java/src/org/openqa/selenium/grid/node/config/NodeOptions.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.common.collect.ImmutableSet;
2525
import com.google.common.collect.Multimap;
2626
import java.io.File;
27+
import java.io.StringReader;
2728
import java.lang.reflect.Method;
2829
import java.lang.reflect.Modifier;
2930
import java.net.URI;
@@ -57,6 +58,7 @@
5758
import org.openqa.selenium.grid.node.SessionFactory;
5859
import org.openqa.selenium.internal.Require;
5960
import org.openqa.selenium.json.Json;
61+
import org.openqa.selenium.json.JsonInput;
6062
import org.openqa.selenium.json.JsonOutput;
6163
import org.openqa.selenium.net.NetworkUtils;
6264
import org.openqa.selenium.net.Urls;
@@ -390,7 +392,7 @@ private void addDriverConfigs(
390392
.forEach(
391393
keyValue -> {
392394
String[] values = keyValue.split("=", 2);
393-
configMap.put(values[0], values[1]);
395+
configMap.put(values[0], unquote(values[1]));
394396
});
395397
driversMap.add(configMap);
396398
}
@@ -716,4 +718,12 @@ private void report(Map.Entry<WebDriverInfo, Collection<SessionFactory>> entry)
716718
entry.getValue().size(),
717719
entry.getKey().isPresent() ? "Host" : "SM"));
718720
}
721+
722+
private String unquote(String input) {
723+
int len = input.length();
724+
if ((input.charAt(0) == '"') && (input.charAt(len - 1) == '"')) {
725+
return new Json().newInput(new StringReader(input)).read(Json.OBJECT_TYPE);
726+
}
727+
return input;
728+
}
719729
}

‎java/test/org/openqa/selenium/firefox/FirefoxOptionsTest.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,12 @@ void binaryPathNeedNotExist() {
9393

9494
@Test
9595
void shouldKeepRelativePathToBinaryAsIs() {
96-
FirefoxOptions options = new FirefoxOptions().setBinary("some/path");
96+
String path = String.join(File.separator, "some", "path");
97+
FirefoxOptions options = new FirefoxOptions().setBinary(path);
9798
assertThat(options.getBinary())
9899
.extracting(FirefoxBinary::getFile)
99100
.extracting(String::valueOf)
100-
.isEqualTo("some/path");
101+
.isEqualTo(path);
101102
}
102103

103104
@Test
@@ -138,11 +139,12 @@ void stringBasedBinaryRemainsAbsoluteIfSetAsAbsolute() {
138139

139140
@Test
140141
void pathBasedBinaryRemainsAbsoluteIfSetAsAbsolute() {
142+
String path = String.join(File.separator, "", "i", "like", "cheese");
141143
Map<String, Object> json = new FirefoxOptions().setBinary(Paths.get("/i/like/cheese")).asMap();
142144

143145
assertThat(json.get(FIREFOX_OPTIONS))
144146
.asInstanceOf(InstanceOfAssertFactories.MAP)
145-
.containsEntry("binary", "/i/like/cheese");
147+
.containsEntry("binary", path);
146148
}
147149

148150
@Test
@@ -311,7 +313,7 @@ void canConvertOptionsWithBinaryToCapabilitiesAndRestoreBack() throws IOExceptio
311313
Object options2 = options.asMap().get(FirefoxOptions.FIREFOX_OPTIONS);
312314
assertThat(options2)
313315
.asInstanceOf(InstanceOfAssertFactories.MAP)
314-
.containsEntry("binary", tempFile.toFile().getPath().replaceAll("\\\\", "/"));
316+
.containsEntry("binary", tempFile.toFile().getPath());
315317
}
316318

317319
@Test

‎java/test/org/openqa/selenium/grid/config/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ java_test_suite(
88
deps = [
99
"//java/src/org/openqa/selenium:core",
1010
"//java/src/org/openqa/selenium/grid/config",
11+
"//java/src/org/openqa/selenium/json",
1112
artifact("com.beust:jcommander"),
1213
artifact("com.google.guava:guava"),
1314
artifact("io.ous:jtoml"),

‎java/test/org/openqa/selenium/grid/config/JsonConfigTest.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@
2020
import static org.assertj.core.api.Assertions.assertThat;
2121

2222
import java.io.StringReader;
23+
import java.util.Arrays;
24+
import java.util.Collections;
25+
import java.util.List;
2326
import java.util.Optional;
27+
2428
import org.junit.jupiter.api.Test;
2529

2630
class JsonConfigTest {
@@ -32,4 +36,83 @@ void shouldUseATableAsASection() {
3236

3337
assertThat(config.get("cheeses", "selected")).isEqualTo(Optional.of("brie"));
3438
}
39+
40+
@Test
41+
void ensureCanReadListOfStrings() {
42+
String raw = String.join("", "",
43+
"{",
44+
"`relay`: {",
45+
"`configs`: [`2`, `{\\`browserName\\`: \\`chrome\\`}`]",
46+
"}",
47+
"}"
48+
).replace("`", "\"");
49+
Config config = new JsonConfig(new StringReader(raw));
50+
51+
List<String> expected = Arrays.asList(
52+
"2",
53+
"{\"browserName\": \"chrome\"}"
54+
);
55+
Optional<List<String>> content = config.getAll("relay", "configs");
56+
assertThat(content).isEqualTo(Optional.of(expected));
57+
}
58+
59+
@Test
60+
void shouldContainConfigFromArrayOfTables() {
61+
String raw = String.join("", "",
62+
"{",
63+
"`cheeses`: {",
64+
"`default`: `manchego`,",
65+
"`type`: [",
66+
"{",
67+
"`name`: `soft cheese`,",
68+
"`default`: `brie`",
69+
"},",
70+
"{",
71+
"`name`: `Medium-hard cheese`,",
72+
"`default`: `Emmental`",
73+
"}",
74+
"]",
75+
"}",
76+
"}"
77+
).replace("`", "\"");
78+
Config config = new JsonConfig(new StringReader(raw));
79+
80+
assertThat(config.get("cheeses", "default")).isEqualTo(Optional.of("manchego"));
81+
82+
List<String> expected =
83+
Arrays.asList(
84+
"default=\"brie\"", "name=\"soft cheese\"",
85+
"default=\"Emmental\"", "name=\"Medium-hard cheese\"");
86+
assertThat(config.getAll("cheeses", "type").orElse(Collections.emptyList()))
87+
.isEqualTo(expected);
88+
assertThat(config.getAll("cheeses", "type").orElse(Collections.emptyList()).subList(0, 2))
89+
.isEqualTo(expected.subList(0, 2));
90+
}
91+
92+
@Test
93+
void ensureCanReadListOfMaps() {
94+
String raw = String.join("", "",
95+
"{",
96+
"`node`: {",
97+
"`detect-drivers`: false,",
98+
"`driver-configuration`: [",
99+
"{",
100+
"`display-name`: `htmlunit`,",
101+
"`stereotype`: {",
102+
"`browserName`: `htmlunit`,",
103+
"`browserVersion`: `chrome`",
104+
"}",
105+
"}",
106+
"]",
107+
"}",
108+
"}"
109+
).replace("`", "\"");
110+
Config config = new JsonConfig(new StringReader(raw));
111+
List<String> expected = Arrays.asList(
112+
"display-name=\"htmlunit\"",
113+
"stereotype={\"browserName\": \"htmlunit\",\"browserVersion\": \"chrome\"}"
114+
);
115+
Optional<List<String>> content = config.getAll("node", "driver-configuration");
116+
assertThat(content).isEqualTo(Optional.of(expected));
117+
}
35118
}

0 commit comments

Comments
 (0)