Step 1 (PR-C): Add addAllDomainClasses helper to GrailsDataTckManager#15683
Conversation
…cross TCK specs Introduces a small helper method on the TCK base manager that replaces the slightly awkward `manager.domainClasses.addAll([...])` pattern with `manager.addAllDomainClasses([...])`. The implementation is one line that delegates to the existing list mutation. Mechanically replaces all 181 callers across: - grails-datamapping-tck/src/main/... (13 TCK base specs + 1 manager method definition) - grails-datamapping-core-test/src/test/... (49 core test specs) - grails-data-hibernate5/core/src/test/... (27 hibernate5 specs) - grails-data-mongodb/core/src/test/... (92 mongo specs) 182 files changed total: the helper method definition plus 181 call sites. The previous form required the caller to know that `domainClasses` was a mutable list on the manager, which leaked an implementation detail. The helper hides the field access and makes the call site read more naturally. This change was originally pulled forward into the hibernate7 staging branch (PR #15654 commit ed0916d) and was flagged by reviewers as unrelated to the Hibernate 7 clone. Extracting it here so it can land on 8.0.x on its own merits as a prerequisite for the Hibernate 7 work. Once merged here, the corresponding "Pull forward manager.addAllDomainClasses change" commit on the hibernate7 staging branch should be removed since it will arrive through the next merge of 8.0.x. Assisted-by: claude-code:claude-4.7-opus
There was a problem hiding this comment.
Pull request overview
Adds a small convenience API to GrailsDataTckManager for registering domain classes in TCK-based specs, then mechanically updates specs across the data-mapping, Hibernate 5, and MongoDB test suites to use the helper instead of mutating manager.domainClasses directly.
Changes:
- Add
GrailsDataTckManager#addAllDomainClasses(Collection<Class>)helper. - Replace
manager.domainClasses.addAll(...)withmanager.addAllDomainClasses(...)across affected specs.
Reviewed changes
Copilot reviewed 182 out of 182 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/base/GrailsDataTckManager.groovy | Add addAllDomainClasses(Collection<Class>) helper wrapping domainClasses.addAll(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/BuiltinUniqueConstraintWorksWithTargetProxiesConstraintsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/ConstraintsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/DirtyCheckingAfterListenerSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/DirtyCheckingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/DisableAutotimeStampSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/FirstAndLastMethodSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/PersistenceEventListenerSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/PropertyComparisonQuerySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/ProxyInitializationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/QueryEventsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/UniqueConstraintSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/UpdateWithProxyPresentSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/WhereLazySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/AbstractNonGormParentClassSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/CircularCascadeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/DetachedCriteriaAssociationQuerySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/TransactionalTransformOnServiceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/WhereMethodSpec.groovy | Switch to manager.addAllDomainClasses(...) (including list variable case). |
| grails-datamapping-core-test/src/test/groovy/grails/gorm/tests/validation/ArrayMaxSizeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/AddToAndInjectedServiceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/AddToMethodWithBasicCollectionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/AddToMethodWithEmbeddedCollectionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/AssignedIdentifierSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/AutoLinkOneToManyAssociationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/BasicTypeHasManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/BeforeUpdateEventSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/BidirectionalOneToManyWithInheritanceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CacheAndJoinSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CircularManyToManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CircularManyToOneSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CircularOneToManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CompositeIdentifierSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CriteriaProjectedResultsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomSequenceIdentifierSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomStringIdentifierSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomTypeMarshallingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/DeindexingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/DetachedCriteriaJpaEntitySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/DirtyCheckingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/DomainWithPrimitiveGetterSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/DynamicFinderHungarianNotationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/EmbeddedAssociationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/EmbeddedNonEntityAssociationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/EnumHasManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/FindByDomainInListSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/GormDirtyCheckingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/HasManyDefaultMappedBySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/HasOneSetInverseSideSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/InOperatorWithAssociationsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/InheritanceWithOneToManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/ListOrderByHungarianNotationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/ManyToManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/MappedByNoneSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/NestedAssociationQuerySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/NotNullQuerySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/QueryNonIndexedPropertySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/ReadOnlyCriteriaResultsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/SaveWithFailOnErrorDefaultSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/UUIDTypeIdentifierSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/UUIIdentifierSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-datamapping-core-test/src/test/groovy/org/grails/datastore/mapping/EntityReflectorSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/hibernate/mapping/HibernateOptimisticLockingStyleMappingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/AutoTimestampSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CascadeToBidirectionalAsssociationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CountByWithEmbeddedSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/DeleteAllWhereSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/DetachCriteriaSubquerySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/DetachedCriteriaJoinSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/DomainGetterSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/EnumMappingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/LastUpdateWithDynamicUpdateSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/ManyToOneSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/MultiColumnUniqueConstraintSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/RLikeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/ReadOperationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/SizeConstraintSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/ToOneProxySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/WhereQueryWithAssociationSortSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/WithNewSessionAndExistingTransactionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/autoimport/AutoImportSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/belongsto/BidirectionalOneToOneWithUniqueSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/inheritance/SubclassToOneProxySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/inheritance/TablePerConcreteClassAndDateCreatedSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/inheritance/TablePerConcreteClassImportedSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/mappedby/MultipleOneToOneSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/proxy/ByteBuddyProxySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/traits/InterfacePropertySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/DeepValidationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/tests/DirtyCheckEmbeddedCollectionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/tests/FindNativeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/grails/gorm/tests/listener/PersistenceEventListenerSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/grails/mongodb/cascade/MongoCascadeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/AggregateMethodSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/AssignedIdentifierSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/AutowireServicesSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BasicArraySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BasicCollectionTypeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BasicCollectionsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BatchUpdateDeleteSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BeforeInsertUpdateSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BeforeUpdatePropertyPersistenceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BigDecimalSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BrokenManyToManyAssociationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CascadeDeleteOneToOneSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CascadeDeleteSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CircularBidirectionalOneToManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CircularEmbeddedListSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CircularOneToManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ClearCollectionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CustomCollectionAndAttributeMappingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CustomIdProxySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CustomMongoEventListenerSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/CustomTypeMarshallingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DBObjectConversionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DbRefWithEmbeddedSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DefaultSortOrderSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DirtyCheckUpdateSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DisableVersionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DistinctPropertySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/DocumentMappingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedAssociationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedBiDirectionalSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedCollectionAndInheritanceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedCollectionWithIdSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedCollectionWithOneToOneSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedListWithCustomTypeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedMapSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedSetAssignedIdSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedSimpleObjectSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedStringListInsideEmbeddedCollectionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedUnsetSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWhereClauseSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithCustomFieldMappingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithNonEmbeddedAssociationsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithNonEmbeddedCollectionsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EmbeddedWithinEmbeddedAssociationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EnumCollectionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EnumTypeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/EventsWithAbstractInheritanceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/FindOrCreateWhereSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GeoJSONTypePersistenceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GeospacialQuerySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GetAllWithStringIdSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/GreaterThanAndLessThanCriteriaSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/HasOneSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/IndexAttributesAndCompoundKeySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/IndexWithInheritanceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/InheritanceQueryingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/InheritanceWithSingleEndedAssociationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/InnerEnumSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/IsNullSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/JakartaValidationSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/LastUpdatedSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ListOneToManyOrderingSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MapOfDomainsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MarkDirtyFalseSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MongoDynamicPropertyOnEmbeddedSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MongoGormEnhancerSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/MongoTypesSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/NegationEnumSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/NullsAreNotStoredSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ObjectIdPersistenceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ObjectIdPropertySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/OneToManyWithInheritanceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/OneToOneNoReferenceSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/OptimisticLockingWithExceptionSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ProjectionsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/QueriesWithIdenticallyNamedPartsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/ReadManyObjectsSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SetRetrievalSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SimpleHasManySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SingleEmbeddedAssignNullToNonNullSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/StatelessSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/SwitchDatabaseAtRuntimeSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/TestSearchSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/TransientPropertySpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/WhereQueryInCriteriaSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/WriteConcernSpec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/bugs/GPMongoDB295Spec.groovy | Switch to manager.addAllDomainClasses(...). |
| grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/bugs/StringIdWithObjectIdStorageSpec.groovy | Switch to manager.addAllDomainClasses(...). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This commit reverts the portions of this branch that have been split out into standalone PRs against 8.0.x. The goal is to shrink the PR-A review surface to focus on the actual hibernate5 -> hibernate7 clone and let the unrelated cleanups be reviewed on their own merits. Once PRs B/C/D/E land on 8.0.x and 8.0.x is merged into this branch, the reverted changes will arrive through the merge so the final state of stage-hibernate7 is unchanged - only the diff visible on this PR is reduced. Reverted content: Async defensive cleanup (PR #15682, PR-B) 9 files in grails-async/. Reverts e3cad41. Null-safety guards, @OverRide annotations, catch(Exception ignored) over catch(Throwable), constant-on-left equality, varargs. addAllDomainClasses helper + adoption (PR #15683, PR-C) Helper method on GrailsDataTckManager plus all callers across the data mapping test suites. Reverts ed0916d plus all orphan callers added by subsequent commits (50 specs) so the branch compiles without the helper method. MongoDatastoreSpec base class + mongo package rename (PR #15685, PR-E) Reverts c8cb43f (MongoDatastoreSpec introduction + ~107 spec refactors) and the mongo portion of 01c27f9 (8 file renames grails.gorm.tests -> grails.gorm.specs plus 18 import-update modifications in non-renamed mongo specs). DetachedCriteriaSpec command-chain syntax (PR #15684, PR-D) 18 `eq(...)` and `like(...)` parens restorations in the TCK DetachedCriteriaSpec. Reverts the parens hunk of ee4ab14 while leaving the rest of that commit (setupSpec addition, import reorder, and the other 35 TCK file changes) intact. NOT reverted (intentionally kept in PR-A): - The hibernate5 -> hibernate7 baseline clone (~100k lines, the actual work being reviewed) - 89 hibernate5 and 90 hibernate7 grails.gorm.tests -> grails.gorm.specs package renames (non-mongo portion of 01c27f9, since these touch the cloned tests and reverting would require re-doing both sides) - 18 grails-datamapping-core-test package renames (non-mongo portion of 01c27f9) - All other "Pull forward..." commits (styling, build config, test additions, etc.) which the reviewers have not specifically objected to Assisted-by: claude-code:claude-4.7-opus
16 mongo specs in this PR called manager.addAllDomainClasses(...), a helper method that does not exist on 8.0.x. The helper is introduced by a sibling PR (#15683, the GrailsDataTckManager helper extraction) that this PR does not depend on. The setupSpec calls were inherited from the staging branch where both changes already coexisted. On 8.0.x in isolation, the helper method is undefined and the specs would throw MissingMethodException at runtime. Converting all 16 callers to the existing manager.domainClasses.addAll pattern so this PR is self-contained against 8.0.x. After both this PR and #15683 land, the callers can be optionally converted to use the helper in a follow-up (or naturally via the next merge of 8.0.x into the staging branch), but that is not a prerequisite for this PR. Files updated: - DisjunctionQuerySpec.groovy - EmbeddedHasManyWithBeforeUpdateSpec.groovy - EmbeddedWithIdSpecifiedSpec.groovy - GetAllSpec.groovy - HintQueryArgumentSpec.groovy - InListQuerySpec.groovy - LikeQuerySpec.groovy - MongoResultsListIndexSpec.groovy - NegateInListSpec.groovy - NullifyPropertySpec.groovy - OneToOneIntegritySpec.groovy - OrderWithPaginationSpec.groovy - ReadConcernArgumentSpec.groovy - ResultsWithGroovyCollectionMethodsSpec.groovy - SchemalessSpec.groovy - SessionCachingSpec.groovy Verified locally with ./gradlew :grails-data-mongodb-core:compileTestGroovy. Assisted-by: claude-code:claude-4.7-opus
matrei
left a comment
There was a problem hiding this comment.
I am against this change in its current form.
It changes/adds nothing of value and you can still access manager.domainClasses.addAll() or manager.domainClasses += [Book].
For reference, see HibernateValidationSpec.
|
We need to update the code to make the domain classes list immutable. Domain binding should only be done once in a lifecycle of an application. |
sbglasius
left a comment
There was a problem hiding this comment.
Clear and easy to review. If it has value for the downstream implementation of Hibernate 7, go for it.
|
I believe the original point of this change was to ensure a consistent list of classes (centralize into one location), although we didn't make the base list access private. I think that would resolve the concern @matrei has? |
Can that be done in this PR to demonstrate the need for these changes? |
Commit ba235da reverted the addAllDomainClasses helper from GrailsDataTckManager and its callers, but six grails-data-mongodb specs were missed, leaving them calling a method that no longer exists. This caused initializationError failures in :grails-data-mongodb-core:test (MissingMethodException: GrailsDataMongoTckManager.addAllDomainClasses). Restore these specs to use manager.domainClasses.addAll([...]) (identical to the 8.0.x baseline and behaviorally equivalent to the removed helper), so the branch compiles without the helper as intended. The helper and its callers will return together when PR-C (#15683) lands on 8.0.x and is merged back. Assisted-by: claude-code:claude-4.8-opus
|
Take a look at GrailsDataTckManager in the end branch. The following changes would also need to be done as part of this: There is also a lot more missing from this PR - all of the tests should add the domains they want at the start in setupSpec(). The point of this change was to have 1 spot where we pick the domains that are required for tests. |
- Modify GrailsDataTckManager to use an empty private list for domainClasses - Expose an unmodifiable view via getDomainClasses() - Change all TCK tests that modified manager.domainClasses directly to use the addAllDomainClasses helper
@jdaugherty Done |
| * | ||
| * @param classes The classes to register | ||
| */ | ||
| void registerDomainClasses(Class... classes) { |
There was a problem hiding this comment.
This method is currently unused. Why are we not using it?
There was a problem hiding this comment.
If we rename addAllDomainClasses in this repository, downstream callers will not automatically update, and it will cause compile-time failures for several reasons:
- External Repository Breakages: GORM implementations and plugins that reside in external repositories (such as Neo4j, Cassandra, or custom third-party datastores) depend on grails-datamapping-tck
as a published library. If addAllDomainClasses is renamed or deleted, their builds will break immediately upon upgrading because they still call the old method. - IntelliJ Groovy Refactoring Limits: Even within the same monorepo, IntelliJ's "Rename Refactoring" tool is notoriously unreliable for Groovy. Because of Groovy's dynamic nature, the IDE frequently
misses dynamic call sites or uses of the method in dynamic test specs, requiring a lot of manual search-and-replace anyway.
By keeping addAllDomainClasses as a @deprecated wrapper delegation to registerDomainClasses , we preserve backward compatibility for downstream/external TCK consumers while indicating that they
should move to the new API.
There was a problem hiding this comment.
- How can external repositories break by removing a method that was added in this PR, and thus does not exist outside this PR?
- If that is the case, was it not same when changing from
manager.domainClasses.addAll()tomanager.addAllDomainClasses()?
There was a problem hiding this comment.
Not external repositoriess you are right, we would have to change rename the methods over 250 times and IDE does not do that well
There was a problem hiding this comment.
Not external repositoriess you are right, we would have to change rename the methods over 250 times and IDE does not do that well
This is not a problem, I'd be happy to do it.
There was a problem hiding this comment.
We should just rename the method.
There was a problem hiding this comment.
I used AI to do it , change the name and change the args and change all the callers
Explicitly register required domain classes in the setupSpec method of each specification that depends on them, rather than relying on a default pre-populated registry.
b4f3d9c to
0714a59
Compare
|
@borinquenkid why are we force pushing to these changes? It makes it harder to review them. Can you let us know what was changed? |
@jdaugherty Essentially manager.domainClasses.addAll` --> manager.addAllDomainClassesover 250 times I forced pushed because when one does a change that break 100s of tests on a private branch and you ar enot sure which change made it break and you know that before the change everyhting was working and it is a private branch you reset hard and add them one at a time until you find the culprit and then you force push. |
…gSpec ( OptLockVersioned , OptLockNotVersioned ) and HibernateValidationSpec ( TestEntity , ChildEntity ) to fix the GORM
initialization failures on Hibernate 5.
2. Fixed CodeNarc violations in the TCK project:
• Fixed ConsecutiveBlankLines and Indentation issues in GrailsDataTckManager.groovy .
• Removed an UnusedImport ( Simples ) in QueryEventsSpec.groovy .
…rate calls to varargs
jdaugherty
left a comment
There was a problem hiding this comment.
I think there are some usages of the old method still, once fixed I think we can merge if @matrei agrees.
| } | ||
| }) | ||
| mappingContext.addPersistentEntities(allClasses as Class[]) | ||
| mappingContext.addPersistentEntities(domainClasses as Class[]) |
There was a problem hiding this comment.
I think you still want to store a reference to the classes prior to the creation of the context.
There was a problem hiding this comment.
this is really a codesmell from subclassing breaking encapsulation. the correct structure passed around should be an immutable collection. But I really want to get out of this branch, the amount of churn here and what we got out of it is too big IMHO.
matrei
left a comment
There was a problem hiding this comment.
We don't need to cast the result from getDomainClasses() to Class[] anymore as that is what is returned.
Thank you @borinquenkid!
✅ All tests passed ✅🏷️ Commit: 1170851 Learn more about TestLens at testlens.app. |
…tree PR-C (#15683) renamed the TCK helper addAllDomainClasses to registerDomainClasses and migrated callers to varargs. It made GrailsDataTckManager.domainClasses private, so the old manager.domainClasses.addAll([...]) form no longer compiles. PR-C landed on 8.0.x but could not touch the grails-data-hibernate7 clone (it does not exist on 8.0.x), so merging 8.0.x back left the h7 core test tree broken. This mirrors PR-C's API migration across the 27 affected h7 specs: manager.domainClasses.addAll([A, B]) -> manager.registerDomainClasses(A, B). Only the registration call changes; the registered classes are unchanged. Verified: ./gradlew :grails-data-hibernate7-core:compileTestGroovy passes. Assisted-by: claude-code:claude-opus-4-8
Step 1 (PR-C) prerequisite for Hibernate 7 work - extracted from the staging branch so it can be reviewed on its own merits.
Context
This change was originally pulled forward into the hibernate7 staging branch as commit
ed0916dd93on PR #15654. Reviewers (@matrei, @sbglasius) correctly pointed out it is unrelated to the Hibernate 7 clone and should be a standalone PR. Extracting it here against8.0.xso it can land on its own and then flow naturally into both the staging branch (PR #15654) and Step 2 (PR #15568).Scope
Adds a small helper method to
GrailsDataTckManagerand mechanically replaces all callers:The helper hides the implementation detail that
domainClassesis a mutable list field, and reads more naturally at the call site.Files changed
182 files total - the method definition + 181 call sites:
grails-datamapping-tck/src/main/...grails-datamapping-core-test/src/test/...grails-data-hibernate5/core/src/test/...grails-data-mongodb/core/src/test/...The change is purely mechanical - I verified a sample of files diff identically to commit
ed0916dd93on the staging branch.Why a separate PR
PR #15654 (Step 1) is meant to be a near-pure clone of
hibernate5→hibernate7. PR #15568 (Step 2) is the actual Hibernate 7 logic. Both reviewers asked for cleanup of this nature to be split out so the review effort on Step 1 and Step 2 can stay focused on hibernate-related diffs.Once this PR is merged, the corresponding "Pull forward manager.addAllDomainClasses change" commit on the hibernate7 staging branch should be removed since the change will arrive through the next merge of
8.0.x.Related
logback.groovyand the unrelatedstepByStep.adocchange)Note on MongoDatastoreSpec base class
The companion change on the staging branch (commit
c8cb43f2ec- introducingMongoDatastoreSpecas a thin base class to reduce generic noise across 108 mongo specs) was not included in this PR because it depends on a separatetests/→specs/package rename that hasn't been applied to8.0.x. That refactor can be a follow-up PR or stay in the staging branch since @sbglasius's review comment on it was conditional ("if needed, ok").