You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by vi...@apache.org on 2020/10/09 13:56:01 UTC
[apisix-dashboard] branch refactor updated: fix: append sort for
list and using sync.Map instead of map
This is an automated email from the ASF dual-hosted git repository.
vinci pushed a commit to branch refactor
in repository https://gitbox.apache.org/repos/asf/apisix-dashboard.git
The following commit(s) were added to refs/heads/refactor by this push:
new e7c00d2 fix: append sort for list and using sync.Map instead of map
e7c00d2 is described below
commit e7c00d2f07ef9949b9c943c0040c8bb179ba44ba
Author: ShiningRush <27...@qq.com>
AuthorDate: Fri Oct 9 21:55:46 2020 +0800
fix: append sort for list and using sync.Map instead of map
---
api/internal/core/store/store.go | 64 +++++++---
api/internal/core/store/store_test.go | 223 +++++++++++++++++++---------------
2 files changed, 169 insertions(+), 118 deletions(-)
diff --git a/api/internal/core/store/store.go b/api/internal/core/store/store.go
index 263a49f..a82089f 100644
--- a/api/internal/core/store/store.go
+++ b/api/internal/core/store/store.go
@@ -23,6 +23,8 @@ import (
"fmt"
"log"
"reflect"
+ "sort"
+ "sync"
"time"
"github.com/shiningrush/droplet/data"
@@ -43,7 +45,7 @@ type Interface interface {
type GenericStore struct {
Stg storage.Interface
- cache map[string]interface{}
+ cache sync.Map
opt GenericStoreOption
cancel context.CancelFunc
@@ -75,8 +77,7 @@ func NewGenericStore(opt GenericStoreOption) (*GenericStore, error) {
return nil, fmt.Errorf("obj type is invalid")
}
s := &GenericStore{
- opt: opt,
- cache: make(map[string]interface{}),
+ opt: opt,
}
s.Stg = &storage.EtcdV3Storage{}
@@ -95,7 +96,7 @@ func (s *GenericStore) Init() error {
if err != nil {
return err
}
- s.cache[s.opt.KeyFunc(objPtr)] = objPtr
+ s.cache.Store(s.opt.KeyFunc(objPtr), objPtr)
}
c, cancel := context.WithCancel(context.TODO())
@@ -114,9 +115,9 @@ func (s *GenericStore) Init() error {
log.Println("value convert to obj failed", err)
continue
}
- s.cache[event.Events[i].Key[len(s.opt.BasePath)+1:]] = objPtr
+ s.cache.Store(event.Events[i].Key[len(s.opt.BasePath)+1:], objPtr)
case storage.EventTypeDelete:
- delete(s.cache, event.Events[i].Key[len(s.opt.BasePath)+1:])
+ s.cache.Delete(event.Events[i].Key[len(s.opt.BasePath)+1:])
}
}
}
@@ -126,7 +127,7 @@ func (s *GenericStore) Init() error {
}
func (s *GenericStore) Get(key string) (interface{}, error) {
- ret, ok := s.cache[key]
+ ret, ok := s.cache.Load(key)
if !ok {
return nil, data.ErrNotFound
}
@@ -138,6 +139,7 @@ type ListInput struct {
PageSize int
// start from 1
PageNumber int
+ Less func(i, j interface{}) bool
}
type ListOutput struct {
@@ -145,14 +147,27 @@ type ListOutput struct {
TotalSize int `json:"total_size"`
}
+var defLessFunc = func(i, j interface{}) bool {
+ iBase := i.(entity.BaseInfoGetter).GetBaseInfo()
+ jBase := j.(entity.BaseInfoGetter).GetBaseInfo()
+ if iBase.CreateTime != jBase.CreateTime {
+ return iBase.CreateTime < jBase.CreateTime
+ }
+ if iBase.UpdateTime != jBase.UpdateTime {
+ return iBase.UpdateTime < jBase.UpdateTime
+ }
+ return iBase.ID < jBase.ID
+}
+
func (s *GenericStore) List(input ListInput) (*ListOutput, error) {
var ret []interface{}
- for k := range s.cache {
- if input.Predicate != nil && !input.Predicate(s.cache[k]) {
- continue
+ s.cache.Range(func(key, value interface{}) bool {
+ if input.Predicate != nil && !input.Predicate(value) {
+ return true
}
- ret = append(ret, s.cache[k])
- }
+ ret = append(ret, value)
+ return true
+ })
//should return an empty array not a null for client
if ret == nil {
@@ -163,6 +178,14 @@ func (s *GenericStore) List(input ListInput) (*ListOutput, error) {
Rows: ret,
TotalSize: len(ret),
}
+ if input.Less == nil {
+ input.Less = defLessFunc
+ }
+
+ sort.Slice(output.Rows, func(i, j int) bool {
+ return input.Less(output.Rows[i], output.Rows[j])
+ })
+
if input.PageSize > 0 && input.PageNumber > 0 {
skipCount := (input.PageNumber - 1) * input.PageSize
if skipCount > output.TotalSize {
@@ -181,7 +204,7 @@ func (s *GenericStore) List(input ListInput) (*ListOutput, error) {
return output, nil
}
-func (s *GenericStore) ingestValidate(obj interface{}) error {
+func (s *GenericStore) ingestValidate(obj interface{}) (err error) {
if s.opt.Validator != nil {
if err := s.opt.Validator.Validate(obj); err != nil {
return err
@@ -189,13 +212,14 @@ func (s *GenericStore) ingestValidate(obj interface{}) error {
}
if s.opt.StockCheck != nil {
- for k := range s.cache {
- if err := s.opt.StockCheck(obj, s.cache[k]); err != nil {
- return err
+ s.cache.Range(func(key, value interface{}) bool {
+ if err = s.opt.StockCheck(obj, value); err != nil {
+ return false
}
- }
+ return true
+ })
}
- return nil
+ return err
}
func (s *GenericStore) Create(ctx context.Context, obj interface{}) error {
@@ -216,7 +240,7 @@ func (s *GenericStore) Create(ctx context.Context, obj interface{}) error {
if key == "" {
return fmt.Errorf("key is required")
}
- _, ok := s.cache[key]
+ _, ok := s.cache.Load(key)
if ok {
return fmt.Errorf("key: %s is conflicted", key)
}
@@ -241,7 +265,7 @@ func (s *GenericStore) Update(ctx context.Context, obj interface{}) error {
if key == "" {
return fmt.Errorf("key is required")
}
- oldObj, ok := s.cache[key]
+ oldObj, ok := s.cache.Load(key)
if !ok {
return fmt.Errorf("key: %s is not found", key)
}
diff --git a/api/internal/core/store/store_test.go b/api/internal/core/store/store_test.go
index 4f29abc..f7ad5f4 100644
--- a/api/internal/core/store/store_test.go
+++ b/api/internal/core/store/store_test.go
@@ -37,6 +37,7 @@ func TestNewGenericStore(t *testing.T) {
dfFunc := func(obj interface{}) string { return "" }
tests := []struct {
giveOpt GenericStoreOption
+ giveCache map[string]interface{}
wantStore *GenericStore
wantErr error
}{
@@ -47,8 +48,7 @@ func TestNewGenericStore(t *testing.T) {
KeyFunc: dfFunc,
},
wantStore: &GenericStore{
- Stg: &storage.EtcdV3Storage{},
- cache: map[string]interface{}{},
+ Stg: &storage.EtcdV3Storage{},
opt: GenericStoreOption{
BasePath: "test",
ObjType: reflect.TypeOf(GenericStoreOption{}),
@@ -125,7 +125,6 @@ func TestGenericStore_Init(t *testing.T) {
{
caseDesc: "sanity",
giveStore: &GenericStore{
- cache: map[string]interface{}{},
opt: GenericStoreOption{
BasePath: "test",
ObjType: reflect.TypeOf(TestStruct{}),
@@ -215,7 +214,10 @@ func TestGenericStore_Init(t *testing.T) {
tc.giveWatchCh <- tc.giveResp
time.Sleep(1 * time.Second)
close(tc.giveWatchCh)
- assert.Equal(t, tc.wantCache, tc.giveStore.cache, tc.caseDesc)
+ tc.giveStore.cache.Range(func(key, value interface{}) bool {
+ assert.Equal(t, tc.wantCache[key.(string)], value)
+ return true
+ })
}
}
@@ -224,24 +226,24 @@ func TestGenericStore_Get(t *testing.T) {
caseDesc string
giveId string
giveStore *GenericStore
+ giveCache map[string]interface{}
wantRet interface{}
wantErr error
}{
{
caseDesc: "sanity",
giveId: "test1",
- giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test2": TestStruct{
- Field1: "test2-f1",
- Field2: "test2-f2",
- },
- "test1": TestStruct{
- Field1: "test1-f1",
- Field2: "test1-f2",
- },
+ giveCache: map[string]interface{}{
+ "test2": TestStruct{
+ Field1: "test2-f1",
+ Field2: "test2-f2",
+ },
+ "test1": TestStruct{
+ Field1: "test1-f1",
+ Field2: "test1-f2",
},
},
+ giveStore: &GenericStore{},
wantRet: TestStruct{
Field1: "test1-f1",
Field2: "test1-f2",
@@ -250,23 +252,26 @@ func TestGenericStore_Get(t *testing.T) {
{
caseDesc: "not found",
giveId: "not",
- giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test2": TestStruct{
- Field1: "test2-f1",
- Field2: "test2-f2",
- },
- "test1": TestStruct{
- Field1: "test1-f1",
- Field2: "test1-f2",
- },
+ giveCache: map[string]interface{}{
+ "test2": TestStruct{
+ Field1: "test2-f1",
+ Field2: "test2-f2",
+ },
+ "test1": TestStruct{
+ Field1: "test1-f1",
+ Field2: "test1-f2",
},
},
- wantErr: data.ErrNotFound,
+ giveStore: &GenericStore{},
+ wantErr: data.ErrNotFound,
},
}
for _, tc := range tests {
+ for k, v := range tc.giveCache {
+ tc.giveStore.cache.Store(k, v)
+ }
+
ret, err := tc.giveStore.Get(tc.giveId)
assert.Equal(t, tc.wantRet, ret, tc.caseDesc)
assert.Equal(t, tc.wantErr, err, tc.caseDesc)
@@ -278,6 +283,7 @@ func TestGenericStore_List(t *testing.T) {
caseDesc string
giveInput ListInput
giveStore *GenericStore
+ giveCache map[string]interface{}
wantRet *ListOutput
wantErr error
}{
@@ -293,26 +299,25 @@ func TestGenericStore_List(t *testing.T) {
return false
},
},
- giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test1": &TestStruct{
- Field1: "test1-f1",
- Field2: "test1-f2",
- },
- "test2": &TestStruct{
- Field1: "test2-f1",
- Field2: "test2-f2",
- },
- "test3": &TestStruct{
- Field1: "test3-f1",
- Field2: "test3-f2",
- },
- "test4": &TestStruct{
- Field1: "test4-f1",
- Field2: "test4-f2",
- },
+ giveCache: map[string]interface{}{
+ "test1": &TestStruct{
+ Field1: "test1-f1",
+ Field2: "test1-f2",
+ },
+ "test2": &TestStruct{
+ Field1: "test2-f1",
+ Field2: "test2-f2",
+ },
+ "test3": &TestStruct{
+ Field1: "test3-f1",
+ Field2: "test3-f2",
+ },
+ "test4": &TestStruct{
+ Field1: "test4-f1",
+ Field2: "test4-f2",
},
},
+ giveStore: &GenericStore{},
wantRet: &ListOutput{
Rows: []interface{}{
&TestStruct{
@@ -341,29 +346,34 @@ func TestGenericStore_List(t *testing.T) {
PageSize: 1,
PageNumber: 2,
},
- giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test1": &TestStruct{
- Field1: "test1-f1",
- Field2: "test1-f2",
- },
- "test2": &TestStruct{
- Field1: "test2-f1",
- Field2: "test2-f2",
- },
- "test3": &TestStruct{
- Field1: "test3-f1",
- Field2: "test3-f2",
- },
- "test4": &TestStruct{
- Field1: "test4-f1",
- Field2: "test4-f2",
+ giveCache: map[string]interface{}{
+ "test1": &TestStruct{
+ Field1: "test1-f1",
+ Field2: "test1-f2",
+ },
+ "test2": &TestStruct{
+ Field1: "test2-f1",
+ Field2: "test2-f2",
+ },
+ "test3": &TestStruct{
+ BaseInfo: entity.BaseInfo{
+ CreateTime: 100,
},
+ Field1: "test3-f1",
+ Field2: "test3-f2",
+ },
+ "test4": &TestStruct{
+ Field1: "test4-f1",
+ Field2: "test4-f2",
},
},
+ giveStore: &GenericStore{},
wantRet: &ListOutput{
Rows: []interface{}{
&TestStruct{
+ BaseInfo: entity.BaseInfo{
+ CreateTime: 100,
+ },
Field1: "test3-f1",
Field2: "test3-f2",
},
@@ -385,26 +395,25 @@ func TestGenericStore_List(t *testing.T) {
PageSize: 1,
PageNumber: 33,
},
- giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test1": &TestStruct{
- Field1: "test1-f1",
- Field2: "test1-f2",
- },
- "test2": &TestStruct{
- Field1: "test2-f1",
- Field2: "test2-f2",
- },
- "test3": &TestStruct{
- Field1: "test3-f1",
- Field2: "test3-f2",
- },
- "test4": &TestStruct{
- Field1: "test4-f1",
- Field2: "test4-f2",
- },
+ giveCache: map[string]interface{}{
+ "test1": &TestStruct{
+ Field1: "test1-f1",
+ Field2: "test1-f2",
+ },
+ "test2": &TestStruct{
+ Field1: "test2-f1",
+ Field2: "test2-f2",
+ },
+ "test3": &TestStruct{
+ Field1: "test3-f1",
+ Field2: "test3-f2",
+ },
+ "test4": &TestStruct{
+ Field1: "test4-f1",
+ Field2: "test4-f2",
},
},
+ giveStore: &GenericStore{},
wantRet: &ListOutput{
Rows: []interface{}{},
TotalSize: 2,
@@ -413,6 +422,10 @@ func TestGenericStore_List(t *testing.T) {
}
for _, tc := range tests {
+ for k, v := range tc.giveCache {
+ tc.giveStore.cache.Store(k, v)
+ }
+
ret, err := tc.giveStore.List(tc.giveInput)
assert.Equal(t, tc.wantRet.TotalSize, ret.TotalSize, tc.caseDesc)
assert.ElementsMatch(t, tc.wantRet.Rows, ret.Rows, tc.caseDesc)
@@ -423,17 +436,17 @@ func TestGenericStore_List(t *testing.T) {
func TestGenericStore_ingestValidate(t *testing.T) {
tests := []struct {
giveStore *GenericStore
+ giveCache map[string]interface{}
giveObj interface{}
giveStockCheck func(obj interface{}, stockObj interface{}) error
giveValidateErr error
wantErr error
}{
{
- giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test1-f1": &TestStruct{Field1: "test1-f1", Field2: "test1-f2"},
- "test2-f1": &TestStruct{Field1: "test2-f1", Field2: "test2-f2"},
- },
+ giveStore: &GenericStore{},
+ giveCache: map[string]interface{}{
+ "test1-f1": &TestStruct{Field1: "test1-f1", Field2: "test1-f2"},
+ "test2-f1": &TestStruct{Field1: "test2-f1", Field2: "test2-f2"},
},
giveObj: &TestStruct{
Field1: "test3-f1",
@@ -456,6 +469,10 @@ func TestGenericStore_ingestValidate(t *testing.T) {
}
for _, tc := range tests {
+ for k, v := range tc.giveCache {
+ tc.giveStore.cache.Store(k, v)
+ }
+
validateCalled := false
mValidator := &MockValidator{}
mValidator.On("Validate", mock.Anything).Run(func(args mock.Arguments) {
@@ -475,6 +492,7 @@ func TestGenericStore_Create(t *testing.T) {
tests := []struct {
caseDesc string
giveStore *GenericStore
+ giveCache map[string]interface{}
giveObj *TestStruct
giveErr error
giveValidateErr error
@@ -521,10 +539,10 @@ func TestGenericStore_Create(t *testing.T) {
Field1: "test1",
Field2: "test2",
},
+ giveCache: map[string]interface{}{
+ "test1": struct{}{},
+ },
giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test1": struct{}{},
- },
opt: GenericStoreOption{
BasePath: "test/path",
KeyFunc: func(obj interface{}) string {
@@ -540,10 +558,10 @@ func TestGenericStore_Create(t *testing.T) {
Field1: "test1",
Field2: "test2",
},
+ giveCache: map[string]interface{}{
+ "test1": struct{}{},
+ },
giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test1": struct{}{},
- },
opt: GenericStoreOption{},
},
giveValidateErr: fmt.Errorf("validate failed"),
@@ -552,6 +570,10 @@ func TestGenericStore_Create(t *testing.T) {
}
for _, tc := range tests {
+ for k, v := range tc.giveCache {
+ tc.giveStore.cache.Store(k, v)
+ }
+
createCalled, validateCalled := false, false
mStorage := &storage.MockInterface{}
mStorage.On("Create", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
@@ -588,6 +610,7 @@ func TestGenericStore_Update(t *testing.T) {
tests := []struct {
caseDesc string
giveStore *GenericStore
+ giveCache map[string]interface{}
giveObj *TestStruct
giveErr error
giveValidateErr error
@@ -600,10 +623,10 @@ func TestGenericStore_Update(t *testing.T) {
Field1: "test1",
Field2: "test2",
},
+ giveCache: map[string]interface{}{
+ "test1": struct{}{},
+ },
giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test1": struct{}{},
- },
opt: GenericStoreOption{
BasePath: "test/path",
KeyFunc: func(obj interface{}) string {
@@ -619,10 +642,10 @@ func TestGenericStore_Update(t *testing.T) {
Field1: "test1",
Field2: "test2",
},
+ giveCache: map[string]interface{}{
+ "test1": struct{}{},
+ },
giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test1": struct{}{},
- },
opt: GenericStoreOption{
BasePath: "test/path",
KeyFunc: func(obj interface{}) string {
@@ -640,10 +663,10 @@ func TestGenericStore_Update(t *testing.T) {
Field1: "test1",
Field2: "test2",
},
+ giveCache: map[string]interface{}{
+ "test2": struct{}{},
+ },
giveStore: &GenericStore{
- cache: map[string]interface{}{
- "test2": struct{}{},
- },
opt: GenericStoreOption{
BasePath: "test/path",
KeyFunc: func(obj interface{}) string {
@@ -656,6 +679,10 @@ func TestGenericStore_Update(t *testing.T) {
}
for _, tc := range tests {
+ for k, v := range tc.giveCache {
+ tc.giveStore.cache.Store(k, v)
+ }
+
createCalled, validateCalled := false, false
mStorage := &storage.MockInterface{}
mStorage.On("Update", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {