Fix caching of OneToOneType from second level cache.

[deAtog]

This fixes issue HHH-14216. Changes are based on the ManyToOneType and make it so that when the cached Id is null, it is reliably so. This fixes an otherwise expensive query that occurs to verify the associated one-to-one entity is null when the OneToOneType Id is null.

[NathanQingyangXu]

Thanks for the PR. As a general rule, we require JIRA ticket id showing up as git commit prefix so we can correlate git commit to its ticket easily. Including it in PR title is recommended but not required.

[NathanQingyangXu]

Please come up with a compelling testing case (or suite) to demonstrate that

1. Without your code changes, the testing would fail;
2. With your code changes, the testing would succeed.

This is invaluable both in vallidation and long-term maintaining. Feature PR without testing case would be seen as incomplete work. Please see existing testing cases for example or template.

[deAtog]

Please come up with a compelling testing case (or suite) to demonstrate that

1. Without your code changes, the testing would fail;
2. With your code changes, the testing would succeed.

This is invaluable both in vallidation and long-term maintaining. Feature PR without testing case would be seen as incomplete work. Please see existing testing cases for example or template.

My apologies, the work here is still a work in progress and based on changes I'm working on in NHibernate. The NHibernate test suite has tests for these changes, but it did not have the failing test in Hibernate. I don't have a build environment for Hibernate ORM so I'm using this pull request to confirm changes. If possible, I will add tests which demonstrate the original cache issue.

[NathanQingyangXu]

Please come up with a compelling testing case (or suite) to demonstrate that

1. Without your code changes, the testing would fail;
2. With your code changes, the testing would succeed.

This is invaluable both in vallidation and long-term maintaining. Feature PR without testing case would be seen as incomplete work. Please see existing testing cases for example or template.

My apologies, the work here is still a work in progress and based on changes I'm working on in NHibernate. The NHibernate test suite has tests for these changes, but it did not have the failing test in Hibernate. I don't have a build environment for Hibernate ORM so I'm using this pull request to confirm changes. If possible, I will add tests which demonstrate the original cache issue.

No worries. A good feature of github is the DRAFT PR creation option which makes your progress transparent without the danger to have your PR merged by accident.
Testing case(s) should be mandatory for such PR. It is highly advised to create broken testing case(s) first before changing code.

[sebersole]

Overall looks great! Thanks for working on this

One minor request... we like to have newlines at the end of files and quite a few of the files here do not.

Other than that, +1 from me

[sebersole]

Thanks for adding the line endings. And thanks for the contribution - it looks great

[cristatus]

This change is breaking the L2 cache behaviour and prevents us to migrate to 5.5.x. I would suggest to fallback to old behaviour if referenced entity is not or can not be cached.

[deAtog]

@cristatus I've been using this in production with a L2 cache without issue. Can you provide a test case that demonstrates the problem you are experiencing?

[cristatus]

@deAtog sorry for the late reply but the issue is already fixed upstream #4228. For the reference and test case check https://hibernate.atlassian.net/browse/HHH-14826.

[deAtog]

@cristatus I took a look at your test case. While I agree there could be an issue, I'm not sure I agree with the cause. Your test calls ProductConfig.setProduct but not Product.setConfig. As such your in-memory objects at the time of persist are not consistent. As such, the cascade from ProductConfig to Product never occurs and your cache is incorrect. There are two ways to resolve this issue. 1. Either explicitly call Product.setConfig before calling persist() or 2. Modify ProductConfig.setProduct and Product.setConfig to call each other whenever the value changes. The patch you've provided greatly decreases performance as every null value must be verified by a database lookup.

[cristatus]

@deAtog I see. BTW, the code was working fine with earlier versions, so it should be considered as breaking change.

[NathanQingyangXu]

@deAtog , I confirmed a regression issue and created a PR here: #5061 (for https://hibernate.atlassian.net/browse/HHH-15045). Please take a look to confirm my understanding is correct. Thanks.

[raissi]

Hi,
I think because of adding the dirty check here, we have this issue: https://hibernate.atlassian.net/browse/HHH-15235
In the provided test case (see linked repo https://github.com/MKaciuba/sb267-one-to-one-property-access-exception), the uniqueKeyPropertyName is: "bar" while we are looking for the identifier (must be id), called here with current object of type Foo.
A dirty and fast solution would be to check in line 157 if this.entityName is the same as current and/or old values. Better solutions would be to fix the calling code ?