[JENKINS-21419] Can't open sloccount table portlet

- The code used System.getProperty("file.separator") for detection of unix/windows directory separator. This technique may be dangerous now, because the SLOCCount report (output from sloccount utility) may be generated on slave (unix/windows), the same file then transfered to master (windows/unix) and lazy loaded/parsed on master. So both unix and windows separators should be considered during the parsing. - Directory separator member variables removed from SloccountReport, System.getProperty("file.separator") is no longer used, because it lost its sense and may return not suitable value. - Method extractFolder() tests unix separator first and then the windows one. If there is no match, empty string will be returned. -1/not found returned from lastIndexOf() will never cause StringIndexOutOfBoundsException. - Method updateRootFolderPath() updated to try unix separator first and then the windows one. - Unit/integration tests for the changes above created. - Tested only on Linux, I don't have Windows machine available. - This commit is partially inspired by André Wegmüller's two years old pull request. The code is now too incompatible, sorry. - If no file is matching the input pattern, the publisher will output a warning to the build log.
jenkinsci · Jan 18, 2014 · 2174a03 · 2174a03
1 parent 0504b70
commit 2174a03
Show file tree

Hide file tree

Showing 3 changed files with 167 additions and 24 deletions.
diff --git a/src/main/java/hudson/plugins/sloccount/SloccountPublisher.java b/src/main/java/hudson/plugins/sloccount/SloccountPublisher.java
@@ -79,12 +79,14 @@ public boolean perform(AbstractBuild<?,?> build, Launcher launcher, BuildListene
             return false;
         }
 
+        if (report.getSourceFiles().size() == 0) {
+            logger.format("[SLOCCount] No file is matching the input pattern: %s\n",
+                    getRealPattern());
+        }
+
         SloccountResult result = new SloccountResult(report.getStatistics(),
                 getRealEncoding(), null, build);
-
-        SloccountBuildAction buildAction = new SloccountBuildAction(build, result);
-
-        build.addAction(buildAction);
+        build.addAction(new SloccountBuildAction(build, result));
 
         try{
             copyFilesToBuildDirectory(report.getSourceFiles(),

diff --git a/src/main/java/hudson/plugins/sloccount/model/SloccountReport.java b/src/main/java/hudson/plugins/sloccount/model/SloccountReport.java
@@ -19,22 +19,9 @@ public class SloccountReport extends FileContainer implements SloccountReportInt
 
     /** The longest folder path common to all folders. */
     private String[] rootFolderPath = null;
-    private transient String fileSeparator;
-    private transient String regex_fileSeparator;
 
     public SloccountReport(){
-
         super();
-
-        this.fileSeparator = System.getProperty("file.separator");
-
-        if(this.fileSeparator.equals("\\")){
-            // Windows environment -> escape the backslash for regex
-            this.regex_fileSeparator = "\\\\";
-        }else{ 
-            // Unix environment -> use as given
-            this.regex_fileSeparator = this.fileSeparator;            
-        }
     }
 
     public SloccountReport(SloccountReport old, FileFilter filter){
@@ -47,8 +34,7 @@ public SloccountReport(SloccountReport old, FileFilter filter){
     }
 
     public void add(String filePath, String languageName, int lineCount){
-
-        String folderPath = this.extractFolder(filePath);
+        String folderPath = extractFolder(filePath);
 
         File file = new File(filePath, languageName, lineCount);
         this.addFile(file);
@@ -68,9 +54,41 @@ public void add(String filePath, String languageName, int lineCount){
         language.addFile(file);
     }
 
-    private String extractFolder(String filePath){
-        int index = filePath.lastIndexOf(this.fileSeparator);
-        return filePath.substring(0, index);
+    /**
+     * Extract directory part of a path. The method searches the directory
+     * separator from right in the following order: unix '/', windows '\'.
+     * 
+     * Examples of input and output:
+     * (empty string) - (empty string)
+     * file.java - (empty string)
+     * /test/file.java - /test
+     * /cygdrive/c/test/file.java - /cygdrive/c/test
+     * c:\test\file.java - c:\test
+     * /test/ - /test
+     * /test - (empty string) ... is it file or directory?
+     * 
+     * @param filePath
+     *            the path containing folders and file name
+     * @return the path without the file name; if no separator is found in
+     *            the input path an empty string will be returned
+     */
+    public static String extractFolder(String filePath){
+        // Try Unix separator
+        int index = filePath.lastIndexOf("/");
+
+        if(index != -1) {
+            return filePath.substring(0, index);
+        }
+
+        // Try Windows separator
+        index = filePath.lastIndexOf("\\");
+
+        if(index != -1) {
+            return filePath.substring(0, index);
+        }
+
+        // No separator found, probably only a file name
+        return "";
     }
 
     public Folder getFolder(String name){
@@ -133,11 +151,16 @@ public String getRootFolder(){
     }
 
     private void updateRootFolderPath(String newFolderName){
-        String[] newFolderPath = newFolderName.split(this.regex_fileSeparator);
+        // Unix directory separator
+        String[] newFolderPath = newFolderName.split("/");
+
+        if(newFolderPath.length == 1){
+            // Windows directory separator '\'
+            newFolderPath = newFolderName.split("\\\\");
+        }
 
         if(this.rootFolderPath == null){
             this.rootFolderPath = newFolderPath;
-
         }else{
             for(int i = 0; i < this.rootFolderPath.length && i < newFolderPath.length; i ++){
                 if(!this.rootFolderPath[i].equals(newFolderPath[i])){

diff --git a/src/test/java/hudson/plugins/sloccount/model/SloccountReportTest.java b/src/test/java/hudson/plugins/sloccount/model/SloccountReportTest.java
@@ -0,0 +1,118 @@
+package hudson.plugins.sloccount.model;
+
+import junit.framework.Assert;
+import org.junit.Test;
+
+/**
+ * Test case for Unix and Windows directory separators used in SloccountReport.
+ * 
+ * @author Michal Turek
+ */
+public class SloccountReportTest {
+    @Test
+    public void testExtractFolder() {
+        Assert.assertEquals("/test",
+                SloccountReport.extractFolder("/test/file.java"));
+
+        Assert.assertEquals("/cygdrive/c/test",
+                SloccountReport.extractFolder("/cygdrive/c/test/file.java"));
+
+        Assert.assertEquals("c:/test",
+                SloccountReport.extractFolder("c:/test/file.java"));
+
+        Assert.assertEquals("c:\\test",
+                SloccountReport.extractFolder("c:\\test\\file.java"));
+
+        Assert.assertEquals("",
+                SloccountReport.extractFolder("file.java"));
+
+        Assert.assertEquals("",
+                SloccountReport.extractFolder(""));
+
+        Assert.assertEquals("test",
+                SloccountReport.extractFolder("test/file.java"));
+
+        Assert.assertEquals("test",
+                SloccountReport.extractFolder("test\\file.java"));
+
+        // It searches the separator from right
+        Assert.assertEquals("/test",
+                SloccountReport.extractFolder("/test/"));
+
+        // It searches the separator from right
+        Assert.assertEquals("",
+                SloccountReport.extractFolder("/test"));
+    }
+
+    @Test
+    public void testFileSeparator_Linux() {
+        SloccountReport report = new SloccountReport();
+        Assert.assertEquals("", report.getRootFolder());
+
+        report.add("/foo/bar/fubar/test.java", "java", 42);
+        Assert.assertEquals("/foo/bar/fubar", report.getRootFolder());
+
+        report.add("/foo/bar/fubar/test.java", "java", 42);
+        Assert.assertEquals("/foo/bar/fubar", report.getRootFolder());
+
+        report.add("/foo/bar/fubar/dir/test.java", "java", 42);
+        Assert.assertEquals("/foo/bar/fubar", report.getRootFolder());
+
+        report.add("/foo/bar/fubar/test.java", "java", 42);
+        Assert.assertEquals("/foo/bar/fubar", report.getRootFolder());
+
+        report.add("/foo/bar/test.java", "java", 42);
+        Assert.assertEquals("/foo/bar", report.getRootFolder());
+
+        report.add("/foo/bar/dir/test.java", "java", 42);
+        Assert.assertEquals("/foo/bar", report.getRootFolder());
+    }
+
+    @Test
+    public void testFileSeparator_WindowsSlash() {
+        SloccountReport report = new SloccountReport();
+        Assert.assertEquals("", report.getRootFolder());
+
+        report.add("C:/foo/bar/fubar/test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar/fubar", report.getRootFolder());
+
+        report.add("C:/foo/bar/fubar/test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar/fubar", report.getRootFolder());
+
+        report.add("C:/foo/bar/fubar/dir/test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar/fubar", report.getRootFolder());
+
+        report.add("C:/foo/bar/fubar/test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar/fubar", report.getRootFolder());
+
+        report.add("C:/foo/bar/test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar", report.getRootFolder());
+
+        report.add("C:/foo/bar/dir/test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar", report.getRootFolder());
+    }
+
+    @Test
+    public void testFileSeparator_WindowsBackSlash() {
+        SloccountReport report = new SloccountReport();
+        Assert.assertEquals("", report.getRootFolder());
+
+        report.add("C:\\foo\\bar\\fubar\\test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar/fubar", report.getRootFolder());
+
+        report.add("C:\\foo\\bar\\fubar\\test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar/fubar", report.getRootFolder());
+
+        report.add("C:\\foo\\bar\\fubar\\dir\\test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar/fubar", report.getRootFolder());
+
+        report.add("C:\\foo\\bar\\fubar\\test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar/fubar", report.getRootFolder());
+
+        report.add("C:\\foo\\bar\\test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar", report.getRootFolder());
+
+        report.add("C:\\foo\\bar\\dir\\test.java", "java", 42);
+        Assert.assertEquals("C:/foo/bar", report.getRootFolder());
+    }
+}