ali-jawad opened a new issue, #16740:
URL: https://github.com/apache/grails-core/issues/16740
### Summary
With GORM for Hibernate 5 in Grails 8.0.0, `save()` no longer sets the
back-reference of a new child in a bidirectional one-to-one where the child
`belongsTo` the owner. On 7.2.4 it did. The owner can be new or already
persisted. Depending on the mapping, the child is then saved with a null
in-memory back-reference, its insert fails (`hasOne`), or validation rejects it
(a non-nullable back-reference).
### Versions
- Broken: Grails 8.0.0, `org.apache.grails:grails-hibernate5-bom:8.0.0` →
`org.apache.grails.data:grails-data-hibernate5-core:8.0.0`, Hibernate ORM
5.6.15.Final (`hibernate-core-jakarta`)
- Works: Grails 7.2.4,
`org.apache.grails.data:grails-data-hibernate5-core:7.2.4`, the same Hibernate
ORM 5.6.15.Final
- The change first shipped in 8.0.0-M2. The `7.2.x` branch does not have it.
### Steps to reproduce
Use an app on GORM for Hibernate 5 with H2 on the test runtime classpath,
and add this spec to `src/test/groovy`:
```groovy
import grails.gorm.annotation.Entity
import grails.test.hibernate.HibernateSpec
class OneToOneBackReferenceSpec extends HibernateSpec {
List<Class> getDomainClasses() { [Author, Profile] }
void "saving the owner sets the back-reference of its new one-to-one
child"() {
given:
Profile profile = new Profile(bio: 'Writes about GORM')
Author author = new Author(name: 'Jane', profile: profile)
when:
author.save(flush: true)
then:
profile.author.is(author)
!author.hasErrors()
Profile.count() == 1
}
}
@Entity
class Author {
String name
Profile profile
static constraints = {
profile nullable: true
}
}
@Entity
class Profile {
String bio
static belongsTo = [author: Author]
}
```
### Expected
`save()` sets `profile.author` to `author`, as on 7.2.4, and both rows are
inserted.
### Actual
`profile.author` stays `null` after `save()`. What follows depends on the
mapping:
- **The shape above, with Grails 8's defaults** (properties nullable):
`save()` succeeds and both rows are stored with the correct foreign key. Only
the in-memory back-reference stays `null`, so `profile.author.is(author)` fails.
- **`static hasOne = [profile: Profile]`** (foreign key in the child): the
flush fails with `DataIntegrityViolationException`, NULL not allowed for column
`AUTHOR_ID`, on the insert into the child's table. The same happens for a
persisted owner with a new child, and for `hasOne` without `belongsTo`.
- **With `grails.gorm.default.nullable = false`** (or `nullable: false` on
the back-reference): the cascaded validation rejects the null `author`, so
`save()` returns `null`, and `failOnError: true` throws a `ValidationException`.
Inside a Grails application, the map constructor and `bindData` set the
back-reference through data binding, which hides the bug there. Plain
assignment (`author.profile = profile`) shows it.
### Cause
`save()` calls `HibernateRuntimeUtils.autoAssociateBidirectionalOneToOnes()`
before validation:
https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/AbstractHibernateGormInstanceApi.groovy#L130
For each owning-side bidirectional one-to-one, it skips the association
unless `proxyHandler.isInitialized(child, '<backReference>')` is true, and then
sets the back-reference if it is null. This method is the same in 7.2.4
([L125-L134](https://github.com/apache/grails-core/blob/ea4ddc768f362a77fceb926ced14491c18524ca3/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/support/HibernateRuntimeUtils.groovy#L125-L134))
and in 8.0.0:
https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/support/HibernateRuntimeUtils.groovy#L125-L134
`HibernateProxyHandler.isInitialized(Object, String)` reads the property and
passes its value to `isInitialized(Object)`. For a new child, that value is
`null`:
- **7.2.4:** `isInitialized(Object)` delegates to
`Hibernate.isInitialized(null)` ([handler
L47-L50](https://github.com/apache/grails-core/blob/ea4ddc768f362a77fceb926ced14491c18524ca3/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler.java#L47-L50)),
and that call returns `true` for `null`, which matches none of its
`instanceof` branches and reaches the final `else` ([Hibernate 5.6.15
L84-L101](https://github.com/hibernate/hibernate-orm/blob/e924c27e1259b0b5915819e9521d86fcb8164a46/hibernate-core/src/main/java/org/hibernate/Hibernate.java#L84-L101)).
- **8.0.0:** `isInitialized(Object)` returns `false` for `null`:
https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler.java#L54-L73
The `null` guard came in with 70fde10c8257d029d79beef9f02446cd9376ac74
during the Hibernate 7 work, which reached `8.0.x` through #15568.
`HibernateProxyHandler5Spec` pins it
([L132-L135](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler5Spec.groovy#L132-L135)).
The guard is meant to skip an uninitialized proxy, but it now also skips the
one case the method acts on: a null back-reference.
No test caught it, because the one-to-one specs set the back-reference by
hand. Examples are the TCK `OneToOneSpec`
([L73-L74](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/OneToOneSpec.groovy#L73-L74)),
`BidirectionalOneToOneWithUniqueSpec` and `UniqueWithHasOneSpec`.
The same branch also handles `static hasOne = [profile: Profile]`, because
`ToOne.isOwningSide()` is true when the foreign key is in the child. Both
shapes reproduce (see *Actual*).
### Suggested fix
In `autoAssociateBidirectionalOneToOnes`, treat a null back-reference as
nothing to initialize. The Hibernate 7 module's copy of this method already has
no such guard
([L96-L125](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/support/HibernateRuntimeUtils.groovy#L96-L125)),
and #16624 makes the same change to `MappingContextTraversableResolver` ("a
null value has nothing to initialize"):
```diff
def otherSidePropertyName = otherSide.getName()
- if (!proxyHandler.isInitialized(inverseObject,
otherSidePropertyName)) {
- continue
- }
-
def associationReflector =
mappingContext.getEntityReflector(association.associatedEntity)
def propertyValue =
associationReflector.getProperty(inverseObject, otherSidePropertyName)
if (propertyValue == null) {
```
A non-null back-reference is never overwritten, initialized or not, so the
guard only ever changed the outcome for `null`. The earlier
`isInitialized(target, propertyName)` check still keeps an uninitialized child
proxy from being loaded.
Another option is to restore the 7.x contract, `isInitialized(null) ==
true`, which `GroovyProxyFactory.isInitialized(Object, String)` still follows
([L153-L156](https://github.com/apache/grails-core/blob/081cd1c93ab753a234e111e2828f089ebdd810ee/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy#L153-L156)).
That reverses behaviour a spec now pins, so the fix in the caller looks
narrower. Either way, a spec that saves the owner without setting the
back-reference would guard against this regression.
### Workaround
Set the back-reference before saving the owner:
```groovy
profile.author = author
author.profile = profile
author.save()
```
A fix with tests is ready: it removes that guard, adds a spec covering both
shapes, subclasses, proxies and an owner that is already set, and passes the
full hibernate5 core suite including the TCK. I can open a PR once this is
acknowledged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
