From 0e5274b4b5fdd4dd6751d9c2186ca0c2c83932be Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 18 Oct 2017 22:07:59 -0400 Subject: [PATCH 1/2] [JENKINS-47517] Revert the change to the default behavior of Queue.Task.getCauseOfBlockage. --- core/src/main/java/hudson/model/Queue.java | 3 --- test/src/test/java/hudson/model/QueueTest.java | 14 -------------- 2 files changed, 17 deletions(-) diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java index 3ea6e8dacf09..214213c48847 100644 --- a/core/src/main/java/hudson/model/Queue.java +++ b/core/src/main/java/hudson/model/Queue.java @@ -1841,9 +1841,6 @@ public interface Task extends ModelObject, SubTask { */ @CheckForNull default CauseOfBlockage getCauseOfBlockage() { - if (isBuildBlocked()) { - return CauseOfBlockage.fromMessage(Messages._Queue_Unknown()); - } return null; } diff --git a/test/src/test/java/hudson/model/QueueTest.java b/test/src/test/java/hudson/model/QueueTest.java index 516bb4677a84..1518b404abf2 100644 --- a/test/src/test/java/hudson/model/QueueTest.java +++ b/test/src/test/java/hudson/model/QueueTest.java @@ -988,20 +988,6 @@ public String getShortDescription() { } } - @Test - public void testDefaultImplementationOfGetCauseOfBlockageForBlocked() throws Exception { - Queue queue = r.getInstance().getQueue(); - queue.schedule2(new TestTask(new AtomicInteger(0), true), 0); - - queue.maintain(); - - assertEquals(1, r.jenkins.getQueue().getBlockedItems().size()); - CauseOfBlockage actual = r.jenkins.getQueue().getBlockedItems().get(0).getCauseOfBlockage(); - CauseOfBlockage expected = CauseOfBlockage.fromMessage(Messages._Queue_Unknown()); - - assertEquals(expected.getShortDescription(), actual.getShortDescription()); - } - @Test public void testGetCauseOfBlockageForNonConcurrentFreestyle() throws Exception { Queue queue = r.getInstance().getQueue(); From 19714f201809cf1bb4e278e53bc58eafef23d36f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 19 Oct 2017 13:06:35 -0400 Subject: [PATCH 2/2] Deprecate isBuildBlocked & getWhyBlocked in favor of getCauseOfBlockage, which you needed to implement anyway. --- .../java/hudson/model/AbstractProject.java | 26 ++---- core/src/main/java/hudson/model/Queue.java | 14 ++-- .../hudson/model/queue/QueueTaskFilter.java | 2 + .../model/queue/AbstractQueueTaskTest.java | 83 +++++++++++++++++++ .../src/test/java/hudson/model/QueueTest.java | 3 +- 5 files changed, 103 insertions(+), 25 deletions(-) create mode 100644 core/src/test/java/hudson/model/queue/AbstractQueueTaskTest.java diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java index 954011f650d4..ffee6413d89f 100644 --- a/core/src/main/java/hudson/model/AbstractProject.java +++ b/core/src/main/java/hudson/model/AbstractProject.java @@ -1015,24 +1015,6 @@ public Object getSameNodeConstraint() { return this; // in this way, any member that wants to run with the main guy can nominate the project itself } - /** - * {@inheritDoc} - * - *

- * A project must be blocked if its own previous build is in progress, - * or if the blockBuildWhenUpstreamBuilding option is true and an upstream - * project is building, but derived classes can also check other conditions. - */ - @Override - public boolean isBuildBlocked() { - return getCauseOfBlockage()!=null; - } - - public String getWhyBlocked() { - CauseOfBlockage cb = getCauseOfBlockage(); - return cb!=null ? cb.getShortDescription() : null; - } - /** * @deprecated use {@link BlockedBecauseOfBuildInProgress} instead. */ @@ -1075,6 +1057,14 @@ public String getShortDescription() { } } + /** + * {@inheritDoc} + * + *

+ * A project must be blocked if its own previous build is in progress, + * or if the blockBuildWhenUpstreamBuilding option is true and an upstream + * project is building, but derived classes can also check other conditions. + */ @Override public CauseOfBlockage getCauseOfBlockage() { // Block builds until they are done with post-production diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java index 214213c48847..ece3da76cfa4 100644 --- a/core/src/main/java/hudson/model/Queue.java +++ b/core/src/main/java/hudson/model/Queue.java @@ -1814,18 +1814,22 @@ public interface Task extends ModelObject, SubTask { /** * Returns true if the execution should be blocked * for temporary reasons. - * - *

- * Short-hand for {@code getCauseOfBlockageForItem()!=null}. + * @deprecated Use {@link #getCauseOfBlockage} != null */ - boolean isBuildBlocked(); + @Deprecated + default boolean isBuildBlocked() { + return getCauseOfBlockage() != null; + } /** * @deprecated as of 1.330 * Use {@link CauseOfBlockage#getShortDescription()} instead. */ @Deprecated - String getWhyBlocked(); + default String getWhyBlocked() { + CauseOfBlockage cause = getCauseOfBlockage(); + return cause != null ? cause.getShortDescription() : null; + } /** * If the execution of this task should be blocked for temporary reasons, diff --git a/core/src/main/java/hudson/model/queue/QueueTaskFilter.java b/core/src/main/java/hudson/model/queue/QueueTaskFilter.java index 4688551b8e18..9bf4887247da 100644 --- a/core/src/main/java/hudson/model/queue/QueueTaskFilter.java +++ b/core/src/main/java/hudson/model/queue/QueueTaskFilter.java @@ -56,10 +56,12 @@ public Node getLastBuiltOn() { return base.getLastBuiltOn(); } + @Deprecated public boolean isBuildBlocked() { return base.isBuildBlocked(); } + @Deprecated public String getWhyBlocked() { return base.getWhyBlocked(); } diff --git a/core/src/test/java/hudson/model/queue/AbstractQueueTaskTest.java b/core/src/test/java/hudson/model/queue/AbstractQueueTaskTest.java new file mode 100644 index 000000000000..9a31d139c38d --- /dev/null +++ b/core/src/test/java/hudson/model/queue/AbstractQueueTaskTest.java @@ -0,0 +1,83 @@ +/* + * The MIT License + * + * Copyright 2017 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package hudson.model.queue; + +import hudson.model.Queue; +import java.io.IOException; +import org.junit.Test; +import static org.junit.Assert.*; +import org.jvnet.hudson.test.Issue; + +@SuppressWarnings("deprecation") +public class AbstractQueueTaskTest { + + @Issue("JENKINS-47517") + @Test + public void causeOfBlockageOverrides() { + Queue.Task t = new LegacyTask(); + assertFalse(t.isBuildBlocked()); + assertNull(t.getWhyBlocked()); + assertNull(t.getCauseOfBlockage()); + } + static class LegacyTask extends AbstractQueueTask { + @Override + public boolean isBuildBlocked() { + return getCauseOfBlockage() != null; + } + @Override + public String getWhyBlocked() { + CauseOfBlockage causeOfBlockage = getCauseOfBlockage(); + return causeOfBlockage != null ? causeOfBlockage.getShortDescription() : null; + } + @Override + public String getName() { + return null; + } + @Override + public String getFullDisplayName() { + return null; + } + @Override + public void checkAbortPermission() { + } + @Override + public boolean hasAbortPermission() { + return false; + } + @Override + public String getUrl() { + return null; + } + @Override + public String getDisplayName() { + return null; + } + @Override + public Queue.Executable createExecutable() throws IOException { + throw new IOException(); + } + } + +} diff --git a/test/src/test/java/hudson/model/QueueTest.java b/test/src/test/java/hudson/model/QueueTest.java index 1518b404abf2..ca506b3a9534 100644 --- a/test/src/test/java/hudson/model/QueueTest.java +++ b/test/src/test/java/hudson/model/QueueTest.java @@ -547,8 +547,7 @@ static class TestTask implements Queue.Task { @Override public int hashCode() { return cnt.hashCode(); } - @Override public boolean isBuildBlocked() {return isBlocked;} - @Override public String getWhyBlocked() {return null;} + @Override public CauseOfBlockage getCauseOfBlockage() {return isBlocked ? CauseOfBlockage.fromMessage(Messages._Queue_Unknown()) : null;} @Override public String getName() {return "test";} @Override public String getFullDisplayName() {return "Test";} @Override public void checkAbortPermission() {}