Re-enable grails-datamapping-rx module by borinquenkid · Pull Request #16135 · apache/grails-core

borinquenkid · 2026-08-11T16:10:38Z

Summary

Restores the grails-datamapping-rx module to the active build (was disabled in both settings.gradle and gradle/publish-root-config.gradle pending a port to the current Grails release)
Fixes 7 real bugs uncovered while re-enabling it:
- RxSchedule.java: an @interface illegally declared a default annotation member with a method body instead of a constant, and referenced a removed GroovyTransformOrder constant
- grails.gorm.rx.DetachedCriteria.clone() illegally narrowed visibility from its public parent
- RxEntity.groovy was missing import rx.Observable
- RxScheduleIOTransformation.groovy referenced the same stale GroovyTransformOrder constant
- FindByFinder/FindAllByFinder (rx) called a 3-arg buildQuery(...) overload that no longer exists after the finders' Groovy->Java conversion; inlined the steps the current 2-arg buildQuery performs internally
Brings the module's Checkstyle/CodeNarc compliance up to date with the rest of the codebase (it had never been linted while disabled)

Test plan

:grails-datamapping-rx:compileGroovy — BUILD SUCCESSFUL
:grails-datamapping-rx:test — 8/8 passing
:grails-datamapping-rx:codeStyle — Checkstyle + CodeNarc both clean

🤖 Generated with Claude Code

@interface

The module was disabled in settings.gradle (and gradle/publish-root-config.gradle, which independently gated GrailsPublishExtension registration) pending a port to the current Grails release. This restores it to the active build: - Fix RxSchedule.java: an @interface illegally declared a `default` annotation member with a method body instead of a constant default value, and referenced a removed GroovyTransformOrder constant (RX_SCHEDULE -> RX_SCHEDULER_ORDER). - Fix grails.gorm.rx.DetachedCriteria.clone(): illegally narrowed visibility from the inherited public AbstractDetachedCriteria.clone(). - Fix RxEntity.groovy: missing `import rx.Observable`. - Fix RxScheduleIOTransformation.groovy: same stale GroovyTransformOrder constant reference (RX_SCHEDULE_IO_ORDER -> RX_SCHEDULER_ORDER). - Fix FindByFinder/FindAllByFinder (rx): both called a 3-arg buildQuery(...) overload that no longer exists after the finders' Groovy->Java conversion; inlined the query-building steps (applyAdditionalCriteria/applyDetachedCriteria/ configureQueryWithArguments/getJunction) that the current 2-arg buildQuery performs internally, using the query the finder already creates via RxDatastoreClient instead of a Session. - Bring the module's Checkstyle and CodeNarc compliance up to date with the rest of the codebase (it had never been checked while disabled). Compiles, and its test suite passes (8/8). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Re-enables the grails-datamapping-rx module in the Gradle build/publish configuration and updates the module’s source to compile against the current Grails/Groovy APIs while meeting current linting rules.

Changes:

Re-added grails-datamapping-rx to settings.gradle and root publishing configuration.
Updated AST transformation + RxGORM code to align with current Graovy/Grails constants and finder/query building APIs.
Applied broad Checkstyle/CodeNarc-driven formatting/import cleanups across the module.

Reviewed changes

Copilot reviewed 50 out of 56 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
settings.gradle	Re-includes `grails-datamapping-rx` in the multi-project build.
gradle/publish-root-config.gradle	Re-enables publishing for `grails-datamapping-rx`.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/transform/RxScheduleIOTransformation.groovy	Updates transformation ordering constant + formatting/import cleanup.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/transform/RxAstUtils.groovy	Formatting/lint updates; string literal normalization.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/services/support/RxServiceSupport.groovy	Removes unused import to satisfy linting.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/services/implementers/SingleResultAdapter.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/services/implementers/ObservableServiceImplementerAdapter.groovy	Removes unused import + formatting.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/services/implementers/ObservableResultAdapter.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindOrSaveByFinder.groovy	Formatting/lint updates (semicolon removal).
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindOrCreateByFinder.groovy	Removes unused import + formatting around Rx chain/subscribers.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindByFinder.groovy	Replaces removed `buildQuery(...)` overload by inlining query configuration steps.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindByBooleanFinder.groovy	Formatting/lint updates; simplifies visibility/semicolons.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindAllByFinder.groovy	Mirrors `FindByFinder` query configuration changes for “findAllBy”.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindAllByBooleanFinder.groovy	Formatting/lint updates; simplifies visibility/semicolons.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/CountByFinder.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/ValidationEventListener.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/MultiTenantEventListener.groovy	Updates assignability checks + formatting; tweaks exception message formatting.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/DomainEventListener.groovy	Formatting/lint updates (semicolons removed).
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/AutoTimestampEventListener.groovy	Formatting/lint updates (semicolons removed).
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/config/Settings.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/api/multitenancy/TenantDelegatingRxGormOperations.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/api/RxGormValidationApi.groovy	Formatting/lint updates; whitespace and argument cleanup.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/api/RxGormStaticApi.groovy	Formatting/lint updates; string literal normalization & whitespace fixes.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/api/RxGormInstanceApi.groovy	Removes unused import.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/api/RxGormEnhancer.groovy	Formatting/lint updates; minor string literal changes.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/query/RxQueryUtils.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/query/RxQuery.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/query/QueryState.groovy	Removes redundant empty constructor; formatting changes.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/proxy/RxJavassistProxyFactory.java	Import ordering + formatting updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/proxy/QueryObservableProxyMethodHandler.java	Import ordering + formatting updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/proxy/ProxyFactory.java	Import ordering.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/proxy/IdentifierObservableProxyMethodHandler.java	Removes unused import + import ordering/formatting.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/proxy/IdQueryObservableProxyMethodHandler.java	Import ordering + formatting updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/proxy/AbstractObservableProxyMethodHandler.java	Import ordering + formatting updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/internal/RxDatastoreClientImplementor.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/collection/RxPersistentSortedSet.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/collection/RxPersistentSet.groovy	Removes unused imports + formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/collection/RxPersistentList.groovy	Removes unused imports + formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/collection/RxCollectionUtils.groovy	Formatting/lint updates (switch formatting).
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/collection/RxCollection.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/batch/BatchOperation.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/RxDatastoreClientAware.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/RxDatastoreClient.groovy	Removes unused import + formatting/lint updates.
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/AbstractRxDatastoreClient.groovy	Replaces wildcard event imports + formatting/lint updates.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/services/RxSchedule.java	Fixes invalid annotation default member; updates transform order constant usage.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/multitenancy/Tenants.groovy	Formatting/lint updates; string literal normalization.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/collection/RxPersistentCollection.java	Removes unused imports.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/collection/ObservableCollection.java	Import ordering + formatting/lint updates.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/api/RxGormStaticOperations.groovy	Formatting/lint updates (whitespace cleanup).
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/api/RxGormOperations.groovy	Removes unused import + formatting/lint updates.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/api/RxGormInstanceOperations.groovy	Formatting/lint updates (whitespace cleanup).
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/api/RxGormAllOperations.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/RxEntity.groovy	Adds missing `rx.Observable` import; formatting/lint updates.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/MultiTenant.groovy	Formatting/lint updates.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/DetachedCriteria.groovy	Fixes `clone()` visibility; formatting updates.
grails-datamapping-rx/src/main/groovy/grails/gorm/rx/CriteriaBuilder.groovy	Formatting/lint updates (semicolons/whitespace).

Suppressed comments (5)

grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindOrCreateByFinder.groovy:1

The onError(Throwable e) handler signals onCompleted() instead of propagating the error. This will silently swallow persistence failures and may leave downstream subscribers believing the create/save succeeded. Change onError to call s.onError(e) (or explicitly handle/transform the error if the intended behavior is to treat errors as empty).
grails-datamapping-rx/src/main/groovy/org/grails/datastore/rx/query/QueryState.groovy:48
This is a check-then-act update on a ConcurrentHashMap. Under concurrency, two threads can both see null, create separate maps, and the later put(type, loadedByType) can overwrite the earlier one (losing entries). Use an atomic initialization pattern (e.g., computeIfAbsent / putIfAbsent) to ensure per-type maps are created once and updates aren’t dropped.

    void addLoadedEntity(Class type, Serializable id, Object object) {
        def loadedByType = loadedEntities.get(type)
        if (loadedByType == null) {
            loadedByType = new ConcurrentHashMap<Serializable, Object>()
            loadedByType.put(id, object)
            loadedEntities.put(type, loadedByType)
        }
        else {
            loadedByType.put(id, object)
        }
    }

grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/api/RxGormEnhancer.groovy:1

This containsKey + put sequence is non-atomic and can race under concurrent initialization. Prefer using putIfAbsent (or computeIfAbsent if you need to derive the value) so only the first initializer wins without redundant writes.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindByFinder.groovy:1
This inlined query-configuration sequence is duplicated (also appears in the Rx FindAllByFinder). To reduce drift risk if these steps change again, consider extracting a small shared helper (e.g., configureQueryFromInvocation(invocation, javaClass, query)) or delegating to a single overridden method in a common base for the Rx finders.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/MultiTenantEventListener.groovy:1
The error message has a grammatical typo: use 'Could not assign tenant id …' instead of 'Could not assigned tenant id …'. Consider also simplifying punctuation near 'id!:' to keep the message readable.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

codecov · 2026-08-11T17:13:38Z

Codecov Report

❌ Patch coverage is 11.06557% with 217 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.3608%. Comparing base (4b7c10d) to head (131b769).

Files with missing lines	Patch %	Lines
...ails/datastore/rx/AbstractRxDatastoreClient.groovy	0.0000%	36 Missing ⚠️
...roovy/org/grails/gorm/rx/api/RxGormEnhancer.groovy	0.0000%	17 Missing ⚠️
...oovy/org/grails/gorm/rx/api/RxGormStaticApi.groovy	0.0000%	17 Missing ⚠️
.../org/grails/datastore/rx/query/RxQueryUtils.groovy	0.0000%	15 Missing ⚠️
...ils/gorm/rx/events/MultiTenantEventListener.groovy	0.0000%	15 Missing ⚠️
...ls/datastore/rx/proxy/RxJavassistProxyFactory.java	0.0000%	12 Missing ⚠️
.../rx/proxy/IdQueryObservableProxyMethodHandler.java	0.0000%	8 Missing ⚠️
...grails/gorm/rx/finders/FindOrCreateByFinder.groovy	0.0000%	8 Missing ⚠️
.../main/groovy/grails/gorm/rx/CriteriaBuilder.groovy	0.0000%	7 Missing ⚠️
...rvices/implementers/ObservableResultAdapter.groovy	46.1538%	1 Missing and 6 partials ⚠️
... and 22 more

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.1.x     #16135        +/-   ##
==================================================
- Coverage     51.9816%   51.3608%   -0.6209%     
- Complexity      18035      18145       +110     
==================================================
  Files            2038       2079        +41     
  Lines           95705      97189      +1484     
  Branches        16648      16871       +223     
==================================================
+ Hits            49749      49917       +168     
- Misses          38619      39915      +1296     
- Partials         7337       7357        +20

Files with missing lines	Coverage Δ
...-rx/src/main/groovy/grails/gorm/rx/RxEntity.groovy	`0.0000% <ø> (ø)`
...rg/grails/datastore/rx/batch/BatchOperation.groovy	`0.0000% <ø> (ø)`
...vy/org/grails/gorm/rx/api/RxGormInstanceApi.groovy	`0.0000% <ø> (ø)`
...ltitenancy/TenantDelegatingRxGormOperations.groovy	`0.0000% <ø> (ø)`
...vy/org/grails/gorm/rx/finders/CountByFinder.groovy	`0.0000% <ø> (ø)`
...menters/ObservableServiceImplementerAdapter.groovy	`100.0000% <100.0000%> (ø)`
...s/gorm/rx/services/support/RxServiceSupport.groovy	`40.0000% <ø> (ø)`
.../org/grails/gorm/rx/api/RxGormValidationApi.groovy	`0.0000% <0.0000%> (ø)`
...ails/gorm/rx/finders/FindAllByBooleanFinder.groovy	`0.0000% <0.0000%> (ø)`
.../grails/gorm/rx/finders/FindByBooleanFinder.groovy	`0.0000% <0.0000%> (ø)`
... and 29 more

... and 13 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 · 2026-08-11T19:49:10Z

✅ All tests passed ✅

🏷️ Commit: 131b769
▶️ Tests: 57740 executed
⚪️ Checks: 61/61 completed

Learn more about TestLens at testlens.app.

Copilot AI lite review requested due to automatic review settings August 11, 2026 16:10

github-project-automation Bot added this to Apache Grails Aug 11, 2026

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Copilot started reviewing on behalf of borinquenkid August 11, 2026 17:03 View session

Navigation Menu

Uh oh!

Uh oh!

Re-enable grails-datamapping-rx module - #16135

Re-enable grails-datamapping-rx module#16135
borinquenkid wants to merge 1 commit into
8.1.xfrom
feat/enable-datamapping-rx

borinquenkid commented Aug 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

codecov Bot commented Aug 11, 2026

Uh oh!

testlens-app Bot commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

borinquenkid commented Aug 11, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov Bot commented Aug 11, 2026

Codecov Report

Uh oh!

testlens-app Bot commented Aug 11, 2026

✅ All tests passed ✅

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants