Added conformance tests for DeleteWithPrefix (#3288)

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>

state/requests.go

@@ -76,7 +76,7 @@ type DeleteWithPrefixRequest struct {
 }
 
 func (r *DeleteWithPrefixRequest) Validate() error {
-	if r.Prefix == "" {
+	if r.Prefix == "" || r.Prefix == "||" {
 		return fmt.Errorf("a prefix is required for deleteWithPrefix request")
 	}
 	if !strings.HasSuffix(r.Prefix, "||") {

state/sqlite/sqlite.go

@@ -101,7 +101,7 @@ func (s *SQLiteStore) Multi(ctx context.Context, request *state.TransactionalSta
 	return s.dbaccess.ExecuteMulti(ctx, request.Operations)
 }
 
-// DeleteWithPrefix deletes an actor's state
+// DeleteWithPrefix deletes objects with a prefix.
 func (s *SQLiteStore) DeleteWithPrefix(ctx context.Context, req state.DeleteWithPrefixRequest) (state.DeleteWithPrefixResponse, error) {
 	return s.dbaccess.DeleteWithPrefix(ctx, req)
 }

state/sqlite/sqlite_dbaccess.go

@@ -421,21 +421,17 @@ func (a *sqliteDBAccess) Delete(ctx context.Context, req *state.DeleteRequest) e
 }
 
 func (a *sqliteDBAccess) DeleteWithPrefix(ctx context.Context, req state.DeleteWithPrefixRequest) (state.DeleteWithPrefixResponse, error) {
-	if req.Prefix == "" {
-		return state.DeleteWithPrefixResponse{}, fmt.Errorf("missing prefix in delete with prefix operation")
-	}
-	ctx, cancel := context.WithTimeout(ctx, a.metadata.Timeout)
-	defer cancel()
-
 	err := req.Validate()
 	if err != nil {
 		return state.DeleteWithPrefixResponse{}, err
 	}
 
+	ctx, cancel := context.WithTimeout(ctx, a.metadata.Timeout)
+	defer cancel()
+
 	// Concatenation is required for table name because sql.DB does not substitute parameters for table names.
 	//nolint:gosec
-	result, err := a.db.ExecContext(ctx, "DELETE FROM "+a.metadata.TableName+" WHERE prefix = ?",
-		req.Prefix)
+	result, err := a.db.ExecContext(ctx, "DELETE FROM "+a.metadata.TableName+" WHERE prefix = ?", req.Prefix)
 	if err != nil {
 		return state.DeleteWithPrefixResponse{}, err
 	}

state/store.go

@@ -21,6 +21,9 @@ import (
 	"github.com/dapr/components-contrib/metadata"
 )
 
+// ErrPingNotImplemented is returned by Ping if the state store does not implement the Pinger interface
+var ErrPingNotImplemented = errors.New("ping is not implemented by this state store")
+
 // Store is an interface to perform operations on store.
 type Store interface {
 	metadata.ComponentWithMetadata
@@ -58,11 +61,11 @@ func Ping(ctx context.Context, store Store) error {
 	if storeWithPing, ok := store.(health.Pinger); ok {
 		return storeWithPing.Ping(ctx)
 	} else {
-		return errors.New("ping is not implemented by this state store")
+		return ErrPingNotImplemented
 	}
 }
 
-// DeleteWithPrefix is an interface to delete objects with a prefix.
+// DeleteWithPrefix is an optional interface to delete objects with a prefix.
 type DeleteWithPrefix interface {
 	DeleteWithPrefix(ctx context.Context, req DeleteWithPrefixRequest) (DeleteWithPrefixResponse, error)
 }

tests/config/state/tests.yml

@@ -1,4 +1,4 @@
-# Supported operations: transaction, etag, first-write, query, ttl
+# Supported operations: transaction, etag, first-write, query, ttl, delete-with-prefix
 # Supported config: 
 # - badEtag: string containing a value for the bad etag, for exaple if the component uses numeric etags (default: "bad-etag")
 componentType: state
@@ -55,7 +55,7 @@ components:
       # This component requires etags to be UUIDs
       badEtag: "e9b9e142-74b1-4a2e-8e90-3f4ffeea2e70"
   - component: sqlite
-    operations: [ "transaction", "etag",  "first-write", "ttl" ]
+    operations: [ "transaction", "etag",  "first-write", "ttl", "delete-with-prefix" ]
   - component: mysql.mysql
     operations: [ "transaction", "etag",  "first-write", "ttl" ]
   - component: mysql.mariadb

tests/conformance/state/state.go

@@ -337,7 +337,7 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
 		// so will only assert require.NoError(t, err) finally, i.e. when current implementation
 		// implements ping in existing stable components
 		if err != nil {
-			require.EqualError(t, err, "ping is not implemented by this state store")
+			require.ErrorIs(t, err, state.ErrPingNotImplemented)
 		} else {
 			require.NoError(t, err)
 		}
@@ -575,7 +575,7 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
 			}
 
 			transactionStore, ok := statestore.(state.TransactionalStore)
-			assert.True(t, ok)
+			require.True(t, ok)
 			sort.Ints(transactionGroups)
 			for _, transactionGroup := range transactionGroups {
 				t.Logf("Testing transaction #%d", transactionGroup)
@@ -704,7 +704,12 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
 			}
 		})
 	} else {
-		t.Run("transactional feature not present", func(t *testing.T) {
+		t.Run("component does not implement TransactionalStore interface", func(t *testing.T) {
+			_, ok := statestore.(state.TransactionalStore)
+			require.False(t, ok)
+		})
+
+		t.Run("Transactional feature not present", func(t *testing.T) {
 			features := statestore.Features()
 			assert.False(t, state.FeatureTransactional.IsPresent(features))
 		})
@@ -1302,6 +1307,109 @@ func ConformanceTests(t *testing.T, props map[string]string, statestore state.St
 			})
 		})
 	}
+
+	if config.HasOperation("delete-with-prefix") {
+		keys := map[string]bool{
+			"prefix||key1":          true,
+			"prefix||key2":          true,
+			"prefix||prefix2||key3": true,
+			"other-prefix||key1":    true,
+			"no-prefix":             true,
+		}
+		validateFn := func() func(t *testing.T) {
+			return func(t *testing.T) {
+				for key, exists := range keys {
+					res, err := statestore.Get(context.Background(), &state.GetRequest{Key: key})
+					require.NoErrorf(t, err, "Error retrieving key '%s'", key)
+					if exists {
+						require.NotEmptyf(t, res.Data, "Expected key '%s' to be not empty", key)
+					} else {
+						require.Emptyf(t, res.Data, "Expected key '%s' to be empty, but contained data: %s", key, string(res.Data))
+					}
+				}
+			}
+		}
+
+		var statestoreDeleteWithPrefix state.DeleteWithPrefix
+		t.Run("component implements DeleteWithPrefix interface", func(t *testing.T) {
+			var ok bool
+			statestoreDeleteWithPrefix, ok = statestore.(state.DeleteWithPrefix)
+			require.True(t, ok)
+		})
+
+		t.Run("DeleteWithPrefix feature present", func(t *testing.T) {
+			features := statestore.Features()
+			require.True(t, state.FeatureDeleteWithPrefix.IsPresent(features))
+		})
+
+		t.Run("set test data", func(t *testing.T) {
+			err := statestore.BulkSet(context.Background(), []state.SetRequest{
+				{Key: "prefix||key1", Value: []byte("Ovid, Metamorphoseon")},
+				{Key: "prefix||key2", Value: []byte("In nova fert animus mutatas dicere formas")},
+				{Key: "prefix||prefix2||key3", Value: []byte("corpora; di, coeptis (nam vos mutastis et illas)")},
+				{Key: "other-prefix||key1", Value: []byte("adspirate meis primaque ab origine mundi")}, // Note this still has "prefix||" but not at the start of the string
+				{Key: "no-prefix", Value: []byte("ad mea perpetuum deducite tempora carmen.")},
+			}, state.BulkStoreOpts{})
+			require.NoError(t, err)
+
+			t.Run("all keys are set", validateFn())
+		})
+
+		require.False(t, t.Failed(), "Cannot continue if previous test failed")
+
+		t.Run("delete with prefix", func(t *testing.T) {
+			res, err := statestoreDeleteWithPrefix.DeleteWithPrefix(context.Background(), state.DeleteWithPrefixRequest{
+				// Does not delete "prefix||prefix2||key3"
+				Prefix: "prefix||",
+			})
+			require.NoError(t, err)
+			assert.Equal(t, int64(2), res.Count)
+
+			keys["prefix||key1"] = false
+			keys["prefix||key2"] = false
+
+			t.Run("validate keys present", validateFn())
+		})
+
+		t.Run("delete with prefix appends ||", func(t *testing.T) {
+			res, err := statestoreDeleteWithPrefix.DeleteWithPrefix(context.Background(), state.DeleteWithPrefixRequest{
+				// Appends || automatically
+				Prefix: "other-prefix",
+			})
+			require.NoError(t, err)
+			assert.Equal(t, int64(1), res.Count)
+
+			keys["other-prefix||key1"] = false
+
+			t.Run("validate keys present", validateFn())
+		})
+
+		t.Run("error when prefix is empty", func(t *testing.T) {
+			_, err := statestoreDeleteWithPrefix.DeleteWithPrefix(context.Background(), state.DeleteWithPrefixRequest{
+				Prefix: "",
+			})
+			require.Error(t, err)
+			require.ErrorContains(t, err, "prefix is required")
+		})
+
+		t.Run("error when prefix is ||", func(t *testing.T) {
+			_, err := statestoreDeleteWithPrefix.DeleteWithPrefix(context.Background(), state.DeleteWithPrefixRequest{
+				Prefix: "||",
+			})
+			require.Error(t, err)
+			require.ErrorContains(t, err, "prefix is required")
+		})
+	} else {
+		t.Run("component does not implement DeleteWithPrefix interface", func(t *testing.T) {
+			_, ok := statestore.(state.DeleteWithPrefix)
+			require.False(t, ok)
+		})
+
+		t.Run("DeleteWithPrefix feature not present", func(t *testing.T) {
+			features := statestore.Features()
+			require.False(t, state.FeatureDeleteWithPrefix.IsPresent(features))
+		})
+	}
 }
 
 func assertEquals(t *testing.T, value any, res *state.GetResponse) {