Skip to content

Commit

Permalink
[grid] Invoking the create method from MemoizedConfig instead of Config
Browse files Browse the repository at this point in the history
When the create method was being invoked from Config, the actual
configuration was getting lost. This was causing the creation of
two in memory buses when the Node was being created through the
default implementation class.
  • Loading branch information
diemol committed Nov 13, 2020
1 parent 6a58c62 commit b210003
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.openqa.selenium.grid.log.LoggingOptions;
import org.openqa.selenium.grid.node.Node;
import org.openqa.selenium.grid.node.ProxyNodeCdp;
import org.openqa.selenium.grid.node.local.LocalNodeFactory;
import org.openqa.selenium.grid.node.config.NodeOptions;
import org.openqa.selenium.grid.router.Router;
import org.openqa.selenium.grid.security.Secret;
import org.openqa.selenium.grid.security.SecretOptions;
Expand Down Expand Up @@ -192,7 +192,7 @@ protected Handlers createHandlers(Config config) {
Route.post("/graphql").to(() -> graphqlHandler),
Route.get("/readyz").to(() -> readinessCheck));

Node node = LocalNodeFactory.create(config);
Node node = new NodeOptions(config).getNode();
combinedHandler.addHandler(node);
distributor.add(node);

Expand Down
17 changes: 13 additions & 4 deletions java/server/src/org/openqa/selenium/grid/config/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ default <X> X getClass(String section, String option, Class<X> typeOfClass, Stri
// We don't declare a constant on the interface, natch.
Logger.getLogger(Config.class.getName()).fine(String.format("Creating %s as instance of %s", clazz, typeOfClass));

try {
Method create = getCreateMethod(clazz, typeOfClass);
return typeOfClass.cast(create.invoke(null, this));
} catch (ReflectiveOperationException e) {
throw new IllegalArgumentException("Unable to find class: " + clazz, e);
}
}

default <X> Method getCreateMethod(String clazz, Class<X> typeOfClass) {
try {
// Use the context class loader since this is what the `--ext`
// flag modifies.
Expand All @@ -58,18 +67,18 @@ default <X> X getClass(String section, String option, Class<X> typeOfClass, Stri

if (!Modifier.isStatic(create.getModifiers())) {
throw new IllegalArgumentException(String.format(
"Class %s's `create(Config)` method must be static", clazz));
"Class %s's `create(Config)` method must be static", clazz));
}

if (!typeOfClass.isAssignableFrom(create.getReturnType())) {
throw new IllegalArgumentException(String.format(
"Class %s's `create(Config)` method must be static", clazz));
"Class %s's `create(Config)` method must be static", clazz));
}

return typeOfClass.cast(create.invoke(null, this));
return create;
} catch (NoSuchMethodException e) {
throw new IllegalArgumentException(String.format(
"Class %s must have a static `create(Config)` method", clazz));
"Class %s must have a static `create(Config)` method", clazz));
} catch (ReflectiveOperationException e) {
throw new IllegalArgumentException("Unable to find class: " + clazz, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import org.openqa.selenium.internal.Require;

import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -96,7 +97,9 @@ public <X> X getClass(String section, String option, Class<X> typeOfX, String de
new Key(section, option, typeOfX.toGenericString(), defaultClassName),
ignored -> {
try {
return delegate.getClass(section, option, typeOfX, defaultClassName);
String clazz = delegate.get(section, option).orElse(defaultClassName);
Method create = delegate.getCreateMethod(clazz, typeOfX);
return typeOfX.cast(create.invoke(null, this));
} catch (Exception e) {
thrown.set(e);
return null;
Expand Down

0 comments on commit b210003

Please sign in to comment.