[JENKINS-17680] Upgrade is applied also to MatrixProject.

jenkinsci · Aug 3, 2013 · b42c215 · b42c215
1 parent 854deb1
commit b42c215
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 13 deletions.
diff --git a/src/main/java/hudson/plugins/copyartifact/CopyArtifact.java b/src/main/java/hudson/plugins/copyartifact/CopyArtifact.java
@@ -145,6 +145,16 @@ private static synchronized void setUpgradeNeeded() {
         }
     }
 
+    // get all CopyArtifacts configured to AbstractProject. This works both for Project and MatrixProject.
+    private static List<CopyArtifact> getCopiers(AbstractProject<?,?> project) throws IOException {
+        DescribableList<Builder,Descriptor<Builder>> list =
+                project instanceof Project ? ((Project<?,?>)project).getBuildersList()
+                  : (project instanceof MatrixProject ?
+                      ((MatrixProject)project).getBuildersList() : null);
+        if (list == null) return Collections.emptyList();
+        return list.getAll(CopyArtifact.class);
+    }
+
     @Initializer(after=InitMilestone.JOB_LOADED)
     public static void upgradeCopyArtifact() {
         if (!upgradeNeeded) {
@@ -153,15 +163,19 @@ public static void upgradeCopyArtifact() {
         upgradeNeeded = false;
 
         boolean isUpgraded = false;
-        for (Project<?,?> project: Jenkins.getInstance().getAllItems(Project.class)) {
-            for (CopyArtifact target: Util.filter(project.getBuilders(), CopyArtifact.class)) {
-                try {
-                    if (target.upgradeIfNecessary(project)) {
-                        isUpgraded = true;
+        for (AbstractProject<?,?> project: Jenkins.getInstance().getAllItems(AbstractProject.class)) {
+            try {
+                for (CopyArtifact target: getCopiers(project)) {
+                    try {
+                        if (target.upgradeIfNecessary(project)) {
+                            isUpgraded = true;
+                        }
+                    } catch(IOException e) {
+                        LOGGER.log(Level.SEVERE, String.format("Failed to upgrade CopyArtifact in %s", project.getFullName()), e);
                     }
-                } catch(IOException e) {
-                    LOGGER.log(Level.SEVERE, String.format("Failed to upgrade CopyArtifact in %s", project.getFullName()), e);
                 }
+            } catch (IOException e) {
+                LOGGER.log(Level.SEVERE, String.format("Failed to upgrade CopyArtifact in %s", project.getFullName()), e);
             }
         }
 
@@ -391,12 +405,7 @@ else if (ca.getProjectName().startsWith(oldName + '/'))
         }
 
         private static List<CopyArtifact> getCopiers(AbstractProject<?,?> project) throws IOException {
-            DescribableList<Builder,Descriptor<Builder>> list =
-                    project instanceof Project ? ((Project<?,?>)project).getBuildersList()
-                      : (project instanceof MatrixProject ?
-                          ((MatrixProject)project).getBuildersList() : null);
-            if (list == null) return Collections.emptyList();
-            List<CopyArtifact> copiers = list.getAll(CopyArtifact.class);
+            List<CopyArtifact> copiers = CopyArtifact.getCopiers(project);
             for (CopyArtifact copier : copiers) {
                 copier.upgradeIfNecessary(project);
             }

diff --git a/src/test/java/hudson/plugins/copyartifact/CopyArtifactTest.java b/src/test/java/hudson/plugins/copyartifact/CopyArtifactTest.java
@@ -920,6 +920,14 @@ public void testProjectNameSplit() throws Exception {
         assertTrue(configXml, configXml.contains("<project>parameterized</project>"));
         assertTrue(configXml, configXml.contains("<parameters>good=true</parameters>"));
         assertTrue(configXml, configXml.contains("<project>matrix/which=two</project>"));
+
+        MatrixProject matrixCopier = Jenkins.getInstance().getItemByFullName("matrix-copier", MatrixProject.class);
+        assertNotNull(matrixCopier);
+        configXml = matrixCopier.getConfigFile().asString();
+        assertFalse(configXml, configXml.contains("<projectName>"));
+        // When a project is specified with a variable, it is split improperly.
+        assertTrue(configXml, configXml.contains("<project>matrix</project>"));
+        assertTrue(configXml, configXml.contains("<parameters>which=${which}</parameters>"));
     }
 
     @Bug(17447)

diff --git a/src/test/resources/hudson/plugins/copyartifact/CopyArtifactTest/testProjectNameSplit.zip b/src/test/resources/hudson/plugins/copyartifact/CopyArtifactTest/testProjectNameSplit.zip