Skip to content

Commit c173b02

Browse files
authored
Ignore 404 errors in transactional cosmos delete operations (#3894)
Signed-off-by: Albert Callarisa <[email protected]>
1 parent 1fdd753 commit c173b02

File tree

2 files changed

+56
-13
lines changed

2 files changed

+56
-13
lines changed

state/azure/cosmosdb/cosmosdb.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,14 +567,27 @@ func (c *StateStore) Multi(ctx context.Context, request *state.TransactionalStat
567567
}
568568

569569
if !batchResponse.Success {
570-
// Transaction failed, look for the offending operation
570+
var transactionError error
571+
571572
for index, operation := range batchResponse.OperationResults {
573+
// delete operations with no etag check are allowed to fail with a 404
574+
deleteReq, isDelete := request.Operations[index].(state.DeleteRequest)
575+
if isDelete && operation.StatusCode == http.StatusNotFound && !deleteReq.HasETag() {
576+
continue
577+
}
578+
572579
if operation.StatusCode != http.StatusFailedDependency {
573580
c.logger.Errorf("Transaction failed due to operation %v which failed with status code %d", index, operation.StatusCode)
574581
return fmt.Errorf("transaction failed due to operation %v which failed with status code %d", index, operation.StatusCode)
575582
}
583+
transactionError = errors.New("transaction failed")
584+
}
585+
586+
// If all errors are from delete operations with a 404 (and no etag check), we end up here with a nil error.
587+
// This is expected, as we allow delete operations to fail with a 404.
588+
if transactionError != nil {
589+
return transactionError
576590
}
577-
return errors.New("transaction failed")
578591
}
579592

580593
// Transaction succeeded

tests/conformance/state/state.go

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,8 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
296296
query: `
297297
{
298298
"filter": {
299-
"OR": [
300-
{
299+
"OR": [
300+
{
301301
"AND": [
302302
{
303303
"EQ": {"message": "` + key + `message"}
@@ -310,7 +310,7 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
310310
}
311311
]
312312
},
313-
{
313+
{
314314
"AND": [
315315
{
316316
"EQ": {"message": "` + key + `message"}
@@ -702,6 +702,27 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
702702
}
703703
})
704704

705+
t.Run("delete non-existent key", func(t *testing.T) {
706+
// for CosmosDB
707+
partitionMetadata := map[string]string{
708+
"partitionKey": "myPartition",
709+
}
710+
operations := []state.TransactionalStateOperation{
711+
state.DeleteRequest{
712+
Key: "non-existent-key",
713+
},
714+
}
715+
716+
transactionStore, ok := statestore.(state.TransactionalStore)
717+
require.True(t, ok)
718+
719+
err := transactionStore.Multi(t.Context(), &state.TransactionalStateRequest{
720+
Operations: operations,
721+
Metadata: partitionMetadata,
722+
})
723+
require.NoError(t, err)
724+
})
725+
705726
t.Run("transaction-order", func(t *testing.T) {
706727
// Arrange
707728
firstKey := key + "-key1"
@@ -796,6 +817,12 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
796817

797818
metadataTest1 := map[string]string{
798819
"contentType": "application/json",
820+
// for CosmosDB
821+
"partitionKey": "myPartition",
822+
}
823+
metadataTest2 := map[string]string{
824+
// for CosmosDB
825+
"partitionKey": "myPartition",
799826
}
800827

801828
operations := []state.TransactionalStateOperation{
@@ -817,13 +844,17 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
817844

818845
expectedMetadata := map[string]map[string]string{
819846
keyTest1: metadataTest1,
847+
keyTest2: metadataTest2,
820848
}
821849

822850
// Act
823851
transactionStore, ok := statestore.(state.TransactionalStore)
824852
assert.True(t, ok)
825853
err := transactionStore.Multi(t.Context(), &state.TransactionalStateRequest{
826854
Operations: operations,
855+
Metadata: map[string]string{
856+
"partitionKey": "myPartition",
857+
},
827858
})
828859
require.NoError(t, err)
829860

@@ -833,17 +864,16 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
833864
Key: k,
834865
Metadata: expectedMetadata[k],
835866
})
836-
expectedValue := res.Data
867+
require.NoError(t, err)
868+
receivedValue := res.Data
837869

838870
// In redisjson when set the value with contentType = application/Json store the value in base64
839-
if strings.HasPrefix(string(expectedValue), "\"ey") {
840-
valueBase64 := strings.Trim(string(expectedValue), "\"")
841-
expectedValueDecoded, _ := base64.StdEncoding.DecodeString(valueBase64)
842-
require.NoError(t, err)
843-
assert.Equal(t, expectedValueDecoded, v)
871+
if strings.HasPrefix(string(receivedValue), "\"ey") {
872+
valueBase64 := strings.Trim(string(receivedValue), "\"")
873+
receivedValueDecoded, _ := base64.StdEncoding.DecodeString(valueBase64)
874+
assert.JSONEq(t, string(v), string(receivedValueDecoded))
844875
} else {
845-
require.NoError(t, err)
846-
assert.Equal(t, expectedValue, v)
876+
assert.JSONEq(t, string(v), string(receivedValue))
847877
}
848878
}
849879
}

0 commit comments

Comments
 (0)