Don't validate if the only change is to a @Version attribute (e.g. em.find() with PESSIMISTIC_FORCE_INCREMENT) #2265
Conversation
Signed-off-by: Jonathan Gallimore <[email protected]>
|
Hi - just wondering if I can nudge this? If there's anything that this needs to get it over the line, please do let me know, I'm very happy to receive any feedback. Equally, if its a "no", let me know, I'll understand :-) |
| @@ -22,6 +22,7 @@ public BeanValidationTableCreator() { | |||
| setName("BeanValidationEmployeeProject"); | |||
There was a problem hiding this comment.
Please update year in the copyright header into Copyright (c) 2009, 2024 Oracle......
It's about all updated files *.java, persistence.xml
| @@ -0,0 +1,62 @@ | |||
| /* | |||
| * Copyright (c) 2009, 2022 Oracle and/or its affiliates. All rights reserved. | |||
There was a problem hiding this comment.
For the new files correct one is Copyright (c) 2024 Oracle and/or.....
| public class Task { | ||
|
|
||
| @Id | ||
| private int id; |
|
|
||
| commitTransaction(em); | ||
|
|
||
| Vector resultSet = getDatabaseSession().executeSQL("select * from CMP3_BV_TASK where ID=900"); |
There was a problem hiding this comment.
What about Vector<DatabaseRecord>? We prefer this style instead of generic collections.
| task.setPriority(2); | ||
|
|
||
| commitTransaction(em); | ||
| } catch (ConstraintViolationException e) { |
There was a problem hiding this comment.
Multiple catch expressions? I think, that this one should split into two separated tests. Each one should cover mentioned exception. In this form is unclear what is exactly tested.
* Copyright headers * Formatting/spacing * Remove exception block from test case and attempt to make the test case clearer Signed-off-by: Jonathan Gallimore <[email protected]>
|
Hi @rfelcman , thank you so much for the review and feedback. I've pushed another commit, which hopefully addresses the feedback. Does this need to be squashed into a single commit? I'm very happy to do that. On the behavior change - I completely understand. From my own perspective, it would cool to backport it - would a feature flag be something that could enable that? No problem if not, I'm just thinking out loud. Thanks again for the review. If there's other feedback I'm happy to rework things further. |
I wonder if I could propose this change. I'm very happy to incorporate any feedback. Thank you in advance for your reviews and consideration.
A call like:
Task task = em.find(Task.class, 900, LockModeType.PESSIMISTIC_FORCE_INCREMENT);will increment the
@Versionattribute of the entity, and this will be persisted when the transaction completes. The entity will also be validated with Bean Validation at this point, even though there are no other changes to the object. If the data read from the database happens to be invalid according to the bean validation constraints, the validation will fail, and the transaction will be rolled back.I can't see a reference in the specification as to whether the entity should be validated in this instance, but the behaviour of Eclipselink appears to differ from Hibernate and OpenJPA, which will only attempt to validate the object if a change has has been made.