Skip to content

Commit

Permalink
[java] removed use of guava from devtools (#12943)
Browse files Browse the repository at this point in the history
Co-authored-by: Puja Jagani <puja.jagani93@gmail.com>
  • Loading branch information
joerg1985 and pujagani authored Oct 16, 2023
1 parent a98e61f commit 0074a7c
Show file tree
Hide file tree
Showing 25 changed files with 141 additions and 93 deletions.
2 changes: 0 additions & 2 deletions java/src/org/openqa/selenium/devtools/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ java_library(
deps = [
"//java/src/org/openqa/selenium:core",
"//java/src/org/openqa/selenium/json",
artifact("com.google.guava:guava"),
],
)

Expand Down Expand Up @@ -76,7 +75,6 @@ java_library(
"//java/src/org/openqa/selenium:core",
"//java/src/org/openqa/selenium/json",
"//java/src/org/openqa/selenium/remote/http",
artifact("com.google.guava:guava"),
],
)

Expand Down
64 changes: 48 additions & 16 deletions java/src/org/openqa/selenium/devtools/CdpClientGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -287,7 +288,8 @@ private void dumpMainClass(Path target) {
unit.addImport(Command.class);
unit.addImport(Event.class);
unit.addImport(ConverterFunctions.class);
unit.addImport(ImmutableMap.class);
unit.addImport(Map.class);
unit.addImport(LinkedHashMap.class);
unit.addImport(JsonInput.class);

ClassOrInterfaceDeclaration classDecl = unit.addClass(name);
Expand Down Expand Up @@ -640,7 +642,7 @@ public MethodDeclaration toMethodDeclaration() {
String.format(
"java.util.Objects.requireNonNull(%s, \"%s is required\");",
name, name)));
body.addStatement("ImmutableMap.Builder<String, Object> params = ImmutableMap.builder();");
body.addStatement("LinkedHashMap<String, Object> params = new LinkedHashMap<>();");
parameters.forEach(
parameter -> {
if (parameter.optional) {
Expand All @@ -655,16 +657,17 @@ public MethodDeclaration toMethodDeclaration() {

if (type instanceof VoidType) {
body.addStatement(
String.format("return new Command<>(\"%s.%s\", params.build());", domain.name, name));
String.format(
"return new Command<>(\"%s.%s\", Map.copyOf(params));", domain.name, name));
} else if (type instanceof ObjectType) {
body.addStatement(
String.format(
"return new Command<>(\"%s.%s\", params.build(), input -> %s);",
"return new Command<>(\"%s.%s\", Map.copyOf(params), input -> %s);",
domain.name, name, type.getMapper()));
} else {
body.addStatement(
String.format(
"return new Command<>(\"%s.%s\", params.build(), ConverterFunctions.map(\"%s\","
"return new Command<>(\"%s.%s\", Map.copyOf(params), ConverterFunctions.map(\"%s\","
+ " %s));",
domain.name, name, type.getName(), type.getTypeToken()));
}
Expand Down Expand Up @@ -792,8 +795,7 @@ public String getName() {
@Override
public String getTypeToken() {
if (type.equals("object")) {
return "new com.google.common.reflect.TypeToken<java.util.Map<String, Object>>()"
+ " {}.getType()";
return "java.util.Map.class";
} else {
return getJavaType() + ".class";
}
Expand Down Expand Up @@ -844,7 +846,7 @@ public TypeDeclaration<?> toTypeDeclaration() {
ClassOrInterfaceDeclaration classDecl = new ClassOrInterfaceDeclaration().setName(name);

if (type.equals("object")) {
classDecl.addExtendedType("com.google.common.collect.ForwardingMap<String, Object>");
classDecl.addExtendedType("java.util.AbstractMap<String, Object>");
}

String propertyName = decapitalize(name);
Expand All @@ -860,6 +862,31 @@ public TypeDeclaration<?> toTypeDeclaration() {
propertyName, propertyName, name));

if (type.equals("object")) {
// we only need to implement entrySet and put to have a working map
MethodDeclaration entrySet = classDecl.addMethod("entrySet").setPublic(true);
entrySet.setType("java.util.Set<java.util.Map.Entry<String, Object>>");
entrySet.getBody().get().addStatement(String.format("return %s.entrySet();", propertyName));

MethodDeclaration put = classDecl.addMethod("put").setPublic(true);
put.setType("Object");
put.addParameter("String", "key");
put.addParameter("Object", "value");
put.getBody().get().addStatement(String.format("return %s.put(key, value);", propertyName));

// containsKey and get are implemented to have better performance
MethodDeclaration containsKey = classDecl.addMethod("containsKey").setPublic(true);
containsKey.setType("boolean");
containsKey.addParameter("String", "key");
containsKey
.getBody()
.get()
.addStatement(String.format("return %s.containsKey(key);", propertyName));

MethodDeclaration get = classDecl.addMethod("get").setPublic(true);
get.setType("Object");
get.addParameter("String", "key");
get.getBody().get().addStatement(String.format("return %s.get(key);", propertyName));

MethodDeclaration delegate = classDecl.addMethod("delegate").setProtected(true);
delegate.setType("java.util.Map<String, Object>");
delegate.getBody().get().addStatement(String.format("return %s;", propertyName));
Expand Down Expand Up @@ -906,8 +933,7 @@ public String getMapper() {
case "any":
return "input.read(Object.class)";
case "object":
return "input.read(new com.google.common.reflect.TypeToken<java.util.Map<String,"
+ " Object>>() {}.getType())";
return "(java.util.Map<String, Object>) input.read(java.util.Map.class)";
case "array":
return "input.nextString()";
default:
Expand Down Expand Up @@ -1192,8 +1218,7 @@ public ArrayType(String name) {

@Override
public String getTypeToken() {
return String.format(
"new com.google.common.reflect.TypeToken<%s>() {}.getType()", getJavaType());
return "java.util.List.class";
}

@Override
Expand Down Expand Up @@ -1229,7 +1254,7 @@ public TypeDeclaration<?> toTypeDeclaration() {
MethodDeclaration fromJson = classDecl.addMethod("fromJson").setPrivate(true).setStatic(true);
fromJson.setType(name);
fromJson.addParameter(JsonInput.class, "input");
fromJson.getBody().get().addStatement(String.format("return %s;", getMapper()));
fromJson.getBody().get().addStatement(String.format("return new %s(%s);", name, getMapper()));

MethodDeclaration toString = classDecl.addMethod("toString").setPublic(true);
toString.setType(String.class);
Expand All @@ -1240,9 +1265,16 @@ public TypeDeclaration<?> toTypeDeclaration() {

@Override
public String getMapper() {
return String.format(
"input.read(new com.google.common.reflect.TypeToken<java.util.List<%s>>() {}.getType())",
itemType.getJavaType());
String itemTypeToken = itemType.getTypeToken();

if (getTypeToken().equals(itemTypeToken)) {
// This would end up with a List<List<Map<String, Object>>> instead of the target type of
// List<List<T>>. It is unlikely this must ever be supported, fail with an error while
// generating the CDP client code for now.
throw new UnsupportedOperationException("nested arrays are not supported");
}

return "input.readArray(" + itemTypeToken + ")";
}

public void parse(Domain domain, Map<String, Object> json) {
Expand Down
3 changes: 1 addition & 2 deletions java/src/org/openqa/selenium/devtools/CdpVersionFinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.openqa.selenium.devtools;

import com.google.common.collect.ImmutableSet;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -53,7 +52,7 @@ public CdpVersionFinder(int versionFudgeFactor, Collection<CdpInfo> infos) {

Require.nonNull("CDP versions", infos);

this.infos = ImmutableSet.copyOf(infos);
this.infos = Set.copyOf(infos);
}

/**
Expand Down
3 changes: 1 addition & 2 deletions java/src/org/openqa/selenium/devtools/Command.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.openqa.selenium.devtools;

import com.google.common.collect.ImmutableMap;
import java.lang.reflect.Type;
import java.util.Map;
import java.util.function.Function;
Expand Down Expand Up @@ -49,7 +48,7 @@ private Command(
Function<JsonInput, X> mapper,
boolean sendsResponse) {
this.method = Require.nonNull("Method name", method);
this.params = ImmutableMap.copyOf(Require.nonNull("Command parameters", params));
this.params = Map.copyOf(Require.nonNull("Command parameters", params));
this.mapper = Require.nonNull("Mapper for result", mapper);

this.sendsResponse = sendsResponse;
Expand Down
27 changes: 14 additions & 13 deletions java/src/org/openqa/selenium/devtools/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@
import static org.openqa.selenium.json.Json.MAP_TYPE;
import static org.openqa.selenium.remote.http.HttpMethod.GET;

import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Multimap;
import java.io.Closeable;
import java.io.StringReader;
import java.time.Duration;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -70,7 +71,7 @@ public class Connection implements Closeable {
private final Map<Long, Consumer<Either<Throwable, JsonInput>>> methodCallbacks =
new ConcurrentHashMap<>();
private final ReadWriteLock callbacksLock = new ReentrantReadWriteLock(true);
private final Multimap<Event<?>, Consumer<?>> eventCallbacks = HashMultimap.create();
private final Map<Event<?>, List<Consumer<?>>> eventCallbacks = new HashMap<>();
private final HttpClient client;
private final String url;
private final AtomicBoolean isClosed;
Expand Down Expand Up @@ -144,7 +145,7 @@ public <X> CompletableFuture<X> send(SessionID sessionId, Command<X> command) {
}));
}

ImmutableMap.Builder<String, Object> serialized = ImmutableMap.builder();
Map<String, Object> serialized = new LinkedHashMap<>();
serialized.put("id", id);
serialized.put("method", command.getMethod());
serialized.put("params", command.getParams());
Expand All @@ -154,7 +155,7 @@ public <X> CompletableFuture<X> send(SessionID sessionId, Command<X> command) {

StringBuilder json = new StringBuilder();
try (JsonOutput out = JSON.newOutput(json).writeClassName(false)) {
out.write(serialized.build());
out.write(Map.copyOf(serialized));
}
LOG.log(getDebugLogLevel(), "-> {0}", json);
socket.sendText(json);
Expand Down Expand Up @@ -191,7 +192,7 @@ public <X> void addListener(Event<X> event, Consumer<X> handler) {
Lock lock = callbacksLock.writeLock();
lock.lock();
try {
eventCallbacks.put(event, handler);
eventCallbacks.computeIfAbsent(event, (key) -> new ArrayList<>()).add(handler);
} finally {
lock.unlock();
}
Expand Down Expand Up @@ -285,14 +286,14 @@ private void handle(CharSequence data) {
}
try {
// TODO: Also only decode once.
eventCallbacks.keySet().stream()
eventCallbacks.entrySet().stream()
.peek(
event ->
LOG.log(
getDebugLogLevel(),
"Matching {0} with {1}",
new Object[] {raw.get("method"), event.getMethod()}))
.filter(event -> raw.get("method").equals(event.getMethod()))
new Object[] {raw.get("method"), event.getKey().getMethod()}))
.filter(event -> raw.get("method").equals(event.getKey().getMethod()))
.forEach(
event -> {
// TODO: This is grossly inefficient. I apologise, and we should fix this.
Expand All @@ -303,7 +304,7 @@ private void handle(CharSequence data) {
while (input.hasNext()) {
switch (input.nextName()) {
case "params":
value = event.getMapper().apply(input);
value = event.getKey().getMapper().apply(input);
break;

default:
Expand All @@ -320,13 +321,13 @@ private void handle(CharSequence data) {

final Object finalValue = value;

for (Consumer<?> action : eventCallbacks.get(event)) {
for (Consumer<?> action : event.getValue()) {
@SuppressWarnings("unchecked")
Consumer<Object> obj = (Consumer<Object>) action;
LOG.log(
getDebugLogLevel(),
"Calling callback for {0} using {1} being passed {2}",
new Object[] {event, obj, finalValue});
new Object[] {event.getKey(), obj, finalValue});
obj.accept(finalValue);
}
}
Expand Down
16 changes: 8 additions & 8 deletions java/src/org/openqa/selenium/devtools/events/CdpEventTypes.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.openqa.selenium.json.Json.MAP_TYPE;

import com.google.common.io.Resources;
import java.io.IOException;
import java.net.URL;
import java.io.InputStream;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
Expand Down Expand Up @@ -68,13 +67,14 @@ public void initializeListener(WebDriver webDriver) {
public static EventType<Void> domMutation(Consumer<DomMutationEvent> handler) {
Require.nonNull("Handler", handler);

URL url = CdpEventTypes.class.getResource("/org/openqa/selenium/devtools/mutation-listener.js");
if (url == null) {
throw new IllegalStateException("Unable to find helper script");
}
String script;
try {
script = Resources.toString(url, UTF_8);
try (InputStream stream =
CdpEventTypes.class.getResourceAsStream(
"/org/openqa/selenium/devtools/mutation-listener.js")) {
if (stream == null) {
throw new IllegalStateException("Unable to find helper script");
}
script = new String(stream.readAllBytes(), UTF_8);
} catch (IOException e) {
throw new IllegalStateException("Unable to read helper script");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.openqa.selenium.devtools.events;

import com.google.common.collect.ImmutableList;
import java.time.Instant;
import java.util.List;
import java.util.Objects;
Expand All @@ -36,7 +35,7 @@ public ConsoleEvent(String type, Instant timestamp, List<Object> modifiedArgs, O
this.type = type;
this.timestamp = timestamp;
this.modifiedArgs = modifiedArgs;
this.args = ImmutableList.copyOf(args);
this.args = List.of(args);
}

public String getType() {
Expand Down
3 changes: 0 additions & 3 deletions java/src/org/openqa/selenium/devtools/v116/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
load("@rules_jvm_external//:defs.bzl", "artifact")
load("//common:defs.bzl", "copy_file")
load("//java:defs.bzl", "java_export", "java_library")
load("//java:version.bzl", "SE_VERSION")
Expand All @@ -25,7 +24,6 @@ java_export(
"//java/src/org/openqa/selenium:core",
"//java/src/org/openqa/selenium/json",
"//java/src/org/openqa/selenium/remote",
artifact("com.google.guava:guava"),
],
)

Expand All @@ -41,7 +39,6 @@ java_library(
"//java/src/org/openqa/selenium:core",
"//java/src/org/openqa/selenium/json",
"//java/src/org/openqa/selenium/remote",
artifact("com.google.guava:guava"),
],
)

Expand Down
4 changes: 2 additions & 2 deletions java/src/org/openqa/selenium/devtools/v116/v116Events.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@

package org.openqa.selenium.devtools.v116;

import com.google.common.collect.ImmutableList;
import java.time.Instant;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import org.openqa.selenium.JavascriptException;
import org.openqa.selenium.devtools.Command;
import org.openqa.selenium.devtools.DevTools;
Expand Down Expand Up @@ -67,7 +67,7 @@ protected ConsoleEvent toConsoleEvent(ConsoleAPICalled event) {
List<Object> modifiedArgs =
event.getArgs().stream()
.map(obj -> new RemoteObject(obj.getType().toString(), obj.getValue().orElse(null)))
.collect(ImmutableList.toImmutableList());
.collect(Collectors.toUnmodifiableList());

return new ConsoleEvent(
event.getType().toString(), Instant.ofEpochMilli(ts), modifiedArgs, event.getArgs());
Expand Down
Loading

0 comments on commit 0074a7c

Please sign in to comment.