Refactoring

* More logging and also logging more to build log.
* Adding optional path prefix, to adjust to reported file names.
* Getting workspace from build instead of WORKSPACE variable. Variable may not be correct for parallell builds.

debug.sh

@@ -1,4 +1,4 @@
 #!/bin/sh
 cd plugin
-mvnDebug -q hpi:run
+mvnDebug -q hpi:run -Djava.util.logging.config.file=../logging.properties
 

logging.properties

@@ -0,0 +1,6 @@
+# Logging
+handlers = java.util.logging.ConsoleHandler
+org.jenkinsci.plugins.jvcts.level = FINE
+
+# Console Logging
+java.util.logging.ConsoleHandler.level = FINE

plugin-test/src/test/java/org/jenkinsci/plugins/jvcts/test/ConfigurationWebTest.java

@@ -22,6 +22,7 @@
 
 public class ConfigurationWebTest {
  private static final String CHECKSTYLE_PATTERN = "descriptor.config.parserConfigs[0].pattern";
+ private static final String PATH_PREFIX = "descriptor.config.parserConfigs[0].pathPrefix";
  private static final String CODENARC_PATTERN = "descriptor.config.parserConfigs[1].pattern";
  private static final String CPD_PATTERN = "descriptor.config.parserConfigs[2].pattern";
  private static final String CPPLINT_PATTERN = "descriptor.config.parserConfigs[3].pattern";
@@ -98,6 +99,7 @@ public void testPluginConfiguration() throws InterruptedException {
   assertTrue(consoleText, consoleText.contains("stashPullRequestId: 100"));
   assertTrue(consoleText, consoleText.contains("stashRepo: a_repo"));
   assertTrue(consoleText, consoleText.contains("checkstyle: **/checkstyle-report.xml"));
+  assertTrue(consoleText, consoleText.contains("checkstyle pathPrefix: pathPrefix"));
   assertTrue(consoleText, consoleText.contains("pmd: **/pmd-report.xml"));
   assertTrue(consoleText, consoleText.contains("jslint: **/jslint-report.xml"));
   assertTrue(consoleText, consoleText.contains("csslint: **/csslint-report.xml"));
@@ -125,6 +127,7 @@ public void testPluginConfiguration() throws InterruptedException {
   webDriver.findElement(name(STASH_PROJECT)).clear();
   webDriver.findElement(name(STASH_REPO)).clear();
   webDriver.findElement(name(STASH_PULL_REQUEST_ID)).clear();
+  webDriver.findElement(name(PATH_PREFIX)).clear();
   webDriver.findElement(name(CHECKSTYLE_PATTERN)).clear();
   webDriver.findElement(name(CODENARC_PATTERN)).clear();
   webDriver.findElement(name(CPD_PATTERN)).clear();
@@ -329,6 +332,7 @@ private void enterDetails() {
   webDriver.findElement(name(STASH_PROJECT)).sendKeys("ABC");
   webDriver.findElement(name(STASH_REPO)).sendKeys("a_repo");
   webDriver.findElement(name(STASH_PULL_REQUEST_ID)).sendKeys("100");
+  webDriver.findElement(name(PATH_PREFIX)).sendKeys("pathPrefix");
   webDriver.findElement(name(CHECKSTYLE_PATTERN)).sendKeys("**/checkstyle-report.xml");
   webDriver.findElement(name(PMD_PATTERN)).sendKeys("**/pmd-report.xml");
   webDriver.findElement(name(JSLINT_PATTERN)).sendKeys("**/jslint-report.xml");

plugin/src/main/java/org/jenkinsci/plugins/jvcts/JvctsLogger.java

@@ -0,0 +1,30 @@
+package org.jenkinsci.plugins.jvcts;
+
+import hudson.model.BuildListener;
+
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+public class JvctsLogger {
+ private static Logger logger = Logger.getLogger(JvctsLogger.class.getName());
+
+ private JvctsLogger() {
+ }
+
+ public static void doLog(BuildListener listener, Level level, String string, Throwable t) {
+  listener.getLogger().println(string);
+  doLog(level, string, t);
+ }
+
+ public static void doLog(Level level, String string, Throwable t) {
+  logger.log(level, string, t);
+ }
+
+ public static void doLog(BuildListener listener, Level level, String string) {
+  doLog(listener, level, string, null);
+ }
+
+ public static void doLog(Level level, String string) {
+  doLog(level, string, null);
+ }
+}

plugin/src/main/java/org/jenkinsci/plugins/jvcts/ViolationsToStashDescriptor.java

@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.jvcts;
 
 import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_PATTERN;
+import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_PREFIX;
 import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_BASE_URL;
 import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_PASSWORD;
 import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_PROJECT;
@@ -55,6 +56,10 @@ public Publisher newInstance(StaplerRequest req, JSONObject formData) throws hud
   for (String pattern : (List<String>) formData.get(FIELD_PATTERN)) {
    config.getParserConfigs().get(i++).setPattern(pattern);
   }
+  i = 0;
+  for (String pathPrefix : (List<String>) formData.get(FIELD_PREFIX)) {
+   config.getParserConfigs().get(i++).setPathPrefix(pathPrefix);
+  }
   ViolationsToStashRecorder publisher = new ViolationsToStashRecorder();
   publisher.setConfig(config);
   return publisher;

plugin/src/main/java/org/jenkinsci/plugins/jvcts/config/ParserConfig.java

@@ -1,8 +1,15 @@
 package org.jenkinsci.plugins.jvcts.config;
 
+import static com.google.common.base.Optional.fromNullable;
+import static com.google.common.base.Strings.emptyToNull;
+import static com.google.common.base.Strings.nullToEmpty;
+
+import com.google.common.base.Optional;
+
 public class ParserConfig {
  private String pattern;
  private String parserTypeDescriptorName;
+ private String pathPrefix;
 
  public ParserConfig() {
 
@@ -28,4 +35,16 @@ public String getParserTypeDescriptorName() {
  public void setParserTypeDescriptorName(String parserTypeDescriptorName) {
   this.parserTypeDescriptorName = parserTypeDescriptorName;
  }
+
+ public void setPathPrefix(String pathPrefix) {
+  this.pathPrefix = pathPrefix;
+ }
+
+ public String getPathPrefix() {
+  return pathPrefix;
+ }
+
+ public Optional<String> getPathPrefixOpt() {
+  return fromNullable(emptyToNull(nullToEmpty(pathPrefix).trim()));
+ }
 }

plugin/src/main/java/org/jenkinsci/plugins/jvcts/config/ViolationsToStashConfigHelper.java

@@ -22,6 +22,7 @@ public class ViolationsToStashConfigHelper {
  public static final String SIMIAN_NAME = "simian";
  public static final String STYLECOP_NAME = "stylecop";
  public static final String FIELD_PATTERN = "pattern";
+ public static final String FIELD_PREFIX = "pathPrefix";
  public static final String FIELD_STASH_PULL_REQUEST_ID = "stashPullRequestId";
  public static final String FIELD_STASH_REPO = "stashRepo";
  public static final String FIELD_STASH_PROJECT = "stashProject";

plugin/src/main/java/org/jenkinsci/plugins/jvcts/perform/FullBuildModelWrapper.java

@@ -3,7 +3,10 @@
 import static com.google.common.base.Strings.nullToEmpty;
 import static com.google.common.collect.Maps.newHashMap;
 import static hudson.plugins.violations.TypeDescriptor.TYPES;
+import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.SEVERE;
+import static org.jenkinsci.plugins.jvcts.JvctsLogger.doLog;
+import hudson.model.BuildListener;
 import hudson.plugins.violations.TypeDescriptor;
 import hudson.plugins.violations.model.FullBuildModel;
 import hudson.plugins.violations.model.FullFileModel;
@@ -16,7 +19,6 @@
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.TreeSet;
-import java.util.logging.Logger;
 
 import org.apache.tools.ant.Project;
 import org.apache.tools.ant.types.FileSet;
@@ -25,50 +27,88 @@
 
 public class FullBuildModelWrapper {
 
- private static final Logger logger = Logger.getLogger(FullBuildModelWrapper.class.getName());
- private final FullBuildModel model = new FullBuildModel();
+ private final Map<String, FullBuildModel> models = newHashMap();
+ private final ViolationsToStashConfig config;
+ private final BuildListener listener;
 
- public FullBuildModelWrapper(ViolationsToStashConfig config, File workspace) {
+ public FullBuildModelWrapper(ViolationsToStashConfig config, File workspace, BuildListener listener) {
+  this.config = config;
+  this.listener = listener;
   for (ParserConfig parserConfig : config.getParserConfigs()) {
-   for (String fileName : findFilesFromPattern(workspace, parserConfig.getPattern())) {
-    String[] sourcePaths = {};
-    TypeDescriptor type = TYPES.get(parserConfig.getParserTypeDescriptorName());
-    try {
-     type.createParser().parse(model, workspace, fileName, sourcePaths);
-    } catch (IOException e) {
-     logger.log(SEVERE,
-       "Error while parsing \"" + fileName + "\" for type " + parserConfig.getParserTypeDescriptorName(), e);
+   for (String pattern : parserConfig.getPattern().split(",")) {
+    for (String fileName : findFilesFromPattern(workspace, pattern)) {
+     String[] sourcePaths = {};
+     TypeDescriptor type = TYPES.get(parserConfig.getParserTypeDescriptorName());
+     try {
+      if (!models.containsKey(parserConfig.getParserTypeDescriptorName())) {
+       models.put(parserConfig.getParserTypeDescriptorName(), new FullBuildModel());
+      }
+      type.createParser().parse(models.get(parserConfig.getParserTypeDescriptorName()), workspace, fileName,
+        sourcePaths);
+     } catch (IOException e) {
+      doLog(SEVERE, "Error while parsing \"" + fileName + "\" for type " + parserConfig.getParserTypeDescriptorName(),
+        e);
+     }
     }
    }
   }
  }
 
  public Map<String, List<Violation>> getViolationsPerFile() {
   Map<String, List<Violation>> violationsPerFile = newHashMap();
-  for (String fileModel : model.getFileModelMap().keySet()) {
-   FullFileModel fullFileModel = model.getFileModel(fileModel);
-   String sourceFile = null;
-   if (fullFileModel.getSourceFile() != null) {
-    sourceFile = fullFileModel.getSourceFile().getAbsolutePath();
-   } else if (model.getFileModel(fileModel).getDisplayName() != null) {
-    sourceFile = model.getFileModel(fileModel).getDisplayName();
-   }
-   if (sourceFile == null) {
-    logger.log(SEVERE, "Could not determine source file from: " + fileModel);
-   }
-   TreeMap<String, TreeSet<Violation>> typeMap = model.getFileModel(fileModel).getTypeMap();
-   for (String type : typeMap.keySet()) {
-    for (Violation violation : typeMap.get(type)) {
-     if (!violationsPerFile.containsKey(sourceFile)) {
-      violationsPerFile.put(sourceFile, new ArrayList<Violation>());
+  for (ParserConfig parserConfig : config.getParserConfigs()) {
+   String typeDescriptorName = parserConfig.getParserTypeDescriptorName();
+   if (models.containsKey(typeDescriptorName)) {
+    for (String fileModel : models.get(typeDescriptorName).getFileModelMap().keySet()) {
+     String sourceFile = determineSourcePath(fileModel, parserConfig);
+     if (sourceFile == null) {
+      doLog(listener, SEVERE, "Could not determine source file from: " + fileModel);
+      continue;
+     }
+     TreeMap<String, TreeSet<Violation>> typeMap = models.get(typeDescriptorName).getFileModel(fileModel).getTypeMap();
+     for (String type : typeMap.keySet()) {
+      for (Violation violation : typeMap.get(type)) {
+       if (!violationsPerFile.containsKey(sourceFile)) {
+        violationsPerFile.put(sourceFile, new ArrayList<Violation>());
+       }
+       violationsPerFile.get(sourceFile).add(violation);
+      }
      }
-     violationsPerFile.get(sourceFile).add(violation);
     }
    }
   }
   return violationsPerFile;
  }
 
+ private String determineSourcePath(String fileModel, ParserConfig parserConfig) {
+  doLog(FINE, "Determining source path for " + fileModel);
+  FullBuildModel model = models.get(parserConfig.getParserTypeDescriptorName());
+  FullFileModel fullFileModel = model.getFileModel(fileModel);
+  if (fullFileModel.getSourceFile() != null) {
+   if (fullFileModel.getSourceFile().exists()) {
+    return fullFileModel.getSourceFile().getAbsolutePath();
+   } else {
+    doLog(FINE, "Not found: " + fullFileModel.getSourceFile().getAbsolutePath());
+   }
+  }
+  File withDisplayName = new File(fullFileModel.getDisplayName());
+  if (withDisplayName.exists()) {
+   return withDisplayName.getAbsolutePath();
+  } else {
+   doLog(FINE, "Not found: " + withDisplayName.getAbsolutePath());
+  }
+  if (parserConfig.getPathPrefixOpt().isPresent()) {
+   File withPrefix = new File(parserConfig.getPathPrefixOpt().get() + fullFileModel.getDisplayName());
+   if (withPrefix.exists()) {
+    return withPrefix.getAbsolutePath();
+   } else {
+    doLog(FINE, "Not found: " + withPrefix.getAbsolutePath());
+   }
+  }
+  doLog(FINE, "Using: " + fullFileModel.getDisplayName());
+  return fullFileModel.getDisplayName();
+ }
+
  /**
   * Include matching pattern within workspace.
   *

plugin/src/main/java/org/jenkinsci/plugins/jvcts/perform/JvctsPerformer.java

@@ -4,14 +4,13 @@
 import static com.google.common.base.Strings.emptyToNull;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.collect.Lists.newArrayList;
+import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.SEVERE;
+import static org.jenkinsci.plugins.jvcts.JvctsLogger.doLog;
 import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_BASE_URL;
 import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_PROJECT;
 import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_PULL_REQUEST_ID;
 import static org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfigHelper.FIELD_STASH_REPO;
-import static org.jenkinsci.plugins.jvcts.stash.JvctsStashClient.commentPullRequest;
-import static org.jenkinsci.plugins.jvcts.stash.JvctsStashClient.getChangedFileInPullRequest;
-import static org.jenkinsci.plugins.jvcts.stash.JvctsStashClient.removeCommentsFromPullRequest;
 import hudson.EnvVars;
 import hudson.model.BuildListener;
 import hudson.model.AbstractBuild;
@@ -23,13 +22,14 @@
 import java.util.Map;
 import java.util.logging.Logger;
 
+import org.jenkinsci.plugins.jvcts.JvctsLogger;
 import org.jenkinsci.plugins.jvcts.config.ParserConfig;
 import org.jenkinsci.plugins.jvcts.config.ViolationsToStashConfig;
+import org.jenkinsci.plugins.jvcts.stash.JvctsStashClient;
 
 import com.google.common.annotations.VisibleForTesting;
 
 public class JvctsPerformer {
- public static final String WORKSPACE = "WORKSPACE";
  private static final Logger logger = Logger.getLogger(JvctsPerformer.class.getName());
 
  public static void jvctsPerform(ViolationsToStashConfig config, AbstractBuild<?, ?> build, BuildListener listener) {
@@ -44,29 +44,32 @@ public static void jvctsPerform(ViolationsToStashConfig config, AbstractBuild<?,
    listener.getLogger().println("Running Jenkins Violation Comments To Stash");
    listener.getLogger().println("Will comment " + config.getStashPullRequestId());
 
-   File workspace = new File(env.expand("$" + WORKSPACE));
-   doPerform(config, workspace);
+   File workspace = build.getRootDir();
+   doPerform(config, workspace, listener);
   } catch (Exception e) {
    logger.log(SEVERE, "", e);
    return;
   }
  }
 
  @VisibleForTesting
- static void doPerform(ViolationsToStashConfig config, File workspace) throws MalformedURLException {
-  commentStash(new FullBuildModelWrapper(config, workspace).getViolationsPerFile(), config);
+ static void doPerform(ViolationsToStashConfig config, File workspace, BuildListener listener)
+   throws MalformedURLException {
+  commentStash(new FullBuildModelWrapper(config, workspace, listener).getViolationsPerFile(), config, listener);
  }
 
- private static void commentStash(Map<String, List<Violation>> violationsPerFile, ViolationsToStashConfig config)
-   throws MalformedURLException {
-  for (String changedFileInStash : getChangedFileInPullRequest(config)) {
+ private static void commentStash(Map<String, List<Violation>> violationsPerFile, ViolationsToStashConfig config,
+   BuildListener listener) throws MalformedURLException {
+  JvctsStashClient jvctsStashClient = new JvctsStashClient(config, listener);
+  for (String changedFileInStash : jvctsStashClient.getChangedFileInPullRequest()) {
    /**
     * Should always use filename reported by Stash, then we know Stash will
     * recognize it.
     */
-   removeCommentsFromPullRequest(config, changedFileInStash);
-   for (Violation violation : getViolationsForFile(violationsPerFile, changedFileInStash)) {
-    commentPullRequest(config, changedFileInStash, violation.getLine(), constructCommentMessage(violation));
+   doLog(listener, FINE, "Changed file in pull request: \"" + changedFileInStash + "\"");
+   jvctsStashClient.removeCommentsFromPullRequest(changedFileInStash);
+   for (Violation violation : getViolationsForFile(violationsPerFile, changedFileInStash, listener)) {
+    jvctsStashClient.commentPullRequest(changedFileInStash, violation.getLine(), constructCommentMessage(violation));
    }
   }
  }
@@ -75,12 +78,19 @@ private static void commentStash(Map<String, List<Violation>> violationsPerFile,
   * Get list of violations that has files ending with changed file in Stash.
   * Violation instances may have absolute or relative paths, we can not trust
   * that.
+  *
+  * @param listener
   */
  private static List<Violation> getViolationsForFile(Map<String, List<Violation>> violationsPerFile,
-   String changedFileInStash) {
-  for (String s : violationsPerFile.keySet()) {
-   if (s.endsWith(changedFileInStash)) {
-    return violationsPerFile.get(s);
+   String changedFileInStash, BuildListener listener) {
+  for (String reportedFile : violationsPerFile.keySet()) {
+   if (reportedFile.endsWith(changedFileInStash)) {
+    JvctsLogger.doLog(listener, FINE, "Changed file and reported file matches. Stash: \"" + changedFileInStash
+      + "\" Reported: \"" + reportedFile + "\"");
+    return violationsPerFile.get(reportedFile);
+   } else {
+    doLog(listener, FINE, "Changed file and reported file not matching. Stash: \"" + changedFileInStash
+      + "\" Reported: \"" + reportedFile + "\"");
    }
   }
   return newArrayList();
@@ -112,6 +122,7 @@ private static ViolationsToStashConfig expand(ViolationsToStashConfig config, En
    ParserConfig p = new ParserConfig();
    p.setParserTypeDescriptorName(environment.expand(parserConfig.getParserTypeDescriptorName()));
    p.setPattern(environment.expand(parserConfig.getPattern()));
+   p.setPathPrefix(environment.expand(parserConfig.getPathPrefixOpt().or("")));
    expanded.getParserConfigs().add(p);
   }
   return expanded;
@@ -127,6 +138,10 @@ private static void logConfiguration(ViolationsToStashConfig config, AbstractBui
   listener.getLogger().println(FIELD_STASH_REPO + ": " + config.getStashRepo());
   for (ParserConfig parserConfig : config.getParserConfigs()) {
    listener.getLogger().println(parserConfig.getParserTypeDescriptorName() + ": " + parserConfig.getPattern());
+   if (parserConfig.getPathPrefixOpt().isPresent()) {
+    listener.getLogger().println(
+      parserConfig.getParserTypeDescriptorName() + " pathPrefix: " + parserConfig.getPathPrefix());
+   }
   }
  }
 }