From 56b215a7580f899d873eab3f435c8265c9574633 Mon Sep 17 00:00:00 2001 From: Simon Stewart Date: Fri, 23 Jun 2017 15:56:20 +0100 Subject: [PATCH] Delete methods from `Session` that were used by `SessionCleaner` This makes the interface more focussed. That's a Good Thing, right? --- .../WebDriverBackedSeleniumServlet.java | 2 -- .../remote/server/DefaultDriverSessions.java | 4 --- .../remote/server/DefaultSession.java | 26 +------------------ .../selenium/remote/server/DriverServlet.java | 1 - .../remote/server/SeleniumServer.java | 1 - .../selenium/remote/server/Session.java | 10 ------- .../remote/server/WebDriverServlet.java | 1 - .../WebDriverBackedSeleniumServletTest.java | 2 -- .../server/test/org/openqa/selenium/Main.java | 2 -- .../remote/server/DefaultSessionTest.java | 1 - .../remote/server/DriverSessionTest.java | 8 +++--- .../auth/AuthenticatedWebDriverServer.java | 2 -- .../remote/server/handler/UploadFileTest.java | 3 --- .../test/org/openqa/testing/TestSession.java | 10 +------ 14 files changed, 6 insertions(+), 67 deletions(-) diff --git a/java/server/src/com/thoughtworks/selenium/webdriven/WebDriverBackedSeleniumServlet.java b/java/server/src/com/thoughtworks/selenium/webdriven/WebDriverBackedSeleniumServlet.java index 01641bcb265ca..b84363fde0e93 100644 --- a/java/server/src/com/thoughtworks/selenium/webdriven/WebDriverBackedSeleniumServlet.java +++ b/java/server/src/com/thoughtworks/selenium/webdriven/WebDriverBackedSeleniumServlet.java @@ -37,7 +37,6 @@ import org.openqa.selenium.remote.server.DefaultDriverSessions; import org.openqa.selenium.remote.server.DriverSessions; import org.openqa.selenium.remote.server.Session; -import org.openqa.selenium.remote.server.SystemClock; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -79,7 +78,6 @@ public WebDriverBackedSeleniumServlet() { if (attribute == null) { attribute = new DefaultDriverSessions( new DefaultDriverFactory(Platform.getCurrent()), - new SystemClock(), MINUTES.toMillis(5)); } return (DriverSessions) attribute; diff --git a/java/server/src/org/openqa/selenium/remote/server/DefaultDriverSessions.java b/java/server/src/org/openqa/selenium/remote/server/DefaultDriverSessions.java index 9bd81a0dc5204..43a2724f21dce 100644 --- a/java/server/src/org/openqa/selenium/remote/server/DefaultDriverSessions.java +++ b/java/server/src/org/openqa/selenium/remote/server/DefaultDriverSessions.java @@ -37,16 +37,13 @@ public class DefaultDriverSessions implements DriverSessions { private final DriverFactory factory; - private final Clock clock; private final Cache sessionIdToDriver; public DefaultDriverSessions( DriverFactory factory, - Clock clock, long inactiveSessionTimeoutMs) { this.factory = factory; - this.clock = clock; RemovalListener listener = notification -> { Session session = notification.getValue(); @@ -71,7 +68,6 @@ public SessionId newSession(Capabilities desiredCapabilities) throws Exception { Session session = DefaultSession.createSession( factory, TemporaryFilesystem.getTmpFsBasedOn(Files.createTempDir()), - clock, desiredCapabilities); sessionIdToDriver.put(session.getSessionId(), session); diff --git a/java/server/src/org/openqa/selenium/remote/server/DefaultSession.java b/java/server/src/org/openqa/selenium/remote/server/DefaultSession.java index 841cbc0206c2a..b2464180853de 100644 --- a/java/server/src/org/openqa/selenium/remote/server/DefaultSession.java +++ b/java/server/src/org/openqa/selenium/remote/server/DefaultSession.java @@ -64,30 +64,24 @@ public class DefaultSession implements Session { */ private final KnownElements knownElements; private final Capabilities capabilities; // todo: Investigate memory model implications of map - private final Clock clock; // elements inside capabilities. private volatile String base64EncodedImage; - private volatile long lastAccess; - private volatile Thread inUseWithThread = null; private TemporaryFilesystem tempFs; public static Session createSession( DriverFactory factory, TemporaryFilesystem tempFs, - Clock clock, Capabilities capabilities) throws Exception { - return new DefaultSession(factory, tempFs, clock, capabilities); + return new DefaultSession(factory, tempFs, capabilities); } private DefaultSession( final DriverFactory factory, TemporaryFilesystem tempFs, - Clock clock, final Capabilities capabilities) throws Exception { this.knownElements = new KnownElements(); this.tempFs = tempFs; - this.clock = clock; final BrowserCreator browserCreator = new BrowserCreator(factory, capabilities); // Ensure that the browser is created on the single thread. @@ -101,8 +95,6 @@ private DefaultSession( this.driver = initialDriver; this.capabilities = browserCreator.getCapabilityDescription(); this.sessionId = browserCreator.getSessionId(); - - updateLastAccessTime(); } private static boolean isQuietModeEnabled( @@ -123,17 +115,6 @@ private static boolean isQuietModeEnabled( return propertySaysQuiet && !isExplicitlyDisabledByCapability; } - /** - * Touches the session. - */ - public void updateLastAccessTime() { - lastAccess = clock.now(); - } - - public boolean isTimedOut(long timeout) { - return timeout > 0 && (lastAccess + timeout) < clock.now(); - } - public void close() { try { WebDriver driver = getDriver(); @@ -154,7 +135,6 @@ public void close() { } public WebDriver getDriver() { - updateLastAccessTime(); return driver; } @@ -256,8 +236,4 @@ public TemporaryFilesystem getTemporaryFileSystem() { return tempFs; } - public boolean isInUse() { - return inUseWithThread != null; - } - } diff --git a/java/server/src/org/openqa/selenium/remote/server/DriverServlet.java b/java/server/src/org/openqa/selenium/remote/server/DriverServlet.java index bb9d86288a394..9fb4447e41f92 100644 --- a/java/server/src/org/openqa/selenium/remote/server/DriverServlet.java +++ b/java/server/src/org/openqa/selenium/remote/server/DriverServlet.java @@ -323,7 +323,6 @@ public DriverSessions get() { if (attribute == null) { attribute = new DefaultDriverSessions( new DefaultDriverFactory(Platform.getCurrent()), - new SystemClock(), getInactiveSessionTimeout()); } return (DriverSessions) attribute; diff --git a/java/server/src/org/openqa/selenium/remote/server/SeleniumServer.java b/java/server/src/org/openqa/selenium/remote/server/SeleniumServer.java index 2a3a81925af30..c5e45b4ace7c4 100644 --- a/java/server/src/org/openqa/selenium/remote/server/SeleniumServer.java +++ b/java/server/src/org/openqa/selenium/remote/server/SeleniumServer.java @@ -128,7 +128,6 @@ public void boot() { driverSessions = new DefaultDriverSessions( new DefaultDriverFactory(Platform.getCurrent()), - new SystemClock(), TimeUnit.SECONDS.toMillis(inactiveSessionTimeoutSeconds)); handler.setAttribute(DriverServlet.SESSIONS_KEY, driverSessions); handler.setContextPath("/"); diff --git a/java/server/src/org/openqa/selenium/remote/server/Session.java b/java/server/src/org/openqa/selenium/remote/server/Session.java index 47c352a60033e..f85d4e3622a3a 100644 --- a/java/server/src/org/openqa/selenium/remote/server/Session.java +++ b/java/server/src/org/openqa/selenium/remote/server/Session.java @@ -36,16 +36,6 @@ public interface Session { String getAndClearScreenshot(); - boolean isTimedOut(long timeout); - - /** - * Indicates that the session is in use at this moment (being forwarded to browser) - * @return true if the session is active inside the browser - */ - boolean isInUse(); - - void updateLastAccessTime(); - SessionId getSessionId(); TemporaryFilesystem getTemporaryFileSystem(); diff --git a/java/server/src/org/openqa/selenium/remote/server/WebDriverServlet.java b/java/server/src/org/openqa/selenium/remote/server/WebDriverServlet.java index 9c760bb08e169..a13e45755e48b 100644 --- a/java/server/src/org/openqa/selenium/remote/server/WebDriverServlet.java +++ b/java/server/src/org/openqa/selenium/remote/server/WebDriverServlet.java @@ -69,7 +69,6 @@ public void init() throws ServletException { legacyDriverSessions = new DefaultDriverSessions( new DefaultDriverFactory(Platform.getCurrent()), - new SystemClock(), inactiveSessionTimeout); getServletContext().setAttribute(SESSIONS_KEY, legacyDriverSessions); } diff --git a/java/server/test/com/thoughtworks/selenium/webdriven/WebDriverBackedSeleniumServletTest.java b/java/server/test/com/thoughtworks/selenium/webdriven/WebDriverBackedSeleniumServletTest.java index dc41be51510e3..13d9189189a01 100644 --- a/java/server/test/com/thoughtworks/selenium/webdriven/WebDriverBackedSeleniumServletTest.java +++ b/java/server/test/com/thoughtworks/selenium/webdriven/WebDriverBackedSeleniumServletTest.java @@ -36,7 +36,6 @@ import org.openqa.selenium.remote.server.DefaultDriverFactory; import org.openqa.selenium.remote.server.DefaultDriverSessions; import org.openqa.selenium.remote.server.DriverServlet; -import org.openqa.selenium.remote.server.SystemClock; import org.seleniumhq.jetty9.server.Connector; import org.seleniumhq.jetty9.server.HttpConfiguration; import org.seleniumhq.jetty9.server.HttpConnectionFactory; @@ -60,7 +59,6 @@ public void setUpServer() throws Exception { DefaultDriverSessions webdriverSessions = new DefaultDriverSessions( new DefaultDriverFactory(Platform.getCurrent()), - new SystemClock(), 18000); handler.setAttribute(DriverServlet.SESSIONS_KEY, webdriverSessions); handler.setContextPath("/"); diff --git a/java/server/test/org/openqa/selenium/Main.java b/java/server/test/org/openqa/selenium/Main.java index 6b3208438b720..4a76040f34cac 100644 --- a/java/server/test/org/openqa/selenium/Main.java +++ b/java/server/test/org/openqa/selenium/Main.java @@ -25,7 +25,6 @@ import org.openqa.selenium.remote.server.DefaultDriverSessions; import org.openqa.selenium.remote.server.DriverServlet; import org.openqa.selenium.remote.server.DriverSessions; -import org.openqa.selenium.remote.server.SystemClock; import org.seleniumhq.jetty9.server.Connector; import org.seleniumhq.jetty9.server.Server; import org.seleniumhq.jetty9.server.ServerConnector; @@ -66,7 +65,6 @@ public static void main(String[] args) throws Exception { ServletContextHandler driverContext = new ServletContextHandler(); DriverSessions driverSessions = new DefaultDriverSessions( new DefaultDriverFactory(Platform.getCurrent()), - new SystemClock(), 18000); driverContext.setAttribute(DriverServlet.SESSIONS_KEY, driverSessions); driverContext.setContextPath("/"); diff --git a/java/server/test/org/openqa/selenium/remote/server/DefaultSessionTest.java b/java/server/test/org/openqa/selenium/remote/server/DefaultSessionTest.java index 4a551cea18e30..0d4e94cfaf03c 100644 --- a/java/server/test/org/openqa/selenium/remote/server/DefaultSessionTest.java +++ b/java/server/test/org/openqa/selenium/remote/server/DefaultSessionTest.java @@ -41,7 +41,6 @@ public void shouldClearTempFsWhenSessionCloses() throws Exception { Session session = DefaultSession.createSession( factory, tempFs, - new SystemClock(), DesiredCapabilities.firefox()); session.close(); diff --git a/java/server/test/org/openqa/selenium/remote/server/DriverSessionTest.java b/java/server/test/org/openqa/selenium/remote/server/DriverSessionTest.java index 1ff4030e254eb..9341ae6011ad9 100644 --- a/java/server/test/org/openqa/selenium/remote/server/DriverSessionTest.java +++ b/java/server/test/org/openqa/selenium/remote/server/DriverSessionTest.java @@ -33,7 +33,7 @@ public class DriverSessionTest { @Test public void testShouldRegisterCorrectDefaultsOnMac() { DriverFactory factory = new DefaultDriverFactory(Platform.MAC); - new DefaultDriverSessions(factory, new SystemClock(), 18000); + new DefaultDriverSessions(factory, 18000); assertTrue(factory.hasMappingFor(DesiredCapabilities.chrome())); assertTrue(factory.hasMappingFor(DesiredCapabilities.firefox())); @@ -43,7 +43,7 @@ public void testShouldRegisterCorrectDefaultsOnMac() { @Test public void testShouldRegisterCorrectDefaultsOnLinux() { DriverFactory factory = new DefaultDriverFactory(Platform.LINUX); - new DefaultDriverSessions(factory, new SystemClock(), 18000); + new DefaultDriverSessions(factory, 18000); assertTrue(factory.hasMappingFor(DesiredCapabilities.chrome())); assertTrue(factory.hasMappingFor(DesiredCapabilities.firefox())); @@ -53,7 +53,7 @@ public void testShouldRegisterCorrectDefaultsOnLinux() { @Test public void testShouldRegisterCorrectDefaultsOnWindows() { DriverFactory factory = new DefaultDriverFactory(Platform.VISTA); - new DefaultDriverSessions(factory, new SystemClock(), 18000); + new DefaultDriverSessions(factory, 18000); assertTrue(factory.hasMappingFor(DesiredCapabilities.chrome())); assertTrue(factory.hasMappingFor(DesiredCapabilities.firefox())); @@ -63,7 +63,7 @@ public void testShouldRegisterCorrectDefaultsOnWindows() { @Test public void testShouldBeAbleToRegisterOwnDriver() { DriverFactory factory = new DefaultDriverFactory(Platform.VISTA); - DriverSessions sessions = new DefaultDriverSessions(factory, new SystemClock(), 18000); + DriverSessions sessions = new DefaultDriverSessions(factory, 18000); Capabilities capabilities = new DesiredCapabilities("foo", "1", Platform.ANY); sessions.registerDriver(capabilities, AbstractDriver.class); diff --git a/java/server/test/org/openqa/selenium/remote/server/auth/AuthenticatedWebDriverServer.java b/java/server/test/org/openqa/selenium/remote/server/auth/AuthenticatedWebDriverServer.java index 5eacbc3016eaf..6905d0b9c3e13 100644 --- a/java/server/test/org/openqa/selenium/remote/server/auth/AuthenticatedWebDriverServer.java +++ b/java/server/test/org/openqa/selenium/remote/server/auth/AuthenticatedWebDriverServer.java @@ -22,7 +22,6 @@ import org.openqa.selenium.remote.server.DefaultDriverFactory; import org.openqa.selenium.remote.server.DefaultDriverSessions; import org.openqa.selenium.remote.server.DriverServlet; -import org.openqa.selenium.remote.server.SystemClock; import org.seleniumhq.jetty9.security.AbstractLoginService; import org.seleniumhq.jetty9.security.ConstraintMapping; import org.seleniumhq.jetty9.security.ConstraintSecurityHandler; @@ -76,7 +75,6 @@ public static void main(String[] args) throws Exception { DriverServlet.SESSIONS_KEY, new DefaultDriverSessions( new DefaultDriverFactory(Platform.getCurrent()), - new SystemClock(), 18000)); context.addServlet(new ServletHolder(DriverServlet.class), "/*"); context.setSecurityHandler(securityHandler); diff --git a/java/server/test/org/openqa/selenium/remote/server/handler/UploadFileTest.java b/java/server/test/org/openqa/selenium/remote/server/handler/UploadFileTest.java index 48d2f831314d9..4a6b8e1641734 100644 --- a/java/server/test/org/openqa/selenium/remote/server/handler/UploadFileTest.java +++ b/java/server/test/org/openqa/selenium/remote/server/handler/UploadFileTest.java @@ -41,7 +41,6 @@ import org.openqa.selenium.remote.server.DefaultSession; import org.openqa.selenium.remote.server.DriverFactory; import org.openqa.selenium.remote.server.Session; -import org.openqa.selenium.remote.server.SystemClock; import java.io.File; import java.io.IOException; @@ -73,7 +72,6 @@ public void shouldWriteABase64EncodedZippedFileToDiskAndKeepName() throws Except Session session = DefaultSession.createSession( driverFactory, tempFs, - new SystemClock(), DesiredCapabilities.firefox()); File tempFile = touch(null, "foo"); @@ -93,7 +91,6 @@ public void shouldThrowAnExceptionIfMoreThanOneFileIsSent() throws Exception { Session session = DefaultSession.createSession( driverFactory, tempFs, - new SystemClock(), DesiredCapabilities.firefox()); File baseDir = Files.createTempDir(); diff --git a/java/server/test/org/openqa/testing/TestSession.java b/java/server/test/org/openqa/testing/TestSession.java index 74954626b5f96..01d628293a427 100644 --- a/java/server/test/org/openqa/testing/TestSession.java +++ b/java/server/test/org/openqa/testing/TestSession.java @@ -92,11 +92,7 @@ public String getAndClearScreenshot() { return null; } - public boolean isTimedOut(long timeout) { - return timeout > 0 && (lastAccess + timeout) < System.currentTimeMillis(); - } - - public void updateLastAccessTime() { + private void updateLastAccessTime() { lastAccess = System.currentTimeMillis(); } @@ -108,8 +104,4 @@ public TemporaryFilesystem getTemporaryFileSystem() { return null; } - public boolean isInUse() { - return inUseWithThread != null; - } - }