Merge pull request jenkinsci#128 from jenkinsci/bug-fixes

Add logging, and fix merge.

src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java

@@ -21,6 +21,7 @@
 import hudson.model.AbstractProject;
 import hudson.model.Cause;
 import hudson.model.Item;
+import hudson.model.ItemGroup;
 import hudson.model.Result;
 import hudson.model.Saveable;
 import hudson.model.TaskListener;
@@ -47,6 +48,8 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import jenkins.model.Jenkins;
+
 /**
  * @author janinko
  */
@@ -367,10 +370,26 @@ public static void addIfMissing(DescribableList<GhprbExtension, GhprbExtensionDe
         extensions.add(ext);
     }
 
-    public static StandardCredentials lookupCredentials(Item project, String credentialId, String uri) {
-        return (credentialId == null) ? null : CredentialsMatchers.firstOrNull(
-                    CredentialsProvider.lookupCredentials(StandardCredentials.class, project, ACL.SYSTEM,
-                            URIRequirementBuilder.fromUri(uri).build()),
+    public static StandardCredentials lookupCredentials(Item context, String credentialId, String uri) {
+        String contextName = "(Jenkins.instance)";
+        if (context != null) {
+            contextName = context.getFullName();
+        }
+        logger.log(Level.FINE, "Looking up credentials for {0}, using context {1} for url {2}", new Object[] { credentialId, contextName, uri });
+
+        List<StandardCredentials> credentials;
+
+        if (context == null) {
+            credentials = CredentialsProvider.lookupCredentials(StandardCredentials.class, Jenkins.getInstance(), ACL.SYSTEM,
+                    URIRequirementBuilder.fromUri(uri).build());
+        } else {
+            credentials = CredentialsProvider.lookupCredentials(StandardCredentials.class, context, ACL.SYSTEM,
+                    URIRequirementBuilder.fromUri(uri).build());
+        }
+
+        logger.log(Level.FINE, "Found {0} credentials", new Object[]{credentials.size()});
+
+        return (credentialId == null) ? null : CredentialsMatchers.firstOrNull(credentials,
                     CredentialsMatchers.withId(credentialId));
     }
 

src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java

@@ -4,7 +4,6 @@
 import static hudson.Util.fixEmptyAndTrim;
 
 import java.io.IOException;
-import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -30,10 +29,6 @@
 import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
 import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
 import com.cloudbees.plugins.credentials.domains.DomainRequirement;
-import com.cloudbees.plugins.credentials.domains.HostnamePortRequirement;
-import com.cloudbees.plugins.credentials.domains.HostnameRequirement;
-import com.cloudbees.plugins.credentials.domains.PathRequirement;
-import com.cloudbees.plugins.credentials.domains.SchemeRequirement;
 import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
 import com.google.common.base.Joiner;
 
@@ -112,7 +107,8 @@ private static GitHubBuilder getBuilder(Item context, String serverAPIUrl, Strin
         GitHubBuilder builder = new GitHubBuilder()
             .withEndpoint(serverAPIUrl)
             .withConnector(new HttpConnectorWithJenkinsProxy());
-        String contextName = context == null ? "(null)" : context.getFullDisplayName();
+        String contextName = context == null ? "(Jenkins.instance)" : context.getFullDisplayName();
+
         if (StringUtils.isEmpty(credentialsId)) {
             logger.log(Level.WARNING, "credentialsId not set for context {0}, using anonymous connection", contextName);
             return builder;

src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java

@@ -148,27 +148,8 @@ public void check(GHPullRequest pr) {
             source = pr.getHead().getRef(); // If this instance was created before target was introduced (before v1.8), it can be null.
         }
 
-        if (isUpdated(pr)) {
-            logger.log(Level.INFO, "Pull request #{0} was updated on {1} at {2} by {3}", new Object[] { id, reponame, updated, author });
-
-            // the title could have been updated since the original PR was opened
-            title = pr.getTitle();
-            int commentsChecked = checkComments(pr);
-            boolean newCommit = checkCommit(pr.getHead().getSha());
-
-            if (!newCommit && commentsChecked == 0) {
-                logger.log(Level.INFO, "Pull request #{0} was updated on repo {1} but there aren''t any new comments nor commits; "
-                        + "that may mean that commit status was updated.", 
-                        new Object[] { id, reponame }
-                );
-            }
-            try {
-                updated = pr.getUpdatedAt();
-            } catch (IOException e) {
-                e.printStackTrace();
-                updated = new Date();
-            }
-        }
+        updatePR(pr, author);
+
         checkSkipBuild(pr);
         tryBuild(pr);
     }
@@ -180,21 +161,40 @@ public void check(GHIssueComment comment) {
         }
         try {
             checkComment(comment);
-            updated = comment.getUpdatedAt();
         } catch (IOException ex) {
             logger.log(Level.SEVERE, "Couldn't check comment #" + comment.getId(), ex);
             return;
         }
+
 
         GHPullRequest pr = null;
         try {
             pr = repo.getPullRequest(id);
+            updatePR(pr, comment.getUser());
         } catch (IOException e) {
             logger.log(Level.SEVERE, "Couldn't get GHPullRequest for checking mergeable state");
         }
         checkSkipBuild(comment.getParent());
         tryBuild(pr);
     }
+
+    private void updatePR(GHPullRequest pr, GHUser author) {
+        if (pr != null && isUpdated(pr)) {
+            logger.log(Level.INFO, "Pull request #{0} was updated on {1} at {2} by {3}", new Object[] { id, reponame, updated, author });
+
+            // the title could have been updated since the original PR was opened
+            title = pr.getTitle();
+            int commentsChecked = checkComments(pr);
+            boolean newCommit = checkCommit(pr.getHead().getSha());
+
+            if (!newCommit && commentsChecked == 0) {
+                logger.log(Level.INFO, "Pull request #{0} was updated on repo {1} but there aren''t any new comments nor commits; "
+                        + "that may mean that commit status was updated.", 
+                        new Object[] { id, reponame }
+                );
+            }
+        }
+    }
 
     public boolean isWhiteListedTargetBranch() {
         List<GhprbBranch> branches = helper.getWhiteListTargetBranches();
@@ -215,13 +215,18 @@ public boolean isWhiteListedTargetBranch() {
     }
 
     private boolean isUpdated(GHPullRequest pr) {
+        if (pr == null) {
+            return false;
+        }
         Date lastUpdated = new Date();
+        boolean ret = false;
         try {
             lastUpdated = pr.getUpdatedAt();
-        } catch (IOException e) {
-            e.printStackTrace();
+            ret = updated.compareTo(lastUpdated) < 0;
+            updated = lastUpdated;
+        } catch (Exception e) {
+            logger.log(Level.WARNING, "Unable to update last updated date", e);
         }
-        boolean ret = updated.compareTo(lastUpdated) < 0;
         ret = ret || !pr.getHead().getSha().equals(head);
         return ret;
     }

src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java

@@ -130,21 +130,22 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, final Build
         }
 
         boolean merge = true;
+        String commentBody = cause.getCommentBody();
 
-        if (isOnlyAdminsMerge() && !helper.isAdmin(triggerSender)) {
+        if (isOnlyAdminsMerge() && (triggerSender == null || !helper.isAdmin(triggerSender) )) {
             merge = false;
             logger.println("Only admins can merge this pull request, " + triggerSender.getLogin() + " is not an admin.");
             commentOnRequest(String.format("Code not merged because %s is not in the Admin list.", triggerSender.getName()));
         }
 
-        if (isOnlyTriggerPhrase() && !helper.isTriggerPhrase(cause.getCommentBody())) {
+        if (isOnlyTriggerPhrase() && (commentBody == null || !helper.isTriggerPhrase(cause.getCommentBody()) )) {
             merge = false;
             logger.println("The comment does not contain the required trigger phrase.");
 
             commentOnRequest(String.format("Please comment with '%s' to automerge this request", trigger.getTriggerPhrase()));
         }
 
-        if (isDisallowOwnCode() && isOwnCode(pr, triggerSender)) {
+        if (isDisallowOwnCode() && (triggerSender == null || isOwnCode(pr, triggerSender) )) {
             merge = false;
             logger.println("The commentor is also one of the contributors.");
             commentOnRequest(String.format("Code not merged because %s has committed code in the request.", triggerSender.getName()));

src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java

@@ -267,6 +267,10 @@ public GitHub getGitHub() throws IOException {
     }
 
     public AbstractProject<?, ?> getActualProject() {
+
+        if (_project != null) {
+            return _project;
+        }
 
         @SuppressWarnings("rawtypes")
         List<AbstractProject> projects = Jenkins.getInstance().getAllItems(AbstractProject.class);

src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java

@@ -310,7 +310,7 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException
         verify(ghPullRequest, times(3)).getBase();
         verify(ghPullRequest, times(5)).getNumber();
         verify(ghPullRequest, times(1)).getHtmlUrl();
-        verify(ghPullRequest, times(4)).getUpdatedAt();
+        verify(ghPullRequest, times(3)).getUpdatedAt();
 
         verify(ghPullRequest, times(1)).getComments();
         verify(ghPullRequest, times(1)).listCommits();
@@ -390,7 +390,7 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException
         verify(ghPullRequest, times(8)).getHead();
         verify(ghPullRequest, times(3)).getBase();
         verify(ghPullRequest, times(5)).getNumber();
-        verify(ghPullRequest, times(4)).getUpdatedAt();
+        verify(ghPullRequest, times(3)).getUpdatedAt();
         verify(ghPullRequest, times(1)).getHtmlUrl();
 
         verify(ghPullRequest, times(1)).getComments();