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

Would like to be able to define a custom PrettyPrint class and have it used as the default by my ObjectMapper class. #689

Closed
derknorton opened this issue Jan 27, 2015 · 11 comments

Comments

@derknorton
Copy link

I have a custom MyObjectMapper class and would like to have it use a custom MyPrettyPrint class as the default pretty printer everywhere one is used by the object mapper class. Ideally, I would like to be able to do something as simple as this:

public class MyObjectMapper extends ObjectMapper {

    @Override
    protected PrettyPrinter _defaultPrettyPrinter() {
        return new MyPrettyPrinter();
    }

}

Currently, the ObjectMapper class generates new DefaultPrettyPrinter instances directly which limits the usefulness of overriding the _defaultPrettyPrinter method.

This enhancement request is a follow on to the discussion listed here.

@cowtowncoder
Copy link
Member

I am not necessarily opposed to this, but out of curiosity: is it not feasible use and share ObjectWriters instead of ObjectMapper. Being thread-safe (being immutable), and safely reconfigurable (by creating new instances from an existing one), it is recommended for much of use, similar to how ObjectReader is to be used. In fact, the idea is for ObjectMapper to mostly just become a factory for constructing readers and writers, and not to be used directly.
And specifically in this case it does allow (re)configuration with any PrettyPrinter you want.

Adding this setting in ObjectMapper is technically doable and could/should propagate to ObjectWriter. I just want to understand use cases and usage patterns better.

@derknorton
Copy link
Author

Its a valid question... We use a custom ObjectMapper called SmartObjectMapper that handles serialization and deserialization of specific custom attribute types within our RESTful resource entities that need special handling (usually using toString() ). The SmartObjectMapper class is part of another github project located here if you need to see more details on how it currently works.

We bind it together using a spring Java configuration file like the following:

@Configuration
public abstract class EntityResourceConfiguration extends BaseResourceConfiguration {

    private JacksonJsonProvider jsonProvider(Class<? extends Entity> clazz) {
        SimpleModule module = new SimpleModule();
        module.addAbstractTypeMapping(Entity.class, clazz);
        ObjectMapper mapper = new SmartObjectMapper(module);
        return new JacksonJsonProvider(mapper);
    }
...
}

The default pretty printed JSON has some quirks we would like to tweak around how lists are formatted. We would like them to use the same DefaultIndenter that is used for objects. So we would like to change the default pretty printer class that is used by our custom object mapper. Does this help clarify the need?

@digulla
Copy link

digulla commented Jan 28, 2015

To add my 2 cents: When I started using jackson, my feeling was that I should do everything via the ObjectMapper since it knows how to build and wire everything.

I understand that it's simple to create your own ObjectWriter and configure it correctly yourself ( @derknorton: You may want to look at my unit tests to see how it's done).

But when I implemented it this way, I always felt "then what is the purpose of ObjectMapper? What's the point of having a factory which you can configure in some parts and not this other part?"

Looking at the design, I realize that ObjectWriter and other classes eventually have to build more types (like the parsers) and for those, they could use ObjectMapper because the code is already there. Alas, this isn't possible because ObjectMapper doesn't inject itself everywhere; probably for the good reason to keep dependencies to a minimum.

Still, it feels like a skewed design. ObjectMapper can do a lot but it's oddly "crippled" in certain places.

@cowtowncoder
Copy link
Member

As I said earlier, excluding the fact that ObjectMapper was there first and hence was more features than it should have -- in fact, it probably should have none of readValue() or writeValue() methods -- its main reason is to be mutable/thread-safe-only-when-configured factory for ObjectReaders and ObjectWriters.

Being factory for readers/writers, it could still have defaults for instances. This is why I am not opposed to having direct method there.
I guess it is valid question as to why this particular default setting would not be configurable.
I was first thinking along lines of "well, type to bind to is not configurable", but there is difference between things that are rarely global settings (such as type to bind to/from), and ones that may well be.

Thank you for all the comments. I can implement this for 2.6.0 then.

@derknorton
Copy link
Author

I will see if there is a way to plugin just the readers and writers into the JacksonJsonProviders but so far we have always plugged in object mappers.

I think I just assumed that since there is a DefaultPrettyPrinter that I could provide an alternate PrettyPrinter ;-)

@cowtowncoder
Copy link
Member

I'll add the setter in 2.6.0, so that's fine.

The last point I'll just mention is that ObjectWriter are both fully reusable (concurrently as well), but also re-configurable. So there isn't much actual need for ObjectMapper. I don't know if this is clear from javadocs; and pretty much all external Jackson documentation uses ObjectMapper for most work; exception being methods only -Reader and -Writer have, writeValues(), and readValues().

@digulla
Copy link

digulla commented Feb 2, 2015

Maybe to clarify my point: When I stepped through the code which ObjectMapper uses to build an ObjectWriter, my first through was: I would never want to set this up myself, it's just so complicated. Another thing is that I have a many custom (de-)serializers and the mapper allows me to set them up once.

For both reasons, I was very surprised when you suggested to create the ObjectWriter directly. From the API and examples, I felt this was an internal implementation detail and I should stay away from it.

@cowtowncoder
Copy link
Member

@digulla Oh -- no, not directly instantiated at all. Indeed I would not recommend doing that. Unfortunate phrasing on my part. Used directly, not instantiated.

The expected usage pattern is to create ObjectWriter or ObjectReader initially from seed ObjectMapper, and then pass that around as the blueprint (or factory). It works as the factory via various with methods; so really one passes ObjectWriter instead of ObjectMapper.
The main downside really is that since it only allows writing JSON (et al), many use cases then require passing of both; this may be inconvenient. When using dependency injection it should be less of an issue.

@cowtowncoder
Copy link
Member

In the end implemented so that there is now method ObjectMapper.setDefaultPrettyPrinter(...), use of which does change the PrettyPrinter used when DeserializationFeature#INDENT_OUTPUT is enabled. Works for all generation whether directly from ObjectMapper or ObjectWriter.

One thing worth noting is that one case where this does not work is if JsonGenerator.useDefaultPrettyPrinter() is called: there is no way to support that use case.
But databind package does not call that method.

@digulla
Copy link

digulla commented May 13, 2015

Thanks, the new API works fine for me.

Re JsonGenerator.useDefaultPrettyPrinter(): Maybe deprecate this API to avoid confusion? Or add a comment that ObjectMapper.setDefaultPrettyPrinter() doesn't affect this call.

@cowtowncoder
Copy link
Member

@digulla yes, at least documentation should reflect it. Question of method on JsonGenerator is bit trickier. It may also come down to simply documenting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants