Skip to content

Commit b21d906

Browse files
codeconsolecodeconsole
committed
Native gorm mongodb String to ObjectId conversion
1 parent 80c0e6a commit b21d906

14 files changed

Lines changed: 777 additions & 9 deletions

File tree

‎grails-data-mongodb/bson/src/main/groovy/org/grails/datastore/bson/codecs/encoders/IdentityEncoder.groovy‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,25 @@ class IdentityEncoder implements PropertyEncoder<Identity> {
4141
void encode(BsonWriter writer, Identity property, Object id, EntityAccess parentAccess, EncoderContext encoderContext, CodecRegistry codecRegistry) {
4242
writer.writeName(getIdentifierName(property))
4343

44+
Class<?> storedAs = resolveStoredAs(property)
45+
if (storedAs != null && id != null) {
46+
if (ObjectId.class.isAssignableFrom(storedAs) && !(id instanceof ObjectId)) {
47+
String hex = id.toString()
48+
// Guard against natural-key strings accidentally paired with storedAs: ObjectId.
49+
// new ObjectId(<non-hex>) throws IllegalArgumentException, which would surface
50+
// deep inside the BSON write pipeline. Fall through to writeString for consistency
51+
// with the converter-based paths (MongoCodecSession, MongoCodecEntityPersister).
52+
if (ObjectId.isValid(hex)) {
53+
writer.writeObjectId(new ObjectId(hex))
54+
return
55+
}
56+
}
57+
if (String.class.isAssignableFrom(storedAs) && !(id instanceof String)) {
58+
writer.writeString(id.toString())
59+
return
60+
}
61+
}
62+
4463
if (id instanceof ObjectId) {
4564
writer.writeObjectId(id)
4665
} else if (id instanceof Number) {
@@ -51,6 +70,14 @@ class IdentityEncoder implements PropertyEncoder<Identity> {
5170

5271
}
5372

73+
private static Class<?> resolveStoredAs(Identity property) {
74+
try {
75+
return property?.owner?.mapping?.identifier?.storedAs
76+
} catch (Throwable ignored) {
77+
return null
78+
}
79+
}
80+
5481
protected String getIdentifierName(Identity property) {
5582
String[] identifierName = property.getOwner().mapping.identifier?.identifierName
5683
if (identifierName != null) {

‎grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoCodecSession.groovy‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class MongoCodecSession extends AbstractMongoSession {
152152
DirtyCheckable changedObject = (DirtyCheckable) update.getNativeEntry()
153153
PersistentEntityCodec codec = (PersistentEntityCodec) datastore.codecRegistry.get(changedObject.getClass())
154154

155-
final Object nativeKey = update.nativeKey
155+
final Object nativeKey = coerceIdToStoredType(update.nativeKey, persistentEntity)
156156
final Document id = new Document(MongoEntityPersister.MONGO_ID_FIELD, nativeKey)
157157

158158
EntityAccess entityAccess = update.entityAccess
@@ -200,7 +200,7 @@ class MongoCodecSession extends AbstractMongoSession {
200200

201201
if (delete.vetoed) continue
202202

203-
final Object k = delete.nativeKey
203+
final Object k = coerceIdToStoredType(delete.nativeKey, persistentEntity)
204204
if (k) {
205205
nativeKeys << k
206206
final List cascadeOperations = delete.cascadeOperations
@@ -286,6 +286,30 @@ class MongoCodecSession extends AbstractMongoSession {
286286
return entityWrites
287287
}
288288

289+
/**
290+
* If the entity's id mapping declares {@code storedAs} and it differs from the in-memory
291+
* native key type, coerce the key so that update/delete filters target BSON values of
292+
* the correct type (otherwise {@code {_id: "<hex>"}} sent as a BSON String would never
293+
* match an {@code _id: ObjectId(...)} document on disk, and the write would silently miss,
294+
* surfacing as a misleading {@link OptimisticLockingException}).
295+
*/
296+
protected Object coerceIdToStoredType(Object nativeKey, PersistentEntity entity) {
297+
if (nativeKey == null) return null
298+
Class<?> storedAs = null
299+
try {
300+
storedAs = entity?.mapping?.identifier?.storedAs
301+
} catch (Throwable ignored) {
302+
return nativeKey
303+
}
304+
if (storedAs == null) return nativeKey
305+
if (storedAs.isInstance(nativeKey)) return nativeKey
306+
try {
307+
return mappingContext.conversionService.convert(nativeKey, storedAs)
308+
} catch (Throwable ignored) {
309+
return nativeKey
310+
}
311+
}
312+
289313
@Override
290314
protected Transaction beginTransactionInternal() {
291315
return new SessionOnlyTransaction<MongoClient>(getNativeInterface(), this)

‎grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/config/MongoMappingContext.java‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
import org.bson.types.Decimal128;
4747
import org.bson.types.ObjectId;
4848
import org.bson.types.Symbol;
49+
import org.slf4j.Logger;
50+
import org.slf4j.LoggerFactory;
4951

5052
import org.springframework.core.convert.converter.Converter;
5153
import org.springframework.core.convert.converter.ConverterRegistry;
@@ -101,6 +103,8 @@
101103
*/
102104
@SuppressWarnings("rawtypes")
103105
public class MongoMappingContext extends DocumentMappingContext {
106+
107+
private static final Logger log = LoggerFactory.getLogger(MongoMappingContext.class);
104108
private static final String DECIMAL_TYPE_CLASS_NAME = "org.bson.types.Decimal128";
105109
/**
106110
* Java types supported as mongo property types.
@@ -133,6 +137,20 @@ public class MongoMappingContext extends DocumentMappingContext {
133137

134138
private CodecRegistry codecRegistry;
135139
private Map<Class, Boolean> hasCodecCache = new HashMap<>();
140+
/**
141+
* Global default storage type for {@code String id} fields that don't declare an explicit
142+
* {@code id storedAs: ...} in their mapping. Null means "no default — use the declared
143+
* Java type" (current GORM behavior). See {@link MongoSettings#SETTING_STRING_IDS_DEFAULT_STORED_AS}.
144+
*/
145+
private Class<?> stringIdDefaultStoredAs;
146+
147+
public Class<?> getStringIdDefaultStoredAs() {
148+
return stringIdDefaultStoredAs;
149+
}
150+
151+
public void setStringIdDefaultStoredAs(Class<?> stringIdDefaultStoredAs) {
152+
this.stringIdDefaultStoredAs = stringIdDefaultStoredAs;
153+
}
136154

137155
public MongoMappingContext(String defaultDatabaseName) {
138156
this(defaultDatabaseName, null);
@@ -165,7 +183,28 @@ public MongoMappingContext(String defaultDatabaseName, Closure defaultMapping, C
165183
*/
166184
@Deprecated
167185
public MongoMappingContext(PropertyResolver configuration, Class... classes) {
168-
this(getDefaultDatabaseName(configuration), configuration.getProperty(MongoSettings.SETTING_DEFAULT_MAPPING, Closure.class, null), classes);
186+
super(getDefaultDatabaseName(configuration), configuration.getProperty(MongoSettings.SETTING_DEFAULT_MAPPING, Closure.class, null));
187+
// Must run BEFORE initialize(classes) so that MongoDocumentMappingFactory.createIdentity
188+
// (invoked during entity registration) can read the global default.
189+
String storedAsDefault = configuration.getProperty(MongoSettings.SETTING_STRING_IDS_DEFAULT_STORED_AS, String.class, null);
190+
this.stringIdDefaultStoredAs = parseStoredAs(storedAsDefault);
191+
initialize(classes);
192+
}
193+
194+
private static Class<?> parseStoredAs(String value) {
195+
if (value == null) return null;
196+
switch (value.toLowerCase()) {
197+
case "objectid":
198+
case "object_id":
199+
return ObjectId.class;
200+
case "string":
201+
return String.class;
202+
default:
203+
log.warn("Unrecognized value '{}' for {}; accepted values are 'objectid' or 'string'. " +
204+
"Falling back to default behavior (no coercion).",
205+
value, MongoSettings.SETTING_STRING_IDS_DEFAULT_STORED_AS);
206+
return null;
207+
}
169208
}
170209

171210
/**
@@ -176,6 +215,9 @@ public MongoMappingContext(PropertyResolver configuration, Class... classes) {
176215
*/
177216
public MongoMappingContext(AbstractMongoConnectionSourceSettings settings, Class... classes) {
178217
super(settings.getDatabase(), settings);
218+
// Must run BEFORE initialize(classes) so that MongoDocumentMappingFactory.createIdentity
219+
// (invoked during entity registration) can read the global default.
220+
this.stringIdDefaultStoredAs = parseStoredAs(settings.getStringIdsDefaultStoredAs());
179221
initialize(classes);
180222
}
181223

@@ -333,7 +375,14 @@ protected Class<MongoCollection> getEntityMappedFormType() {
333375
@Override
334376
public Identity<MongoAttribute> createIdentity(PersistentEntity owner, MappingContext context, PropertyDescriptor pd) {
335377
Identity<MongoAttribute> identity = super.createIdentity(owner, context, pd);
336-
identity.getMapping().getMappedForm().setTargetName(MongoConstants.MONGO_ID_FIELD);
378+
MongoAttribute mappedForm = identity.getMapping().getMappedForm();
379+
mappedForm.setTargetName(MongoConstants.MONGO_ID_FIELD);
380+
// Apply the global default storedAs for String-id domains that don't declare their own.
381+
if (mappedForm.getStoredAs() == null &&
382+
stringIdDefaultStoredAs != null &&
383+
String.class.equals(pd.getPropertyType())) {
384+
mappedForm.setStoredAs(stringIdDefaultStoredAs);
385+
}
337386
return identity;
338387
}
339388

‎grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/config/MongoSettings.groovy‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,18 @@ interface MongoSettings extends Settings {
9595

9696
String SETTING_ENGINE = 'grails.mongodb.engine'
9797

98+
/**
99+
* Global default storage type for {@code String id} fields when no per-domain
100+
* {@code id storedAs: ...} mapping is declared. Accepted values are the names (or hex
101+
* aliases) {@code 'string'} (default, current behavior) and {@code 'objectid'}.
102+
*
103+
* <p>When set to {@code 'objectid'}, every domain that declares {@code String id}
104+
* without an explicit {@code storedAs} will persist {@code _id} as a BSON ObjectId,
105+
* while keeping the {@code String} ergonomics in application code. Domains that use
106+
* natural string keys (slug, email, UUID) should opt out per-domain via
107+
* {@code static mapping = { id storedAs: String }}.
108+
*
109+
* @since 7.1.1
110+
*/
111+
String SETTING_STRING_IDS_DEFAULT_STORED_AS = 'grails.mongodb.stringIdsDefaultStoredAs'
98112
}

‎grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/connections/AbstractMongoConnectionSourceSettings.groovy‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,17 @@ abstract class AbstractMongoConnectionSourceSettings extends ConnectionSourceSet
8787
*/
8888
boolean decimalType = true
8989

90+
/**
91+
* Global default storage type for {@code String id} fields that don't declare an
92+
* explicit {@code id storedAs: ...} in their mapping. Accepted values are
93+
* {@code 'string'} (default, current behavior) or {@code 'objectid'}.
94+
*
95+
* <p>Corresponds to {@link org.grails.datastore.mapping.mongo.config.MongoSettings#SETTING_STRING_IDS_DEFAULT_STORED_AS}.
96+
*
97+
* @since 7.1.1
98+
*/
99+
String stringIdsDefaultStoredAs
100+
90101
/**
91102
* The collection name to use to resolve connections when using {@link MongoConnectionSources}
92103
*/

‎grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/engine/MongoCodecEntityPersister.groovy‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,11 @@ class MongoCodecEntityPersister extends ThirdPartyCacheEntityPersister<Object> {
128128
// don't bother with query if list of keys is empty
129129
return []
130130
} else {
131+
// Coerce each key to the storage type so {_id: {$in: [...]}} uses BSON values
132+
// that actually match on disk when 'storedAs' differs from the declared type.
133+
def coercedIds = idList.collect { coerceIdToStoredType(it, pe) }
131134
createQuery()
132-
.in(pe.identity.name, idList)
135+
.in(pe.identity.name, coercedIds)
133136
.list()
134137

135138
}
@@ -153,7 +156,7 @@ class MongoCodecEntityPersister extends ThirdPartyCacheEntityPersister<Object> {
153156
return null
154157
} else {
155158
MongoCollection mongoCollection = getMongoCollection(pe)
156-
Document idQuery = createIdQuery(key)
159+
Document idQuery = createIdQuery(coerceIdToStoredType(key, pe))
157160
o = mongoCollection
158161
.withDocumentClass(persistentEntity.javaClass)
159162
.withCodecRegistry(mongoDatastore.codecRegistry)
@@ -175,6 +178,27 @@ class MongoCodecEntityPersister extends ThirdPartyCacheEntityPersister<Object> {
175178
new Document(AbstractMongoObectEntityPersister.MONGO_ID_FIELD, key)
176179
}
177180

181+
/**
182+
* Coerce an identifier value to the {@code storedAs} type declared in the entity's id
183+
* mapping, so that point-lookup queries target the BSON type actually on disk.
184+
* See {@code IdentityMapping#getStoredAs()}.
185+
*/
186+
protected Object coerceIdToStoredType(Object key, PersistentEntity entity) {
187+
if (key == null) return key
188+
Class<?> storedAs = null
189+
try {
190+
storedAs = entity?.mapping?.identifier?.storedAs
191+
} catch (Throwable ignored) {
192+
return key
193+
}
194+
if (storedAs == null || storedAs.isInstance(key)) return key
195+
try {
196+
return mappingContext.conversionService.convert(key, storedAs)
197+
} catch (Throwable ignored) {
198+
return key
199+
}
200+
}
201+
178202
@Override
179203
protected Serializable persistEntity(PersistentEntity entity, Object obj, boolean isInsert) {
180204

‎grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/query/MongoQuery.java‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,66 @@ public void handle(EmbeddedQueryEncoder queryEncoder, IdEquals criterion, Docume
138138
Object value = criterion.getValue();
139139
MappingContext mappingContext = entity.getMappingContext();
140140
PersistentProperty identity = entity.getIdentity();
141-
Object converted = mappingContext.getConversionService().convert(value, identity.getType());
141+
// Prefer the configured storage type ('storedAs' on the id mapping) so query BSON
142+
// matches what's actually on disk. Falls back to the declared Java type otherwise.
143+
Class<?> targetType = null;
144+
try {
145+
if (entity.getMapping() != null && entity.getMapping().getIdentifier() != null) {
146+
targetType = entity.getMapping().getIdentifier().getStoredAs();
147+
}
148+
} catch (Throwable ignored) {
149+
// defensive: some mapping implementations may not expose storedAs yet
150+
}
151+
if (targetType == null) {
152+
targetType = identity.getType();
153+
}
154+
Object converted = mappingContext.getConversionService().convert(value, targetType);
142155
query.put(MongoEntityPersister.MONGO_ID_FIELD, converted);
143156
}
144157
});
145158

159+
// Override the In handler so that criteria targeting the identity (findAllByIdInList,
160+
// Domain.createCriteria().list { 'in'('id', [...]) }, etc.) honor 'storedAs' the same way
161+
// IdEquals does. Without this, a domain declaring storedAs: ObjectId would send BSON Strings
162+
// in {_id: {$in: [...]}} and miss all stored ObjectId documents.
163+
queryHandlers.put(In.class, new QueryHandler<In>() {
164+
public void handle(EmbeddedQueryEncoder queryEncoder, In in, Document query, PersistentEntity entity) {
165+
Document inQuery = new Document();
166+
List<Object> values = getInListQueryValues(entity, in);
167+
168+
Class<?> storedAs = null;
169+
try {
170+
PersistentProperty identityProp = entity.getIdentity();
171+
if (identityProp != null && identityProp.getName().equals(in.getProperty()) &&
172+
entity.getMapping() != null && entity.getMapping().getIdentifier() != null) {
173+
storedAs = entity.getMapping().getIdentifier().getStoredAs();
174+
}
175+
} catch (Throwable ignored) {
176+
// defensive: mapping implementations without storedAs support fall through
177+
}
178+
if (storedAs != null) {
179+
MappingContext mappingContext = entity.getMappingContext();
180+
List<Object> coerced = new ArrayList<>(values.size());
181+
for (Object v : values) {
182+
if (v == null || storedAs.isInstance(v)) {
183+
coerced.add(v);
184+
} else {
185+
try {
186+
coerced.add(mappingContext.getConversionService().convert(v, storedAs));
187+
} catch (Throwable ignored) {
188+
coerced.add(v);
189+
}
190+
}
191+
}
192+
values = coerced;
193+
}
194+
195+
inQuery.put(IN_OPERATOR, values);
196+
String propertyName = getPropertyName(entity, in);
197+
query.put(propertyName, inQuery);
198+
}
199+
});
200+
146201
queryHandlers.put(AssociationQuery.class, new QueryHandler<AssociationQuery>() {
147202
public void handle(EmbeddedQueryEncoder queryEncoder, AssociationQuery criterion, Document query, PersistentEntity entity) {
148203
Association<?> association = criterion.getAssociation();

0 commit comments

Comments
 (0)