jdaugherty commented on code in PR #16686: URL: https://github.com/apache/grails-core/pull/16686#discussion_r4366680396
########## grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/MultipleUnidirectionalHasManySpec.groovy: ########## @@ -0,0 +1,196 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.gorm.tests + +import grails.gorm.annotation.Entity + +/** + * An owner with several unidirectional hasMany associations keeps a join table for each of them, whether they point + * at different classes or at the same class with join tables of their own, and an element added to one of them is + * written to its own join table. A sibling to-many that is not bound as a collection, such as a serializable one, + * neither blocks the session factory nor counts as a sharer of the join table. + */ +class MultipleUnidirectionalHasManySpec extends HibernateGormDatastoreSpec { + + void setupSpec() { + manager.registerDomainClasses(MuhItem, MuhRole, MuhTag, MuhAccount, MuhNote, MuhSharedAccount) + } + + void cleanup() { + MuhItem.withNewTransaction { + MuhItem.executeUpdate('delete from MuhItem', [:]) + MuhAccount.executeUpdate('delete from MuhAccount', [:]) + MuhSharedAccount.executeUpdate('delete from MuhSharedAccount', [:]) + MuhNote.executeUpdate('delete from MuhNote', [:]) + MuhRole.executeUpdate('delete from MuhRole', [:]) + MuhTag.executeUpdate('delete from MuhTag', [:]) + } + } + + void "an element added to one of two unidirectional hasMany associations is written to its join table"() { + given: "a persisted item with two unidirectional hasMany associations to different classes" + def item = new MuhItem(name: 'Example item').save(flush: true) + + when: "a role is added to the managed item and the item is flushed" + item.addToRoles(new MuhRole(authority: 'ROLE_EXAMPLE')) + item.save(flush: true) + MuhItem.withSession { it.clear() } + + then: "the role is found through the reloaded item" + MuhItem.get(item.id).roles*.authority == ['ROLE_EXAMPLE'] + MuhItem.get(item.id).tags.isEmpty() + } + + void "elements added to both unidirectional hasMany associations are written to their own join tables"() { + given: + def item = new MuhItem(name: 'Example item').save(flush: true) + + when: + item.addToRoles(new MuhRole(authority: 'ROLE_A')) + item.addToTags(new MuhTag(name: 'tag-a')) + item.save(flush: true) + MuhItem.withSession { it.clear() } + + then: + MuhItem.get(item.id).roles*.authority == ['ROLE_A'] + MuhItem.get(item.id).tags*.name == ['tag-a'] + } + + void "two unidirectional hasMany associations to the same class with their own join tables are each written"() { + given: "a persisted account whose allow and deny roles map their own join tables" + def account = new MuhAccount(name: 'Example account').save(flush: true) + + when: "a role is added to each association and the account is flushed" + account.addToRoles(new MuhRole(authority: 'ROLE_ALLOW')) + account.addToDenyRoles(new MuhRole(authority: 'ROLE_DENY')) + account.save(flush: true) + MuhAccount.withSession { it.clear() } + + then: "each role is found through its own association of the reloaded account" + MuhAccount.get(account.id).roles*.authority == ['ROLE_ALLOW'] + MuhAccount.get(account.id).denyRoles*.authority == ['ROLE_DENY'] + } + + void "two unidirectional hasMany associations sharing one join table keep their rows across a flush of the owner"() { + given: "a persisted account whose allow and deny roles share one join table, with a row for each in that table" + def account = new MuhSharedAccount(name: 'Shared account').save(flush: true) + def allow = new MuhRole(authority: 'ROLE_ALLOW').save(flush: true) + def deny = new MuhRole(authority: 'ROLE_DENY').save(flush: true) + MuhSharedAccount.withSession { session -> + session.createNativeMutationQuery( + "insert into muh_shared_account_roles (account_id, role_id, deny_role_id) values (${account.id}, ${allow.id}, null)".toString() + ).executeUpdate() + session.createNativeMutationQuery( + "insert into muh_shared_account_roles (account_id, role_id, deny_role_id) values (${account.id}, null, ${deny.id})".toString() + ).executeUpdate() + session.clear() + } + + expect: "each association of the reloaded account reads the row with its own key column" + MuhSharedAccount.get(account.id).roles*.authority == ['ROLE_ALLOW'] + MuhSharedAccount.get(account.id).denyRoles*.authority == ['ROLE_DENY'] + + when: "the account changes, gains a deny role and is flushed, then the session is cleared" + MuhSharedAccount account1 = MuhSharedAccount.get(account.id) + account1.name = 'Renamed account' + account1.addToDenyRoles(new MuhRole(authority: 'ROLE_DENY_2')) + account1.save(flush: true) + MuhSharedAccount.withSession { it.clear() } + + then: "the shared table keeps both rows: the non-updatable key stops one association from deleting or nulling the other's" + sharedRows() == [[account.id, allow.id, null], [account.id, null, deny.id]] + MuhSharedAccount.get(account.id).roles*.authority == ['ROLE_ALLOW'] + + and: "an element added to an association sharing its join table is not written, which is issue 10811 and not changed here" + MuhSharedAccount.get(account.id).denyRoles*.authority == ['ROLE_DENY'] + } + + void "a unidirectional hasMany next to a serializable to-many is written to its join table"() { + given: "a persisted note whose tags are a serializable column, not a collection" + def note = new MuhNote(name: 'Example note', tags: ['tag-a', 'tag-b'] as Set).save(flush: true) + + when: "a role is added to the managed note and the note is flushed" + note.addToRoles(new MuhRole(authority: 'ROLE_NOTE')) + note.save(flush: true) + MuhNote.withSession { it.clear() } + + then: "the role is found through the reloaded note and the serializable tags are intact" + MuhNote.get(note.id).roles*.authority == ['ROLE_NOTE'] + MuhNote.get(note.id).tags == ['tag-a', 'tag-b'] as Set + } + private static List<List> sharedRows() { + MuhSharedAccount.withSession { session -> + session.createNativeQuery( + 'select account_id, role_id, deny_role_id from muh_shared_account_roles order by role_id nulls last', Object[] + ).list()*.toList() + } + } +} + +@Entity +class MuhItem { + Long id + String name + static hasMany = [roles: MuhRole, tags: MuhTag] +} + +@Entity +class MuhRole { + Long id + String authority +} + +@Entity +class MuhTag { + Long id + String name +} + +@Entity +class MuhAccount { + Long id + String name + static hasMany = [roles: MuhRole, denyRoles: MuhRole] + static mapping = { + roles joinTable: 'muh_account_allow_roles' + denyRoles joinTable: 'muh_account_deny_roles' + } +} + +@Entity +class MuhNote { + Long id + String name + Set<String> tags + static hasMany = [roles: MuhRole, tags: String] + static mapping = { + tags type: 'serializable' + } +} + +@Entity +class MuhSharedAccount { + Long id + String name + static hasMany = [roles: MuhRole, denyRoles: MuhRole] + static mapping = { + roles joinTable: [name: 'muh_shared_account_roles', key: 'account_id', column: 'role_id'] + denyRoles joinTable: [name: 'muh_shared_account_roles', key: 'account_id', column: 'deny_role_id'] Review Comment: This fixture is the mirror of the #10811 shape. #10811, and `CKCUSharedTableOwner` in the updater spec, are two `hasMany` to the same class with no mapping: they share the default join table, which has one element column and a key column per association. On this branch `static hasMany = [roles: Role, denyRoles: Role]` creates `item_role` with `role_id`, `item_roles_id` and `item_deny_roles_id`, all nullable. Here both associations share the key column `account_id` and have an element column each, so the labels do not match what runs: line 105 says each association reads the row with its own key column, but they read it with their own element column, and the upgrade note's "with a key column each" describes the other shape. The non-updatable key exists for the default shape, where a row of the second collection leaves the first key column null. Could the fixture drop the `joinTable` mapping so the round trip is #10811 verbatim? The table is then `muh_shared_account_muh_role` with `muh_role_id`, `muh_shared_account_roles_id` and `muh_shared_account_deny_roles_id`, the two inserted rows become `(allow, account, null)` and `(deny, null, account)`, and the `expect:` label is true as written. It is also the shape #16701 changes, so its assertion flip lands on the same fixture. One more label, line 116: the non-updatable key "stops one association from deleting or nulling the other's" rows, but this flush adds an element to a loaded collection without replacing it, so no delete runs; with an updatable key the result would be a third row, not a lost one. "the flush writes nothing to the shared table" is what the assertion shows. ########## grails-doc/src/en/guide/upgrading/upgrading80x.adoc: ########## @@ -1456,6 +1456,12 @@ Hibernate 7 changed the `org.hibernate.usertype.UserType` contract. The interfac A type that spans several columns no longer implements `UserType` with several `sqlTypes()`. It implements `org.hibernate.usertype.CompositeUserType`, whose `embeddable()` method names a class whose persistent attributes describe the columns. GORM binds such a property as a component: the columns of its `type` mapping are assigned to the attributes in the alphabetical order of their names, which is also the index that `getPropertyValue()` and `instantiate()` use, and a column that is not listed is named after the property and the attribute. In Hibernate 5, the columns followed the order of `getPropertyNames()` instead, so check the column order of every multi-column mapping when you port a `CompositeUserType`. The GORM guide describes the mapping under _Custom Hibernate Types_. +===== 26.12 Several Unidirectional hasMany Associations on One Owner + +Under Hibernate 7, an owner with two or more unidirectional `hasMany` associations wrote no rows to their join tables: an element added to any of them was lost at flush (issue https://github.com/apache/grails-core/issues/16665[#16665]). This is fixed when each association has a join table of its own, which is the default (`item_roles` and `item_tags` for `static hasMany = [roles: Role, tags: Tag]`) and also the case when two associations to the same class map different tables with `joinTable`. Each such join table is written, as under Hibernate 5. + +Two unidirectional associations mapped onto one shared join table, with a key column each, are read but their new elements are still not written. That is issue https://github.com/apache/grails-core/issues/10811[#10811] and is not fixed by this change; map a table per association until it is. Review Comment: A reader with `static hasMany = [roles: Role, denyRoles: Role]` and no mapping is in this case and cannot tell from the note: two associations to the same class share one join table by default, so for them "the default" in the previous sentence is the unfixed shape. Could the note say when the sharing happens? Something like: > Two unidirectional associations to the same class share one join table by default, named after the owner and the element class, with a key column per association (`item_role` with `item_roles_id` and `item_deny_roles_id` for `static hasMany = [roles: Role, denyRoles: Role]`). Such a table is read, but new elements are not written. That is issue #10811 and is not fixed by this change; name a `joinTable` per association to give each its own table. Those are the table and columns this branch creates for that mapping on H2, all nullable. ########## grails-data-hibernate7/docs/src/docs/asciidoc/domainClasses/gormAssociation/oneToMany.adoc: ########## @@ -62,6 +62,8 @@ class Book { The join table name defaults to `author_books` and can be customised — see xref:ormdsl-tableAndColumnNames[Table and Column Names]. +An owner may declare several unidirectional one-to-many associations, such as `static hasMany = [roles: Role, tags: Tag]`. Each one is written to its own join table, including two associations to the same class that map different tables with `joinTable`. Two associations mapped onto one shared join table, with a key column each, are read but their new elements are not written (issue https://github.com/apache/grails-core/issues/10811[#10811]); give each association a table of its own. Review Comment: Same point as on the upgrade note: two associations to the same class share one join table by default (`item_role` with `item_roles_id` and `item_deny_roles_id` for `static hasMany = [roles: Role, denyRoles: Role]`), which is the shape the last sentence describes. Saying so here tells the reader with that mapping that "give each association a table of its own" applies to them, and that `joinTable` is how. -- 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]
