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 <[email protected]>
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) {

Reply via email to