jasminb / jsonapi-converter

JSONAPI-Converter is a Java/Android library that provides support for working with JSONAPI spec
Apache License 2.0
273 stars 81 forks source link

Abstract JSON serializer #138

Closed Kevinrob closed 2 years ago

Kevinrob commented 7 years ago

This library works great! The job about JSONAPI is done. But, it's not really its responsibility to handle (de)serialization of JSON.

Mybe we can abstract this? With that, every people can choose (or implement) the (de)serialization.

Motivation: Jackson is very big (methods count) for Android...

jasminb commented 7 years ago

Hey @Kevinrob,

I agree that cutting the dependency between the library and jackson would be usefull thing to do. However, it would pose great barrier for users if they were forced to implement some kind of a bridge between their preferred JSON processor and the APIs that lib requires.

One of the things that could (potentially) be easier to do would be to give a choice to use either jackson or gson.

Kevinrob commented 7 years ago

Yes, if we do that, we must implement the adapter for Jackson at least. With that, it's not more difficult to use the library. Just adding more flexibility for advanced users.

DrPierlu commented 6 years ago

Hi, adding the possibility of using gson would be a really appreciated and useful feature

ndoyle24 commented 6 years ago

I would love to see a GSON adapter as well. I'm working on migrating a large codebase and it would be great to be able to reuse a lot of custom converters, etc, rather than having to convert it all to Jackson.

iamareebjamal commented 5 years ago

@jasminb Would you be comfortable if I create a PR for this?

Will have to split the project in core, jackson-adapter and gson-adapter module

jasminb commented 5 years ago

My idea of allowing pluggable implementations is to abstract away all the jackson specific things into set of abstractions defined by the lib itself.

Next step would be to provide 'reference' implementations based on gson and jackson with classpath-based detection to make it easier for existing project users (eg. no change at all).

I prefer not to introduce new modules but rather abstractions that need to be implemented (and are provided for jackson/gson assuming there are proper dependencies present on the classpath).

iamareebjamal commented 5 years ago

I don't think any library on Android relies on classpath based detection of libraries. If you want to maintain the project as is without introducing new modules, it will only make maintaining it difficult. See retrofit, scarlet, room or any other library. Everyone of those have pluggable adapters.

Does this mean that people will have to change their implementation and break compatibility? Yes. That's what happens with every library, retrofit, okhttp, kotlin. There are always breaking changes.

By sticking with this implementation, you're limiting the scope of the library. Which is understandable, this is your project. However, when community is offering to help, I think you should consider it.

Also, the way you're describing the extraction of Jackson under an abstraction will also break compatibility as the library is directly using ObjectMapper and JsonNode everywhere. If you wrap it under an abstraction, then also you break both binary and source compatibility.

Even after that, it's not easy to build an interface for wrapping ObjectMapper and Gson universally such that it can work with future projects like moshi or kotlin.serialization. In my opinion, it should be made available using a simple converter factory and certain utility and helper classes should be made available across implementation.

This library is quite mature and the reason for our use of this one rather than some else is that it does not require classes to be extended with a resource type. But to be honest, the API can be improved a lot. And for that, you will have to break the compatibility with older versions. My opinion will be to start a thread with people chiming in with their suggestions and pain points about the current API and design it with adapters in mind and make it 1.x

jasminb commented 5 years ago

@iamareebjamal thank you for taking the time to provide your insights into the current state of the library.

One of the reasons I am proposing creation of abstraction layer that than needs to be implemented by different serialisation providers/modules is current deep coupling with jackson (especially with JsonNode). From my perspective it could prove to be difficult to separate core from the serialisation/deserialisation engine (probably there is a simple solution here that I did not think of).

If you have the time please provide more specifics related to removing serialisation from the core and making it pluggable.

PS I am not against modularisation but I do care about backwards compatibility and giving users new features without breaking existing stuff.

iamareebjamal commented 5 years ago

Thank you. I was converting the library to GSON yesterday and indeed it is heavily tied to it. I'll have a go at it and let you know how it goes and my opinion about abstraction method