diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcher.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcher.java index fbfeefbaef..414d928ccb 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcher.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcher.java @@ -20,7 +20,6 @@ import com.google.cloud.tools.jib.event.events.ProgressEvent; import com.google.cloud.tools.jib.event.progress.Allocation; import com.google.common.base.Preconditions; -import com.google.common.base.Verify; import java.io.Closeable; /** @@ -139,18 +138,28 @@ public void close() { * @param progressUnits units of progress */ public void dispatchProgress(long progressUnits) { - decrementRemainingAllocationUnits(progressUnits); - eventDispatcher.dispatch(new ProgressEvent(allocation, progressUnits, buildStepType)); + long unitsDecremented = decrementRemainingAllocationUnits(progressUnits); + eventDispatcher.dispatch(new ProgressEvent(allocation, unitsDecremented, buildStepType)); } - private void decrementRemainingAllocationUnits(long units) { + /** + * Decrements remaining allocation units by {@code units} but no more than the remaining + * allocation units (which may be 0). Returns the actual units decremented, which never exceeds + * {@code units}. + * + * @param units units to decrement + * @return units actually decremented + */ + private long decrementRemainingAllocationUnits(long units) { Preconditions.checkState(!closed); - remainingAllocationUnits -= units; - Verify.verify( - remainingAllocationUnits >= 0, - "Remaining allocation units less than 0 for '%s': %s", - allocation.getDescription(), - remainingAllocationUnits); + if (remainingAllocationUnits > units) { + remainingAllocationUnits -= units; + return units; + } + + long actualDecrement = remainingAllocationUnits; + remainingAllocationUnits = 0; + return actualDecrement; } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/event/progress/Allocation.java b/jib-core/src/main/java/com/google/cloud/tools/jib/event/progress/Allocation.java index e9ae352b4b..ef874dddab 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/event/progress/Allocation.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/event/progress/Allocation.java @@ -65,7 +65,7 @@ public static Allocation newRoot(String description, long allocationUnits) { private Allocation(String description, long allocationUnits, @Nullable Allocation parent) { this.description = description; - this.allocationUnits = allocationUnits; + this.allocationUnits = allocationUnits < 0 ? 0 : allocationUnits; this.parent = parent; this.fractionOfRoot = (parent == null ? 1.0 : parent.fractionOfRoot) / allocationUnits; diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcherTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcherTest.java index 16cc527773..f114da01c1 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcherTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcherTest.java @@ -18,7 +18,6 @@ import com.google.cloud.tools.jib.event.EventDispatcher; import com.google.cloud.tools.jib.event.events.ProgressEvent; -import com.google.common.base.VerifyException; import java.util.List; import org.junit.Assert; import org.junit.Test; @@ -59,37 +58,58 @@ public void testDispatch() { } @Test - public void testDispatch_tooMuchProgress() { + public void testDispatch_safeWithtooMuchProgress() { try (ProgressEventDispatcher progressEventDispatcher = ProgressEventDispatcher.newRoot(mockEventDispatcher, "allocation description", 10)) { - progressEventDispatcher.dispatchProgress(10); - try { - progressEventDispatcher.dispatchProgress(10); - Assert.fail(); - - } catch (VerifyException ex) { - Assert.assertEquals( - "Remaining allocation units less than 0 for 'allocation description': -10", - ex.getMessage()); - } + progressEventDispatcher.dispatchProgress(6); + progressEventDispatcher.dispatchProgress(8); + progressEventDispatcher.dispatchProgress(1); } + + ArgumentCaptor eventsCaptor = ArgumentCaptor.forClass(ProgressEvent.class); + Mockito.verify(mockEventDispatcher, Mockito.times(4)).dispatch(eventsCaptor.capture()); + List progressEvents = eventsCaptor.getAllValues(); + + Assert.assertSame(progressEvents.get(0).getAllocation(), progressEvents.get(1).getAllocation()); + Assert.assertSame(progressEvents.get(1).getAllocation(), progressEvents.get(2).getAllocation()); + Assert.assertSame(progressEvents.get(2).getAllocation(), progressEvents.get(3).getAllocation()); + + Assert.assertEquals(10, progressEvents.get(0).getAllocation().getAllocationUnits()); + + Assert.assertEquals(0, progressEvents.get(0).getUnits()); + Assert.assertEquals(6, progressEvents.get(1).getUnits()); + Assert.assertEquals(4, progressEvents.get(2).getUnits()); + Assert.assertEquals(0, progressEvents.get(3).getUnits()); } @Test - public void testDispatch_tooManyChildren() { + public void testDispatch_safeWithTooManyChildren() { try (ProgressEventDispatcher progressEventDispatcher = - ProgressEventDispatcher.newRoot(mockEventDispatcher, "allocation description", 1)) { - progressEventDispatcher.newChildProducer(); - - try { - progressEventDispatcher.newChildProducer(); - Assert.fail(); - - } catch (VerifyException ex) { - Assert.assertEquals( - "Remaining allocation units less than 0 for 'allocation description': -1", - ex.getMessage()); - } + ProgressEventDispatcher.newRoot(mockEventDispatcher, "allocation description", 1); + ProgressEventDispatcher ignored1 = + progressEventDispatcher.newChildProducer().create(BuildStepType.ALL, "ignored", 5); + ProgressEventDispatcher ignored2 = + progressEventDispatcher.newChildProducer().create(BuildStepType.ALL, "ignored", 4)) { + // empty } + + ArgumentCaptor eventsCaptor = ArgumentCaptor.forClass(ProgressEvent.class); + Mockito.verify(mockEventDispatcher, Mockito.times(5)).dispatch(eventsCaptor.capture()); + List progressEvents = eventsCaptor.getAllValues(); + + Assert.assertEquals(1, progressEvents.get(0).getAllocation().getAllocationUnits()); + Assert.assertEquals(5, progressEvents.get(1).getAllocation().getAllocationUnits()); + Assert.assertEquals(4, progressEvents.get(2).getAllocation().getAllocationUnits()); + + // child1 (of allocation 5) opening and closing + Assert.assertSame(progressEvents.get(1).getAllocation(), progressEvents.get(4).getAllocation()); + // child1 (of allocation 4) opening and closing + Assert.assertSame(progressEvents.get(2).getAllocation(), progressEvents.get(3).getAllocation()); + + Assert.assertEquals(0, progressEvents.get(0).getUnits()); // 0-progress sent when root creation + Assert.assertEquals(0, progressEvents.get(1).getUnits()); // when child1 creation + Assert.assertEquals(0, progressEvents.get(2).getUnits()); // when child2 creation + Assert.assertEquals(4, progressEvents.get(3).getUnits()); // when child2 closes + Assert.assertEquals(5, progressEvents.get(4).getUnits()); // when child1 closes } } diff --git a/jib-gradle-plugin/CHANGELOG.md b/jib-gradle-plugin/CHANGELOG.md index 606a17f2dc..833c0a11e8 100644 --- a/jib-gradle-plugin/CHANGELOG.md +++ b/jib-gradle-plugin/CHANGELOG.md @@ -11,7 +11,8 @@ All notable changes to this project will be documented in this file. ### Fixed -- Fixed an issue where setting `allowInsecureRegistries` may fail to try HTTP. +- Fixed an issue where setting `allowInsecureRegistries` may fail to try HTTP. ([#1517](https://github.com/GoogleContainerTools/jib/issues/1517)) +- Crash on talking to servers that do not set the `Content-Length` HTTP header or send an incorrect value. ([#1512](https://github.com/GoogleContainerTools/jib/issues/1512)) ## 1.0.1 diff --git a/jib-maven-plugin/CHANGELOG.md b/jib-maven-plugin/CHANGELOG.md index b9c04ac9be..8f5bced685 100644 --- a/jib-maven-plugin/CHANGELOG.md +++ b/jib-maven-plugin/CHANGELOG.md @@ -11,7 +11,8 @@ All notable changes to this project will be documented in this file. ### Fixed -- Fixed an issue where setting `` may fail to try HTTP. +- Fixed an issue where setting `` may fail to try HTTP. ([#1517](https://github.com/GoogleContainerTools/jib/issues/1517)) +- Crash on talking to servers that do not set the `Content-Length` HTTP header or send an incorrect value. ([#1512](https://github.com/GoogleContainerTools/jib/issues/1512)) ## 1.0.1