Differences between APIs - GET before PUT

[martinsumner]

The protocol buffers API for PUT is simple.

Step 1

• If either if-none-match or if-not-modified request parameters are passed, then do a GET before the PUT (as long as the bucket is not strongly consistent). In response to the GET validate the if-none-match and/or if-not-modified criteria based on the result of the fetch, and proceed with the PUT with these request parameters stripped.

https://github.com/basho/riak_kv/blob/riak_kv-3.0.12/src/riak_kv_pb_object.erl#L286-L324

Step 2

• If neither the if-none-match or if-not-modified-request is passed (which will be the case if they are passed, once the above logic has been validated), then form a new empty riak object and replace the passed in metadata and values on that new object with those passed via the PUT. Then call riak_client:put/3:

https://github.com/basho/riak_kv/blob/riak_kv-3.0.12/src/riak_kv_pb_object.erl#L326-L403

If the bucket type is strongly consistent then if-none-match is added back into the options before call ing riak_client:put/3 in Step 2.

Most PUT requests, without conditional request parameters, will just have a Step 2, and so will not require a GET before the PUT, although there will be a local GET or HEAD (with leveled backend) at each vnode as part of the PUT process.

The If-None-Match is a standard HTTP header. The if-not-modified behaviour seems to align with the If-Match http header and not If-UnModified-Since, in that the check is ETag/Vector-Clock based not Modification Date based.

The HTTP API however, is different. In the HTTP API, regardless of the existence of conditional request headers, all PUTs have a call to riak_client:get prior to forming the PUT.

Repeatedly throughout the process of validating the request, there are calls to ensure_doc/1, which will prompt an attempt to riak_client:get/4 should the doc be undefined.

https://github.com/basho/riak_kv/blob/riak_kv-3.0.12/src/riak_kv_wm_object.erl#L301 https://github.com/basho/riak_kv/blob/riak_kv-3.0.12/src/riak_kv_wm_object.erl#L653 https://github.com/basho/riak_kv/blob/riak_kv-3.0.12/src/riak_kv_wm_object.erl#L710

Flow diagram for http handler.

The actual PUT is handled within the accept_doc_body/2 function. If the object does not exist, a new empty object is created, and then has each part replaced by the information on the inbound object. If the object does exist, the existing object is taken, and then has each part replaced by the information on the inbound object.

In the case of HTTP, the If-None-Match header is ignored, unless it is a consistent object. There is no support for If-Match to align with if-not-modified on the PB API.

So this creates the following issues:

• There is a significant non-functional difference between the API, in that the HTTP API requires an additional GET for every PUT, that the PB API does not require.
• There is a functional difference in that conditional requests are supported for non-consistent objects only via the PB API.
• There exists the potential for a functional difference as the actual object PUT is based on replacing all an empty object (PB API) rather than replacing all of the existing object (HTTP API).

There appear to be no comments explaining why the difference between these APIs exist. There is nothing in riak_test to confirm expectations on either if_none_match or if_not_modified conditional requests. The commit for this feature on PB does not explain why it was not included in the HTTP API.

[martinsumner]

This may be all related to underlying assumptions of webmachine, and the flow chart that webmachine will go through.

Functionality such as if_none_match and if_match are included in the standard webmachine flow. However, that standard flow requires the stage G7 "Resource Exists" to be passed for both GET and POST. If we bypass the check for resource existence, we may well bypass inherent features for conditional handling of HTTP headers.

Some features that may have to be handled explicitly in the PB API, may be handled implicitly by webmachine.

[martinsumner]

The riak_kv_wm_object module has to cover both GET, PUT and DELETE. Reviewing the code, there are some tricky issues for these flows.

On the GET path, one issue is that the content-type of an object returned, should be the content-type of the object which was used when it was PUT.

The natural place to fetch the resource in the webmachine flow is v3g7 - resource_exists/2. However, this only occurs after validation, including validation of the encoding the client says it will accept. However, the inbound Accept-Encoding on the request cannot be validated until the object has been fetched (and we know what content-type it is to be presented back as).

To try and ensure this works there is an ensure_doc/2 functions that does the GET, only if there is not an object (i.e doc) already on the context. This ensure_doc/2 is then called in its natural place on resource_exists/2, but also at any validation stage that may require knowledge of the content returned from the fetch. The ensure_doc/2 means the GET only occurs once, when ever information about the document is first needed.

Some of the exportable functions needed in validation such as encodings_provided are also required as part of the PUT path (e.g. to return_body). these functions may be called either before or after resource_exists on both GET and PUT. therefore we cannot simply ignore this when the method is PUT/POST.

[martinsumner]

For DELETE, the same initial path in webmachine applied as for GET. This means that DELETE will also try and GET the resource prior to calling riak_client:delete/4 or riak_client:delete_vclock/4.

This means a further unnecessary GET, but also means that {error, notfound} is treated as a failure - whereby on the PB API {error, notfound} leads to a positive response, on the basis that the object is not there (even though it not being there is potentially unexpected).

[martinsumner]

Replacing with https://github.com/basho/riak_kv/issues/1849.

This issue misunderstands the workings of webmachine. The problem is better explained in kv1849.