Guice7 Module usage with XmlMapper and JsonMapper concurrently

[mattbaumann]

I've an application where I need to use the XmlMapper and JsonMapper in the same global guice injector. Therefore I need a way to distinguish between those two mappers. I know of two options here: (1) I could make use of @Named("xml") and ObjectMapper superclass to distinguish between them. Yet, (2) I like to use the type safe variant by defining the relevant subclasses (JsonMapper, XmlMapper) with Guice. I decided to go with the second route as this allows me to make use of type checking and has a generally easier Guice interfacing.

Unfortunately, in combination with Guice7 Module this requires to do the following messy initialization:

final var jsonKey = (Key<ObjectMapper>) Key.get(JsonMapper.class).ofType((Type) JsonMapper.class);
final var jsonMapper = JsonMapper.builder()
        .addModule(new JavaTimeModule())
        .build();
final var xmlKey = (Key<ObjectMapper>) Key.get(XmlMapper.class).ofType((Type) XmlMapper.class);
final var xmlMapper = XmlMapper.builder()
        .addModule(new JavaTimeModule())
        .build();
Guice.createInjector(
       new ObjectMapperModule(jsonKey).withObjectMapper(jsonMapper),
       new ObjectMapperModule(xmlKey)..withObjectMapper(xmlMapper));

Since the new ObjectMapperModule(jsonKey) constructor only allows keys of type Key<ObjectMapper> without subclass types it forces me to do ugly type and casting Magic. Therefore I propose to add a constructor to ObjectMapperModule with a definition like Key<? extends ObjectMapper>:

public ObjectMapperModule(Key<? extends ObjectMapper> objectMapperKey)
  {
      ...
  }

Which would lead to the following API:

var injector = Guice.createInjector(new ObjectMapperModule(Key.get(XmlMapper.class)).withObjectMapper(xmlMapper));
XmlMapper mapper = injector.getInstance(XmlMapper.class);

[cowtowncoder]

Sounds reasonable to me, but I am not very familiar with Guice. If you have time and interest, a PR against 2.19 would probably be a good idea (I don't think change could go in a patch, but also wouldn't need to wait for 3.0 (master) which is still some ways off from release).

[mattbaumann]

@cowtowncoder I looked into it. We have two backwards-compatible Options and I would like to leave the final call with you.

Option 1: Extend the constructor this way:

public ObjectMapperModule(Key<? extends ObjectMapper> objectMapperKey)
{
  this.objectMapperKey = (Key<ObjectMapper>) objectMapperKey;
}

This has the big drawback that (even though the java compiler guarantees through generics compatibility), the compiler cannot guarantee type compatibility and we have to catch a ClassCastException that should never be raised as you have to do some bytecode patching to provoke the error. Never the less, type compatibility is broken.

2. Option: Extend ObjectMapperModule with a generic type argument

public class ObjectMapperModule<I extends ObjectMapper> implements com.google.inject.Module

This will lead to forgotten generic argument warnings when upgrading the code:

new ObjectMapperModule() // <-- compiler warning, forgotten generic argument

and must be replaced with to suppress the warnings: new ObjectMapperModule<>() in case of ObjectMapper type or new ObjectMapperModule<XmlMapper>(Key.get(XmlMapper.class)) to select a specific implementation.

Personally, I'd prefer the second option, but cannot determine the fallout of the change. What's you're opinion on that?

[cowtowncoder]

@mattbaumann Hmmmh. I must say I am not sure either, but second option does feel like better, even if it causes warnings. But it is probably good idea to surface the change that way, as long as it is (which I think you are saying) backwards compatible wrt binary compatibility (can just upgrade version of Jackson & module without recompile) and unless build is configure to fail on warnings, source compatible as well.
So I would trust your preference here.

[mattbaumann]

Thanks. I'll prepare an pull-request, then we can have a look at it.