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

Inconsistencies between ObjectMapper#writeValue(File/Writer/OutputStream, Object) variants and #writeValue(JsonGenerator, Object) variant #12

Closed
kdonald opened this issue Apr 19, 2012 · 10 comments

Comments

@kdonald
Copy link

kdonald commented Apr 19, 2012

The following methods:
ObjectMapper#writeValue(OutputStream, Object)
ObjectMapper#writeValue(File, Object)
ObjectMapper#writeValue(Writer, Object)
ObjectMapper#writeValueAsString(Object)
ObjectMapper#writeValueAsBytes(Object)

all call _configAndWriteValue(JsonGenerator jgen, Object value) internally. This internal method applies several "Features" such as pretty printing and closeable support that are configurable at the root ObjectMapper level via ObjectMapper#configure(...).

Unfortunately, when calling:
ObjectMapper#writeValue(JsonGenerator, Object)

.... which is necessary when supporting the different JsonEncoding options, _configAndWriteValue is NOT called and any configured ObjectMapper features such as pretty printing are not applied.

This may be by design, but it is somewhat confusing since I used the ObjectMapper to get the JsonFactory that constructed the JsonGenerator. Given that, I expect when I set the following option:

    ObjectMapper objectMapper = new ObjectMapper();
    objectMapper.configure(SerializationFeature.INDENT_OUTPUT, true);

... that JsonGenerators created by the ObjectMapper would have the feature applied. I agree it should be possible to override a custom JsonGenerator's properties externally, but I would still expect the initial values to match what was configured on the root ObjectMapper.

You can see this causing some confusion in a Spring MVC context here:
http://stackoverflow.com/questions/6541757/when-using-spring-mvc-for-rest-how-do-you-enable-jackson-to-pretty-print-render

@cowtowncoder
Copy link
Member

My first thinking is that yes, it is intentional: assumption being that if you pass a JsonGenerator or JsonParser, you have configured to your exact liking.

However, question wrt ObjectMapper.configure(SerializationFeature.INDENT_OUTPUT, true) is interesting -- I can see why it should probably trigger configuration of JsonGenerator to use the default PrettyPrinter (or none, if false) given.

The only minor concern here is that it is still behavioral change and as such should probably go in 2.1, not in a patch version. Although it seems unlikely that app code would be counting on JsonGenerator NOT being configured there.

@kdonald
Copy link
Author

kdonald commented Apr 19, 2012

Here's how I worked around it in our Spring MVC context:

@Override
protected void writeInternal(Object object, HttpOutputMessage outputMessage)
        throws IOException, HttpMessageNotWritableException {

    JsonEncoding encoding = getJsonEncoding(outputMessage.getHeaders().getContentType());
    JsonGenerator jsonGenerator =
            this.objectMapper.getJsonFactory().createJsonGenerator(outputMessage.getBody(), encoding);
    // this is a workaround for the fact JsonGenerators created by ObjectMapper#getJsonFactory do not have ObjectMapper serialization features applied.
    // see https://github.com/FasterXML/jackson-databind/issues/12
    if (objectMapper.isEnabled(SerializationFeature.INDENT_OUTPUT)) {
        jsonGenerator.useDefaultPrettyPrinter();
    }
    try {
        if (this.prefixJson) {
            jsonGenerator.writeRaw("{} && ");
        }
        this.objectMapper.writeValue(jsonGenerator, object);
    }
    catch (JsonProcessingException ex) {
        throw new HttpMessageNotWritableException("Could not write JSON: " + ex.getMessage(), ex);
    }
}

@cowtowncoder
Copy link
Member

Is the reason to use JsonGenerator just to be able to use writeRaw()? Otherwise it would seem that you could just use ObjectMapper?

@kdonald
Copy link
Author

kdonald commented Apr 19, 2012

The reason is support for the various JsonEncodings.

@cowtowncoder
Copy link
Member

I see, thanks! Another possible improvement would be to add a way to pass encoding for ObjectWriter. But that is sort of orthogonal to the original issue here.

@prestoncrawford
Copy link

So this still doesn't work? I'm kind of stunned. I've worked through numerous versions of Jackson and countless posts on StackOverflow, etc. I'm using Jackson 2.0.5 and I'm amazed that this functionality isn't there yet. I'm looking to adopt Spring MVC with REST right now and this is the stumbling block. I've tried getting INDENT_OUTPUT to work with Jackson 1, then Jackson 2. I've upgraded Spring to do this. Nothing works and then I find this. Is there a light at the end of the tunnel? I don't really fancy unpacking Jackson and inserting my own code. That seems insane. Unless I'm missing something here.

@cowtowncoder
Copy link
Member

What functionality? Amazed how? What specific problem are you having? (same as the original one? something else?)

And as mentioned this should not be done for 2.0.x anyway: it is a backwards-incompatible change in behavior.

@prestoncrawford
Copy link

I'm having the same problem as everyone else is. I create my own ObjectMapper in some fashion and try to set the INDENT_OUTPUT SerializationFeature.

mapper.configure(SerializationFeature.INDENT_OUTPUT, true);

I couldn't get this to work without extending MappingJackson2HttpMessageConverter and overwriting writeInternal() with my own version of the method that makes the explicit call to use the Pretty Printer.

    if (super.getObjectMapper().isEnabled(SerializationFeature.INDENT_OUTPUT)) {
        jsonGenerator.useDefaultPrettyPrinter();
    }

It's something that looks like it should work and I beat my head against the wall for quite a while trying to figure out why it didn't. I found posts that said it was fixed in 2.0.x.

@cowtowncoder
Copy link
Member

Ok. Thank you for clarification. I will try to make sure it gets resolved for 2.1 -- and if it seems safe enough, perhaps even for 2.0.

My main concern is that in some cases users could define PrettyPrinter first, and if so that should not be overridden. But presumably they would not use the feature in that case.

@cowtowncoder
Copy link
Member

Implemented for 2.1 and 2.0.6. If anyone can verify locally (from 2.0 branch or master), would appreciate help; there are multiple affected code paths.

jhoeller added a commit to spring-projects/spring-framework that referenced this issue Jul 9, 2014
… 2.1+ createGenerator API

Also removing workaround for FasterXML/jackson-databind#12 (fixed in 2.1+)

Issue: SPR-11262
lfarmer pushed a commit to lfarmer/spring-framework that referenced this issue Jul 10, 2014
… 2.1+ createGenerator API

Also removing workaround for FasterXML/jackson-databind#12 (fixed in 2.1+)

Issue: SPR-11262
christophercurrie pushed a commit to christophercurrie/jackson-databind that referenced this issue Jul 19, 2015
Fix for JsonParser.getValueAsLong()
codicusmaximus pushed a commit to codicusmaximus/jackson-databind that referenced this issue Mar 10, 2017
Convert jackson.datatype.guava Optional test cases to jdk8
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