Skip to content

Improve Spring Session auto-configuration tests #11063

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 1 commit into from

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Nov 17, 2017

While putting together a sample Spring Session WebFlux app (see #11055), I've ran into spring-projects/spring-session-data-mongodb#19 - this should have arguably been picked up in Spring Session auto-configuration tests.

IMO it would be reasonable to improve session repository validation to include a check for SessionRepositoryFilter bean in Servlet-based app, or WebSessionManager bean in reactive app.

I've added that change in this PR. This caused all of ReactiveSessionAutoConfigurationMongoTests to fail, as expected, until spring-projects/spring-session-data-mongodb#19 is fixed. But the change also causes SessionAutoConfigurationJdbcTests#defaultConfigWithUniqueStoreImplementation and SessionAutoConfigurationHazelcastTests#defaultConfigWithUniqueStoreImplementation - I haven't figured out why yet. Here are the details:

-------------------------------------------------------------------------------
Test set: org.springframework.boot.autoconfigure.session.SessionAutoConfigurationJdbcTests
-------------------------------------------------------------------------------
Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.103 s <<< FAILURE! - in org.springframework.boot.autoconfigure.session.SessionAutoConfigurationJdbcTests
defaultConfigWithUniqueStoreImplementation(org.springframework.boot.autoconfigure.session.SessionAutoConfigurationJdbcTests)  Time elapsed: 0.219 s  <<< FAILURE!
java.lang.AssertionError: 

Expecting:
 <Started application org.springframework.web.context.support.AnnotationConfigWebApplicationContext@534a0b5e[id=org.springframework.web.context.support.AnnotationConfigWebApplicationContext@534a0b5e,applicationName=,beanDefinitionCount=51]>
to have a single bean of type:
 <org.springframework.session.web.http.SessionRepositoryFilter>
but found no beans of that type
	at org.springframework.boot.autoconfigure.session.AbstractSessionAutoConfigurationTests.validateSessionRepository(AbstractSessionAutoConfigurationTests.java:38)
	at org.springframework.boot.autoconfigure.session.SessionAutoConfigurationJdbcTests.validateDefaultConfig(SessionAutoConfigurationJdbcTests.java:78)
	at org.springframework.boot.test.context.runner.AbstractApplicationContextRunner.accept(AbstractApplicationContextRunner.java:292)
	at org.springframework.boot.test.context.runner.AbstractApplicationContextRunner.lambda$run$0(AbstractApplicationContextRunner.java:245)
	at org.springframework.boot.test.util.TestPropertyValues.applyToSystemProperties(TestPropertyValues.java:130)
	at org.springframework.boot.test.context.runner.AbstractApplicationContextRunner.run(AbstractApplicationContextRunner.java:243)
	at org.springframework.boot.autoconfigure.session.SessionAutoConfigurationJdbcTests.defaultConfigWithUniqueStoreImplementation(SessionAutoConfigurationJdbcTests.java:74)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:239)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:369)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:275)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:239)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:160)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:373)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:334)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:119)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:407)
-------------------------------------------------------------------------------
Test set: org.springframework.boot.autoconfigure.session.SessionAutoConfigurationHazelcastTests
-------------------------------------------------------------------------------
Tests run: 4, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.482 s <<< FAILURE! - in org.springframework.boot.autoconfigure.session.SessionAutoConfigurationHazelcastTests
defaultConfigWithUniqueStoreImplementation(org.springframework.boot.autoconfigure.session.SessionAutoConfigurationHazelcastTests)  Time elapsed: 0.071 s  <<< FAILURE!
java.lang.AssertionError: 

Expecting:
 <Started application org.springframework.web.context.support.AnnotationConfigWebApplicationContext@49e3330c[id=org.springframework.web.context.support.AnnotationConfigWebApplicationContext@49e3330c,applicationName=,beanDefinitionCount=22]>
to have a single bean of type:
 <org.springframework.session.web.http.SessionRepositoryFilter>
but found no beans of that type
	at org.springframework.boot.autoconfigure.session.AbstractSessionAutoConfigurationTests.validateSessionRepository(AbstractSessionAutoConfigurationTests.java:38)
	at org.springframework.boot.autoconfigure.session.SessionAutoConfigurationHazelcastTests.validateDefaultConfig(SessionAutoConfigurationHazelcastTests.java:71)
	at org.springframework.boot.test.context.runner.AbstractApplicationContextRunner.accept(AbstractApplicationContextRunner.java:292)
	at org.springframework.boot.test.context.runner.AbstractApplicationContextRunner.lambda$run$0(AbstractApplicationContextRunner.java:245)
	at org.springframework.boot.test.util.TestPropertyValues.applyToSystemProperties(TestPropertyValues.java:130)
	at org.springframework.boot.test.context.runner.AbstractApplicationContextRunner.run(AbstractApplicationContextRunner.java:243)
	at org.springframework.boot.autoconfigure.session.SessionAutoConfigurationHazelcastTests.defaultConfigWithUniqueStoreImplementation(SessionAutoConfigurationHazelcastTests.java:67)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:369)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:275)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:239)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:160)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:373)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:334)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:119)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:407)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 17, 2017
@snicoll snicoll added priority: normal type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 17, 2017
@snicoll snicoll self-assigned this Nov 17, 2017
@philwebb philwebb added this to the 2.0.0.M7 milestone Nov 17, 2017
@philwebb philwebb added the status: blocked An issue that's blocked on an external project change label Nov 19, 2017
@philwebb
Copy link
Member

Unfortunately the tests fail unless we also upgrade the dependencies and I got the following error trying to do that:

The type org.springframework.session.config.annotation.web.server.SpringWebSessionConfiguration cannot be resolved. It is indirectly referenced from required .class files MongoReactiveSessionConfiguration.java /spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session line 45

@vpavic
Copy link
Contributor Author

vpavic commented Nov 19, 2017

Yes, this one is blocked for the same reason as #11055 and requires the upgrade of Spring Session Data MongoDB - note that spring-projects/spring-session-data-mongodb#19 was merged in the meanwhile.

If I upgrade the spring-session-data-mongodb.version to 2.0.0.BUILD-SNAPSHOT the ReactiveSessionAutoConfigurationMongoTests are passing for me.

@vpavic vpavic force-pushed the session-config-tests branch from a504cf4 to c0dfd1a Compare November 20, 2017 07:52
@vpavic
Copy link
Contributor Author

vpavic commented Nov 20, 2017

With spring-session-data-mongodb bumped to 2.0.0.BUILD-SNAPSHOT in e4e635f the ReactiveSessionAutoConfigurationMongoTests are now passing.

SessionAutoConfigurationJdbcTests#defaultConfigWithUniqueStoreImplementation and SessionAutoConfigurationHazelcastTests#defaultConfigWithUniqueStoreImplementation still fail as described in the opening comment.

@snicoll snicoll added status: blocked An issue that's blocked on an external project change and removed status: blocked An issue that's blocked on an external project change labels Nov 20, 2017
@snicoll
Copy link
Member

snicoll commented Nov 20, 2017

@vpavic are you still looking at those two test failures or do you need help figuring out what is happening?

@vpavic
Copy link
Contributor Author

vpavic commented Nov 20, 2017

@snicoll I did take another look at this after rebasing the PR this morning but wasn't able to figure it out so I'd appreciate if you could take a look.

While debugging I noticed that SpringHttpSessionConfiguration's init method (annotated with @PostConstruct) gets invoked but two @Bean annotated methods in that class, sessionEventHttpSessionListenerAdapter and springSessionRepositoryFilter, do not.

This happens only with those two mentioned tests that manipulate the class loader using FilteredClassLoader. But at the same time, the equivalent tests in SessionAutoConfigurationRedisTests and SessionAutoConfigurationMongoTests are working fine.

@wilkinsona
Copy link
Member

wilkinsona commented Nov 20, 2017

The failure of SessionAutoConfigurationJdbcTests#defaultConfigWithUniqueStoreImplementation is an interesting one.

It fails because MongoSessionConfiguration is included during the parse configuration condition phase and is only excluded once the register bean phase is reached. As a result, SpringHttpSessionConfiguration is seen as a superclass via SpringBootMongoHttpSessionConfiguration and MongoHttpSessionConfiguration. This prevents it from being seen as a superclass via SpringBootJdbcHttpSessionConfiguration due to the check for a known super-class in ConfigurationClassParser.processConfigurationClass(ConfigurationClass). When MongoSessionConfiguration is then excluded due to condition evaluation in the register bean phase SpringHttpSessionConfiguration is lost as the only thing that "imported" it has now been skipped.

org.springframework.boot.autoconfigure.session.SessionAutoConfigurationJdbcTests.defaultConfig() passes because the explicit configuration of the store type means that MongoHttpSessionConfiguration is skipped during the parse configuration phase and SpringHttpSessionConfiguration is first seen as a superclass via SpringBootJdbcHttpSessionConfiguration.

In short, I think this is a bug in Spring Framework. It should be possible to reproduce this above-described behaviour in a standalone test case. I'll open an SPR issue if I am able to do so.

@wilkinsona
Copy link
Member

I've opened SRP-16217.

@wilkinsona wilkinsona assigned wilkinsona and unassigned snicoll Nov 20, 2017
@wilkinsona wilkinsona removed the status: blocked An issue that's blocked on an external project change label Nov 20, 2017
wilkinsona added a commit that referenced this pull request Nov 20, 2017
* gh-11063:
  Polish "Improve Spring Session auto-configuration tests"
  Improve Spring Session auto-configuration tests
@wilkinsona
Copy link
Member

Thanks for the PR, @vpavic. Having figured out the Framework problem described above, I realised it was only a problem here as MongoSessionConfiguration wasn't backing off when MongoOperationsSessionRepository was filtered out by the class loader. Tweaking its conditions accordingly allows us to avoid the problem in the Framework as MongoSessionConfiguration is now skipped in the parse configuration phase of condition evaluation.

@vpavic vpavic deleted the session-config-tests branch November 20, 2017 17:24
@vpavic
Copy link
Contributor Author

vpavic commented Nov 20, 2017

Interesting problem indeed. Thanks for providing the detailed insight @wilkinsona.

One thing that is still not clear to me is how come that Redis and its equivalent test, SessionAutoConfigurationRedisTests#defaultConfigWithUniqueStoreImplementation, wasn't affected by this?

And would it be a good idea to apply the change analogous to 8a3502b to MongoReactiveSessionConfiguration as well?

diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/MongoReactiveSessionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/MongoReactiveSessionConfiguration.java
index d5cdeb81c9..42965a2de1 100644
--- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/MongoReactiveSessionConfiguration.java
+++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/MongoReactiveSessionConfiguration.java
@@ -27,6 +27,7 @@ import org.springframework.context.annotation.Conditional;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.data.mongodb.core.ReactiveMongoOperations;
 import org.springframework.session.ReactiveSessionRepository;
+import org.springframework.session.data.mongo.ReactiveMongoOperationsSessionRepository;
 import org.springframework.session.data.mongo.config.annotation.web.reactive.ReactiveMongoWebSessionConfiguration;
 
 /**
@@ -35,7 +36,8 @@ import org.springframework.session.data.mongo.config.annotation.web.reactive.Rea
  * @author Andy Wilkinson
  */
 @Configuration
-@ConditionalOnClass(ReactiveMongoWebSessionConfiguration.class)
+@ConditionalOnClass({ ReactiveMongoOperations.class,
+		ReactiveMongoOperationsSessionRepository.class })
 @ConditionalOnMissingBean(ReactiveSessionRepository.class)
 @ConditionalOnBean(ReactiveMongoOperations.class)
 @Conditional(ReactiveSessionCondition.class)

@philwebb
Copy link
Member

@vpavic I think it makes sense to align them. I've push 6f3ca45. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants