Re: [ietf-types] Request for review: application/json-patch

[mike amundsen <[e-mail]>]

responses inline....

On Mon, Oct 24, 2011 at 19:34, Paul C. Bryan <[e-mail]> wrote:
> Hi Mike:
>
> Thanks for the review and comments. My responses, inline...
>
> On Mon, 2011-10-24 at 19:19 -0400, mike amundsen wrote:
>> Paul:
>>
>> checking the I-D document itself:
>>
>> Sec #4 Operations:
>> "It is an error condition if an operation object contains more than
>> one operation member."
>>
>> I am not clear on what this is telling me:
>> - the same JSON Pointer expression cannot appear more than once in a document
>> - something else?
>
> What I'm trying to say (evidently not very well yet!) is that you can't
> have a operation object with "add" and "remove" at the same time. Only
> one operation member per operation object.

ok, so this is probably just a "vocabulary" detail. "operation",
"operation object", "operation member" maybe a glossary or just some
simple oneline definitions up front can help clear that up.

>
>> possibly consider adding an example of a document that should be
>> rejected as it is malformed.
>
> Will do, thanks.
cool.

>
>> Sec #5 Error Handling
>> "In the event of an error condition, evaluation of the JSON Patch
>> document terminates and modification of the target document fails to
>> complete."
>>
>> Does this mean the server should treat the document as an
>> "all-or-nothing" case? when an error occurs is the document left in a
>> "partial state" or left at the "initial state" before the request? is
>> this something you want to detail in the spec? if yes, you might want
>> to use RFC 2119 words here (MAY, SHOULD, MUST, etc.) .
>
> HTTP PATCH is clear on the semantics, namely, "If the entire patch
> document cannot be successfully applied, then the server MUST NOT apply
> any of the changes." I've left this out of my spec, but I think there's
> no good reason not to be clearer by saying application of a sequence of
> patches SHOULD be atomic. Implementations are free to be even stricter
> with a MUST of their own, as HTTP PATCH is. Thoughts?

yep, the PATCH semantics are clear on that and it's a good idea to
stick with them. I'd proly reference those semantics in the I-D then.
either in the error handling section or possibly in another area of
the I-D.


>
>> It looks like your operations account for an error condition at the
>> individual "node" level:
>> "It is an error condition if the value to be replaced does not exist.", etc.
>
> Indeed.
>
>> Are there error conditions that might occur at the "document" level
>> (or some other node above the target?
>>
>> You do not indicate any use of 4xx codes here (i.e. mapping 4xx codes
>> to common error conditions in processing the patch); is this something
>> you want to include here or leave that up to the server to decide?
>
> Since JSON Patch is not specific to HTTP, I'm content to defer to HTTP
> PATCH and HTTP specifications, which amply document how to handle
> situations such as when one attempts to apply an HTTP PATCH against a
> resource that does not exist.

ok, i can see that. however, you've outline some error conditions in
your document. do you have any plans to indicate servers
[MUST|SHOULD|MAY] return the details of the errors encountered
("member to be added in the object already exists", "the specified
index is greater than the number of elements in the existing array",
etc.) and if yes, any suggestions on what the response would look
like? I'm not sure this is needed (maybe you want to leave all that to
implementors), but wanted to ask about it here.

Mike

>
> Thanks again!
>
> Paul
>
> _______________________________________________
> ietf-types mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ietf-types
>