borinquenkid commented on PR #16135:
URL: https://github.com/apache/grails-core/pull/16135#issuecomment-5284147972
Thanks for the review, @jdaugherty — addressed all three points:
1. **`grails-data-docs/guide-rx`** — re-included in `settings.gradle` (also
fixed a stale `projectDir` path left over from a prior restructuring).
`:grails-data-docs-guide-rx:asciidoctor` builds clean. (1d8f3e7)
2. **`@author` tags** — removed from all 56 touched files under
`grails-datamapping-rx/src/main`. (1d8f3e7)
3. **Are the tests completely new? Yes** — and since you mentioned the
commented-out tests were disabled over unresolved failures, I went back and
root-caused them instead of leaving them commented:
- Restored the `grails-datamapping-rx` test dependency and un-commented
`RxServiceImplSpec` in `grails-datamapping-core-test`.
- Root-caused the underlying `GroovyCastException` bug:
`SingleResultAdapter`/`ObservableResultAdapter` never actually narrowed the
generated method's declared return type before delegating to the wrapped
implementer, so the automatic `@Transactional` decoration built its wrapping
cast against the Rx-wrapped type (e.g. `Single<Number>`) instead of the type
the synchronous body actually returns — an always-throwing cast. (9eb7887)
- Found and fixed a second, related bug: the RX-delegating closure let
Groovy's static compiler infer its return type from context, injecting an
invalid narrowing cast whenever the delegate returned an `Iterable` to flatten
(the multi-result/`Observable` path). (10516d794)
- Along the way, fixed two of the test's own bugs from 2017 (a
`.first()`/`.value()` mixup on the wrong `Single`/`Observable` overload, and a
stale "cannot implement" comment blocking a property projection that compiles
fine today). (770448e4, 9a276e66c)
- All 7 previously-commented/pending cases in `RxServiceImplSpec` now
pass.
Also caught and fixed an earlier Copilot review pass (2026-08-11) that had
gone unaddressed — two check-then-act races on `ConcurrentHashMap`
(`QueryState.addLoadedEntity`, `RxGormEnhancer.registerEntity`) fixed with
`computeIfAbsent`/`putIfAbsent` plus regression tests, and a message typo in
`MultiTenantEventListener`. (506228bd)
Full history: 2632f6fc6c..506228bd92
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
