ContextLoader is supplied composed annotation class instead of test class for default resource detection [SPR-11455]

[spring-projects-issues]

Dave Syer opened SPR-11455 and commented

Overview

I guess it all works when the annotation is @ContextConfiguration but when it is a composed annotation (i.e., an annotation meta-annotated with @ContextConfiguration) then the test framework all goes a bit haywire. I think if this bug were fixed then custom annotations composed with @ContextConfiguration would pick up the default context locations or configuration classes automatically. As things stand they don't, and there's no workaround as far as I can tell.

Analysis

ContextConfigurationAttributes has a declaringClass attribute that looks as if it is supposed to contain the test class. ContextLoaderUtils.resolveContextConfigurationAttributes(Class) loses that information somehow, maybe by using the declaringClass instead of the rootDeclaringClass of the AnnotationDescriptor it is working with, or maybe because the ContextConfigurationAttributes needs a new property for the rootDeclaringClass (my guess is the former because who would need the meta-annotation class)?

Further Analysis

The same problem applies to the @ContextHierarchy processing code, and a similar problem exists for the invocation of ActiveProfilesResolver instances (see #16093).

Consequences

One consequence is that there is no way for a ContextLoader (or SmartContextLoader) to access the declaring test class to discover default resource locations or configuration classes when @ContextConfiguration is used as a meta-annotation.

---

Affects: 4.0 GA

Attachments:

• spring-test-4.0.3.BUILD-SNAPSHOT.jar (437.47 kB)

Issue Links:

• Provide meta-annotation support for test-related annotations [SPR-7827] #12483 Provide meta-annotation support for test-related annotations
• Support meta-annotation attribute overrides in the TestContext framework [SPR-11038] #15666 Support meta-annotation attribute overrides in the TestContext framework
• @Configuration class is not detected as default in composed annotation in the TestContext framework [SPR-11641] #16264 @Configuration class is not detected as default in composed annotation in the TestContext framework
• ActiveProfilesResolver is supplied composed annotation class instead of test class [SPR-11467] #16093 ActiveProfilesResolver is supplied composed annotation class instead of test class

[spring-projects-issues]

Sam Brannen commented

This issue is closely related to #12483 and #15666.

[spring-projects-issues]

Sam Brannen commented

Hi Dave,

The behavior you are witnessing is actually by design.

Historically, the look-up mechanism for default resource locations and configuration classes has been to search for the presence of defaults relative to the declaration of @ContextConfiguration. Prior to Spring 4.0 this meant relative to the test class that is directly annotated with either @ContextConfiguration or @ContextHierarchy. However, with the support for using these annotations as meta-annotations that was introduced in Spring 4.0, the semantics for "relative" changed to mean relative to the declaring class, where "class" is one of the following:

• a test class that is directly annotated with @ContextConfiguration or @ContextHierarchy
• a composed annotation class that is directly annotated with @ContextConfiguration or @ContextHierarchy

As you pointed out, in ContextLoaderUtils.resolveContextConfigurationAttributes(Class) the declaringClass follows the semantics I described above; whereas, the rootDeclaringClass is only used to ensure that the test class hierarchy can be further traversed in order to support inheritance and overrides.

My guess is the former because who would need the meta-annotation class?

That's a good question. This was actually an important design decision that had to be made in order to support:

1. the use of test-related annotations as meta-annotations (including attribute overrides)
2. centralized configuration of integration tests via annotations instead of abstract base test classes

The desire for "centralized configuration" was the impetus for having the composed annotation serve as the "declaring class" for the purpose of default resource location and configuration class detection.

For example, consider the common scenario of integration testing your repository layer against an in-memory database. In such scenarios there is likely no need to alter the configuration across test classes: ideally this can be done in a single place.... in the custom composed annotation (e.g., @TransactionalRepositoryTest).

@Transactional
@ContextConfiguration
public @interface TransactionalRepositoryTest {

  @Configuration
  @Import(RepositoryConfig.class)
  static class Config {

    @Bean
    public DataSource dataSource() {
      // create embedded database...
    }
  }
}

@RunWith(SpringJUnit4ClassRunner.class)
@TransactionalRepositoryTest
public class UserRepositoryTests {

  @Autowired
  private UserRepository userRepository;
  
  // @Test methods...
}

The above declaration of @TransactionalRepositoryTest only works if default look-ups are relative to the composed annotation.

Thus the current implementation is intended to support the above configuration style.

However, if we change the current implementation for what is supplied as the "declaring class" to the look-up mechanism, @TransactionalRepositoryTest would have to be rewritten as follows:

@Transactional
@ContextConfiguration(classes = TransactionalRepositoryTest.Config.class)
public @interface TransactionalRepositoryTest {

  @Configuration
  @Import(RepositoryConfig.class)
  static class Config {

    @Bean
    public DataSource dataSource() {
      // create embedded database...
    }
  }
}

The difference is chiefly a matter of style with regard to the programming model.

For an example of the status quo in action, see MetaAnnotationConfigWacTests and @WebTestStereotype as well as the classes in the org.springframework.test.context.junit4.annotation.meta package in Spring's test suite.

[spring-projects-issues]

Sam Brannen commented

Dave Syer,

I have pushed a preliminary version of the work necessary to implement the desired changes to my GitHub account.

https://github.com/sbrannen/spring-framework/commits/SPR-11455

Please have a look and let me know if you think that now meets your expectations, paying special attention to the various integration testing scenarios in the org.springframework.test.context.junit4.annotation.meta package.

Thanks,

Sam

p.s. why do you have two JIRA accounts: david_syer and davids? ;)

[spring-projects-issues]

Dave Syer commented

Thanks! Any chance you could build a jar file and send me a link to that (building Spring from scratch is beyond me these days, or I might have had a bash at a pull request)?

I didn't know I had 2 accounts (I only use david_syer).

[spring-projects-issues]

Sam Brannen commented

Attached 4.0.3 build snapshot for spring-test (based on private branch).

[spring-projects-issues]

Sam Brannen commented

Any chance you could build a jar file and send me a link to that (building Spring from scratch is beyond me these days, or I might have had a bash at a pull request)?

Yep, give the attached snapshot a try.

I didn't know I had 2 accounts (I only use david_syer).

Ahhh, then maybe you should try to get the other deleted and merged into your real account.

- Sam

[spring-projects-issues]

Sam Brannen commented

Dave Syer, I haven't heard back from you regarding the patch, but I'm going to assume that it's OK.

Furthermore, after further consideration, I have decided that it is in fact better to always provide the actual declaring test class to context loaders, as this aligns better with the convention that has been used over the years -- as you pointed out as a consequence above. ;)

So I'll go ahead and merge this into master for Spring Framework 4.0.3.

Regards,

Sam

[spring-projects-issues]

Sam Brannen commented

Fixed as described in the comments for GitHub commit 39eb744:

Always supply test class to ContextLoaders in TCF

Prior to this commit, the following methods in ContextLoaderUtils
treated a composed @ContextConfiguration annotation (i.e., a custom
annotation that is meta-annotated with @ContextConfiguration) as the
"declaring class" instead of the actual test class.

• resolveContextConfigurationAttributes()
• resolveContextHierarchyAttributes()

As a consequence, if @ContextConfiguration is used as a
meta-annotation, the meta-annotated class is stored as the
"declaringClass" in ContextConfigurationAttributes. Thus, when a
ContextLoader (or SmartContextLoader) attempts to detect default
resource locations or configuration classes, it does so for the
composed annotation class instead of for the declaring test class.

This commit ensures that ContextLoaders are supplied the declaring test
class instead of the composed annotation class in such use cases.

[spring-projects-issues]

Dave Syer commented

Sorry, I got distracted. I wasn't able to get my code working with the JAR you posted here. I'll try a snapshot from repo.spring.io.

[spring-projects-issues]

Sam Brannen commented

No worries, and yeah... I figured it would be easier to have all of the modules and Maven metadata from a single snapshot build. So that was another reason to push the changes to master. ;)

Please let me know if it solves your problems.

[spring-projects-issues]

Dave Syer commented

Yes, thanks: spring-projects/spring-boot@26ebc98. Should work when 4.0.3 comes out if you don't change anything.

I was wondering though, why I have to do the default classes detection myself (Spring could do it for me now).

[spring-projects-issues]

Sam Brannen commented

Hi Dave,

Thanks for testing the fix. I'm glad it works for you!

I was wondering though, why I have to do the default classes detection myself (Spring could do it for me now).

Well, default resource detection is the responsibility of a SmartContextLoader. So if you extend AnnotationConfigContextLoader, GenericXmlContextLoader, AnnotationConfigWebContextLoader, or GenericXmlWebContextLoader, then Spring will in fact perform the detection for you. ;)

Otherwise, it's up to the author of the SmartContextLoader to ensure that defaults are detected.

Does that answer your question?

[spring-projects-issues]

Sam Brannen commented

Dave Syer, on a somewhat related note, I noticed that Spring Boot's SpringApplicationContextLoader is a bit of a hybrid approach to implementing a SmartContextLoader, in that it attempts to combine support for both resource locations and configuration classes. It also supports web contexts as well.

I say attempts, because it fails to actually detect default resource locations. Its implementation of processContextConfiguration(ContextConfigurationAttributes) overrides the default in AbstractContextLoader which is responsible for detecting default resource locations.

I understood the desire to create a single context loader that supports locations and classes as well as standard and web contexts; however, the current infrastructure is not well suited for this, simply due to historical reasons regarding backwards compatibility. Having said that, the current implementation of SpringApplicationContextLoader unfortunately appears to break some of the contracts defined by the SmartContextLoader SPI and Spring in general.

For example:

1. What happens if a Spring Boot user declares both resource locations and configuration classes?
2. What happens if ApplicationContextInitializer instances are invoked in the incorrect order (i.e., Ordered and @Order are not honored)?
   • Or does SpringApplication ensure the proper order behind the scenes?
3. Can SpringApplicationContextLoader even be used if the Servlet 3.0 API is not in the classpath?

I have to admit that I have not looked at all of this in detail (other than cursory in GitHub). So if you'd like to take this offline or discuss these topics in a Spring Boot JIRA issue, just let me know.

Regards,

Sam