Use the CreateSessionRequest class when creating sessions

SeleniumHQ · Mar 19, 2019 · 93a3292 · 93a3292
1 parent f5fecf2
commit 93a3292
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 41 deletions.
diff --git a/java/server/src/org/openqa/selenium/grid/session/BUCK b/java/server/src/org/openqa/selenium/grid/session/BUCK
@@ -1,6 +1,9 @@
 java_library(
     name = "session",
     srcs = glob(["*.java"]),
+    exported_deps = [
+        "//java/server/src/org/openqa/selenium/grid/data:data",
+    ],
     deps = [
         "//java/client/src/org/openqa/selenium/remote:remote",
         "//java/server/src/org/openqa/selenium/grid/web:web",

diff --git a/java/server/src/org/openqa/selenium/grid/session/SessionFactory.java b/java/server/src/org/openqa/selenium/grid/session/SessionFactory.java
@@ -19,13 +19,12 @@
 
 
 import org.openqa.selenium.Capabilities;
-import org.openqa.selenium.remote.Dialect;
+import org.openqa.selenium.grid.data.CreateSessionRequest;
 
 import java.util.Optional;
-import java.util.Set;
 
 public interface SessionFactory {
   boolean isSupporting(Capabilities capabilities);
 
-  Optional<ActiveSession> apply(Set<Dialect> downstreamDialects, Capabilities capabilities);
+  Optional<ActiveSession> apply(CreateSessionRequest sessionRequest);
 }
diff --git a/java/server/src/org/openqa/selenium/grid/session/remote/ServicedSession.java b/java/server/src/org/openqa/selenium/grid/session/remote/ServicedSession.java
@@ -23,6 +23,7 @@
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.InvalidArgumentException;
 import org.openqa.selenium.SessionNotCreatedException;
+import org.openqa.selenium.grid.data.CreateSessionRequest;
 import org.openqa.selenium.grid.session.ActiveSession;
 import org.openqa.selenium.net.PortProber;
 import org.openqa.selenium.remote.Dialect;
@@ -38,8 +39,8 @@
 import java.lang.reflect.Method;
 import java.net.URL;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.logging.Level;
@@ -146,10 +147,9 @@ public boolean isSupporting(Capabilities capabilities) {
     }
 
     @Override
-    public Optional<ActiveSession> apply(
-        Set<Dialect> downstreamDialects,
-        Capabilities capabilities) {
-      DriverService service = createService.apply(capabilities);
+    public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
+      Objects.requireNonNull(sessionRequest);
+      DriverService service = createService.apply(sessionRequest.getCapabilities());
 
       try {
         service.start();
@@ -158,7 +158,11 @@ public Optional<ActiveSession> apply(
 
         URL url = service.getUrl();
 
-        return performHandshake(service, url, downstreamDialects, capabilities);
+        return performHandshake(
+            service,
+            url,
+            sessionRequest.getDownstreamDialects(),
+            sessionRequest.getCapabilities());
       } catch (IOException | IllegalStateException | NullPointerException | InvalidArgumentException e) {
         log.log(Level.INFO, e.getMessage(), e);
         service.stop();

diff --git a/java/server/src/org/openqa/selenium/remote/server/ActiveSessionFactory.java b/java/server/src/org/openqa/selenium/remote/server/ActiveSessionFactory.java
@@ -35,17 +35,16 @@
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.grid.data.CreateSessionRequest;
 import org.openqa.selenium.grid.session.ActiveSession;
 import org.openqa.selenium.grid.session.SessionFactory;
 import org.openqa.selenium.grid.session.remote.ServicedSession;
 import org.openqa.selenium.json.Json;
-import org.openqa.selenium.remote.Dialect;
 
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.ServiceLoader;
-import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.logging.Logger;
@@ -172,12 +171,12 @@ public boolean isSupporting(Capabilities capabilities) {
   }
 
   @Override
-  public Optional<ActiveSession> apply(Set<Dialect> downstreamDialects, Capabilities caps) {
-    LOG.info("Capabilities are: " + new Json().toJson(caps));
+  public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
+    LOG.info("Capabilities are: " + new Json().toJson(sessionRequest.getCapabilities()));
     return factories.stream()
-        .filter(factory -> factory.isSupporting(caps))
+        .filter(factory -> factory.isSupporting(sessionRequest.getCapabilities()))
         .peek(factory -> LOG.info(String.format("Matched factory %s", factory)))
-        .map(factory -> factory.apply(downstreamDialects, caps))
+        .map(factory -> factory.apply(sessionRequest))
         .filter(Optional::isPresent)
         .map(Optional::get)
         .findFirst();

diff --git a/java/server/src/org/openqa/selenium/remote/server/InMemorySession.java b/java/server/src/org/openqa/selenium/remote/server/InMemorySession.java
@@ -26,6 +26,7 @@
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.HasCapabilities;
 import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.grid.data.CreateSessionRequest;
 import org.openqa.selenium.grid.session.ActiveSession;
 import org.openqa.selenium.grid.session.SessionFactory;
 import org.openqa.selenium.io.TemporaryFilesystem;
@@ -37,6 +38,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
@@ -134,20 +136,24 @@ public boolean isSupporting(Capabilities capabilities) {
     }
 
     @Override
-    public Optional<ActiveSession> apply(Set<Dialect> downstreamDialects, Capabilities caps) {
+    public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
+      Objects.requireNonNull(sessionRequest);
+
       // Assume the blob fits in the available memory.
       try {
-        if (!provider.canCreateDriverInstanceFor(caps)) {
+        if (!provider.canCreateDriverInstanceFor(sessionRequest.getCapabilities())) {
           return Optional.empty();
         }
 
-        WebDriver driver = provider.newInstance(caps);
+        WebDriver driver = provider.newInstance(sessionRequest.getCapabilities());
 
         // Prefer the OSS dialect.
+        Set<Dialect> downstreamDialects = sessionRequest.getDownstreamDialects();
         Dialect downstream = downstreamDialects.contains(Dialect.OSS) || downstreamDialects.isEmpty() ?
                              Dialect.OSS :
                              downstreamDialects.iterator().next();
-        return Optional.of(new InMemorySession(driver, caps, downstream));
+        return Optional.of(
+            new InMemorySession(driver, sessionRequest.getCapabilities(), downstream));
       } catch (IllegalStateException e) {
         return Optional.empty();
       }

diff --git a/java/server/src/org/openqa/selenium/remote/server/NewSessionPipeline.java b/java/server/src/org/openqa/selenium/remote/server/NewSessionPipeline.java
@@ -18,20 +18,20 @@
 package org.openqa.selenium.remote.server;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.SessionNotCreatedException;
+import org.openqa.selenium.grid.data.CreateSessionRequest;
 import org.openqa.selenium.grid.session.ActiveSession;
 import org.openqa.selenium.grid.session.SessionFactory;
-import org.openqa.selenium.remote.Dialect;
 import org.openqa.selenium.remote.NewSessionPayload;
 
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
 import java.util.function.Function;
 
 public class NewSessionPipeline {
@@ -63,17 +63,26 @@ public ActiveSession createNewSession(NewSessionPayload payload) {
         })
         .map(caps -> factories.stream()
             .filter(factory -> factory.isSupporting(caps))
-            .map(factory -> factory.apply(payload.getDownstreamDialects(), caps))
+            .map(factory -> factory.apply(
+                new CreateSessionRequest(
+                    payload.getDownstreamDialects(),
+                    caps,
+                    ImmutableMap.of())))
             .filter(Optional::isPresent)
             .map(Optional::get)
             .findFirst())
         .filter(Optional::isPresent)
         .map(Optional::get)
         .findFirst()
-        .orElseGet(() ->
-          fallback.apply(payload.getDownstreamDialects(), new ImmutableCapabilities())
-              .orElseThrow(
-                  () -> new SessionNotCreatedException("Unable to create session from " + payload))
+        .orElseGet(
+            () -> fallback.apply(
+                new CreateSessionRequest(
+                    payload.getDownstreamDialects(),
+                    new ImmutableCapabilities(),
+                    ImmutableMap.of()))
+                .orElseThrow(
+                    () -> new SessionNotCreatedException(
+                        "Unable to create session from " + payload))
         );
   }
 
@@ -86,7 +95,7 @@ public boolean isSupporting(Capabilities capabilities) {
       }
 
       @Override
-      public Optional<ActiveSession> apply(Set<Dialect> downstreamDialects, Capabilities capabilities) {
+      public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
         return Optional.empty();
       }
     };

diff --git a/java/server/test/org/openqa/selenium/remote/server/ActiveSessionFactoryTest.java b/java/server/test/org/openqa/selenium/remote/server/ActiveSessionFactoryTest.java
@@ -20,19 +20,20 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.grid.data.CreateSessionRequest;
 import org.openqa.selenium.grid.session.ActiveSession;
 import org.openqa.selenium.grid.session.SessionFactory;
 import org.openqa.selenium.remote.Dialect;
 
 import java.util.Optional;
-import java.util.Set;
 
 public class ActiveSessionFactoryTest {
 
@@ -49,7 +50,9 @@ protected Iterable<DriverProvider> loadDriverProviders() {
       }
     };
 
-    ActiveSession session = sessionFactory.apply(ImmutableSet.of(Dialect.W3C), caps).get();
+    ActiveSession session = sessionFactory.apply(
+        new CreateSessionRequest(ImmutableSet.of(Dialect.W3C), caps, ImmutableMap.of()))
+        .get();
     assertEquals(driver, session.getWrappedDriver());
   }
 
@@ -67,13 +70,17 @@ public boolean isSupporting(Capabilities capabilities) {
                 }
 
                 @Override
-                public Optional<ActiveSession> apply(Set<Dialect> downstreamDialects,
-                                                     Capabilities capabilities) {
+                public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
                   return Optional.of(session);
                 }
               });
 
-    ActiveSession created = sessionFactory.apply(ImmutableSet.copyOf(Dialect.values()), toPayload("cheese")).get();
+    ActiveSession created = sessionFactory.apply(
+        new CreateSessionRequest(
+            ImmutableSet.copyOf(Dialect.values()),
+            toPayload("cheese"),
+            ImmutableMap.of()))
+        .get();
 
     assertSame(session, created);
   }

diff --git a/java/server/test/org/openqa/selenium/remote/server/NewSessionPipelineTest.java b/java/server/test/org/openqa/selenium/remote/server/NewSessionPipelineTest.java
@@ -46,7 +46,7 @@ public void shouldCallSessionFactory() {
     SessionFactory fallback = mock(SessionFactory.class);
     when(fallback.isSupporting(any())).thenReturn(true);
     ActiveSession session = mock(ActiveSession.class);
-    when(factory.apply(any(), any())).thenReturn(Optional.of(session));
+    when(factory.apply(any())).thenReturn(Optional.of(session));
 
     ImmutableMap<String, ImmutableMap<String, ImmutableMap<String, String>>> caps = ImmutableMap.of(
         "capabilities", ImmutableMap.of(
@@ -56,7 +56,7 @@ public void shouldCallSessionFactory() {
         .add(factory).fallback(fallback).create();
 
     pipeline.createNewSession(NewSessionPayload.create(caps));
-    verify(factory).apply(any(), argThat(cap -> cap.getCapability("browserName").equals("firefox")));
+    verify(factory).apply(argThat(req -> req.getCapabilities().getCapability("browserName").equals("firefox")));
     verifyZeroInteractions(fallback);
   }
 
@@ -65,8 +65,8 @@ public void shouldBeAbleToFallBack() {
     SessionFactory factory = mock(SessionFactory.class);
     SessionFactory fallback = mock(SessionFactory.class);
     ActiveSession session = mock(ActiveSession.class);
-    when(factory.apply(any(), any())).thenReturn(Optional.empty());
-    when(fallback.apply(any(), any())).thenReturn(Optional.of(session));
+    when(factory.apply(any())).thenReturn(Optional.empty());
+    when(fallback.apply(any())).thenReturn(Optional.of(session));
 
     ImmutableMap<String, ImmutableMap<String, ImmutableMap<String, String>>> caps = ImmutableMap.of(
         "capabilities", ImmutableMap.of(
@@ -76,15 +76,15 @@ public void shouldBeAbleToFallBack() {
         .add(factory).fallback(fallback).create();
 
     pipeline.createNewSession(NewSessionPayload.create(caps));
-    verify(fallback).apply(any(), argThat(cap -> cap.asMap().size() == 0));
+    verify(fallback).apply(argThat(req -> req.getCapabilities().getCapabilityNames().size() == 0));
   }
 
   @Test
   public void shouldUseMutators() {
     SessionFactory factory = mock(SessionFactory.class);
     when(factory.isSupporting(any())).thenReturn(true);
     ActiveSession session = mock(ActiveSession.class);
-    when(factory.apply(any(), any())).thenReturn(Optional.of(session));
+    when(factory.apply(any())).thenReturn(Optional.of(session));
     FirefoxMutator mutator = new FirefoxMutator(new ImmutableCapabilities(
         "browserName", "firefox",
         "marionette", true
@@ -100,19 +100,19 @@ public void shouldUseMutators() {
         .create();
 
     pipeline.createNewSession(NewSessionPayload.create(caps));
-    verify(factory).apply(any(), argThat(cap -> cap.getCapability("marionette").equals(true)));
+    verify(factory).apply(argThat(req -> req.getCapabilities().getCapability("marionette").equals(true)));
   }
 
   @Test
   public void shouldNotUseFactoriesThatDoNotSupportTheCapabilities() {
     SessionFactory toBeIgnored = mock(SessionFactory.class);
     when(toBeIgnored.isSupporting(any())).thenReturn(false);
-    when(toBeIgnored.apply(any(), any())).thenThrow(new AssertionError("Must not be called"));
+    when(toBeIgnored.apply(any())).thenThrow(new AssertionError("Must not be called"));
 
     ActiveSession session = mock(ActiveSession.class);
     SessionFactory toBeUsed = mock(SessionFactory.class);
     when(toBeUsed.isSupporting(any())).thenReturn(true);
-    when(toBeUsed.apply(any(), any())).thenReturn(Optional.of(session));
+    when(toBeUsed.apply(any())).thenReturn(Optional.of(session));
 
     NewSessionPipeline pipeline = NewSessionPipeline.builder()
         .add(toBeIgnored)