Fix URL to URI (SeleniumHQ#8448)

Fix URL to URI and add host annd port support
titusfortner · Aug 13, 2020 · 961b88a · 961b88a
1 parent d096e5e
commit 961b88a
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 27 deletions.
diff --git a/java/server/src/org/openqa/selenium/grid/commands/Hub.java b/java/server/src/org/openqa/selenium/grid/commands/Hub.java
@@ -131,7 +131,7 @@ protected void execute(Config config) {
     handler.addHandler(distributor);
 
     Router router = new Router(tracer, clientFactory, sessions, distributor);
-    GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri().toString());
+    GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri());
     HttpHandler readinessCheck = req -> {
       boolean ready = router.isReady() && bus.isReady();
       return new HttpResponse()

diff --git a/java/server/src/org/openqa/selenium/grid/commands/Standalone.java b/java/server/src/org/openqa/selenium/grid/commands/Standalone.java
@@ -147,7 +147,7 @@ protected void execute(Config config) {
     };
 
     BaseServerOptions serverOptions = new BaseServerOptions(config);
-    GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri().toString());
+    GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri());
     HttpHandler httpHandler = combine(
       router,
       Route.prefix("/wd/hub").to(combine(router)),

diff --git a/java/server/src/org/openqa/selenium/grid/graphql/GraphqlHandler.java b/java/server/src/org/openqa/selenium/grid/graphql/GraphqlHandler.java
@@ -38,6 +38,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UncheckedIOException;
+import java.net.URI;
 import java.util.Objects;
 import java.util.concurrent.ExecutionException;
 
@@ -50,12 +51,12 @@ public class GraphqlHandler implements HttpHandler {
   public static final String GRID_SCHEMA = "/org/openqa/selenium/grid/graphql/selenium-grid-schema.graphqls";
   public static final Json JSON = new Json();
   private final Distributor distributor;
-  private final String publicUrl;
+  private final URI publicUri;
   private final GraphQL graphQl;
 
-  public GraphqlHandler(Distributor distributor, String publicUrl) {
+  public GraphqlHandler(Distributor distributor, URI publicUri) {
     this.distributor = Objects.requireNonNull(distributor);
-    this.publicUrl = Objects.requireNonNull(publicUrl);
+    this.publicUri = Objects.requireNonNull(publicUri);
 
     GraphQLSchema schema = new SchemaGenerator()
       .makeExecutableSchema(buildTypeDefinitionRegistry(), buildRuntimeWiring());
@@ -103,7 +104,7 @@ private RuntimeWiring buildRuntimeWiring() {
       .scalar(Types.Uri)
       .scalar(Types.Url)
       .type("GridQuery", typeWiring -> typeWiring
-        .dataFetcher("grid", new GridData(distributor, publicUrl)))
+        .dataFetcher("grid", new GridData(distributor, publicUri)))
       .build();
   }
 

diff --git a/java/server/src/org/openqa/selenium/grid/graphql/Grid.java b/java/server/src/org/openqa/selenium/grid/graphql/Grid.java
@@ -23,23 +23,24 @@
 import org.openqa.selenium.grid.distributor.Distributor;
 import org.openqa.selenium.internal.Require;
 
+import java.net.URI;
 import java.util.List;
 import java.util.function.Supplier;
 
 public class Grid {
 
-  private final String url;
+  private final URI uri;
   private final Supplier<DistributorStatus> distributorStatus;
 
-  public Grid(Distributor distributor, String url) {
+  public Grid(Distributor distributor, URI uri) {
     Require.nonNull("Distributor", distributor);
-    this.url = Require.nonNull("Grid's public URL", url);
+    this.uri = Require.nonNull("Grid's public URI", uri);
 
     this.distributorStatus = Suppliers.memoize(distributor::getStatus);
   }
 
-  public String getUrl() {
-    return url;
+  public URI getUri() {
+    return uri;
   }
 
   public List<Node> getNodes() {

diff --git a/java/server/src/org/openqa/selenium/grid/graphql/GridData.java b/java/server/src/org/openqa/selenium/grid/graphql/GridData.java
@@ -22,17 +22,19 @@
 import org.openqa.selenium.grid.distributor.Distributor;
 import org.openqa.selenium.internal.Require;
 
+import java.net.URI;
+
 public class GridData implements DataFetcher {
   private final Distributor distributor;
-  private final String publicUrl;
+  private final URI publicUri;
 
-  public GridData(Distributor distributor, String publicUrl) {
+  public GridData(Distributor distributor, URI publicUri) {
     this.distributor = Require.nonNull("Distributor", distributor);
-    this.publicUrl = Require.nonNull("Grid's public URL", publicUrl);
+    this.publicUri = Require.nonNull("Grid's public URI", publicUri);
   }
 
   @Override
   public Object get(DataFetchingEnvironment environment) {
-    return new Grid(distributor, publicUrl);
+    return new Grid(distributor, publicUri);
   }
 }
diff --git a/java/server/src/org/openqa/selenium/grid/graphql/selenium-grid-schema.graphqls b/java/server/src/org/openqa/selenium/grid/graphql/selenium-grid-schema.graphqls
@@ -3,7 +3,6 @@ schema {
 }
 
 scalar Uri
-scalar Url
 
 enum Status {
   UP
@@ -23,7 +22,7 @@ type GridQuery {
 }
 
 type Grid {
-  url: Url!
+  uri: Uri!
   nodes: [Node!]!
   totalSlots: Int
   usedSlots: Int

diff --git a/java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java b/java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java
@@ -113,7 +113,7 @@ protected void execute(Config config) {
         distributorUrl
     );
 
-    GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri().toString());
+    GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri());
 
     Route handler = Route.combine(
       new Router(tracer, clientFactory, sessions, distributor),

diff --git a/java/server/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java b/java/server/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java
@@ -53,11 +53,14 @@
 
 public class GraphqlHandlerTest {
 
-  private final String publicUrl = "http://example.com/grid-o-matic";
+  private final URI publicUri = new URI("http://example.com/grid-o-matic");
   private Distributor distributor;
   private Tracer tracer;
   private EventBus events;
 
+  public GraphqlHandlerTest() throws URISyntaxException {
+  }
+
   @Before
   public void setupGrid() {
     tracer = DefaultTestTracer.createTracer();
@@ -69,17 +72,17 @@ public void setupGrid() {
   }
 
   @Test
-  public void shouldBeAbleToGetGridUrl() {
-    GraphqlHandler handler = new GraphqlHandler(distributor, publicUrl);
+  public void shouldBeAbleToGetGridUri() {
+    GraphqlHandler handler = new GraphqlHandler(distributor, publicUri);
 
-    Map<String, Object> topLevel = executeQuery(handler, "query { grid { url } }");
+    Map<String, Object> topLevel = executeQuery(handler, "query { grid { uri } }");
 
-    assertThat(topLevel).isEqualTo(Map.of("data", Map.of("grid", Map.of("url", publicUrl))));
+    assertThat(topLevel).isEqualTo(Map.of("data", Map.of("grid", Map.of("uri", publicUri))));
   }
 
   @Test
   public void shouldReturnAnEmptyListForNodesIfNoneAreRegistered() {
-    GraphqlHandler handler = new GraphqlHandler(distributor, publicUrl);
+    GraphqlHandler handler = new GraphqlHandler(distributor, publicUri);
 
     Map<String, Object> topLevel = executeQuery(handler, "query { grid { nodes { uri } } }");
 
@@ -92,7 +95,7 @@ public void shouldReturnAnEmptyListForNodesIfNoneAreRegistered() {
   public void shouldBeAbleToGetUrlsOfAllNodes() throws URISyntaxException {
     Capabilities stereotype = new ImmutableCapabilities("cheese", "stilton");
     String nodeUri = "http://localhost:5556";
-    Node node = LocalNode.builder(tracer, events, new URI(nodeUri), new URI(publicUrl), null)
+    Node node = LocalNode.builder(tracer, events, new URI(nodeUri), publicUri, null)
       .add(stereotype, new SessionFactory() {
         @Override
         public Optional<ActiveSession> apply(CreateSessionRequest createSessionRequest) {
@@ -105,9 +108,9 @@ public boolean test(Capabilities capabilities) {
         }
       })
       .build();
-    distributor.add(node);
+    distributor.add(node);String
 
-    GraphqlHandler handler = new GraphqlHandler(distributor, publicUrl);
+    GraphqlHandler handler = new GraphqlHandler(distributor, publicUri);
     Map<String, Object> topLevel = executeQuery(handler, "query { grid { nodes { uri } } }");
 
     assertThat(topLevel)