Honor If-Match when deleting documents (#21)

Compare the document ETag and delete it in one datastore operation so a stale If-Match returns 412 and leaves the newer document in place.

Co-authored-by: Cursor Agent <[email protected]>

api/handlers/documents.go

@@ -67,7 +67,7 @@ func (h *Handlers) DeleteDocument(c *gin.Context) {
 	collectionId := c.Param("collId")
 	documentId := c.Param("docId")
 
-	status := h.dataStore.DeleteDocument(databaseId, collectionId, documentId)
+	status := h.dataStore.DeleteDocumentIfMatch(databaseId, collectionId, documentId, c.GetHeader(headers.IfMatch))
 	if status == datastore.StatusOk {
 		c.Status(http.StatusNoContent)
 		return
@@ -78,6 +78,12 @@ func (h *Handlers) DeleteDocument(c *gin.Context) {
 		return
 	}
 
+	if status == datastore.PreconditionFailed {
+		c.Header(headers.ErrorCode, "PreconditionFailed")
+		c.JSON(http.StatusPreconditionFailed, constants.PreconditionFailedResponse)
+		return
+	}
+
 	c.IndentedJSON(http.StatusInternalServerError, constants.UnknownErrorResponse)
 }
 
@@ -424,6 +430,8 @@ func dataStoreStatusToResponseCode(status datastore.DataStoreStatus) int {
 		return http.StatusConflict
 	case datastore.BadRequest:
 		return http.StatusBadRequest
+	case datastore.PreconditionFailed:
+		return http.StatusPreconditionFailed
 	default:
 		return http.StatusInternalServerError
 	}

api/tests/documents_test.go

@@ -670,3 +670,128 @@ func Test_Documents(t *testing.T) {
 		})
 	})
 }
+
+func Test_Documents_Delete_IfMatch(t *testing.T) {
+	presets := []testPreset{PresetJsonStore, PresetBadgerStore}
+
+	runTestsWithPresets(t, "Test_Documents_Delete_IfMatch", presets, func(t *testing.T, ts *TestServer, client *azcosmos.Client) {
+		collectionClient := documents_InitializeDb(t, ts)
+
+		t.Run("Should fail delete with stale etag", func(t *testing.T) {
+			ctx := context.TODO()
+
+			readResponse, err := collectionClient.ReadItem(ctx, azcosmos.PartitionKey{}, "12345", nil)
+			assert.Nil(t, err)
+			staleETag := readResponse.ETag
+			assert.NotEmpty(t, staleETag)
+
+			item := map[string]interface{}{"id": "12345", "pk": "123", "isCool": true}
+			bytes, err := json.Marshal(item)
+			assert.Nil(t, err)
+
+			_, err = collectionClient.ReplaceItem(ctx, azcosmos.PartitionKey{}, "12345", bytes, nil)
+			assert.Nil(t, err)
+
+			updated, err := collectionClient.ReadItem(ctx, azcosmos.PartitionKey{}, "12345", nil)
+			assert.Nil(t, err)
+			assert.NotEmpty(t, updated.ETag)
+			assert.NotEqual(t, string(staleETag), string(updated.ETag))
+
+			_, err = collectionClient.DeleteItem(
+				ctx,
+				azcosmos.PartitionKey{},
+				"12345",
+				&azcosmos.ItemOptions{IfMatchEtag: &staleETag},
+			)
+			assert.NotNil(t, err)
+
+			var respErr *azcore.ResponseError
+			if errors.As(err, &respErr) {
+				assert.Equal(t, http.StatusPreconditionFailed, respErr.StatusCode)
+				assert.Equal(t, "PreconditionFailed", respErr.RawResponse.Header.Get("x-ms-error-code"))
+
+				responseBody, readErr := io.ReadAll(respErr.RawResponse.Body)
+				assert.Nil(t, readErr)
+				assert.JSONEq(t,
+					`{"code":"PreconditionFailed","message":"Operation cannot be performed because one of the specified precondition is not met."}`,
+					string(responseBody),
+				)
+			} else {
+				panic(err)
+			}
+
+			document, status := ts.DataStore.GetDocument(testDatabaseName, testCollectionName, "12345")
+			assert.Equal(t, datastore.StatusOk, status)
+			assert.Equal(t, true, document["isCool"])
+			assert.Equal(t, string(updated.ETag), document["_etag"])
+		})
+
+		t.Run("Should delete with matching etag", func(t *testing.T) {
+			ctx := context.TODO()
+
+			readResponse, err := collectionClient.ReadItem(ctx, azcosmos.PartitionKey{}, "67890", nil)
+			assert.Nil(t, err)
+			etag := readResponse.ETag
+			assert.NotEmpty(t, etag)
+
+			response, err := collectionClient.DeleteItem(
+				ctx,
+				azcosmos.PartitionKey{},
+				"67890",
+				&azcosmos.ItemOptions{IfMatchEtag: &etag},
+			)
+			assert.Nil(t, err)
+			assert.Equal(t, http.StatusNoContent, response.RawResponse.StatusCode)
+
+			_, status := ts.DataStore.GetDocument(testDatabaseName, testCollectionName, "67890")
+			assert.Equal(t, datastore.StatusNotFound, status)
+		})
+
+		t.Run("Should delete unconditionally", func(t *testing.T) {
+			ctx := context.TODO()
+			_, status := ts.DataStore.CreateDocument(testDatabaseName, testCollectionName, map[string]interface{}{
+				"id": "unconditional-delete", "pk": "pk", "isCool": false,
+			})
+			assert.Equal(t, datastore.StatusOk, status)
+
+			response, err := collectionClient.DeleteItem(ctx, azcosmos.PartitionKey{}, "unconditional-delete", nil)
+			assert.Nil(t, err)
+			assert.Equal(t, http.StatusNoContent, response.RawResponse.StatusCode)
+
+			_, status = ts.DataStore.GetDocument(testDatabaseName, testCollectionName, "unconditional-delete")
+			assert.Equal(t, datastore.StatusNotFound, status)
+		})
+
+		t.Run("Should return not found when deleting a missing document", func(t *testing.T) {
+			_, err := collectionClient.DeleteItem(context.TODO(), azcosmos.PartitionKey{}, "missing-document", nil)
+			assertDocumentDeleteNotFound(t, err)
+		})
+
+		t.Run("Should return not found when deleting a missing document with if-match", func(t *testing.T) {
+			etag := azcore.ETag("\"stale-etag\"")
+			_, err := collectionClient.DeleteItem(
+				context.TODO(),
+				azcosmos.PartitionKey{},
+				"missing-document-if-match",
+				&azcosmos.ItemOptions{IfMatchEtag: &etag},
+			)
+			assertDocumentDeleteNotFound(t, err)
+		})
+	})
+}
+
+func assertDocumentDeleteNotFound(t *testing.T, err error) {
+	t.Helper()
+	assert.NotNil(t, err)
+
+	var respErr *azcore.ResponseError
+	if errors.As(err, &respErr) {
+		assert.Equal(t, http.StatusNotFound, respErr.StatusCode)
+		responseBody, readErr := io.ReadAll(respErr.RawResponse.Body)
+		assert.Nil(t, readErr)
+		assert.JSONEq(t, `{"message":"NotFound"}`, string(responseBody))
+		return
+	}
+
+	panic(err)
+}

internal/datastore/badger_datastore/documents.go

@@ -64,20 +64,36 @@ func (r *BadgerDataStore) GetDocument(databaseId string, collectionId string, do
 }
 
 func (r *BadgerDataStore) DeleteDocument(databaseId string, collectionId string, documentId string) datastore.DataStoreStatus {
+	return r.deleteDocument(databaseId, collectionId, documentId, "")
+}
+
+func (r *BadgerDataStore) DeleteDocumentIfMatch(databaseId string, collectionId string, documentId string, ifMatch string) datastore.DataStoreStatus {
+	return r.deleteDocument(databaseId, collectionId, documentId, ifMatch)
+}
+
+func (r *BadgerDataStore) deleteDocument(databaseId string, collectionId string, documentId string, ifMatch string) datastore.DataStoreStatus {
 	documentKey := generateDocumentKey(databaseId, collectionId, documentId)
 
 	txn := r.db.NewTransaction(true)
 	defer txn.Discard()
 
-	exists, err := keyExists(txn, documentKey)
-	if err != nil {
-		return datastore.Unknown
-	}
-	if !exists {
+	var document datastore.Document
+	status := getKey(txn, documentKey, &document)
+	if status == datastore.StatusNotFound {
 		return datastore.StatusNotFound
 	}
+	if status != datastore.StatusOk {
+		return status
+	}
 
-	err = txn.Delete([]byte(documentKey))
+	if ifMatch != "" {
+		etag, _ := document["_etag"].(string)
+		if etag != ifMatch {
+			return datastore.PreconditionFailed
+		}
+	}
+
+	err := txn.Delete([]byte(documentKey))
 	if err != nil {
 		logger.ErrorLn("Error while deleting document:", err)
 		return datastore.Unknown

internal/datastore/datastore.go

@@ -15,6 +15,11 @@ type DataStore interface {
 	GetDocumentIterator(databaseId string, collectionId string) (DocumentIterator, DataStoreStatus)
 	GetDocument(databaseId string, collectionId string, documentId string) (Document, DataStoreStatus)
 	DeleteDocument(databaseId string, collectionId string, documentId string) DataStoreStatus
+	// DeleteDocumentIfMatch removes a document when ifMatch is empty or equals its _etag.
+	// The comparison and delete happen together, so a concurrent update cannot land between them.
+	// A mismatched etag returns PreconditionFailed and leaves the document in place.
+	// A missing document returns StatusNotFound.
+	DeleteDocumentIfMatch(databaseId string, collectionId string, documentId string, ifMatch string) DataStoreStatus
 	CreateDocument(databaseId string, collectionId string, document map[string]interface{}) (Document, DataStoreStatus)
 
 	GetAllTriggers(databaseId string, collectionId string) ([]Trigger, DataStoreStatus)

internal/datastore/json_datastore/documents.go

@@ -45,6 +45,14 @@ func (r *JsonDataStore) GetDocument(databaseId string, collectionId string, docu
 }
 
 func (r *JsonDataStore) DeleteDocument(databaseId string, collectionId string, documentId string) datastore.DataStoreStatus {
+	return r.deleteDocument(databaseId, collectionId, documentId, "")
+}
+
+func (r *JsonDataStore) DeleteDocumentIfMatch(databaseId string, collectionId string, documentId string, ifMatch string) datastore.DataStoreStatus {
+	return r.deleteDocument(databaseId, collectionId, documentId, ifMatch)
+}
+
+func (r *JsonDataStore) deleteDocument(databaseId string, collectionId string, documentId string, ifMatch string) datastore.DataStoreStatus {
 	r.storeState.Lock()
 	defer r.storeState.Unlock()
 
@@ -56,10 +64,18 @@ func (r *JsonDataStore) DeleteDocument(databaseId string, collectionId string, d
 		return datastore.StatusNotFound
 	}
 
-	if _, ok := r.storeState.Documents[databaseId][collectionId][documentId]; !ok {
+	document, ok := r.storeState.Documents[databaseId][collectionId][documentId]
+	if !ok {
 		return datastore.StatusNotFound
 	}
 
+	if ifMatch != "" {
+		etag, _ := document["_etag"].(string)
+		if etag != ifMatch {
+			return datastore.PreconditionFailed
+		}
+	}
+
 	delete(r.storeState.Documents[databaseId][collectionId], documentId)
 
 	return datastore.StatusOk

internal/datastore/models.go

@@ -11,12 +11,13 @@ type Database struct {
 type DataStoreStatus int
 
 const (
-	StatusOk       DataStoreStatus = 1
-	StatusNotFound DataStoreStatus = 2
-	Conflict       DataStoreStatus = 3
-	BadRequest     DataStoreStatus = 4
-	IterEOF        DataStoreStatus = 5
-	Unknown        DataStoreStatus = 6
+	StatusOk           DataStoreStatus = 1
+	StatusNotFound     DataStoreStatus = 2
+	Conflict           DataStoreStatus = 3
+	BadRequest         DataStoreStatus = 4
+	IterEOF            DataStoreStatus = 5
+	Unknown            DataStoreStatus = 6
+	PreconditionFailed DataStoreStatus = 7
 )
 
 type TriggerOperation string