-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
feat: flush with Panache #2899
feat: flush with Panache #2899
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, but it needs some tests to illustrate the use-case by throwing exceptions, I think.
@FroMage I added an example of |
ff7a047
to
f4b33c1
Compare
I meant tests, not just docs :) |
@FroMage just to be clear, there is no /src/test/java directory on the Panache Hibernate extension so by test you mean adding the usage of the flush and persistAndFlush methods on the hibernate-orm-panache integration-test ? And ideally with an exception ? I will try to do this on Monday. |
Provide flush method on Panache entity and Panache repository. Also provide a shorthand method persistAndFlush as it is the most common use case.
f4b33c1
to
f971d60
Compare
Add integration test for persistAndFliush methods on both PanacheEntity and PanacheRepository.
@FroMage I added an integration test for the flushAndPersist() method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment about doc
Ok
…On Tue 25 Jun 2019 at 09:38, Loïc Mathieu ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/src/main/asciidoc/hibernate-orm-panache-guide.adoc
<#2899 (comment)>:
> @@ -381,6 +381,8 @@ Make sure to wrap methods modifying your database (e.g. `entity.persist()`) with
CDI bean method ***@***.***` will do that for you and make that method a transaction boundary. We recommend doing
so at your application entry point boundaries like your REST endpoint controllers.
+Be aware that JPA may send the changes you done on your entities only at transaction boundary, this means that those changes (insert/update/delete queries) will be sent to the database after the execution of your method so you will not be able to catch any `PersistenceException` in your code if you need to (for example to send an event somewhere if you cannot commit to the database). Panache provides `flush()` and `persistAndFlush()` methods to force a flush on the underlying `EntityManager`. Calling one of these methods will send all pending changes (of all entities) to the database (but will not commit the transaction).
I think it's important to keep the reference to the fact that with this we
can catch PesistenceException.
@emmanuelbernard <https://github.com/emmanuelbernard> I would propose
something like this:
JPA batches changes you make to entity and sends changes (it's called
flush) at the end of the transaction or before a query.
This is usually a good thing as it's more efficient.
But if you want to check optimistic locking failures, do object validation
right away or generally want to get immediate feedback, you can force the
flush operation by calling entity.flush() or even use
entity.persistAndFlush() to make it a single method call. This will allow
you to catch any PersistenceException that could occurs when JPA send
those changes to the database.
Remember, this is less efficient so don't abuse it.
And your transaction still has to be committed.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2899?email_source=notifications&email_token=AACJNWAQMUILCCDRJNIIL5DP4HDODA5CNFSM4HZP3QI2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB4QPY6Y#discussion_r297045760>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACJNWHUNMWS6ZOLYLDBNF3P4HDODANCNFSM4HZP3QIQ>
.
|
@emmanuelbernard @FroMage I updated the documentation part as requested. |
Thanks for the test! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small doc tweak requested, but otherwise this is OK to go IMO.
35c38c8
to
8f18bf3
Compare
@emmanuelbernard can you re-review please? |
Thank you @loicmathieu ! |
Provide flush method on Panache entity and Panache repository.
Also provide a shorthand method persistAndFlush as it is the most common use case.
I add some documentation to the transaction section of Panache Hibernate.
Fixes #2755