Serialization of Iterables and Maps is not using default codec

[hborchardt]

Hi, thanks a lot for mongojack!

We recently upgraded mongojack from 4.2.1 to 4.8.2 and the following construct no longer works:

MongoCollection<Document> collection = mongoDB.getTemplateCollection(Document.class);
collection.aggregate(List.of(new Document("$match", List.of(Filters.and(Filters.eq("a", "b"), Filters.eq("b", "c"))))))
    .into(new ArrayList<>());

throws a

org.mongojack.MongoJsonMappingException: Error mapping BSON to POJOs
	at org.mongojack.internal.stream.JacksonEncoder.encode(JacksonEncoder.java:43)
	at org.mongojack.internal.stream.JacksonCodec.encode(JacksonCodec.java:61)
	at org.bson.codecs.EncoderContext.encodeWithChildContext(EncoderContext.java:91)
	at org.bson.codecs.DocumentCodec.writeValue(DocumentCodec.java:212)
	at org.bson.codecs.DocumentCodec.encode(DocumentCodec.java:170)
	at org.bson.codecs.DocumentCodec.encode(DocumentCodec.java:44)
	at org.bson.BsonDocumentWrapper.getUnwrapped(BsonDocumentWrapper.java:199)
	at org.bson.BsonDocumentWrapper.containsKey(BsonDocumentWrapper.java:124)
	at com.mongodb.client.internal.AggregateIterableImpl.getOutNamespace(AggregateIterableImpl.java:240)
...
Caused by: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No serializer found for class com.mongodb.client.model.Filters$AndFilter and no properties discovered to create BeanSerializer (to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS) (through reference chain: java.util.ImmutableCollections$List12[0])
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:77)
	at com.fasterxml.jackson.databind.SerializerProvider.reportBadDefinition(SerializerProvider.java:1308)
	at com.fasterxml.jackson.databind.DatabindContext.reportBadDefinition(DatabindContext.java:414)
	at com.fasterxml.jackson.databind.ser.impl.UnknownSerializer.failForEmpty(UnknownSerializer.java:53)
	at com.fasterxml.jackson.databind.ser.impl.UnknownSerializer.serialize(UnknownSerializer.java:30)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider._serialize(DefaultSerializerProvider.java:479)
	at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:318)
	at com.fasterxml.jackson.databind.ObjectWriter$Prefetch.serialize(ObjectWriter.java:1572)
	at com.fasterxml.jackson.databind.ObjectWriter.writeValue(ObjectWriter.java:1061)
	at org.mongojack.internal.stream.JacksonEncoder.encode(JacksonEncoder.java:41)
	... 84 more

After some investigation, the issue arises because previously, bson 4.3.x serialized Iterables and Maps directly, which was changed for bson 4.4.0 in this PR: mongodb/mongo-java-driver#811
Now, bson delegates serialization of Iterables and Maps to the codecRegistry, which for us is mongojack's JacksonCodecRegistry. But the JacksonCodecRegistry.java:isDefault(Class<T> clazz) function does not list Iterables and Maps as classes that the default registry should handle, so Jackson tries to serialize the List and therefore also the Filters$AndFilter, which results in the changed behavior.

A workaround for us is to override the isDefault function:

    pojoCodecRegistry = new JacksonCodecRegistry(mapper, MongoClientSettings.getDefaultCodecRegistry(), null,
        UuidRepresentation.STANDARD) {
      @Override
      protected <T> boolean isDefault(Class<T> clazz) {
        if (Iterable.class.isAssignableFrom(clazz) || 
            Map.class.isAssignableFrom(clazz)) {
          return true;
        }
        return super.isDefault(clazz);
      }
    };

but maybe you would like to include this in the standard isDefault function.

[rlodge]

I can reproduce this with the code you supplied, but I'm able to fix the query without any changes to the registry.

You supplied:

        c.aggregate(
                List.of(
                    new Document(
                        "$match",
                        List.of(
                            Filters.and(
                                Filters.eq(
                                    "a",
                                    "b"
                                ),
                                Filters.eq(
                                    "b",
                                    "c"
                                )
                            )
                        )
                    )
                )
            )
            .into(
                new ArrayList<>()
            );

but I don't think { "$match": [] } is actually valid aggregation syntax. When I change it by removing the intervening list, like this:

        c.aggregate(
                List.of(
                    new Document(
                        "$match",
                        //List.of(
                            Filters.and(
                                Filters.eq(
                                    "a",
                                    "b"
                                ),
                                Filters.eq(
                                    "b",
                                    "c"
                                )
                            )
                        //)
                    )
                )
            )
            .into(
                new ArrayList<>()
            );

it works fine. So does using the Aggregates helper:

        c.aggregate(
            List.of(
                Aggregates.match(
                    Filters.and(
                        Filters.eq(
                            "a",
                            "b"
                        ),
                        Filters.eq(
                            "b",
                            "c"
                        )
                    )
                )
            )
        ).into(
            new ArrayList<>()
        );

I also am a bit reluctant to change the core behavior of the registry since it's not clear to me that I want the default behavior for maps and iterables, since I would actually expect those to be handled fine by Jackson.

Can you supply a test case where this happens with actual POJO serialization / deserialization, rather than in an aggregation query like this?

[hborchardt]

Thanks for trying it out.

I agree that it is not a valid query, just wanted to show the simplest reproduction. Actually, it happens for us in a $unionWith stage like

    Document unionStage = new Document("$unionWith", new Document("coll", "otherCollection").append("pipeline",
        List.of(Aggregates.match(Filters.and(Filters.eq("a", "b"), Filters.eq("b", "c"))))));
    collection
        .aggregate(List.of(unionStage))
        .into(new ArrayList<>());

While a valid pipeline, again, the correct way would be to use the Aggegates.unionWith function instead, which sidesteps the problem as Jackson is not involved in serializing that.

Maybe just always using the helpers is the better way, it just means that the new Document("$brandNewStage", ...) escape hatch (which the MongoDB Compass loves to generate, because it's easy) works only when the Document does not contain a helper-generated Bson value inside a List or Map.

I cannot come up with a use case for wanting to serialize Bson inside a POJO. Of course technically it is possible to write this, but there's no reason to support it:

collection.insertOne(new Document("a", List.of(Filters.eq("b", "c"))));

Just wanted to mention the changed behavior and the associated error message, if you prefer to keep it as is, no problem.

[rlodge]

The proposed solution, though, seems to pass anything contained in the map through the default serialization, or at least does in some cases. For example when I add that check to isDefault, I get a failure in org.mongojack.TestCustomObjectMapper#customObjectMapperShouldWorkForUpdate, which specifically involves maps with non-string keys. Not sure what a solution that avoided that might be.

[rlodge]

Feels to me almost like I want to approach it from the other end: jackson needs to know it's a Bson value and call the right serializer.

[rlodge]

Ok, I published (to sonatype snapshots) a version 4.8.3-SNAPSHOT (and also 5.0.0-SNAPSHOT, and FWIW, 4.11.1-SNAPSHOT) that contains a potential fix for this. It requires you configure your object mapper, though:

ObjectMapperConfigurer.configureObjectMapper(
        new ObjectMapper(),
        new MongoJackModuleConfiguration().with(MongoJackModuleFeature.ENABLE_BSON_VALUE_SERIALIZATION)
    )

You can look at this test case for examples.

Are you able to test the snapshot and see if it resolves your concerns?

[hborchardt]

That's a very thorough approach to solving my issue, much appreciated.

When running the snapshot with the configuration, I now get a different error from another part of our codebase, where we serialize a BsonNull as part of the pipeline:

    List<Bson> pipeline = Arrays.asList(new Document("$match",
        new Document("$and", Arrays.asList(new Document("receiver",
            new Document("$exists", true)
                .append("$ne",
                    new BsonNull()))))));
collection.aggregate(pipeline)
          .into(new ArrayList<>());

com.fasterxml.jackson.databind.JsonMappingException: Asked to write unknown type class org.bson.BsonNull (through reference chain: java.util.Arrays$ArrayList[0])

I guess it just needs another clause for BsonNull here:

mongojack/src/main/java/org/mongojack/internal/util/DocumentSerializationUtilsImpl.java

Line 219 in 83903f8

} else {

[rlodge]

I made a slightly different fix to that null and pushed new snapshots.

[hborchardt]

Hi, I tested the newest snapshot and it is all running fine. Thanks a lot.