From cbe7243c926c364d979dc6d8d0d16b289dfb01cb Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Wed, 16 Sep 2026 09:52:24 +0530 Subject: [PATCH] [java] Parse each BiDi command response only once (#18011) --- .../src/org/openqa/selenium/bidi/Command.java | 19 +- .../org/openqa/selenium/bidi/Connection.java | 40 +--- .../selenium/bidi/ConverterFunctions.java | 39 ++-- .../bidi/browsingcontext/BrowsingContext.java | 58 ++--- .../openqa/selenium/bidi/module/Browser.java | 32 +-- .../openqa/selenium/bidi/module/Network.java | 3 +- .../openqa/selenium/bidi/module/Script.java | 76 +++---- .../openqa/selenium/bidi/module/Storage.java | 22 +- java/src/org/openqa/selenium/json/Json.java | 28 +++ .../test/org/openqa/selenium/bidi/BUILD.bazel | 21 +- .../openqa/selenium/bidi/ConnectionTest.java | 204 ++++++++++++++++++ .../org/openqa/selenium/json/JsonTest.java | 71 ++++++ javascript/atoms/BUILD.bazel | 12 ++ 13 files changed, 453 insertions(+), 172 deletions(-) create mode 100644 java/test/org/openqa/selenium/bidi/ConnectionTest.java diff --git a/java/src/org/openqa/selenium/bidi/Command.java b/java/src/org/openqa/selenium/bidi/Command.java index d4e2d5dc379cd..3bc720c949137 100644 --- a/java/src/org/openqa/selenium/bidi/Command.java +++ b/java/src/org/openqa/selenium/bidi/Command.java @@ -26,14 +26,16 @@ import org.jspecify.annotations.Nullable; import org.openqa.selenium.Beta; import org.openqa.selenium.internal.Require; -import org.openqa.selenium.json.JsonInput; +import org.openqa.selenium.json.Json; @Beta public class Command { + private static final Json JSON = new Json(); + private final String method; private final Map params; - private final Function mapper; + private final Function<@Nullable Object, X> mapper; private final boolean sendsResponse; public Command(String method, Map params) { @@ -42,18 +44,23 @@ public Command(String method, Map params) { public Command(String method, Map params, Type typeOfX) { this( - method, params, input -> input.readNonNull(Require.nonNull("Type to convert to", typeOfX))); + method, + params, + result -> + Require.nonNull( + "Command result", + JSON.convert(result, Require.nonNull("Type to convert to", typeOfX)))); } public Command( - String method, Map params, Function mapper) { + String method, Map params, Function<@Nullable Object, X> mapper) { this(method, params, mapper, true); } public Command( String method, Map params, - Function mapper, + Function<@Nullable Object, X> mapper, boolean sendsResponse) { this.method = Require.nonNull("Method name", method); this.params = unmodifiableMap(new HashMap<>(Require.nonNull("Command parameters", params))); @@ -73,7 +80,7 @@ public boolean getSendsResponse() { return sendsResponse; } - Function getMapper() { + Function<@Nullable Object, X> getMapper() { return mapper; } } diff --git a/java/src/org/openqa/selenium/bidi/Connection.java b/java/src/org/openqa/selenium/bidi/Connection.java index 45376716bec5c..8e63e68265658 100644 --- a/java/src/org/openqa/selenium/bidi/Connection.java +++ b/java/src/org/openqa/selenium/bidi/Connection.java @@ -23,7 +23,6 @@ import static org.openqa.selenium.remote.http.HttpMethod.GET; import java.io.Closeable; -import java.io.StringReader; import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; @@ -50,7 +49,6 @@ import org.openqa.selenium.internal.Either; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; -import org.openqa.selenium.json.JsonInput; import org.openqa.selenium.json.JsonOutput; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.http.HttpRequest; @@ -69,7 +67,7 @@ public class Connection implements Closeable { return thread; }); private static final AtomicLong NEXT_ID = new AtomicLong(1L); - private final Map>> methodCallbacks = + private final Map>> methodCallbacks = new ConcurrentHashMap<>(); private final ReadWriteLock callbacksLock = new ReentrantReadWriteLock(true); private final Map, Map>> eventCallbacks = new HashMap<>(); @@ -297,17 +295,16 @@ public void onClose(int code, String reason) { } private void handle(CharSequence data) { - // It's kind of gross to decode the data twice, but this lets us get started on something - // that feels nice to users. - // TODO: decode once, and once only - + // Parse the incoming message once, here, into a Map. A response then passes its "result" + // value to the command's mapper and an event passes its "params" Map to the event's mapper; + // neither re-parses the message text. String asString = String.valueOf(data); LOG.log(getDebugLogLevel(), "<- {0}", asString); Map raw = JSON.toType(asString, MAP_TYPE); if (raw.get("id") instanceof Number && (raw.get("result") != null || raw.get("error") != null)) { - handleResponse(asString, raw); + handleResponse(raw); } else if (raw.get("method") instanceof String && raw.get("params") instanceof Map) { handleEventResponse(raw); } else { @@ -315,32 +312,17 @@ private void handle(CharSequence data) { } } - private void handleResponse(String rawDataString, Map rawDataMap) { - Consumer> consumer = + private void handleResponse(Map rawDataMap) { + Consumer> consumer = methodCallbacks.remove(((Number) rawDataMap.get("id")).longValue()); if (consumer == null) { return; } - try (StringReader reader = new StringReader(rawDataString); - JsonInput input = JSON.newInput(reader)) { - input.beginObject(); - while (input.hasNext()) { - switch (input.nextName()) { - case "result": - consumer.accept(Either.right(input)); - break; - - case "error": - consumer.accept(Either.left(new WebDriverException(rawDataString))); - input.skipValue(); - break; - - default: - input.skipValue(); - } - } - input.endObject(); + if (rawDataMap.get("error") != null) { + consumer.accept(Either.left(new WebDriverException(JSON.toJson(rawDataMap)))); + } else { + consumer.accept(Either.right(rawDataMap.get("result"))); } } diff --git a/java/src/org/openqa/selenium/bidi/ConverterFunctions.java b/java/src/org/openqa/selenium/bidi/ConverterFunctions.java index 5ce68cca4d1ce..4faa48c448ad1 100644 --- a/java/src/org/openqa/selenium/bidi/ConverterFunctions.java +++ b/java/src/org/openqa/selenium/bidi/ConverterFunctions.java @@ -18,38 +18,43 @@ package org.openqa.selenium.bidi; import java.lang.reflect.Type; +import java.util.Map; import java.util.function.Function; import org.jspecify.annotations.Nullable; import org.openqa.selenium.Beta; import org.openqa.selenium.internal.Require; -import org.openqa.selenium.json.JsonInput; +import org.openqa.selenium.json.Json; @Beta public class ConverterFunctions { + private static final Json JSON = new Json(); + private ConverterFunctions() { throw new IllegalStateException("Utility class"); } - public static Function map(final String keyName, Type typeOfX) { + /** + * Build a {@link Command} result mapper for the common case where the useful value is a single + * field of the response's {@code result} object. + * + *

The returned function is applied to a command's {@code result} value, which {@link + * org.openqa.selenium.bidi.Connection} has already parsed into a {@code Map}. It + * reads {@code keyName} from that map and deserializes it to {@code typeOfX} via {@link + * Json#convert(Object, Type)}, without re-parsing any JSON text. Both the {@code result} and the + * field are required: a missing {@code result} or a {@code null}/absent field is an error. + * + * @param keyName the field to read from the command's {@code result} object + * @param typeOfX the type to deserialize that field to (class or {@link + * org.openqa.selenium.json.TypeToken}) + */ + public static Function<@Nullable Object, X> map(String keyName, Type typeOfX) { Require.nonNull("Key name", keyName); Require.nonNull("Type to convert to", typeOfX); - return input -> { - X value = null; - - input.beginObject(); - while (input.hasNext()) { - String name = input.nextName(); - if (keyName.equals(name)) { - value = input.read(typeOfX); - } else { - input.skipValue(); - } - } - input.endObject(); - - return value; + return result -> { + Object value = ((Map) Require.nonNull("Command result", result)).get(keyName); + return Require.nonNull("Field '" + keyName + "'", JSON.convert(value, typeOfX)); }; } } diff --git a/java/src/org/openqa/selenium/bidi/browsingcontext/BrowsingContext.java b/java/src/org/openqa/selenium/bidi/browsingcontext/BrowsingContext.java index e018877988129..9fd7d1039e56c 100644 --- a/java/src/org/openqa/selenium/bidi/browsingcontext/BrowsingContext.java +++ b/java/src/org/openqa/selenium/bidi/browsingcontext/BrowsingContext.java @@ -32,11 +32,11 @@ import org.openqa.selenium.WindowType; import org.openqa.selenium.bidi.BiDi; import org.openqa.selenium.bidi.Command; +import org.openqa.selenium.bidi.ConverterFunctions; import org.openqa.selenium.bidi.HasBiDi; import org.openqa.selenium.bidi.script.RemoteValue; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; -import org.openqa.selenium.json.JsonInput; import org.openqa.selenium.json.TypeToken; import org.openqa.selenium.print.PrintOptions; @@ -51,28 +51,28 @@ public class BrowsingContext { private static final String RELOAD = "browsingContext.reload"; private static final String HANDLE_USER_PROMPT = "browsingContext.handleUserPrompt"; - private static final Function browsingContextIdMapper = - json -> { - return json.readMap().getOrDefault(CONTEXT, "").toString(); - }; + private static final Function<@Nullable Object, String> browsingContextIdMapper = + result -> asMap(result).getOrDefault(CONTEXT, "").toString(); - private static final Function navigationInfoMapper = - json -> (NavigationResult) json.readNonNull(NavigationResult.class); + private static final Function<@Nullable Object, NavigationResult> navigationInfoMapper = + result -> Require.nonNull("Navigation result", JSON.convert(result, NavigationResult.class)); - private static final Function> + private static final Function<@Nullable Object, List> browsingContextInfoListMapper = - json -> { + result -> { Type type = new TypeToken>>() {}.getType(); - Map> result = json.readNonNull(type); - return result.getOrDefault("contexts", emptyList()); + Map> converted = + Require.nonNull("Command result", JSON.convert(result, type)); + return converted.getOrDefault("contexts", emptyList()); }; - private static final Function> nodesMapper = - json -> { - Type type = new TypeToken>>() {}.getType(); - Map> result = json.readNonNull(type); - return result.get("nodes"); - }; + private static final Function<@Nullable Object, List> nodesMapper = + ConverterFunctions.map("nodes", new TypeToken>() {}.getType()); + + @SuppressWarnings("unchecked") + private static Map asMap(@Nullable Object result) { + return (Map) Require.nonNull("Command result", result); + } public BrowsingContext(WebDriver driver, String id) { Require.nonNull("WebDriver", driver); @@ -234,9 +234,7 @@ public String captureScreenshot() { new Command<>( "browsingContext.captureScreenshot", Map.of(CONTEXT, id), - jsonInput -> { - return (String) jsonInput.readMap().get("data"); - })); + ConverterFunctions.map("data", String.class))); } public String captureScreenshot(CaptureScreenshotParameters parameters) { @@ -248,9 +246,7 @@ public String captureScreenshot(CaptureScreenshotParameters parameters) { new Command<>( "browsingContext.captureScreenshot", params, - jsonInput -> { - return (String) jsonInput.readMap().get("data"); - })); + ConverterFunctions.map("data", String.class))); } public String captureBoxScreenshot(double x, double y, double width, double height) { @@ -267,9 +263,7 @@ public String captureBoxScreenshot(double x, double y, double width, double heig "y", y, "width", width, "height", height)), - jsonInput -> { - return (String) jsonInput.readMap().get("data"); - })); + ConverterFunctions.map("data", String.class))); } public String captureElementScreenshot(String elementId) { @@ -281,9 +275,7 @@ public String captureElementScreenshot(String elementId) { id, "clip", Map.of("type", "element", "element", Map.of("sharedId", elementId))), - jsonInput -> { - return (String) jsonInput.readMap().get("data"); - })); + ConverterFunctions.map("data", String.class))); } public String captureElementScreenshot(String elementId, String handle) { @@ -296,9 +288,7 @@ public String captureElementScreenshot(String elementId, String handle) { "clip", Map.of( "type", "element", "element", Map.of("sharedId", elementId, "handle", handle))), - jsonInput -> { - return (String) jsonInput.readMap().get("data"); - })); + ConverterFunctions.map("data", String.class))); } public void setViewport(int width, int height) { @@ -381,9 +371,7 @@ public String print(PrintOptions printOptions) { new Command<>( "browsingContext.print", printOptionsParams, - jsonInput -> { - return (String) jsonInput.readMap().get("data"); - })); + ConverterFunctions.map("data", String.class))); } public void traverseHistory(long delta) { diff --git a/java/src/org/openqa/selenium/bidi/module/Browser.java b/java/src/org/openqa/selenium/bidi/module/Browser.java index 777c36b8b0d84..bc428b9bcae29 100644 --- a/java/src/org/openqa/selenium/bidi/module/Browser.java +++ b/java/src/org/openqa/selenium/bidi/module/Browser.java @@ -21,39 +21,45 @@ import java.util.Map; import java.util.function.Function; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.openqa.selenium.Beta; import org.openqa.selenium.WebDriver; import org.openqa.selenium.bidi.BiDi; import org.openqa.selenium.bidi.Command; +import org.openqa.selenium.bidi.ConverterFunctions; import org.openqa.selenium.bidi.HasBiDi; import org.openqa.selenium.bidi.browser.ClientWindowInfo; import org.openqa.selenium.bidi.browser.SetDownloadBehaviorParameters; -import org.openqa.selenium.json.JsonInput; +import org.openqa.selenium.internal.Require; @Beta public class Browser { private final BiDi bidi; - private static final Function userContextInfoMapper = - json -> json.readMapElement("userContext"); - - private static final Function> userContextsInfoMapper = - json -> { - List> userContexts = json.readMapElement("userContexts"); + private static final Function<@Nullable Object, String> userContextInfoMapper = + ConverterFunctions.map("userContext", String.class); + private static final Function<@Nullable Object, List> userContextsInfoMapper = + result -> { + List> userContexts = asList(result, "userContexts"); return userContexts.stream() .map(map -> map.get("userContext")) .collect(Collectors.toList()); }; - private static final Function> clientWindowsInfoMapper = - json -> { - List> clientWindows = json.readMapElement("clientWindows"); - return clientWindows.stream() - .map(map -> ClientWindowInfo.fromJson(map)) - .collect(Collectors.toList()); + private static final Function<@Nullable Object, List> clientWindowsInfoMapper = + result -> { + List> clientWindows = asList(result, "clientWindows"); + return clientWindows.stream().map(ClientWindowInfo::fromJson).collect(Collectors.toList()); }; + @SuppressWarnings("unchecked") + private static List asList(@Nullable Object result, String key) { + return (List) + Require.nonNull( + "Field " + key, ((Map) Require.nonNull("Command result", result)).get(key)); + } + public Browser(WebDriver driver) { this.bidi = ((HasBiDi) driver).getBiDi(); } diff --git a/java/src/org/openqa/selenium/bidi/module/Network.java b/java/src/org/openqa/selenium/bidi/module/Network.java index 000e1af5f6876..2527b92fb742c 100644 --- a/java/src/org/openqa/selenium/bidi/module/Network.java +++ b/java/src/org/openqa/selenium/bidi/module/Network.java @@ -28,6 +28,7 @@ import org.openqa.selenium.WebDriver; import org.openqa.selenium.bidi.BiDi; import org.openqa.selenium.bidi.Command; +import org.openqa.selenium.bidi.ConverterFunctions; import org.openqa.selenium.bidi.Event; import org.openqa.selenium.bidi.HasBiDi; import org.openqa.selenium.bidi.network.AddInterceptParameters; @@ -87,7 +88,7 @@ public String addIntercept(AddInterceptParameters parameters) { new Command<>( "network.addIntercept", parameters.toMap(), - jsonInput -> jsonInput.readMapElement("intercept"))); + ConverterFunctions.map("intercept", String.class))); } public void removeIntercept(String interceptId) { diff --git a/java/src/org/openqa/selenium/bidi/module/Script.java b/java/src/org/openqa/selenium/bidi/module/Script.java index a80dfaa5cc216..0c832734a98f2 100644 --- a/java/src/org/openqa/selenium/bidi/module/Script.java +++ b/java/src/org/openqa/selenium/bidi/module/Script.java @@ -20,7 +20,6 @@ import static java.util.Collections.emptyMap; import java.io.Closeable; -import java.io.StringReader; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -35,6 +34,7 @@ import org.openqa.selenium.WebDriver; import org.openqa.selenium.bidi.BiDi; import org.openqa.selenium.bidi.Command; +import org.openqa.selenium.bidi.ConverterFunctions; import org.openqa.selenium.bidi.Event; import org.openqa.selenium.bidi.HasBiDi; import org.openqa.selenium.bidi.script.CallFunctionParameters; @@ -52,7 +52,6 @@ import org.openqa.selenium.bidi.script.ResultOwnership; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; -import org.openqa.selenium.json.JsonInput; import org.openqa.selenium.json.TypeToken; @Beta @@ -63,47 +62,20 @@ public class Script implements Closeable { private final BiDi bidi; - private static final Function evaluateResultMapper = - jsonInput -> createEvaluateResult(jsonInput.readMap()); + private static final Function<@Nullable Object, EvaluateResult> evaluateResultMapper = + result -> createEvaluateResult(asMap(result)); - private static final Function> realmInfoMapper = - jsonInput -> { - Object realms = jsonInput.readMapElement("realms"); - try (StringReader reader = new StringReader(JSON.toJson(realms)); - JsonInput input = JSON.newInput(reader)) { - return input.readNonNull(new TypeToken>() {}.getType()); - } - }; + private static final Function<@Nullable Object, List> realmInfoMapper = + ConverterFunctions.map("realms", new TypeToken>() {}.getType()); private static final Event messageEvent = - new Event<>( - "script.message", - params -> { - try (StringReader reader = new StringReader(JSON.toJson(params)); - JsonInput input = JSON.newInput(reader)) { - return input.readNonNull(Message.class); - } - }); + new Event<>("script.message", params -> JSON.convert(params, Message.class)); private static final Event realmCreated = - new Event<>( - "script.realmCreated", - params -> { - try (StringReader reader = new StringReader(JSON.toJson(params)); - JsonInput input = JSON.newInput(reader)) { - return input.readNonNull(RealmInfo.class); - } - }); + new Event<>("script.realmCreated", params -> JSON.convert(params, RealmInfo.class)); private static final Event realmDestroyed = - new Event<>( - "script.realmDestroyed", - params -> { - try (StringReader reader = new StringReader(JSON.toJson(params)); - JsonInput input = JSON.newInput(reader)) { - return input.readNonNull(RealmInfo.class); - } - }); + new Event<>("script.realmDestroyed", params -> JSON.convert(params, RealmInfo.class)); public Script(WebDriver driver) { this(new HashSet<>(), driver); @@ -290,7 +262,7 @@ public String addPreloadScript(String functionDeclaration) { new Command<>( "script.addPreloadScript", parameters, - jsonInput -> jsonInput.readMapElement("script").toString())); + result -> asMap(result).get("script").toString())); } public String addPreloadScript(String functionDeclaration, List arguments) { @@ -306,7 +278,7 @@ public String addPreloadScript(String functionDeclaration, List ar new Command<>( "script.addPreloadScript", parameters, - jsonInput -> jsonInput.readMapElement("script"))); + result -> asMap(result).get("script").toString())); } public String addPreloadScript(String functionDeclaration, String sandbox) { @@ -323,7 +295,7 @@ public String addPreloadScript(String functionDeclaration, String sandbox) { new Command<>( "script.addPreloadScript", parameters, - jsonInput -> jsonInput.readMapElement("script").toString())); + result -> asMap(result).get("script").toString())); } public String addPreloadScript( @@ -341,7 +313,7 @@ public String addPreloadScript( new Command<>( "script.addPreloadScript", parameters, - jsonInput -> jsonInput.readMapElement("script").toString())); + result -> asMap(result).get("script").toString())); } public void removePreloadScript(String id) { @@ -428,26 +400,26 @@ private Map getEvaluateParams( return params; } + @SuppressWarnings("unchecked") + private static Map asMap(@Nullable Object value) { + return (Map) Require.nonNull("Command result", value); + } + private static EvaluateResult createEvaluateResult(Map response) { String type = (String) response.get("type"); EvaluateResult evaluateResult; String realmId = (String) response.get("realm"); if (type.equals(EvaluateResult.Type.SUCCESS.toString())) { - final RemoteValue remoteValue; - try (StringReader reader = new StringReader(JSON.toJson(response.get("result"))); - JsonInput input = JSON.newInput(reader)) { - remoteValue = input.readNonNull(RemoteValue.class); - } - + RemoteValue remoteValue = + Require.nonNull( + "Evaluate result", JSON.convert(response.get("result"), RemoteValue.class)); evaluateResult = new EvaluateResultSuccess(EvaluateResult.Type.SUCCESS, realmId, remoteValue); } else { - final ExceptionDetails exceptionDetails; - try (StringReader reader = new StringReader(JSON.toJson(response.get("exceptionDetails"))); - JsonInput input = JSON.newInput(reader)) { - exceptionDetails = input.readNonNull(ExceptionDetails.class); - } - + ExceptionDetails exceptionDetails = + Require.nonNull( + "Exception details", + JSON.convert(response.get("exceptionDetails"), ExceptionDetails.class)); evaluateResult = new EvaluateResultExceptionValue( EvaluateResult.Type.EXCEPTION, realmId, exceptionDetails); diff --git a/java/src/org/openqa/selenium/bidi/module/Storage.java b/java/src/org/openqa/selenium/bidi/module/Storage.java index a308f0f03d1c4..b876e751266d8 100644 --- a/java/src/org/openqa/selenium/bidi/module/Storage.java +++ b/java/src/org/openqa/selenium/bidi/module/Storage.java @@ -17,13 +17,13 @@ package org.openqa.selenium.bidi.module; -import java.io.StringReader; -import java.util.Map; import java.util.function.Function; +import org.jspecify.annotations.Nullable; import org.openqa.selenium.Beta; import org.openqa.selenium.WebDriver; import org.openqa.selenium.bidi.BiDi; import org.openqa.selenium.bidi.Command; +import org.openqa.selenium.bidi.ConverterFunctions; import org.openqa.selenium.bidi.HasBiDi; import org.openqa.selenium.bidi.storage.DeleteCookiesParameters; import org.openqa.selenium.bidi.storage.GetCookiesParameters; @@ -31,26 +31,14 @@ import org.openqa.selenium.bidi.storage.PartitionKey; import org.openqa.selenium.bidi.storage.SetCookieParameters; import org.openqa.selenium.internal.Require; -import org.openqa.selenium.json.Json; -import org.openqa.selenium.json.JsonInput; @Beta public class Storage { - private static final Json JSON = new Json(); private final BiDi bidi; - private static final Function getCookiesResultMapper = - jsonInput -> jsonInput.readNonNull(GetCookiesResult.class); - - private static final Function partitionKeyResultMapper = - jsonInput -> { - Map partitionKey = jsonInput.readMapElement("partitionKey"); - try (StringReader reader = new StringReader(JSON.toJson(partitionKey)); - JsonInput input = JSON.newInput(reader)) { - return input.readNonNull(PartitionKey.class); - } - }; + private static final Function<@Nullable Object, PartitionKey> partitionKeyResultMapper = + ConverterFunctions.map("partitionKey", PartitionKey.class); public Storage(WebDriver driver) { Require.nonNull("WebDriver", driver); @@ -64,7 +52,7 @@ public Storage(WebDriver driver) { public GetCookiesResult getCookies(GetCookiesParameters params) { return this.bidi.send( - new Command<>("storage.getCookies", params.toMap(), getCookiesResultMapper)); + new Command<>("storage.getCookies", params.toMap(), GetCookiesResult.class)); } public PartitionKey setCookie(SetCookieParameters params) { diff --git a/java/src/org/openqa/selenium/json/Json.java b/java/src/org/openqa/selenium/json/Json.java index dc392a387491a..48d4d98fee1e4 100644 --- a/java/src/org/openqa/selenium/json/Json.java +++ b/java/src/org/openqa/selenium/json/Json.java @@ -214,6 +214,34 @@ public T toType(Reader source, Type typeOfT, PropertySetting setter) { } } + /** + * Deserialize an already-parsed JSON value into an object of the specified type. The source is + * the {@link java.util.Map}/{@link java.util.List}/{@link String}/{@link Number}/{@link + * Boolean}/{@code null} structure that {@link #toType(String, Type)} returns for {@link + * #MAP_TYPE} or {@link #OBJECT_TYPE} - not a JSON string. + * + *

This is the object-input counterpart of {@link #toType(String, Type)}. Use it when a value + * has already been parsed out of a larger document - typically one field of a {@code Map} - so that the surrounding document does not have to be parsed a second time to reach + * it. The value itself is still round-tripped once, via {@link #toJson(Object)} then {@link + * #toType(String, Type)}, and therefore inherits {@link JsonOutput#MAX_DEPTH} - a source nested + * deeper than that limit is rejected. A {@code null} source yields {@code null}. + * + * @param source an already-parsed JSON value ({@code Map}, {@code List}, {@code String}, {@code + * Number}, {@code Boolean}, or {@code null}) + * @param typeOfT data type for deserialization (class or {@link TypeToken}) + * @return object of the specified type, or {@code null} if {@code source} is {@code null} + * @param result type (as specified by [typeOfT]) + * @throws JsonException if the source cannot be coerced to the specified type, or is nested + * deeper than {@link JsonOutput#MAX_DEPTH} + */ + public @Nullable T convert(@Nullable Object source, Type typeOfT) { + if (source == null) { + return null; + } + return toType(toJson(source), typeOfT); + } + /** * Create a new {@code JsonInput} object to traverse the JSON string supplied the specified {@code * Reader}.
diff --git a/java/test/org/openqa/selenium/bidi/BUILD.bazel b/java/test/org/openqa/selenium/bidi/BUILD.bazel index 902f5eed785b2..6ee6237b78187 100644 --- a/java/test/org/openqa/selenium/bidi/BUILD.bazel +++ b/java/test/org/openqa/selenium/bidi/BUILD.bazel @@ -1,10 +1,27 @@ load("@rules_jvm_external//:defs.bzl", "artifact") -load("//java:defs.bzl", "BIDI_BROWSERS", "JUNIT5_DEPS", "java_selenium_test_suite") +load("//java:defs.bzl", "BIDI_BROWSERS", "JUNIT5_DEPS", "java_selenium_test_suite", "java_test_suite") + +java_test_suite( + name = "small-tests", + size = "small", + srcs = ["ConnectionTest.java"], + deps = [ + "//java/src/org/openqa/selenium:core", + "//java/src/org/openqa/selenium/bidi", + "//java/src/org/openqa/selenium/json", + "//java/src/org/openqa/selenium/remote/http", + artifact("org.junit.jupiter:junit-jupiter-api"), + artifact("org.assertj:assertj-core"), + ] + JUNIT5_DEPS, +) java_selenium_test_suite( name = "large-tests", size = "large", - srcs = glob(["*Test.java"]), + srcs = glob( + ["*Test.java"], + exclude = ["ConnectionTest.java"], + ), browsers = BIDI_BROWSERS, tags = [ "selenium-remote", diff --git a/java/test/org/openqa/selenium/bidi/ConnectionTest.java b/java/test/org/openqa/selenium/bidi/ConnectionTest.java new file mode 100644 index 0000000000000..877597ead4a25 --- /dev/null +++ b/java/test/org/openqa/selenium/bidi/ConnectionTest.java @@ -0,0 +1,204 @@ +// 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.bidi; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.WebDriverException; +import org.openqa.selenium.json.Json; +import org.openqa.selenium.remote.http.HttpClient; +import org.openqa.selenium.remote.http.HttpRequest; +import org.openqa.selenium.remote.http.HttpResponse; +import org.openqa.selenium.remote.http.Message; +import org.openqa.selenium.remote.http.TextMessage; +import org.openqa.selenium.remote.http.WebSocket; + +@Tag("UnitTests") +class ConnectionTest { + + private static final Json JSON = new Json(); + + private FakeWebSocket socket; + private WebSocket.Listener listener; + private Connection connection; + + @BeforeEach + void setUp() { + socket = new FakeWebSocket(); + connection = + new Connection( + new FakeHttpClient( + (request, l) -> { + listener = l; + return socket; + }), + "ws://localhost:4444/session/1/se/bidi"); + } + + @Test + void deliversTheParsedResultToATypedCommandMapper() throws Exception { + CompletableFuture> future = + connection.send( + new Command>("session.status", Map.of(), Json.MAP_TYPE)); + + respondWithResult(lastSentId(), "{\"ready\": true, \"message\": \"ok\"}"); + + assertThat(get(future)).containsEntry("ready", true).containsEntry("message", "ok"); + } + + @Test + void handsTheMapperTheAlreadyParsedResultObject() throws Exception { + Function mapper = result -> ((Map) result).get("token").toString(); + CompletableFuture future = + connection.send(new Command<>("session.new", Map.of(), mapper)); + + respondWithResult(lastSentId(), "{\"token\": \"abc-123\"}"); + + assertThat(get(future)).isEqualTo("abc-123"); + } + + @Test + void completesExceptionallyForAnErrorResponse() { + CompletableFuture future = + connection.send(new Command<>("browsingContext.navigate", Map.of())); + + respond( + "{\"type\": \"error\", \"id\": " + + lastSentId() + + ", \"error\": \"unknown command\", \"message\": \"nope\"}"); + + assertThatThrownBy(() -> get(future)) + .isInstanceOf(ExecutionException.class) + .cause() + .isInstanceOf(WebDriverException.class); + } + + @Test + void propagatesAnExceptionThrownByTheMapper() { + Function mapper = + result -> { + throw new IllegalStateException("boom"); + }; + CompletableFuture future = + connection.send(new Command<>("script.evaluate", Map.of(), mapper)); + + respondWithResult(lastSentId(), "{}"); + + assertThatThrownBy(() -> get(future)) + .isInstanceOf(ExecutionException.class) + .cause() + .isInstanceOf(IllegalStateException.class) + .hasMessage("boom"); + } + + @Test + void dropsTheCallbackOnceHandledSoALateDuplicateResponseChangesNothing() throws Exception { + CompletableFuture> future = + connection.send( + new Command>("session.status", Map.of(), Json.MAP_TYPE)); + long id = lastSentId(); + + respondWithResult(id, "{\"ready\": true}"); + assertThat(get(future)).containsEntry("ready", true); + + // A second response carrying the same id must not throw or re-complete the future. + respondWithResult(id, "{\"ready\": false}"); + assertThat(future.get(2, TimeUnit.SECONDS)).containsEntry("ready", true); + } + + @Test + void ignoresAResponseForAnUnknownIdAndStillHandlesRealOnes() throws Exception { + CompletableFuture> future = + connection.send( + new Command>("session.status", Map.of(), Json.MAP_TYPE)); + long id = lastSentId(); + + respondWithResult(id + 987_654L, "{\"ready\": false}"); // nobody is waiting for this id + respondWithResult(id, "{\"ready\": true}"); // the registered callback still fires + + assertThat(get(future)).containsEntry("ready", true); + } + + private long lastSentId() { + String sent = socket.lastText.get(); + assertThat(sent).as("a command frame was written to the socket").isNotNull(); + Map frame = JSON.toType(sent, Json.MAP_TYPE); + return ((Number) frame.get("id")).longValue(); + } + + private void respondWithResult(long id, String resultJson) { + respond("{\"type\": \"success\", \"id\": " + id + ", \"result\": " + resultJson + "}"); + } + + private void respond(String message) { + listener.onText(message); + } + + private static T get(CompletableFuture future) throws Exception { + return future.get(5, TimeUnit.SECONDS); + } + + private static class FakeWebSocket implements WebSocket { + + private final AtomicReference lastText = new AtomicReference<>(); + + @Override + public WebSocket send(Message message) { + if (message instanceof TextMessage) { + lastText.set(((TextMessage) message).text()); + } + return this; + } + + @Override + public void close() {} + } + + private static class FakeHttpClient implements HttpClient { + + private interface SocketOpener { + WebSocket open(HttpRequest request, WebSocket.Listener listener); + } + + private final SocketOpener opener; + + private FakeHttpClient(SocketOpener opener) { + this.opener = opener; + } + + @Override + public WebSocket openSocket(HttpRequest request, WebSocket.Listener listener) { + return opener.open(request, listener); + } + + @Override + public HttpResponse execute(HttpRequest request) { + throw new UnsupportedOperationException("execute"); + } + } +} diff --git a/java/test/org/openqa/selenium/json/JsonTest.java b/java/test/org/openqa/selenium/json/JsonTest.java index df45725e65647..738a3de49eadf 100644 --- a/java/test/org/openqa/selenium/json/JsonTest.java +++ b/java/test/org/openqa/selenium/json/JsonTest.java @@ -30,6 +30,7 @@ import java.time.Instant; import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -167,6 +168,76 @@ void canConstructASimpleString() { assertThat(text).isEqualTo("cheese"); } + @Test + void convertReturnsNullForANullSource() { + String converted = new Json().convert(null, String.class); + assertThat(converted).isNull(); + } + + @Test + void convertCoercesAScalarSource() { + String text = new Json().convert("cheese", String.class); + Boolean flag = new Json().convert(true, Boolean.class); + assertThat(text).isEqualTo("cheese"); + assertThat(flag).isTrue(); + } + + @Test + void convertWidensNumbersTheSameWayAStringParseDoes() { + // A parsed JSON integer is a Long; asking for a Double must still widen it, just as + // toType("3", Double.class) would. + Double widened = new Json().convert(3L, Double.class); + assertThat(widened).isEqualTo(3.0d); + } + + @Test + void convertCoercesAMapSourceIntoABean() { + Object source = Map.of("value", "cheese"); + + NoDefaultConstructor bean = new Json().convert(source, NoDefaultConstructor.class); + + assertThat(bean.getValue()).isEqualTo("cheese"); + } + + @Test + void convertCoercesAListSourceIntoATypedList() { + Object source = List.of(Map.of("value", "brie"), Map.of("value", "cheddar")); + + List beans = + new Json().convert(source, new TypeToken>() {}.getType()); + + assertThat(beans).extracting(NoDefaultConstructor::getValue).containsExactly("brie", "cheddar"); + } + + @Test + void convertingAFieldOfAParsedMapMatchesParsingThatFieldDirectly() { + Map parsedOnce = + new Json().toType("{\"result\": {\"value\": \"cheese\"}}", MAP_TYPE); + + NoDefaultConstructor viaConvert = + new Json().convert(parsedOnce.get("result"), NoDefaultConstructor.class); + NoDefaultConstructor viaParse = + new Json().toType("{\"value\": \"cheese\"}", NoDefaultConstructor.class); + + assertThat(viaConvert.getValue()).isEqualTo(viaParse.getValue()); + } + + @Test + void convertRejectsASourceNestedDeeperThanTheOutputDepthLimit() { + // convert() round-trips the source through toJson(), so it inherits JsonOutput.MAX_DEPTH. + Map deep = new HashMap<>(); + Map cursor = deep; + for (int i = 0; i < JsonOutput.MAX_DEPTH + 5; i++) { + Map next = new HashMap<>(); + cursor.put("child", next); + cursor = next; + } + + assertThatThrownBy(() -> new Json().convert(deep, MAP_TYPE)) + .isInstanceOf(JsonException.class) + .hasMessageContaining("maximum depth"); + } + @Test void canPopulateAMap() { String raw = "{\"cheese\": \"brie\", \"foodstuff\": \"cheese\"}"; diff --git a/javascript/atoms/BUILD.bazel b/javascript/atoms/BUILD.bazel index 33063c61807f3..19db8e3fe08c7 100644 --- a/javascript/atoms/BUILD.bazel +++ b/javascript/atoms/BUILD.bazel @@ -27,6 +27,10 @@ js_run_binary( "none", "--moduleResolution", "node", + # These atoms are self-contained and import nothing; pin typeRoots to the (type-free) + # source dir so unrelated ambient @types packages in node_modules cannot break the build. + "--typeRoots", + "javascript/atoms/typescript", "--removeComments", "--pretty", "false", @@ -82,6 +86,10 @@ js_run_binary( "none", "--moduleResolution", "node", + # These atoms are self-contained and import nothing; pin typeRoots to the (type-free) + # source dir so unrelated ambient @types packages in node_modules cannot break the build. + "--typeRoots", + "javascript/atoms/typescript", "--removeComments", "--pretty", "false", @@ -125,6 +133,10 @@ js_run_binary( "none", "--moduleResolution", "node", + # These atoms are self-contained and import nothing; pin typeRoots to the (type-free) + # source dir so unrelated ambient @types packages in node_modules cannot break the build. + "--typeRoots", + "javascript/atoms/typescript", "--removeComments", "--pretty", "false",