Merge pull request jenkinsci#272 from jenkinsci/master

Merge jenkinsci into janinko

README.md

@@ -86,6 +86,13 @@ If you want to manually build the job, in the job setting check ``This build is
 
 ### Updates
 
+#### -> 1.21
+* Move all commenting logic out into extensions.
+
+#### -> 1.20.1
+* Null Pointer fix for trigger.
+* Added clarity to error message when access is forbidden.
+
 #### -> 1.20
 * PullRequestMerger now notifies the taskListener of failures.
 * AutoCloseFailedPullRequest has been extracted from the published URL check.

pom.xml

@@ -10,7 +10,7 @@
 
     <artifactId>ghprb</artifactId>
     <name>GitHub Pull Request Builder</name>
-    <version>1.21-SNAPSHOT</version>
+    <version>1.22-SNAPSHOT</version>
     <packaging>hpi</packaging>
 
     <url>https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin</url>

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

@@ -5,8 +5,19 @@
 import hudson.Util;
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
+import hudson.model.Result;
+import hudson.model.Saveable;
+import hudson.util.DescribableList;
 import hudson.util.LogTaskListener;
 
+import org.apache.commons.collections.Predicate;
+import org.apache.commons.collections.PredicateUtils;
+import org.apache.commons.collections.functors.InstanceofPredicate;
+import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension;
+import org.jenkinsci.plugins.ghprb.extensions.GhprbExtensionDescriptor;
+import org.jenkinsci.plugins.ghprb.extensions.GhprbGlobalExtension;
+import org.jenkinsci.plugins.ghprb.extensions.GhprbProjectExtension;
+import org.kohsuke.github.GHCommitState;
 import org.kohsuke.github.GHUser;
 
 import java.util.*;
@@ -178,5 +189,97 @@ public static String replaceMacros(AbstractBuild<?, ?> build, String inputString
         return returnString;
 
     }
+
+    public static GHCommitState getState(AbstractBuild<?, ?> build) {
+
+        GHCommitState state;
+        if (build.getResult() == Result.SUCCESS) {
+            state = GHCommitState.SUCCESS;
+        } else if (build.getResult() == Result.UNSTABLE) {
+            state = GhprbTrigger.getDscp().getUnstableAs();
+        } else {
+            state = GHCommitState.FAILURE;
+        }
+        return state;
+    }
+
+    public static Set<String> createSet(String list) {
+        String listString = list == null ? "" : list;
+        List<String> listList = Arrays.asList(listString.split("\\s+"));
+        Set<String> listSet = new HashSet<String>(listList);
+        listSet.remove("");
+        return listSet;
+    }
+
+    public static GhprbTrigger extractTrigger(AbstractBuild<?, ?> build) {
+        return extractTrigger(build.getProject());
+    }
+
+    public static GhprbTrigger extractTrigger(AbstractProject<?, ?> p) {
+        GhprbTrigger trigger = p.getTrigger(GhprbTrigger.class);
+        if (trigger == null || (!(trigger instanceof GhprbTrigger))) {
+            return null;
+        }
+        return trigger;
+    }
+
+    private static List<Predicate> createPredicate(Class<?> ...types) {
+        List<Predicate> predicates = new ArrayList<Predicate>(types.length);
+        for (Class<?> type : types) {
+            predicates.add(InstanceofPredicate.getInstance(type));
+        }
+        return predicates;
+    }
+
+    public static void filterList(DescribableList<GhprbExtension, GhprbExtensionDescriptor> descriptors, Predicate predicate) {
+        for (GhprbExtension descriptor : descriptors) {
+            if (!predicate.evaluate(descriptor)) {
+                descriptors.remove(descriptor);
+            }
+        }
+    }
+
+    private static DescribableList<GhprbExtension, GhprbExtensionDescriptor> copyExtensions(DescribableList<GhprbExtension, GhprbExtensionDescriptor> ...extensionsList){
+        DescribableList<GhprbExtension, GhprbExtensionDescriptor> copiedList = new DescribableList<GhprbExtension, GhprbExtensionDescriptor>(Saveable.NOOP);
+        for (DescribableList<GhprbExtension, GhprbExtensionDescriptor> extensions: extensionsList) {
+            copiedList.addAll(extensions);
+        }
+        return copiedList;
+    }
+
+    @SuppressWarnings("unchecked")
+    public static DescribableList<GhprbExtension, GhprbExtensionDescriptor> getJobExtensions(GhprbTrigger trigger, Class<?> ...types) {
+
+        // First get all global extensions
+        DescribableList<GhprbExtension, GhprbExtensionDescriptor> copied = copyExtensions(trigger.getDescriptor().getExtensions());
+
+        // Remove extensions that are specified by job
+        filterList(copied, PredicateUtils.notPredicate(InstanceofPredicate.getInstance(GhprbProjectExtension.class)));
+
+        // Then get the rest of the extensions from the job
+        copied = copyExtensions(copied, trigger.getExtensions());
+
+        // Filter extensions by desired interface
+        filterList(copied, PredicateUtils.anyPredicate(createPredicate(types)));
+        return copied;
+    }
+
+    public static DescribableList<GhprbExtension, GhprbExtensionDescriptor> matchesAll(DescribableList<GhprbExtension, GhprbExtensionDescriptor> extensions, Class<?> ...types) {
+        Predicate predicate = PredicateUtils.allPredicate(createPredicate(types));
+        DescribableList<GhprbExtension, GhprbExtensionDescriptor> copyExtensions = new DescribableList<GhprbExtension, GhprbExtensionDescriptor>(Saveable.NOOP);
+
+        copyExtensions.addAll(extensions);
+        filterList(copyExtensions, predicate);
+        return copyExtensions;
+    }
+
+    public static DescribableList<GhprbExtension, GhprbExtensionDescriptor> matchesSome(DescribableList<GhprbExtension, GhprbExtensionDescriptor> extensions, Class<?> ...types) {
+        Predicate predicate = PredicateUtils.anyPredicate(createPredicate(types));
+        DescribableList<GhprbExtension, GhprbExtensionDescriptor> copyExtensions = new DescribableList<GhprbExtension, GhprbExtensionDescriptor>(Saveable.NOOP);
+
+        copyExtensions.addAll(extensions);
+        filterList(copyExtensions, predicate);
+        return copyExtensions;
+    }
 
 }

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

@@ -29,6 +29,6 @@ public void onCompleted(AbstractBuild<?, ?> build, TaskListener listener) {
     }
 
     private static Optional<GhprbTrigger> findTrigger(AbstractBuild<?, ?> build) {
-        return Optional.fromNullable(GhprbTrigger.extractTrigger(build.getProject()));
+        return Optional.fromNullable(Ghprb.extractTrigger(build));
     }
 }

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

@@ -2,26 +2,19 @@
 
 import hudson.model.AbstractBuild;
 import hudson.model.Cause;
-import hudson.model.Result;
 import hudson.model.TaskListener;
 import hudson.model.queue.QueueTaskFuture;
 import hudson.plugins.git.util.BuildData;
 
-import org.apache.commons.io.FileUtils;
-
-import org.jenkinsci.plugins.ghprb.manager.GhprbBuildManager;
-import org.jenkinsci.plugins.ghprb.manager.configuration.JobConfiguration;
-import org.jenkinsci.plugins.ghprb.manager.factory.GhprbBuildManagerFactoryUtil;
-
+import org.jenkinsci.plugins.ghprb.extensions.GhprbCommentAppender;
+import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension;
 import org.kohsuke.github.GHCommitState;
 import org.kohsuke.github.GHIssueState;
 import org.kohsuke.github.GHPullRequest;
 import org.kohsuke.github.GHUser;
 
-import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
-import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -123,20 +116,10 @@ public void onCompleted(AbstractBuild<?, ?> build, TaskListener listener) {
         }
 
         GHCommitState state;
-        if (build.getResult() == Result.SUCCESS) {
-            state = GHCommitState.SUCCESS;
-        } else if (build.getResult() == Result.UNSTABLE) {
-            state = GHCommitState.valueOf(GhprbTrigger.getDscp().getUnstableAs());
-        } else {
-            state = GHCommitState.FAILURE;
-        }
+        state = Ghprb.getState(build);
         repo.createCommitStatus(build, state, "Build finished.", c.getPullID(), trigger.getCommitStatusContext(), listener.getLogger());
 
-
-        String publishedURL = GhprbTrigger.getDscp().getPublishedURL();
-        if (publishedURL != null && !publishedURL.isEmpty()) {
-            buildResultMessage(build, listener, state, c);
-        }
+        buildResultMessage(build, listener, state, c);
         // close failed pull request automatically
         if (state == GHCommitState.FAILURE && trigger.isAutoCloseFailedPullRequests()) {
             closeFailedRequest(listener, c);
@@ -158,89 +141,17 @@ private void closeFailedRequest(TaskListener listener, GhprbCause c) {
 
     private void buildResultMessage(AbstractBuild<?, ?> build, TaskListener listener, GHCommitState state, GhprbCause c) {
         StringBuilder msg = new StringBuilder();
-        String commentFilePath = trigger.getCommentFilePath();
-
-        if (commentFilePath != null && !commentFilePath.isEmpty()) {
-            try {
-                String scriptFilePathResolved = Ghprb.replaceMacros(build, commentFilePath);
-
-                String content = FileUtils.readFileToString(new File(scriptFilePathResolved));
-                msg.append("Build comment file: \n--------------\n");
-                msg.append(content);
-                msg.append("\n--------------\n");
-            } catch (IOException e) {
-                msg.append("\n!!! Couldn't read commit file !!!\n");
-                listener.getLogger().println("Couldn't read comment file");
-                e.printStackTrace(listener.getLogger());
-            }
-        }
-
-        msg.append("\nRefer to this link for build results (access rights to CI server needed): \n");
-        msg.append(generateCustomizedMessage(build));
-
-        int numLines = GhprbTrigger.getDscp().getlogExcerptLines();
-        if (state != GHCommitState.SUCCESS && numLines > 0) {
-            // on failure, append an excerpt of the build log
-            try {
-                // wrap log in "code" markdown
-                msg.append("\n\n**Build Log**\n*last ").append(numLines).append(" lines*\n");
-                msg.append("\n ```\n");
-                List<String> log = build.getLog(numLines);
-                for (String line : log) {
-                    msg.append(line).append('\n');
-                }
-                msg.append("```\n");
-            } catch (IOException ex) {
-                listener.getLogger().println("Can't add log excerpt to commit comments");
-                ex.printStackTrace(listener.getLogger());
-            }
-        }
-
-        String buildMessage = null;
-        if (state == GHCommitState.SUCCESS) {
-            if (trigger.getMsgSuccess() != null && !trigger.getMsgSuccess().isEmpty()) {
-                buildMessage = trigger.getMsgSuccess();
-            } else if (GhprbTrigger.getDscp().getMsgSuccess(build) != null 
-                    && !GhprbTrigger.getDscp().getMsgSuccess(build).isEmpty()) {
-                buildMessage = GhprbTrigger.getDscp().getMsgSuccess(build);
-            }
-        } else if (state == GHCommitState.FAILURE) {
-            if (trigger.getMsgFailure() != null && !trigger.getMsgFailure().isEmpty()) {
-                buildMessage = trigger.getMsgFailure();
-            } else if (GhprbTrigger.getDscp().getMsgFailure(build) != null 
-                    && !GhprbTrigger.getDscp().getMsgFailure(build).isEmpty()) {
-                buildMessage = GhprbTrigger.getDscp().getMsgFailure(build);
+
+        for (GhprbExtension ext : Ghprb.getJobExtensions(trigger, GhprbCommentAppender.class)){
+            if (ext instanceof GhprbCommentAppender) {
+                msg.append(((GhprbCommentAppender) ext).postBuildComment(build, listener));
             }
         }
-        // Only Append the build's custom message if it has been set.
-        if (buildMessage != null && !buildMessage.isEmpty()) {
-            // When the msg is not empty, append a newline first, to seperate it from the rest of the String
-            if (!"".equals(msg.toString())) {
-                msg.append("\n");
-            }
-            msg.append(buildMessage);
-        }
-
+
         if (msg.length() > 0) {
             listener.getLogger().println(msg);
             repo.addComment(c.getPullID(), msg.toString(), build, listener);
         }
     }
 
-    private String generateCustomizedMessage(AbstractBuild<?, ?> build) {
-        JobConfiguration jobConfiguration = JobConfiguration.builder()
-                .printStackTrace(trigger.isDisplayBuildErrorsOnDownstreamBuilds()).build();
-
-        GhprbBuildManager buildManager = GhprbBuildManagerFactoryUtil.getBuildManager(build, jobConfiguration);
-
-        StringBuilder sb = new StringBuilder();
-
-        sb.append(buildManager.calculateBuildUrl());
-
-        if (build.getResult() != Result.SUCCESS) {
-            sb.append(buildManager.getTestResults());
-        }
-
-        return sb.toString();
-    }
 }

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

@@ -23,8 +23,11 @@ private void connect() throws IOException {
         String serverAPIUrl = GhprbTrigger.getDscp().getServerAPIUrl();
         if (accessToken != null && !accessToken.isEmpty()) {
             try {
-                gh = new GitHubBuilder().withEndpoint(serverAPIUrl)
-                        .withOAuthToken(accessToken).withConnector(new HttpConnectorWithJenkinsProxy()).build();
+                gh = new GitHubBuilder()
+                    .withEndpoint(serverAPIUrl)
+                    .withOAuthToken(accessToken)
+                    .withConnector(new HttpConnectorWithJenkinsProxy())
+                    .build();
             } catch (IOException e) {
                 logger.log(Level.SEVERE, "Can''t connect to {0} using oauth", serverAPIUrl);
                 throw e;

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

@@ -86,7 +86,7 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, final Build
             return true;
         }
 
-        trigger = GhprbTrigger.extractTrigger(project);
+        trigger = Ghprb.extractTrigger(project);
         if (trigger == null)
             return false;
 

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

@@ -6,6 +6,9 @@
 import hudson.model.TaskListener;
 import jenkins.model.Jenkins;
 
+import org.jenkinsci.plugins.ghprb.extensions.GhprbCommentAppender;
+import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension;
+import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildStatus;
 import org.kohsuke.github.*;
 import org.kohsuke.github.GHEventPayload.IssueComment;
 import org.kohsuke.github.GHEventPayload.PullRequest;
@@ -76,7 +79,7 @@ public void check() {
         if (!initGhRepository()) {
             return;
         }
-        
+
         if (helper.isProjectDisabled()) {
             logger.log(Level.FINE, "Project is disabled, not checking github state");
             return;
@@ -144,16 +147,12 @@ public void createCommitStatus(AbstractBuild<?, ?> build, String sha1, GHCommitS
             } else {
                 ghRepository.createCommitStatus(sha1, state, url, message);
             }
-        } catch (FileNotFoundException ex) {
-            newMessage = "FileNotFoundException means that the credentials Jenkins is using is probably wrong. Or that something is really wrong with github.";
-            if (stream != null) {
-                stream.println(newMessage);
-                ex.printStackTrace(stream);
+        } catch (IOException ex) {
+            if (ex instanceof FileNotFoundException) {
+              newMessage = "FileNotFoundException means that the credentials Jenkins is using is probably wrong. Or the user account does not have write access to the repo.";
             } else {
-                logger.log(Level.INFO, newMessage, ex);
+              newMessage = "Could not update commit status of the Pull Request on GitHub.";
             }
-        } catch (IOException ex) {
-            newMessage = "Could not update commit status of the Pull Request on GitHub.";
             if (stream != null) {
                 stream.println(newMessage);
                 ex.printStackTrace(stream);
@@ -162,14 +161,21 @@ public void createCommitStatus(AbstractBuild<?, ?> build, String sha1, GHCommitS
             }
             if (GhprbTrigger.getDscp().getUseComments()) {
 
-                if (state == GHCommitState.SUCCESS) {
-                    message = message + " " + GhprbTrigger.getDscp().getMsgSuccess(build);
-                } else if (state == GHCommitState.FAILURE) {
-                    message = message + " " + GhprbTrigger.getDscp().getMsgFailure(build);
+                StringBuilder msg = new StringBuilder(message);
+
+                if (build != null) {
+                    msg.append("\n");
+                    GhprbTrigger trigger = Ghprb.extractTrigger(build);
+                    for (GhprbExtension ext : Ghprb.matchesAll(trigger.getExtensions(), GhprbBuildStatus.class)) {
+                        if (ext instanceof GhprbCommentAppender) {
+                            msg.append(((GhprbCommentAppender) ext).postBuildComment(build, null));
+                        }
+                    }
                 }
+
                 if (GhprbTrigger.getDscp().getUseDetailedComments() || (state == GHCommitState.SUCCESS || state == GHCommitState.FAILURE)) {
                     logger.log(Level.INFO, "Trying to send comment.", ex);
-                    addComment(id, message);
+                    addComment(id, msg.toString());
                 }
             } else {
                 logger.log(Level.SEVERE, "Could not update commit status of the Pull Request on GitHub.");