Fix embedded where queries

[jdaugherty]

Description

Fixes #15955

where {} queries failed with a Hibernate QueryException ("Criteria objects cannot be created directly on components") when a predicate referenced a property of an embedded component (e.g. extRef1.value =~ "%ABC%") and was combined with any other condition (&&, ||, or additional criteria). A lone embedded predicate worked because AbstractHibernateQuery.add() special-cases Embedded associations, but once the predicate was nested inside a junction, translation went through AbstractHibernateCriterionAdapter, whose DetachedAssociationCriteria and AssociationQuery adapters unconditionally created a Hibernate sub-criteria/alias on the component — which Hibernate rejects.

Changes:

• grails-datamapping-core — DetachedCriteriaTransformer no longer discards an embedded block's component type when resolving the association type from generics (a collection association carries its element type in its generics; an embedded component does not), which previously caused the block's criteria to be silently dropped. It also recomputes variable scopes after applying the where transform so the generated nested closures capture exactly the variables they reference instead of inheriting the outer closure's scope nondeterministically. AbstractDetachedCriteria now uses explicit null checks instead of Groovy truth when looking up an existing association criteria — Groovy truth on a DetachedCriteria invokes asBoolean(), which executed the criteria as a spurious query on every repeated association-block reference (and failed outright for embedded components, whose class is not a queryable root entity).
• grails-data-hibernate5 — the DetachedAssociationCriteria and AssociationQuery criterion adapters now short-circuit for Embedded associations and apply the nested criteria using dotted property paths (extRef1.value), mirroring the existing top-level handling in AbstractHibernateQuery.add().
• grails-data-hibernate7 — CriteriaMethodInvoker now recognizes JPA EMBEDDED attributes (which the JPA metamodel does not consider associations, but the GORM model does) and builds a DetachedAssociationCriteria for the embedded block without creating a join — a component's columns live in the owning entity's table, so an explicit join-type argument on an embedded block is intentionally ignored.
• grails-data-mongodb — MongoQuery previously prefixed every key of an embedded sub-query with the embedded property's path, so an embedded block containing a junction produced keys like {"extRef1.$or": [...]} that silently matched nothing. The rewrite is now recursive: logical operators ($and, $or, …) stay at the correct level and only property keys are qualified. A $-operator whose value cannot be rewritten (e.g. $where from a property-to-property comparison, or $text) inside an embedded block now throws UnsupportedOperationException instead of silently matching nothing.
• grails-data-simple — the in-memory implementation had the same class of bug: SimpleMapQuery.queryAssociation() treated Embedded (a ToOne subtype) as a real association and called session.retrieve() with the inlined embedded map as an id, throwing NonPersistentTypeException. It now evaluates the criteria directly against the stored embedded value.
• grails-datamapping-tck — new WhereQueryEmbeddedSpec plus WorkItem/SystemManaged/ExternalRef test domains reproducing the reported shape (embedded component declared in an abstract non-entity superclass). Covers =~ and == on embedded properties, chained query.where {} composition, aliased blocks, and the conjunction/disjunction cases that triggered the bug. Where-query specs that were previously duplicated per implementation (WhereQueryIssueVerificationSpec, WhereQueryLeftJoinSpec, DeleteAllSpec) were moved into the TCK, and the TCK test properties were standardized to one location in the Gradle test configs. The TCK runs against all four implementations: hibernate5, hibernate7, simple (in-memory), and mongodb.
• grails-test-examples (geb) — includes an unrelated flaky-test fix in PerTestRecordingSpec: recording-directory discovery is now scoped to directories timestamped at or after the current JVM's start time instead of racing on the "most recent" directory, which broke under parallel test execution.

All test suites for the affected modules pass, along with the project style checks. The full ./gradlew build --rerun-tasks suite was not run; verification covered the affected modules.

Contributor Checklist

Please review the following checklist before submitting your pull request. Pull requests that do not meet these requirements may be closed without review.

Issue and Scope

• This PR is linked to an existing issue that has been acknowledged or approved by the project team. If no approved issue exists, please give background on why this change is necessary. Tickets are preferred for release change log history.
• This PR addresses the complete scope of the linked issue. Partial implementations or unfinished work should not be submitted for review.
• This PR contains a single, focused change. Unrelated changes should be submitted as separate pull requests. (One exception, disclosed above: the PerTestRecordingSpec flaky-test fix.)
• This PR targets the correct branch for the type of change:
  • Patch release branches (e.g., 7.0.x): Bug fixes only. No new features or API changes.
  • Minor release branches (e.g., 7.1.x): New features are welcome, but breaking existing APIs must be avoided.
  • Major release branches (e.g., 8.0.x): Reserved for major changes. Breaking API changes are permitted.

Code Quality

• I have added or updated tests that cover the changes introduced in this PR. All code contributions are expected to include appropriate test coverage.
• I have verified that all existing tests pass by running ./gradlew build --rerun-tasks.
• My code follows the project's code style guidelines. I have run ./gradlew codeStyle and resolved any violations. See Code Style for details.
• This PR does not include mass reformatting, style-only changes, or large-scale refactoring unless it was explicitly approved in the linked issue. Unsolicited reformatting will not be accepted.
• If generative AI tooling was used in preparing this contribution, a quality model was used to ensure contributions are consistent with the project's quality standards.

Licensing and Attribution

• All contributed code is provided under the Apache License 2.0, and new source files include the appropriate Apache license header.
• I have the necessary rights to submit this contribution and confirm it is my own original work (see Legal Notice).
• If generative AI tooling was used in preparing this contribution, I have followed the Apache Software Foundation's policy on generative tooling and have properly attributed its use.

Documentation

• If this PR introduces user-facing changes, I have included or updated the relevant documentation.
• If this PR adds a new feature, I have updated the What's New section of the Grails Guide.
• If this PR introduces breaking changes or changes that require user action during an upgrade, I have updated the Upgrade Notes for the corresponding version in the Grails Guide.
• The PR description clearly explains what was changed and why.

[codecov]

Codecov Report

❌ Patch coverage is 97.16981% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.0308%. Comparing base (08bd0bf) to head (eaf194d).
⚠️ Report is 2 commits behind head on 8.0.x.

Files with missing lines	Patch %	Lines
...rnate/query/AbstractHibernateCriterionAdapter.java	66.6667%	2 Missing and 3 partials ⚠️
...ails/datastore/mapping/mongo/query/MongoQuery.java	90.4762%	1 Missing and 1 partial ⚠️
...ls/orm/hibernate/query/AbstractHibernateQuery.java	66.6667%	0 Missing and 1 partial ⚠️
...tastore/mapping/simple/query/SimpleMapQuery.groovy	80.0000%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #15962        +/-   ##
==================================================
+ Coverage     49.6709%   51.0308%   +1.3599%     
- Complexity      17017      17414       +397     
==================================================
  Files            2004       2012         +8     
  Lines           93896      94202       +306     
  Branches        16448      16460        +12     
==================================================
+ Hits            46639      48072      +1433     
+ Misses          40078      38926      -1152     
- Partials         7179       7204        +25     

Files with missing lines	Coverage Δ
...orm/query/criteria/AbstractDetachedCriteria.groovy	73.5736% <100.0000%> (+3.6941%)	⬆️
...m/query/transform/DetachedCriteriaTransformer.java	52.0908% <ø> (+18.5593%)	⬆️
...ails/data/testing/tck/domains/SystemManaged.groovy	100.0000% <100.0000%> (ø)
...pache/grails/data/testing/tck/domains/WqFoo.groovy	100.0000% <100.0000%> (ø)
...ails/data/testing/tck/domains/WqGroupedItem.groovy	100.0000% <100.0000%> (ø)
...e/grails/data/testing/tck/domains/WqStudent.groovy	100.0000% <100.0000%> (ø)
.../grails/data/testing/tck/domains/WqUserRole.groovy	100.0000% <100.0000%> (ø)
...grails/data/testing/tck/tests/DeleteAllSpec.groovy	100.0000% <100.0000%> (ø)
...ta/testing/tck/tests/WhereQueryEmbeddedSpec.groovy	100.0000% <100.0000%> (ø)
...g/tck/tests/WhereQueryIssueVerificationSpec.groovy	100.0000% <100.0000%> (ø)
... and 5 more

... and 61 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.

[borinquenkid]

Needs coverage added as per codecov

[jdaugherty]

I've gone ahead and moved the duplicated tests in h5/h7 for where queries to the tck as well as part of this

[borinquenkid]

Got it. Looking at the expanded file list in the Codecov report, the remaining coverage gaps are spread across the Simple, Core, and MongoDB modules.

Here is the regenerated list of missing test scenarios needed to cover the newly visible files:

1. SimpleMapQuery.groovy (grails-data-simple)

The newly added Embedded handling block (lines 295-300) inside queryAssociation is completely uncovered (0% patch coverage).

• Missing Scenario: You need a test that executes a criteria or Where query against an Embedded property, specifically utilizing the SimpleMapEntityPersister (the in-memory/simple datastore).
• Note: To fully cover this, ensure the test hits both branches of the if (embedded != null) check on line 297 (i.e., one record where the embedded component exists, and one where it is null).

2. DetachedCriteriaTransformer.java (grails-datamapping-core)

The AST transformation logic added to retain the type of embedded components (lines 812-818) is completely missed.

• Missing Scenario: A unit test verifying the AST transformation of a Where query block that targets an embedded component. The test needs to trigger compilation of a detached criteria where AstUtils.isDomainClass(type) is false, forcing it into the getAssociationTypeFromGenerics block to retain the associationType.

3. MongoQuery.java (grails-data-mongodb)

Most of the MongoDB embedded query rewriting is covered, but there are a few missed branches (partial coverage) regarding logical operators within embedded queries.

• Missing Scenario A (Line 202): The condition if (associatedEntity instanceof EmbeddedPersistentEntity || association instanceof Embedded) has partial coverage. You likely have a test covering one side of this OR condition but not the other.
• Missing Scenario B (Line 761): The condition if (key.charAt(0) == '$' && value instanceof List) inside prefixEmbeddedQuery has partial coverage. You need a MongoDB query on an embedded property that utilizes a logical operator like $and or $or (which generates a list of conditions) to hit the true branch of this check.
• Missing Scenario C (Line 764): The condition if (element instanceof Document) within the list iteration also has partial coverage. The test for Scenario B will likely cover the true branch, but you may need a scenario where an element inside a Mongo $in or similar array operator evaluates to false (e.g., checking a list of primitives instead of nested Documents).

[borinquenkid]

LGTM

[codeconsole]

Approving with a few minor changes requested (inline). The fix is correct and consistent across all four implementations, and I verified the two extra concerns beyond the diff itself:

MongoDB impact: positive, not neutral. The MongoQuery change fixes a real silent wrong-results bug — the old code prefixed every key of an embedded sub-query, so an embedded block containing a disjunction produced {"extRef1.$or": [...]}, a key that matches nothing. The new recursive rewrite keeps logical operators at the correct level, and the new TCK tests exercise exactly this shape on the Mongo suites (all green on the head commit).

Performance impact: neutral to slightly positive. Non-embedded association paths gain only an instanceof/attribute-type branch. The AbstractDetachedCriteria null-check fix is an actual win — the old Groovy-truth check invoked DetachedCriteria.asBoolean(), executing the criteria as a spurious database query on every repeated association-block reference. The Mongo handler also now computes getPropertyName() once instead of per key.

Two things to clean up in the PR description before merge:

• It states "Hibernate 7 and MongoDB required no code changes" — no longer true; both CriteriaMethodInvoker and MongoQuery have functional changes in the current head.
• The closing sentence ("Say the word and I'll run the full build in the background...") is a leftover conversational artifact and should be removed — the description becomes the merge record.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: eaf194d
▶️ Tests: 35265 executed
⚪️ Checks: 61/61 completed

---

Learn more about TestLens at testlens.app.