helun / Ektorp

Java API for CouchDB
Apache License 2.0
300 stars 146 forks source link

StdObjectMapperFactory Bug #69

Open jvabob opened 12 years ago

jvabob commented 12 years ago

In my application I need to register some MixIn modules to deal with 3rd party library POJO's that do not have a no arg constructor. I created a jackson ObjectMapper and registered my modules. I then created an instance of StdObjectMapperFactory and used it's setObjectMapper to put my instance of ObjectMapper into the factory. I then pass the factory to the StdCouchDbConnector constructor.

createObjectMapper works just like you would expect and passes the ObjectMapper I have given the class back out.

public synchronized ObjectMapper createObjectMapper() {
    if (instance == null) {
        instance = new ObjectMapper();
        applyDefaultConfiguration(instance);
    }
    return instance;
}

However, the StdObjectMapperFactory.createObjectMapper(CouchDbConnector) method ignores the existence of the "instance" of ObjectMapper I gave it.

public ObjectMapper createObjectMapper(CouchDbConnector connector) {
    ObjectMapper objectMapper = new ObjectMapper();
    applyDefaultConfiguration(objectMapper);
    objectMapper.registerModule(new EktorpJacksonModule(connector, objectMapper));
    return objectMapper;
}

Looks to me like the to lines

    ObjectMapper objectMapper = new ObjectMapper();
    applyDefaultConfiguration(objectMapper);

should be replaced with something like

        ObjectMapper objectMapper = createObjectMapper();

Thanks in advance!

helun commented 12 years ago

Yes StdObjectMapperFactory has a broken API, the historic reason is that a singleton instance could not be used together with the @DocumentReferences annotation. I think this might have changed with later versions of Jackson, I'll look in to it.

In the mean time I suggest that you create your own implementation of ObjectMapperFactory that work for you.

gesellix commented 11 years ago

@helun did you have some time looking into it?

In combination with the current release 1.4 I needed to add a JodaModule module to the default ObjectMapper, so I came along this issue. I guess you removed the JodaModule due to Android support, but now we need to have a more convenient way of configuring the ObjectMapper.

klehelley commented 6 years ago

Just so you know, I've been bitten by that very issue over the last couple of days. Took me some time to find out what the actual issue was, since StdObjectMapperFactory ended up being the very last class I checked. Hopefully the way Ektorp is architectured providing my own implementation was simple enough.

I'd be curious to know if the reason stated by helun's comment is still an issue or if it could be fixed. Would creating a new ObjectMapper instance from the one provided to StdObjectMapperFactory by using [ObjectMapper.copy()](https://fasterxml.github.io/jackson-databind/javadoc/2.5/com/fasterxml/jackson/databind/ObjectMapper.html#copy()) would be a good enough fix? It is not clear to me if such a change would have a negative impact of some sort...