[JENKINS-34395] Found out that github-api plays fast and loose with t…

…he ref/ prefix
jenkinsci · Sep 7, 2017 · d094402 · d094402
1 parent 51415fa
commit d094402
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 18 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.eclipse.jgit.lib.Constants;
 import org.kohsuke.github.GHRepository;
 import org.kohsuke.github.GHUser;
@@ -50,7 +51,16 @@ public class GitHubSCMFileSystem extends SCMFileSystem implements GitHubClosable
     private final String ref;
     private boolean open;
 
-    protected GitHubSCMFileSystem(GitHub gitHub, GHRepository repo, String ref, @CheckForNull SCMRevision rev) throws IOException {
+    /**
+     * Constructor.
+     *
+     * @param gitHub the {@link GitHub}
+     * @param repo the {@link GHRepository}
+     * @param refName the ref name, e.g. {@code heads/branchName}, {@code tags/tagName}, {@code pull/N/head} or the SHA.
+     * @param rev the optional revision.
+     * @throws IOException if I/O errors occur.
+     */
+    protected GitHubSCMFileSystem(GitHub gitHub, GHRepository repo, String refName, @CheckForNull SCMRevision rev) throws IOException {
         super(rev);
         this.gitHub = gitHub;
         this.open = true;
@@ -63,10 +73,10 @@ protected GitHubSCMFileSystem(GitHub gitHub, GHRepository repo, String ref, @Che
             } else if (rev instanceof AbstractGitSCMSource.SCMRevisionImpl) {
                 this.ref = ((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash();
             } else {
-                this.ref = ref;
+                this.ref = refName;
             }
         } else {
-            this.ref = ref;
+            this.ref = refName;
         }
     }
 
@@ -135,11 +145,11 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch
                             apiUri == null ? GitHubSCMSource.GITHUB_URL : apiUri);
                     throw new IOException(message);
                 }
-                String ref;
+                String refName;
                 if (head instanceof BranchSCMHead) {
-                    ref = Constants.R_HEADS + head.getName();
+                    refName = "heads/" + head.getName();
                 } else if (head instanceof GitHubTagSCMHead) {
-                    ref = Constants.R_TAGS + head.getName();
+                    refName = "tags/" + head.getName();
                 } else if (head instanceof PullRequestSCMHead) {
                     PullRequestSCMHead pr = (PullRequestSCMHead) head;
                     if (!pr.isMerge() && pr.getSourceRepo() != null) {
@@ -184,9 +194,9 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch
                     return null;
                 }
                 if (rev == null) {
-                    rev = new AbstractGitSCMSource.SCMRevisionImpl(head, repo.getRef(ref).getObject().getSha());
+                    rev = new AbstractGitSCMSource.SCMRevisionImpl(head, repo.getRef(refName).getObject().getSha());
                 }
-                return new GitHubSCMFileSystem(github, repo, ref, rev);
+                return new GitHubSCMFileSystem(github, repo, refName, rev);
             } catch (IOException | RuntimeException e) {
                 Connector.release(github);
                 throw e;

diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMProbe.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMProbe.java
@@ -36,6 +36,7 @@
 import jenkins.scm.api.SCMProbe;
 import jenkins.scm.api.SCMProbeStat;
 import jenkins.scm.api.SCMRevision;
+import org.apache.commons.lang.StringUtils;
 import org.eclipse.jgit.lib.Constants;
 import org.kohsuke.github.GHCommit;
 import org.kohsuke.github.GHContent;
@@ -60,11 +61,11 @@ public GitHubSCMProbe(GitHub github, GHRepository repo, SCMHead head, SCMRevisio
         this.name = head.getName();
         if (head instanceof PullRequestSCMHead) {
             PullRequestSCMHead pr = (PullRequestSCMHead) head;
-            this.ref = "refs/pull/" + pr.getNumber() + (pr.isMerge() ? "/merge" : "/head");
+            this.ref = "pull/" + pr.getNumber() + (pr.isMerge() ? "/merge" : "/head");
         } else if (head instanceof GitHubTagSCMHead){
-            this.ref = Constants.R_TAGS + head.getName();
+            this.ref = "tags/" + head.getName();
         } else {
-            this.ref = Constants.R_HEADS + head.getName();
+            this.ref = "heads/" + head.getName();
         }
     }
 
@@ -115,7 +116,7 @@ public long lastModified() {
             }
         } else if (revision == null) {
             try {
-                GHRef ref = repo.getRef(this.ref);
+                GHRef ref = repo.getRef(StringUtils.removeStart(this.ref, Constants.R_REFS));
                 GHCommit commit = repo.getCommit(ref.getObject().getSha());
                 return commit.getCommitDate().getTime();
             } catch (IOException e) {

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
@@ -1220,7 +1220,7 @@ protected SCMRevision retrieve(@NonNull String headName, @NonNull TaskListener l
                 // maybe it's a tag
             }
             try {
-                GHRef tag = ghRepository.getRef(Constants.R_TAGS + headName);
+                GHRef tag = ghRepository.getRef("tags/" + headName);
                 if (tag != null && context.wantTags()) {
                     long tagDate = 0L;
                     String tagSha = tag.getObject().getSha();
@@ -2071,7 +2071,7 @@ protected Iterable<GHRef> create() {
                 if (tagNames != null && tagNames.size() == 1) {
                     String tagName = tagNames.iterator().next();
                     request.listener().getLogger().format("%n  Getting remote tag %s...%n", tagName);
-                    return Collections.singletonList(repo.getRef(Constants.R_TAGS + tagName));
+                    return Collections.singletonList(repo.getRef("tags/" + tagName));
                 }
                 request.listener().getLogger().format("%n  Getting remote tags...%n");
                 return repo.listRefs("tags");

diff --git a/src/test/resources/api/mappings/mapping-yolo-contents-64985.json b/src/test/resources/api/mappings/mapping-yolo-contents-64985.json
@@ -1,6 +1,6 @@
 {
   "request" : {
-    "url" : "/repos/cloudbeers/yolo/contents/?ref=refs%2Fpull%2F2%2Fhead",
+    "url" : "/repos/cloudbeers/yolo/contents/?ref=pull%2F2%2Fhead",
     "method" : "GET"
   },
   "response" : {

diff --git a/src/test/resources/api/mappings/mapping-yolo-contents-8rd37.json b/src/test/resources/api/mappings/mapping-yolo-contents-8rd37.json
@@ -1,6 +1,6 @@
 {
   "request" : {
-    "url" : "/repos/cloudbeers/yolo/contents/?ref=refs%2Fpull%2F2%2Fmerge",
+    "url" : "/repos/cloudbeers/yolo/contents/?ref=pull%2F2%2Fmerge",
     "method" : "GET"
   },
   "response" : {

diff --git a/src/test/resources/api/mappings/mapping-yolo-contents-DRrwo.json b/src/test/resources/api/mappings/mapping-yolo-contents-DRrwo.json
@@ -1,6 +1,6 @@
 {
   "request" : {
-    "url" : "/repos/cloudbeers/yolo/contents/?ref=refs%2Fheads%2Fmaster",
+    "url" : "/repos/cloudbeers/yolo/contents/?ref=heads%2Fmaster",
     "method" : "GET"
   },
   "response" : {

diff --git a/src/test/resources/api/mappings/mapping-yolo-contents-pcVDb.json b/src/test/resources/api/mappings/mapping-yolo-contents-pcVDb.json
@@ -1,6 +1,6 @@
 {
   "request" : {
-    "url" : "/repos/cloudbeers/yolo/contents/?ref=refs%2Fheads%2Fstephenc-patch-1",
+    "url" : "/repos/cloudbeers/yolo/contents/?ref=heads%2Fstephenc-patch-1",
     "method" : "GET"
   },
   "response" : {