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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep only one JSON dependency #1045

Closed
sbernard31 opened this issue Jul 1, 2021 · 7 comments
Closed

Keep only one JSON dependency #1045

sbernard31 opened this issue Jul 1, 2021 · 7 comments
Labels
core Impact core of Leshan demo Impact our demo (not libraries) housekeeping Refactoring, cleaning code or API
Milestone

Comments

@sbernard31
Copy link
Contributor

sbernard31 commented Jul 1, 2021

Currently we depend on 3 JSON libraries 馃槚 :

  • GSON which is the historic one (it should remain only in our demos, currently it is only used in demos)
  • minimal-json which replaced it because of simple API. (but it was not replace in demo because of lack of time)
  • jackson which is the last choice mainly because it supports BigInteger which is needed to support unsigned integer from LWM2M v1.1 but also because minimal-json is not so much active. (e.g.Deadlock during Json / JsonValue class initialization聽ralfstx/minimal-json#88). We have also some hint which could make us thought that Jackson could be faster. The main drawback of jackson the lib is fat (~1,8 Mo vs ~33 ko) but I guess we will live with it. (except if someone propose a better solution 馃く)

So there is a lot of Serializer/Deserializer to rewrite with Jackson to be able to remove GSON and minimal-json dependencies

@sbernard31 sbernard31 added this to the 2.0.0 milestone Jul 1, 2021
@sbernard31 sbernard31 added core Impact core of Leshan housekeeping Refactoring, cleaning code or API demo Impact our demo (not libraries) labels Jul 1, 2021
@Michal-Wadowski
Copy link
Contributor

While I'm waiting for CR of #1098 I will take this task.

@Michal-Wadowski
Copy link
Contributor

@sbernard31 You said that we should keep GSON only in our demos. You mean subprojects with "-demo" suffix? I found in demos dependencies to leshan-core which uses minimal-json (JsonSerDes). If we want remove minimal-json/gson from non-demo projects, we have to remove it in demos also, I think.

@sbernard31
Copy link
Contributor Author

sbernard31 commented Oct 5, 2021

While I'm waiting for CR of #1098 I will take this task.

I will look at this as soon as possible. (I hope this afternoon or tomorrow)

@sbernard31 You said that we should keep GSON only in our demos. You mean subprojects with "-demo" suffix? I found in demos dependencies to leshan-core which uses minimal-json (JsonSerDes). If we want remove minimal-json/gson from non-demo projects, we have to remove it in demos also, I think.

I guess I was not clear enough.
The Idea is to keep only one JSON dependency in Leshan. (I mean using the same library everywhere)
Currently the idea is to keep only Jackson. (please do not hesitate to provide your opinion about this choice)

In the description of this issue, I try to explain the current state and how we come to this "crappy" situation. (historic reasons/choices which lead to this situation)

GSON which is the historic one (it should remain only in our demos)

"it should remain only in our demos" I didn't mean that we must do that. I wanted to say that AFAIK GSON dependency remains only in demos. (I guess my English is not so good 馃槄, I will change this)

So from my point of view the tasks could be :

  1. Be sure that Jackson is the right choice. (If you have no opinion I guess we can go with it unless your want to investigate this)
  2. Keep only 1 dependency in leshan library (I mean the reusable part of leshan), I guess this will be about removing minimal-json code.
  3. keep only 1 dependency in leshan demos, this will be about removing GSON code and probably some minimal-json code too

@Michal-Wadowski
Copy link
Contributor

No, I have no strong opinion about JSON library. I think the Jackson is quite popular in java, and if it has required feature (BigInteger) we can keep it.
BTW, I will update version to at least 2.12.1, because it's needed for complete #1096 task.

@sbernard31
Copy link
Contributor Author

About jackson upgrade, #1105 bump it to 2.12.5

@Michal-Wadowski
Copy link
Contributor

I created PR #1109

sbernard31 pushed a commit that referenced this issue Oct 25, 2021
Signed-off-by: Micha艂 Wadowski <Michal.Wadowski@orange.com>
Also-by: Simon Bernard <sbernard@sierrawireless.com>
sbernard31 pushed a commit that referenced this issue Oct 25, 2021
Signed-off-by: Micha艂 Wadowski <Michal.Wadowski@orange.com>
Also-by: Simon Bernard <sbernard@sierrawireless.com>
sbernard31 pushed a commit that referenced this issue Oct 25, 2021
Signed-off-by: Micha艂 Wadowski <Michal.Wadowski@orange.com>
Also-by: Simon Bernard <sbernard@sierrawireless.com>
sbernard31 pushed a commit that referenced this issue Oct 25, 2021
Signed-off-by: Micha艂 Wadowski <Michal.Wadowski@orange.com>
Also-by: Simon Bernard <sbernard@sierrawireless.com>
@sbernard31
Copy link
Contributor Author

Done via #1109. 馃憤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Impact core of Leshan demo Impact our demo (not libraries) housekeeping Refactoring, cleaning code or API
Projects
None yet
Development

No branches or pull requests

2 participants