Embedded collections doesn't work with gorm 7.3.0

[lucas-napse]

Hi @puneetbehl !

In our system we have a large number of domain classes. In our recent upgrade to grails 5 (5.3.2) that makes use of GORM 7.3.0 of our system, which was working fine with the Grails 2.5.6, we noticed that the embedded entities were not loaded in the object that contained it: For example,

class Country {

    List<Map> regions

    static embedded = ['regions']

}

In the above Country has embedded regions, these regions are initialized as an empty list when loaded through GORM. Whereas, in the database collection Country has regions. Even, when using the MongoDB API, these regions are loaded correctly as list of Document.

This does not work when using either Country.findAll(), or Country.createCriteria().list{}. It maybe breaking with other GORM queries as well.

Upon, debugging, we noticed that the problem might be caused because the database returns a Map instead of a Document. The Groovy class MongoEntityPersister exclusively checks for the class Document which breaks certain conditions. For example:

1. MongoEntityPersister.loadEmbeddedCollection().

   When the method is executed, I noticed that in our case it goes through the else branch because the following condition evaluates to false

 (if(Map.class.isAssignableFrom(embeddedCollection.getType())))

The above results in embedded regions to return an empty List because it is a Map.

    if (embeddedInstances instanceof Collection) {                    // true
                Collection coll = (Collection)embeddedInstances;
                for (Object dbo : coll) {                             // for each embedded element...
                    if (dbo instanceof Document) {                    // <------ this condition is problematic: is a Map, not a Document
                        Document nativeEntry = (Document)dbo;

By modifying the above to following, might fixes the problem:

      if (embeddedInstances instanceof Collection) {
                Collection coll = (Collection)embeddedInstances;
                for (Object dbo : coll) { 
                    if (dbo instanceof Map) {        // Map, instead Document, in this section
                        Document nativeEntry;
                        if(dbo instanceof Document){    // then, it is a Document? then the same thing
                            nativeEntry = (Document)dbo;
                        }
                        else{
                            nativeEntry = new Document((Map)dbo);  // if it is a Map, a new Document instance is created based on that Map.
                        }

The above change solves the our problem where the regions are correctly loaded via the GORM Country.findAll method.

Doing more tests I notice that there is a second problem: if this embedded entity uses composition, some related attributes are loaded and others are not.
For example: my country has regions, and a region has a region type. This type of region is a Domain. The regions are embedded in the country, and a region has only one type of region, as an associated instance (composition).

This instance, the region type, is not loaded.

Performing more debugging, I find that the code goes by:
NativeEntityPersister#refreshObjectStateFromNativeEntry(PersistentEntity, Object, Serializable, T, boolean)

 for (final PersistentProperty prop : props) {         // for each attribute of my region, in the case of regionType...
    ... more code ...
   
               else if (prop instanceof ToOne) {                       // true
            if (prop instanceof Embedded) {                          // true
                Embedded embedded = (Embedded) prop;    // isnt null, ok
                if(embedded.getAssociatedEntity() != null) {   // true

                    T embeddedEntry = getEmbedded(nativeEntry, propKey);  // <----- THIS section returns null. Why?

... continuing debugging leads to this method ...

  public abstract class AbstractMongoObectEntityPersister<T> extends NativeEntryEntityPersister<T, Object> { ...
    ... more code ...
protected T getEmbedded(T nativeEntry, String key) {
    Object embeddedDocument = this.getValueRetrievalStrategy().getValue(nativeEntry, key);
    return this.isEmbeddedEntry(embeddedDocument) ? embeddedDocument : null;   // <-------- THIS section is the problem...
}

... continuing debugging leads to this method ...

In MongoEntityPersister....

@Override
protected boolean isEmbeddedEntry(Object entry) {
    return entry instanceof Document;
}

...clearly this method gives true when it is a Document, but my region type appears as Map, which gives false. This, by giving false, the caller (AbstractMongoObectEntityPersister#getEmbedded(T, String)) determines that it is null, so it does not load my region type.

Making this change should fix it:

@Override
protected boolean isEmbeddedEntry(Object entry) {
    return entry instanceof Map;
}

The problem now is that internally we get various errors related to the Map and Document type. And these are related to the getValue() and setValue() methods of the MongoEntityPersister class:

public static final ValueRetrievalStrategy<Document> VALUE_RETRIEVAL_STRATEGY = new ValueRetrievalStrategy<Document>() {
    @Override
    public Object getValue(Document document, String name) {  // <---- Designed for Document, but internally from mongo you get a Map
        return document.get(name);
    }

    @Override
    public void setValue(Document document, String name, Object value) {  // <---- Designed for Document, but internally from mongo you get a Map
        document.put(name, value);
    }
};

This is the error you get after making all the changes I mentioned above:

Caused by: java.lang.ClassCastException: class java.util.LinkedHashMap cannot be cast to class org.bson.Document (java.util.LinkedHashMap is in module java.base of loader 'bootstrap'; org.bson.Document is in unnamed module of loader 'app')
	at org.grails.datastore.mapping.mongo.engine.MongoEntityPersister$1.getValue(MongoEntityPersister.java:58)
	at org.grails.datastore.mapping.mongo.engine.AbstractMongoObectEntityPersister.discriminatePersistentEntity(AbstractMongoObectEntityPersister.java:315)
	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.createObjectFromEmbeddedNativeEntry(NativeEntryEntityPersister.java:368)
	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.refreshObjectStateFromNativeEntry(NativeEntryEntityPersister.java:443)
	at org.grails.datastore.mapping.mongo.engine.AbstractMongoObectEntityPersister.refreshObjectStateFromNativeEntry(AbstractMongoObectEntityPersister.java:278)
	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.createObjectFromEmbeddedNativeEntry(NativeEntryEntityPersister.java:370)
	at org.grails.datastore.mapping.mongo.engine.MongoEntityPersister.loadEmbeddedCollection(MongoEntityPersister.java:115)
	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.refreshObjectStateFromNativeEntry(NativeEntryEntityPersister.java:507)
	at org.grails.datastore.mapping.mongo.engine.AbstractMongoObectEntityPersister.refreshObjectStateFromNativeEntry(AbstractMongoObectEntityPersister.java:281)
	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.createObjectFromNativeEntry(NativeEntryEntityPersister.java:363)
	at org.grails.datastore.mapping.mongo.query.MongoQuery$MongoResultList.convertDBObject(MongoQuery.java:1392)
	at org.grails.datastore.mapping.mongo.query.MongoQuery$MongoResultList.convertObject(MongoQuery.java:1378)
	at org.grails.datastore.gorm.query.AbstractResultList.convertObject(AbstractResultList.java:99)
	at org.grails.datastore.gorm.query.AbstractResultList.initializeFully(AbstractResultList.java:59)
	at org.grails.datastore.gorm.query.AbstractResultList.size(AbstractResultList.java:206)
	at grails.gorm.PagedResultList.size(PagedResultList.java:114)
	at grails.gorm.PagedResultList.isEmpty(PagedResultList.java:119)
	at org.grails.plugins.web.rest.render.html.DefaultHtmlRenderer.resolveModelVariableName(DefaultHtmlRenderer.groovy:105)
	at org.grails.plugins.web.rest.render.html.DefaultHtmlRenderer.applyModel(DefaultHtmlRenderer.groovy:88)
	at org.grails.plugins.web.rest.render.html.DefaultHtmlRenderer.render(DefaultHtmlRenderer.groovy:79)
	at grails.artefact.controller.RestResponder$Trait$Helper.internalRespond(RestResponder.groovy:216)
	at grails.artefact.controller.RestResponder$Trait$Helper.respond(RestResponder.groovy:83)
	at <our index method of our system controller>
	... 72 common frames omitted

As a solution we will try to use an older version of gorm, but if this problem can be solved it would be great since it is always necessary to have the latest. If you need more information, please comment and I will respond as soon as possible.

Some details:
| Grails Version: 5.3.2
| JVM Version: 11.0.16.1
| Gorm version: 7.3.0
| Windows 11, with last updates.

(build.gradle)
implementation 'org.grails.plugins:mongodb:7.3.0'
implementation "org.grails:gorm-mongodb-spring-boot:7.3.0"

configurations.all {
    // Genera conflictos, tomaba el 3.6 viejo.
    exclude module:'mongodb-driver'
}

[lucas-napse]

@puneetbehl
I have the changes of MongoEntityPersister in my ide.
Do you want me to upload it as a pull request?

The change:
OLD

 if (embeddedInstances instanceof Collection) {                    // true
        Collection coll = (Collection)embeddedInstances;
        for (Object dbo : coll) {                             // for each embedded element...
            if (dbo instanceof Document) {                    // <------ this condition is problematic: is a Map, not a Document
                Document nativeEntry = (Document)dbo;

NEW

  if (embeddedInstances instanceof Collection) {
        Collection coll = (Collection)embeddedInstances;
        for (Object dbo : coll) { 
            if (dbo instanceof Map) {        // Map, instead Document, in this section
                Document nativeEntry;
                if(dbo instanceof Document){    // then, it is a Document? then the same thing
                    nativeEntry = (Document)dbo;
                }
                else{
                    nativeEntry = new Document((Map)dbo);  // if it is a Map, a new Document instance is created based on that Map.
                }

OLD EDIT:

@puneetbehl
I cant upload this PR, the server returns 403 "no permission" when i try to push to branch 73x.

NEW EDIT. IMPORTANT!

This is not the solution. The solution is found below, in the modification of the getValue() method

[lucas-napse]

@puneetbehl
The same occurrs in grails 4.1.2, with 'org.grails.plugins:mongodb:7.1.0', and "org.grails:gorm-mongodb-spring-boot:7.1.0".

[lucas-napse]

@puneetbehl
Hi.
We can solve this problem. It is due to lack of conversion to Document, for the mechanism to work correctly. Using our system (a large, very complex system) we have been able to verify that it works correctly. Since I don't have pull request permissions, I leave the change to be made here:

class MongoEntityPersister

FROM

public static final ValueRetrievalStrategy<Document> VALUE_RETRIEVAL_STRATEGY = new ValueRetrievalStrategy<Document>() {
    @Override
    public Object getValue(Document document, String name) {
        return document.get(name);
    }

TO

public static final ValueRetrievalStrategy<Document> VALUE_RETRIEVAL_STRATEGY = new ValueRetrievalStrategy<Document>() {
    @Override
    public Object getValue(Document document, String name) {
        Object object = document.get(name);

        if(object instanceof Document){
            return (Document) object;
        }
        else if(object instanceof Map){
            return new Document((Map) object);
        }

        return object;
    }

[seanhastings]

I believe I'm running into the same issue. We're also upgrading to Grails 5.3.2 (from 3.3.10) and GORM for MongoDB 7.2.1 (from 6.1.7).

We're finding that when you're dealing with two levels or more of hasMany relationships, the second level doesn't come back from the database in a GORM call.

For example, given the following data structure:

Country {
  ...
  static hasMany = [regions: Region]
}

Region {
  ...
  static hasMany = [states: State]
}

State {
  ...
}

If I were to do a GORM call Country.findByName('USA'), accessing regions off of that would return a list of Region objects, and if you were to loop through those Regions, their properties would be populated, but the states property would be a list of null ([null]).

@lucas-napse are you just running with a local fork of this repo with your changes included in your dependencies?

[lucas-napse]

@seanhastings
Exactly. I downloaded this project, and made the change in MongoEntityPersister, only in the getValue() method, as shown in the last comment. With gradle I could compile the jar, and using that jar in our project (replacing the existing one in the gradle cache [intellij: find MongoEntityPersister class, then: Select opened files - circle in top of project tree, and in the tree right click, copy path/reference path -> absolut path]) we were able to verify that it was what solved the problem.

@puneetbehl
Please add this patch because it is an important feature which presents the bug.
In addition, the solution is available !!!
Thank you very much.

[seanhastings]

@lucas-napse Unfortunately it seems like the fix for your issue didn't fix mine, I'll need to keep digging.

[lucas-napse]

please, @puneetbehl , review this change. We need to have this solution in our system so that it can work without problems. Very thankful.

[puneetbehl]

I'm attempting to recreate the issue through testing with GORM MongoDB. Could you kindly generate a sample application that mimics the problem?

Furthermore, I believe employing the subsequent approach would offer a superior solution:

Within org/grails/datastore/mapping/mongo/engine/MongoEntityPersister.java file:

dbo.getClass().isAssignableFrom(Document.class)

In relation to the pull request, my understanding is that the procedure involves initially forking the repository, implementing your modifications, and subsequently initiating a new pull request from the forked repository. Please inform me if this method is ineffective for you.

[mgm7734]

We also urgently need this fixed.

If I generate a sample app for this problem, can I hope for at least a patch release relatively soon?

I'm also concerned that the assumption seems wide-spread that the driver returns a Document when it actually returns a LinkedHashMap.

[practical-programmer]

+1 - we faced the same issue after upgrade

[practical-programmer]

So for us issue is also caused in MongoEntityPersister but in different place. Maybe @seanhastings you can check it for yourself too:

Issue is in loadEmbeddedCollection, if embedded collection is not a map, this method will create collection and try to map instances, but it also checks only for "Document" and ignores any other values.

It happens on this line:

                Collection coll = (Collection)embeddedInstances;
                for (Object dbo : coll) {
                    if (dbo instanceof Document) { // existing entries are ignored on this line```

[practical-programmer]

But I had issue to reproduce this on fresh project and I did more debugging. The difference I found is that our migrated project did not use the codec engine. After I added grails.mongodb.engine=codec the issue seems to be gone now.

What I was not able to find for now is why codec is used for freshly generated project but is not used for my legacy one.

I could see if codec is used in MongoQuery constructor which is setting isCodecPersister

[mgm7734]

Using the grails.mongodb.engine property (my fresh project had none) fixed for me. Thanks!

I'm running into 2 more incompatible errors.

1. Embedded SortedSet doesn't work. Changing type to List fixes the problem.
2. Embedded List properties created with no entries result in null values instead of empty array. So, for example, parent.embeddeds.size() gives an NPE and needs to be changed to parent.embedddeds?.size() :? 0

[seanhastings]

@practical-programmer forgot to get back to this here with my own solution, but I also was able to fix our problem by switching to the "codec" engine, we were on "mapping".

[lucas-napse]

Hi!
Thanks for the solution grails.mongodb.engine=codec
This change of value from mapping to codec fix our problem too.

@puneetbehl
In this case, should a pull request be raised to add to the manual in the version update section, that it should be changed from "mapping" to "codec" if there is a problem with embedded entities?
This clarification would really help a lot.
Thank you very much.

[mgm7734]

Is there any documentation on grails.mongodb.engine and differencens between codec v.s. mapping?

[puneetbehl]

@lucas-napse I think it would make sense to add this to upgrade notes under documentation.

[AntonioAure]

I had that problem, I couldn't find the solution, in my case I couldn't update embedded documents and the solution I had found was adding engine:mapping in the .yml.
But I had the problem with this post that when I performed the get it did not return the elements of the list.
The solution I found was moving my embedded class to the domain folder.

[carlosorrego]

But I had issue to reproduce this on fresh project and I did more debugging. The difference I found is that our migrated project did not use the codec engine. After I added grails.mongodb.engine=codec the issue seems to be gone now.

What I was not able to find for now is why codec is used for freshly generated project but is not used for my legacy one.

I could see if codec is used in MongoQuery constructor which is setting isCodecPersister
in wich file do you add this setting? thanks