diff --git a/state/requests.go b/state/requests.go index 498763b77..22dbe226d 100644 --- a/state/requests.go +++ b/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, "||") { diff --git a/state/sqlite/sqlite.go b/state/sqlite/sqlite.go index 226ce4bb9..1e2c76e6a 100644 --- a/state/sqlite/sqlite.go +++ b/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) } diff --git a/state/sqlite/sqlite_dbaccess.go b/state/sqlite/sqlite_dbaccess.go index 8e4e57950..ccc07e204 100644 --- a/state/sqlite/sqlite_dbaccess.go +++ b/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 } diff --git a/state/store.go b/state/store.go index ceb48fcd8..28f7cdb7d 100644 --- a/state/store.go +++ b/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) } diff --git a/tests/config/state/tests.yml b/tests/config/state/tests.yml index 0da782d41..e281780aa 100644 --- a/tests/config/state/tests.yml +++ b/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 diff --git a/tests/conformance/state/state.go b/tests/conformance/state/state.go index 95aaf7647..f038f704b 100644 --- a/tests/conformance/state/state.go +++ b/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) {