You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@grails.apache.org by "codeconsole (via GitHub)" <gi...@apache.org> on 2026/05/29 07:02:35 UTC

[PR] Fix constraints overwriting mapping index configuration (#15680) [grails-core]

codeconsole opened a new pull request, #15700:
URL: https://github.com/apache/grails-core/pull/15700

## Summary

Fixes #15680.

When a domain property has both `index: true, indexAttributes: [unique: true]` in `static mapping` and `unique: true` in `static constraints`, the constraint evaluation overwrites the mapping-configured property, so the database-level unique index is silently never created.

## Root Cause

`AbstractGormMappingFactory.createMappedForm()` evaluates both closures through the same `DefaultMappingConfigurationBuilder`:
1. The mapping closure (`name index: true, indexAttributes: [unique: true]`) goes through `builder.invokeMethod`, which stores a `MongoAttribute` with `index=true, indexAttributes={unique:true}` in `builder.properties['name']`.
2. The constraint closure (`name unique: true`) goes through `Entity.methodMissing`, which creates a SEPARATE instance in `target.propertyConfigs['name']` containing only `unique=true`.
3. `builder.getProperties()` then does `properties.putAll(target.propertyConfigs)`, overwriting the mapping-set property with the constraint-only version. `index` reverts to `false` and `indexAttributes` becomes `null`.

`MongoDatastore.initializeIndices()` reads `property.getMapping().getMappedForm().isIndex()`, which now returns `false`, so the MongoDB index is silently never created.

## Fix

Change `getProperties()` to use `putIfAbsent` semantics — properties already configured by the mapping closure are preserved when the constraint closure provides a conflicting entry.

## Test

`DefaultMappingConfigurationBuilderSpec` covers the scenario directly: a property simulating GORM's `Entity.methodMissing` path against a builder that already has a mapping-configured property. The test FAILS before the fix and PASSES after.

## Verified

Tested against a real Pixoto3 Grails 7.2 app with embedded MongoDB:
- Before: `Category.name` declared `index: true, indexAttributes: [unique: true]` in mapping + `unique: true` in constraints → only `_id_` index in MongoDB
- After: same declarations → `name_1` unique index correctly created in MongoDB

## Backward Compatibility

Behavior change: if a user previously relied on constraints overwriting mapping (unlikely — that's the bug), they would observe their mapping-configured property surviving instead. The opposite direction (constraint values being applied on top of an existing mapping property) was already happening through `builder.invokeMethod` reusing the existing instance, so the typical "add a constraint to a mapped property" flow is unaffected.

--
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix constraints overwriting mapping index configuration (#15680) [grails-core]

Posted by "codeconsole (via GitHub)" <gi...@apache.org>.

codeconsole commented on code in PR #15700:
URL: https://github.com/apache/grails-core/pull/15700#discussion_r3323182904


##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/groovy/DefaultMappingConfigurationBuilder.groovy:
##########
@@ -47,7 +47,11 @@ class DefaultMappingConfigurationBuilder implements MappingConfigurationBuilder
 
     Map<String, Property> getProperties() {
         if (!target.propertyConfigs.isEmpty()) {
-            properties.putAll(target.propertyConfigs)
+            for (Map.Entry entry : target.propertyConfigs.entrySet()) {
+                if (!properties.containsKey(entry.key)) {
+                    properties.put(entry.key, entry.value)
+                }

Review Comment:
   Good call — added a DEBUG-level log statement when a key collision is detected. Kept it at debug because in a real Grails-app boot this fires for every property that appears in both a `static mapping` and `static constraints` block (which is most domains with any field validation alongside any explicit mapping), so info-level would be too noisy for a steady-state condition. Anyone investigating an unexpected precedence outcome can flip the logger for `org.grails.datastore.mapping.config.groovy.DefaultMappingConfigurationBuilder` to DEBUG and see exactly which keys collided.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix constraints overwriting mapping index configuration (#15680) [grails-core]

Posted by "sbglasius (via GitHub)" <gi...@apache.org>.

sbglasius commented on code in PR #15700:
URL: https://github.com/apache/grails-core/pull/15700#discussion_r3322953422


##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/groovy/DefaultMappingConfigurationBuilder.groovy:
##########
@@ -47,7 +47,11 @@ class DefaultMappingConfigurationBuilder implements MappingConfigurationBuilder
 
     Map<String, Property> getProperties() {
         if (!target.propertyConfigs.isEmpty()) {
-            properties.putAll(target.propertyConfigs)
+            for (Map.Entry entry : target.propertyConfigs.entrySet()) {
+                if (!properties.containsKey(entry.key)) {
+                    properties.put(entry.key, entry.value)
+                }

Review Comment:
   Would it make sense to make a log statement if the `properties` actually contains the `entry` and that way give the developer a chance to know that there is a conflicting property?



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org