JacksonJsonNodeJsonProvider disallows POJOs

[ben-manes]

In 2.2.0 the provider allowed Jackson POJOs to be used as values when setting a property. This is an useful feature that I very frequently. This is also a breaking change that was not documented.

In my case the entire application is built around json schema and metadata. The schemas have triggers associated to them so that when an entity written it is inspected, a processing pipeline constructed, and operated upon. JsonPath is used at each of these stages to read and write to the entity before it is finally stored in the database, etc. The JsonNode provider is used to force all writes to normalize back into a consistent format. At least previously the JacksonJsonProvider would retain the type of the written value that could cause surprises on the next read (e.g. insert a UUID, cast exception if reading back as a String).

As of 2.3.0, using an unknown object results in an exception. It would be helpful if this restriction was reverted.

[jlolling]

Hi @ben-manes I would like to know what you actually expect as replacement for the exception.
The problem is, we have to convert an arbitrary object into a capable value and if we are not able to do so - an exception should be thrown.
We could use the toString() method to use a POJO as value - would that help you?
The previous solution was a nightmare for all working with JsonNodes directly - like me.

[ben-manes]

I'd have to retest, but at least previously the JacksonJsonProvider had inconsistent behavior. If a UUID was written to a field and then read back as a String, using the explicit class methods, it would result in a ClassCastException. The JsonNode provider ensured this was normalized to a basic type.

The API doesn't convey these restrictions and arbitrarily fail based on the providers, which is frustrating. This breaks many API design practices, e.g. principle of least astonishment. I'd be fine with a configuration on JacksonJsonNodeJsonProvider if there is no way to come to an agreement.

In your case it sounded like you didn't want to have the JsonNode copied on return. If a value is being written (potentially overwriting) then I don't think this is problematic as the subtree could be replaced. How would switching from throwing an exception to the previous behavior of tree-ifying the object break you?

[jlolling]

How would switching from throwing an exception to the previous behavior of tree-ifying the object break you?
This would render the whole project as useless because I work heavily with the JsonNodes and I actually expect a JsonPath always returns the SAME and not a "looks like" the same object.
A UUID is actually a String - how could we detect a UUID within the JSON if we only have the JSON source?
I guess you have POJOs which will render themself into a JSON? What about an additional option to allow POJOs to be handled with the object mapper valueTree method?
We do not need a this-or-nothing-solution.
I suggest, you suggest a change which does not break the same object policy and allows to use POJOs to be rendered as JSON - perhaps an additional option of the JacksonJsonProvider.

[jlolling]

What about converting your POJO into a JsonNode just before calling this method?

[jlolling]

What about replacing the exception with this code:

objectNode.set(key.toString(), objectMapper.valueToTree(value))

[ben-manes]

Yes, I think we're in agreement. 2.2.0 was incompatible with your usage, while 2.3.0 broke mine. As is, one of us would have to supply our own fork of the provider, which is unfortunate but not horrid.

I use Jackson POJOs via jsonschema2pojo, so they are mapped. I still don't see why the throws new IllegalArgumentException couldn't call valueToTree instead. If you only work with JsonNode then the behavior shouldn't change, but arbitrary objects could be written.

In my case that would be significant boilerplate, as it is used in hundreds of places (and growing). I would instead use a custom provider if need be.

[ben-manes]

What about replacing the exception with this code ...

Yes, that was my question as well. I think it would work beautifully.

[jlolling]

Sorry for breaking your use case. It was not my intention. I will suggest this change!
I am fine with this change and you also - a win-win situation.

[ben-manes]

Thanks! :)

[jlolling]

Could you explain what you mean with the UUID? I have no idea how to recognize a UUID within a Json document.

[ben-manes]

Since I use json schema, the type information is provided there. Each of my entities include a section on the schemas that they implement. Then I can inspect the schema to know what data types are present (simple like Strings or complex like an Address). Based on this, hooks are attached to definitions or schemas. When an entity is POST'ed, the schemas are used to discover all hooks that must be called, constructs a DAG, executes them, and persists. Because a hook only can be called when the entity is of that type, it knows that it conforms to its schema and a field must be a UUID. So it can deserialize / serialize in a more ad hoc fashion since the preceding validation gave it confidence.

In json schema a UUID would be represented as,

{
  "type":"object",
  "properties": {
    "id": {
      "type": "string",
      "format": "uuid"
    }
  }
}

You can see how that is translated into a POJO in the code generator.

[jlolling]

I have some difficulties to write a test case for your use case. Could you provide a test case?

[ben-manes]

You might have forgotten to configure JsonPath, since that is normally done in a global location. Here is a test that prints out the json in 2.2.0 and throws an exception in 2.3.0.

public final class JsonPathTest {

  @Test
  public void writeObject() {
    ObjectMapper mapper = new ObjectMapper();
    Configuration.setDefaults(new Defaults() {
      @Override public Set<Option> options() {
        return Collections.singleton(Option.SUPPRESS_EXCEPTIONS);
      }
      @Override public JsonProvider jsonProvider() {
        return new JacksonJsonNodeJsonProvider(mapper);
      }
      @Override public MappingProvider mappingProvider() {
        return new JacksonMappingProvider(mapper);
      }
    });

    DocumentContext context = JsonPath.parse("{}");
    context.put("$", "data", new Data("[email protected]"));
    System.out.println(context.jsonString());
  }

  public static final class Data {
    @JsonProperty("email")
    String email;

    Data(String email) {
      this.email = email;
    }
  }
}

2.2.0

{"data":{"email":"[email protected]"}}
PASSED: writeObject

2.3.0

FAILED: writeObject
java.lang.IllegalArgumentException: Cannot handle object type: JsonPathTest$Data
  at com.jayway.jsonpath.spi.json.JacksonJsonNodeJsonProvider.setValueInObjectNode(JacksonJsonNodeJsonProvider.java:273)

[jochenberger]

Hi @ben-manes ;-)
We could start off by creating a PR with the test and then see how we can fix it.

[jlolling]

Thanks a lot. I will integrate your nice test case and provide you asap a branch.

[ben-manes]

It looks like I can't reproduce my UUID / String error with the regular Jackson provider. It must have been bugs due to using read(path) and not read(path, class) consistently. The type of the object is retained and, since generics are erased, the deserializer can't infer the type if not provided. This is mostly a flaw in the API which would require a deprecation. So it may be that I can switch to the normal provider, but I think we settled on a nice compromise.

DocumentContext context = JsonPath.parse("{}");
context.put("$", "uuid", UUID.randomUUID());
context.put("$", "id", UUID.randomUUID().toString());
System.out.println(context.jsonString());

Object uuid = context.read("$.uuid");
System.out.println("uuid: " + uuid.getClass() + " " + uuid);

Object id = context.read("$.id");
System.out.println("id: " + id.getClass() + " " + id);

Object uuidToString = context.read("$.uuid", String.class);
System.out.println("uuidToString: " + uuidToString.getClass());

Object idToUUID = context.read("$.id", UUID.class);
System.out.println("idToUUID: " + idToUUID.getClass());

{"uuid":"558cf204-dfba-4ba3-bcea-daecf84017f7","id":"5e90dd02-7a8c-4a7f-bfae-32fc2173462b"}
uuid: class java.util.UUID 558cf204-dfba-4ba3-bcea-daecf84017f7
id: class java.lang.String 5e90dd02-7a8c-4a7f-bfae-32fc2173462b
uuidToString: class java.lang.String
idToUUID: class java.util.UUID

For now I'm fine since I downgraded back to 2.2.0, so I'm in no rush.

[ben-manes]

Oh, here's the test case that fails for me. The nested object isn't serialized to a tree so that a nested path can be evaluated with later. This test passes with the JsonNode provider, but fails with the Jackson object provider by returning null. It isn't clear if this is a bug or not, but is the reason I switched to JacksonJsonNodeJsonProvider.

@Test
public void writeObject() {
  ...
  DocumentContext context = JsonPath.parse("{}");
  context.put("$", "data", new Data(UUID.randomUUID()));
  System.out.println(context.jsonString());
  
  Object id = context.read("$.data.id");
  System.out.println("id: " + id.getClass() + " " + id);
}

public static final class Data {
  @JsonProperty("id")
  UUID id;

  @JsonCreator
  Data(@JsonProperty("id") UUID id) {
    this.id = id;
  }
}

JacksonJsonNodeJsonProvider

{"data":{"id":"662541c1-d14b-482e-9266-bf943139e10e"}}
id: class com.fasterxml.jackson.databind.node.TextNode "662541c1-d14b-482e-9266-bf943139e10e"
PASSED: writeObject

JacksonJsonProvider

{"data":{"id":"0eaf1092-7eff-4bdc-8f19-a53dfe1cab7d"}}
FAILED: writeObject
java.lang.NullPointerException

[jlolling]

Could you provide the full stack trace?

[jlolling]

At the moment the change cause an empty json node. Do I have to register a dedicated serializer?

[ben-manes]

No dedicated serializer should be required. You might try with the previous version to better compare if the changes don't line up.

The NPE was due to a test failure when printing because the value is null at the path. Just meant to show that limitation.

[jochenberger]

Caused by a1605a1/#211/#212