Skip to content

Commit

Permalink
server: Adding a guard that prevents starting "IE instead of Opera" (…
Browse files Browse the repository at this point in the history
…or some other unwanted browser that obviously does not match the desired capabilities). Previously if a new session request is forwarded to a node it results in a driver instance creation in any case. For example, let's suppose a user starts a node with -browser browserName=opera option, and there is no operadriver in the classpath. Then the user requests a new remote driver with browserName=opera. The hub forwards the request to the node, and the node attempts to find the "best matching" driver provider. As far as opera is not available, it can start any other browser. Because "best matching" does not imply matching. The new guard prevents this unwanted behavior.
  • Loading branch information
barancev committed Jun 8, 2015
1 parent ed4a89e commit 42b8f6b
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import org.openqa.selenium.Capabilities;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebDriverException;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -54,7 +55,14 @@ DriverProvider getProviderMatching(Capabilities desired) {
}

public WebDriver newInstance(Capabilities capabilities) {
return getProviderMatching(capabilities).newInstance(capabilities);
DriverProvider provider = getProviderMatching(capabilities);
if (provider.canCreateDriverInstanceFor(capabilities)) {
return getProviderMatching(capabilities).newInstance(capabilities);
} else {
throw new WebDriverException(String.format(
"The best matching driver provider %s can't create a new driver instance for %s",
provider, capabilities));
}
}

public boolean hasMappingFor(Capabilities capabilities) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* This driver provider uses reflection to find and call a driver constructor that accepts
* a parameter of Capabilities type.
*/
public class DefaultDriverProvider implements DriverProvider {

private static final Logger LOG = Logger.getLogger(DefaultDriverProvider.class.getName());
Expand All @@ -48,11 +52,25 @@ public Capabilities getProvidedCapabilities() {
return capabilities;
}

/**
* Checks that driver class can be loaded.
*/
@Override
public boolean canCreateDriverInstances() {
return getDriverClass() != null;
}

/**
* Checks that the browser name set in the provided capabilities matches the browser name
* set in the desired capabilities.
* @param capabilities The desired capabilities
* @return true if the browser name is the same, false otherwise
*/
@Override
public boolean canCreateDriverInstanceFor(Capabilities capabilities) {
return this.capabilities.getBrowserName().equals(capabilities.getBrowserName());
}

private Class<? extends WebDriver> getDriverClass() {
if (driverClass != null) {
return driverClass;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,19 @@ public interface DriverProvider {
Capabilities getProvidedCapabilities();

/**
* Checks if the provider can create driver instances.
* Checks if the provider can create driver instances "in general".
*
* @return true if the provider can create driver instances.
*/
boolean canCreateDriverInstances();

/**
* Checks if the provider can create driver instance with the desired capabilities.
*
* @return true if the provider can create driver instance with the desired capabilities.
*/
boolean canCreateDriverInstanceFor(Capabilities capabilities);

/**
* Creates a new driver instance. The specified capabilities are to be passed to the driver
* constructor.
Expand Down

0 comments on commit 42b8f6b

Please sign in to comment.