Fixed issue with build parameters not being correctly set. Fixes #19

jenkinsci · Jul 29, 2016 · d9f0ea0 · d9f0ea0
1 parent b9b0cd6
commit d9f0ea0
Show file tree

Hide file tree

Showing 5 changed files with 129 additions and 37 deletions.
diff --git a/build.gradle b/build.gradle
@@ -49,6 +49,10 @@ dependencies {
     compile "org.jenkins-ci.lib:xtrigger-lib:0.25"
     compile "com.sun.mail:javax.mail:1.5.1"
 
+    // Useful for interrogating the injected environment variables...
+//    compile "org.jenkins-ci.plugins:envinject:1.92.1"
+//    compile("org.jenkins-ci.lib:envinject-lib:1.23"){ force = true }
+
     testCompile('org.hamcrest:hamcrest-core:1.3') { force = true }
     testCompile('junit:junit:4.12') { force = true }
     testCompile 'org.jvnet.mock-javamail:mock-javamail:1.9'
@@ -66,7 +70,7 @@ configurations.findAll {!it.name.endsWith('junit-dep')}.each { conf ->
 
 jpi.outputs.upToDateWhen { false }
 jpi.doLast {
-    verifySize(file(jpi.archivePath), 638)
+    verifySize(file(jpi.archivePath), 639)
 }
 
 task unzip(type: Copy) {

diff --git a/src/main/groovy/org/jenkinsci/plugins/pollmailboxtrigger/PollMailboxTrigger.java b/src/main/groovy/org/jenkinsci/plugins/pollmailboxtrigger/PollMailboxTrigger.java
@@ -194,10 +194,10 @@ public static CustomProperties initialiseDefaults(
         return p;
     }
 
-    public static FormValidation checkForEmails(final CustomProperties properties, final XTriggerLog log, final boolean testConnection, final PollMailboxTrigger pmt) {
+    public static FormValidation checkForEmails(final CustomProperties properties, final XTriggerLog logger, final boolean testConnection, final PollMailboxTrigger pmt) {
         MailReader mailbox = null;
         final SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT_TEXT);
-        List<String> testing = new ArrayList<String>();
+        TeeLogger log = new TeeLogger(logger, new ArrayList<String>());
         try {
             Enum[] requiredProps = {Properties.host, Properties.storeName, Properties.username, Properties.password};
             List<String> errors = new ArrayList<String>();
@@ -224,13 +224,20 @@ public static FormValidation checkForEmails(final CustomProperties properties, f
                     properties.get(Properties.username),
                     decryptedPassword,
                     storeNameValue,
-                    new Logger.XTriggerLoggerWrapper(log),
+                    new Logger.XTriggerLoggerWrapper(log.getLog()),
                     properties
             ).connect();
-            final String connected = "Connected to mailbox. ";
-            log.info(connected + "Searching for messages where:");
-            testing.add(connected);
 
+            // output found properties...
+            log.info("Found properties:");
+            List<String> keys = new ArrayList<String>(properties.keySet());
+            Collections.sort(keys);
+            for (String key : keys) {
+                String obfuscatedValue = Properties.password.name().equalsIgnoreCase(key) ? "***" : properties.get(key);
+                log.info("- [" + key + ":" + obfuscatedValue + "]");
+            }
+
+            log.info("Connected to mailbox. Searching for messages where:");
             // search for messages
             List<SearchTerm> searchTerms = new ArrayList<SearchTerm>();
             // unread
@@ -240,21 +247,25 @@ public static FormValidation checkForEmails(final CustomProperties properties, f
             if (properties.has(subjectContains)) {
                 searchTerms.add(subject(properties.get(subjectContains)));
                 log.info("- [subject contains '" + properties.get(subjectContains) + "']");
+            } else {
+                log.info("- [subject contains - filter not set]");
             }
             // received since X minutes ago
             if (!isStorePop3(storeNameValue) && properties.has(receivedXMinutesAgo)) {
                 final int minsAgo = Integer.parseInt(properties.get(receivedXMinutesAgo)) * -1;
                 Date date = relativeDate(Calendar.MINUTE, minsAgo);
                 searchTerms.add(receivedSince(date));
                 log.info("- [received date is greater than '" + dateFormat.format(date) + "']");
+            } else {
+                log.info("- [received date - filter not set]");
             }
             log.info("...");
             if (!properties.has(folder)) {
                 throw new FolderNotFoundException();
             } else {
                 // look for mail...
                 final MailWrapperUtils.FolderWrapper mbFolder = mailbox.folder(properties.get(folder));
-                testing.add("Searching folder...");
+                log.info("Searching folder...");
                 String downloadAttachments = properties.get(Properties.attachments);
                 MessagesWrapper messagesTool = mbFolder.search(searchTerms);
                 List<Message> messageList = messagesTool.getMessages();
@@ -265,7 +276,6 @@ public static FormValidation checkForEmails(final CustomProperties properties, f
                 }
                 final String foundEmails = "Found matching email(s) : " + messageList.size() + subjects.toString();
                 log.info(foundEmails);
-                testing.add(foundEmails);
                 if (!testConnection) {
                     // trigger jobs...
                     for (Message message : messageList) {
@@ -286,30 +296,32 @@ public static FormValidation checkForEmails(final CustomProperties properties, f
 
                         String jobCause = "Job was triggered by email sent from " + stringify(message.getFrom());
                         // start a jenkins job...
-                        pmt.startJob(log, jobCause, buildParams.getMap());
+                        pmt.startJob(log.getLog(), jobCause, buildParams.getMap());
                         // finally mark the message as read so that we don't reprocess it...
                         messagesTool.markAsRead(message);
                     }
+                } else {
+                    log.info("'Test connection' mode enabled - server connection skipped");
                 }
             }
             // return success
             if (testConnection) {
-                testing.add("\nResult: Success!");
-                return FormValidation.ok(stringify(testing, "\n"));
+                log.info("\nResult: Success!");
+                return FormValidation.ok(stringify(log.getTesting(), "\n"));
             }
         } catch (FolderNotFoundException e) {
             // list any folders we can find...
             try {
-                testing.add("Please set the 'folder=XXX' parameter to one of the following values: ");
+                log.info("Please set the 'folder=XXX' parameter to one of the following values: ");
                 final String folders = stringify(mailbox.getFolders());
-                testing.add("Folders: " + folders);
+                log.info("Folders: " + folders);
                 log.info(folders);
-                return FormValidation.error(stringify(testing, "\n"));
+                return FormValidation.error(stringify(log.getTesting(), "\n"));
             } catch (Throwable t) {
-                return handleError(log, testing, t);
+                return handleError(log.getLog(), log.getTesting(), t);
             }
         } catch (Throwable t) {
-            return handleError(log, testing, t);
+            return handleError(log.getLog(), log.getTesting(), t);
         } finally {
             // cleanup connections
             if (mailbox != null) {
@@ -433,10 +445,14 @@ protected void startJob(final XTriggerLog log, final String jobTriggerCause, fin
             AbstractProject project = getJob();
             List<Action> actions = new ArrayList<Action>();
             actions.addAll(getScheduledXTriggerActions(log));
-            actions.add(new ParametersAction(getParameterizedParams(project, envVars)));
-            actions.add(new ParametersAction(convertToBuildParams(envVars)));
 
-            // build parameters for schedule job...
+            // setup build params...
+            List<ParameterValue> buildParams = new ArrayList<ParameterValue>();
+            buildParams.addAll(getParameterizedParams(log, project, envVars));
+            buildParams.addAll(convertToBuildParams(envVars));
+            actions.add(new ParametersAction(buildParams));
+
+            // schedule job...
             Cause cause = new NewEmailCause(getName(), jobTriggerCause, true);
             Action[] actionsArray = actions.toArray(new Action[actions.size()]);
             project.scheduleBuild(0, cause, actionsArray);
@@ -472,11 +488,10 @@ private List<ParameterValue> convertToBuildParams(final Map<String, String> envV
      * map values if possible, else use defaults.
      */
     @SuppressWarnings("unchecked")
-    private List<ParameterValue> getParameterizedParams(final AbstractProject project, final Map<String, String> envVars) {
+    private List<ParameterValue> getParameterizedParams(final XTriggerLog log, final AbstractProject project, final Map<String, String> envVars) {
         List<ParameterValue> buildParams = new ArrayList<ParameterValue>();
         if (project.isParameterized()) {
-            Class<? extends JobProperty> clazz = ParametersDefinitionProperty.class;
-            JobProperty properties = project.getProperty(clazz);
+            JobProperty properties = project.getProperty(ParametersDefinitionProperty.class);
             if (properties != null && properties instanceof ParametersDefinitionProperty) {
                 ParametersDefinitionProperty parameterizedProperties = (ParametersDefinitionProperty) properties;
                 for (ParameterDefinition parameterDef : parameterizedProperties.getParameterDefinitions()) {
@@ -485,13 +500,23 @@ private List<ParameterValue> getParameterizedParams(final AbstractProject projec
                     if (envVars.containsKey(parameterName)) {
                         if (parameterDef instanceof SimpleParameterDefinition) {
                             SimpleParameterDefinition simpleParamDef = (SimpleParameterDefinition) parameterDef;
-                            parameterValue = simpleParamDef.createValue(envVars.get(parameterName));
+                            String newValue = envVars.get(parameterName);
+                            log.info(String.format("Replacing default parameter '%s' with value '%s'", parameterName, newValue));
+                            parameterValue = simpleParamDef.createValue(newValue);
+                        } else {
+                            log.info(String.format("Job property '%s' is not a Simple parameter. Keeping default value.", parameterName));
                         }
                         envVars.remove(parameterName);
+                    } else {
+                        log.info(String.format("Job property '%s' doesn't exist in env vars, using default value.", parameterName));
                     }
                     buildParams.add(parameterValue);
                 }
+            } else {
+                log.info("Job has no properties.");
             }
+        } else {
+            log.info("Job has no parameters.");
         }
         return buildParams;
     }

diff --git a/src/main/groovy/org/jenkinsci/plugins/pollmailboxtrigger/TeeLogger.java b/src/main/groovy/org/jenkinsci/plugins/pollmailboxtrigger/TeeLogger.java
@@ -0,0 +1,38 @@
+package org.jenkinsci.plugins.pollmailboxtrigger;
+
+import org.jenkinsci.lib.xtrigger.XTriggerLog;
+
+import java.util.List;
+
+/**
+ * I pity the fool, who tries to log to two places...
+ * <b>See also:</b> <a href="https://en.wikipedia.org/wiki/Tee_(command)">https://en.wikipedia.org/wiki/Tee_(command)</a>
+ */
+public class TeeLogger {
+
+    private XTriggerLog log;
+    private List<String> testing;
+
+    public TeeLogger(final XTriggerLog log, final List<String> testing) {
+        this.log = log;
+        this.testing = testing;
+    }
+
+    public void info(final String message) {
+        log.info(message);
+        testing.add(message);
+    }
+
+    public void error(final String message) {
+        log.error(message);
+        testing.add(message);
+    }
+
+    public XTriggerLog getLog() {
+        return log;
+    }
+
+    public List<String> getTesting() {
+        return testing;
+    }
+}
diff --git a/src/test/resources/org/jenkinsci/plugins/pollmailboxtrigger/Runtime - Start Job.feature b/src/test/resources/org/jenkinsci/plugins/pollmailboxtrigger/Runtime - Start Job.feature
@@ -36,21 +36,38 @@ Feature: Runtime - Start Job
     """
     Connecting to the mailbox...
     [Poll Mailbox Trigger] - Connected!
+    Found properties:
+    - [attachments:IGNORE]
+    - [folder:INBOX]
+    - [host:mail.com]
+    - [mail.debug:false]
+    - [mail.debug.auth:false]
+    - [mail.imap.connectiontimeout:1000]
+    - [mail.imap.host:mail.com]
+    - [mail.imap.port:143]
+    - [password:***]
+    - [receivedXMinutesAgo:1440]
+    - [storeName:imap]
+    - [subjectContains:jenkins >]
+    - [username:rick]
     Connected to mailbox. Searching for messages where:
     - [flag is unread]
     - [subject contains 'jenkins >']
     - [received date is greater than '<date>']
     ...
+    Searching folder...
     Found matching email(s) : 2
 
     - Jenkins > Build Plugin #3 (<date>)
 
     - Jenkins > Build Plugin #4 (<date>)
     Download attachments? IGNORE
     Changes found. Scheduling a build.
+    Job has no parameters.
     Marked email(s) as read : 1
     Download attachments? IGNORE
     Changes found. Scheduling a build.
+    Job has no parameters.
     Marked email(s) as read : 1
 
     """

diff --git a/.../resources/org/jenkinsci/plugins/pollmailboxtrigger/Test Connection - Happy Paths.feature b/.../resources/org/jenkinsci/plugins/pollmailboxtrigger/Test Connection - Happy Paths.feature
@@ -1,7 +1,7 @@
 
 Feature: Test Connection - Happy Paths
 
-  Background: The tests are setup.
+  Background: The tests are setup
     Given the plugin is initialised
     And a mailbox with domain mail.com and username rick
     And I set the configuration to
@@ -13,55 +13,63 @@ Feature: Test Connection - Happy Paths
     mail.imap.connectiontimeout=2000
     """
 
-  Scenario: Zero Emails.
+  Scenario: Zero Emails
     Given the emails
       | Subject | SentXMinutesAgo | IsSeenFlag |
     When I test the connection
-    Then the response should be OK with message 'Connected to mailbox. <br>Searching folder...<br>Found matching email(s) : 0<br><br>Result: Success!'
+    Then the response should be OK with message 'Connected to mailbox.'
+    Then the response should be OK with message 'Found matching email(s) : 0<br>'
 
     # One Email
 
-  Scenario: One Email - Subject doesn't match.
+  Scenario: One Email - Subject doesn't match
     Given the emails
       | Subject                  | SentXMinutesAgo | IsSeenFlag |
       | Hudson > Build Plugin #1 | 5               | false      |
     When I test the connection
-    Then the response should be OK with message 'Connected to mailbox. <br>Searching folder...<br>Found matching email(s) : 0<br><br>Result: Success!'
+    Then the response should be OK with message 'Connected to mailbox.'
+    Then the response should be OK with message 'Found matching email(s) : 0<br>'
 
 
-  Scenario: One Email - SentXMinutesAgo is outside of range.
+  Scenario: One Email - SentXMinutesAgo is outside of range
     Given the emails
       | Subject                   | SentXMinutesAgo | IsSeenFlag |
       | Jenkins > Build Plugin #1 | 1445            | false      |
     When I test the connection
-    Then the response should be OK with message 'Connected to mailbox. <br>Searching folder...<br>Found matching email(s) : 0<br><br>Result: Success!'
+    Then the response should be OK with message 'Connected to mailbox.'
+    Then the response should be OK with message 'Found matching email(s) : 0<br>'
 
 
-  Scenario: One Email - Email has already been read.
+  Scenario: One Email - Email has already been read
     Given the emails
       | Subject                   | SentXMinutesAgo | IsSeenFlag |
       | Jenkins > Build Plugin #1 | 5               | true       |
     When I test the connection
-    Then the response should be OK with message 'Connected to mailbox. <br>Searching folder...<br>Found matching email(s) : 0<br><br>Result: Success!'
+    Then the response should be OK with message 'Connected to mailbox.'
+    Then the response should be OK with message 'Found matching email(s) : 0<br>'
 
 
-  Scenario: One Email - subject, sentDateTime and isSeen flag match.
+  Scenario: One Email - subject, sentDateTime and isSeen flag match
     Given the emails
       | Subject                   | SentXMinutesAgo | IsSeenFlag |
       | Jenkins > Build Plugin #1 | 5               | false      |
     When I test the connection
-    Then the response should be OK with message 'Connected to mailbox. <br>Searching folder...<br>Found matching email(s) : 1<br><br>- Jenkins > Build Plugin #1'
+    Then the response should be OK with message 'Connected to mailbox.'
+    Then the response should be OK with message 'Found matching email(s) : 1<br>'
+    Then the response should be OK with message '- Jenkins > Build Plugin #1'
 
     # Multiple Emails
 
-  Scenario: Multiple Emails - all match.
+  Scenario: Multiple Emails - all match
     Given the emails
       | Subject                   | SentXMinutesAgo | IsSeenFlag |
       | Jenkins > Build Plugin #1 | 5               | false      |
       | Jenkins > Build Plugin #2 | 10              | false      |
       | Jenkins > Build Plugin #3 | 15              | false      |
     When I test the connection
-    Then the response should be OK with message 'Connected to mailbox. <br>Searching folder...<br>Found matching email(s) : 3<br><br>- Jenkins > Build Plugin #1'
+    Then the response should be OK with message 'Connected to mailbox.'
+    Then the response should be OK with message 'Found matching email(s) : 3<br>'
+    Then the response should be OK with message '- Jenkins > Build Plugin #1'
 
 
 #  Scenario: I want the plugin to tell me if authentication failed, so that I can troubleshoot the root cause of any issues.