[FIXED JENKINS-24412][FIXED JENKINS-24410] Save only ItemGroups

jenkinsci · Aug 26, 2014 · 0cb1752 · 0cb1752
1 parent 259de16
commit 0cb1752
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 23 deletions.
diff --git a/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistory.java b/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistory.java
@@ -3,24 +3,28 @@
 import hudson.Plugin;
 import hudson.XmlFile;
 import hudson.maven.MavenModule;
-import hudson.maven.MavenModuleSet;
 import hudson.model.AbstractProject;
 import hudson.model.Descriptor.FormException;
 import hudson.model.Hudson;
 import hudson.model.Item;
 import hudson.model.ItemGroup;
 import hudson.model.Saveable;
+import hudson.model.TopLevelItem;
 import hudson.util.FormValidation;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
+
 import javax.servlet.ServletException;
+
 import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
+
 import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
@@ -347,22 +351,15 @@ protected File getConfigFile(final File historyDir) {
      * @return true if the item configuration should be saved.
      */
     boolean isSaveable(final Saveable item, final XmlFile xmlFile) {
-        boolean saveable = false;
-        final boolean group = item instanceof ItemGroup;
-        if (item instanceof Item && !group) {
-            saveable = true;
-        } else if (xmlFile.getFile().getParentFile().equals(getJenkinsHome())) {
-            saveable = checkRegex(xmlFile);
-        } else if (saveItemGroupConfiguration && group) {
-            saveable = true;
-        }
-        if (item instanceof MavenModuleSet) {
-            saveable = true;
-        }
-        if (item instanceof MavenModule && !saveModuleConfiguration) {
-            saveable = false;
-        }
-        return saveable;
+        if (item instanceof TopLevelItem) return true;
+
+        if (saveItemGroupConfiguration && item instanceof ItemGroup) return true;
+
+        if (xmlFile.getFile().getParentFile().equals(getJenkinsHome())) return checkRegex(xmlFile);
+
+        if (item instanceof MavenModule && saveModuleConfiguration) return true;
+
+        return false;
     }
 
     /**

diff --git a/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectAction.java b/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectAction.java
@@ -6,6 +6,7 @@
 import hudson.model.Api;
 import hudson.model.Hudson;
 import hudson.model.Item;
+import hudson.model.TopLevelItem;
 import hudson.plugins.jobConfigHistory.SideBySideView.Line;
 import hudson.security.AccessControlled;
 
@@ -23,7 +24,6 @@
 import javax.xml.transform.Source;
 import javax.xml.transform.stream.StreamSource;
 
-
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.export.ExportedBean;
@@ -69,11 +69,14 @@ public final String getIconFileName() {
         if (!hasConfigurePermission()) {
             return null;
         }
-        if (!getPlugin().getSaveModuleConfiguration() && project instanceof MavenModule) {
-            return null;
+        if (project instanceof TopLevelItem) {
+            return JobConfigHistoryConsts.ICONFILENAME;
+        }
+        if (getPlugin().getSaveModuleConfiguration() && project instanceof MavenModule) {
+            return JobConfigHistoryConsts.ICONFILENAME;
         }
 
-        return JobConfigHistoryConsts.ICONFILENAME;
+        return null;
     }
 
     /**

diff --git a/src/test/java/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectActionTest.java b/src/test/java/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectActionTest.java
@@ -1,20 +1,30 @@
 package hudson.plugins.jobConfigHistory;
 
+import hudson.matrix.MatrixConfiguration;
+import hudson.matrix.MatrixProject;
 import hudson.maven.MavenModule;
 import hudson.model.AbstractItem;
 import hudson.model.AbstractProject;
 import hudson.model.Hudson;
 import hudson.model.ItemGroup;
+
 import java.io.IOException;
 import java.util.List;
+
+import jenkins.model.AbstractTopLevelItem;
+
 import org.acegisecurity.AccessDeniedException;
 import org.apache.commons.io.FileUtils;
 import org.hamcrest.CoreMatchers;
 import org.junit.Test;
+
 import static org.junit.Assert.*;
+
 import org.junit.Before;
 import org.junit.Rule;
+
 import static org.mockito.Mockito.*;
+
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
@@ -31,7 +41,7 @@ public class JobConfigHistoryProjectActionTest {
     private final MavenModule mockedMavenModule = mock(MavenModule.class);
     private final JobConfigHistory mockedPlugin = mock(JobConfigHistory.class);
     private final Hudson mockedHudson = mock(hudson.model.Hudson.class);
-    private final AbstractItem mockedProject = mock(AbstractItem.class);
+    private final AbstractTopLevelItem mockedProject = mock(AbstractTopLevelItem.class);
     private final StaplerRequest mockedRequest = mock(StaplerRequest.class);
     private final StaplerResponse mockedResponse = mock(StaplerResponse.class);
     private HistoryDao historyDao;
@@ -90,6 +100,24 @@ public void testGetIconFileNameDoNotSaveMavenModules() {
         assertNull(sut.getIconFileName());
     }
 
+    @Test
+    public void testGetIconFileNameMatrixProject() {
+        MatrixProject project = mock(MatrixProject.class);
+        when(project.hasPermission(AbstractProject.CONFIGURE)).thenReturn(true);
+
+        JobConfigHistoryProjectActionImpl action = new JobConfigHistoryProjectActionImpl(mockedHudson, project);
+        assertEquals(JobConfigHistoryConsts.ICONFILENAME, action.getIconFileName());
+    }
+
+    @Test
+    public void testGetIconFileNameMatrixConfiguration() {
+        MatrixConfiguration configuration = mock(MatrixConfiguration.class);
+        when(configuration.hasPermission(AbstractProject.CONFIGURE)).thenReturn(true);
+
+        JobConfigHistoryProjectActionImpl action = new JobConfigHistoryProjectActionImpl(mockedHudson, configuration);
+        assertNull(action.getIconFileName());
+    }
+
     /**
      * Test of getJobConfigs method, of class JobConfigHistoryProjectAction.
      */

diff --git a/src/test/java/hudson/plugins/jobConfigHistory/JobConfigHistoryTest.java b/src/test/java/hudson/plugins/jobConfigHistory/JobConfigHistoryTest.java
@@ -24,21 +24,31 @@
 package hudson.plugins.jobConfigHistory;
 
 import hudson.XmlFile;
+import hudson.matrix.MatrixConfiguration;
+import hudson.matrix.MatrixProject;
 import hudson.model.AbstractProject;
 import hudson.model.Descriptor;
 import hudson.model.ItemGroup;
+import hudson.model.TopLevelItem;
 import hudson.model.User;
 import hudson.security.ACL;
 import hudson.util.FormValidation;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.regex.Pattern;
+
 import javax.servlet.ServletException;
+
 import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
+
 import org.junit.Test;
+
 import static org.junit.Assert.*;
+
 import org.junit.Rule;
+
 import static org.hamcrest.CoreMatchers.*;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -332,14 +342,18 @@ public void testIsSaveable() throws IOException, ServletException, Descriptor.Fo
         final JSONObject formData = createFormData();
 
         sut.configure(null, formData);
-        assertTrue(sut.isSaveable(mock(AbstractProject.class), xmlFile));
+        assertTrue(sut.isSaveable(mock(TopLevelItem.class), xmlFile));
+        assertTrue(sut.isSaveable(mock(MatrixProject.class), xmlFile));
         assertTrue(sut.isSaveable(null, new XmlFile(unpackResourceZip.getResource("config.xml"))));
         assertTrue(sut.isSaveable(mock(ItemGroup.class), xmlFile));
         assertFalse(sut.isSaveable(null, xmlFile));
+        assertFalse(sut.isSaveable(mock(MatrixConfiguration.class), xmlFile));
 
         formData.put("saveItemGroupConfiguration", false);
         sut.configure(null, formData);
         assertFalse(sut.isSaveable(mock(ItemGroup.class), xmlFile));
+        assertFalse(sut.isSaveable(mock(MatrixConfiguration.class), xmlFile));
+        assertTrue(sut.isSaveable(mock(MatrixProject.class), xmlFile));
     }
 
     /**