[JENKINS-41453] Fix fallout from fixing the overzealous migration of …

…heads - The pull requests are missing semi-required information and we need to migrate the key fields to prevent a build storm
jenkinsci · Jan 25, 2017 · 5f102ac · 5f102ac
1 parent a43905d
commit 5f102ac
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 5 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java
@@ -38,6 +38,7 @@
 import jenkins.scm.api.SCMHead;
 import jenkins.scm.api.SCMRevision;
 import jenkins.scm.api.SCMSource;
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.github.GHRepository;
 import org.kohsuke.github.GitHub;
 import org.kohsuke.github.HttpException;
@@ -117,7 +118,7 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch
                 ref = head.getName();
             } else if (head instanceof PullRequestSCMHead) {
                 PullRequestSCMHead pr = (PullRequestSCMHead) head;
-                if (!pr.isMerge()) {
+                if (!pr.isMerge() && pr.getSourceRepo() != null) {
                     return new GitHubSCMFileSystem(
                             github.getUser(pr.getSourceOwner()).getRepository(pr.getSourceRepo()),
                             pr.getSourceBranch(),

diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java
@@ -825,7 +825,7 @@ public SCM build(SCMHead head, SCMRevision revision) {
                 setBrowser(scm, repoUrl);
             }
             return scm;
-        } else if (head instanceof PullRequestSCMHead) {
+        } else if (head instanceof PullRequestSCMHead && ((PullRequestSCMHead) head).getSourceRepo() != null) {
             if (revision instanceof PullRequestSCMRevision) {
                 PullRequestSCMRevision prRev = (PullRequestSCMRevision) revision;
                 // we rely on GitHub exposing the pull request revision on the target repository

diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java
@@ -25,8 +25,12 @@
 package org.jenkinsci.plugins.github_branch_source;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import java.util.logging.Logger;
 import jenkins.scm.api.SCMHead;
 import jenkins.scm.api.mixin.ChangeRequestSCMHead;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.github.GHPullRequest;
 
 /**
@@ -35,6 +39,8 @@
  */
 public final class PullRequestSCMHead extends SCMHead implements ChangeRequestSCMHead {
 
+    private static final Logger LOGGER = Logger.getLogger(PullRequestSCMHead.class.getName());
+
     private static final long serialVersionUID = 1;
 
     private Boolean merge;
@@ -43,6 +49,7 @@ public final class PullRequestSCMHead extends SCMHead implements ChangeRequestSC
     private final String sourceOwner;
     private final String sourceRepo;
     private final String sourceBranch;
+    private transient Metadata metadata;
 
     PullRequestSCMHead(GHPullRequest pr, String name, boolean merge) {
         super(name);
@@ -56,6 +63,17 @@ public final class PullRequestSCMHead extends SCMHead implements ChangeRequestSC
         this.sourceBranch = pr.getHead().getRef();
     }
 
+    PullRequestSCMHead(@NonNull String name, boolean merge, int number,
+                       BranchSCMHead target, String sourceOwner, String sourceRepo, String sourceBranch) {
+        super(name);
+        this.merge = merge;
+        this.number = number;
+        this.target = target;
+        this.sourceOwner = sourceOwner;
+        this.sourceRepo = sourceRepo;
+        this.sourceBranch = sourceBranch;
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -68,18 +86,33 @@ public int getNumber() {
         return number;
     }
 
-    /** Default for old settings. */
+    /**
+     * Default for old settings.
+     */
+    @SuppressFBWarnings("SE_PRIVATE_READ_RESOLVE_NOT_INHERITED") // because JENKINS-41453
     private Object readResolve() {
         if (merge == null) {
             merge = true;
         }
-        // leave trusted at false to be on the safe side
+        if (metadata != null) {
+            // the source branch info is missing, thankfully, the missing information is not part of the key
+            // so we can just use dummy values and this should only affect SCMFileSystem API. Once
+            // there is an index, the head will be replaced with Branch API 2.0.x and it should all go away
+            return new PullRequestSCMHead(
+                    getName(),
+                    merge,
+                    metadata.getNumber(),
+                    new BranchSCMHead(metadata.getBaseRef()),
+                    metadata.getUserLogin(),
+                    null,
+                    null
+            );
+        }
         return this;
     }
 
     /**
      * Whether we intend to build the merge of the PR head with the base branch.
-     * 
      */
     public boolean isMerge() {
         return merge;
@@ -114,4 +147,35 @@ public String getSourceBranch() {
     public String getSourceRepo() {
         return sourceRepo;
     }
+
+    @Restricted(NoExternalUse.class)
+    public static class Metadata {
+        private final int number;
+        private final String url;
+        private final String userLogin;
+        private final String baseRef;
+
+        public Metadata(int number, String url, String userLogin, String baseRef) {
+            this.number = number;
+            this.url = url;
+            this.userLogin = userLogin;
+            this.baseRef = baseRef;
+        }
+
+        public int getNumber() {
+            return number;
+        }
+
+        public String getUrl() {
+            return url;
+        }
+
+        public String getUserLogin() {
+            return userLogin;
+        }
+
+        public String getBaseRef() {
+            return baseRef;
+        }
+    }
 }