codeconsole opened a new pull request, #16494:
URL: https://github.com/apache/grails-core/pull/16494
## Problem
GORM for MongoDB creates and reconciles the indexes a mapping declares, but
it never drops one. An index that a mapping stops declaring, or declares again
with its keys in another order, stays on the server and is maintained on every
write. An application has no way to ask which indexes no domain class accounts
for any more, short of comparing `listIndexes` with its mappings by hand.
Building that comparison exposed a second problem, which has to be fixed
first or the comparison would be wrong. With a `'*'` entry in the default
constraints, as in `grails.gorm.default.constraints = { '*'(nullable: true) }`,
`DefaultMappingConfigurationBuilder` started every Map-style property entry
from a fresh clone of `'*'` and stored it in place of the property's existing
entry. Default constraints are evaluated before a domain class's own closures,
so the constraints closure replaced what the mapping closure had configured for
the same property:
```groovy
static mapping = {
passwordResetToken index: true // configured here...
}
static constraints = {
passwordResetToken nullable: true // ...and replaced here, starting
again from '*'
}
```
That index is never built, and the build summary says nothing about it.
#15680 fixed the same overwrite for entries that arrive through
`Entity.propertyConfigs`; this is the path through the builder's own map. In an
application we run, four field-level indexes declared this way were missing
from production. A cleanup that trusted the declarations would have treated any
such index that does exist as undeclared and dropped it.
## Change
Two commits.
**Keep a property's mapping when a `'*'` default constraint is configured.**
The `'*'` default now seeds a property's first entry only; a later closure
configures the entry that is already there. A property first configured after
the default still starts from it, and the default itself is not changed.
**`MongoDatastore.findUndeclaredIndexes()` and `dropUndeclaredIndexes()`.**
- `findUndeclaredIndexes()` lists, for the collections the index build
covers, each index whose key pattern no domain class mapped to that collection
declares, and changes nothing. A key pattern matches a declaration when it has
the same fields in the same order, through `compoundIndex`, `index` or a
property's `index: true`.
- Names and options are ignored, since those are the differences the build
reconciles. The declarations of every class mapped to a collection count, so a
subclass's index on its root's collection is declared. A declared text index
matches the synthetic `{_fts: 'text', _ftsx: 1}` key MongoDB reports for it.
The `_id` index and collections no domain class maps are never reported.
- Each result is an `UndeclaredIndex` record: database, collection, name,
key pattern and the whole `listIndexes` description.
- `dropUndeclaredIndexes()` drops what find reports, logging each drop at
`INFO`. `dropUndeclaredIndexes(List)` drops a reviewed subset. An index or
collection that has gone since it was listed is skipped. The driver answers a
`dropIndex` on a collection that no longer exists as a success, so the indexes
still present are listed first rather than inferred from the drop.
- As with `buildIndex()`, each named connection covers its own domain
classes through `getDatastoreForConnection(...)`.
- The build and the new methods read the same declarations:
`initializeIndices` applies the list a new private `declaredIndexes()` returns,
in the same order as before, and `findIndexByKeyPattern` shares its matching
rule with the new lookup.
Nothing drops automatically, and there is no setting to make it. The docs
say why: to an instance still on an earlier release, an index that a later
release declares, or one created by hand ahead of a deployment, is undeclared,
so dropping is a deliberate step for after every instance runs the release that
declares the indexes to keep. An index created by an `initializeIndices`
override, or by application code calling `createIndex` itself, is not a
declaration and is reported like any other.
Docs: a "Finding and Dropping Undeclared Indexes" section in the GORM for
MongoDB guide, its release notes for both commits, and a paragraph in the
Grails Guide's What's New under GORM for MongoDB indexes.
Not included: `Entity.getOrInitializePropertyConfig` has a related defect.
With a `'*'` entry in `propertyConfigs`, it configures a clone and never stores
it. Hibernate 5 and 7's `Mapping` resolve their property configuration through
that method, so changing it belongs in its own pull request with those suites
behind it.
## Tests
- `DefaultMappingConfigurationBuilderSpec`: a mapping entry survives a
constraints entry for the same property under a `'*'` default (fails before the
fix), and a property first configured after the default starts from it without
changing the default.
- `BuildIndexesDefaultConstraintsSpec`: a datastore configured with
`grails.gorm.default.constraints = { '*'(nullable: true) }` builds a plain and
an attributed field-level index on constrained properties. It fails before the
fix, with `{code: 1}` never built.
- `UndeclaredIndexesSpec`: what is and is not reported (direction and order
differences, a declared key pattern under another name and options,
inheritance, two classes sharing a collection, a text index, `_id`, an unmapped
collection); dropping removes exactly the undeclared indexes and logs each, and
a second run finds nothing; a reviewed list skips an index and a collection
dropped since it was listed; a named connection reports its own database.
- Full suites of the modules the change reaches, 0 failures:
`grails-datastore-core` (314 tests), `grails-data-mongodb-core` (953) and
`grails-data-neo4j-core` (605). The skips are in existing specs.
- `./gradlew clean aggregateViolations --continue`: Checkstyle, CodeNarc,
PMD and repository conventions report no violations (SpotBugs is disabled in
the baseline). `:grails-data-mongodb-docs:asciidoctor` builds without warnings,
with both new anchors resolving.
- I have not run the whole `./gradlew build` or `:grails-test-report:check`.
--
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]
