tokers commented on a change in pull request #1318: URL: https://github.com/apache/apisix-dashboard/pull/1318#discussion_r559270738
########## File path: api/internal/core/store/mock_interface.go ########## @@ -0,0 +1,131 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package store + +import ( + "context" + + "github.com/stretchr/testify/mock" +) + +type MockInterface struct { + mock.Mock +} + +func (m *MockInterface) Get(key string) (interface{}, error) { + ret := m.Called(key) + + if rf, ok := ret.Get(0).(func(key string) (interface{}, error)); ok { Review comment: Could we just use a type assert switch statement here? The type assertions are all around. ########## File path: api/internal/core/store/store_mock.go ########## @@ -18,48 +18,184 @@ package store import ( "context" + "encoding/json" + "fmt" + "sort" + "github.com/apisix/manager-api/internal/core/entity" + "github.com/shiningrush/droplet/data" "github.com/stretchr/testify/mock" ) -type MockInterface struct { - mock.Mock +type Mock struct { + MockInterface + storage map[string]interface{} + validator Validator + keyFunc func(obj interface{}) string } -func (m *MockInterface) Get(key string) (interface{}, error) { - ret := m.Mock.Called(key) - return ret.Get(0), ret.Error(1) +func NewMock(keyFunc func(obj interface{}) string, validator Validator) *Mock { + m := &Mock{ + storage: make(map[string]interface{}), + validator: validator, + keyFunc: keyFunc, + } + + m.onCreate() + m.onGet() + m.onUpdate() + m.onList() + m.onBatchDelete() + return m +} + +func (m *Mock) create(ctx context.Context, obj interface{}) (interface{}, error) { + if setter, ok := obj.(entity.BaseInfoSetter); ok { + info := setter.GetBaseInfo() + info.Creating() + } + + if m.validator != nil { + if err := m.validator.Validate(obj); err != nil { + return nil, err + } + } + + key := m.keyFunc(obj) + if key == "" { + return nil, fmt.Errorf("key is required") + } + if _, exist := m.storage[key]; exist { + return nil, fmt.Errorf("key: %s is conflicted", key) + } + + _, err := json.Marshal(obj) + if err != nil { + return nil, fmt.Errorf("json marshal failed: %s", err) + } + + m.storage[key] = obj + return obj, nil } -func (m *MockInterface) List(input ListInput) (*ListOutput, error) { - ret := m.Called(input) +func (m *Mock) onCreate() { + m.On("Create", mock.Anything, mock.Anything).Return(m.create, nil) +} - var ( - r0 *ListOutput - r1 error - ) +func (m *Mock) onGet() { + rf := func(key string) (interface{}, error) { + r, exist := m.storage[key] + if !exist { + return nil, data.ErrNotFound + } - if rf, ok := ret.Get(0).(func(ListInput) *ListOutput); ok { - r0 = rf(input) - } else { - r0 = ret.Get(0).(*ListOutput) + return r, nil } - r1 = ret.Error(1) - return r0, r1 + m.On("Get", mock.Anything).Return(rf, nil) } -func (m *MockInterface) Create(ctx context.Context, obj interface{}) (interface{}, error) { - ret := m.Mock.Called(ctx, obj) - return ret.Get(0), ret.Error(1) +func (m *Mock) onList() { + rf := func(input ListInput) (*ListOutput, error) { + var ret = make([]interface{}, 0) + for _, v := range m.storage { + value := v + if input.Predicate != nil && !input.Predicate(v) { + continue + } + + if input.Format != nil { + value = input.Format(v) + } + + ret = append(ret, value) + } + + output := &ListOutput{ + 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 { + output.Rows = []interface{}{} + return output, nil + } + + endIdx := skipCount + input.PageSize + if endIdx >= output.TotalSize { + output.Rows = ret[skipCount:] + return output, nil + } + output.Rows = ret[skipCount:endIdx] + } + + return output, nil + } + + m.On("List", mock.Anything).Return(rf, nil) } -func (m *MockInterface) Update(ctx context.Context, obj interface{}, createOnFail bool) error { - ret := m.Mock.Called(ctx, obj, createOnFail) - return ret.Error(0) +func (m *Mock) onUpdate() { + rf := func(ctx context.Context, obj interface{}, createIfNotExist bool) error { + if m.validator != nil { + if err := m.validator.Validate(obj); err != nil { + return err + } + } + + key := m.keyFunc(obj) + if key == "" { + return fmt.Errorf("key is required") Review comment: Ditto, just use `errors.New` is enough. ########## File path: api/internal/core/store/store_mock.go ########## @@ -18,48 +18,184 @@ package store import ( "context" + "encoding/json" + "fmt" + "sort" + "github.com/apisix/manager-api/internal/core/entity" + "github.com/shiningrush/droplet/data" "github.com/stretchr/testify/mock" ) -type MockInterface struct { - mock.Mock +type Mock struct { + MockInterface Review comment: It would be better to keep an empty line after the embed object. ########## File path: api/internal/core/store/store_mock_test.go ########## @@ -0,0 +1,200 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package store + +import ( + "context" + "fmt" + "testing" + + "github.com/apisix/manager-api/internal/core/entity" + "github.com/apisix/manager-api/internal/utils" + "github.com/shiningrush/droplet/data" + "github.com/stretchr/testify/assert" +) + +type testObj struct { + entity.BaseInfo Review comment: Keep an empty line after the embed object. ########## File path: api/internal/core/store/store_mock.go ########## @@ -18,48 +18,184 @@ package store import ( "context" + "encoding/json" + "fmt" + "sort" + "github.com/apisix/manager-api/internal/core/entity" + "github.com/shiningrush/droplet/data" "github.com/stretchr/testify/mock" ) -type MockInterface struct { - mock.Mock +type Mock struct { + MockInterface + storage map[string]interface{} + validator Validator + keyFunc func(obj interface{}) string } -func (m *MockInterface) Get(key string) (interface{}, error) { - ret := m.Mock.Called(key) - return ret.Get(0), ret.Error(1) +func NewMock(keyFunc func(obj interface{}) string, validator Validator) *Mock { + m := &Mock{ + storage: make(map[string]interface{}), + validator: validator, + keyFunc: keyFunc, + } + + m.onCreate() + m.onGet() + m.onUpdate() + m.onList() + m.onBatchDelete() + return m +} + +func (m *Mock) create(ctx context.Context, obj interface{}) (interface{}, error) { + if setter, ok := obj.(entity.BaseInfoSetter); ok { + info := setter.GetBaseInfo() + info.Creating() + } + + if m.validator != nil { + if err := m.validator.Validate(obj); err != nil { + return nil, err + } + } + + key := m.keyFunc(obj) + if key == "" { + return nil, fmt.Errorf("key is required") Review comment: If the error construction is just a plain string, just use `errors.New`. ########## File path: api/internal/core/store/store_mock_test.go ########## @@ -0,0 +1,200 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package store + +import ( + "context" + "fmt" + "testing" + + "github.com/apisix/manager-api/internal/core/entity" + "github.com/apisix/manager-api/internal/utils" + "github.com/shiningrush/droplet/data" + "github.com/stretchr/testify/assert" +) + +type testObj struct { + entity.BaseInfo + Name string + Value string +} + +func keyFunc(obj interface{}) string { + return utils.InterfaceToString(obj.(*testObj).Name) +} + +func TestMock_Create(t *testing.T) { + ctx := context.Background() + m := NewMock(keyFunc, nil) + + // create success + ret, err := m.Create(ctx, &testObj{Name: "test"}) + assert.Nil(t, err) + assert.Equal(t, "test", ret.(*testObj).Name) + + // create failed, "key is required" + ret, err = m.Create(ctx, &testObj{Name: ""}) + assert.Equal(t, fmt.Errorf("key is required"), err) Review comment: 1. Maybe `assert.Error` method is more proper. 2. Use `errors.New`. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
