Skip to content

Add specs and clean up transactions.transform/transform in grails-datamapping-core - #16166

Open
borinquenkid wants to merge 1 commit into
8.1.xfrom
chore/8.1.x-datamapping-core-transform-cleanup
Open

Add specs and clean up transactions.transform/transform in grails-datamapping-core#16166
borinquenkid wants to merge 1 commit into
8.1.xfrom
chore/8.1.x-datamapping-core-transform-cleanup

Conversation

@borinquenkid

Copy link
Copy Markdown
Member

Summary

  • Closes direct unit-spec coverage gaps in org.grails.datastore.gorm.transactions.transform and org.grails.datastore.gorm.transform left after Add test coverage and fix compiler crashes in GORM AST transforms #16148: RollbackTransform, AbstractDatastoreMethodDecoratingTransformation, AbstractMethodDecoratingTransformation, AstMethodDispatchUtils, and AstPropertyResolveUtils.
  • Writing the AstPropertyResolveUtils spec surfaced a real correctness bug: its property cache was keyed by class name (a String), so two distinct ClassNode instances sharing a name (e.g. from separate compilations of dynamically-generated/test classes) silently corrupted each other's cached property data under concurrent use. Fixed by keying the cache on ClassNode identity via a synchronized IdentityHashMap.
  • Extracted the duplicated applied-marker idempotency check/mark pattern (repeated across AbstractGormASTTransformation, AbstractMethodDecoratingTransformation, and AbstractDatastoreMethodDecoratingTransformation) into shared isAlreadyApplied/markApplied helpers.
  • Small cleanups flagged by static analysis: equals() calls replaced with ==, an unused method parameter removed, Java 21 instanceof pattern variables replacing raw-type casts, String#isEmpty() over length()==0, Class#getDeclaredConstructor().newInstance() over the deprecated Class#newInstance(), and two stray doc/comment fixes.

Test plan

  • ./gradlew :grails-datamapping-core:test (full module suite)
  • ./gradlew :grails-datamapping-core:codeStyle :grails-datamapping-core:codenarcMain :grails-datamapping-core:codenarcTest
  • New specs directly verify the AstPropertyResolveUtils cache-corruption fix, including under concurrent access

🤖 Generated with Claude Code

…amapping-core

Closes direct unit-spec coverage gaps left after PR #16148 for
RollbackTransform, AbstractDatastoreMethodDecoratingTransformation,
AbstractMethodDecoratingTransformation, AstMethodDispatchUtils, and
AstPropertyResolveUtils. Writing the AstPropertyResolveUtils spec
surfaced a real correctness bug: its property cache was keyed by
class name (a String), so two distinct ClassNode instances sharing a
name (e.g. from separate compilations of dynamically-generated/test
classes) silently corrupted each other's cached property data under
concurrent use. Fixed by keying the cache on ClassNode identity via a
synchronized IdentityHashMap.

Also extracts the duplicated applied-marker idempotency check/mark
pattern (repeated across AbstractGormASTTransformation,
AbstractMethodDecoratingTransformation, and
AbstractDatastoreMethodDecoratingTransformation) into shared
isAlreadyApplied/markApplied helpers, and applies a batch of small
cleanups flagged by static analysis: equals() calls replaced with ==,
an unused method parameter removed, Java 21 instanceof pattern
variables replacing raw-type casts, String#isEmpty() over
length()==0, Class#getDeclaredConstructor().newInstance() over the
deprecated Class#newInstance(), and two stray doc/comment fixes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 18, 2026 17:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.4265%. Comparing base (75cd618) to head (1720abd).

Files with missing lines Patch % Lines
...astore/gorm/transform/AstPropertyResolveUtils.java 80.9524% 0 Missing and 4 partials ⚠️
...orm/transform/AbstractGormASTTransformation.groovy 85.7143% 0 Missing and 1 partial ⚠️
...form/AbstractMethodDecoratingTransformation.groovy 50.0000% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##                8.1.x     #16166        +/-   ##
==================================================
+ Coverage     53.4149%   53.4265%   +0.0116%     
- Complexity      19459      19469        +10     
==================================================
  Files            2081       2081                
  Lines           98993      98994         +1     
  Branches        17361      17359         -2     
==================================================
+ Hits            52877      52889        +12     
+ Misses          38566      38560         -6     
+ Partials         7550       7545         -5     
Files with missing lines Coverage Δ
...ansactions/transform/TransactionalTransform.groovy 92.5000% <100.0000%> (ø)
...ractDatastoreMethodDecoratingTransformation.groovy 92.2078% <100.0000%> (+2.4642%) ⬆️
.../AbstractTraitApplyingGormASTTransformation.groovy 88.0952% <ø> (ø)
...re/gorm/transform/OrderedGormTransformation.groovy 88.8889% <100.0000%> (ø)
...orm/transform/AbstractGormASTTransformation.groovy 83.3333% <85.7143%> (+4.7619%) ⬆️
...form/AbstractMethodDecoratingTransformation.groovy 91.9355% <50.0000%> (+2.3355%) ⬆️
...astore/gorm/transform/AstPropertyResolveUtils.java 80.7229% <80.9524%> (-1.4200%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@testlens-app

testlens-app Bot commented Aug 18, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 1720abd
▶️ Tests: 63691 executed
⚪️ Checks: 77/77 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants