Skip to content
This repository has been archived by the owner on Mar 21, 2022. It is now read-only.

Remove unnecessary null checks in AutoValue builders #925

Merged
merged 11 commits into from
Nov 20, 2017
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -387,24 +387,15 @@ static ContainerSpec create(
.stopGracePeriod(stopGracePeriod)
.healthcheck(healthcheck)
.hosts(hosts)
.dnsConfig(dnsConfig);
.dnsConfig(dnsConfig)
.command(command)
.secrets(secrets)
.configs(configs);

if (labels != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value set by autovalue is an empty map. If I remove this null check, labels changes from being an empty map to being null. This would be a backwards breaking change. See test here which passes before any changes are made to this class. The test fails if you remove the null check here.

Turns out the default is an empty collection only if you have a builder property that accumulates values.

builder.labels(labels);
}

if (command != null) {
builder.command(command);
}

if (secrets != null) {
builder.secrets(secrets);
}

if (configs != null) {
builder.configs(configs);
}

return builder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,11 @@ static DnsConfig create(
@JsonProperty("Nameservers") final List<String> nameServers,
@JsonProperty("Search") final List<String> search,
@JsonProperty("Options") final List<String> options) {
final Builder builder = builder();
if (nameServers != null) {
builder.nameServers(nameServers);
}
if (search != null) {
builder.search(search);
}
if (options != null) {
builder.options(options);
}
return builder.build();
return builder()
.nameServers(nameServers)
.search(search)
.options(options)
.build();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -188,21 +188,15 @@ static ServiceSpec create(
@JsonProperty("UpdateConfig") final UpdateConfig updateConfig,
@JsonProperty("Networks") final List<NetworkAttachmentConfig> networks,
@JsonProperty("EndpointSpec") final EndpointSpec endpointSpec) {
final Builder builder = builder()
return builder()
.name(name)
.labels(labels)
.taskTemplate(taskTemplate)
.mode(mode)
.updateConfig(updateConfig)
.endpointSpec(endpointSpec);

if (labels != null) {
builder.labels(labels);
}
if (networks != null) {
builder.networks(networks);
}

return builder.build();
.endpointSpec(endpointSpec)
.labels(labels)
.networks(networks)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,11 @@ static SwarmInit create(
@JsonProperty("AdvertiseAddr") final String advertiseAddr,
@JsonProperty("ForceNewCluster") final Boolean forceNewCluster,
@JsonProperty("Spec") final SwarmSpec swarmSpec) {
final Builder builder = builder()
return builder()
.listenAddr(listenAddr)
.advertiseAddr(advertiseAddr)
.forceNewCluster(forceNewCluster);

if (swarmSpec != null) {
builder.swarmSpec(swarmSpec);
}

return builder.build();

.forceNewCluster(forceNewCluster)
.swarmSpec(swarmSpec)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,13 @@ static TaskSpec create(
@JsonProperty("Placement") final Placement placement,
@JsonProperty("Networks") final List<NetworkAttachmentConfig> networks,
@JsonProperty("LogDriver") final Driver logDriver) {
final Builder builder = builder()
return builder()
.containerSpec(containerSpec)
.resources(resources)
.restartPolicy(restartPolicy)
.placement(placement)
.logDriver(logDriver);

if (networks != null) {
builder.networks(networks);
}

return builder.build();
.logDriver(logDriver)
.networks(networks)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*-
* -\-\-
* docker-client
* --
* Copyright (C) 2016 - 2017 Spotify AB
* --
* 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.spotify.docker.client.messages.swarm;

import static com.spotify.docker.FixtureUtil.fixture;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.spotify.docker.client.ObjectMapperProvider;
import org.junit.Test;

public class ContainerSpecTest {

private ObjectMapper objectMapper = ObjectMapperProvider.objectMapper();

@Test
public void test1_32_WithoutNullables() throws Exception {
final ContainerSpec spec = objectMapper.readValue(fixture(
"fixtures/1.32/containerSpecWithoutNullables.json"), ContainerSpec.class);
assertThat(spec.labels().size(), equalTo(0));
assertThat(spec.command(), is(nullValue()));
assertThat(spec.secrets(), is(nullValue()));
assertThat(spec.configs(), is(nullValue()));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*-
* -\-\-
* docker-client
* --
* Copyright (C) 2016 - 2017 Spotify AB
* --
* 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.spotify.docker.client.messages.swarm;

import static com.spotify.docker.FixtureUtil.fixture;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.spotify.docker.client.ObjectMapperProvider;
import org.junit.Test;

public class DnsConfigTest {

private ObjectMapper objectMapper = ObjectMapperProvider.objectMapper();

@Test
public void test1_32() throws Exception {
final DnsConfig config = objectMapper.readValue(fixture(
"fixtures/1.32/dnsConfig.json"), DnsConfig.class);
assertThat(config.nameServers(), contains("8.8.8.8"));
assertThat(config.search(), contains("example.org"));
assertThat(config.options(), contains("timeout:3"));
}

@Test
public void test1_32_WithoutNullables() throws Exception {
final DnsConfig config = objectMapper.readValue("{}", DnsConfig.class);
assertThat(config.nameServers(), is(nullValue()));
assertThat(config.search(), is(nullValue()));
assertThat(config.options(), is(nullValue()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*-
* -\-\-
* docker-client
* --
* Copyright (C) 2016 - 2017 Spotify AB
* --
* 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.spotify.docker.client.messages.swarm;

import static com.spotify.docker.FixtureUtil.fixture;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableMap;
import com.spotify.docker.client.ObjectMapperProvider;
import java.util.Collections;
import org.junit.Test;

public class DriverTest {

private ObjectMapper objectMapper = ObjectMapperProvider.objectMapper();

@Test
public void test1_32() throws Exception {
final Driver driver = objectMapper.readValue(fixture(
"fixtures/1.32/driver.json"), Driver.class);
assertThat(driver.name(), equalTo("my-driver"));
assertThat(driver.options(), equalTo(ImmutableMap.of("1", "A", "2", "B")));
}

@Test
public void test1_32_WithoutNullables() throws Exception {
final Driver driver = objectMapper.readValue("{}", Driver.class);
assertThat(driver.name(), is(nullValue()));
assertThat(driver.options(), is(Collections.<String, String>emptyMap()));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*-
* -\-\-
* docker-client
* --
* Copyright (C) 2016 - 2017 Spotify AB
* --
* 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.spotify.docker.client.messages.swarm;

import static com.spotify.docker.FixtureUtil.fixture;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.spotify.docker.client.ObjectMapperProvider;
import org.junit.Test;

public class ServiceSpecTest {

private ObjectMapper objectMapper = ObjectMapperProvider.objectMapper();

@Test
public void test1_32_WithoutNullables() throws Exception {
final ServiceSpec spec = objectMapper.readValue(fixture(
"fixtures/1.32/serviceSpecWithoutNullables.json"), ServiceSpec.class);
assertThat(spec.name(), is(nullValue()));
assertThat(spec.labels(), is(nullValue()));
assertThat(spec.taskTemplate(), is(notNullValue()));
assertThat(spec.mode(), is(nullValue()));
assertThat(spec.updateConfig(), is(nullValue()));
assertThat(spec.networks(), is(nullValue()));
assertThat(spec.endpointSpec(), is(nullValue()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*-
* -\-\-
* docker-client
* --
* Copyright (C) 2016 - 2017 Spotify AB
* --
* 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.spotify.docker.client.messages.swarm;

import static com.spotify.docker.FixtureUtil.fixture;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.spotify.docker.client.ObjectMapperProvider;
import org.junit.Test;

public class SwarmInitTest {

private ObjectMapper objectMapper = ObjectMapperProvider.objectMapper();

@Test
public void test1_32_WithoutNullables() throws Exception {
final SwarmInit init = objectMapper.readValue(fixture(
"fixtures/1.32/swarmInitWithoutNullables.json"), SwarmInit.class);
assertThat(init.listenAddr(), equalTo("0.0.0.0:2377"));
assertThat(init.advertiseAddr(), equalTo("192.168.1.1:2377"));
assertThat(init.forceNewCluster(), is(nullValue()));
assertThat(init.swarmSpec(), is(nullValue()));
}
}
Loading