search

eclipse-archived / smarthome

Eclipse SmartHome™ project
https://www.eclipse.org/smarthome/
Eclipse Public License 2.0
861 stars 784 forks source link

Evaluate and potentially use JSON-P #4741

Open sjsf opened 6 years ago

sjsf commented 6 years ago

...as a replacement for GSON. With JSR-374 there is a Java 8 compatible standard library now.

So while there might be good reasons for sticking to GSON, IMHO we should at least make them transparent as

kaikreuzer commented 6 years ago

FTR: JSON-P is now even an Eclipse project, so it should be our natural choice: https://github.com/eclipse-ee4j/jsonp

I'd vote for replacing our recommendation for gson by json-p, although it means that we will have to carry around both for quite a while, I'm afraid...

Tony080 commented 6 years ago

I'd vote for replacing our recommendation for gson by json-p, although it means that we will have to carry around both for quite a while, I'm afraid...

I agree with @kaikreuzer. It will cost much time to do the migration. For JSON-P, it has a special interface JsonObject to store the JSON as a Java Object. And for GSON, you know, every simple POJO can be converted to JSON. As you can see, they used totally different strategies. So I guess that's a time-consuming job.

htreu commented 6 years ago

Thanks @Tony-Hu for pointing out this huge feature gap. I did take a first look at JSON P and it seems to be "just" a very low level JSON API right now. In case I didn't overlook something it does not match our requirements for a GSON replacement.

flaviocosta-net commented 6 years ago

I had a closer look into this topic, JSON-P indeed only provides a low-level JSON processing API, there is no automatic JSON-to-Java binding functionality that makes Gson convenient to work with POJOs.

Jersey actually has EclipseLink MOXy as its default/recommended JSON support library, but while it is considered a fully-featured implementation, it comes with a footprint of at least 623 Kb. The comparison may not be completely accurate, because using Gson with Jersey requires having a custom GsonProvider, but the size difference still seems to be significant.

If the priority is to have a compact implementation and convenient POJO data binding, it looks like Gson should remain so this issue should be closed accordingly, unless we want to implement this data binding on top of JSON-P and then see if there are any savings at the end.

htreu commented 6 years ago

Imho we should not deal with data binding for serialisation/deserialisation ourselves. @sjka as you brought up this topic: Please close or comment accordingly.

sjsf commented 6 years ago

while there might be good reasons for sticking to GSON, IMHO we should at least make them transparent

With the findings above, I happily consider this to be done. Thanks!!!

Tony080 commented 6 years ago

I found that there exists a "JSON-B" project from JSR-367. Eclipse has an official implementation and named it Yasson. JSON-B/Yasson is based on JSON-P, and it has the functionality to convert a POJO to JSON and vice versa. Perhaps JSON-B/Yasson is what we are looking for to be the candidate to replace Gson. One problem is, Yasson is 289 KB but Gson is 227KB as sjka mentioned from start.

kaikreuzer commented 6 years ago

Thanks @Tony-Hu, that looks indeed like the missing piece! I therefore suggest to re-open this issue as it after all looks like if this can be a proper replacement.

flaviocosta-net commented 6 years ago

@Tony-Hu, nice finding, thanks! If the direction to go is to replace Gson with JSON-B due to it being a new standard, regardless of the implementation size, we can try that one. Since I'm already working on the (de)serialization for the sitemap REST API, I will create a branch to implement a test using JSON-B and see how well it works - eventually in a replacement for GsonProvider.

kaikreuzer commented 6 years ago

Excellent, @flaviocosta-net, looking forward to hear about your experiences! 👍

htreu commented 6 years ago

One thing to look out for is the ability to write a JSON String to an output writer. We use the JsonWriter to stream JSON responses.

https://github.com/eclipse/smarthome/blob/efadb967febd8648955cc41ee4664b3ef208205c/bundles/io/org.eclipse.smarthome.io.rest/src/main/java/org/eclipse/smarthome/io/rest/JSONResponse.java#L170-L172

flaviocosta-net commented 6 years ago

Just tried it, the result serializing DTOs with Gson and JSON-B is equivalent.

This is the Gson output:

[
  {
    "version": 2,
    "type": "smarthome",
    "name": "newdemo",
    "label": "New Demo Sitemap",
    "link": "http://localhost:8080/rest/sitemaps/newdemo",
    "homepage": {
      "link": "http://localhost:8080/rest/sitemaps/newdemo/rendering"
    }
  },
  {
    "version": 2,
    "type": "smarthome",
    "name": "dynamic",
    "label": "My home automation",
    "link": "http://localhost:8080/rest/sitemaps/dynamic",
    "homepage": {
      "link": "http://localhost:8080/rest/sitemaps/dynamic/rendering"
    }
  },
  {
    "version": 1,
    "name": "demo",
    "label": "Demo Sitemap",
    "link": "http://localhost:8080/rest/sitemaps/demo",
    "homepage": {
      "timeout": false,
      "leaf": false,
      "widgets": [],
      "link": "http://localhost:8080/rest/sitemaps/demo/demo"
    }
  }
]

And this with JSON-B:

[
    {
        "homepage": {
            "link": "http://localhost:8080/rest/sitemaps/newdemo/rendering"
        },
        "label": "New Demo Sitemap",
        "link": "http://localhost:8080/rest/sitemaps/newdemo",
        "name": "newdemo",
        "type": "smarthome",
        "version": 2
    },
    {
        "homepage": {
            "link": "http://localhost:8080/rest/sitemaps/dynamic/rendering"
        },
        "label": "My home automation",
        "link": "http://localhost:8080/rest/sitemaps/dynamic",
        "name": "dynamic",
        "type": "smarthome",
        "version": 2
    },
    {
        "homepage": {
            "link": "http://localhost:8080/rest/sitemaps/demo/demo",
            "leaf": false,
            "timeout": false,
            "widgets": [
            ]
        },
        "label": "Demo Sitemap",
        "link": "http://localhost:8080/rest/sitemaps/demo",
        "name": "demo",
        "version": 1
    }
]

Functionally they are very similar (i.e. JSON-B is very "inspired" on Gson). Gson offers a method toJson(Object, Appendable) while JSON-B has toJson(Object, Writer). The code to generate the JSON response is nearly identical with both solutions.

However, for it to work I had to add a total of 7 jar files (626 Kb) under targetplatform\third-party:

  1. cdi-api-1.2.jar
  2. javax.el-api-3.0.0.jar
  3. javax.interceptor-api-1.2.jar
  4. javax.json-1.1.jar
  5. javax.json-api-1.1.jar
  6. javax.json.bind-api-1.0.0-RC2.jar
  7. yasson-1.0.0-RC1.jar

I still need to see how to fix an issue when working with generics. From a technical or functional point of view, there seems to be no advantage of using JSON-B, other than it being the de jure standard for JSON data binding since J2EE 8.

If there is consensus that this is the way to go, I can create a PR to replace the GsonProvider with a Jsonb-based implementation for REST response serialization, the code for that is almost ready.

afuechsel commented 6 years ago

Although JSON-P and -B is a Java standard, it is IMHO a JEE (sorry. JakartaEE) standard. As we are not using JEE but JavaSE I would not replace GSON with the above mentioned as we would add several other dependencies to the runtime, as @flaviocosta-net already mentioned. GSON works like a charm, is small and easy to use, so why should we replace it?

JochenHiller commented 6 years ago

Thanks @flaviocosta-net for your investigation. Normally I would also vote for following standards if somehow possible and appropriate. But the "price" seems to be very high: increase footprint by 2-3x the size of GSON (626 kB compared to GSON with about 235 kB), without a real benefit. Is there any chance that this huge number of dependencies can be reduced? Sometimes stuff is not really used, and just by making import-packages optional OSGi can resolve a bundle even if dependencies would not be there. And no problem if they are not used. But this would only make sense if the projects would accept PRs to bring this into these libraries. Maintaining own versions of these bundles would be the worst case and contradict to follow standards.

afuechsel commented 6 years ago

@JochenHiller I think the dependencies are mainly due to the fact, that JSON libs are JEE and not JSE. JSE just does not provide these dependencies.

kaikreuzer commented 6 years ago

@afuechsel You should have learned yesterday that JSON-P is part of Eclipse MicroProfile 1.0 and thus meant to be pretty tiny and self-contained. Definitely without any dependencies to Java EE (and also not Jakarta EE).

flaviocosta-net commented 6 years ago

@JochenHiller, I only added the jar files after the runtime complained about the classes missing, so unfortunately none of those dependencies can be made optional. There's also another issue I didn't mention: JSON-B/Yasson relies on the org.glassfish.json package, which is included on javax.json-1.1.jar but is not exported by its MANIFEST. Therefore, I also needed to add this jar directly to the build path of the project using it, which is not ideal.

@kaikreuzer, while this may be true for JSON-P, the JSON-B implementation seems to rely on those EE dependencies.

afuechsel commented 6 years ago

@flaviocosta-net one more point for NOT adding the JEE libs to a JSE runtime.

kaikreuzer commented 6 years ago

You are right, JSON-B actually only comes with MicroProfile 2.0 (which is supposed to be release any day now) - for this, I would expect that Yasson should be a suitable implementation (which itself claims that it only requires JavaSE8). The dependency on the org.glassfish.json package, if it still exists, is probably rather a bug than a real requirement.

flaviocosta-net commented 6 years ago

Is the idea to add MicroProfile 2.0 and all its dependencies to the ESH distribution? If that's the case, then it could make sense to replace Gson with JSON-B when that happens, since these requirements would already be there. This obviously assumes that the issue with generics and org.glassfish.json are resolved in the meantime, so we don't lose functionality or need workarounds to have it working properly.

kaikreuzer commented 6 years ago

Is the idea to add MicroProfile 2.0 and all its dependencies to the ESH distribution?

Clearly not - I only pointed to it as the idea of the MicroProfile is to not depend on EE either and thus "all its dependencies" should merely be JavaSE. But clearly MicroProfile is just a specification and as such defines the APIs only and does not deal with the implementations. So it might indeed be that JSON-B has no EE dependencies, while Yasson (due to it history) requires EE stuff like javax.el etc.

sjsf commented 6 years ago

The org.glassfish.json JAR is titled with "JSR 374:Java API for Processing JSON RI". Which in turn only is JSON-P. Sounds like some legacy. Therefore I'd second the assumption that this either will be changed or otherwise is a bug which should be fixed.

JochenHiller commented 6 years ago

I did further tests how JSON-P, JSON-B and their RIs does work in OSGi in general. And about their pros and cons about use in ESH.

Short summary of my findings:

I created a list of bugs to address my findings in these projects. See

Meanwhile I would propose to stick at Gson, until these basics in JSON-P, JSON-B are clarified. And then we should check again the compact 2 compliance and the different footprints from Gson compared to JSON-B.

@mmilinkov FYI

flaviocosta-net commented 6 years ago

@JochenHiller, thanks for the detailed analysis, it sounds like we may eventually want to migrate to JSON-B, but there's still some improvements or corrections missing on their side.

Everyone, as you can see on the reference above, I have created a PR to help make this transition easier in the future. Please let me know if there is anything wrong with that (I see there is a conflict that needs to be resolved).

kgoderis commented 4 years ago

@kaikreuzer After reading all of the above, and being more than a year later, I was wondering if anything has changed with respect to the choice between gson and json-p/b ?

I have some code that I am building (with json-p), and I would like to know if I am in for a conversion exercise or not

kaikreuzer commented 4 years ago

I cannot speak for ESH, but for openHAB the decision was to stick with Gson, see https://github.com/openhab/openhab-core/pull/611#issuecomment-482280535.