Skip to content

Commit

Permalink
feat: Add returning value for HTTP PUT and PATCH methods (#1322)
Browse files Browse the repository at this point in the history 
* Add returning value for Updating entity (PUT/PATCH)

Signed-off-by: imjoey <[email protected]>

* Fix missing return obj when create-if-not-exist

Signed-off-by: imjoey <[email protected]>

* Fix backend Unit and e2e test

Signed-off-by: imjoey <[email protected]>
  • Loading branch information
@imjoey
imjoey authored Jan 19, 2021
1 parent 0b28c12 commit 4cf46b0
Show file tree
Hide file tree
Showing 21 changed files with 183 additions and 51 deletions.
19 changes: 9 additions & 10 deletions api/internal/core/store/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Interface interface {
Get(ctx context.Context, key string) (interface{}, error)
List(ctx context.Context, input ListInput) (*ListOutput, error)
Create(ctx context.Context, obj interface{}) (interface{}, error)
Update(ctx context.Context, obj interface{}, createIfNotExist bool) error
Update(ctx context.Context, obj interface{}, createIfNotExist bool) (interface{}, error)
BatchDelete(ctx context.Context, keys []string) error
}

Expand Down Expand Up @@ -272,23 +272,22 @@ func (s *GenericStore) Create(ctx context.Context, obj interface{}) (interface{}
return obj, nil
}

func (s *GenericStore) Update(ctx context.Context, obj interface{}, createIfNotExist bool) error {
func (s *GenericStore) Update(ctx context.Context, obj interface{}, createIfNotExist bool) (interface{}, error) {
if err := s.ingestValidate(obj); err != nil {
return err
return nil, err
}

key := s.opt.KeyFunc(obj)
if key == "" {
return fmt.Errorf("key is required")
return nil, fmt.Errorf("key is required")
}
storedObj, ok := s.cache.Load(key)
if !ok {
if createIfNotExist {
_, err := s.Create(ctx, obj)
return err
return s.Create(ctx, obj)
}
log.Warnf("key: %s is not found", key)
return fmt.Errorf("key: %s is not found", key)
return nil, fmt.Errorf("key: %s is not found", key)
}

if setter, ok := obj.(entity.BaseInfoGetter); ok {
Expand All @@ -301,13 +300,13 @@ func (s *GenericStore) Update(ctx context.Context, obj interface{}, createIfNotE
bs, err := json.Marshal(obj)
if err != nil {
log.Errorf("json marshal failed: %s", err)
return fmt.Errorf("json marshal failed: %s", err)
return nil, fmt.Errorf("json marshal failed: %s", err)
}
if err := s.Stg.Update(ctx, s.GetObjStorageKey(obj), string(bs)); err != nil {
return err
return nil, err
}

return nil
return obj, nil
}

func (s *GenericStore) BatchDelete(ctx context.Context, keys []string) error {
Expand Down
4 changes: 2 additions & 2 deletions api/internal/core/store/store_mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ func (m *MockInterface) Create(ctx context.Context, obj interface{}) (interface{
return ret.Get(0), ret.Error(1)
}

func (m *MockInterface) Update(ctx context.Context, obj interface{}, createOnFail bool) error {
func (m *MockInterface) Update(ctx context.Context, obj interface{}, createOnFail bool) (interface{}, error) {
ret := m.Mock.Called(ctx, obj, createOnFail)
return ret.Error(0)
return ret.Get(0), ret.Error(1)
}

func (m *MockInterface) BatchDelete(ctx context.Context, keys []string) error {
Expand Down
7 changes: 6 additions & 1 deletion api/internal/core/store/store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,12 +726,17 @@ func TestGenericStore_Update(t *testing.T) {
tc.giveStore.Stg = mStorage
tc.giveStore.opt.Validator = mValidator

err := tc.giveStore.Update(context.TODO(), tc.giveObj, false)
ret, err := tc.giveStore.Update(context.TODO(), tc.giveObj, false)
assert.True(t, validateCalled, tc.caseDesc)
if err != nil {
assert.Equal(t, tc.wantErr, err, tc.caseDesc)
continue
}
retTs, ok := ret.(*TestStruct)
assert.True(t, ok)
// The returned value (retTs) should be the same as the input (tc.giveObj)
assert.Equal(t, tc.giveObj.Field1, retTs.Field1, tc.caseDesc)
assert.Equal(t, tc.giveObj.Field2, retTs.Field2, tc.caseDesc)
assert.True(t, createCalled, tc.caseDesc)
}
}
Expand Down
5 changes: 3 additions & 2 deletions api/internal/handler/consumer/consumer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,12 @@ func (h *Handler) Set(c droplet.Context) (interface{}, error) {
input.Consumer.ID = input.Consumer.Username
ensurePluginsDefValue(input.Plugins)

if err := h.consumerStore.Update(c.Context(), &input.Consumer, true); err != nil {
ret, err := h.consumerStore.Update(c.Context(), &input.Consumer, true)
if err != nil {
return handler.SpecCodeResponse(err), err
}

return nil, nil
return ret, nil
}

func ensurePluginsDefValue(plugins map[string]interface{}) {
Expand Down
58 changes: 54 additions & 4 deletions api/internal/handler/consumer/consumer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ func TestHandler_Create(t *testing.T) {
giveInput *SetInput
giveCtx context.Context
giveErr error
giveRet interface{}
wantErr error
wantInput *SetInput
wantRet interface{}
Expand All @@ -193,6 +194,17 @@ func TestHandler_Create(t *testing.T) {
},
},
giveCtx: context.WithValue(context.Background(), "test", "value"),
giveRet: &entity.Consumer{
BaseInfo: entity.BaseInfo{
ID: "name",
},
Username: "name",
Plugins: map[string]interface{}{
"jwt-auth": map[string]interface{}{
"exp": 86400,
},
},
},
wantInput: &SetInput{
Consumer: entity.Consumer{
BaseInfo: entity.BaseInfo{
Expand All @@ -206,7 +218,17 @@ func TestHandler_Create(t *testing.T) {
},
},
},
wantRet: nil,
wantRet: &entity.Consumer{
BaseInfo: entity.BaseInfo{
ID: "name",
},
Username: "name",
Plugins: map[string]interface{}{
"jwt-auth": map[string]interface{}{
"exp": 86400,
},
},
},
wantCalled: true,
},
{
Expand All @@ -221,6 +243,9 @@ func TestHandler_Create(t *testing.T) {
},
},
},
giveRet: &data.SpecCodeResponse{
StatusCode: http.StatusInternalServerError,
},
giveErr: fmt.Errorf("create failed"),
wantInput: &SetInput{
Consumer: entity.Consumer{
Expand Down Expand Up @@ -252,7 +277,7 @@ func TestHandler_Create(t *testing.T) {
assert.Equal(t, tc.giveCtx, args.Get(0))
assert.Equal(t, &tc.wantInput.Consumer, args.Get(1))
assert.True(t, args.Bool(2))
}).Return(tc.giveErr)
}).Return(tc.giveRet, tc.giveErr)

h := Handler{consumerStore: mStore}
ctx := droplet.NewContext()
Expand All @@ -271,6 +296,7 @@ func TestHandler_Update(t *testing.T) {
caseDesc string
giveInput *SetInput
giveCtx context.Context
giveRet interface{}
giveErr error
wantErr error
wantInput *entity.Consumer
Expand All @@ -290,6 +316,17 @@ func TestHandler_Update(t *testing.T) {
},
},
giveCtx: context.WithValue(context.Background(), "test", "value"),
giveRet: &entity.Consumer{
BaseInfo: entity.BaseInfo{
ID: "name",
},
Username: "name",
Plugins: map[string]interface{}{
"jwt-auth": map[string]interface{}{
"exp": 500,
},
},
},
wantInput: &entity.Consumer{
BaseInfo: entity.BaseInfo{
ID: "name",
Expand All @@ -301,7 +338,17 @@ func TestHandler_Update(t *testing.T) {
},
},
},
wantRet: nil,
wantRet: &entity.Consumer{
BaseInfo: entity.BaseInfo{
ID: "name",
},
Username: "name",
Plugins: map[string]interface{}{
"jwt-auth": map[string]interface{}{
"exp": 500,
},
},
},
wantCalled: true,
},
{
Expand All @@ -314,6 +361,9 @@ func TestHandler_Update(t *testing.T) {
},
},
},
giveRet: &data.SpecCodeResponse{
StatusCode: http.StatusInternalServerError,
},
giveErr: fmt.Errorf("create failed"),
wantInput: &entity.Consumer{
BaseInfo: entity.BaseInfo{
Expand Down Expand Up @@ -343,7 +393,7 @@ func TestHandler_Update(t *testing.T) {
assert.Equal(t, tc.giveCtx, args.Get(0))
assert.Equal(t, tc.wantInput, args.Get(1))
assert.True(t, args.Bool(2))
}).Return(tc.giveErr)
}).Return(tc.giveRet, tc.giveErr)

h := Handler{consumerStore: mStore}
ctx := droplet.NewContext()
Expand Down
10 changes: 6 additions & 4 deletions api/internal/handler/global_rule/global_rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,12 @@ func (h *Handler) Set(c droplet.Context) (interface{}, error) {
input.GlobalPlugins.ID = input.ID
}

if err := h.globalRuleStore.Update(c.Context(), &input.GlobalPlugins, true); err != nil {
ret, err := h.globalRuleStore.Update(c.Context(), &input.GlobalPlugins, true)
if err != nil {
return handler.SpecCodeResponse(err), err
}

return nil, nil
return ret, nil
}

func Patch(c *gin.Context) (interface{}, error) {
Expand All @@ -170,11 +171,12 @@ func Patch(c *gin.Context) (interface{}, error) {
return handler.SpecCodeResponse(err), err
}

if err := routeStore.Update(c, &globalRule, false); err != nil {
ret, err := routeStore.Update(c, &globalRule, false)
if err != nil {
return handler.SpecCodeResponse(err), err
}

return nil, nil
return ret, nil
}

type BatchDeleteInput struct {
Expand Down
19 changes: 17 additions & 2 deletions api/internal/handler/global_rule/global_rule_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ func TestHandler_Set(t *testing.T) {
caseDesc string
giveInput *SetInput
giveCtx context.Context
giveRet interface{}
giveErr error
wantErr error
wantInput *entity.GlobalPlugins
Expand All @@ -228,13 +229,24 @@ func TestHandler_Set(t *testing.T) {
},
},
giveCtx: context.WithValue(context.Background(), "test", "value"),
giveRet: &entity.GlobalPlugins{
BaseInfo: entity.BaseInfo{ID: "name"},
Plugins: map[string]interface{}{
"jwt-auth": map[string]interface{}{},
},
},
wantInput: &entity.GlobalPlugins{
BaseInfo: entity.BaseInfo{ID: "name"},
Plugins: map[string]interface{}{
"jwt-auth": map[string]interface{}{},
},
},
wantRet: nil,
wantRet: &entity.GlobalPlugins{
BaseInfo: entity.BaseInfo{ID: "name"},
Plugins: map[string]interface{}{
"jwt-auth": map[string]interface{}{},
},
},
wantCalled: true,
},
{
Expand All @@ -244,6 +256,9 @@ func TestHandler_Set(t *testing.T) {
GlobalPlugins: entity.GlobalPlugins{},
},
giveErr: fmt.Errorf("create failed"),
giveRet: &data.SpecCodeResponse{
StatusCode: http.StatusInternalServerError,
},
wantInput: &entity.GlobalPlugins{
BaseInfo: entity.BaseInfo{ID: "name"},
Plugins: map[string]interface{}(nil),
Expand All @@ -265,7 +280,7 @@ func TestHandler_Set(t *testing.T) {
assert.Equal(t, tc.giveCtx, args.Get(0))
assert.Equal(t, tc.wantInput, args.Get(1))
assert.True(t, args.Bool(2))
}).Return(tc.giveErr)
}).Return(tc.giveRet, tc.giveErr)

h := Handler{globalRuleStore: mStore}
ctx := droplet.NewContext()
Expand Down
12 changes: 7 additions & 5 deletions api/internal/handler/route/route.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,12 @@ func Patch(c *gin.Context) (interface{}, error) {
return handler.SpecCodeResponse(err), err
}

if err := routeStore.Update(c, &route, false); err != nil {
ret, err := routeStore.Update(c, &route, false)
if err != nil {
return handler.SpecCodeResponse(err), err
}

return nil, nil
return ret, nil
}

type GetInput struct {
Expand Down Expand Up @@ -427,7 +428,7 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
}

//save original conf
if err = h.scriptStore.Update(c.Context(), script, true); err != nil {
if _, err = h.scriptStore.Update(c.Context(), script, true); err != nil {
//if not exists, create
if err.Error() == fmt.Sprintf("key: %s is not found", script.ID) {
if _, err := h.scriptStore.Create(c.Context(), script); err != nil {
Expand All @@ -448,11 +449,12 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
}
}

if err := h.routeStore.Update(c.Context(), &input.Route, true); err != nil {
ret, err := h.routeStore.Update(c.Context(), &input.Route, true)
if err != nil {
return handler.SpecCodeResponse(err), err
}

return nil, nil
return ret, nil
}

type BatchDelete struct {
Expand Down
Loading

0 comments on commit 4cf46b0

Please sign in to comment.