Skip to content

Commit

Permalink
Merge pull request #38 from olivergondza/revisit-saveables
Browse files Browse the repository at this point in the history
[FIXED JENKINS-24412][FIXED JENKINS-24410] Save only TopLevelItems
  • Loading branch information
Stefan Brausch committed Sep 3, 2014
2 parents 259de16 + d679c9c commit 6810fe5
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 49 deletions.
38 changes: 17 additions & 21 deletions src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistory.java
Expand Up @@ -3,24 +3,27 @@
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;
Expand Down Expand Up @@ -57,7 +60,8 @@ public class JobConfigHistory extends Plugin {
private transient boolean saveSystemConfiguration; //NOPMD

/** Flag to indicate ItemGroups configuration is saved as well. */
private boolean saveItemGroupConfiguration;
@Deprecated
private transient boolean saveItemGroupConfiguration;

/** Flag to indicate if we should save history when it
* is a duplication of the previous saved configuration.
Expand Down Expand Up @@ -98,7 +102,6 @@ public void configure(StaplerRequest req, JSONObject formData)
setMaxHistoryEntries(formData.getString("maxHistoryEntries").trim());
setMaxDaysToKeepEntries(formData.getString("maxDaysToKeepEntries").trim());
setMaxEntriesPerPage(formData.getString("maxEntriesPerPage").trim());
saveItemGroupConfiguration = formData.getBoolean("saveItemGroupConfiguration");
skipDuplicateHistory = formData.getBoolean("skipDuplicateHistory");
excludePattern = formData.getString("excludePattern");
saveModuleConfiguration = formData.getBoolean("saveModuleConfiguration");
Expand Down Expand Up @@ -198,8 +201,9 @@ boolean isPositiveInteger(String numberString) {
/**
* @return True if item group configurations should be saved.
*/
@Deprecated
public boolean getSaveItemGroupConfiguration() {
return saveItemGroupConfiguration;
return true;
}

/**
Expand Down Expand Up @@ -347,22 +351,14 @@ 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 (xmlFile.getFile().getParentFile().equals(getJenkinsHome())) return checkRegex(xmlFile);

if (item instanceof MavenModule && saveModuleConfiguration) return true;

return false;
}

/**
Expand Down
Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down
Expand Up @@ -17,9 +17,6 @@
<f:textbox name="maxEntriesPerPage" value="${it.maxEntriesPerPage}"
checkUrl="'${rootURL}/plugin/jobConfigHistory/checkMaxEntriesPerPage?value='+escape(this.value)"/>
</f:entry>
<f:entry title="${%Save folder configuration changes}" help="/plugin/jobConfigHistory/help/help-saveItemGroupConfiguration.html">
<f:checkbox name="saveItemGroupConfiguration" checked="${it.saveItemGroupConfiguration}"/>
</f:entry>
<f:entry title="${%System configuration exclude file pattern}" help="/plugin/jobConfigHistory/help/help-excludePattern.html">
<f:textbox name="excludePattern" value="${it.excludePattern}" default="${it.defaultExcludePattern}"
checkUrl="'${rootURL}/plugin/jobConfigHistory/checkExcludePattern?value='+escape(this.value)"/>
Expand All @@ -41,4 +38,4 @@
</f:entry>
</f:advanced>
</f:section>
</j:jelly>
</j:jelly>
@@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down
Expand Up @@ -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 org.jvnet.hudson.test.MockFolder;

import static org.hamcrest.CoreMatchers.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -176,17 +186,6 @@ public void testIsPositiveInteger() {

}

/**
* Test of getSaveItemGroupConfiguration method, of class JobConfigHistory.
*/
@Test
public void testGetSaveItemGroupConfiguration() {
JobConfigHistory sut = createSut();
boolean expResult = false;
boolean result = sut.getSaveItemGroupConfiguration();
assertEquals(expResult, result);
}

/**
* Test of getSkipDuplicateHistory method, of class JobConfigHistory.
*/
Expand Down Expand Up @@ -332,14 +331,12 @@ 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(mock(MockFolder.class), xmlFile));
assertTrue(sut.isSaveable(null, new XmlFile(unpackResourceZip.getResource("config.xml"))));
assertTrue(sut.isSaveable(mock(ItemGroup.class), xmlFile));
assertFalse(sut.isSaveable(null, xmlFile));

formData.put("saveItemGroupConfiguration", false);
sut.configure(null, formData);
assertFalse(sut.isSaveable(mock(ItemGroup.class), xmlFile));
assertFalse(sut.isSaveable(mock(MatrixConfiguration.class), xmlFile));
}

/**
Expand Down Expand Up @@ -415,7 +412,6 @@ JSONObject createFormData() {
"\"maxHistoryEntries\": \"5\"," +
"\"maxDaysToKeepEntries\": \"5\"," +
"\"maxEntriesPerPage\": \"50\"," +
"\"saveItemGroupConfiguration\": true," +
"\"skipDuplicateHistory\": true," +
"\"excludePattern\": \"5\"," +
"\"saveModuleConfiguration\": true," +
Expand Down

0 comments on commit 6810fe5

Please sign in to comment.