jdaugherty commented on code in PR #15583:
URL: https://github.com/apache/grails-core/pull/15583#discussion_r3141992348
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/config/MongoMappingContext.java:
##########
@@ -176,6 +215,10 @@ public MongoMappingContext(PropertyResolver configuration,
Class... classes) {
*/
public MongoMappingContext(AbstractMongoConnectionSourceSettings settings,
Class... classes) {
super(settings.getDatabase(), settings);
+ // Must run BEFORE initialize(classes) so that
MongoDocumentMappingFactory.createIdentity
+ // (invoked during entity registration) can read the global default.
+ String storedAsDefault = settings.getStringIds() != null ?
settings.getStringIds().getDefaultStoredAs() : null;
+ this.stringIdDefaultStoredAs = parseStoredAs(storedAsDefault);
Review Comment:
Asymmetry to flag: the deprecated `(PropertyResolver, Class...)` constructor
and this `(AbstractMongoConnectionSourceSettings, Class...)` constructor both
read `stringIdDefaultStoredAs` from settings, but the public
`MongoMappingContext(String defaultDatabaseName, Closure defaultMapping,
Class... classes)` constructor does not — so anyone wiring the context manually
with that signature silently loses the global default. Either factor a private
`applyStoredAsFromConfig(...)` helper and invoke it from all constructors that
have the relevant inputs, or document on the bare constructor that it does not
honor `grails.mongodb.stringIdsDefaultStoredAs`. Today this is the kind of
inconsistency that gets discovered only when a test using the bare constructor
passes locally and the production datastore — using the settings constructor —
diverges.
--
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]
