Cleanup null validation when configuring image

jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java

@@ -82,11 +82,7 @@ public Builder<T> setEnvironmentVariable(String name, String value) {
      * @return this
      */
     public Builder<T> setEntrypoint(@Nullable List<String> entrypoint) {
-      if (entrypoint == null) {
-        this.entrypoint = null;
-      } else {
-        this.entrypoint = ImmutableList.copyOf(entrypoint);
-      }
+      this.entrypoint = (entrypoint == null) ? null : ImmutableList.copyOf(entrypoint);
       return this;
     }
 
@@ -97,11 +93,7 @@ public Builder<T> setEntrypoint(@Nullable List<String> entrypoint) {
      * @return this
      */
     public Builder<T> setJavaArguments(@Nullable List<String> javaArguments) {
-      if (javaArguments == null) {
-        this.javaArguments = null;
-      } else {
-        this.javaArguments = ImmutableList.copyOf(javaArguments);
-      }
+      this.javaArguments = (javaArguments == null) ? null : ImmutableList.copyOf(javaArguments);
       return this;
     }
 
@@ -112,11 +104,7 @@ public Builder<T> setJavaArguments(@Nullable List<String> javaArguments) {
      * @return this
      */
     public Builder<T> setExposedPorts(@Nullable List<Port> exposedPorts) {
-      if (exposedPorts == null) {
-        this.exposedPorts = null;
-      } else {
-        this.exposedPorts = ImmutableList.copyOf(exposedPorts);
-      }
+      this.exposedPorts = (exposedPorts == null) ? null : ImmutableList.copyOf(exposedPorts);
       return this;
     }
 

jib-core/src/test/java/com/google/cloud/tools/jib/configuration/BuildConfigurationTest.java

@@ -29,11 +29,8 @@
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
-import java.util.TreeMap;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -215,90 +212,4 @@ public void testBuilder_missingValues() {
           ex.getMessage());
     }
   }
-
-  @Test
-  @SuppressWarnings("JdkObsolete")
-  public void testBuilder_nullValues() {
-    // Java arguments element should not be null.
-    try {
-      BuildConfiguration.builder(Mockito.mock(JibLogger.class))
-          .setContainerConfiguration(
-              ContainerConfiguration.builder()
-                  .setProgramArguments(Arrays.asList("first", null))
-                  .build());
-      Assert.fail("The IllegalArgumentException should be thrown.");
-    } catch (IllegalArgumentException ex) {
-      Assert.assertNull(ex.getMessage());
-    }
-
-    // Entrypoint element should not be null.
-    try {
-      BuildConfiguration.builder(Mockito.mock(JibLogger.class))
-          .setContainerConfiguration(
-              ContainerConfiguration.builder().setEntrypoint(Arrays.asList("first", null)).build());
-      Assert.fail("The IllegalArgumentException should be thrown.");
-    } catch (IllegalArgumentException ex) {
-      Assert.assertNull(ex.getMessage());
-    }
-
-    // Exposed ports element should not be null.
-    try {
-      BuildConfiguration.builder(Mockito.mock(JibLogger.class))
-          .setContainerConfiguration(
-              ContainerConfiguration.builder()
-                  .setExposedPorts(Arrays.asList(new Port(1000, Protocol.TCP), null))
-                  .build());
-      Assert.fail("The IllegalArgumentException should be thrown.");
-    } catch (IllegalArgumentException ex) {
-      Assert.assertNull(ex.getMessage());
-    }
-
-    // Labels element should not be null.
-    Map<String, String> badLabels = new HashMap<>();
-    badLabels.put("label-key", null);
-    try {
-      ContainerConfiguration.builder().setLabels(badLabels);
-      Assert.fail("The IllegalArgumentException should be thrown.");
-    } catch (IllegalArgumentException ex) {
-      Assert.assertNull(ex.getMessage());
-    }
-
-    // Environment keys element should not be null.
-    Map<String, String> nullKeyMap = new HashMap<>();
-    nullKeyMap.put(null, "value");
-
-    try {
-      BuildConfiguration.builder(Mockito.mock(JibLogger.class))
-          .setContainerConfiguration(
-              ContainerConfiguration.builder().setEnvironment(nullKeyMap).build());
-      Assert.fail("The IllegalArgumentException should be thrown.");
-    } catch (IllegalArgumentException ex) {
-      Assert.assertNull(ex.getMessage());
-    }
-
-    // Environment values element should not be null.
-    Map<String, String> nullValueMap = new HashMap<>();
-    nullValueMap.put("key", null);
-    try {
-      BuildConfiguration.builder(Mockito.mock(JibLogger.class))
-          .setContainerConfiguration(
-              ContainerConfiguration.builder().setEnvironment(nullValueMap).build());
-      Assert.fail("The IllegalArgumentException should be thrown.");
-    } catch (IllegalArgumentException ex) {
-      Assert.assertNull(ex.getMessage());
-    }
-
-    // Can accept empty environment.
-    BuildConfiguration.builder(Mockito.mock(JibLogger.class))
-        .setContainerConfiguration(
-            ContainerConfiguration.builder().setEnvironment(ImmutableMap.of()).build());
-
-    // Environment map can accept TreeMap and Hashtable.
-    BuildConfiguration.builder(Mockito.mock(JibLogger.class))
-        .setContainerConfiguration(
-            ContainerConfiguration.builder().setEnvironment(new TreeMap<>()).build());
-    BuildConfiguration.builder(Mockito.mock(JibLogger.class))
-        .setContainerConfiguration(
-            ContainerConfiguration.builder().setEnvironment(new Hashtable<>()).build());
-  }
 }

jib-core/src/test/java/com/google/cloud/tools/jib/configuration/ContainerConfigurationTest.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright 2018 Google LLC. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.google.cloud.tools.jib.configuration;
+
+import com.google.cloud.tools.jib.configuration.Port.Protocol;
+import com.google.common.collect.ImmutableMap;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeMap;
+import org.junit.Assert;
+import org.junit.Test;
+
+/** Tests for {@link ContainerConfiguration}. */
+public class ContainerConfigurationTest {
+
+  @Test
+  public void testBuilder_nullValues() {
+    // Java arguments element should not be null.
+    try {
+      ContainerConfiguration.builder().setProgramArguments(Arrays.asList("first", null));
+      Assert.fail("The IllegalArgumentException should be thrown.");
+    } catch (IllegalArgumentException ex) {
+      Assert.assertNull(ex.getMessage());
+    }
+
+    // Entrypoint element should not be null.
+    try {
+      ContainerConfiguration.builder().setEntrypoint(Arrays.asList("first", null));
+      Assert.fail("The IllegalArgumentException should be thrown.");
+    } catch (IllegalArgumentException ex) {
+      Assert.assertNull(ex.getMessage());
+    }
+
+    // Exposed ports element should not be null.
+    List<Port> badPorts = Arrays.asList(new Port(1000, Protocol.TCP), null);
+    try {
+      ContainerConfiguration.builder().setExposedPorts(badPorts);
+      Assert.fail("The IllegalArgumentException should be thrown.");
+    } catch (IllegalArgumentException ex) {
+      Assert.assertNull(ex.getMessage());
+    }
+
+    // Labels element should not be null.
+    Map<String, String> badLabels = new HashMap<>();
+    badLabels.put("label-key", null);
+    try {
+      ContainerConfiguration.builder().setLabels(badLabels);
+      Assert.fail("The IllegalArgumentException should be thrown.");
+    } catch (IllegalArgumentException ex) {
+      Assert.assertNull(ex.getMessage());
+    }
+
+    // Environment keys element should not be null.
+    Map<String, String> nullKeyMap = new HashMap<>();
+    nullKeyMap.put(null, "value");
+
+    try {
+      ContainerConfiguration.builder().setEnvironment(nullKeyMap);
+      Assert.fail("The IllegalArgumentException should be thrown.");
+    } catch (IllegalArgumentException ex) {
+      Assert.assertNull(ex.getMessage());
+    }
+
+    // Environment values element should not be null.
+    Map<String, String> nullValueMap = new HashMap<>();
+    nullValueMap.put("key", null);
+    try {
+      ContainerConfiguration.builder().setEnvironment(nullValueMap);
+      Assert.fail("The IllegalArgumentException should be thrown.");
+    } catch (IllegalArgumentException ex) {
+      Assert.assertNull(ex.getMessage());
+    }
+  }
+
+  @Test
+  @SuppressWarnings("JdkObsolete") // for hashtable
+  public void testBuilder_environmentMapTypes() {
+    // Can accept empty environment.
+    ContainerConfiguration.builder().setEnvironment(ImmutableMap.of()).build();
+
+    // Can handle other map types (https://github.com/GoogleContainerTools/jib/issues/632)
+    ContainerConfiguration.builder().setEnvironment(new TreeMap<>());
+    ContainerConfiguration.builder().setEnvironment(new Hashtable<>());
+  }
+}