Skip to content

Commit

Permalink
Delete methods from Session that were used by SessionCleaner
Browse files Browse the repository at this point in the history
This makes the interface more focussed. That's a
Good Thing, right?
  • Loading branch information
shs96c committed Jun 23, 2017
1 parent b988885 commit 56b215a
Show file tree
Hide file tree
Showing 14 changed files with 6 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -79,7 +78,6 @@ public WebDriverBackedSeleniumServlet() {
if (attribute == null) {
attribute = new DefaultDriverSessions(
new DefaultDriverFactory(Platform.getCurrent()),
new SystemClock(),
MINUTES.toMillis(5));
}
return (DriverSessions) attribute;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,13 @@
public class DefaultDriverSessions implements DriverSessions {

private final DriverFactory factory;
private final Clock clock;

private final Cache<SessionId, Session> sessionIdToDriver;

public DefaultDriverSessions(
DriverFactory factory,
Clock clock,
long inactiveSessionTimeoutMs) {
this.factory = factory;
this.clock = clock;

RemovalListener<SessionId, Session> listener = notification -> {
Session session = notification.getValue();
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -101,8 +95,6 @@ private DefaultSession(
this.driver = initialDriver;
this.capabilities = browserCreator.getCapabilityDescription();
this.sessionId = browserCreator.getSessionId();

updateLastAccessTime();
}

private static boolean isQuietModeEnabled(
Expand All @@ -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();
Expand All @@ -154,7 +135,6 @@ public void close() {
}

public WebDriver getDriver() {
updateLastAccessTime();
return driver;
}

Expand Down Expand Up @@ -256,8 +236,4 @@ public TemporaryFilesystem getTemporaryFileSystem() {
return tempFs;
}

public boolean isInUse() {
return inUseWithThread != null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,6 @@ public DriverSessions get() {
if (attribute == null) {
attribute = new DefaultDriverSessions(
new DefaultDriverFactory(Platform.getCurrent()),
new SystemClock(),
getInactiveSessionTimeout());
}
return (DriverSessions) attribute;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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("/");
Expand Down
10 changes: 0 additions & 10 deletions java/server/src/org/openqa/selenium/remote/server/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ public void init() throws ServletException {

legacyDriverSessions = new DefaultDriverSessions(
new DefaultDriverFactory(Platform.getCurrent()),
new SystemClock(),
inactiveSessionTimeout);
getServletContext().setAttribute(SESSIONS_KEY, legacyDriverSessions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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("/");
Expand Down
2 changes: 0 additions & 2 deletions java/server/test/org/openqa/selenium/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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("/");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public void shouldClearTempFsWhenSessionCloses() throws Exception {

Session session = DefaultSession.createSession(
factory, tempFs,
new SystemClock(),
DesiredCapabilities.firefox());

session.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand All @@ -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()));
Expand All @@ -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()));
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,7 +72,6 @@ public void shouldWriteABase64EncodedZippedFileToDiskAndKeepName() throws Except
Session session = DefaultSession.createSession(
driverFactory,
tempFs,
new SystemClock(),
DesiredCapabilities.firefox());

File tempFile = touch(null, "foo");
Expand All @@ -93,7 +91,6 @@ public void shouldThrowAnExceptionIfMoreThanOneFileIsSent() throws Exception {
Session session = DefaultSession.createSession(
driverFactory,
tempFs,
new SystemClock(),
DesiredCapabilities.firefox());
File baseDir = Files.createTempDir();

Expand Down
10 changes: 1 addition & 9 deletions java/server/test/org/openqa/testing/TestSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -108,8 +104,4 @@ public TemporaryFilesystem getTemporaryFileSystem() {
return null;
}

public boolean isInUse() {
return inUseWithThread != null;
}

}

0 comments on commit 56b215a

Please sign in to comment.