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

Add RawValue helper type #743

Closed
cowtowncoder opened this issue Mar 30, 2015 · 16 comments
Closed

Add RawValue helper type #743

cowtowncoder opened this issue Mar 30, 2015 · 16 comments
Milestone

Comments

@cowtowncoder
Copy link
Member

(note: related to #737, #348)

Supporting handling of "raw values" is tricky due to various reasons. But one possibly workable way would be to allow passing of raw values as JsonToken.VALUE_EMBEDDED_OBJECT of specific type.
Since there is nothing existing that quite fits the use case it is probably best to just add a simple value type in com.fasterxml.jackson.databind.util, say, "JsonRawValue".
It should be possible to construct from java.lang.String, but possible also from other sources to support alternate forms of writeRawValue().
The main method for accessing data should be one to simply write contents using given JsonGenerator, but alternate accessors may be added as well.

Once this type is added, it should be possible to work on end-to-end handling of raw values, to support use cases where such values need to be passed through various conversions.

@cowtowncoder cowtowncoder changed the title Add JsonRawValue helper type Add RawValue helper type Apr 2, 2015
cowtowncoder added a commit that referenced this issue Apr 2, 2015
@cowtowncoder cowtowncoder added this to the 2.6.0 milestone Apr 2, 2015
cowtowncoder added a commit that referenced this issue Apr 2, 2015
@cowtowncoder
Copy link
Member Author

Additional work done:

  1. RawValue works fully with TokenBuffer, so that writeRawValue() methods that take String or SerializableString will construct RawValue under the hood; and serialization method will write it using JsonGenerator.writeRawValue()
  2. When reading from TokenBuffer, raw values are exposed as JsonToken.VALUE_EMBEDDED_OBJECT.
  3. Tree Model supports raw values as well:
    • JsonNodeFactory has rawValueNode() factory method
      *ObjectNode has putRawValue()
    • ArrayNode has addRawValue

So: it should be possible to inject raw values into TokenBuffer and JsonNode, for serialization purposes. Deserialization can not be supported, by definition, although it is possible to recover RawValue wrapper from TokenBuffer if necessary.

@martin-g
Copy link

Hi,

I've just tried to use JsonGenerator.writeRawValue() but I'm not sure whether I use it correctly.
At martin-g/wicket-jquery-selectors@ac61f56#diff-370a00d3f658d65a011325ed28899f48R19 I needed to use JsonGenerator.writeObject(new RV(someString)) instead of JsonGenerator.writeRawValue(someString) because if I use the latter Jackson serializes it as: "someKey": com.fasterxml.jackson.databind.util.RawValue@4386d484.
RV is just an extension of com.fasterxml.jackson.databind.util.RawValue that overrides #toString().

@cowtowncoder
Copy link
Member Author

@martin-g Unless there is specific reason to do so, you should call JsonGenerator.writeRawValue(...) directly. This has much less overhead and should work reliably. JsonGenerator.writeObject() is a method that perhaps should not have been added in the API because it will need a back-reference to ObjectMapper, and induces significant setup overhead. But from method definition it can appear as a simple piece of functionality.

@martin-g
Copy link

@cowtowncoder I would like to use JsonGenerator.writeRawValue(...) but I can't because it produces wrong result.
I'd like to produce "someKey": someValue (note there are no quotes around someValue). If I use JsonGenerator.writeRawValue(...) it produces "someKey": com.fasterxml.jackson.databind.util.RawValue@12345678

@cowtowncoder
Copy link
Member Author

@martin-g Hmmh. How exactly are you calling writeRawValue() here? It does not take RawValue but a String, so maybe you are accidentally callling toString() on it... ?

So I guess a code snippet would be helpful here in understanding the problem.

@martin-g
Copy link

See martin-g/wicket-jquery-selectors@ac61f56#diff-370a00d3f658d65a011325ed28899f48R18. Here I try to serialize my own custom type Json.RawValue as Jackson's RawValue.
My first attempt was to use jsonGenerator.writeRawValue(value.value()) (where value.value() returns a String). But this produced "someKey": com.fasterxml.jackson.databind.util.RawValue@12345678.
So I did my workaround - override Jackson's RawValue just to override its toString() - martin-g/wicket-jquery-selectors@ac61f56#diff-370a00d3f658d65a011325ed28899f48R34

It should be easy for you to reproduce the problem by cloning https://github.com/l0rdn1kk0n/wicket-jquery-selectors, checkout branch jackson-raw-value, change the code in src/main/java/de/agilecoders/wicket/jquery/util/serializer/RawSerializer.java to use generator.writeRawValue() and execute the tests with mvn clean test.
If this is too much for you then I'll try to create a failing unit test in Jackson tomorrow :-)

@cowtowncoder
Copy link
Member Author

@martin-g ok I will try to diagnose what is going on here.

@cowtowncoder
Copy link
Member Author

@martin-g Hmmh. I don't understand this -- I pointed that the method to use is "writeRawValue()" and NOT "writeObject()". Yet code is:

 public class RawSerializer extends JsonSerializer<Json.RawValue> {
     @Override
     public void serialize(Json.RawValue value, JsonGenerator jsonGenerator, SerializerProvider provider) throws IOException {
-        jsonGenerator.writeObject(value.value());
+        jsonGenerator.writeObject(new RV(value.value()));
+    }

so the proper call would be

   jsonGenerator.writeRawValue(value.value());

But it also seems unnecessary to add custom serializer, given that RawValue is designed to be auto-serializable by Jackson. You may of course sub-class it and override serialization method, if some changes are needed.

@martin-g
Copy link

@cowtowncoder Here is a self-contained test case:

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.module.SimpleModule;
import org.junit.Assert;
import org.junit.Test;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

/**
 * Unit test for simple App.
 */
public class AppTest {

    @Test
    public void jacksonRawValue() {
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.registerModule(new MyModule());

        MyType myType = new MyType("Jackson");

        Map<String, Object> object = new HashMap<String, Object>();
        object.put("key", myType);
        JsonNode jsonNode = objectMapper.valueToTree(object);
        String json = jsonNode.toString();
        Assert.assertEquals("{\"key\":Jackson}", json);
    }

    public static class MyTypeSerializer extends JsonSerializer<MyType> {
        @Override
        public void serialize(MyType myType, JsonGenerator jsonGenerator, SerializerProvider provider) throws IOException {
            jsonGenerator.writeRawValue(myType.value());
        }
    }

    public static class MyType {

        private final String value;

        public MyType(String value) {
            this.value = value;
        }

        public String value() {
            return value;
        }
    }

    private static class MyModule extends SimpleModule {
        public MyModule() {
            super("jackson-raw-value-test");

            addSerializer(MyType.class, new MyTypeSerializer());
        }
    }
}

It uses 2.6.0-SNAPSHOT from Sonatype OSS snapshots:

    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-core</artifactId>
      <version>2.6.0-SNAPSHOT</version>
    </dependency>

    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-databind</artifactId>
      <version>2.6.0-SNAPSHOT</version>
    </dependency>

As I explained earlier I needed to use writeObject() because #writeRawValue() produces bad output.

@martin-g
Copy link

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.util.RawValue;
import org.junit.Assert;
import org.junit.Test;

import java.util.HashMap;
import java.util.Map;

/**
 * Unit test for simple App.
 */
public class AppTest {

    @Test
    public void jacksonRawValue() {
        ObjectMapper objectMapper = new ObjectMapper();

        RawValue myType = new RawValue("Jackson");

        Map<String, Object> object = new HashMap<String, Object>();
        object.put("key", myType);
        JsonNode jsonNode = objectMapper.valueToTree(object);
        String json = jsonNode.toString();
        Assert.assertEquals("{\"key\":Jackson}", json);
    }

}

fails the same way:

org.junit.ComparisonFailure: 
Expected :{"key":Jackson}
Actual   :{"key":com.fasterxml.jackson.databind.util.RawValue@56d73c7a}

@cowtowncoder
Copy link
Member Author

Thank you, I will confirm I can see the failure.

@cowtowncoder
Copy link
Member Author

Yes, I can confirm this. Looks like this is related to intermediate TokenBuffer being used.

@cowtowncoder
Copy link
Member Author

Ok. So I did find one underlying bug, which prevented a proper raw value containing node from getting added.

But there is also a problem with the test: it assumes that jsonNode.toString() produces valid serialization. This is actually not true: because of missing context, toString() is not designed to produce proper JSON serialization, and should not be used for that purpose. It is only to be used for diagnostics. Production code should serialize nodes using writeValue() (and related) methods of ObjectMapper or ObjectWriter.

Having said that, I will go ahead and see if I can improve toString() handling in this case to actually output expected contents, as that is a reasonable expectation. :)

cowtowncoder added a commit that referenced this issue May 25, 2015
… propagated, improve `POJONode.toString()` a bit
@cowtowncoder
Copy link
Member Author

I improved output of toString() a bit, but it won't be usable for serialization as-is. Added a unit test to ensure that ObjectMapper.writeValueAsString() works correctly on the node however.

@martin-g
Copy link

ObjectMapper.writeValueAsString() works as desired!
Thank you!

@cowtowncoder
Copy link
Member Author

@martin-g good!

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

2 participants