Refactoring, Unit test and localization

puhko · Aug 21, 2011 · 80ab9a5 · 80ab9a5
1 parent 058ab9c
commit 80ab9a5
Show file tree

Hide file tree

Showing 13 changed files with 320 additions and 107 deletions.
diff --git a/.gitignore b/.gitignore
@@ -0,0 +1,6 @@
+work/
+target/
+.project
+.classpath
+.settings/
+*.iml
diff --git a/pom.xml b/pom.xml
@@ -63,9 +63,22 @@
       <optional>true</optional>
     </dependency>
     <dependency>
-      <groupId>org.codehaus.groovy.maven.runtime</groupId>
-      <artifactId>gmaven-runtime-1.6</artifactId>
-      <version>1.0</version>
+      <groupId>org.jenkins-ci.plugins</groupId>
+      <artifactId>token-macro</artifactId>
+      <version>1.4</version>
+      <optional>true</optional>
+    </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci.plugins</groupId>
+      <artifactId>email-ext</artifactId>
+      <version>2.14</version>
+      <optional>true</optional>
+    </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>1.8.5</version>
+      <scope>test</scope>
     </dependency>
   </dependencies>
 
@@ -84,23 +97,4 @@ artifacts that we need -->
       <url>http://maven.glassfish.org/content/groups/public/</url>
     </pluginRepository>
   </pluginRepositories>
-  <build>
-    <plugins>
-      <plugin>
-        <groupId>org.codehaus.groovy.maven</groupId>
-        <artifactId>gmaven-plugin</artifactId>
-        <version>1.0</version>
-        <executions>
-          <execution>
-            <goals>
-              <goal>generateStubs</goal>
-              <goal>compile</goal>
-              <goal>generateTestStubs</goal>
-              <goal>testCompile</goal>
-            </goals>
-          </execution>
-        </executions>
-      </plugin>
-    </plugins>
-  </build>
 </project>
diff --git a/src/main/java/org/jenkins/plugins/all_changes/AllChangesAction.java b/src/main/java/org/jenkins/plugins/all_changes/AllChangesAction.java
@@ -26,13 +26,15 @@
 
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.HashMultimap;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
 import hudson.model.Action;
 import hudson.scm.ChangeLogSet;
 
+import java.util.List;
 import java.util.Set;
 
 /**
@@ -44,6 +46,8 @@
 public class AllChangesAction implements Action {
 
     private AbstractProject<?, ?> project;
+    transient
+    List<ChangesAggregator> aggregators;
 
     AllChangesAction(AbstractProject<?, ?> project) {
         this.project = project;
@@ -67,7 +71,7 @@ public String getUrlName() {
      * @param build
      * @return
      */
-    public static Multimap<ChangeLogSet.Entry, AbstractBuild> getAllChanges(AbstractBuild build) {
+    public Multimap<ChangeLogSet.Entry, AbstractBuild> getAllChanges(AbstractBuild build) {
         Set<AbstractBuild> builds = getContributingBuilds(build);
         Multimap<String, ChangeLogSet.Entry> changes = ArrayListMultimap.create();
         for (AbstractBuild changedBuild : builds) {
@@ -91,17 +95,20 @@ public static Multimap<ChangeLogSet.Entry, AbstractBuild> getAllChanges(Abstract
      *
      * @return all changes which contribute to the given build
      */
-    public static Set<AbstractBuild> getContributingBuilds(AbstractBuild build) {
+    public Set<AbstractBuild> getContributingBuilds(AbstractBuild build) {
+        if (aggregators == null) {
+            aggregators = ImmutableList.copyOf(ChangesAggregator.all());
+        }
         Set<AbstractBuild> builds = Sets.newHashSet();
         builds.add(build);
         int size = 0;
         // Saturate the build Set
         do {
             size = builds.size();
             Set<AbstractBuild> newBuilds = Sets.newHashSet();
-            for (ChangesAggregator aggregator : ChangesAggregator.all()) {
-                for (AbstractBuild abstractBuild : builds) {
-                    newBuilds.addAll(aggregator.aggregateBuildsWithChanges(build));
+            for (ChangesAggregator aggregator : aggregators) {
+                for (AbstractBuild depBuild : builds) {
+                    newBuilds.addAll(aggregator.aggregateBuildsWithChanges(depBuild));
                 }
             }
             builds.addAll(newBuilds);

diff --git a/src/main/java/org/jenkins/plugins/all_changes/ChangesAggregator.java b/src/main/java/org/jenkins/plugins/all_changes/ChangesAggregator.java
@@ -29,10 +29,10 @@
 import hudson.model.AbstractBuild;
 import jenkins.model.Jenkins;
 
-import java.util.List;
+import java.util.Collection;
 
 public abstract class ChangesAggregator implements ExtensionPoint {
-    public abstract List<AbstractBuild> aggregateBuildsWithChanges(AbstractBuild build);
+    public abstract Collection<AbstractBuild> aggregateBuildsWithChanges(AbstractBuild build);
 
     public static ExtensionList<ChangesAggregator> all() {
         return Jenkins.getInstance().getExtensionList(ChangesAggregator.class);

diff --git a/src/main/java/org/jenkins/plugins/all_changes/DependencyChangesAggregator.java b/src/main/java/org/jenkins/plugins/all_changes/DependencyChangesAggregator.java
@@ -29,7 +29,7 @@
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
 
-import java.util.List;
+import java.util.Collection;
 import java.util.Map;
 
 /**
@@ -38,7 +38,7 @@
 @Extension
 public class DependencyChangesAggregator extends ChangesAggregator {
     @Override
-    public List<AbstractBuild> aggregateBuildsWithChanges(AbstractBuild build) {
+    public Collection<AbstractBuild> aggregateBuildsWithChanges(AbstractBuild build) {
         ImmutableList.Builder<AbstractBuild> builder = ImmutableList.<AbstractBuild>builder();
         Map<AbstractProject, AbstractBuild.DependencyChange> depChanges = build.getDependencyChanges((AbstractBuild) build.getPreviousBuild());
         for (AbstractBuild.DependencyChange depChange : depChanges.values()) {

diff --git a/src/main/java/org/jenkins/plugins/all_changes/SubProjectChangesAggregator.java b/src/main/java/org/jenkins/plugins/all_changes/SubProjectChangesAggregator.java
@@ -24,6 +24,8 @@
 
 package org.jenkins.plugins.all_changes;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import hudson.Extension;
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
@@ -34,51 +36,69 @@
 import hudson.tasks.Builder;
 import hudson.util.RunList;
 
-import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
+import java.util.Set;
 
 /**
  * @author wolfs
  */
 @Extension
 public class SubProjectChangesAggregator extends ChangesAggregator {
     @Override
-    public List<AbstractBuild> aggregateBuildsWithChanges(AbstractBuild build) {
+    public Collection<AbstractBuild> aggregateBuildsWithChanges(AbstractBuild build) {
         AbstractProject project = build.getProject();
-        List<AbstractProject<?, ?>> subProjects = new ArrayList<AbstractProject<?, ?>>();
+        Set<AbstractProject<?, ?>> subProjects = Sets.newHashSet();
+        Set<AbstractBuild> builds = Sets.newHashSet();
         if (project instanceof FreeStyleProject) {
-            FreeStyleProject proj = (FreeStyleProject) project;
-            List<Builder> builders = proj.getBuilders();
-            for (Builder builder : builders) {
-                if (builder instanceof TriggerBuilder) {
-                    TriggerBuilder tBuilder = (TriggerBuilder) builder;
-                    for (BlockableBuildTriggerConfig config : tBuilder.getConfigs()) {
-                        for (AbstractProject<?, ?> abstractProject : config.getProjectList()) {
-                            if (config.getBlock() != null) {
-                                subProjects.add(abstractProject);
-                            }
-                        }
-                    }
-                }
-            }
+            subProjects.addAll(getSubProjects((FreeStyleProject) project));
+            builds.addAll(getTriggeredBuilds(build, subProjects));
         }
-        ArrayList<AbstractBuild> builds = new ArrayList<AbstractBuild>();
+
+        return builds;
+    }
+
+    private List<AbstractBuild> getTriggeredBuilds(AbstractBuild build, Collection<AbstractProject<?, ?>> subProjects) {
+        List<AbstractBuild> builds = Lists.newArrayList();
         for (AbstractProject<?, ?> subProject : subProjects) {
             RunList<? extends AbstractBuild<?, ?>> subBuildsDuringBuild = subProject.getBuilds().byTimestamp(build.getTimeInMillis(), build.getTimeInMillis() + build.getDuration());
-            for (AbstractBuild<?, ?> subBuild : subBuildsDuringBuild) {
-                List<Cause.UpstreamCause> upstreamCauses = new ArrayList<Cause.UpstreamCause>();
-                List<Cause> causes = subBuild.getCauses();
-                for (Cause cause : causes) {
-                    if (cause instanceof Cause.UpstreamCause) {
-                        Cause.UpstreamCause upstreamCause = (Cause.UpstreamCause) cause;
-                        if (upstreamCause.pointsTo(build)) {
-                            builds.add(subBuild);
+            for (AbstractBuild<?, ?> candidate : subBuildsDuringBuild) {
+                if (isSubBuild(build, candidate)) {
+                    builds.add(candidate);
+                }
+            }
+        }
+        return builds;
+    }
+
+    private List<AbstractProject<?, ?>> getSubProjects(FreeStyleProject project) {
+        List<AbstractProject<?, ?>> subProjects = Lists.newArrayList();
+        List<Builder> builders = project.getBuilders();
+        for (Builder builder : builders) {
+            if (builder instanceof TriggerBuilder) {
+                TriggerBuilder tBuilder = (TriggerBuilder) builder;
+                for (BlockableBuildTriggerConfig config : tBuilder.getConfigs()) {
+                    for (AbstractProject<?, ?> abstractProject : config.getProjectList()) {
+                        if (config.getBlock() != null) {
+                            subProjects.add(abstractProject);
                         }
                     }
                 }
             }
         }
+        return subProjects;
+    }
 
-        return builds;
+    private boolean isSubBuild(AbstractBuild build, AbstractBuild<?, ?> subBuild) {
+        List<Cause> causes = subBuild.getCauses();
+        for (Cause cause : causes) {
+            if (cause instanceof Cause.UpstreamCause) {
+                Cause.UpstreamCause upstreamCause = (Cause.UpstreamCause) cause;
+                if (upstreamCause.pointsTo(build)) {
+                    return true;
+                }
+            }
+        }
+        return false;
     }
 }
diff --git a/src/main/resources/index.jelly b/src/main/resources/index.jelly
@@ -26,5 +26,5 @@
   This view is used to render the installed plugins page.
 -->
 <div>
-  This plugin shows all changes (also from dependent projects, subprojects, ...) for a project
+  This plugin shows all changes (also from dependent projects, sub-projects, ...) for a project
 </div>
diff --git a/src/main/resources/org/jenkins/plugins/all_changes/AllChangesAction/index.groovy b/src/main/resources/org/jenkins/plugins/all_changes/AllChangesAction/index.groovy
@@ -30,70 +30,80 @@ import hudson.model.AbstractBuild
 import hudson.model.AbstractBuild.DependencyChange
 import hudson.scm.ChangeLogSet
 import java.text.DateFormat
-import org.apache.commons.jelly.XMLOutput
-import org.dom4j.io.SAXContentHandler
 import org.jvnet.localizer.LocaleProvider
 
 f = namespace(lib.FormTagLib)
 l = namespace(lib.LayoutTagLib)
 t = namespace("/lib/hudson")
 st = namespace("jelly:stapler")
 
-private def wrapOutput(Closure viewInstructions) {
-  def sc = new SAXContentHandler()
-  def old = setOutput(new XMLOutput(sc))
-  viewInstructions();
-  setOutput(old);
-  return sc
-}
-
-l.layout(title: _("All Changes")) {
+l.layout(title: _("all.changes.title", my.project.name)) {
   st.include(page: "sidepanel.jelly", it: my.project)
   l.main_panel() {
     def from = request.getParameter('from')
     def to = request.getParameter('to')
 
     h1(_("All Changes"))
     def builds = Functions.filter(my.project.buildsAsMap, from, to).values()
-    for (build in builds) {
-      Multimap<ChangeLogSet.Entry, AbstractBuild> changes = my.getAllChanges(build);
-      if (changes.empty) {
-        continue
-      }
-      h2() {
-        a(href: "${my.project.absoluteUrl}/${build.number}/changes",
-                "${build.displayName}  (${DateFormat.getDateTimeInstance(DateFormat.MEDIUM, DateFormat.MEDIUM, LocaleProvider.locale).format(build.timestamp.time)})")
-      }
-      ol() {
-        for (entry in changes.keySet()) {
-          li() {
-            showChangeSet(entry)
-            boolean firstDrawn = false
-            for (AbstractBuild b in changes.get(entry)) {
-              if (b != build) {
-                if (!firstDrawn) {
-                  text(" (")
-                  firstDrawn = true
-                }
-                else {
-                  text(", ")
-                }
-                a(href: "${rootURL}/${b.project.url}") {text(b.project.displayName)}
-                st.nbsp()
-                a(href: "${rootURL}/${b.url}") {
-                  text(b.displayName)
-                }
-              }
-            }
-            if (firstDrawn) {
-              text(")")
-            }
+    if (builds.empty) {
+      text(_("No builds."))
+    } else {
+      showChanges(builds)
+    }
+  }
+}
 
-          }
+private showChanges(Collection<AbstractBuild> builds) {
+  boolean hadChanges = false;
+  for (AbstractBuild build in builds) {
+    Multimap<ChangeLogSet.Entry, AbstractBuild> changes = my.getAllChanges(build);
+    if (changes.empty) {
+      continue
+    }
+    hadChanges = true
+    h2() {
+      a(href: "${my.project.absoluteUrl}/${build.number}/changes",
+              """${build.displayName}  (${
+                DateFormat.getDateTimeInstance(
+                      DateFormat.MEDIUM,
+                      DateFormat.MEDIUM,
+                      LocaleProvider.locale).format(build.timestamp.time)})""")
+    }
+    ol() {
+      for (entry in changes.keySet()) {
+        li() {
+          showEntry(entry, build, changes.get(entry))
         }
       }
     }
   }
+  if (!hadChanges) {
+    text(_("No changes in any of the builds."))
+  }
+}
+
+private def showEntry(entry, AbstractBuild build, Collection<AbstractBuild> builds) {
+  showChangeSet(entry)
+  boolean firstDrawn = false
+  for (AbstractBuild b in builds) {
+    if (b != build) {
+      if (!firstDrawn) {
+        text(" (")
+        firstDrawn = true
+      }
+      else {
+        text(", ")
+      }
+      a(href: "${rootURL}/${b.project.url}") {text(b.project.displayName)}
+      st.nbsp()
+      a(href: "${rootURL}/${b.url}") {
+        text(b.displayName)
+      }
+    }
+  }
+  if (firstDrawn) {
+    text(")")
+  }
 }
 
 private def showChangeSet(ChangeLogSet.Entry c) {