From a9d3de3456b36039525317aa1fc54f12a2da8d8b Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 27 Jun 2019 18:34:25 -0400 Subject: [PATCH 1/2] Remove ImageLayers --- .../BuildAndCacheApplicationLayerStep.java | 35 ++-- .../google/cloud/tools/jib/image/Image.java | 8 +- .../cloud/tools/jib/image/ImageLayers.java | 160 ------------------ ...BuildAndCacheApplicationLayerStepTest.java | 23 +-- .../tools/jib/image/ImageLayersTest.java | 111 ------------ 5 files changed, 31 insertions(+), 306 deletions(-) delete mode 100644 jib-core/src/main/java/com/google/cloud/tools/jib/image/ImageLayers.java delete mode 100644 jib-core/src/test/java/com/google/cloud/tools/jib/image/ImageLayersTest.java diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStep.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStep.java index 4a4de43f40..bcb50c8189 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStep.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStep.java @@ -28,6 +28,7 @@ import com.google.cloud.tools.jib.image.ReproducibleLayerBuilder; import com.google.common.collect.ImmutableList; import java.io.IOException; +import java.util.List; import java.util.Optional; import java.util.concurrent.Callable; @@ -43,31 +44,25 @@ class BuildAndCacheApplicationLayerStep implements Callable static ImmutableList makeList( BuildConfiguration buildConfiguration, ProgressEventDispatcher.Factory progressEventDispatcherFactory) { - int layerCount = buildConfiguration.getLayerConfigurations().size(); + List layerConfigurations = buildConfiguration.getLayerConfigurations(); try (ProgressEventDispatcher progressEventDispatcher = progressEventDispatcherFactory.create( - "preparing application layer builders", layerCount); + "preparing application layer builders", layerConfigurations.size()); TimerEventDispatcher ignored = new TimerEventDispatcher(buildConfiguration.getEventHandlers(), DESCRIPTION)) { - ImmutableList.Builder buildAndCacheApplicationLayerSteps = - ImmutableList.builderWithExpectedSize(layerCount); - for (LayerConfiguration layerConfiguration : buildConfiguration.getLayerConfigurations()) { - // Skips the layer if empty. - if (layerConfiguration.getLayerEntries().isEmpty()) { - continue; - } - - buildAndCacheApplicationLayerSteps.add( - new BuildAndCacheApplicationLayerStep( - buildConfiguration, - progressEventDispatcher.newChildProducer(), - layerConfiguration.getName(), - layerConfiguration)); - } - ImmutableList steps = - buildAndCacheApplicationLayerSteps.build(); - return steps; + return layerConfigurations + .stream() + // Skips the layer if empty. + .filter(layerConfiguration -> !layerConfiguration.getLayerEntries().isEmpty()) + .map( + layerConfiguration -> + new BuildAndCacheApplicationLayerStep( + buildConfiguration, + progressEventDispatcher.newChildProducer(), + layerConfiguration.getName(), + layerConfiguration)) + .collect(ImmutableList.toImmutableList()); } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java b/jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java index 8e3dac8b6d..bbadd46223 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java @@ -39,7 +39,7 @@ public class Image { public static class Builder { private final Class imageFormat; - private final ImageLayers.Builder imageLayersBuilder = ImageLayers.builder(); + private final ImmutableList.Builder imageLayersBuilder = ImmutableList.builder(); private final ImmutableList.Builder historyBuilder = ImmutableList.builder(); // Don't use ImmutableMap.Builder because it does not allow for replacing existing keys with new @@ -287,7 +287,7 @@ public static Builder builder(Class imageFormat) { private final String os; /** The layers of the image, in the order in which they are applied. */ - private final ImageLayers layers; + private final ImmutableList layers; /** The commands used to build each layer of the image */ private final ImmutableList history; @@ -324,7 +324,7 @@ private Image( @Nullable Instant created, String architecture, String os, - ImageLayers layers, + ImmutableList layers, ImmutableList history, @Nullable ImmutableMap environment, @Nullable ImmutableList entrypoint, @@ -415,7 +415,7 @@ public String getUser() { } public ImmutableList getLayers() { - return layers.getLayers(); + return layers; } public ImmutableList getHistory() { diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/image/ImageLayers.java b/jib-core/src/main/java/com/google/cloud/tools/jib/image/ImageLayers.java deleted file mode 100644 index fd37b6b29e..0000000000 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/image/ImageLayers.java +++ /dev/null @@ -1,160 +0,0 @@ -/* - * Copyright 2017 Google LLC. - * - * 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.image; - -import com.google.cloud.tools.jib.api.DescriptorDigest; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; -import javax.annotation.Nullable; - -/** Holds the layers for an image. */ -public class ImageLayers implements Iterable { - - public static class Builder { - - private final List layers = new ArrayList<>(); - private final ImmutableSet.Builder layerDigestsBuilder = - ImmutableSet.builder(); - private boolean removeDuplicates = false; - - /** - * Adds a layer. Removes any prior occurrences of the same layer. - * - *

Note that only subclasses of {@link Layer} that implement {@code equals/hashCode} will be - * guaranteed to not be duplicated. - * - * @param layer the layer to add - * @return this - * @throws LayerPropertyNotFoundException if adding the layer fails - */ - public Builder add(Layer layer) throws LayerPropertyNotFoundException { - layerDigestsBuilder.add(layer.getBlobDescriptor().getDigest()); - layers.add(layer); - return this; - } - - /** - * Adds all layers in {@code layers}. - * - * @param layers the layers to add - * @return this - * @throws LayerPropertyNotFoundException if adding a layer fails - */ - public Builder addAll(ImageLayers layers) throws LayerPropertyNotFoundException { - for (Layer layer : layers) { - add(layer); - } - return this; - } - - /** - * Remove any duplicate layers, keeping the last occurrence of the layer. - * - * @return this - */ - public Builder removeDuplicates() { - removeDuplicates = true; - return this; - } - - public ImageLayers build() { - if (!removeDuplicates) { - return new ImageLayers(ImmutableList.copyOf(layers), layerDigestsBuilder.build()); - } - - // LinkedHashSet maintains the order but keeps the first occurrence. Keep last occurrence by - // adding elements in reverse, and then reversing the result - Set dedupedButReversed = new LinkedHashSet<>(Lists.reverse(this.layers)); - ImmutableList deduped = ImmutableList.copyOf(dedupedButReversed).reverse(); - return new ImageLayers(deduped, layerDigestsBuilder.build()); - } - } - - public static Builder builder() { - return new Builder(); - } - - /** The layers of the image, in the order in which they are applied. */ - private final ImmutableList layers; - - /** Keeps track of the layers already added. */ - private final ImmutableSet layerDigests; - - private ImageLayers(ImmutableList layers, ImmutableSet layerDigests) { - this.layers = layers; - this.layerDigests = layerDigests; - } - - /** @return a read-only view of the image layers. */ - public ImmutableList getLayers() { - return layers; - } - - /** @return the layer count */ - public int size() { - return layers.size(); - } - - public boolean isEmpty() { - return layers.isEmpty(); - } - - /** - * @param index the index of the layer to get - * @return the layer at the specified index - */ - public Layer get(int index) { - return layers.get(index); - } - - /** - * @param digest the digest used to retrieve the layer - * @return the layer found, or {@code null} if not found - * @throws LayerPropertyNotFoundException if getting the layer's blob descriptor fails - */ - @Nullable - public Layer get(DescriptorDigest digest) throws LayerPropertyNotFoundException { - if (!has(digest)) { - return null; - } - for (Layer layer : layers) { - if (layer.getBlobDescriptor().getDigest().equals(digest)) { - return layer; - } - } - throw new IllegalStateException("Layer digest exists but layer not found"); - } - - /** - * @param digest the digest to check for - * @return true if the layer with the specified digest exists; false otherwise - */ - public boolean has(DescriptorDigest digest) { - return layerDigests.contains(digest); - } - - @Override - public Iterator iterator() { - return getLayers().iterator(); - } -} diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStepTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStepTest.java index a8c7a76068..38a45bbf07 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStepTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStepTest.java @@ -27,7 +27,7 @@ import com.google.cloud.tools.jib.cache.CachedLayer; import com.google.cloud.tools.jib.configuration.BuildConfiguration; import com.google.cloud.tools.jib.event.EventHandlers; -import com.google.cloud.tools.jib.image.ImageLayers; +import com.google.cloud.tools.jib.image.Layer; import com.google.cloud.tools.jib.image.LayerPropertyNotFoundException; import com.google.common.collect.ImmutableList; import com.google.common.io.Resources; @@ -36,6 +36,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; import java.util.stream.Stream; import org.junit.Assert; import org.junit.Before; @@ -79,12 +81,11 @@ private static void assertBlobsEqual(Blob expectedBlob, Blob blob) throws IOExce Assert.assertArrayEquals(Blobs.writeToByteArray(expectedBlob), Blobs.writeToByteArray(blob)); } - @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); @Mock private BuildConfiguration mockBuildConfiguration; private Cache cache; - @Mock private EventHandlers mockEventHandlers; private LayerConfiguration fakeDependenciesLayerConfiguration; private LayerConfiguration fakeSnapshotDependenciesLayerConfiguration; @@ -119,25 +120,25 @@ public void setUp() throws IOException, URISyntaxException { cache = Cache.withDirectory(temporaryFolder.newFolder().toPath()); - Mockito.when(mockBuildConfiguration.getEventHandlers()).thenReturn(mockEventHandlers); + Mockito.when(mockBuildConfiguration.getEventHandlers()).thenReturn(EventHandlers.NONE); Mockito.when(mockBuildConfiguration.getApplicationLayersCache()).thenReturn(cache); } - private ImageLayers buildFakeLayersToCache() + private List buildFakeLayersToCache() throws LayerPropertyNotFoundException, IOException, CacheCorruptedException { - ImageLayers.Builder applicationLayersBuilder = ImageLayers.builder(); + List applicationLayers = new ArrayList<>(); ImmutableList buildAndCacheApplicationLayerSteps = BuildAndCacheApplicationLayerStep.makeList( mockBuildConfiguration, - ProgressEventDispatcher.newRoot(mockEventHandlers, "ignored", 1).newChildProducer()); + ProgressEventDispatcher.newRoot(EventHandlers.NONE, "ignored", 1).newChildProducer()); for (BuildAndCacheApplicationLayerStep buildAndCacheApplicationLayerStep : buildAndCacheApplicationLayerSteps) { - applicationLayersBuilder.add(buildAndCacheApplicationLayerStep.call().getCachedLayer()); + applicationLayers.add(buildAndCacheApplicationLayerStep.call().getCachedLayer()); } - return applicationLayersBuilder.build(); + return applicationLayers; } @Test @@ -154,7 +155,7 @@ public void testRun() .thenReturn(fakeLayerConfigurations); // Populates the cache. - ImageLayers applicationLayers = buildFakeLayersToCache(); + List applicationLayers = buildFakeLayersToCache(); Assert.assertEquals(5, applicationLayers.size()); ImmutableList dependenciesLayerEntries = @@ -215,7 +216,7 @@ public void testRun_emptyLayersIgnored() throws IOException, CacheCorruptedExcep .thenReturn(fakeLayerConfigurations); // Populates the cache. - ImageLayers applicationLayers = buildFakeLayersToCache(); + List applicationLayers = buildFakeLayersToCache(); Assert.assertEquals(3, applicationLayers.size()); ImmutableList dependenciesLayerEntries = diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/image/ImageLayersTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/image/ImageLayersTest.java deleted file mode 100644 index 828150e22f..0000000000 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/image/ImageLayersTest.java +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Copyright 2017 Google LLC. - * - * 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.image; - -import com.google.cloud.tools.jib.api.DescriptorDigest; -import com.google.cloud.tools.jib.blob.BlobDescriptor; -import java.util.Arrays; -import java.util.List; -import org.hamcrest.CoreMatchers; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; - -/** Tests for {@link ImageLayers}. */ -@RunWith(MockitoJUnitRunner.class) -public class ImageLayersTest { - - @Mock private Layer mockLayer; - @Mock private ReferenceLayer mockReferenceLayer; - @Mock private DigestOnlyLayer mockDigestOnlyLayer; - @Mock private Layer mockLayer2; - - @Before - public void setUpFakes() throws LayerPropertyNotFoundException { - DescriptorDigest mockDescriptorDigest1 = Mockito.mock(DescriptorDigest.class); - DescriptorDigest mockDescriptorDigest2 = Mockito.mock(DescriptorDigest.class); - DescriptorDigest mockDescriptorDigest3 = Mockito.mock(DescriptorDigest.class); - - BlobDescriptor layerBlobDescriptor = new BlobDescriptor(0, mockDescriptorDigest1); - BlobDescriptor referenceLayerBlobDescriptor = new BlobDescriptor(0, mockDescriptorDigest2); - BlobDescriptor referenceNoDiffIdLayerBlobDescriptor = - new BlobDescriptor(0, mockDescriptorDigest3); - // Intentionally the same digest as the mockLayer. - BlobDescriptor anotherBlobDescriptor = new BlobDescriptor(0, mockDescriptorDigest1); - - Mockito.when(mockLayer.getBlobDescriptor()).thenReturn(layerBlobDescriptor); - Mockito.when(mockReferenceLayer.getBlobDescriptor()).thenReturn(referenceLayerBlobDescriptor); - Mockito.when(mockDigestOnlyLayer.getBlobDescriptor()) - .thenReturn(referenceNoDiffIdLayerBlobDescriptor); - Mockito.when(mockLayer2.getBlobDescriptor()).thenReturn(anotherBlobDescriptor); - } - - @Test - public void testAddLayer_success() throws LayerPropertyNotFoundException { - List expectedLayers = Arrays.asList(mockLayer, mockReferenceLayer, mockDigestOnlyLayer); - - ImageLayers imageLayers = - ImageLayers.builder() - .add(mockLayer) - .add(mockReferenceLayer) - .add(mockDigestOnlyLayer) - .build(); - - Assert.assertThat(imageLayers.getLayers(), CoreMatchers.is(expectedLayers)); - } - - @Test - public void testAddLayer_maintainDuplicates() throws LayerPropertyNotFoundException { - // must maintain duplicate - List expectedLayers = - Arrays.asList(mockLayer, mockReferenceLayer, mockDigestOnlyLayer, mockLayer2, mockLayer); - - ImageLayers imageLayers = - ImageLayers.builder() - .add(mockLayer) - .add(mockReferenceLayer) - .add(mockDigestOnlyLayer) - .add(mockLayer2) - .add(mockLayer) - .build(); - - Assert.assertEquals(expectedLayers, imageLayers.getLayers()); - } - - @Test - public void testAddLayer_removeDuplicates() throws LayerPropertyNotFoundException { - // remove duplicates: last layer should be kept - List expectedLayers = - Arrays.asList(mockReferenceLayer, mockDigestOnlyLayer, mockLayer2, mockLayer); - - ImageLayers imageLayers = - ImageLayers.builder() - .removeDuplicates() - .add(mockLayer) - .add(mockReferenceLayer) - .add(mockDigestOnlyLayer) - .add(mockLayer2) - .add(mockLayer) - .build(); - - Assert.assertEquals(expectedLayers, imageLayers.getLayers()); - } -} From 6b07083d52715413fa4224757450d6313f3e2990 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Fri, 28 Jun 2019 13:06:23 -0400 Subject: [PATCH 2/2] Fix TimerEvent message --- .../jib/builder/steps/BuildAndCacheApplicationLayerStep.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStep.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStep.java index 1309a4a5b2..96a3d3cafd 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStep.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildAndCacheApplicationLayerStep.java @@ -51,7 +51,8 @@ static ImmutableList makeList( progressEventDispatcherFactory.create( "preparing application layer builders", layerConfigurations.size()); TimerEventDispatcher ignored = - new TimerEventDispatcher(buildConfiguration.getEventHandlers(), DESCRIPTION)) { + new TimerEventDispatcher( + buildConfiguration.getEventHandlers(), "Preparing application layer builders")) { return layerConfigurations .stream() // Skips the layer if empty.