This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch 8.0.x-hibernate7 in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 4c256115fa7fe715502554990925e92298df3277 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Sun Feb 22 19:25:02 2026 -0600 CompileStatic easy wins --- grails-data-hibernate7/COMPILE-STATIC-AUDIT.md | 162 +++++++++++++++++ ...7-UPGRADE-PROGRESS.md => HIBERNATE7-BINDING.md} | 0 .../HIBERNATE7-GRAILS8-UPGRADE.md | 195 +++++++++++++++++++++ .../orm/hibernate/HibernateGormInstanceApi.groovy | 37 ++-- .../orm/hibernate/HibernateGormStaticApi.groovy | 4 +- .../generator/GrailsSequenceGeneratorEnum.groovy | 3 +- .../dirty/GrailsEntityDirtinessStrategy.groovy | 6 +- .../hibernate/support/DataSourceFactoryBean.groovy | 2 + .../GrailsSequenceGeneratorEnumSpec.groovy | 120 ++++++++----- .../DatabaseMigrationException.groovy | 3 + .../plugins/databasemigration/NoopVisitor.groovy | 2 + .../databasemigration/PluginConstants.groovy | 3 + .../liquibase/GrailsLiquibaseFactory.groovy | 2 + 13 files changed, 469 insertions(+), 70 deletions(-) diff --git a/grails-data-hibernate7/COMPILE-STATIC-AUDIT.md b/grails-data-hibernate7/COMPILE-STATIC-AUDIT.md new file mode 100644 index 0000000000..b61d845715 --- /dev/null +++ b/grails-data-hibernate7/COMPILE-STATIC-AUDIT.md @@ -0,0 +1,162 @@ +# `@CompileStatic` Eligibility Audit — `grails-data-hibernate7` + +All `.groovy` files under `src/main` across all four submodules (`core`, `boot-plugin`, `grails-plugin`, `dbmigration`). + +Legend: +- ✅ **Done** — `@CompileStatic` at class level, no workarounds needed +- ⚠️ **Partial** — class-level `@CompileStatic` present but some methods still carry `@CompileDynamic` +- ❌ **Not Eligible** — dynamic dispatch is inherent; cannot be made static + +--- + +## `core` submodule + +### Config / Model classes + +| File | Status | Notes | +|------|--------|-------| +| `cfg/CacheConfig.groovy` | ✅ Done | | +| `cfg/ColumnConfig.groovy` | ✅ Done | | +| `cfg/CompositeIdentity.groovy` | ✅ Done | | +| `cfg/DiscriminatorConfig.groovy` | ✅ Done | | +| `cfg/Identity.groovy` | ✅ Done | | +| `cfg/InstanceProxy.groovy` | ✅ Done | | +| `cfg/JoinTable.groovy` | ✅ Done | | +| `cfg/NaturalId.groovy` | ✅ Done | | +| `cfg/PropertyConfig.groovy` | ✅ Done | | +| `cfg/PropertyDefinitionDelegate.groovy` | ✅ Done | | +| `cfg/SortConfig.groovy` | ✅ Done | | +| `cfg/Table.groovy` | ✅ Done | | +| `cfg/Mapping.groovy` | ⚠️ Partial | `methodMissing` intentionally `@CompileDynamic` — DSL property dispatch hook. No change needed. | + +### DSL / Mapping builder + +| File | Status | Notes | +|------|--------|-------| +| `cfg/domainbinding/hibernate/HibernateMappingBuilder.groovy` | ✅ Done | Full `@CompileStatic` rewrite; `methodMissing` is `def` (required Groovy hook) | +| `cfg/domainbinding/hibernate/HibernateMappingFactory.groovy` | ✅ Done | | +| `cfg/domainbinding/hibernate/GrailsJpaMappingConfigurationStrategy.groovy` | ✅ Done | | +| `cfg/domainbinding/generator/GrailsSequenceGeneratorEnum.groovy` | ✅ Done | `@CompileStatic` added; spec rewritten to extend `HibernateGormDatastoreSpec` + Postgres Testcontainer (replaces `GroovyMock(global:true)`) | + +### GORM APIs + +| File | Status | Notes | +|------|--------|-------| +| `HibernateGormEnhancer.groovy` | ✅ Done | | +| `HibernateGormValidationApi.groovy` | ✅ Done | | +| `AbstractHibernateGormValidationApi.groovy` | ✅ Done | | +| `HibernateGormInstanceApi.groovy` | ✅ Done | Stale `@CompileDynamic` removed from `isDirty`, `findDirty`, `getDirtyPropertyNames`; typed with `EntityEntry`, `NonIdentifierAttribute[]`, explicit for-loops | +| `HibernateGormStaticApi.groovy` | ⚠️ Partial | `findWithSql`/`findAllWithSql` fixed. `getAllInternal` still has untyped locals — see detail below | + +### Infrastructure + +| File | Status | Notes | +|------|--------|-------| +| `GrailsHibernateTransactionManager.groovy` | ✅ Done | | +| `MetadataIntegrator.groovy` | ✅ Done | | +| `HibernateEntity.groovy` | ✅ Done | | +| `mapping/MappingBuilder.groovy` | ✅ Done | | +| `compiler/HibernateEntityTransformation.groovy` | ✅ Done | | +| `connections/HibernateConnectionSourceSettings.groovy` | ✅ Done | | +| `connections/HibernateConnectionSourceSettingsBuilder.groovy` | ✅ Done | | +| `dirty/GrailsEntityDirtinessStrategy.groovy` | ✅ Done | Stale `@CompileDynamic` removed from `getStatus`; typed `EntityEntry`, explicit null check | +| `support/HibernateDatastoreConnectionSourcesRegistrar.groovy` | ✅ Done | | +| `support/HibernateDatastoreFactoryBean.groovy` | ✅ Done | | +| `support/HibernateRuntimeUtils.groovy` | ✅ Done | | +| `support/DataSourceFactoryBean.groovy` | ✅ Done | `@CompileStatic` added | + +--- + +## `boot-plugin` submodule + +| File | Status | Notes | +|------|--------|-------| +| `HibernateGormAutoConfiguration.groovy` | ✅ Done | | +| `GormCompilerAutoConfiguration.groovy` | ✅ Done | | + +--- + +## `grails-plugin` submodule + +| File | Status | Notes | +|------|--------|-------| +| `HibernateDatastoreSpringInitializer.groovy` | ✅ Done | | +| `HibernateGrailsPlugin.groovy` | ✅ Done | | +| `commands/SchemaExportCommand.groovy` | ✅ Done | | +| `HibernateSpec.groovy` | ✅ Done | | + +--- + +## `dbmigration` submodule + +| File | Status | Notes | +|------|--------|-------| +| `DatabaseMigrationException.groovy` | ✅ Done | `@CompileStatic` added | +| `PluginConstants.groovy` | ✅ Done | `@CompileStatic` added | +| `NoopVisitor.groovy` | ✅ Done | `@CompileStatic` added | +| `DatabaseMigrationTransactionManager.groovy` | ❌ Not Eligible | Dynamic `Map` property assignment on `DefaultTransactionDefinition`. Inherently dynamic. | +| `DatabaseMigrationGrailsPlugin.groovy` | ❌ Not Eligible | Grails plugin DSL (`doWithSpring`, `doWithApplicationContext`). Inherently dynamic. | +| `EnvironmentAwareCodeGenConfig.groovy` | ⚠️ Partial | `mergeEnvironmentConfig` uses `.environments?."$environment"` (dynamic GPath). Cannot be made static. | +| `command/DatabaseMigrationCommand.groovy` | ⚠️ Partial | `createDatabase` uses `clazz.decode(password)` via dynamic dispatch on a runtime-loaded codec. Cannot be made static without a typed interface. | +| `command/ApplicationContextDatabaseMigrationCommand.groovy` | ✅ Done | | +| `command/DbmChangelogToGroovy.groovy` | ✅ Done | | +| `command/DbmCreateChangelog.groovy` | ✅ Done | | +| `command/ScriptDatabaseMigrationCommand.groovy` | ✅ Done | | +| `liquibase/GrailsLiquibaseFactory.groovy` | ✅ Done | `@CompileStatic` added | +| `liquibase/GrailsLiquibase.groovy` | ✅ Done | | +| `liquibase/ChangelogXml2Groovy.groovy` | ✅ Done | | +| `liquibase/DatabaseChangeLogBuilder.groovy` | ✅ Done | | +| `liquibase/EmbeddedJarPathHandler.groovy` | ✅ Done | | +| `liquibase/GormColumnSnapshotGenerator.groovy` | ✅ Done | | +| `liquibase/GormDatabase.groovy` | ✅ Done | | +| `liquibase/GroovyChange.groovy` | ✅ Done | | +| `liquibase/GroovyChangeLogSerializer.groovy` | ✅ Done | | +| `liquibase/GroovyDiffToChangeLogCommandStep.groovy` | ✅ Done | | +| `liquibase/GroovyGenerateChangeLogCommandStep.groovy` | ✅ Done | | +| `liquibase/GroovyPrecondition.groovy` | ✅ Done | | +| `liquibase/GroovyChangeLogParser.groovy` | ⚠️ Partial | See §GroovyChangeLogParser detail below | + +--- + +## Detailed Analysis of Remaining Partial Files + +### `HibernateGormStaticApi.groovy` — `getAllInternal` + +```groovy +@CompileDynamic +private getAllInternal(List ids) { + def identityType = ... + def identityName = ... + def idsMap = [:] + def root = cq.from(...) + ... +} +``` + +Can be fully typed: `Class identityType`, `String identityName`, `Map<Object,Object> idsMap`, `Root<?> root`. Medium effort. + +--- + +### `cfg/Mapping.groovy` + +`methodMissing` dispatches property-name calls (e.g. `firstName column: 'f_name'`) to `property(name, closure)`. Inherently dynamic — `@CompileDynamic` on this method alone is correct. No change needed. + +--- + +### `liquibase/GroovyChangeLogParser.groovy` + +| Method | Reason dynamic | Can be fixed? | +|--------|---------------|---------------| +| `parseToNode(...)` | `compilerConfiguration.metaClass.respondsTo(...)` metaClass access | **Possibly** — guard can be removed; locals can be typed | +| `setChangeLogProperties(...)` | `changeLogProperties.each { name, value -> ... }` with dynamic `value.contexts` / `value.labels` on `Object` | **Possibly** — type as `Map<String,Object>`, add `instanceof` guards | + +--- + +## Remaining Actions + +| Action | File | Effort | +|--------|------|--------| +| Type untyped locals | `getAllInternal` in `HibernateGormStaticApi` | Medium | +| Remove metaClass guard + type locals | `parseToNode` in `GroovyChangeLogParser` | Medium | +| Add `instanceof` guards for dynamic map values | `setChangeLogProperties` in `GroovyChangeLogParser` | Medium | +| Accept as permanently dynamic | `methodMissing` in `Mapping`, `DatabaseMigrationTransactionManager`, `DatabaseMigrationGrailsPlugin`, `createDatabase` in `DatabaseMigrationCommand`, `mergeEnvironmentConfig` in `EnvironmentAwareCodeGenConfig` | No action | diff --git a/grails-data-hibernate7/core/HIBERNATE7-UPGRADE-PROGRESS.md b/grails-data-hibernate7/HIBERNATE7-BINDING.md similarity index 100% rename from grails-data-hibernate7/core/HIBERNATE7-UPGRADE-PROGRESS.md rename to grails-data-hibernate7/HIBERNATE7-BINDING.md diff --git a/grails-data-hibernate7/HIBERNATE7-GRAILS8-UPGRADE.md b/grails-data-hibernate7/HIBERNATE7-GRAILS8-UPGRADE.md new file mode 100644 index 0000000000..26ceec5eeb --- /dev/null +++ b/grails-data-hibernate7/HIBERNATE7-GRAILS8-UPGRADE.md @@ -0,0 +1,195 @@ +# Hibernate 7 / Grails 8 Upgrade Issues + +Extracted from the full Grails 8 modernisation roadmap (`UPGRADE.md`), this document covers only the Hibernate-specific issues. + +--- + + +--- + +## 1. Module Consolidation: Remove `grails-data-hibernate6` + +The `8.0.x-hibernate7` branch ships three Hibernate modules. Only two should reach GA: + +| Module | Hibernate Version | Binder Architecture | Gaps | Ship in Grails 8? | +|--------|-------------------|---------------------|------|-------------------| +| `grails-data-hibernate5` | 5.6-jakarta | Monolithic `GrailsDomainBinder` (to be refactored) | None (production) | **Yes** — legacy path | +| `grails-data-hibernate7` | 7.2 | Fully decomposed `domainbinding/binder/` hierarchy + generators + entity hierarchy | None | **Yes** — modern path | + +`grails-data-hibernate7` supersedes all `grails-data-hibernate6` work — the 56 files unique to `hibernate6` are the old binder pattern that `hibernate7` replaced with 176 new files in the decomposed hierarchy. +- `hibernate6` still has known gaps (multitenancy, proxy support) that `hibernate7` resolved. +- Maintaining 3 modules instead of 2 triples the surface area for Hibernate API changes. + +**Removal plan:** +3. Verify no other modules reference `grails-data-hibernate6` in their build files. + +--- + +## 2. Binder Refactoring (Apply to Both Versions) + +The `8.0.x-hibernate7` branch decomposes the monolithic `GrailsDomainBinder` into a proper hierarchy in `grails-data-hibernate7`. **This refactoring must also be applied to `grails-data-hibernate5`.** + +**New binder hierarchy in `grails-data-hibernate7`:** +- `RootBinder`, `SubClassBinder`, `SubclassMappingBinder`, `DiscriminatorPropertyBinder` +- `ComponentBinder`, `CollectionSecondPassBinder`, `DependentKeyValueBinder` +- `UnidirectionalOneToManyBinder`, `SimpleValueColumnBinder`, `PrimaryKeyValueCreator` +- `HibernateCriteriaBuilder` refactored with `CriteriaMethodInvoker` and `CriteriaMethods` enum +- `GrailsHibernatePersistentEntity` and `GrailsHibernatePersistentProperty` hierarchy enriched +- Each binder has its own Spock spec (testable in isolation) + +Having `grails-data-hibernate5` use a monolithic `GrailsDomainBinder` while `grails-data-hibernate7` has proper binder separation would make long-term maintenance significantly harder. + +--- + +## 3. Version Selection Mechanism + +Users must be able to choose their Hibernate version at dependency resolution time without classpath conflicts. + +**Proposed approach — Maven classifier + Gradle `requireCapability`:** + +```groovy +// Option A: Hibernate 5.6-jakarta (legacy, default for migration) +implementation 'org.apache.grails:grails-data-hibernate:8.0.0:hibernate56' + +// Option B: Hibernate 7.2 (modern, recommended for new projects) +implementation 'org.apache.grails:grails-data-hibernate:8.0.0:hibernate72' +``` + +Or via Gradle feature variants: + +```groovy +dependencies { + implementation('org.apache.grails:grails-data-hibernate:8.0.0') { + requireCapability 'org.apache.grails:grails-data-hibernate-7.2' + } +} +``` + +**Goals:** +- No classpath conflicts (only one Hibernate version on the classpath at a time). +- BOM/platform support for transitive Hibernate version alignment. +- Clear migration path: start with 5.6, switch to 7.2 when ready. +- Grails Forge can offer the choice at project creation time. + +--- + +## 4. Liquibase / Database Migration Plugin Concerns + +### Hibernate 5.6 path +No license issue. The Legacy Database Migration plugin pins to Liquibase 4.27.0 and `liquibase-hibernate5`, both Apache 2.0 licensed. This continues to work as in Grails 7. + +### Hibernate 7.2 path +PR [liquibase/liquibase-hibernate#844](https://github.com/liquibase/liquibase-hibernate/pull/844) Database Migration plugin is for Hibernate 7 support to `liquibase-hibernate`, and it targets Liquibase 4.29.2, , both Apache 2.0 licensed. + + +--- + +## 5. Hibernate Entity Static Trait — Groovy 5 Joint Compilation Failure + +**Context:** `HibernateEntity` (in `grails-data-hibernate5-core`) has 5 static methods with generic return types. `HibernateMappingContext.java` in the same module directly imports the trait. Under Groovy 5, the stub generator produces invalid Java: + +```java +// Generated stub (INVALID Java) [email protected]() +static java.util.List<D> findAllWithSql(java.lang.CharSequence sql); +// ^ ERROR: non-static type variable D cannot be +// referenced from static context +``` + +In Java, a class-level type parameter (`D`) cannot be referenced from a static method. The Groovy trait works at runtime but the stub generator doesn't account for this. + +**Comparison with other traits:** + +| Trait | Module | Static Methods | Java Files Import It? | Result in Groovy 5 | +|-------|--------|----------------|-----------------------|-------------------| +| `GormEntity` | grails-datamapping-core | 87 | No | Works | +| `HibernateEntity` | grails-data-hibernate5-core | 5 | **Yes** (`HibernateMappingContext.java`) | **Fails** | +| `MongoEntity` | grails-data-mongodb | 18 | No | Works | + +**Decision options:** + +| Approach | Description | Trade-off | +|----------|-------------|-----------| +| **Convert Java class to Groovy** | Rewrite `HibernateMappingContext.java` as `.groovy` so it goes through Groovy compilation | Slower Groovy compilation | + +--- + +## 6. `HibernateMappingBuilder` — `@CompileStatic` Compatibility (Groovy 5) + +`HibernateMappingBuilder` uses `methodMissing` to handle arbitrary domain property names (e.g. `title`, `name`, `dateCreated`). Under Groovy 5, `@CompileStatic` on the class means the compiler cannot see `methodMissing`-dispatched calls as valid. Required changes: + +1. **Replace `methodMissing` for common DSL methods with explicit typed methods** — `methodMissing` remains as the generic fallback but typed DSL entrypoints must exist for the compiler. +2. **Add `@DelegatesTo` to all closure-accepting methods** — allows the Groovy 5 static type checker to validate the DSL body passed by callers. +3. **`Entity.getSort()` type resolution** — `Entity<P>` declares `Object getSort()` returning `defaultSort`; `Mapping` declares a `SortConfig sort` field. Under `@CompileStatic` in `HibernateMappingBuilder`, `mapping.getSort()` resolves to `Object`. Fix: explicit cast `(SortConfig) mapping.getSort()`. +4. **`PropertyConfig.typeParams` (`Properties` vs `Map`)** — `typeParams` is typed as `java.util.Properties`; assigning a raw `Map` fails `@CompileStatic`. Use `Properties.put(Object, Object)` (not `setProperty`, which stringifies values and breaks integer-preserving type params). +5. **`setUnique` overload resolution** — `Property.setUnique` has three overloads: `boolean`, `String` (uniqueness group), and `List<String>`. `@CompileStatic` requires explicit `instanceof` guards for each overload. +6. **`importFrom` handling** — The `importFrom` constraint keyword (used in `static constraints`) must remain handled in `methodMissing`, delegating to `ClassPropertyFetcher.getStaticPropertyValuesFromInheritanceHierarchy()`. Removing it causes `importFrom` constraints to be silently ignored at mapping time, breaking column-length DDL generation. +All done in `HibernateMappingBuilder`. + +--- + +## 7. Why Hibernate 7 Matters + +- **Jakarta Persistence 3.2** compliance (Jakarta EE 11). +- **Spring Boot 4** ships with Hibernate 7.2 as its default ORM provider. +- Hibernate 7 drops many deprecated 5.x APIs that GORM currently wraps. +- Better SQL generation, stateless session improvements, annotation processor support. + +--- + +## 8. Recommended Action for `grails-data-hibernate7` + +Do **not** drop Hibernate 5.6 support in 8.0.0. Ship both: + +| Path | Module | Status | +|------|--------|--------| +| Hibernate 5.6-jakarta | `grails-data-hibernate5` | Safe default for migration from Grails 7 | +| Hibernate 7.2 | `grails-data-hibernate7` | Modern target; recommended for new projects | + + +**Priority**: CRITICAL +**Effort**: Very High (binder refactoring must be applied to both versions; module consolidation required) + + +### 2.6 GORM Query Safety Audit + +**Current state**: GORM's HQL/`@Query` support accepts string interpolation in queries. Dynamic finders are safe by design (parameterized). But `executeQuery()`, `executeUpdate()`, and `@Query` with GString interpolation can produce SQL injection. + +**Potentially for Grails 8**: +- Add compile-time warning for GString-interpolated HQL queries (AST transform or type checking extension) +- Document safe query patterns prominently +- Consider deprecating `executeQuery(String)` in favor of parameterized-only API +- Audit all internal GORM query construction for injection vectors + +**Priority**: **P2** - SQL injection is OWASP #3. Framework should make the safe path the easy path. +**Effort**: Medium + +### 1.4 Static Compilation Coverage Expansion + +**Current state in grails-core/src/main**: 45 out of 73 Groovy files (62%) have `@CompileStatic` or `@GrailsCompileStatic`. + +Without `@CompileStatic`, every Groovy method call goes through dynamic dispatch: +- MetaClass lookup per call +- No JIT inlining +- Indy call site caching defeated by metaclass changes (ties into 1.1) +- GraalVM native image impossible for dynamically-dispatched code + +**Required work**: +1. Audit all `.groovy` files in `src/main` across every module for `@CompileStatic` eligibility + +### 2.6 GORM Query Safety Audit + +**Current state**: GORM's HQL/`@Query` support accepts string interpolation in queries. Dynamic finders are safe by design (parameterized). But `executeQuery()`, `executeUpdate()`, and `@Query` with GString interpolation can produce SQL injection. + +**Potentially for Grails 8**: +- Add compile-time warning for GString-interpolated HQL queries (AST transform or type checking extension) +- Document safe query patterns prominently +- Consider deprecating `executeQuery(String)` in favor of parameterized-only API +- Audit all internal GORM query construction for injection vectors + +**Priority**: **P2** - SQL injection is OWASP #3. Framework should make the safe path the easy path. +**Effort**: Medium + +**8.1+ scope** (requires Groovy 5 features, deeper changes): +- Replace GORM dynamic finders with AST-generated static methods +- Replace `NamedCriteriaProxy.methodMissing` with generated methods diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormInstanceApi.groovy b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormInstanceApi.groovy index e25490be93..6daf2fc332 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormInstanceApi.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormInstanceApi.groovy @@ -432,28 +432,28 @@ class HibernateGormInstanceApi<D> extends GormInstanceApi<D> { * @return true if the field is dirty */ - @CompileDynamic boolean isDirty(D instance, String fieldName) { SessionImplementor session = (SessionImplementor)sessionFactory.currentSession - def entry = findEntityEntry(instance, session) + EntityEntry entry = findEntityEntry(instance, session) if (!entry || !entry.loadedState) { return false } EntityPersister persister = entry.persister Object[] values = persister.getPropertyValues(instance) - def dirtyProperties = findDirty(persister, values, entry, instance, session) - if(dirtyProperties == null) { + int[] dirtyProperties = findDirty(persister, values, entry, instance, session) + if (dirtyProperties == null) { return false } - else { - int fieldIndex = persister.getEntityMetamodel().getProperties().findIndexOf { NonIdentifierAttribute attribute -> fieldName == attribute.name } - return fieldIndex in dirtyProperties + NonIdentifierAttribute[] props = persister.getEntityMetamodel().getProperties() + int fieldIndex = -1 + for (int i = 0; i < props.length; i++) { + if (props[i].name == fieldName) { fieldIndex = i; break } } + return fieldIndex in dirtyProperties } - @CompileDynamic // required for Hibernate 5.2 compatibility - private def findDirty(EntityPersister persister, Object[] values, EntityEntry entry, D instance, SessionImplementor session) { + private int[] findDirty(EntityPersister persister, Object[] values, EntityEntry entry, D instance, SessionImplementor session) { persister.findDirty(values, entry.loadedState, instance, session) } @@ -463,16 +463,15 @@ class HibernateGormInstanceApi<D> extends GormInstanceApi<D> { * @param instance The instance * @return true if it is dirty */ - @CompileDynamic boolean isDirty(D instance) { SessionImplementor session = (SessionImplementor)sessionFactory.currentSession - def entry = findEntityEntry(instance, session) + EntityEntry entry = findEntityEntry(instance, session) if (!entry || !entry.loadedState) { return false } EntityPersister persister = entry.persister Object[] currentState = persister.getPropertyValues(instance) - def dirtyPropertyIndexes = findDirty(persister, currentState, entry, instance, session) + int[] dirtyPropertyIndexes = findDirty(persister, currentState, entry, instance, session) return dirtyPropertyIndexes != null } @@ -482,11 +481,9 @@ class HibernateGormInstanceApi<D> extends GormInstanceApi<D> { * @param instance The instance * @return A list of property names that are dirty */ - - @CompileDynamic - List getDirtyPropertyNames(D instance) { + List<String> getDirtyPropertyNames(D instance) { SessionImplementor session = (SessionImplementor)sessionFactory.currentSession - def entry = findEntityEntry(instance, session) + EntityEntry entry = findEntityEntry(instance, session) if (!entry || !entry.loadedState) { return [] } @@ -495,9 +492,11 @@ class HibernateGormInstanceApi<D> extends GormInstanceApi<D> { Object[] currentState = persister.getPropertyValues(instance) int[] dirtyPropertyIndexes = findDirty(persister, currentState, entry, instance, session) List<String> names = [] - def entityProperties = persister.getEntityMetamodel().getProperties() - for (index in dirtyPropertyIndexes) { - names.add entityProperties[index].name + NonIdentifierAttribute[] entityProperties = persister.getEntityMetamodel().getProperties() + if (dirtyPropertyIndexes != null) { + for (int index : dirtyPropertyIndexes) { + names.add(entityProperties[index].name) + } } return names } diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy index 1ac4eba1bc..9e56b5d482 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy @@ -221,12 +221,10 @@ class HibernateGormStaticApi<D> extends GormStaticApi<D> { doListInternal(query, namedParams, [], args, false) } - @CompileDynamic // required for Hibernate 5.2 compatibility - def <D> D findWithSql(CharSequence sql, Map args = Collections.emptyMap()) { + D findWithSql(CharSequence sql, Map args = Collections.emptyMap()) { doSingleInternal(sql, [:], [], args, true) as D } - @CompileDynamic // required for Hibernate 5.2 compatibility List<D> findAllWithSql(CharSequence query, Map args = Collections.emptyMap()) { doListInternal(query, [:], [], args, true) } diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/generator/GrailsSequenceGeneratorEnum.groovy b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/generator/GrailsSequenceGeneratorEnum.groovy index b409129348..e7d8e06905 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/generator/GrailsSequenceGeneratorEnum.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/generator/GrailsSequenceGeneratorEnum.groovy @@ -1,6 +1,6 @@ package org.grails.orm.hibernate.cfg.domainbinding.generator - +import groovy.transform.CompileStatic import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment import org.hibernate.generator.Assigned import org.hibernate.generator.Generator @@ -13,6 +13,7 @@ import org.grails.orm.hibernate.cfg.Identity /** * Enum for Grails ID generator strategies. */ +@CompileStatic enum GrailsSequenceGeneratorEnum { IDENTITY("identity"), SEQUENCE("sequence"), diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/dirty/GrailsEntityDirtinessStrategy.groovy b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/dirty/GrailsEntityDirtinessStrategy.groovy index 2b546e52a8..6d13d2289b 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/dirty/GrailsEntityDirtinessStrategy.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/dirty/GrailsEntityDirtinessStrategy.groovy @@ -15,7 +15,6 @@ */ package org.grails.orm.hibernate.dirty -import groovy.transform.CompileDynamic import groovy.transform.CompileStatic import org.grails.datastore.gorm.GormEnhancer import org.grails.datastore.mapping.dirty.checking.DirtyCheckable @@ -27,6 +26,7 @@ import org.grails.datastore.mapping.model.types.Embedded import org.hibernate.CustomEntityDirtinessStrategy import org.hibernate.Hibernate import org.hibernate.Session +import org.hibernate.engine.spi.EntityEntry import org.hibernate.engine.spi.SessionImplementor import org.hibernate.engine.spi.Status import org.hibernate.persister.entity.EntityPersister @@ -140,10 +140,10 @@ class GrailsEntityDirtinessStrategy implements CustomEntityDirtinessStrategy { } } - @CompileDynamic Status getStatus(Session session, Object entity) { SessionImplementor si = (SessionImplementor) session - return si.getPersistenceContext().getEntry(entity)?.getStatus() + EntityEntry entry = si.getPersistenceContext().getEntry(entity) + return entry != null ? entry.getStatus() : null } private DirtyCheckable cast(Object entity) { diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/support/DataSourceFactoryBean.groovy b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/support/DataSourceFactoryBean.groovy index 5e312bde1a..a0612596ab 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/support/DataSourceFactoryBean.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/support/DataSourceFactoryBean.groovy @@ -1,5 +1,6 @@ package org.grails.orm.hibernate.support +import groovy.transform.CompileStatic import org.grails.orm.hibernate.AbstractHibernateDatastore import org.grails.orm.hibernate.connections.HibernateConnectionSource import org.springframework.beans.factory.FactoryBean @@ -13,6 +14,7 @@ import javax.sql.DataSource * * @author James Kleeh */ +@CompileStatic class DataSourceFactoryBean implements FactoryBean<DataSource> { AbstractHibernateDatastore datastore diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/generator/GrailsSequenceGeneratorEnumSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/generator/GrailsSequenceGeneratorEnumSpec.groovy index 571c3a8a63..6ed3c88ab1 100644 --- a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/generator/GrailsSequenceGeneratorEnumSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/generator/GrailsSequenceGeneratorEnumSpec.groovy @@ -1,68 +1,95 @@ package org.grails.orm.hibernate.cfg.domainbinding.generator +import grails.gorm.annotation.Entity +import grails.gorm.hibernate.HibernateEntity +import grails.gorm.specs.HibernateGormDatastoreSpec import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity import org.grails.orm.hibernate.cfg.Identity import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment import org.hibernate.generator.Assigned import org.hibernate.generator.GeneratorCreationContext +import org.hibernate.id.enhanced.SequenceStyleGenerator import org.hibernate.id.uuid.UuidGenerator +import org.hibernate.mapping.Column +import org.hibernate.mapping.Value +import org.hibernate.mapping.Property import org.hibernate.type.Type -import spock.lang.Specification +import org.testcontainers.DockerClientFactory +import org.testcontainers.containers.PostgreSQLContainer +import org.testcontainers.spock.Testcontainers +import spock.lang.Requires +import spock.lang.Shared import spock.lang.Unroll -class GrailsSequenceGeneratorEnumSpec extends Specification { - - Map<Class, Object> mockGenerators = [:] - - def setup() { - mockGenerators[GrailsIdentityGenerator] = Mock(GrailsIdentityGenerator) - GroovyMock(GrailsIdentityGenerator, global: true) - _ * new GrailsIdentityGenerator(*_) >> mockGenerators[GrailsIdentityGenerator] - - mockGenerators[GrailsSequenceStyleGenerator] = Mock(GrailsSequenceStyleGenerator) - GroovyMock(GrailsSequenceStyleGenerator, global: true) - _ * new GrailsSequenceStyleGenerator(*_) >> mockGenerators[GrailsSequenceStyleGenerator] - - mockGenerators[GrailsIncrementGenerator] = Mock(GrailsIncrementGenerator) - GroovyMock(GrailsIncrementGenerator, global: true) - _ * new GrailsIncrementGenerator(*_) >> mockGenerators[GrailsIncrementGenerator] - - mockGenerators[UuidGenerator] = Mock(UuidGenerator) - GroovyMock(UuidGenerator, global: true) - _ * new UuidGenerator(*_) >> mockGenerators[UuidGenerator] - - mockGenerators[Assigned] = Mock(Assigned) - GroovyMock(Assigned, global: true) - _ * new Assigned(*_) >> mockGenerators[Assigned] - - mockGenerators[GrailsTableGenerator] = Mock(GrailsTableGenerator) - GroovyMock(GrailsTableGenerator, global: true) - _ * new GrailsTableGenerator(*_) >> mockGenerators[GrailsTableGenerator] +@Testcontainers +@Requires({ + try { DockerClientFactory.instance().client(); true } catch (ignored) { false } +}) +class GrailsSequenceGeneratorEnumSpec extends HibernateGormDatastoreSpec { + + @Shared PostgreSQLContainer postgres = new PostgreSQLContainer("postgres:16") + + @Override + void setupSpec() { + manager.grailsConfig = [ + 'dataSource.url' : postgres.jdbcUrl, + 'dataSource.driverClassName' : postgres.driverClassName, + 'dataSource.username' : postgres.username, + 'dataSource.password' : postgres.password, + 'dataSource.dbCreate' : 'create-drop', + 'hibernate.dialect' : 'org.hibernate.dialect.PostgreSQLDialect', + 'hibernate.hbm2ddl.auto' : 'create', + ] + manager.addAllDomainClasses([GrailsSequenceGeneratorEnumSpecEntity]) + } - mockGenerators[GrailsNativeGenerator] = Mock(GrailsNativeGenerator) - GroovyMock(GrailsNativeGenerator, global: true) - _ * new GrailsNativeGenerator(*_) >> mockGenerators[GrailsNativeGenerator] + /** + * Build a GeneratorCreationContext stub backed by the real ServiceRegistry and Database + * from the running datastore so that sequence, table and other generators that + * call serviceRegistry.requireService(...) work without NPE. + * Column is sealed so we use a real instance; Value/Property are interfaces so we mock them. + */ + private GeneratorCreationContext buildContext() { + def db = collector.database + def column = new Column("id") + def value = Mock(Value) { + getColumns() >> [column] + } + def property = Mock(Property) { + getName() >> "id" + getValue() >> value + } + def type = Mock(Type) { + getReturnedClass() >> UUID + } + Mock(GeneratorCreationContext) { + getServiceRegistry() >> serviceRegistry + getDatabase() >> db + getProperty() >> property + getType() >> type + } } @Unroll - def "should return correct generator for #strategyName"() { + def "should dispatch #strategyName to #expectedClass"() { given: - def context = Mock(GeneratorCreationContext) - def mappedId = Mock(Identity) - def domainClass = Mock(GrailsHibernatePersistentEntity) - def jdbcEnvironment = Mock(JdbcEnvironment) - - // Setup for UuidGenerator which needs context.getType().getReturnedClass() - def type = Mock(Type) - context.getType() >> type - type.getReturnedClass() >> String + def context = buildContext() + def mappedId = Mock(Identity) { + // Explicit sequence name avoids the implicit-name path that requires TABLE property + getProperties() >> { def p = new Properties(); p.put(SequenceStyleGenerator.SEQUENCE_PARAM, "test_seq"); p } + } + def domainClass = Mock(GrailsHibernatePersistentEntity) { + getMappedForm() >> null + getJavaClass() >> GrailsSequenceGeneratorEnumSpecEntity + } + def jdbcEnvironment = serviceRegistry.requireService(JdbcEnvironment) when: def generator = GrailsSequenceGeneratorEnum.getGenerator(strategyName, context, mappedId, domainClass, jdbcEnvironment) then: - generator == mockGenerators[expectedClass] + expectedClass.isInstance(generator) where: strategyName | expectedClass @@ -77,7 +104,7 @@ class GrailsSequenceGeneratorEnumSpec extends Specification { "enhanced-table" | GrailsTableGenerator "hilo" | GrailsSequenceStyleGenerator "native" | GrailsNativeGenerator - "unknown" | GrailsNativeGenerator // Default + "unknown" | GrailsNativeGenerator } def "fromName should return correct enum"() { @@ -86,3 +113,8 @@ class GrailsSequenceGeneratorEnumSpec extends Specification { GrailsSequenceGeneratorEnum.fromName("nonexistent").isEmpty() } } + +@Entity +class GrailsSequenceGeneratorEnumSpecEntity implements HibernateEntity<GrailsSequenceGeneratorEnumSpecEntity> { + String name +} diff --git a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/DatabaseMigrationException.groovy b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/DatabaseMigrationException.groovy index f1a2399676..38630ff1fe 100644 --- a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/DatabaseMigrationException.groovy +++ b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/DatabaseMigrationException.groovy @@ -18,7 +18,10 @@ */ package org.grails.plugins.databasemigration +import groovy.transform.CompileStatic import groovy.transform.InheritConstructors +@CompileStatic + @InheritConstructors class DatabaseMigrationException extends RuntimeException {} diff --git a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/NoopVisitor.groovy b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/NoopVisitor.groovy index 09baef6d3f..cbc332fb8a 100644 --- a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/NoopVisitor.groovy +++ b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/NoopVisitor.groovy @@ -19,6 +19,7 @@ package org.grails.plugins.databasemigration +import groovy.transform.CompileStatic import liquibase.changelog.ChangeSet import liquibase.changelog.DatabaseChangeLog import liquibase.changelog.filter.ChangeSetFilterResult @@ -26,6 +27,7 @@ import liquibase.changelog.visitor.ChangeSetVisitor import liquibase.database.Database import liquibase.exception.LiquibaseException +@CompileStatic class NoopVisitor implements ChangeSetVisitor { protected Database database diff --git a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/PluginConstants.groovy b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/PluginConstants.groovy index 20f039fce0..f215a58d22 100644 --- a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/PluginConstants.groovy +++ b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/PluginConstants.groovy @@ -19,6 +19,9 @@ package org.grails.plugins.databasemigration +import groovy.transform.CompileStatic + +@CompileStatic class PluginConstants { static final String DATA_SOURCE_NAME_KEY = 'dataSourceName' diff --git a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/liquibase/GrailsLiquibaseFactory.groovy b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/liquibase/GrailsLiquibaseFactory.groovy index ee397d0134..39c69fd1ae 100644 --- a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/liquibase/GrailsLiquibaseFactory.groovy +++ b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/liquibase/GrailsLiquibaseFactory.groovy @@ -19,9 +19,11 @@ package org.grails.plugins.databasemigration.liquibase +import groovy.transform.CompileStatic import org.springframework.beans.factory.config.AbstractFactoryBean import org.springframework.context.ApplicationContext +@CompileStatic class GrailsLiquibaseFactory extends AbstractFactoryBean<GrailsLiquibase> { private final ApplicationContext applicationContext
