Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #57 from fbelzunc/JENKINS-34306
[FIXED JENKINS-34306] When moving an item several destinations showed in the GUI are not valid
  • Loading branch information
recena committed May 9, 2016
2 parents 218a42c + 7bfb09a commit adf523e
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 8 deletions.
Expand Up @@ -24,6 +24,7 @@

package com.cloudbees.hudson.plugins.folder.relocate;

import com.cloudbees.hudson.plugins.folder.Folder;
import hudson.Extension;
import hudson.model.AbstractItem;
import hudson.model.Item;
Expand All @@ -50,7 +51,7 @@
@Extension(ordinal=-1000) public final class StandardHandler extends RelocationHandler {

public HandlingMode applicability(Item item) {
if (item instanceof TopLevelItem && item instanceof AbstractItem && item.getParent() instanceof DirectlyModifiableTopLevelItemGroup) {
if (item instanceof TopLevelItem && item instanceof AbstractItem && item.getParent() instanceof DirectlyModifiableTopLevelItemGroup && !validDestinations(item).isEmpty()) {
return HandlingMode.HANDLE;
} else {
return HandlingMode.SKIP;
Expand All @@ -75,7 +76,10 @@ private static <I extends AbstractItem & TopLevelItem> I doMove(Item item, Direc
@Override public List<? extends ItemGroup<?>> validDestinations(Item item) {
List<DirectlyModifiableTopLevelItemGroup> result = new ArrayList<DirectlyModifiableTopLevelItemGroup>();
Jenkins instance = Jenkins.getActiveInstance();
if (permitted(item, instance)) {
// ROOT context is only added in case there is not any item with the same name
// But we add it in case the one is there is the item itself and not a different job with the same name
// No-op by default
if (permitted(item, instance) && (instance.getItem(item.getName()) == null) || instance.getItem(item.getName()) == item) {
result.add(instance);
}
ITEM: for (Item g : instance.getAllItems()) {
Expand All @@ -85,15 +89,37 @@ private static <I extends AbstractItem & TopLevelItem> I doMove(Item item, Direc
continue;
}
ItemGroup<?> p = itemGroup;
while (p instanceof Item) {
if (p instanceof Item) {
Item i = (Item) p;
// Cannot move a folder into itself or a descendant
if (i == item) {
// Cannot move a folder into itself or a descendant.
continue ITEM;
}
p = i.getParent();
// By default the move is a no-op in case you hit it by mistake
if (item.getParent() == i) {
result.add(itemGroup);
}
// Cannot move an item into a Folder if there is already an item with the same name
if (i instanceof Folder) {
Folder folder = (Folder) i;
if (folder.getItem(item.getName()) != null) {
continue ITEM;
}
}
// Cannot move a folder into a descendant
// Cannot move d1/ into say d1/d2/d3/
ItemGroup itemGroupSubElement = i.getParent();
while (itemGroupSubElement != instance) {
if (itemGroupSubElement instanceof Folder) {
Folder currentFolder = (Folder) itemGroupSubElement;
if (item == currentFolder) {
continue ITEM;
}
itemGroupSubElement = currentFolder.getParent();
}
}
result.add(itemGroup);
}
result.add(itemGroup);
}
}
return result;
Expand Down
Expand Up @@ -54,11 +54,70 @@ public class StandardHandlerTest {
grant(Item.CREATE).onItems(d2).to("joe"));
SecurityContext sc = ACL.impersonate(User.get("joe").impersonate());
try {
assertEquals(Arrays.asList(/* only because we are already here */d1, d2), new StandardHandler().validDestinations(j));
assertEquals(Arrays.asList(/* ditto */r.jenkins, d2), new StandardHandler().validDestinations(d1));
assertEquals(Arrays.asList(d1, d2), new StandardHandler().validDestinations(j));
assertEquals(Arrays.asList(r.jenkins, d2), new StandardHandler().validDestinations(d1));

assertNotEquals(Arrays.asList(r.jenkins, d3), new StandardHandler().validDestinations(j));
assertNotEquals(Arrays.asList(d1, d3), new StandardHandler().validDestinations(d1));
} finally {
SecurityContextHolder.setContext(sc);
}
}

@Test public void getDestinationsUsingSubfolders() throws Exception {
Folder d1 = r.jenkins.createProject(Folder.class, "d1");
Folder d11 = d1.createProject(Folder.class, "d11");
FreeStyleProject j = d1.createProject(FreeStyleProject.class, "j");
Folder d2 = r.jenkins.createProject(Folder.class, "d2");
Folder d3 = r.jenkins.createProject(Folder.class, "d3");
Folder d31 = d3.createProject(Folder.class, "d31");

assertEquals(Arrays.asList(r.jenkins, d1, d11, d2, d3, d31), new StandardHandler().validDestinations(j));
assertEquals(Arrays.asList(r.jenkins, d1, d2, d3, d31), new StandardHandler().validDestinations(d11));

assertNotEquals(d11, new StandardHandler().validDestinations(d11));
}

@Test public void getDestinationsUsingItemsWithSameName() throws Exception {
Folder d1 = r.jenkins.createProject(Folder.class, "d1");
Folder d11 = d1.createProject(Folder.class, "d11");
FreeStyleProject j = d1.createProject(FreeStyleProject.class, "j");
Folder d2 = r.jenkins.createProject(Folder.class, "d2");
FreeStyleProject g = d2.createProject(FreeStyleProject.class, "j");
Folder d3 = r.jenkins.createProject(Folder.class, "d3");
Folder d31 = d3.createProject(Folder.class, "d11");

assertEquals(Arrays.asList(r.jenkins, d1, d11, d3, d31), new StandardHandler().validDestinations(j));
assertEquals(Arrays.asList(r.jenkins, d1, d2, d31), new StandardHandler().validDestinations(d11));

assertNotEquals(d2, new StandardHandler().validDestinations(j));
assertNotEquals(Arrays.asList(d11, d3), new StandardHandler().validDestinations(d11));
}

@Test public void getDestinationsUsingItemsWithSameNameOnRootContext() throws Exception {
FreeStyleProject j = r.jenkins.createProject(FreeStyleProject.class, "j");
Folder d1 = r.jenkins.createProject(Folder.class, "d1");
Folder d11 = d1.createProject(Folder.class, "d11");
Folder d2 = r.jenkins.createProject(Folder.class, "d2");
FreeStyleProject g = d2.createProject(FreeStyleProject.class, "j");
Folder d3 = r.jenkins.createProject(Folder.class, "d3");
Folder d31 = d3.createProject(Folder.class, "d11");

assertEquals(Arrays.asList(r.jenkins, d1, d11, d3, d31), new StandardHandler().validDestinations(j));
assertEquals(Arrays.asList(r.jenkins, d1, d2, d31), new StandardHandler().validDestinations(d11));

assertNotEquals(d2, new StandardHandler().validDestinations(j));
assertNotEquals(d3, new StandardHandler().validDestinations(d11));
}

@Test public void getDestinationsMovingAParentFolderInToTheTree() throws Exception {
Folder d1 = r.jenkins.createProject(Folder.class, "d1");
Folder d11 = d1.createProject(Folder.class, "d2");
Folder d12 = d11.createProject(Folder.class, "d3");
Folder d4 = r.jenkins.createProject(Folder.class, "d4");

assertEquals(Arrays.asList(r.jenkins, d4), new StandardHandler().validDestinations(d1));
assertNotEquals(Arrays.asList(d11, d12), new StandardHandler().validDestinations(d1));
}

}

0 comments on commit adf523e

Please sign in to comment.