update code as suggestions

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
SeleniumHQ · Apr 12, 2024 · 39e101d · 39e101d
1 parent a73ba2f
commit 39e101d
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 14 deletions.
diff --git a/java/src/org/openqa/selenium/docker/ContainerConfig.java b/java/src/org/openqa/selenium/docker/ContainerConfig.java
@@ -53,7 +53,15 @@ public ContainerConfig(
       List<Device> devices,
       String networkName,
       long shmSize) {
-    this(image, portBindings, envVars, volumeBinds, devices, networkName, shmSize, new HashMap<>());
+    this(
+        image,
+        portBindings,
+        envVars,
+        volumeBinds,
+        devices,
+        networkName,
+        shmSize,
+        ImmutableMap.of());
   }
 
   public ContainerConfig(
@@ -138,7 +146,7 @@ public ContainerConfig devices(List<Device> devices) {
         image, portBindings, envVars, volumeBinds, devices, networkName, shmSize);
   }
 
-  public ContainerConfig getHostConfig(Map<String, Object> hostConfig, List<String> configKeys) {
+  public ContainerConfig applyHostConfig(Map<String, Object> hostConfig, List<String> configKeys) {
     Map<String, Object> setHostConfig =
         configKeys.stream()
             .filter(hostConfig::containsKey)
@@ -203,7 +211,7 @@ private Map<String, Object> toJson() {
             "Binds", volumeBinds,
             "Devices", devicesMapping);
 
-    if (this.hostConfig != null && !this.hostConfig.isEmpty()) {
+    if (!this.hostConfig.isEmpty()) {
       Map<String, Object> copyMap = new HashMap<>(hostConfig);
       copyMap.putAll(this.hostConfig);
       hostConfig = ImmutableMap.copyOf(copyMap);

diff --git a/java/src/org/openqa/selenium/docker/v1_41/InspectContainer.java b/java/src/org/openqa/selenium/docker/v1_41/InspectContainer.java
@@ -23,6 +23,7 @@
 import static org.openqa.selenium.remote.http.HttpMethod.GET;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Logger;
@@ -67,7 +68,8 @@ public ContainerInfo apply(ContainerId id) {
     ArrayList<Object> mounts = (ArrayList<Object>) rawInspectInfo.get("Mounts");
     List<Map<String, Object>> mountedVolumes =
         mounts.stream().map(mount -> (Map<String, Object>) mount).collect(Collectors.toList());
-    Map<String, Object> hostConfig = (Map<String, Object>) rawInspectInfo.get("HostConfig");
+    Map<String, Object> hostConfig =
+        (Map<String, Object>) rawInspectInfo.getOrDefault("HostConfig", Collections.emptyMap());
 
     return new ContainerInfo(id, ip, mountedVolumes, networkName, hostConfig);
   }

diff --git a/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java b/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java
@@ -72,13 +72,13 @@ public class DockerFlags implements HasRoles {
   @Parameter(
       names = {"--docker-host-config-keys"},
       description =
-          "Node container host config keys to be passed to the browsers container. Keys name can be"
-              + " found in the Docker API documentation, or by running `docker inspect` the"
-              + " node-docker container.")
+          "Specify which docker host configuration keys should be passed to browser containers."
+              + " Keys name can be found in the Docker API documentation, or by running `docker"
+              + " inspect` the node-docker container.")
   @ConfigValue(
       section = DockerOptions.DOCKER_SECTION,
       name = "host-config-keys",
-      example = "[\"Dns\", \"DnsOptions\", \"DnsSearch\", \"ExtraHosts\"]")
+      example = "[\"Dns\", \"DnsOptions\", \"DnsSearch\", \"ExtraHosts\", \"Binds\"]")
   private List<String> hostConfigKeys;
 
   @Parameter(

diff --git a/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java b/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java
@@ -237,7 +237,7 @@ private String getDockerNetworkName(Optional<ContainerInfo> info) {
 
   @SuppressWarnings("OptionalUsedAsFieldOrParameterType")
   private Map<String, Object> getDockerHostConfig(Optional<ContainerInfo> info) {
-    return info.map(ContainerInfo::getHostConfig).orElse(null);
+    return info.map(ContainerInfo::getHostConfig).orElse(Collections.emptyMap());
   }
 
   @SuppressWarnings("OptionalUsedAsFieldOrParameterType")

diff --git a/java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java b/java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java
@@ -160,8 +160,7 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
               ? "Creating container..."
               : "Creating container, mapping container port 4444 to " + port;
       LOG.info(logMessage);
-      Container container =
-          createBrowserContainer(port, sessionRequest.getDesiredCapabilities(), this.hostConfig);
+      Container container = createBrowserContainer(port, sessionRequest.getDesiredCapabilities());
       container.start();
       ContainerInfo containerInfo = container.inspect();
 
@@ -284,8 +283,7 @@ private Capabilities addForwardCdpEndpoint(
         .setCapability("se:forwardCdp", forwardCdpPath);
   }
 
-  private Container createBrowserContainer(
-      int port, Capabilities sessionCapabilities, Map<String, Object> hostConfig) {
+  private Container createBrowserContainer(int port, Capabilities sessionCapabilities) {
     Map<String, String> browserContainerEnvVars = getBrowserContainerEnvVars(sessionCapabilities);
     long browserContainerShmMemorySize = 2147483648L; // 2GB
     ContainerConfig containerConfig =
@@ -294,7 +292,7 @@ private Container createBrowserContainer(
             .shmMemorySize(browserContainerShmMemorySize)
             .network(networkName)
             .devices(devices)
-            .getHostConfig(hostConfig, hostConfigKeys);
+            .applyHostConfig(hostConfig, hostConfigKeys);
     if (!runningInDocker) {
       containerConfig = containerConfig.map(Port.tcp(4444), Port.tcp(port));
     }