UpsertItemStreamAsync returns Created when IfMatchEtag condition doesn't match and item does not exist

[rorezende]

Version 3.6

Describe the bug UpsertItemStreamAsync returns Created when IfMatchEtag condition doesn't match and item does not exist.

Repro Call UpsertItemStreamAsync with a stream that corresponds to an Item that does not exist in the collection (e.g. item.Id = Guid.NewGuid().ToString()) but set ItemRequestOptions.IfMatchEtag = "W/\"123456789\""

Expected behavior response.StatusCode should be HttpStatusCode.PreconditionFailed

Actual behavior response.StatusCode is HttpStatusCode.Created

Environment summary Tested using the CosmosDb emulator

Additional context Although there is no item for the given Id, there is an Access Condition IfMatch that cannot be satisfied, since there is no corresponding item with such Etag in the collection.

Wonder if Upsert API relies on the POST https://docs.microsoft.com/en-us/rest/api/cosmos-db/create-a-document with header "x-ms-documentdb-is-upsert" true. Reason is that https://docs.microsoft.com/en-us/rest/api/cosmos-db/common-cosmosdb-rest-request-headers says the "If-Match" is "applicable only on PUT and DELETE".

Other APIs

The Table API returns the expected PreconditionFailed for the corresponding operation InsertOrReplace. Tested using Azure Storage.

[jhulbertpmn]

I believe the issue is that the stream operations only throw on client exceptions. I ran into a similar issue with UpsertItemStreamAsync. You can check for the precondition failed status code yourself and throw an exception though - something like this (pardon the formatting - the github source formatting refuses to cooperate) :

ItemRequestOptions iro = new ItemRequestOptions { EnableContentResponseOnWrite = false, IfMatchEtag = viewResp.ETag }; await _viewContainer .UpsertItemStreamAsync(stream, new Microsoft.Azure.Cosmos.PartitionKey(viewFromDb.PK), iro) .ContinueWith(task => { using ResponseMessage response = task.Result;

                            if (response.StatusCode == HttpStatusCode.PreconditionFailed)
                            {
                                throw new CosmosException("Upsert failed - etag mismatch",
                                    HttpStatusCode.PreconditionFailed,
                                    0, string.Empty, 0);
                            }

                            //other errors
                            if (!response.IsSuccessStatusCode)
                            {
                                _logger.LogWarning("Exception writing to cosmos {status}", response.StatusCode);
                            }
                        });

[j82w]

@rorezende I was able to reproduce the behavior where the if-match does not cause a 412 on creates. I will respond later once I get a chance to confirm your theory.

@jhulbertpmn I would recommend doing the following instead. Please take a look at the exception documentation.

if (response.StatusCode == HttpStatusCode.PreconditionFailed)
{
           // This will throw a CosmosException on any failure case, and will keep all the related info
           response.EnsureSuccessStatusCode();
}

[j82w]

@rorezende if you have an etag then you know the item should exists so you can just call replace instead of upsert. The replace will fail with a 404 if the item has been deleted, and a 412 if the etag has a conflict.

The "IfMatchEtag" does not apply to create operations, and is just ignored. It doesn't make since that a create would have a "IfMatchEtag" support because it would always fail if it was set.

[rorezende]

@j82w

Actually, such scenario you described is just one possibility, which there are other ways for the user handle it. Not sure we want to make the assumption that is the only intended scenario from user.

Another perspective: one could be holding a state in the client, maybe some cached value, and in the meanwhile, the item in the service was deleted. When the client attempts to upsert with such state or any alteration on the client state, with your proposal, it would lead to Created. Now, I don't think that is coherent, because the client assumed that the IF-MATCH would be applied preventing creating with an invalid state. Such approach limits the user on what can be done. If the user wanted to force Created/Updated regardless of Etag, user would not set the IfMatch (or set to null ;-) ). Following my suggested behavior, the API would allow the user to not get unwanted created items when it holds an outdated state.

There must be other scenarios, I think we don't need to know all of them.

The RFC 7232, https://tools.ietf.org/html/rfc7232, says that "An origin server that receives an If-Match header field MUST evaluate the condition prior to performing the method." and later "An origin server MUST NOT perform the requested method if a received If-Match condition evaluates to false;". I don't see any exception for that.

So may I ask if the If-Match condition is evaluated in the scenario I described?

If so, then (1) it seems the predicates resolves to true: not null/empty clientEtag matching an inexistent serverEtag. Otherwise, the precondition was not evaluated (2).

Both (1) and (2) are not conforming to the standard in my opinion. The case (2), obviously because it is not evaluating. The case (1), arguably, because I'd say it's not a match. I attempted to seek more info in the RFC about case (1) but didn't spot anything obvious. Finally, the TableAPI provided both by Azure Storage and CosmosDb responds with Precondition failure, hinting into the direction of my proposed expected behavior for the case (1).

[ghost]

Closing due to in-activity, pease feel free to re-open.