Skip to content
Permalink
Browse files
[JENKINS-37317] - Fix problems with requiredClasses
  • Loading branch information
alvarolobato committed Aug 10, 2016
1 parent 268df48 commit f981acf5f178eaa50a495e57ef064584d8d91f1a
Showing 6 changed files with 102 additions and 17 deletions.
21 pom.xml
@@ -41,4 +41,25 @@
<url>http://repo.jenkins-ci.org/public/</url>
</pluginRepository>
</pluginRepositories>
<dependencies>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>beer</artifactId>
<version>1.2</version>
<optional>true</optional>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<classpathDependencyExcludes>
<classpathDependencyExcludes>org.jenkins-ci.plugins:beer</classpathDependencyExcludes>
</classpathDependencyExcludes>
</configuration>
</plugin>
</plugins>
</build>
</project>
@@ -1,13 +1,13 @@
package org.jenkinsci.plugins.variant;

import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Member;

import hudson.Extension;
import hudson.ExtensionFinder.GuiceExtensionAnnotation;
import hudson.PluginWrapper;
import jenkins.model.Jenkins;

import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Member;

/**
* Processes extensions marked with {@link OptionalExtension} and decides when they are activated.
*
@@ -25,13 +25,17 @@ protected double getOrdinal(OptionalExtension annotation) {
}

/**
* If the trigger condition is not met, we filter it by not making the extension active,
* which means extensions are always non-optional as far as {@link GuiceExtensionAnnotation}
* is concerned.
* If the trigger condition is not met, we filter it by not making the extension active, which means extensions
* could always be non-optional as far as {@link GuiceExtensionAnnotation} is concerned. But in some situations, the
* class cannot even be loaded which would make {@link GuiceExtensionAnnotation} to fail, so we need to also make it
* optional.
* <p>
* See <a href="https://issues.jenkins-ci.org/browse/JENKINS-37317">JENKINS-37317</a>
*
*/
@Override
protected boolean isOptional(OptionalExtension annotation) {
return false;
return true;
}

/**
@@ -42,11 +46,23 @@ protected boolean isOptional(OptionalExtension annotation) {
@Override
protected boolean isActive(AnnotatedElement e) {
for (; e!=null; e=getParentOf(e)) {
OptionalExtension a = e.getAnnotation(OptionalExtension.class);
if (a!=null && !isActive(a)) return false;

OptionalPackage b = e.getAnnotation(OptionalPackage.class);
if (b!=null && !isActive(b)) return false;
try {
OptionalExtension a = e.getAnnotation(OptionalExtension.class);
if (a!=null && !isActive(a)) return false;
} catch (ArrayStoreException e1) {
//In this case the annotation is referencing a non existent class, make the extension inactive
// see http://bugs.java.com/view_bug.do?bug_id=7183985
return false;
}

try{
OptionalPackage b = e.getAnnotation(OptionalPackage.class);
if (b!=null && !isActive(b)) return false;
} catch (ArrayStoreException e1) {
//In this case the annotation is referencing a non existent class, make the extension inactive it due to the use of requiredClasses
// see http://bugs.java.com/view_bug.do?bug_id=7183985
return false;
}
}

return true;
@@ -57,10 +73,11 @@ private boolean isActive(OptionalExtension a) {
try {
a.requireClasses();
} catch (ArrayStoreException x) {
//In this case the annotation is referencing a non existent class, make the extension inactive
// see http://bugs.java.com/view_bug.do?bug_id=7183985
return true;
return false;
} catch (TypeNotPresentException x) {
return true;
return false;
}

for (String name : a.requirePlugins()) {
@@ -79,10 +96,11 @@ private boolean isActive(OptionalPackage a) {
try {
a.requireClasses();
} catch (ArrayStoreException x) {
//In this case the annotation is referencing a non existent class, make the extension inactive
// see http://bugs.java.com/view_bug.do?bug_id=7183985
return true;
return false;
} catch (TypeNotPresentException x) {
return true;
return false;
}

for (String name : a.requirePlugins()) {
@@ -0,0 +1,9 @@
package org.jenkinsci.plugins.variant;

import org.jenkinsci.plugins.Beer;
import org.jvnet.hudson.test.Issue;

@OptionalExtension(requireClasses=Beer.class)
@Issue("JENKINS-37317")
public class Negative3 extends Base {
}
@@ -0,0 +1,10 @@
package org.jenkinsci.plugins.variant;

import org.jenkinsci.plugins.Beer;
import org.jvnet.hudson.test.Issue;

@OptionalExtension(requireClasses=Beer.class)
@Issue("JENKINS-37317")
public class Negative4 extends Beer{

}
@@ -0,0 +1,8 @@
package org.jenkinsci.plugins.variant;

import org.jvnet.hudson.test.Issue;

@OptionalExtension(requireClasses = VariantSet.class)
@Issue("JENKINS-37317")
public class Positive4 extends Base {
}
@@ -9,6 +9,7 @@
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

import java.util.ArrayList;
@@ -21,7 +22,6 @@ public class VariantTest extends Assert {
@Rule
public JenkinsRule j = new JenkinsRule();


@BeforeClass
public static void setUp() {
VariantSet.INSTANCE = new VariantSet("test");
@@ -47,4 +47,23 @@ public void test() {
assertTrue(!classes.contains(Negative5.class));
assertTrue(!classes.contains(Negative6.class));
}

@Test
@Issue("JENKINS-37317")
public void testRequiredClass() {
List<Class> classes = new ArrayList<Class>();
for (Action a : j.jenkins.getActions()) {
classes.add(a.getClass());
}
assertTrue("Negative 3 should not exist", !classes.contains(Negative3.class));

for (Class klass : classes) {
System.out.println(klass.getCanonicalName());
assertTrue("Negative 4 should not exist",
!klass.getCanonicalName().equals("org.jenkinsci.plugins.variant.Negative4"));
}

assertTrue(classes.contains(Positive4.class));
}

}

0 comments on commit f981acf

Please sign in to comment.