466: Exception during sponsored integration

528: Exception during integration

Reviewed-by: ehelin

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java

@@ -130,7 +130,7 @@ PullRequestCheckIssueVisitor createVisitor(Hash localHash, CensusInstance census
         return new PullRequestCheckIssueVisitor(checks);
     }
 
-    void executeChecks(Hash localHash, CensusInstance censusInstance, PullRequestCheckIssueVisitor visitor, List<String> additionalConfiguration) throws Exception {
+    void executeChecks(Hash localHash, CensusInstance censusInstance, PullRequestCheckIssueVisitor visitor, List<String> additionalConfiguration) throws IOException {
         try (var issues = JCheck.check(localRepo, censusInstance.census(), CommitMessageParsers.v1, localHash,
                                        pr.targetHash(), additionalConfiguration)) {
             for (var issue : issues) {

bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java

@@ -28,6 +28,7 @@
 
 import java.io.*;
 import java.nio.file.Path;
+import java.time.Duration;
 import java.util.*;
 import java.util.logging.Logger;
 
@@ -85,7 +86,16 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
         }
 
         // Run a final jcheck to ensure the change has been properly reviewed
-        try {
+        try (var integrationLock = IntegrationLock.create(pr, Duration.ofMinutes(10))) {
+            if (!integrationLock.isLocked()) {
+                log.severe("Unable to acquire the integration lock for " + pr.webUrl());
+                reply.print("Unable to acquire the integration lock; aborting integration. The error has been logged and will be investigated.");
+                return;
+            }
+
+            // Now that we have the integration lock, refresh the PR metadata
+            pr = pr.repository().pullRequest(pr.id());
+
             var path = scratchPath.resolve("integrate").resolve(pr.repository().name());
             var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
             var hostedRepositoryPool = new HostedRepositoryPool(seedPath);
@@ -149,9 +159,12 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
                 reply.print("Warning! Your commit did not result in any changes! ");
                 reply.println("No push attempt will be made.");
             }
-        } catch (Exception e) {
+        } catch (IOException | CommitFailure e) {
+            log.severe("An error occurred during integration (" + pr.webUrl() + "): " + e.getMessage());
             log.throwing("IntegrateCommand", "handle", e);
-            reply.println("An error occurred during final integration jcheck. No push attempt will be made.");
+            reply.println("An unexpected error occurred during integration. No push attempt will be made. " +
+                                  "The error has been logged and will be investigated. It is possible that this error " +
+                                  "is caused by a transient issue; feel free to retry the operation.");
         }
     }
 

bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrationLock.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.skara.bots.pr;
+
+import org.openjdk.skara.forge.PullRequest;
+
+import java.time.Duration;
+import java.util.concurrent.*;
+
+public class IntegrationLock implements AutoCloseable {
+    private static final ConcurrentHashMap<String, Semaphore> pendingIntegrations = new ConcurrentHashMap<>();
+
+    static IntegrationLock create(PullRequest pr, Duration timeout) {
+        var repoName = pr.repository().webUrl().toString();
+        var repoPending = pendingIntegrations.computeIfAbsent(repoName, key -> new Semaphore(1));
+        try {
+            var locked = repoPending.tryAcquire(timeout.toMillis(), TimeUnit.MILLISECONDS);
+            return new IntegrationLock(locked ? repoPending : null);
+        } catch (InterruptedException e) {
+            return new IntegrationLock(null);
+        }
+    }
+
+    private final Semaphore semaphore;
+
+    private IntegrationLock(Semaphore semaphore) {
+        this.semaphore = semaphore;
+    }
+
+    @Override
+    public void close() {
+        if (semaphore != null) {
+            semaphore.release();
+        }
+    }
+
+    public boolean isLocked() {
+        return semaphore != null;
+    }
+}

bots/pr/src/main/java/org/openjdk/skara/bots/pr/SponsorCommand.java

@@ -28,6 +28,7 @@
 
 import java.io.*;
 import java.nio.file.Path;
+import java.time.Duration;
 import java.util.*;
 import java.util.logging.Logger;
 
@@ -70,7 +71,16 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
         reply.print("@" + pr.author().userName() + " ");
 
         // Execute merge
-        try {
+        try (var integrationLock = IntegrationLock.create(pr, Duration.ofMinutes(10))) {
+            if (!integrationLock.isLocked()) {
+                log.severe("Unable to acquire the integration lock during sponsoring for " + pr.webUrl());
+                reply.print("Unable to acquire the integration lock; aborting sponsored integration. The error has been logged and will be investigated.");
+                return;
+            }
+
+            // Now that we have the integration lock, refresh the PR metadata
+            pr = pr.repository().pullRequest(pr.id());
+
             var path = scratchPath.resolve("sponsor").resolve(pr.repository().name());
             var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
             var hostedRepositoryPool = new HostedRepositoryPool(seedPath);
@@ -124,9 +134,12 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
                 reply.print("Warning! This commit did not result in any changes! ");
                 reply.println("No push attempt will be made.");
             }
-        } catch (Exception e) {
+        } catch (IOException | CommitFailure e) {
+            log.severe("An error occurred during sponsored integration (" + pr.webUrl() + "): " + e.getMessage());
             log.throwing("SponsorCommand", "handle", e);
-            reply.println("An error occurred during sponsored integration. No push attempt will be made.");
+            reply.println("An unexpected error occurred during sponsored integration. No push attempt will be made. " +
+                                  "The error has been logged and will be investigated. It is possible that this error " +
+                                  "is caused by a transient issue; feel free to retry the operation.");
         }
     }
 

bots/pr/src/test/java/org/openjdk/skara/bots/pr/IntegrationLockTests.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.skara.bots.pr;
+
+import org.junit.jupiter.api.*;
+import org.openjdk.skara.test.*;
+
+import java.io.IOException;
+import java.time.Duration;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+public class IntegrationLockTests {
+    @Test
+    void simple(TestInfo testInfo) throws IOException {
+        try (var credentials = new HostCredentials(testInfo);
+             var tempFolder = new TemporaryDirectory()) {
+            var author = credentials.getHostedRepository();
+
+            // Populate the projects repository
+            var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType());
+            var masterHash = localRepo.resolve("master").orElseThrow();
+            localRepo.push(masterHash, author.url(), "master", true);
+
+            // Make a change with a corresponding PR
+            var editHash = CheckableRepository.appendAndCommit(localRepo);
+            localRepo.push(editHash, author.url(), "refs/heads/edit", true);
+            var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");
+
+            var l1 = IntegrationLock.create(pr, Duration.ofSeconds(10));
+            assertTrue(l1.isLocked());
+
+            var l2 = IntegrationLock.create(pr, Duration.ofMillis(100));
+            assertFalse(l2.isLocked());
+
+            l1.close();
+            var l3 = IntegrationLock.create(pr, Duration.ofSeconds(10));
+            assertTrue(l3.isLocked());
+        }
+    }
+}