diff --git a/pkg/conformance/client_single_sorted_test.go b/pkg/conformance/client_single_sorted_test.go index d26c6a1..a699e76 100644 --- a/pkg/conformance/client_single_sorted_test.go +++ b/pkg/conformance/client_single_sorted_test.go @@ -327,6 +327,116 @@ func Test_Client_Single_Sorted_Delete_Empty(t *testing.T) { } } +func Test_Client_Single_Sorted_Delete_Index(t *testing.T) { + var err error + + var cli redigo.Interface + { + c := client.Config{ + Kind: client.KindSingle, + } + + cli, err = client.New(c) + if err != nil { + t.Fatal(err) + } + + err = cli.Purge() + if err != nil { + t.Fatal(err) + } + } + + { + exi, err := cli.Sorted().Exists().Score("ssk", 0.8) + if err != nil { + t.Fatal(err) + } + if exi { + t.Fatal("element must not exist") + } + } + + { + err := cli.Sorted().Create().Index("ssk", "foo", 0.8, "a", "b") + if err != nil { + t.Fatal(err) + } + } + + { + exi, err := cli.Sorted().Exists().Score("ssk", 0.8) + if err != nil { + t.Fatal(err) + } + if !exi { + t.Fatal("element must exist") + } + } + + // We just created an element that defined the indices a and b. Now we delete + // this very element including its indices. With this test we ensure that + // elements as well as their associated indices get automatically purged when + // deleting indexed elements. + { + err := cli.Sorted().Delete().Index("ssk", "foo") + if err != nil { + t.Fatal(err) + } + } + + // It should be possible to create the exact same element again including the + // same indizes after it has been deleted. This verifies that deleting + // elements including its indizes works as expected. + { + err := cli.Sorted().Create().Index("ssk", "foo", 0.8, "a", "b") + if err != nil { + t.Fatal(err) + } + } + + { + err := cli.Sorted().Delete().Index("ssk", "foo") + if err != nil { + t.Fatal(err) + } + } + + { + exi, err := cli.Sorted().Exists().Score("ssk", 0.8) + if err != nil { + t.Fatal(err) + } + if exi { + t.Fatal("element must not exist") + } + } + + { + err := cli.Sorted().Create().Index("ssk", "foo", 0.8, "a", "b") + if err != nil { + t.Fatal(err) + } + } + + { + exi, err := cli.Sorted().Exists().Score("ssk", 0.8) + if err != nil { + t.Fatal(err) + } + if !exi { + t.Fatal("element must exist") + } + } + + { + err := cli.Sorted().Delete().Score("ssk", 0.8) + if err != nil { + t.Fatal(err) + } + } +} + func Test_Client_Single_Sorted_Delete_Limit(t *testing.T) { var err error @@ -523,85 +633,6 @@ func Test_Client_Single_Sorted_Delete_Score(t *testing.T) { t.Fatal("element must not exist") } } - - { - err := cli.Sorted().Create().Index("ssk", "foo", 0.8, "a", "b") - if err != nil { - t.Fatal(err) - } - } - - { - exi, err := cli.Sorted().Exists().Score("ssk", 0.8) - if err != nil { - t.Fatal(err) - } - if !exi { - t.Fatal("element must exist") - } - } - - // We just created an element that defined the indices a and b. Now we - // delete this very element only using its score. With this test we ensure - // that elements as well as their associated indices get automatically - // purged when deleting elements only using their score. - { - err := cli.Sorted().Delete().Score("ssk", 0.8) - if err != nil { - t.Fatal(err) - } - } - - // It should be possible to create the exact same element again including - // its indizes after it has been deleted before. This verifies that deleting - // elements including its indizes works as expected. - { - err := cli.Sorted().Create().Index("ssk", "foo", 0.8, "a", "b") - if err != nil { - t.Fatal(err) - } - } - - { - err := cli.Sorted().Delete().Index("ssk", "foo") - if err != nil { - t.Fatal(err) - } - } - - { - exi, err := cli.Sorted().Exists().Score("ssk", 0.8) - if err != nil { - t.Fatal(err) - } - if exi { - t.Fatal("element must not exist") - } - } - - { - err := cli.Sorted().Create().Index("ssk", "foo", 0.8, "a", "b") - if err != nil { - t.Fatal(err) - } - } - - { - exi, err := cli.Sorted().Exists().Score("ssk", 0.8) - if err != nil { - t.Fatal(err) - } - if !exi { - t.Fatal("element must exist") - } - } - - { - err := cli.Sorted().Delete().Score("ssk", 0.8) - if err != nil { - t.Fatal(err) - } - } } func Test_Client_Single_Sorted_Delete_Value(t *testing.T) { @@ -1663,7 +1694,7 @@ func Test_Client_Single_Sorted_Search_Value(t *testing.T) { } } -func Test_Client_Single_Sorted_Update(t *testing.T) { +func Test_Client_Single_Sorted_Update_Index(t *testing.T) { var err error var cli redigo.Interface @@ -1751,6 +1782,104 @@ func Test_Client_Single_Sorted_Update(t *testing.T) { } } +func Test_Client_Single_Sorted_Update_Score(t *testing.T) { + var err error + + var cli redigo.Interface + { + c := client.Config{ + Kind: client.KindSingle, + } + + cli, err = client.New(c) + if err != nil { + t.Fatal(err) + } + + err = cli.Purge() + if err != nil { + t.Fatal(err) + } + } + + { + err := cli.Sorted().Create().Value("ssk", "foo", 0.8) + if err != nil { + t.Fatal(err) + } + } + + { + res, err := cli.Sorted().Search().Score("ssk", 0.8, 0.8) + if err != nil { + t.Fatal(err) + } + if len(res) != 1 { + t.Fatal("expected", 1, "got", len(res)) + } + if res[0] != "foo" { + t.Fatal("expected", "foo", "got", res[0]) + } + } + + { + err := cli.Sorted().Create().Value("ssk", "foo", 0.7) + if !sorted.IsAlreadyExistsError(err) { + t.Fatal("expected", "alreadyExistsError", "got", err) + } + } + + { + _, err := cli.Sorted().Update().Score("ssk", "bar", 0.7) + if !sorted.IsNotFound(err) { + t.Fatal("expected", "notFoundError", "got", err) + } + } + + { + upd, err := cli.Sorted().Update().Score("ssk", "bar", 0.8) + if err != nil { + t.Fatal(err) + } + if !upd { + t.Fatal("element must be updated") + } + } + + { + upd, err := cli.Sorted().Update().Score("ssk", "bar", 0.8) + if err != nil { + t.Fatal(err) + } + if upd { + t.Fatal("element must not be updated") + } + } + + { + res, err := cli.Sorted().Search().Score("ssk", 0.8, 0.8) + if err != nil { + t.Fatal(err) + } + if len(res) != 1 { + t.Fatal("expected", 1, "got", len(res)) + } + if res[0] != "bar" { + t.Fatal("expected", "bar", "got", res[0]) + } + } + + { + res, err := cli.Sorted().Search().Order("ssk", 0, -1) + if err != nil { + t.Fatal(err) + } + if len(res) != 1 { + t.Fatal("expected", 1, "got", len(res)) + } + } +} + func contains(lis []string, itm string) bool { for _, l := range lis { if l == itm { diff --git a/pkg/fake/sorted_update.go b/pkg/fake/sorted_update.go index 375a614..0e79245 100644 --- a/pkg/fake/sorted_update.go +++ b/pkg/fake/sorted_update.go @@ -2,6 +2,7 @@ package fake type SortedUpdate struct { FakeIndex func() (bool, error) + FakeScore func() (bool, error) } func (u *SortedUpdate) Index(key string, new string, sco float64, ind ...string) (bool, error) { @@ -11,3 +12,11 @@ func (u *SortedUpdate) Index(key string, new string, sco float64, ind ...string) return false, nil } + +func (u *SortedUpdate) Score(key string, new string, sco float64) (bool, error) { + if u.FakeScore != nil { + return u.FakeScore() + } + + return false, nil +} diff --git a/pkg/sorted/create.go b/pkg/sorted/create.go index 4a40073..5ef6b57 100644 --- a/pkg/sorted/create.go +++ b/pkg/sorted/create.go @@ -115,7 +115,7 @@ func (c *create) Value(key string, val string, sco float64) error { con := c.pool.Get() defer con.Close() - res, err := redis.Int(con.Do("ZADD", prefix.WithKeys(c.prefix, key), sco, val)) + res, err := redis.Int(con.Do("ZADD", prefix.WithKeys(c.prefix, key), "NX", sco, val)) if err != nil { return tracer.Mask(err) } diff --git a/pkg/sorted/delete.go b/pkg/sorted/delete.go index c69390b..447b4fc 100644 --- a/pkg/sorted/delete.go +++ b/pkg/sorted/delete.go @@ -29,19 +29,11 @@ const deleteIndexScript = ` return 0 ` -const deleteScoreScript = ` - redis.call("ZREMRANGEBYSCORE", KEYS[2], ARGV[1], ARGV[1]) - redis.call("ZREMRANGEBYSCORE", KEYS[1], ARGV[1], ARGV[1]) - - return 0 -` - type delete struct { pool *redis.Pool deleteCleanScript *redis.Script deleteIndexScript *redis.Script - deleteScoreScript *redis.Script prefix string } @@ -56,7 +48,7 @@ func (d *delete) Clean(key string) error { arg = append(arg, prefix.WithKeys(d.prefix, index.New(key))) // KEYS[2] } - _, err := redis.Int(d.deleteCleanScript.Do(con, arg...)) + _, err := redis.Int64(d.deleteCleanScript.Do(con, arg...)) if err != nil { return tracer.Mask(err) } @@ -78,7 +70,7 @@ func (d *delete) Index(key string, val ...string) error { } } - _, err := redis.Int(d.deleteIndexScript.Do(con, arg...)) + _, err := redis.Int64(d.deleteIndexScript.Do(con, arg...)) if err != nil { return tracer.Mask(err) } @@ -116,14 +108,7 @@ func (d *delete) Score(key string, sco float64) error { con := d.pool.Get() defer con.Close() - var arg []interface{} - { - arg = append(arg, prefix.WithKeys(d.prefix, key)) // KEYS[1] - arg = append(arg, prefix.WithKeys(d.prefix, index.New(key))) // KEYS[2] - arg = append(arg, sco) // ARGV[1] - } - - _, err := redis.Int(d.deleteScoreScript.Do(con, arg...)) + _, err := redis.Int64(con.Do("ZREMRANGEBYSCORE", prefix.WithKeys(d.prefix, key), sco, sco)) if err != nil { return tracer.Mask(err) } diff --git a/pkg/sorted/interface.go b/pkg/sorted/interface.go index 38723df..27175da 100644 --- a/pkg/sorted/interface.go +++ b/pkg/sorted/interface.go @@ -18,10 +18,12 @@ type Create interface { Index(key string, val string, sco float64, ind ...string) error // Value creates an element within the sorted set under key transparently - // using ZADD. Scores are not enforced to be unique. + // using ZADD and the NX option. Scores are not enforced to be unique, values + // are. // // https://redis.io/commands/zadd // + // TODO rename to Score due to its score interface Value(key string, val string, sco float64) error } @@ -46,9 +48,11 @@ type Delete interface { // Limit(key string, lim int) error - // Score deletes the element identified by score within the specified sorted - // set. Note that indices associated with the underlying element are purged - // automatically as well. + // Score deletes the element identified by the given score within the + // specified sorted set. Non-existing elements are ignored. + // + // https://redis.io/commands/zremrangebyscore + // Score(key string, sco float64) error // Value deletes the elements identified by the given values within the @@ -130,12 +134,25 @@ type Search interface { // Score returns the values associated to the range of scores defined by lef // and rig. Can be used to find a particular value if lef and rig are equal. + // + // https://redis.io/commands/zrange + // Score(key string, lef float64, rig float64) ([]string, error) } type Update interface { - // Index modifies the element identified by sco and sets its value to new. For - // the sorted set implementations scores are static and must never change - // since they get treated like unique IDs. + // Index modifies the element identified by sco and sets its value to new. The + // current implementation requires all indices to be provided that have also + // been used to create the indexed element in the first place. For the sorted + // set implementation here, indices and scores are static and must never + // change since they get treated like unique IDs. The returned bool indicates + // whether the underlying value was updated. An error is returned if the + // underlying element does not exist. Index(key string, new string, sco float64, ind ...string) (bool, error) + // Score modifies the element identified by sco and sets its value to new. For + // the sorted set implementation here, scores are static and must never change + // since they get treated like unique IDs. The returned bool indicates whether + // the underlying value was updated. An error is returned if the underlying + // element does not exist. + Score(key string, new string, sco float64) (bool, error) } diff --git a/pkg/sorted/sorted.go b/pkg/sorted/sorted.go index 6a816af..519bdfa 100644 --- a/pkg/sorted/sorted.go +++ b/pkg/sorted/sorted.go @@ -39,7 +39,6 @@ func New(config Config) (*Sorted, error) { deleteCleanScript: redis.NewScript(2, deleteCleanScript), deleteIndexScript: redis.NewScript(2, deleteIndexScript), - deleteScoreScript: redis.NewScript(2, deleteScoreScript), prefix: config.Prefix, } @@ -89,6 +88,7 @@ func New(config Config) (*Sorted, error) { pool: config.Pool, updateIndexScript: redis.NewScript(2, updateIndexScript), + updateScoreScript: redis.NewScript(1, updateScoreScript), prefix: config.Prefix, } diff --git a/pkg/sorted/update.go b/pkg/sorted/update.go index 4af0a07..345ee09 100644 --- a/pkg/sorted/update.go +++ b/pkg/sorted/update.go @@ -35,7 +35,7 @@ const updateIndexScript = ` return 3 end - local function ver(key, val, sco) + local function ver(key, new, sco) -- Verify if the score does already exist. If there is no element we -- cannot update it. local res = redis.call("ZRANGE", key, sco, sco, "BYSCORE") @@ -47,7 +47,7 @@ const updateIndexScript = ` -- Verify if the existing value is already what we want to update to. If -- the desired state is already reconciled we do not need to proceed -- further. - if (old == val) then + if (old == new) then return 2 end @@ -78,13 +78,38 @@ const updateIndexScript = ` j=j+1 end + return upd(KEYS[1], ARGV[1], ARGV[2]) ` +const updateScoreScript = ` + -- Verify if the score does already exist. If there is no element we + -- cannot update it. + local res = redis.call("ZRANGE", KEYS[1], ARGV[2], ARGV[2], "BYSCORE") + + local old = res[1] + if (old == nil) then + return 0 + end + + -- Verify if the existing value is already what we want to update to. If + -- the desired state is already reconciled we do not need to proceed + -- further. + if (old == ARGV[1]) then + return 1 + end + + redis.call("ZADD", KEYS[1], ARGV[2], ARGV[1]) + redis.call("ZREM", KEYS[1], old) + + return 2 +` + type update struct { pool *redis.Pool updateIndexScript *redis.Script + updateScoreScript *redis.Script prefix string } @@ -124,10 +149,11 @@ func (u *update) Index(key string, new string, sco float64, ind ...string) (bool var arg []interface{} { - arg = append(arg, prefix.WithKeys(u.prefix, key)) - arg = append(arg, prefix.WithKeys(u.prefix, index.New(key))) - arg = append(arg, new) - arg = append(arg, sco) + arg = append(arg, prefix.WithKeys(u.prefix, key)) // KEYS[1] + arg = append(arg, prefix.WithKeys(u.prefix, index.New(key))) // KEYS[2] + arg = append(arg, new) // ARGV[1] + arg = append(arg, sco) // ARGV[2] + for _, s := range ind { arg = append(arg, s) } @@ -151,3 +177,31 @@ func (u *update) Index(key string, new string, sco float64, ind ...string) (bool return false, tracer.Mask(executionFailedError) } + +func (u *update) Score(key string, new string, sco float64) (bool, error) { + con := u.pool.Get() + defer con.Close() + + var arg []interface{} + { + arg = append(arg, prefix.WithKeys(u.prefix, key)) // KEYS[1] + arg = append(arg, new) // ARGV[1] + arg = append(arg, sco) // ARGV[2] + } + + res, err := redis.Int(u.updateScoreScript.Do(con, arg...)) + if err != nil { + return false, tracer.Mask(err) + } + + switch res { + case 0: + return false, tracer.Maskf(notFoundError, "element does not exist in sorted set") + case 1: + return false, nil + case 2: + return true, nil + } + + return false, tracer.Mask(executionFailedError) +}