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

Update JpaPersistService to ignore subsequent calls to start() and stop() #1214

Closed
wants to merge 1 commit into from
Closed

Update JpaPersistService to ignore subsequent calls to start() and stop() #1214

wants to merge 1 commit into from

Conversation

chrisparton1991
Copy link
Contributor

@chrisparton1991 chrisparton1991 commented Oct 12, 2018

This resolves #947 and supersedes #972 and #598. It possibly also supersedes #1132, although I'm not sure what that user's use case is.

The current behaviour is to throw an IllegalStateException when JpaPersistService.start() or JpaPersistService.stop() are called multiple times. However, this clashes with the javadoc in PersistService and UnitOfWork, both of which state that subsequent calls to these methods are no-ops.

JpaPersistService.end() already handles multiple calls, and has the following comment:
// Let's not penalize users for calling end() multiple times.

This PR brings JpaPersistService and related tests into line with the documentation and resolves a pain point that multiple people (including myself) have experienced, e.g. https://stackoverflow.com/questions/17402081/how-to-start-jpa-in-a-guice-quartz-web-application.

Thank you for taking the time to look at my PR.

…op(), which is consistent with the PersistService and UnitOfWork documentation.
@jlannoy
Copy link

jlannoy commented Oct 28, 2018

Thanks Chris ; I came here to fix the same problem... And saw you already did the job. I hope we'll get a release soon :)

@chrisparton1991
Copy link
Contributor Author

@jlannoy You're more than welcome, I hope we do too :) The PR got approved along with a few others recently, so we'll see what happens!

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

Successfully merging this pull request may close these issues.

Documentation of PersistService does not match its implementation
4 participants