Hi JL,

As discussed together - but sharing for others - we must take into account
some points:

1. reading both spec, JSON-Patch enables to handle /- as your first did (ie
consider it is last element). JSON-Patch uses JSON-Pointer but nowhere it
is written it behaves as JSON-Pointer in all cases and it is typically
"integration" definition which can extend an underlying spec (otherwise
most of EE wouldn't be right? ;))
2. On johnzon point of view we can't break this feature which was requested
by user and transitive users (ie user of products built with johnzon)
without at least a clear migration path so if we want to break we should do
a 1.3 (dont think we need a 1.2 maintenance branch, we can do it lazily),
document how to migrate from current behavior to new one (i'll detail it
after) and communicate on it on our website properly (index.html ref and
dedicated page I guess with the release annoucement). Alternative is to
challenge the TCK, it is a failure case so it is typically the kind of case
we can plug custom/vendor behavior (we do in other parts of the JSON-B spec
for ex). Overall idea is to not let users on the road because some TCK
exist (functional and users over procedural work).

On strict TCK side, we can also do a johnzon-tck module where we wrap the
provider to handle that case and pass the TCK, this is purely technical to
be compliant but would avoid to break anything.
Now if we really want to be strict in our implementation we must still
enable this last element case. One option not far from what we have is to
use our json-logic module and add some jsonpatch operators. Combining
multiple operators we can manage to fulfill this common patching need - but
we break the overall API + require a new module to be added to apps).

Lastly I would note that JSON Pointer *enables* our impl:

> Note that the use of the "-" character to index an array will always

   result in such an error condition because by definition it refers to
   a nonexistent array element.  Thus, applications of JSON Pointer need
   to specify how that character is to be handled, if it is to be
   useful.


>  For example, some applications might stop pointer processing upon an

   error, while others may attempt to recover from missing values by
   inserting default ones.


Literally means "this is a case we consider as an error but your
application can recover from it" and we do ;).

Since it is an error case I would start by challenging the TCK to make it
vendor dependent and exclude it from the passing list for now.
If really blocking we can go with plan B and try to have a migration path
but it sounds like a lot of effort for everyone for literally 0 gain IMHO.

Hope it makes sense.

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>


Le ven. 27 nov. 2020 à 10:59, Jean-Louis Monteiro <jlmonte...@tomitribe.com>
a écrit :

> Hi,
>
> I was working to pass the TCK with Johnzon, but I have failures around the
> usage of "/-" in arrays.
>
> From JSON Pointer RCF https://tools.ietf.org/html/rfc6901
>
> > If the currently referenced value is a JSON array, the reference
> >       token MUST contain either:
> >
> >       *  characters comprised of digits (see ABNF below; note that
> >          leading zeros are not allowed) that represent an unsigned
> >          base-10 integer value, making the new referenced value the
> >          array element with the zero-based index identified by the
> >          token, or
> >
> >       *  exactly the single character "-", making the new referenced
> >          value the (nonexistent) member after the last array element.
> >
> >
> And then
>
> > Note that the use of the "-" character to index an array will always
> >    result in such an error condition because by definition it refers to
> >    a nonexistent array element.  Thus, applications of JSON Pointer need
> >    to specify how that character is to be handled, if it is to be
> >    useful.
> >
> >
> When I opened https://issues.apache.org/jira/browse/JOHNZON-325
> I first fixed with
>
> https://github.com/apache/johnzon/pull/68/commits/3ef4fb80bdaf6010d4ad66c481675b70bc1e4bca
>
> And we were 100% JSONP compliant but as you can see in the commit, I had
> to @Ignore some junit tests because they did not make sense.
>
> After talking with Romain, he mentioned some improvements to attempt to be
> backward compatible and still support the previous behavior.
> See commit
>
> https://github.com/apache/johnzon/pull/68/commits/2626806f82b076ada11800221c8458da8ec53794
>
> But unfortunately this is breaking the spec and the TCK.
> My understanding of JSON Pointer and as per the 2 quotes in this email, I
> think using /- for anything but ADD does not make sense and must fail (and
> this is what the spec is testing).
> JSON Path relies on JSON Pointer and therefore should not go against.
>
> So I think I should let the first fix I got in even if it breaks some edge
> cases.
>
> What do you think?
>
> Copying TomEE mailing list because it's breaking JSONP there too.
>
>
> --
> Jean-Louis Monteiro
> http://twitter.com/jlouismonteiro
> http://www.tomitribe.com
>

Reply via email to