Skip to content
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

Improve Spring integration #1

Closed
wants to merge 6 commits into from
Closed

Improve Spring integration #1

wants to merge 6 commits into from

Conversation

marcus-nl
Copy link
Contributor

Spring beans needed for BTM can be configured as follows:

    <bean id="dataSource" class="bitronix.tm.integration.spring.PoolingDataSourceFactoryBean">
        <property name="className" value="..." />
        <property name="uniqueName" value="..." />
        <property name="maxPoolSize" value="..." />
        <property name="etc" value="etc" />
        <property name="driverProperties">
            ...
        </property>
    </bean>

    <bean id="transactionManager" class="bitronix.tm.integration.spring.PlatformTransactionManager"/>

The PoolingDataSourceFactoryBean correctly works together with Spring's lifecycle management.

@marcus-nl
Copy link
Contributor Author

I have added the missing properties.

Regarding the potential race condition: synchronization is indeed taken care of by Spring, although this is not very cleanly documented.

See for example https://github.com/SpringSource/spring-framework/blob/master/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java#L123.

@lorban
Copy link
Contributor

lorban commented Aug 19, 2013

Could you please create a new maven module (like the Jetty / Tomcat lifecycles) instead of adding this to the core module?

@brettwooldridge
Copy link
Contributor

Ok, I'm satisfied that the factory method is properly synchronized at the Spring library level.

@brettwooldridge
Copy link
Contributor

Marcus, thanks for the changes. They look good, and you did a nice job keeping inline with the btm coding style. The last change before we pull code I think would be Lorban's suggestion of creating the new integration as a module along the lines of jetty/tomcat (maybe btm-spring-lifecycle). Thanks again for the nice work.

@marcus-nl
Copy link
Contributor Author

Okay, I can do that. However, are you sure that's what you want for just these two classes? Since the new Spring dependencies are declared as optional, they won't interfere with projects using BTM (that is, they are not transitive dependencies). Let me know.

@lorban
Copy link
Contributor

lorban commented Aug 21, 2013

Yes, I'm quite sure. If you look at the Tomcat and Jetty modules you'll notice that they also contain very minimal code, so don't worry if there isn't much in your module. It is just to keep a clear separation between the core BTM dependencies and the optional modules.

By the way, naming it 'btm-spring' sounds better as the '-lifecycle' suffix really is specific to the servlet container world.

Thanks!

@marcus-nl
Copy link
Contributor Author

Ok, will do.

@marcus-nl
Copy link
Contributor Author

Done

@brettwooldridge
Copy link
Contributor

Pull request reviewed and merged into master.

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

Successfully merging this pull request may close these issues.

3 participants