Skip to content
Permalink
Browse files

[FIXED JENKINS-32359] - Properly assign persisted list owners when lo…

…ading folders from disk (#32)

* [JENKINS-32359] - Direct Unit test for the issue

* [FIXED JENKINS-32359] - Assign persisted list owners when reloading the folder with properties

* [JENKINS-32487] - Expose AbstractPropertyFolder's owner via API

* [JENKINS-32487] - Test suite refactoring

* [JENKINS-32359] - Address comments from @jglick

* [JENKINS-32359] - Add test dependency on matrix-auth plugin

* [JENKINS-32359] - Restrict AbstractFolderProperty#getOwner()
  • Loading branch information...
oleg-nenashev committed Jun 13, 2016
1 parent 4324680 commit bd3792648a12bcdb9b8616e8a793e73e8c457773
@@ -56,6 +56,13 @@
<version>2.0.4</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>matrix-auth</artifactId>
<version>1.3</version>
<scope>test</scope>
<type>jar</type>
</dependency>
</dependencies>

<build>
@@ -191,6 +191,8 @@ protected AbstractFolder(ItemGroup parent, String name) {
protected void init() {
if (properties == null) {
properties = new DescribableList<AbstractFolderProperty<?>,AbstractFolderPropertyDescriptor>(this);
} else {
properties.setOwner(this);
}
for (AbstractFolderProperty p : properties) {
p.setOwner(this);
@@ -35,6 +35,8 @@
import java.util.Collections;
import java.util.List;
import net.sf.json.JSONObject;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.StaplerOverridable;
import org.kohsuke.stapler.StaplerRequest;

@@ -54,10 +56,21 @@
/**
* Hook for performing post-initialization action.
*/
protected void setOwner(C owner) {
protected void setOwner(@NonNull C owner) {
this.owner = owner;
}

/**
* Gets an owner of the property.
* @return Owner of the property.
* It should be always non-null if the property initialized correctly.
*/
@NonNull
@Restricted(NoExternalUse.class)
public C getOwner() {
return owner;
}

@Override
public AbstractFolderPropertyDescriptor getDescriptor() {
return (AbstractFolderPropertyDescriptor) super.getDescriptor();
@@ -24,6 +24,8 @@

package com.cloudbees.hudson.plugins.folder;

import com.cloudbees.hudson.plugins.folder.properties.FolderCredentialsProvider;
import com.cloudbees.plugins.credentials.domains.DomainCredentials;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlInput;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
@@ -39,13 +41,16 @@
import hudson.search.SearchItem;
import hudson.security.ACL;
import hudson.security.WhoAmI;
import hudson.security.Permission;
import hudson.security.ProjectMatrixAuthorizationStrategy;
import hudson.tasks.BuildTrigger;
import hudson.views.BuildButtonColumn;
import hudson.views.JobColumn;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -56,6 +61,7 @@
import jenkins.model.Jenkins;
import jenkins.util.Timer;
import org.acegisecurity.AccessDeniedException;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import org.junit.Rule;
import org.junit.Test;
@@ -325,7 +331,71 @@ private void copyFromGUI(Folder f, JenkinsRule.WebClient wc, String fromName, St
f.addAction(a);
assertNotNull(f.getAction(WhoAmI.class));
}

@Issue("JENKINS-32487")
@Test public void shouldAssignPropertyOwnerOnCreationAndReload() throws Exception {
Folder folder = r.jenkins.createProject(Folder.class, "myFolder");
r.jenkins.setAuthorizationStrategy(new ProjectMatrixAuthorizationStrategy());

// We add a stub property to generate the persisted list
// Then we ensure owner is being assigned properly.
folder.addProperty(new FolderCredentialsProvider.FolderCredentialsProperty(new DomainCredentials[0]));
assertPropertyOwner("After property add", folder, FolderCredentialsProvider.FolderCredentialsProperty.class);

// Reload and ensure that the property owner is set
r.jenkins.reload();
Folder reloadedFolder = r.jenkins.getItemByFullName("myFolder", Folder.class);
assertPropertyOwner("After reload", reloadedFolder, FolderCredentialsProvider.FolderCredentialsProperty.class);
}

@Issue("JENKINS-32359")
@Test public void shouldProperlyPersistFolderPropertiesOnMultipleReloads() throws Exception {
Folder folder = r.jenkins.createProject(Folder.class, "myFolder");

// We add a stub property to generate the persisted list
// After that we save and reload the config in order to drop PersistedListOwner according to the JENKINS-32359 scenario
folder.addProperty(new FolderCredentialsProvider.FolderCredentialsProperty(new DomainCredentials[0]));
r.jenkins.reload();

// Add another property
Map<Permission,Set<String>> grantedPermissions = new HashMap<Permission, Set<String>>();
Set<String> sids = new HashSet<String>();
sids.add("admin");
grantedPermissions.put(Jenkins.ADMINISTER, sids);
folder = r.jenkins.getItemByFullName("myFolder", Folder.class);
r.jenkins.setAuthorizationStrategy(new ProjectMatrixAuthorizationStrategy());
folder.addProperty(new com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty(grantedPermissions));

// Reload folder from disk and check the state
r.jenkins.reload();
Folder reloadedFolder = r.jenkins.getItemByFullName("myFolder", Folder.class);
assertThat("Folder has not been found after the reloading", reloadedFolder, notNullValue());
assertThat("Property has not been reloaded, hence it has not been saved properly",
reloadedFolder.getProperties().get(com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty.class),
notNullValue());

// Also ensure that both property owners are configured correctly
assertPropertyOwner("After reload", reloadedFolder, FolderCredentialsProvider.FolderCredentialsProperty.class);
assertPropertyOwner("After reload", reloadedFolder, com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty.class);
}

/**
* Ensures that the specified property points to the folder.
* @param <T> Property type
* @param folder Folder
* @param propertyClass Property class
* @param step Failure message prefix
*/
private <T extends AbstractFolderProperty<AbstractFolder<?>>> void assertPropertyOwner
(String step, Folder folder, Class<T> propertyClass) {
AbstractFolder<?> propertyOwner = folder.getProperties().get(propertyClass).getOwner();
assertThat(step + ": The property owner should be instance of Folder",
propertyOwner, instanceOf(Folder.class));
assertThat(step + ": The owner field of the " + propertyClass +
" property should point to the owner folder " + folder,
(Folder)propertyOwner, equalTo(folder));
}

private Folder createFolder() throws IOException {
return r.jenkins.createProject(Folder.class, "folder" + r.jenkins.getItems().size());
}

0 comments on commit bd37926

Please sign in to comment.
You can’t perform that action at this time.