Deprecating the json.org parser in sling commons json

[Alexander Klimetschek <ak...@adobe.com>]

tl;dr deprecate json.org classes in sling commons json, and point people to use the real json.org library

Hi,

Sling commons.json includes a very old version of the json.org implementations (state pre 2009, apart from few important bug fixes).

Most notably, it is not useful for parsing json, since the sling version can only work with strings [1] as input. This is obviously bad for memory usage and performance, espcially with larger json files, if you have to convert a request stream or binary (from a resource) into a string first, meaning you end up with potentially 3 in-memory representations of the json (binary stream, string & parsed jsonobject structure). json.org has stream & reader support for a long time [2]. (Also, Apache Jackrabbit has a streaming parser that can be useful for some cases, but that's a different topic [3]).

However, just updating the copy of these classes in commons.json to the newest version or json.org won't work, since the APIs are not backwards compatible (most notably, JsonTokener methods now throwing JsonExceptions they did not before). You could find a hacky way to port it, but then future updates will be again difficult.

The sling commons.json was added and used in Sling mainly for _generating_ json, not _parsing_ it (to dump jcr structures as json or write servlets generating json for client-side consumption). It also has a bunch of extra stuff for that case (groovy, jsonwriter etc.). IIRC, it also keeps the order when writing json objects, which isn't standardized in json, and doesn't apply when you parse json (and sling has an option to use arrays nowadays for exporting jcr as json).

However, people using the sling stack will naturally start using Sling commons json for all their json stuff, so they unknowingly will run into the above problem(s) when they use it for parsing.

Therefore I think there should be a clear deprecation note on the JSONObject et al classes, "when you use this for parsing json, use org.json instead".

wdyt?

[1] https://github.com/apache/sling/blob/trunk/bundles/commons/json/src/main/java/org/apache/sling/commons/json/JSONTokener.java#L53
[2] https://github.com/douglascrockford/JSON-java/blob/master/JSONTokener.java#L74
[3] http://jackrabbit.apache.org/api/2.2/org/apache/jackrabbit/commons/json/JsonParser.html

Cheers,
Alex

Re: Deprecating the json.org parser in sling commons json

[Alexander Klimetschek <ak...@adobe.com>]

On 28.05.2015, at 12:55, Konrad Windszus <ko...@gmx.de> wrote:
> 
> If we deprecate the parsing functionality of commons.json we should rather point to JSR 353

Good point, we should point to all of them. These seem to be pretty new, so not sure how ready they are.

> (https://json-processing-spec.java.net/nonav/releases/1.0/fcs/javadocs/index.html) and one of the implementations (https://jsonp.java.net/ or http://owlike.github.io/genson/ AFAIK) as an alternative instead of the not really maintained json.org implementation (https://github.com/douglascrockford/JSON-java/blob/master/README).

FWIW, it is maintained by someone else now, see commit history: https://github.com/douglascrockford/JSON-java/commits/master

Cheers,
Alex

Re: Deprecating the json.org parser in sling commons json

[Konrad Windszus <ko...@gmx.de>]

If we deprecate the parsing functionality of commons.json we should rather point to JSR 353 (https://json-processing-spec.java.net/nonav/releases/1.0/fcs/javadocs/index.html) and one of the implementations (https://jsonp.java.net/ or http://owlike.github.io/genson/ AFAIK) as an alternative instead of the not really maintained json.org implementation (https://github.com/douglascrockford/JSON-java/blob/master/README).
Since JSR 353 does provide a streaming API (https://json-processing-spec.java.net/nonav/releases/1.0/fcs/javadocs/javax/json/stream/package-summary.html) and JSR 353 together with the reference implementation is part of Java EE 7 I consider it the most reasonable choice nowadays.

I cannot really say whether that would be a good choice for writing JSON objects in Sling as well, because i guess its streaming API keeps the order.
Konrad



> Am 28.05.2015 um 21:06 schrieb Alexander Klimetschek <ak...@adobe.com>:
> 
> tl;dr deprecate json.org classes in sling commons json, and point people to use the real json.org library
> 
> Hi,
> 
> Sling commons.json includes a very old version of the json.org implementations (state pre 2009, apart from few important bug fixes).
> 
> Most notably, it is not useful for parsing json, since the sling version can only work with strings [1] as input. This is obviously bad for memory usage and performance, espcially with larger json files, if you have to convert a request stream or binary (from a resource) into a string first, meaning you end up with potentially 3 in-memory representations of the json (binary stream, string & parsed jsonobject structure). json.org has stream & reader support for a long time [2]. (Also, Apache Jackrabbit has a streaming parser that can be useful for some cases, but that's a different topic [3]).
> 
> However, just updating the copy of these classes in commons.json to the newest version or json.org won't work, since the APIs are not backwards compatible (most notably, JsonTokener methods now throwing JsonExceptions they did not before). You could find a hacky way to port it, but then future updates will be again difficult.
> 
> The sling commons.json was added and used in Sling mainly for _generating_ json, not _parsing_ it (to dump jcr structures as json or write servlets generating json for client-side consumption). It also has a bunch of extra stuff for that case (groovy, jsonwriter etc.). IIRC, it also keeps the order when writing json objects, which isn't standardized in json, and doesn't apply when you parse json (and sling has an option to use arrays nowadays for exporting jcr as json).
> 
> However, people using the sling stack will naturally start using Sling commons json for all their json stuff, so they unknowingly will run into the above problem(s) when they use it for parsing.
> 
> Therefore I think there should be a clear deprecation note on the JSONObject et al classes, "when you use this for parsing json, use org.json instead".
> 
> wdyt?
> 
> [1] https://github.com/apache/sling/blob/trunk/bundles/commons/json/src/main/java/org/apache/sling/commons/json/JSONTokener.java#L53
> [2] https://github.com/douglascrockford/JSON-java/blob/master/JSONTokener.java#L74
> [3] http://jackrabbit.apache.org/api/2.2/org/apache/jackrabbit/commons/json/JsonParser.html
> 
> Cheers,
> Alex