Skip to content

Commit

Permalink
Ensure OSS and W3C New Session payloads make an effort to match
Browse files Browse the repository at this point in the history
Assuming everyone goes through the standalone server. I have
a sneaking suspicion that folks don't always do this, and
nor should they. But, let's see if we can make this work....
  • Loading branch information
shs96c committed Aug 2, 2017
1 parent 61a970e commit e92bb48
Show file tree
Hide file tree
Showing 4 changed files with 253 additions and 20 deletions.
120 changes: 102 additions & 18 deletions java/server/src/org/openqa/selenium/remote/server/NewSessionPayload.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static java.nio.file.StandardOpenOption.CREATE;
import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING;

import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -38,6 +39,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
Expand Down Expand Up @@ -105,26 +107,120 @@ public NewSessionPayload(Map<String, ?> source) throws IOException {

String json = new BeanToJsonConverter().convert(source);

Sources sources;
long size = json.length() * 2; // Each character takes two bytes
if (size > THRESHOLD || Runtime.getRuntime().freeMemory() < size) {
this.root = Files.createTempDirectory("new-session");
this.sources = diskBackedSource(new StringReader(json));
sources = diskBackedSource(new StringReader(json));
} else {
this.root = null;
this.sources = memoryBackedSource(source);
sources = memoryBackedSource(source);
}

validate(sources);
this.sources = rewrite(sources);
}

public NewSessionPayload(long size, Reader source) throws IOException {
Sources sources;
if (size > THRESHOLD || Runtime.getRuntime().freeMemory() < size) {
this.root = Files.createTempDirectory("new-session");
this.sources = diskBackedSource(source);
sources = diskBackedSource(source);
} else {
this.root = null;
this.sources = memoryBackedSource(GSON.fromJson(source, MAP_TYPE));
sources = memoryBackedSource(GSON.fromJson(source, MAP_TYPE));
}

validate(sources);
this.sources = rewrite(sources);
}

private void validate(Sources sources) {
if (!sources.getDialects().contains(Dialect.W3C)) {
return; // Nothing to do
}

// Ensure that the W3C payload looks okay
Map<String, Object> alwaysMatch = sources.getAlwaysMatch().get();
validateSpecCompliance(alwaysMatch);

Set<String> duplicateKeys = sources.getFirstMatch().stream()
.map(Supplier::get)
.peek(this::validateSpecCompliance)
.map(fragment -> Sets.intersection(alwaysMatch.keySet(), fragment.keySet()))
.flatMap(Collection::stream)
.collect(ImmutableSortedSet.toImmutableSortedSet(Ordering.natural()));

if (!duplicateKeys.isEmpty()) {
throw new IllegalArgumentException(
"W3C payload contained keys duplicated between the firstMatch and alwaysMatch items: " +
duplicateKeys);
}
}

private void validateSpecCompliance(Map<String, Object> fragment) {
ImmutableList<String> badKeys = fragment.keySet().stream()
.filter(ACCEPTED_W3C_PATTERNS.negate())
.collect(ImmutableList.toImmutableList());

if (!badKeys.isEmpty()) {
throw new IllegalArgumentException(
"W3C payload contained keys that do not comply with the spec: " + badKeys);
}
}

/**
* If the local end sent a request with a JSON Wire Protocol payload that does not have a matching
* W3C payload, then we need to synthesize one that matches.
*/
private Sources rewrite(Sources sources) {
if (!sources.getDialects().contains(Dialect.OSS)) {
// Yay! Nothing to do!
return sources;
}

if (!sources.getDialects().contains(Dialect.W3C)) {
// Yay! Also nothing to do. I mean, we have an empty payload, but that's cool.
return sources;
}

Map<String, Object> ossPayload = sources.getOss().get().entrySet().stream()
.filter(e -> !("platform".equals(e.getKey()) && "ANY".equals(e.getValue())))
.filter(e -> !("version".equals(e.getKey()) && "".equals(e.getValue())))
.collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue));
Map<String, Object> always = sources.getAlwaysMatch().get();
Optional<ImmutableMap<String, Object>> w3cMatch = sources.getFirstMatch().stream()
.map(Supplier::get)
.map(m -> ImmutableMap.<String, Object>builder().putAll(always).putAll(m).build())
.filter(m -> m.equals(ossPayload))
.findAny();
if (w3cMatch.isPresent()) {
// There's a w3c capability that matches the oss one. Nothing to do.
LOG.fine("Found a w3c capability that matches the oss one.");
return sources;
}

LOG.info( "Mismatched capabilities. Creating a synthetic w3c capability.");

ImmutableList.Builder<Supplier<Map<String, Object>>> newFirstMatches = ImmutableList.builder();
newFirstMatches.add(sources.getOss());
sources.getFirstMatch()
.forEach(m -> newFirstMatches.add(() -> {
ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
builder.putAll(sources.getAlwaysMatch().get());
builder.putAll(m.get());
return builder.build();
}));

return new Sources(
sources.getOriginalPayload(),
sources.getPayloadSize(),
sources.getOss(),
ImmutableMap::of,
newFirstMatches.build(),
sources.getDialects());
}

private Sources memoryBackedSource(Map<String, ?> source) {
LOG.fine("Memory-based payload for: " + source);

Expand Down Expand Up @@ -222,6 +318,7 @@ private Sources diskBackedSource(Reader source) throws IOException {
case "desiredCapabilities":
Path ossCaps = write("oss.json", json);
oss = () -> read(ossCaps);
dialects.add(Dialect.OSS);
break;

default:
Expand Down Expand Up @@ -291,20 +388,7 @@ public Stream<Capabilities> stream() throws IOException {
Map<String, Object> always = sources.getAlwaysMatch().get();
mapStream = sources.getFirstMatch().stream()
.map(Supplier::get)
.peek(m -> {
Set<String> intersection = Sets.intersection(always.keySet(), m.keySet());
if (!intersection.isEmpty()) {
throw new IllegalArgumentException("Duplicate w3c capability keys: " + intersection);
}
})
.map(m -> ImmutableMap.<String, Object>builder().putAll(always).putAll(m).build())
.peek(m -> {
Set<String> illegalKeys = m.keySet().stream().filter(ACCEPTED_W3C_PATTERNS.negate())
.collect(ImmutableSortedSet.toImmutableSortedSet( Ordering.natural()));
if (!illegalKeys.isEmpty()) {
throw new IllegalArgumentException("Illegal w3c capability keys: " + illegalKeys);
}
});
.map(m -> ImmutableMap.<String, Object>builder().putAll(always).putAll(m).build());
} else if (getDownstreamDialects().contains(Dialect.OSS)) {
mapStream = Stream.of(sources.getOss().get());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ public void shouldOfferStreamOfW3cCapabilitiesIfPresent() throws IOException {
"capabilities", ImmutableMap.of(
"alwaysMatch", ImmutableMap.of("browserName", "peas"))));

assertEquals(capabilities.toString(), 1, capabilities.size());
assertEquals("peas", capabilities.get(0).getBrowserName());
// We expect a synthetic w3c capability for the mismatching OSS capabilities
assertEquals(capabilities.toString(), 2, capabilities.size());
assertEquals("peas", capabilities.get(1).getBrowserName());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
ResultConfigTest.class,
SendKeyToActiveElementTest.class,
SessionLogsTest.class,
SyntheticNewSessionPayloadTest.class,
TeeReaderTest.class,
UploadFileTest.class,
ConfigureTimeoutTest.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package org.openqa.selenium.remote.server;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gson.reflect.TypeToken;

import org.junit.Test;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.ImmutableCapabilities;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.reflect.Type;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* There's an interesting problem that appears relatively often with New Session and incorrectly
* prepared payloads. The key problem manifests where something is added to the OSS payload, and
* this is incorrectly reflected in the w3c payload. To work around this, we'll inspect the two sets
* of data. If we can expand the W3C payloads (using the guidelines from the spec) into something
* that matches the OSS payload, we'll just forward the blob unchanged. If, however, we can't then
* we need to do some fancy foot work, in which we'll:
*
* <ol>
* <li>Create a new W3C "firstMatch" blob that matches the OSS payload</li>
* <li>Expand all W3C payloads, so that they are complete</li>
* <li>Forward a new New Session payload composed of the OSS payload and the combined list of
* "firstMatches", with the OSS equivalent first.</li>
* </ol>
* <p>
* This test has been broken out to make this behaviour clearer, and to allow for this comment.
*/
public class SyntheticNewSessionPayloadTest {

private static Type MAP_TYPE = new TypeToken<Map<String, Object>>(){}.getType();

@Test
public void shouldDoNothingIfOssAndW3CPayloadsAreBothEmpty() {
Map<String, Object> empty = ImmutableMap.of(
"desiredCapabilities", ImmutableMap.of(),
"capabilities", ImmutableMap.of(
"alwaysMatch", ImmutableMap.of(),
"firstMatch", ImmutableList.of()));

List<Capabilities> allCaps = getCapabilities(empty);

assertEquals(ImmutableList.of(new ImmutableCapabilities(new HashMap<>())), allCaps);
}

@Test
public void shouldDoNothingIfOssPayloadMatchesAlwaysMatchAndThereAreNoFirstMatches() {
ImmutableMap<String, String> identicalCaps = ImmutableMap.of("browserName", "cheese");

Map<String, Object> payload= ImmutableMap.of(
"desiredCapabilities", identicalCaps,
"capabilities", ImmutableMap.of(
"alwaysMatch", identicalCaps));

List<Capabilities> allCaps = getCapabilities(payload);

assertEquals(ImmutableList.of(new ImmutableCapabilities(identicalCaps)), allCaps);
}

@Test
public void shouldDoNothingIfOssPayloadMatchesAFirstMatchAndThereIsNoAlwaysMatch() {
ImmutableMap<String, String> identicalCaps = ImmutableMap.of("browserName", "cheese");

Map<String, Object> payload= ImmutableMap.of(
"desiredCapabilities", identicalCaps,
"capabilities", ImmutableMap.of(
"firstMatch", ImmutableList.of(identicalCaps)));

List<Capabilities> allCaps = getCapabilities(payload);

assertEquals(ImmutableList.of(new ImmutableCapabilities(identicalCaps)), allCaps);
}

@Test
public void shouldDoNothingIfOssPayloadMatchesAValidMergedW3CPayload() {
ImmutableMap<String, String> caps = ImmutableMap.of(
"browserName", "cheese",
"se:cake", "more cheese");

Map<String, Object> payload= ImmutableMap.of(
"desiredCapabilities", caps,
"capabilities", ImmutableMap.of(
"alwaysMatch", ImmutableMap.of("browserName", "cheese"),
"firstMatch", ImmutableList.of(ImmutableMap.of("se:cake", "more cheese"))));

List<Capabilities> allCaps = getCapabilities(payload);

assertEquals(ImmutableList.of(new ImmutableCapabilities(caps)), allCaps);
}

@Test
public void shouldExpandAllW3CMatchesToFirstMatchesAndRemoveAlwaysMatchIfSynthesizingAPayload()
throws IOException {
Map<String, Object> payload = ImmutableMap.of(
// OSS capabilities request a chrome webdriver
"desiredCapabilities", ImmutableMap.of("browserName", "chrome"),
// Yet the w3c ones ask for IE and edge
"capabilities", ImmutableMap.of(
"alwaysMatch", ImmutableMap.of("se:cake", "cheese"),
"firstMatch", ImmutableList.of(
ImmutableMap.of("browserName", "edge"),
ImmutableMap.of("browserName", "cheese"))));

try (NewSessionPayload newSession = new NewSessionPayload(payload)) {
List<Capabilities> allCaps = newSession.stream().collect(ImmutableList.toImmutableList());

assertEquals(3, allCaps.size());
assertTrue(allCaps.contains(new ImmutableCapabilities(ImmutableMap.of("browserName", "cheese", "se:cake", "cheese"))));
assertTrue(allCaps.contains(new ImmutableCapabilities(ImmutableMap.of("browserName", "chrome"))));
assertTrue(allCaps.contains(new ImmutableCapabilities(ImmutableMap.of("browserName", "edge", "se:cake", "cheese"))));
}
}

// @Test
// public void ossPayloadWillBeFirstW3CPayload() {
// // This is one of the common cases --- asking for marionette to be false. There's no way to
// // encode this legally into the w3c payload (as it doesn't start with "se:"), yet it's a use-
// // case that needs to be properly supported.
// Map<String, Object> rawCapabilities = ImmutableMap.of(
// "desiredCapabilities", ImmutableMap.of("marionette", false),
// "capabilties", ImmutableMap.of(
// "alwaysMatch", ImmutableMap.of("browserName", "chrome")));
//
// List<Capabilities> allCaps = getCapabilities(rawCapabilities);
//
// assertEquals(2, allCaps.size());
// assertEquals(false, allCaps.get(0).getCapability("marionette"));
// }

private List<Capabilities> getCapabilities(Map<String, Object> payload) {
try (NewSessionPayload newSessionPayload = new NewSessionPayload(payload)) {
return newSessionPayload.stream().collect(ImmutableList.toImmutableList());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

}

0 comments on commit e92bb48

Please sign in to comment.