ChainedTransactionManagerPostProcessor applied too early

[zyro23]

Task List

• Steps to reproduce provided
• Stacktrace (if present) provided
• Example that reproduces the problem uploaded to Github
• Full description of the issue provided (see below)

Steps to Reproduce

1. run sample app
2. see the bootstrap assertion fail

Expected Behaviour

the ChainedTransactionManagerPostProcessor should have been applied and the transactionManager should be instanceof ChainedTransactionManager

Actual Behaviour

org.codehaus.groovy.runtime.powerassert.PowerAssertionError:
assert transactionManager in ChainedTransactionManager
       |                  |
       |                  false
       org.grails.orm.hibernate.GrailsHibernateTransactionManager@1ff542a3

the transactionManager is still a GrailsHibernateTransactionManager.

if you debug into the ChainedTransactionManagerPostProcessor, you can see that it when it runs, there is only one transactionManager bean definition, so it is not applied.

reason is likely, that both the ChainedTransactionManagerPostProcessor and the HibernateDatastoreConnectionSourcesRegistrar (which is responsible for creating the secondary transaction managers) are BeanDefinitionRegistryPostProcessors, but only the ChainedTransactionManagerPostProcessor implements Ordered (with Order.HIGHEST_PRECEDENCE), so it runs first.

i guess the HibernateDatastoreConnectionSourcesRegistrar should also be Ordered and the order of ChainedTransactionManagerPostProcessor should be lower than that?

Environment Information

• Operating System: win x64
• Grails Version: 3.3.1, gorm-6.1.7
• JDK Version: oracle 8u131
• Container Version (If Applicable): n/a

Example Application

will be referenced in a minute

[zyro23]

i just tried to come up with a pull-request like this:

// grails-core
public class ChainedTransactionManagerPostProcessor implements BeanDefinitionRegistryPostProcessor, Ordered {
...
    @Override
    public int getOrder() {
        return Ordered.HIGHEST_PRECEDENCE + 200;
    }
...
}

// grails-data-mapping
class HibernateDatastoreConnectionSourcesRegistrar implements BeanDefinitionRegistryPostProcessor, Ordered {
...
    @Override
    int getOrder() {
        return Ordered.HIGHEST_PRECEDENCE + 100
    }
...
}

but testing with the sample-app and local snapshot publications, the ChainedTransactionManagerPostProcessor was still applied before HibernateDatastoreConnectionSourcesRegistrar!

debugging into the spring context refresh logic, i hit the following wall (https://github.com/spring-projects/spring-framework/blob/v4.3.9.RELEASE/spring-context/src/main/java/org/springframework/context/support/PostProcessorRegistrationDelegate.java#L113):

the initially found BeanDefinitionRegistryPostProcessors get ordered.
but subsequent BeanDefinitionRegistryPostProcessor that are added by other BeanDefinitionRegistryPostProcessors are just applied in the order they occur.

as all our BeanDefinitionRegistryPostProcessors are only added after GrailsApplicationPostProcessor.postProcessBeanDefinitionRegistry ran, it seems we cannot rely on correct order...

debugger looks like this:

invokeBeanFactoryPostProcessors:113, PostProcessorRegistrationDelegate (org.springframework.context.support)

// initially added (and ordered) BeanDefinitionRegistryPostProcessors:

postProcessorNames = {String[2]@3859} 
 0 = "org.springframework.context.annotation.internalConfigurationAnnotationProcessor"
 1 = "grailsApplicationPostProcessor"

// new BeanDefinitionRegistryPostProcessors added/applied in arbitrary/wrong order after grailsApplicationPostProcessor.postProcessBeanDefinitionRegistry ran:

postProcessorNames = {String[6]@4582} 
 0 = "org.springframework.context.annotation.internalConfigurationAnnotationProcessor"
 1 = "grailsApplicationPostProcessor"
 2 = "grailsConfigurationClassPostProcessor"
 3 = "chainedTransactionManagerPostProcessor"
 4 = "hibernateDatastoreConnectionSourcesRegistrar"
 5 = "grailsLayoutViewResolverPostProcessor"

and at this point, @graemerocher your feedback would be much appreciated. not sure if/how we should work around this problem or maybe raise a spring issue as this does not look like it is intentional?

[zyro23]

upstream spring issue raised: https://jira.spring.io/browse/SPR-16043

[zyro23]

upstream issue fixed/resolved:
spring-projects/spring-framework@7610210

i verified with the sample app above:

• spring-context:4.3.12.BUILD-SNAPSHOT
• grails-data-mapping:6.18.BUILD-SNAPSHOT (locally published/tested)
• grails-core:3.3.2.BUILD-SNAPSHOT (locally published/tested)

assert transactionManager in ChainedTransactionManager passes, it contains two delegate transaction managers (one for each data source).

i will pull-request the changes now. and ill leave it up to the reviewer if they get merged right away (imho should not have a negative impact) or if we wait until spring(-context) is actually released and used.

[zyro23]

boot 1.5.8 including spring 4.3.12 released

• https://spring.io/blog/2017/10/17/spring-boot-1-5-8-available-now
• Upgrade to Spring Framework 4.3.12.RELEASE spring-projects/spring-boot#10561