Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cherry-pick] refactor: migrate the redis command keys to scan #19148

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/lib/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ var (
ErrNotFound = errors.New("key not found")
)

// Iterator returns the ScanIterator
type Iterator interface {
Next(ctx context.Context) bool
Val() string
}

//go:generate mockery --name Cache --output . --outpkg cache --filename mock_cache_test.go --structname mockCache --inpackage

// Cache cache interface
type Cache interface {
// Contains returns true if key exists
Expand All @@ -57,8 +65,9 @@ type Cache interface {
// Save cache the value by key
Save(ctx context.Context, key string, value interface{}, expiration ...time.Duration) error

// Keys returns the key matched by prefixes
Keys(ctx context.Context, prefixes ...string) ([]string, error)
// Scan scans the keys matched by match string
// NOTICE: memory cache does not support use wildcard, compared by strings.Contains
Scan(ctx context.Context, match string) (Iterator, error)
}

var (
Expand Down
13 changes: 6 additions & 7 deletions src/lib/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ import (
"fmt"
"testing"

"github.com/goharbor/harbor/src/lib/retry"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"

cachetesting "github.com/goharbor/harbor/src/testing/lib/cache"
"github.com/goharbor/harbor/src/testing/mock"
)

type CacheTestSuite struct {
Expand All @@ -30,7 +29,7 @@ type CacheTestSuite struct {

func (suite *CacheTestSuite) SetupSuite() {
Register("mock", func(opts Options) (Cache, error) {
return &cachetesting.Cache{}, nil
return &mockCache{}, nil
})
}

Expand Down Expand Up @@ -62,8 +61,8 @@ func (suite *CacheTestSuite) TestInitialize() {

{
Register("cache", func(opts Options) (Cache, error) {
c := &cachetesting.Cache{}
c.On("Ping", mock.Anything).Return(fmt.Errorf("oops"))
c := &mockCache{}
c.On("Ping", mock.Anything).Return(retry.Abort(fmt.Errorf("oops")))

return c, nil
})
Expand All @@ -75,7 +74,7 @@ func (suite *CacheTestSuite) TestInitialize() {

{
Register("cache", func(opts Options) (Cache, error) {
c := &cachetesting.Cache{}
c := &mockCache{}
c.On("Ping", mock.Anything).Return(nil)

return c, nil
Expand Down
9 changes: 4 additions & 5 deletions src/lib/cache/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/stretchr/testify/suite"

cachetesting "github.com/goharbor/harbor/src/testing/lib/cache"
"github.com/goharbor/harbor/src/testing/mock"
)

Expand All @@ -42,7 +41,7 @@ func (suite *FetchOrSaveTestSuite) SetupSuite() {
}

func (suite *FetchOrSaveTestSuite) TestFetchInternalError() {
c := &cachetesting.Cache{}
c := &mockCache{}

mock.OnAnything(c, "Fetch").Return(fmt.Errorf("oops"))

Expand All @@ -55,7 +54,7 @@ func (suite *FetchOrSaveTestSuite) TestFetchInternalError() {
}

func (suite *FetchOrSaveTestSuite) TestBuildError() {
c := &cachetesting.Cache{}
c := &mockCache{}

mock.OnAnything(c, "Fetch").Return(ErrNotFound)

Expand All @@ -68,7 +67,7 @@ func (suite *FetchOrSaveTestSuite) TestBuildError() {
}

func (suite *FetchOrSaveTestSuite) TestSaveError() {
c := &cachetesting.Cache{}
c := &mockCache{}

mock.OnAnything(c, "Fetch").Return(ErrNotFound)
mock.OnAnything(c, "Save").Return(fmt.Errorf("oops"))
Expand All @@ -83,7 +82,7 @@ func (suite *FetchOrSaveTestSuite) TestSaveError() {
}

func (suite *FetchOrSaveTestSuite) TestSaveCalledOnlyOneTime() {
c := &cachetesting.Cache{}
c := &mockCache{}

var data sync.Map

Expand Down
58 changes: 43 additions & 15 deletions src/lib/cache/memory/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,27 +117,55 @@ func (c *Cache) Save(ctx context.Context, key string, value interface{}, expirat
return nil
}

// Keys returns the key matched by prefixes.
func (c *Cache) Keys(ctx context.Context, prefixes ...string) ([]string, error) {
// if no prefix, means match all keys.
matchAll := len(prefixes) == 0
// range map to get all keys
keys := make([]string, 0)
// Scan scans the keys matched by match string
func (c *Cache) Scan(ctx context.Context, match string) (cache.Iterator, error) {
var keys []string
c.storage.Range(func(k, v interface{}) bool {
ks := k.(string)
if matchAll {
keys = append(keys, ks)
} else {
for _, p := range prefixes {
if strings.HasPrefix(ks, c.opts.Key(p)) {
keys = append(keys, strings.TrimPrefix(ks, c.opts.Prefix))
}
matched := true
if match != "" {
matched = strings.Contains(k.(string), match)
}

if matched {
if v.(*entry).isExpirated() {
c.storage.Delete(k)
} else {
keys = append(keys, strings.TrimPrefix(k.(string), c.opts.Prefix))
}
}
return true
})

return keys, nil
return &ScanIterator{keys: keys}, nil
}

// ScanIterator is a ScanIterator for memory cache
type ScanIterator struct {
mu sync.Mutex
pos int
keys []string
}

// Next checks whether has the next element
func (i *ScanIterator) Next(ctx context.Context) bool {
i.mu.Lock()
defer i.mu.Unlock()

i.pos++
return i.pos <= len(i.keys)
}

// Val returns the key
func (i *ScanIterator) Val() string {
i.mu.Lock()
defer i.mu.Unlock()

var val string
if i.pos <= len(i.keys) {
val = i.keys[i.pos-1]
}

return val
}

// New returns memory cache
Expand Down
71 changes: 49 additions & 22 deletions src/lib/cache/memory/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package memory

import (
"context"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -109,28 +110,54 @@ func (suite *CacheTestSuite) TestPing() {
suite.NoError(suite.cache.Ping(suite.ctx))
}

func (suite *CacheTestSuite) TestKeys() {
key1 := "p1"
key2 := "p2"

var err error
err = suite.cache.Save(suite.ctx, key1, "hello, p1")
suite.Nil(err)
err = suite.cache.Save(suite.ctx, key2, "hello, p2")
suite.Nil(err)

// should match all
keys, err := suite.cache.Keys(suite.ctx, "p")
suite.Nil(err)
suite.ElementsMatch([]string{"p1", "p2"}, keys)
// only get p1
keys, err = suite.cache.Keys(suite.ctx, key1)
suite.Nil(err)
suite.Equal([]string{"p1"}, keys)
// only get p2
keys, err = suite.cache.Keys(suite.ctx, key2)
suite.Nil(err)
suite.Equal([]string{"p2"}, keys)
func (suite *CacheTestSuite) TestScan() {
seed := func(n int) {
for i := 0; i < n; i++ {
key := fmt.Sprintf("test-scan-%d", i)
err := suite.cache.Save(suite.ctx, key, "")
suite.NoError(err)
}
}
clean := func(n int) {
for i := 0; i < n; i++ {
key := fmt.Sprintf("test-scan-%d", i)
err := suite.cache.Delete(suite.ctx, key)
suite.NoError(err)
}
}
{
// no match should return all keys
expect := []string{"test-scan-0", "test-scan-1", "test-scan-2"}
// seed data
seed(3)
// test scan
iter, err := suite.cache.Scan(suite.ctx, "")
suite.NoError(err)
got := []string{}
for iter.Next(suite.ctx) {
got = append(got, iter.Val())
}
suite.ElementsMatch(expect, got)
// clean up
clean(3)
}

{
// with match should return matched keys
expect := []string{"test-scan-1", "test-scan-10"}
// seed data
seed(11)
// test scan
iter, err := suite.cache.Scan(suite.ctx, "test-scan-1")
suite.NoError(err)
got := []string{}
for iter.Next(suite.ctx) {
got = append(got, iter.Val())
}
suite.ElementsMatch(expect, got)
// clean up
clean(11)
}
}

func TestCacheTestSuite(t *testing.T) {
Expand Down
130 changes: 130 additions & 0 deletions src/lib/cache/mock_cache_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading