borinquenkid commented on code in PR #15568:
URL: https://github.com/apache/grails-core/pull/15568#discussion_r3462898856
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/SubclassMultipleListCollectionSpec.groovy:
##########
@@ -41,9 +41,8 @@ class SubclassMultipleListCollectionSpec extends
Specification {
transactionManager = hibernateDatastore.getTransactionManager()
}
- @Ignore // not yet implemented
@Rollback
- @Issue('https://github.com/apache/grails-data-mapping/issues/882')
+ @Issue('https://github.com/grails/grails-data-mapping/issues/882')
Review Comment:
Agreed on the redirect — `grails-data-mapping#882` removed, `#14624` is now
the only reference. The class-level `@Ignore` intentionally stays: the test
fails at `setupSpec` with `MappingException: Repeated column in mapping ...
column: iteration_id`, and #14624 is still open and not addressed in this PR.
Resolving on that basis.
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CompositeIdWithJoinTableSpec.groovy:
##########
@@ -19,45 +19,40 @@
package grails.gorm.tests
-import static grails.gorm.hibernate.mapping.MappingBuilder.define
-
import grails.gorm.annotation.Entity
-import grails.gorm.transactions.Rollback
-import org.grails.orm.hibernate.HibernateDatastore
-import org.springframework.transaction.PlatformTransactionManager
-import spock.lang.AutoCleanup
-import spock.lang.Shared
-import spock.lang.Specification
+
+import static grails.gorm.hibernate.mapping.MappingBuilder.define
/**
* Created by graemerocher on 26/01/2017.
*/
-class CompositeIdWithJoinTableSpec extends Specification {
-
- @AutoCleanup @Shared HibernateDatastore datastore = new
HibernateDatastore(CompositeIdParent, CompositeIdChild)
- @Shared PlatformTransactionManager transactionManager =
datastore.transactionManager
+class CompositeIdWithJoinTableSpec extends HibernateGormDatastoreSpec {
+ def setupSpec() {
+ manager.registerDomainClasses(CompositeIdParent, CompositeIdChild)
+ }
- @Rollback
+ // @Rollback
void "test composite id with join table"() {
- when:"A parent with a composite id and a join table is saved"
- new CompositeIdParent(name: "Test" , last:"Test 2")
- .addToChildren(new CompositeIdChild())
- .save(flush:true)
+ when: "A parent with a composite id and a join table is saved"
+ new CompositeIdParent(name: "Test", last: "Test 2")
+ .addToChildren(new CompositeIdChild(foo: "bar"))
+ .save(flush: true)
- then:"The entity was saved"
+ then: "The entity was saved"
CompositeIdParent.count() == 1
CompositeIdParent.list().first().children.size() == 1
}
}
@Entity
-class CompositeIdParent implements Serializable {
+class CompositeIdParent implements Serializable, Comparable<CompositeIdParent>
{
Review Comment:
Correcting the record first: an earlier reply here said the
`SortedSet`/`Comparable` were reverted — that is wrong, they are still in the
file and intentionally so. The rationale: `children` is a
`SortedSet<CompositeIdChild>`, which requires its elements to be `Comparable`,
and `CompositeIdParent` implements `Comparable` for the same reason on its
side. This is a deliberate modeling choice — deterministic ordering instead of
the default `HashSet` — applied symmetrically to both the H5 and H7 copies of
this spec so the two modules stay in lockstep (they are now identical from the
package declaration down). To be upfront: the assertion only checks `.size() ==
1`, so this is not required for the test to pass; it is a
consistency/best-practice choice for composite-id entities in sorted
collections. On that basis I would prefer to keep it rather than re-diverge the
modules. Resolving — happy to reopen if you feel it should come out.
--
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]
