Reconsider default for spring.datasource.generate-unique-name as the current one makes test cases brittle

[odrotbohm]

Issue

The current default for spring.datasource.generate-unique-name (false) causes issues in combination with the default used for Hibernates DDL setting spring.jpa.generate-ddl (create-drop) in the context of Spring Test's context caching. I've attached a sample project that shows the issue.

Steps to reproduce

1. Unzip the linked file
2. Run mvn clean test
3. See the third test case fail as it cannot find data inserted during bootstrap of the context for the first test case.

Context

Here is what's going on:

1. We have two different context bootstraps. One that's inserting a bit of commonly shared data the application needs. The other one is not including that component. This is just a dummy setup for test cases that use a diverse set of configurations and include components that manipulate the database.
2. When the first test bootstraps, an embedded database instance is created, the schema is created and the component sets up the data in the database.
3. When the second test bootstraps, the default of create-drop causes the database schema being wiped and recreated. As this context does not contain the component setting up the reference data, the database remains initialized but unpopulated. This is fine for this particular context. However, as the database is shared across the application contexts, the other cached one is affected, too.
4. The third test case uses the same context as the first one but now doesn't find the data wiped by the second one anymore.

Underlying problems

There are a couple of problematic aspects about the current setup:

• A bean definition in an application creating a resource that's shared across application contexts is unexpected as that's not the way it works for any other bean.
• Whether the behavior imposes a problem is dependent on the order of the test classes during JUnit execution. If the third test is executed before the second one the problem disappears. This makes diagnosing the problem even harder.

Workarounds

• Explicitly setting spring.datasource.generate-unique-name=true causes database instances to be created per ApplicationContext.

Proposed solution

• Change the default of spring.datasource.generate-unique-name to true.

Side-effects of the solution

• In an internal discussion, @wilkinsona mentioned that a flip in the default would probably cause issues in DevTools, as it is re-bootstrapping the ApplicationContext and – with the new default – would lose the data entered while using the application. This could be mitigated by letting DevTools explicitly disable the property by default.

Further references

• Allow configuration to specify randomly generated database name #7004
• Make in-memory database tests default to using random name #7102

AFAIK, @sbrannen had a few thoughts on that as well.

[sbrannen]

AFAIK, @sbrannen had a few thoughts on that as well.

Yes, I always advocate generating a random database name for embedded databases in Spring integration tests.

Over the years I have seen numerous projects that ran into issues with @DirtiesContext as well as the LRU eviction policy for the ContextCache, and the only viable solution was using a random database name.

In fact, I updated the Spring Framework reference manual quite awhile ago (several years?) to point that out.

• https://docs.spring.io/spring/docs/5.1.7.RELEASE/spring-framework-reference/data-access.html#jdbc-embedded-database-unique-names
• https://docs.spring.io/spring/docs/5.1.7.RELEASE/spring-framework-reference/data-access.html#jdbc-embedded-database-dao-testing
• https://docs.spring.io/spring/docs/5.1.7.RELEASE/spring-framework-reference/data-access.html#jdbc-embedded-database-xml
• https://docs.spring.io/spring/docs/5.1.7.RELEASE/spring-framework-reference/data-access.html#jdbc-embedded-database-java

In summary, if it's feasible I think it would be beneficial if spring.datasource.generate-unique-name=true was the default in Spring Boot for integration tests.

[philwebb]

Digging though the code a bit we think this might be a safe change to make but we'll need some time to experiment to make sure there are no unexpected side effects.

[wilkinsona]

See #4699 for the DevTools side of things.

[odrotbohm]

Can we bring this on the table for 2.3 again?

[philwebb]

@odrotbohm Yeah, we'll try again to get this into the next version

[nightswimmings]

@wilkinsona @philwebb But given the context caching and reuse, isn't this gonna open several db instances in memory?

[wilkinsona]

Yes, and that is intentional and desirable as explained above by @odrotbohm and @sbrannen.

[OrangeDog]

This has the side-effect that name now does nothing unless generate-unique-name is also overridden to false.

I would expect setting the name to set the name, with no other properties set.

[wilkinsona]

I'm not sure why you have that expectation. The name property's documentation states that it is the "Datasource name to use if generate-unique-name is false".