Cosmos state store ignores ETags for multi operations

[dillenmeister]

Expected Behavior

Trying to upsert a state (using multi) with an ETag that does not match the ETag in cosmos should not update the document (when the concurrency mode is first write). A transaction should fail if at least one of the ETags does not match.

Actual Behavior

The ETag is ignored and the state is always updated. This behavior is expected if the concurrency mode is last write, but not if it's first write.

Steps to Reproduce the Problem

This request (incorrectly) succeeds, regardless of what the etag value is.

curl --request POST \
  --url http://localhost:3500/v1.0/state/statestore/transaction \
  --header 'Content-Type: application/json' \
  --data '{
	"operations": [
		{
			"operation": "upsert",
			"request": {
				"key": "key1",
				"value": "myData",
				"etag": "etag1"
			}
		}
	],
	"metadata": {
		"partitionKey": "partition1"
	},
	"options": {
		"concurrency": "first-write"
	}
}'

Release Note

RELEASE NOTE:

[dillenmeister]

This is the code in the stored procedure that upserts the document.

components-contrib/state/azure/cosmosdb/storedprocedure.go

Lines 48 to 55 in 7ec4708

	function tryCreate(doc, callback) {
	var isAccepted = container.upsertDocument(collectionLink, doc, callback);
	// fail if we hit execution bounds
	if (!isAccepted) {
	throw new Error("upsertDocument() not accepted, please retry");
	}
	}

I think it should be something like this instead.

var isAccepted = container.upsertDocument(collectionLink, doc, { etag: doc.etag }, callback);

This is apparently not documented (Azure/azure-cosmosdb-js-server#49), but it can be found in the source code: https://azure.github.io/azure-cosmosdb-js-server/DocDbWrapperScript.js.html#line748

[dillenmeister]

Two other notes:

1. A quick test I did shows that this works correctly wth both redis and sql server as state stores
2. Deletes in transactions should also consider the ETag. I did not test this because it is not relevant for my use case, but a fix should probably also include that.

[dillenmeister]

Initially I didn't know if the bug was in the dotnet-sdk or in the dapr component, so I reproduced it using an asp net core app. I know it's not an issue with the sdk now, so I updated the issue description with a curl request instead and moved the program to this comment instead.

The first endpoint ("/save") which doesn't use the transaction API works as expected. The second endpoint ("/save-transaction") does not.

using Dapr.Client;
using System.Text.Json;

var builder = WebApplication.CreateBuilder(args);

builder.Services
    .AddControllers()
    .AddDapr();

var app = builder.Build();

// Saves state using regular save API. Works as expected.
app.MapPost("/save", async (DaprClient dapr) =>
{
    // 1. Save some state
    var key = Guid.NewGuid().ToString();
    await dapr.SaveStateAsync("statestore", key, new TestState());  

    // 2. Get the saved state and the etag that cosmos generated
    var (state, etag) = await dapr.GetStateAndETagAsync<TestState>("statestore", key);

    // 3. Save the state with a matching etag. The state is overwritten as expected and cosmos will generate 
    //    a new etag.
    await dapr.TrySaveStateAsync("statestore", key, new TestState { Value = "first save" }, etag); 

    // 4. Save the state with the same etag, which no longer match the etag in cosmos. This returns false 
    //    and does not overwrite the document as expected.
    await dapr.TrySaveStateAsync("statestore", key, new TestState { Value = "second save" }, etag);

    // 5. Check that the state was not overwritten
    var stateAfter = await dapr.GetStateAsync<TestState>("statestore", key);
    if (stateAfter.Value == "second save")
        throw new Exception("Second save should not succeed");
});

// Saves state using transactions API. Does not work as expected.
app.MapPost("/save-transaction", async (DaprClient dapr) =>
{
    // 1. Save some state
    var key = Guid.NewGuid().ToString();
    await dapr.SaveStateAsync<TestState>("statestore", key, new TestState());

    // 2. Get the saved state and the etag that cosmos generated
    var (state, etag) = await dapr.GetStateAndETagAsync<TestState>("statestore", key);

    // 3. Save the state with a matching etag. The state is overwritten as expected and cosmos will generate 
    //    a new etag.
    await dapr.ExecuteStateTransactionAsync("statestore", new List<StateTransactionRequest>
    {
        new StateTransactionRequest(
            key, 
            JsonSerializer.SerializeToUtf8Bytes<TestState>(new TestState { Value = "first save" }, dapr.JsonSerializerOptions), 
            StateOperationType.Upsert, 
            etag)
    });

    // 4. Save the state with the same etag, which no longer match the etag in cosmos. This unexpectedly 
    //    overwrites the state.
    await dapr.ExecuteStateTransactionAsync("statestore", new List<StateTransactionRequest>
    {
        new StateTransactionRequest(
            key, 
            JsonSerializer.SerializeToUtf8Bytes<TestState>(new TestState { Value = "second save" }, dapr.JsonSerializerOptions), 
            StateOperationType.Upsert, 
            etag)
    });

    // 5. This throws since the state was overwritten
    var stateAfter = await dapr.GetStateAsync<TestState>("statestore", key);
    if (stateAfter.Value == "second save")
        throw new Exception("Second save should not succeed");
});

app.Run();

public class TestState
{
    public string Value { get; set; }
}

[berndverst]

The Cosmos DB State Store component was completely rewritten using a different SDK. Etag in transactions should be handled correctly now. Code here:

components-contrib/state/azure/cosmosdb/cosmosdb.go

Line 355 in 973ff8f

func (c *StateStore) Multi(request *state.TransactionalStateRequest) (err error) {

Closing this issue as fixed since Dapr 1.9!