From c82b92d38374e242e8130af7282f8cf59c91297d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Warcha=C5=82?= Date: Tue, 15 Nov 2016 13:48:57 +0100 Subject: [PATCH] Allow multiple tags in build image command Several tags for image can be defined by calling multiple time withTag() method of BuildImageCmd. In Netty implementation, the WebTarget class is modified to support several query parameteres with the same name. Fix #720 --- .../dockerjava/api/command/BuildImageCmd.java | 19 +++++ .../core/command/BuildImageCmdImpl.java | 24 +++++- .../dockerjava/jaxrs/BuildImageCmdExec.java | 9 +- .../github/dockerjava/netty/WebTarget.java | 85 +++++++++++++++++-- .../netty/exec/BuildImageCmdExec.java | 34 +++----- .../core/command/BuildImageCmdImplTest.java | 25 ++++++ .../netty/exec/BuildImageCmdExecTest.java | 25 ++++++ 7 files changed, 189 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/github/dockerjava/api/command/BuildImageCmd.java b/src/main/java/com/github/dockerjava/api/command/BuildImageCmd.java index ac98db2f9..a36b1b17e 100644 --- a/src/main/java/com/github/dockerjava/api/command/BuildImageCmd.java +++ b/src/main/java/com/github/dockerjava/api/command/BuildImageCmd.java @@ -4,6 +4,7 @@ import java.io.InputStream; import java.net.URI; import java.util.Map; +import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -34,10 +35,21 @@ public interface BuildImageCmd extends AsyncDockerCmd getTags(); + /** * "remote" in API */ @@ -109,8 +121,15 @@ public interface BuildImageCmd extends AsyncDockerCmd)} + */ + @Deprecated BuildImageCmd withTag(String tag); + BuildImageCmd withTags(Set tags); + BuildImageCmd withRemote(URI remote); BuildImageCmd withBaseDirectory(File baseDirectory); diff --git a/src/main/java/com/github/dockerjava/core/command/BuildImageCmdImpl.java b/src/main/java/com/github/dockerjava/core/command/BuildImageCmdImpl.java index c34b05155..97629568c 100644 --- a/src/main/java/com/github/dockerjava/core/command/BuildImageCmdImpl.java +++ b/src/main/java/com/github/dockerjava/core/command/BuildImageCmdImpl.java @@ -8,6 +8,7 @@ import java.net.URI; import java.util.HashMap; import java.util.Map; +import java.util.Set; import com.github.dockerjava.api.command.BuildImageCmd; import com.github.dockerjava.api.model.AuthConfigurations; @@ -15,17 +16,20 @@ import com.github.dockerjava.core.dockerfile.Dockerfile; import com.github.dockerjava.core.util.FilePathUtil; +import javax.annotation.CheckForNull; + /** - * * Build an image from Dockerfile. - * */ public class BuildImageCmdImpl extends AbstrAsyncDockerCmd implements BuildImageCmd { private InputStream tarInputStream; + @Deprecated private String tag; + private Set tags; + private Boolean noCache; private Boolean remove = true; @@ -82,11 +86,17 @@ public BuildImageCmdImpl(BuildImageCmd.Exec exec, InputStream tarInputStream) { // getters API + @Deprecated @Override public String getTag() { return tag; } + @CheckForNull + public Set getTags() { + return tags; + } + @Override public URI getRemote() { return remote; @@ -178,6 +188,10 @@ public Long getShmsize() { // setters + /** + * @deprecated use #withTags() + */ + @Deprecated @Override public BuildImageCmdImpl withTag(String tag) { checkNotNull(tag, "Tag is null"); @@ -185,6 +199,12 @@ public BuildImageCmdImpl withTag(String tag) { return this; } + @Override + public BuildImageCmd withTags(Set tags) { + this.tags = tags; + return this; + } + @Override public BuildImageCmd withRemote(URI remote) { this.remote = remote; diff --git a/src/main/java/com/github/dockerjava/jaxrs/BuildImageCmdExec.java b/src/main/java/com/github/dockerjava/jaxrs/BuildImageCmdExec.java index 2a7aea40e..ae2302b0d 100644 --- a/src/main/java/com/github/dockerjava/jaxrs/BuildImageCmdExec.java +++ b/src/main/java/com/github/dockerjava/jaxrs/BuildImageCmdExec.java @@ -1,6 +1,7 @@ package com.github.dockerjava.jaxrs; import static javax.ws.rs.client.Entity.entity; +import static org.apache.commons.lang.StringUtils.isNotBlank; import javax.ws.rs.client.Invocation; import javax.ws.rs.client.WebTarget; @@ -65,9 +66,15 @@ protected AbstractCallbackNotifier callbackNotifier(BuildImag if (dockerFilePath != null && command.getRemote() == null && !"Dockerfile".equals(dockerFilePath)) { webTarget = webTarget.queryParam("dockerfile", dockerFilePath); } - if (command.getTag() != null) { + + if (command.getTags() != null && !command.getTags().isEmpty()) { + for (String t : command.getTags()) { + webTarget = webTarget.queryParam("t", t); + } + } else if (isNotBlank(command.getTag())) { webTarget = webTarget.queryParam("t", command.getTag()); } + if (command.getRemote() != null) { webTarget = webTarget.queryParam("remote", command.getRemote().toString()); } diff --git a/src/main/java/com/github/dockerjava/netty/WebTarget.java b/src/main/java/com/github/dockerjava/netty/WebTarget.java index ef1510f5c..96fb42bec 100644 --- a/src/main/java/com/github/dockerjava/netty/WebTarget.java +++ b/src/main/java/com/github/dockerjava/netty/WebTarget.java @@ -1,22 +1,32 @@ package com.github.dockerjava.netty; +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; +import java.nio.charset.Charset; +import java.nio.charset.UnsupportedCharsetException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Set; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableSet; +import io.netty.handler.codec.http.HttpConstants; import org.apache.commons.lang.StringUtils; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; /** - * This class is basically a replacement of javax.ws.rs.client.WebTarget to allow simpler migration of JAX-RS code to a netty based + * This class is basically a replacement of {@link javax.ws.rs.client.WebTarget} to allow simpler migration of JAX-RS code to a netty based * implementation. * * @author Marcus Linke */ public class WebTarget { + private static final ObjectMapper MAPPER = new ObjectMapper(); private final ChannelProvider channelProvider; @@ -24,18 +34,26 @@ public class WebTarget { private final ImmutableMap queryParams; + /** + * Multiple values for the same name param. + */ + private final ImmutableMap> queryParamsSet; + private static final String PATH_SEPARATOR = "/"; public WebTarget(ChannelProvider channelProvider) { - this(channelProvider, ImmutableList.of(), ImmutableMap.of()); + this(channelProvider, ImmutableList.of(), ImmutableMap.of(), + ImmutableMap.>of()); } private WebTarget(ChannelProvider channelProvider, ImmutableList path, - ImmutableMap queryParams) { + ImmutableMap queryParams, + ImmutableMap> queryParamsSet) { this.channelProvider = channelProvider; this.path = path; this.queryParams = queryParams; + this.queryParamsSet = queryParamsSet; } public WebTarget path(String... components) { @@ -45,7 +63,7 @@ public WebTarget path(String... components) { newPath.addAll(Arrays.asList(StringUtils.split(component, PATH_SEPARATOR))); } - return new WebTarget(channelProvider, newPath.build(), queryParams); + return new WebTarget(channelProvider, newPath.build(), queryParams, queryParamsSet); } public InvocationBuilder request() { @@ -53,7 +71,13 @@ public InvocationBuilder request() { List params = new ArrayList<>(); for (Map.Entry entry : queryParams.entrySet()) { - params.add(entry.getKey() + "=" + entry.getValue()); + params.add(entry.getKey() + "=" + encodeComponent(entry.getValue(), HttpConstants.DEFAULT_CHARSET)); + } + + for (Map.Entry> entry : queryParamsSet.entrySet()) { + for (String entryValueValue : entry.getValue()) { + params.add(entry.getKey() + "=" + encodeComponent(entryValueValue, HttpConstants.DEFAULT_CHARSET)); + } } if (!params.isEmpty()) { @@ -63,13 +87,25 @@ public InvocationBuilder request() { return new InvocationBuilder(channelProvider, resource); } + /** + * @see io.netty.handler.codec.http.QueryStringEncoder + */ + private static String encodeComponent(String s, Charset charset) { + // TODO: Optimize me. + try { + return URLEncoder.encode(s, charset.name()).replace("+", "%20"); + } catch (UnsupportedEncodingException ignored) { + throw new UnsupportedCharsetException(charset.name()); + } + } + public WebTarget resolveTemplate(String name, Object value) { ImmutableList.Builder newPath = ImmutableList.builder(); for (String component : path) { component = component.replaceAll("\\{" + name + "\\}", value.toString()); newPath.add(component); } - return new WebTarget(channelProvider, newPath.build(), queryParams); + return new WebTarget(channelProvider, newPath.build(), queryParams, queryParamsSet); } public WebTarget queryParam(String name, Object value) { @@ -77,7 +113,32 @@ public WebTarget queryParam(String name, Object value) { if (value != null) { builder.put(name, value.toString()); } - return new WebTarget(channelProvider, path, builder.build()); + return new WebTarget(channelProvider, path, builder.build(), queryParamsSet); + } + + public WebTarget queryParamsSet(String name, Set values) { + ImmutableMap.Builder> builder = ImmutableMap.>builder().putAll(queryParamsSet); + if (values != null) { + ImmutableSet.Builder valueBuilder = ImmutableSet.builder(); + for (Object value : values) { + valueBuilder.add(value.toString()); + } + builder.put(name, valueBuilder.build()); + } + return new WebTarget(channelProvider, path, queryParams, builder.build()); + } + + public WebTarget queryParamsJsonMap(String name, Map values) { + if (values != null && !values.isEmpty()) { + try { + // when param value is JSON string + return queryParam(name, MAPPER.writeValueAsString(values)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } else { + return this; + } } @Override @@ -97,7 +158,14 @@ public boolean equals(Object o) { if (path != null ? !path.equals(webTarget.path) : webTarget.path != null) { return false; } - return queryParams != null ? queryParams.equals(webTarget.queryParams) : webTarget.queryParams == null; + if (queryParams != null ? !queryParams.equals(webTarget.queryParams) : webTarget.queryParams != null) { + return false; + } + if (queryParamsSet != null ? !queryParamsSet.equals(webTarget.queryParamsSet) : webTarget.queryParamsSet != null) { + return false; + } + + return true; } @Override @@ -105,6 +173,7 @@ public int hashCode() { int result = channelProvider != null ? channelProvider.hashCode() : 0; result = 31 * result + (path != null ? path.hashCode() : 0); result = 31 * result + (queryParams != null ? queryParams.hashCode() : 0); + result = 31 * result + (queryParamsSet != null ? queryParamsSet.hashCode() : 0); return result; } } diff --git a/src/main/java/com/github/dockerjava/netty/exec/BuildImageCmdExec.java b/src/main/java/com/github/dockerjava/netty/exec/BuildImageCmdExec.java index 43152570e..bbca686e0 100644 --- a/src/main/java/com/github/dockerjava/netty/exec/BuildImageCmdExec.java +++ b/src/main/java/com/github/dockerjava/netty/exec/BuildImageCmdExec.java @@ -1,6 +1,5 @@ package com.github.dockerjava.netty.exec; -import com.fasterxml.jackson.databind.ObjectMapper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -14,15 +13,12 @@ import com.github.dockerjava.netty.MediaType; import com.github.dockerjava.netty.WebTarget; -import java.io.IOException; -import java.util.Map; +import static org.apache.commons.lang.StringUtils.isNotBlank; public class BuildImageCmdExec extends AbstrAsyncDockerCmdExec implements BuildImageCmd.Exec { private static final Logger LOGGER = LoggerFactory.getLogger(BuildImageCmdExec.class); - private static final ObjectMapper MAPPER = new ObjectMapper(); - public BuildImageCmdExec(WebTarget baseResource, DockerClientConfig dockerClientConfig) { super(baseResource, dockerClientConfig); } @@ -55,9 +51,13 @@ protected Void execute0(BuildImageCmd command, ResultCallback if (dockerFilePath != null && command.getRemote() == null && !"Dockerfile".equals(dockerFilePath)) { webTarget = webTarget.queryParam("dockerfile", dockerFilePath); } - if (command.getTag() != null) { - webTarget = webTarget.queryParam("t", command.getTag()); + + if (command.getTags() != null && !command.getTags().isEmpty()) { + webTarget = webTarget.queryParamsSet("t", command.getTags()); + } else if (isNotBlank(command.getTag())) { + webTarget = webTarget.queryParam("t", command.getTags()); } + if (command.getRemote() != null) { webTarget = webTarget.queryParam("remote", command.getRemote().toString()); } @@ -86,13 +86,17 @@ protected Void execute0(BuildImageCmd command, ResultCallback webTarget = webTarget.queryParam("cpusetcpus", command.getCpusetcpus()); } - webTarget = writeMap(webTarget, "buildargs", command.getBuildArgs()); + if (command.getBuildArgs() != null) { + webTarget = webTarget.queryParamsJsonMap("buildargs", command.getBuildArgs()); + } if (command.getShmsize() != null) { webTarget = webTarget.queryParam("shmsize", command.getShmsize()); } - webTarget = writeMap(webTarget, "labels", command.getLabels()); + if (command.getLabels() != null) { + webTarget = webTarget.queryParamsJsonMap("labels", command.getLabels()); + } LOGGER.trace("POST: {}", webTarget); @@ -106,16 +110,4 @@ protected Void execute0(BuildImageCmd command, ResultCallback return null; } - - private WebTarget writeMap(WebTarget webTarget, String name, Map value) { - if (value != null && !value.isEmpty()) { - try { - return webTarget.queryParam(name, MAPPER.writeValueAsString(value)); - } catch (IOException e) { - throw new RuntimeException(e); - } - } else { - return webTarget; - } - } } diff --git a/src/test/java/com/github/dockerjava/core/command/BuildImageCmdImplTest.java b/src/test/java/com/github/dockerjava/core/command/BuildImageCmdImplTest.java index cb4c1b2a8..9ba283732 100644 --- a/src/test/java/com/github/dockerjava/core/command/BuildImageCmdImplTest.java +++ b/src/test/java/com/github/dockerjava/core/command/BuildImageCmdImplTest.java @@ -7,13 +7,16 @@ import static org.hamcrest.Matchers.isEmptyString; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.containsInAnyOrder; import java.io.File; import java.io.FileInputStream; import java.io.InputStream; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.UUID; import org.apache.commons.io.FileUtils; @@ -274,6 +277,28 @@ public void labels() throws Exception { assertThat(inspectImageResponse.getConfig().getLabels().get("test"), equalTo("abc")); } + @Test + public void multipleTags() throws Exception { + if (!getVersion(dockerClient).isGreaterOrEqual(RemoteApiVersion.VERSION_1_21)) { + throw new SkipException("API version should be >= 1.21"); + } + + File baseDir = fileFromBuildTestResource("labels"); + + String imageId = dockerClient.buildImageCmd(baseDir).withNoCache(true) + .withTag("fallback-when-withTags-not-called") + .withTags(new HashSet<>(Arrays.asList("docker-java-test:tag1", "docker-java-test:tag2"))) + .exec(new BuildImageResultCallback()) + .awaitImageId(); + + InspectImageResponse inspectImageResponse = dockerClient.inspectImageCmd(imageId).exec(); + assertThat(inspectImageResponse, not(nullValue())); + LOG.info("Image Inspect: {}", inspectImageResponse.toString()); + + assertThat(inspectImageResponse.getRepoTags().size(), equalTo(2)); + assertThat(inspectImageResponse.getRepoTags(), containsInAnyOrder("docker-java-test:tag1", "docker-java-test:tag2")); + } + public void dockerfileNotInBaseDirectory() throws Exception { File baseDirectory = fileFromBuildTestResource("dockerfileNotInBaseDirectory"); File dockerfile = fileFromBuildTestResource("dockerfileNotInBaseDirectory/dockerfileFolder/Dockerfile"); diff --git a/src/test/java/com/github/dockerjava/netty/exec/BuildImageCmdExecTest.java b/src/test/java/com/github/dockerjava/netty/exec/BuildImageCmdExecTest.java index f567dcfc8..55dcf353b 100644 --- a/src/test/java/com/github/dockerjava/netty/exec/BuildImageCmdExecTest.java +++ b/src/test/java/com/github/dockerjava/netty/exec/BuildImageCmdExecTest.java @@ -7,13 +7,16 @@ import static org.hamcrest.Matchers.isEmptyString; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.containsInAnyOrder; import java.io.File; import java.io.FileInputStream; import java.io.InputStream; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.UUID; import org.apache.commons.io.FileUtils; @@ -279,6 +282,28 @@ public void labels() throws Exception { assertThat(inspectImageResponse.getConfig().getLabels().get("test"), equalTo("abc")); } + @Test + public void multipleTags() throws Exception { + if (!getVersion(dockerClient).isGreaterOrEqual(RemoteApiVersion.VERSION_1_21)) { + throw new SkipException("API version should be >= 1.21"); + } + + File baseDir = fileFromBuildTestResource("labels"); + + String imageId = dockerClient.buildImageCmd(baseDir).withNoCache(true) + .withTag("fallback-when-withTags-not-called") + .withTags(new HashSet<>(Arrays.asList("docker-java-test:tag1", "docker-java-test:tag2"))) + .exec(new BuildImageResultCallback()) + .awaitImageId(); + + InspectImageResponse inspectImageResponse = dockerClient.inspectImageCmd(imageId).exec(); + assertThat(inspectImageResponse, not(nullValue())); + LOG.info("Image Inspect: {}", inspectImageResponse.toString()); + + assertThat(inspectImageResponse.getRepoTags().size(), equalTo(2)); + assertThat(inspectImageResponse.getRepoTags(), containsInAnyOrder("docker-java-test:tag1", "docker-java-test:tag2")); + } + public void dockerfileNotInBaseDirectory() throws Exception { File baseDirectory = fileFromBuildTestResource("dockerfileNotInBaseDirectory"); File dockerfile = fileFromBuildTestResource("dockerfileNotInBaseDirectory/dockerfileFolder/Dockerfile");