Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In io.quarkus.jackson.ObjectMapperCustomizer, use JsonMapper instead of ObjectMapper #24019

Open
chonton opened this issue Feb 28, 2022 · 10 comments
Assignees
Labels
area/jackson Issues related to Jackson (JSON library) kind/enhancement New feature or request

Comments

@chonton
Copy link
Contributor

chonton commented Feb 28, 2022

Description

As of 2.10, JsonMapper appears to be the preferred system.

This will require changing the default ObjectMapper producer and change the interface of ObjectMapperCustomizer.

What backward compatibility is required?

Implementation ideas

package io.quarkus.jackson;

import com.fasterxml.jackson.databind.ObjectMapper;

import com.fasterxml.jackson.databind.json.JsonMapper;
import io.quarkus.jackson.runtime.ObjectMapperProducer;
import org.jboss.logging.Logger;

/**
 * Meant to be implemented by a CDI bean that provides arbitrary customization for the default {@link ObjectMapper}.
 * <p>
 * All implementations (that are registered as CDI beans) are taken into account when producing the default
 * {@link ObjectMapper}.
 * <p>
 * See also {@link ObjectMapperProducer#objectMapper}.
 */
public interface ObjectMapperCustomizer extends Comparable<ObjectMapperCustomizer> {

    Logger log = Logger.getLogger(ObjectMapperCustomizer.class);

    int MINIMUM_PRIORITY = Integer.MIN_VALUE;
    // we use this priority to give a chance to other customizers to override serializers / deserializers
    // that might have been added by the modules that Quarkus registers automatically
    // (Jackson will keep the last registered serializer / deserializer for a given type
    // if multiple are registered)
    int QUARKUS_CUSTOMIZER_PRIORITY = MINIMUM_PRIORITY + 100;
    int DEFAULT_PRIORITY = 0;

    /**
     * Customize the ObjectMapper.  By default, does nothing
     * @param objectMapper The ObjectMapper to customize.
     * @deprecated Use {@link #customize(JsonMapper)}
     */
    @Deprecated(since = "")
    default void customize(ObjectMapper objectMapper) {
    }

    /**
     * Customize the JsonMapper
     * @param jsonMapper The JsonMapper to customize.
     */
    default void customize(JsonMapper jsonMapper) {
        log.info("Implement customize(JsonMapper), as customize(ObjectMapper) is deprecated");
    }

    /**
     * Defines the priority that the customizers are applied.
     * A lower integer value means that the customizer will be applied after a customizer with a higher priority
     */
    default int priority() {
        return DEFAULT_PRIORITY;
    }

    default int compareTo(ObjectMapperCustomizer o) {
        return Integer.compare(o.priority(), priority());
    }
}
@chonton chonton added the kind/enhancement New feature or request label Feb 28, 2022
@quarkus-bot quarkus-bot bot added the area/jackson Issues related to Jackson (JSON library) label Feb 28, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 28, 2022

/cc @geoand, @gsmet

@kdubb
Copy link
Contributor

kdubb commented Mar 1, 2022

For outward compatibility producing a JsonMapper will fulfill injection requirements for ObjectMapper.

@geoand
Copy link
Contributor

geoand commented Mar 1, 2022

This will require changing the default ObjectMapper producer and change the interface of ObjectMapperCustomizer.
What backward compatibility is required?

This is a pretty severe breaking change that could affect a lot of people. I am not sure I want to do that just yet.

@geoand
Copy link
Contributor

geoand commented Mar 1, 2022

#23645 is also related

@Eng-Fouad
Copy link
Contributor

This will require changing the default ObjectMapper producer and change the interface of ObjectMapperCustomizer.
What backward compatibility is required?

This is a pretty severe breaking change that could affect a lot of people. I am not sure I want to do that just yet.

What about in Quarkus 3.0?

@geoand
Copy link
Contributor

geoand commented Feb 2, 2023

Perhaps

@Karm Karm self-assigned this Sep 12, 2023
@FWest98
Copy link
Contributor

FWest98 commented Jun 3, 2024

An additional complexity here is that there are two "layers" of customization in Jackson: the main one is the mapper itself (through some MapperBuilder), but the mapper builder itself can also be instantiated with a factory (used by the eventual mapper to create things at runtime), and this factory also has a builder interface. It is very convoluted, unfortunately, but the workflow is roughly as follows:

val factoryBuilder = JsonFactoryBuilder(JsonFactory())
// apply customizations on the factoryBuilder
val factory = factoryBuilder.build()

val mapperBuilder = JsonMapper.builder(factory)
// apply customizations for the mapperBuilder
val mapper = mapperBuilder.build()

Luckily, both these levels can be made more generic using the backing interfaces TSFBuilder and MapperBuilder, so it is possible to implement generic customizations that are applicable to any implementation of these builders - so all Quarkus customizations will be applied to all mappers (not just JSON, but also YAML/CSV/etc)

At my own work, I implemented a version of this that exposes a generic customizer interface that can be used on all levels: either all <factory/mapper> builders, or only the builders specific to a type such as JSON/YAML/CSV. For reference: https://gitlab.com/rug-digitallab/resources/common-quarkus/-/merge_requests/33/diffs. This implementation is currently fully backwards-compatible with the existing customizers.

I would be happy to port this approach to Quarkus at some point when you deem this relevant.

@geoand
Copy link
Contributor

geoand commented Jun 4, 2024

That seems interesting @FWest98!

Would you like to open a draft PR showing what you have in mind?

@kdubb
Copy link
Contributor

kdubb commented Nov 11, 2024

We already do this in our codebase by adding producers for JsonMapper, YamlMapper and CborMapper. All of these are limited to their specific type (with @Typed(JsonMapper.class)) and use the ObjectMapperCustomizer list to configure them equivalent to the standard ObjectMapper.

In our code we don't use ObjectMapper almost anywhere, instead injecting the specific mapper we want/need. We also wired these into JAX-RS to allow @Consumes/@Produces to select the correct variant and use the specific mapper. Note that the JAX-RS integration requires subclassing and replacing some of the Jackson provided @Provider classes because they use */* as the media type which means you get JSON because those classes only use the ObjectMapper type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/jackson Issues related to Jackson (JSON library) kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants