jakartaee / jsonb-api

Jakarta JSON Binding
https://eclipse-ee4j.github.io/jsonb-api/
Other
79 stars 39 forks source link

Convenience method for equating two JSON-B data objects #148

Open aguibert opened 5 years ago

aguibert commented 5 years ago

It is commonly necessary to test if two Java objects representing JSON data are equal to one another. For example, consider the following class:

public class MyData {
   public String prop1;
   public int prop2;
   public String prop3;
   @JsonbIgnore
   public String ignoreThis;
   private Set<String> extraData;
}

The current solutions for doing this are:

  1. Override Object.equals() (tedious if you have lots of properties)
  2. Use Lombok annotations. Not everyone wants to use Lombok. Additionally it does not honor JSON-B overrides such as visibility strategy to make private fields accessible, or @JsonbIgnore to make public fields hidden
  3. Use @JsonbPropertyOrder to force property ordering, call jsonb.toString(pojo) and then compare objects as strings. This is brittle and has performance drawbacks

It would be nice if there was a convenience method that would automatically compare the quality of two JSON-B objects by only comparing equality of their JSON-B properties. For example:

MyData a = // ...
MyData b = // ...

// verifies that the properties: prop1, prop2, and prop3 are equal
assertTrue(Jsonb.equals(a, b));
rmannibucau commented 5 years ago

This is a vicious one, to have implemented it [1], it has actually multiple cases (out of my head i see 4 potential valid implementations) and all break equals contract until you map the pojo to jsonvalue which has not the order issue since all impl guarantee the order in a single classloader - can be clarified at spec level.

So my 2cts would be to priviledge the toJsonValue ( added to johnzon and yasson in almost the same flavor ) and keep the parser for advanced cases.

Wdyt?

[1] https://rmannibucau.metawerx.net/post/jsonp-JsonObject-comparison-without-ordering

aguibert commented 5 years ago

I don't think any mapping to JSON-P types would be necessary here. JSON-B implementations are already capable of parsing out the JSON properties from a POJO class, so it should be a matter of inspecting those JSON properties and comparing them in an order-agnostic way.

This would be extremely valuable to be used in testing environments, where users may want to easily verify equality of their JSON-B data objects without implementing any custom logic.

If I get some time I will take a crack at this in Yasson to see if what the sticking points are.

rmannibucau commented 5 years ago

Jsonb does not bypass jsonp and if it does it is the internal introspector which is not specified at all so sounds like a big chapter to add and it is json unrelated. This is why i referenced jsonp as the main valid option.

aguibert commented 5 years ago

JSON-B uses JSON-P under the covers yes, but I'm thinking the equality checks can be made before we get down to the JSON-P layer.

You are right that the internal introspector is not specified by the spec, and it would not need to be specified in order to implement this feature. I think Sections 3.7 and 4 of the spec do a good job of describing how Java properties get mapped to JSON fields, and how to customize those default mappings. This new feature would just have to specify that those same mappings may be compared by this new convenience method in a property order agnostic way (unless perhaps @JsonbPropertyOrder is specified)

rmannibucau commented 5 years ago

As mentionned inmy first answer, order is important for equals, this is how i ended up on jsonp which teivially enable users to handle arrays and objects equality as they need since it depends the app. Adding more API for this feature sounds out of scope to me, wdyt?

aguibert commented 5 years ago

When you say "order is important for equals", are you saying it is important for equality with JSON data, POJO equality, or both? Since the proposed method only deals with POJO objects, I think order would not be important, or even possible to compare.

I don't think this feature is out of scope, it could save the user from having to write a lot of code when working with JSON-B. Consider:

MyData a = // ...
MyData b = // ...

// What I am propsing (ignores ordering, because we are just comparing values on the same class)
boolean isEqual = jsonb.equals(a, b);

// I believe this is what you are suggesting
boolean isEqual = areEqualsIgnoringOrder(jsonb.toJsonObject(a), jsonb.toJsonObject(b));

public static boolean areEqualsIgnoringOrder(final JsonValue oldValue, final JsonValue newValue) {
  // user needs to implement this
}

Before we discuss further, I want to make sure I understand your counter-proposal properly

rmannibucau commented 5 years ago

Yes, im happy to get an equals in jsonp but Jsonb.equals is very misleading since your proposal ignores json which is the reason to make jsonb.

If you want, Jsonb.equals(jsonValue, pojo) can make sense to me but not pojo/pojo one. Now you still have to define if a Collection is mapped if it is a set or a list....so you still need toggling or go through JsonValue model.

Hope it is clearer this way.

aguibert commented 5 years ago

Yes, im happy to get an equals in jsonp

Yep, I think we could use something at the JSON-P level too.

If you want, Jsonb.equals(jsonValue, pojo) can make sense to me but not pojo/pojo one.

Perhaps some use-cases to backup my reasoning would be useful. Here are a few off the top of my head:

1) JAX-RS + JSON services

When I'm writing an app that uses JSON-B, usually by the time I get a hold of the data it has already been converted to a POJO by JSON-B. The most common example here would be JAX-RS automatically converting APPLICATION_JSON data into a POJO that gets passed into the JAX-RS resource method as a parameter.

2) Integration/system testing of JAX-RS endpoints

Another situation would be integration/system testing of a REST endpoint with a type-safe REST client proxy. Again, the REST client proxy can do the JSON-->POJO conversion for us automatically, but it is nice to be able to compare equality on these objects as if we were comparing the incoming JSON data -- namely, in an order-agnostic fashion.

Consider this example:

PersonService personSvcClient = // create JAX-RS client proxy for some remote "Person" CRUD REST endpoint

@Test
public void testPersonCreated() {
  Person bob = // initialize bob
  Person created = personSvcClient.create(bob);
  assertTrue(jsonb.equals(bob, created));

  Person found = personSvcClient.getById(created.getID());
  assertTrue(jsonb.equals(bob, found));

The Person class may have 10-15 JSON properties on it, and I don't necessarily want to write an equals() method that compares all of them -- that is tedious (and the reason why we have libs like Lombok). Instead, I would like JSON-B to handle that for me because JSON-B knows which fields/methods end up as JSON properties and are things that clients of my PersonService will interact with.

3) Unit testing JSON-B data models

In JSON-B implementations (including Johnzon) we frequently compare equality of JSON-B model classes in testing scenarios. For example: https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbVisitilityTest.java Instead of needing to enumerate each property we want to assert equality on, it would be nice if we had a JSON-B convenience method to do all of this equating for us. This use-case is not just restricted to unit-tests of JSON-B implementations of course. I have found myself needing to do similar things in apps that use JSON-B as well. Right now I either A) override the boilerplate equals() method (tedious, easy to forget to update if new attrs are added) or B) write a boilerplate helper method to compare equality of of two instances of the same class. In both cases, it's boilerplate code that I think could be eliminated safely by JSON-B.


The common theme with all 3 cases is that the user is dealing strictly with POJOs. The user doesn't need to interact directly with JSON data, or even JSON-P (and that's good, that's the point of JSON-B). I'm opposed to a method like Jsonb.equals(jsonValue, pojo) because with the above use-cases we've already converted from JSON-->JsonObject-->POJO, and converting back to POJO-->JsonObject would be unnecessary.

rmannibucau commented 5 years ago

This is surely where i split a bit, here is why:

  1. Not sure where you put equals here but assuming it is idempotence or caching, jsonp is a nicer layer or your java dto code must support a correct equals+hashcode anyway so no real issue IMO,
  2. Sounds like you want a pojo equals here too and if not then jsonp is also a nice layer supported by typed clients, in particular since we will get from/to jsonvalue methods so comparison becomes trivial and at the right level. If not the toJsonValue solution sounds sane and explicit (vs jsonb.xxx not being json related),
  3. Same than 2 + you will likely not use it in jsonb impl to ensure to limit the test scope but that is another story we dont care there.

So back to the point that jsonp is really the level to tackle it IMHO and on jsonb side, we must keep the work to standardize toJsonValue(pojo) - toJsonStructure in yasson - method to enable that use case to be smooth.

No?