Skip to content

Commit

Permalink
Cleanup null validation when configuration image (#768)
Browse files Browse the repository at this point in the history
* Cleanup null validation when configuration image
  • Loading branch information
loosebazooka authored Aug 1, 2018
1 parent c3a5d64 commit 42842a8
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 104 deletions.
18 changes: 3 additions & 15 deletions jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
@@ -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<>());
}
}

0 comments on commit 42842a8

Please sign in to comment.