Switch back to json-simple as dependency

Issue #152 closed
Jan Høydahl created an issue

I don’t like this static forked json-simple fork inside the code base. Perhaps a good choice back then, but I’d prefer going back to external dep. Browsed the forks of the original repo and looks like this is a maintained fork:

https://github.com/cliftonlabs/json-simple

With quite some releases over the years, newest this march:
https://mvnrepository.com/artifact/com.github.cliftonlabs/json-simple

Comments (6)

  1. Travis Spencer

    As long as JSON dependencies are up for discussion, I think the best would be to use JSR 353 (Java API for JSON Processing). That way, different implementations can be used, and standard support is available.

  2. Brian Campbell repo owner

    But JSON dependencies aren’t really up for discussion. It’s working fine for the intended purpose and I have no plans to change it anytime soon.

  3. Travis Spencer

    Then, I think it would be better not to require a new transitive dependency. That could be a breaking change.

  4. Jan Høydahl reporter

    The background for this issue was the comment in JsonUtil :

    The code in the internal.json_simple package was originally taken from the Apache 2.0 licensed json-simple source code (changing the package). I'm a little uneasy about it. But json-simple hasn't changed much in years and jose4j only needs fairly basic JSON processing. Doing this lets me remove one more dependency and avoid any potential dependency conflicts. It also will let me to make changes to the JSON processing like not escaping forward slashes. There’s some risk in this but moving to a new/different processor in the future isn’t really made particularly more difficult by this (as long as this class is the touch point for JSON processing).

    And being 5 years since the fork, the decision could perhaps be challenged. Making it a dependency again would remove 2515 out of 17519 lines of code (15%) to maintain. I see the benefit of avoiding an external dependency. I also see that the cliftonlabs fork has fixed some of the same issues as this project, so the tradeoff is whether being dependency-free outweighs the burden of maintaining the forked classes, keep them free from security vulnerabilities etc.

    Don’t get me wrong. I respect your choice, just wanted to provide the background and reasoning for the proposal 🙂

  5. Brian Campbell repo owner

    That’s fair Jan.

    But please understand that at this point in the project, which is being used in major deployments all around the world, I’m taking an “if it ain’t broke, don’t fix it” attitude to maintenance of this stuff. There is real risk with real implications to any change that’s introduced so I personally need sufficient justification or reason for changes. I haven’t had to touch or even look at the JSON processing in years. Which to me means it’s working just fine 🙂

  6. Log in to comment