Skip to content

Add edge case docs of the value() attribute of ConditionalOnClass #8185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Conversation

phillipuniverse
Copy link
Contributor

When used as a meta-annotation the value() attribute of @ConditionalOnClass will fail silently resulting in the @Conditional nature of the annotation being ignored.

Here is an example that fails silently:

@Target({ ElementType.TYPE, ElementType.METHOD })
@Retention(RetentionPolicy.RUNTIME)
@Documented
@ConditionalOnClass(ClassThatMayNotExistAtRuntime.class)
public @interface ConditionalOnOptionalComponent { }

@Component
@ConditionalOnOptionalComponent
public class SomeComponentToBeOptionallyRegistered {

}

On startup, SomeComponentToBeOptionallyRegistered will still be instantiated as a Spring component. The workaround is simple, simply add the @ConditionalOnClass directly to the component:

@Component
@ConditionalOnClass(ClassThatMayNotExistAtRuntime.class)
public class SomeComponentToBeOptionallyRegistered {

}

The reason that the first example fails is because while the annotations directly on the component are read via bytecode reading, meta-annotations are read via Java reflection which results in class loading (specifically, loading the ClassThatMayNotExistAtRuntime class). The location in which this fails is in Spring Core's AnnotationUtils.getAnnotations(AnnotatedElement annotatedElement). This uses annotatedElement.getAnnotations(), part of Java's reflection package, which triggers a load of the ClassThatMayNotExistAtRuntime.class, and then throws a sun.reflect.annotation.TypeNotPresentExceptionProxy. This ends up failing silently assuming that you do not have debug-logging enabled on this piece, specifically in handleIntrospectionFailure().

This was verified against Spring core 4.3.6.RELEASE and Spring Boot 1.5.1.RELEASE.

When used as a meta-annotation the value() attribute of @ConditionalOnClass will fail silently resulting in the @conditional nature of the annotation being ignored.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 2, 2017
@philwebb
Copy link
Member

philwebb commented Mar 3, 2017

Thanks for the detailed analysis. Rather than document this as an edge case I'd like to check if the core framework might be able to fix the underling issue. I've raise SPR-15311

@philwebb philwebb added status: on-hold We can't start working on this issue yet type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 3, 2017
@philwebb
Copy link
Member

The outcome from the framework is that this is going to be hard to fix. We'll just have to document the limitation.

@philwebb philwebb added priority: normal and removed status: on-hold We can't start working on this issue yet labels Mar 20, 2017
@philwebb philwebb added this to the 1.4.6 milestone Mar 20, 2017
@phillipuniverse
Copy link
Contributor Author

Ok cool, makes sense. Just FYI we need to also update the reference documentation to include the same/similar caveat, wasn't sure exactly where that resides.

@phillipuniverse
Copy link
Contributor Author

Ah ha, it looks like this is actually a duplicate of #1069 and #1065, which is why @ConditionalOnMissingClass.value() was deprecated and then removed. Based on those issues it also looks like you can't use value() on @Bean methods either. Updating the docs to reflect that too.

@phillipuniverse
Copy link
Contributor Author

@philwebb this is good to go from my perspective. In the reference docs I went back and forth between moving the value() attribute docs to a TIP: instead of in the larger paragraph but decided on leaving it in the main prose.

@philwebb
Copy link
Member

@phillipuniverse Thanks. Are you planning to submit a pull-request for these updates?

@philwebb
Copy link
Member

@phillipuniverse Sorry ignore me, it's just been pointed out to me that this is a pull-request :)

@snicoll snicoll self-assigned this Apr 10, 2017
snicoll pushed a commit that referenced this pull request Apr 10, 2017
When used as a meta-annotation the value() attribute of
@ConditionalOnClass will fail silently resulting in the @conditional
nature of the annotation being ignored.

See gh-8185
snicoll added a commit that referenced this pull request Apr 10, 2017
* pr/8185:
  Polish "Clarify edge case docs on ConditionalOnClass"
  Clarify edge case docs on ConditionalOnClass
@snicoll snicoll closed this in 0a55e3e Apr 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants