[java] Synchronize method to get Selenium Manager binary (fix #11620)

[bonigarcia]

Description

The method to get the Selenium Manager binary in the Java bindings is not synchronized, which leading to concurrent problems to get Selenium Manager in Java, as reported in #11620. I have confirmed the bug using the provided example repo. I used that repo as well to test the fix.

Motivation and Context

This PR fixes #11620.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[krmahadevan]

@bonigarcia - I think there's a larger problem here.

• org.openqa.selenium.manager.SeleniumManager has been built to be a singleton.
• The current implementation (as in 4.8.0) right now is such that, it attempts to locate the binary data member ONLY once. This means that lets say I have 2 test methods say firefoxTest() and chromeTest() then the following will happen if I run them in parallel

Assumptions (Only this will kick in the SeleniumManager into action)

• chromedriver and geckodriver are NOT available in my PATH

• I am not specifying the location of chromedriver and geckodriver via the JVM arguments.

• firefoxTest() will cause the binary data member in the SeleniumManager singleton to be set.

• Now because of this edit check we will end up returning the path of geckodriver to chromeTest This will be wrong.

I think the fix should be to do something like below (I have changed SeleniumManager to be an enum since enums are first class singletons)

public enum SeleniumManager {
    manager;
    private static final Logger LOG = Logger.getLogger(SeleniumManager.class.getName());

    private static final String SELENIUM_MANAGER = "selenium-manager";
    private static final String EXE = ".exe";
    private static final String INFO = "INFO\t";

    //We cache the binary path on a per driver flavor basis.
    private final Map<String, File> binaries = new ConcurrentHashMap<>();

    SeleniumManager() {
        Runtime.getRuntime().addShutdownHook(new Thread(() -> binaries.values()
          .stream()
          .filter(Objects::nonNull)
          .filter(File::exists)
          .forEach(SeleniumManager::deleteQuietly)));
    }

  private static void deleteQuietly(File binary) {
    try {
      Files.delete(binary.toPath());
    } catch (IOException e) {
      LOG.warning(String.format("%s deleting temporal file: %s",
        e.getClass().getSimpleName(), e.getMessage()));
    }

  }

    public SeleniumManager getInstance() {
        return manager;
    }

  private File getBinary(String driverName) {
    return this.binaries.computeIfAbsent(driverName, flavor -> {
      try {
        Platform current = Platform.getCurrent();
        String folder = "linux";
        String extension = "";
        if (current.is(WINDOWS)) {
          extension = EXE;
          folder = "windows";
        } else if (current.is(MAC)) {
          folder = "macos";
        }
        String binaryPath = String.format("%s/%s%s", folder, SELENIUM_MANAGER, extension);
        try (InputStream inputStream = this.getClass().getResourceAsStream(binaryPath)) {
          Path tmpPath = Files.createTempDirectory(SELENIUM_MANAGER + System.nanoTime());
          File tmpFolder = tmpPath.toFile();
          tmpFolder.deleteOnExit();
          File binary = new File(tmpFolder, SELENIUM_MANAGER + extension);
          Files.copy(inputStream, binary.toPath(), REPLACE_EXISTING);
          binary.setExecutable(true);
          return binary;
        }
      } catch (Exception e) {
        throw new WebDriverException("Unable to obtain Selenium Manager", e);
      }
    });
  }

    public String getDriverPath(String driverName) {
        if (!ImmutableList.of("geckodriver", "chromedriver", "msedgedriver", "IEDriverServer").contains(driverName)) {
            throw new WebDriverException("Unable to locate driver with name: " + driverName);
        }

        String driverPath = null;
        File binaryFile = getBinary(driverName);
        if (binaryFile != null) {
          driverPath = runCommand(binaryFile.getAbsolutePath(),
                    "--driver", driverName.replaceAll(EXE, ""));
        }
        return driverPath;
    }
}

[bonigarcia]

• Now because of this edit check we will end up returning the path of geckodriver to chromeTest This will be wrong.

That is wrong. That binary path is the Selenium Manager binary, not the driver. The Selenium Manager binary is used to manage (i.e., download, cache, etc.) the drivers (geckodriver, chromedriver, etc.).

[krmahadevan]

• Now because of this edit check we will end up returning the path of geckodriver to chromeTest This will be wrong.

That is wrong. That binary path is the Selenium Manager binary, not the driver. The Selenium Manager binary is used to manage (i.e., download, cache, etc.) the drivers (geckodriver, chromedriver, etc.).

Ah! Yeah now I realise my blunder. The SeleniumManager internally relies on the rust binary to do the actual download. Sorry for the confusion @bonigarcia

[titusfortner]

Is the issue getting the binary or executing the binary? I'm a little confused because of the singleton; does synchronizing that method actually prevent two threads from attempting to execute the binary at the same time? When does the first thread relinquish the lock?

[diemol]

LGTM!

Failing tests are not related to this change.