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 detection of entity state #3604

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Sep 5, 2024

Hibernate use 0 or user provided positive initial value as seed of integer version.
EclipseLink always use 1 as seed of integer version.

Hibernate use 0 or user provided positive initial value as seed of integer version.
EclipseLink always use 1 as seed of integer version.
@@ -219,13 +218,37 @@ public Object getCompositeIdAttributeValue(Object id, String idAttribute) {
@Override
public boolean isNew(T entity) {

if (versionAttribute.isEmpty()
|| versionAttribute.map(Attribute::getJavaType).map(Class::isPrimitive).orElse(false)) {
if (versionAttribute.isEmpty()) {
return super.isNew(entity);
}

BeanWrapper wrapper = new DirectFieldAccessFallbackBeanWrapper(entity);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work when entity is a proxy?

I'd like an integration test, that ensures it has a proxy an based on that tests the isNew method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why it's related to proxy, could you expand?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entity might be a Hibernate proxy. And I basically don't trust any code to properly work with those. Just the normal paranoia after working with JPA to much.

Copy link
Contributor Author

@quaff quaff Sep 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not expert of proxy, as far as I know, proxy only take care of accessing lazy attributes and associations, version is not in that case, and this PR only reuse the existing code wrapper.getPropertyValue(it.getName()) to get value of version to refine the logic.

@mp911de mp911de force-pushed the main branch 2 times, most recently from 52ee55f to 61e6d36 Compare October 9, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants