Skip to content
Permalink
Browse files

Merge pull request #110 from jglick/hasValidDestination-JENKINS-40612

[JENKINS-40612] More efficient idiom for hasValidDestination
  • Loading branch information...
jglick committed Oct 10, 2017
2 parents 84bdaaf + 1db3eab commit 533c71e20349cea16c0b67f51103c20018e42ead
12 pom.xml
@@ -5,12 +5,12 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>2.33</version>
<version>2.36</version>
<relativePath />
</parent>

<artifactId>cloudbees-folder</artifactId>
<version>6.1.3-SNAPSHOT</version>
<version>6.2.0-SNAPSHOT</version>
<packaging>hpi</packaging>

<name>Folders Plugin</name>
@@ -19,13 +19,13 @@
</description>
<url>https://wiki.jenkins-ci.org/display/JENKINS/CloudBees+Folders+Plugin</url>
<properties>
<jenkins.version>1.642.3</jenkins.version>
<jenkins.version>2.46.3</jenkins.version>
<no-test-jar>false</no-test-jar>
</properties>
<licenses>
<license>
<name>MIT</name>
<url>http://opensource.org/licenses/MIT</url>
<url>https://opensource.org/licenses/MIT</url>
</license>
</licenses>

@@ -39,13 +39,13 @@
<repositories>
<repository>
<id>repo.jenkins-ci.org</id>
<url>http://repo.jenkins-ci.org/public/</url>
<url>https://repo.jenkins-ci.org/public/</url>
</repository>
</repositories>
<pluginRepositories>
<pluginRepository>
<id>repo.jenkins-ci.org</id>
<url>http://repo.jenkins-ci.org/public/</url>
<url>https://repo.jenkins-ci.org/public/</url>
</pluginRepository>
</pluginRepositories>

@@ -564,11 +564,7 @@ public HttpResponse doBuild(@QueryParameter TimeDuration delay) {
if (!isBuildable()) {
return null;
}
Jenkins j = Jenkins.getInstance();
if (j == null) {
return null;
}
return j.getQueue().schedule2(this, quietPeriod, Arrays.asList(actions)).getItem();
return Queue.getInstance().schedule2(this, quietPeriod, Arrays.asList(actions)).getItem();
}

/**
@@ -685,7 +681,7 @@ public Authentication getDefaultAuthentication(Queue.Item item) {
*/
@Override
public Label getAssignedLabel() {
Jenkins j = Jenkins.getInstance();
Jenkins j = Jenkins.getInstanceOrNull();
if (j == null) {
return null;
}
@@ -114,11 +114,7 @@ public void doRun() {
* @param cal the date to check for.
*/
public void checkTriggers(final Calendar cal) {
Jenkins inst = Jenkins.getInstance();
if (inst == null) {
return;
}
for (ComputedFolder<?> p : inst.getAllItems(ComputedFolder.class)) {
for (ComputedFolder<?> p : Jenkins.getInstance().allItems(ComputedFolder.class)) {
for (Trigger<?> t : p.getTriggers().values()) {
LOGGER.log(Level.FINE, "cron checking {0}", p.getName());
CronTabList tabs;
@@ -60,9 +60,6 @@ public CauseOfBlockage canRun(Queue.Item item) {
*/
public int indexingCount() {
Jenkins j = Jenkins.getInstance();
if (j == null) {
return 0;
}
int result = indexingCount(j);
for (Node n : j.getNodes()) {
result += indexingCount(n);
@@ -24,7 +24,6 @@

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

import com.cloudbees.hudson.plugins.folder.AbstractFolder;
import com.cloudbees.hudson.plugins.folder.Folder;
import hudson.Extension;
import hudson.model.AbstractItem;
@@ -33,6 +32,7 @@
import hudson.model.Items;
import hudson.model.Job;
import hudson.model.TopLevelItem;
import hudson.security.ACL;
import hudson.security.AccessControlled;
import java.io.IOException;
import java.util.ArrayList;
@@ -48,9 +48,11 @@
/**
* Handler which can move items which are both {@link AbstractItem} and {@link TopLevelItem} into a {@link DirectlyModifiableTopLevelItemGroup}.
*/
@SuppressWarnings("rawtypes")
@Restricted(NoExternalUse.class)
@Extension(ordinal=-1000) public final class StandardHandler extends RelocationHandler {

@Override
public HandlingMode applicability(Item item) {
if (item instanceof TopLevelItem
&& item instanceof AbstractItem
@@ -78,16 +80,15 @@ public HandlingMode applicability(Item item) {
}

public boolean hasValidDestination(Item item) {
Jenkins instance = Jenkins.getActiveInstance();
Jenkins instance = Jenkins.getInstance();
if (permitted(item, instance) && instance.getItem(item.getName()) == null) {
// we can move to the root if there is none with the same name.
return true;
}
// TODO use Items.allItems(instance, Item.class) once baseline Jenkins 2.37+
ITEM: for (Item g : instance.getAllItems()) {
ITEM: for (Item g : Items.allItems(ACL.SYSTEM, instance, Item.class)) {
if (g instanceof DirectlyModifiableTopLevelItemGroup) {
DirectlyModifiableTopLevelItemGroup itemGroup = (DirectlyModifiableTopLevelItemGroup) g;
if (!permitted(item, itemGroup)) {
if (!permitted(item, itemGroup) || /* unlikely since we just checked CREATE, but just in case: */ !g.hasPermission(Item.READ)) {
continue;
}
// Cannot move a folder into itself or a descendant
@@ -126,8 +127,8 @@ public boolean hasValidDestination(Item item) {

@Override
public List<? extends ItemGroup<?>> validDestinations(Item item) {
List<DirectlyModifiableTopLevelItemGroup> result = new ArrayList<DirectlyModifiableTopLevelItemGroup>();
Jenkins instance = Jenkins.getActiveInstance();
List<DirectlyModifiableTopLevelItemGroup> result = new ArrayList<>();
Jenkins instance = Jenkins.getInstance();
// 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
@@ -29,10 +29,9 @@
import hudson.model.Item;
import hudson.model.User;
import hudson.security.ACL;
import hudson.security.ACLContext;
import java.util.Arrays;
import jenkins.model.Jenkins;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import static org.junit.Assert.*;
import org.junit.Rule;
import org.junit.Test;
@@ -52,15 +51,12 @@
r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
grant(Jenkins.READ, Item.READ).everywhere().to("joe").
grant(Item.CREATE).onItems(d2).to("joe"));
SecurityContext sc = ACL.impersonate(User.get("joe").impersonate());
try {
try (ACLContext ctx = ACL.as(User.get("joe"))) {
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);
}
}

0 comments on commit 533c71e

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