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

fix(be): Modify the upstream to use the new JSON patch package #1395

Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 0 additions & 2 deletions api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ replace google.golang.org/grpc => google.golang.org/grpc v1.26.0
replace github.com/coreos/bbolt => go.etcd.io/bbolt v1.3.5

require (
github.com/api7/go-jsonpatch v0.0.0-20180223123257-a8710867776e
github.com/cameront/go-jsonpatch v0.0.0-20180223123257-a8710867776e // indirect
github.com/coreos/bbolt v0.0.0-00010101000000-000000000000 // indirect
github.com/coreos/etcd v3.3.25+incompatible // indirect
github.com/coreos/go-semver v0.3.0 // indirect
Expand Down
4 changes: 0 additions & 4 deletions api/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk5
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
github.com/api7/go-jsonpatch v0.0.0-20180223123257-a8710867776e h1:TX/8xM53DHaIIBr4wU+ifYI8IkUzS8+HrX8kzIzaaG0=
github.com/api7/go-jsonpatch v0.0.0-20180223123257-a8710867776e/go.mod h1:yc49guNPyTy2deyszk3erQN+2vO2NwLKPNicXurj7Hs=
github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=
github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY=
github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
Expand All @@ -33,8 +31,6 @@ github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kB
github.com/boj/redistore v0.0.0-20180917114910-cd5dcc76aeff/go.mod h1:+RTT1BOk5P97fT2CiHkbFQwkK3mjsFAP6zCYV2aXtjw=
github.com/bradfitz/gomemcache v0.0.0-20190329173943-551aad21a668/go.mod h1:H0wQNHz2YrLsuXOZozoeDmnHXkNCRmMW0gwFWDfEZDA=
github.com/bradleypeabody/gorilla-sessions-memcache v0.0.0-20181103040241-659414f458e1/go.mod h1:dkChI7Tbtx7H1Tj7TqGSZMOeGpMP5gLHtjroHd4agiI=
github.com/cameront/go-jsonpatch v0.0.0-20180223123257-a8710867776e h1:6c3+GQuYUWljNcReOg4gxMUss9Gjll+5Y9vqDM+ILy8=
github.com/cameront/go-jsonpatch v0.0.0-20180223123257-a8710867776e/go.mod h1:kdPJxKAfR3ZdD+MWYorN1oTdV9+qwJy9jO/0meJmcxU=
github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ=
github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
Expand Down
49 changes: 25 additions & 24 deletions api/internal/handler/upstream/upstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
package upstream

import (
"encoding/json"
"net/http"
"reflect"
"strings"

"github.com/api7/go-jsonpatch"
nic-chen marked this conversation as resolved.
Show resolved Hide resolved
"github.com/gin-gonic/gin"
"github.com/shiningrush/droplet"
"github.com/shiningrush/droplet/data"
Expand All @@ -31,6 +31,7 @@ import (
"github.com/apisix/manager-api/internal/core/entity"
"github.com/apisix/manager-api/internal/core/store"
"github.com/apisix/manager-api/internal/handler"
"github.com/apisix/manager-api/internal/utils"
"github.com/apisix/manager-api/internal/utils/consts"
)

Expand All @@ -56,7 +57,9 @@ func (h *Handler) ApplyRoute(r *gin.Engine) {
r.PUT("/apisix/admin/upstreams/:id", wgin.Wraps(h.Update,
wrapper.InputType(reflect.TypeOf(UpdateInput{}))))
r.PATCH("/apisix/admin/upstreams/:id", wgin.Wraps(h.Patch,
wrapper.InputType(reflect.TypeOf(UpdateInput{}))))
wrapper.InputType(reflect.TypeOf(PatchInput{}))))
r.PATCH("/apisix/admin/upstreams/:id/*path", wgin.Wraps(h.Patch,
wrapper.InputType(reflect.TypeOf(PatchInput{}))))
r.DELETE("/apisix/admin/upstreams/:ids", wgin.Wraps(h.BatchDelete,
wrapper.InputType(reflect.TypeOf(BatchDelete{}))))

Expand Down Expand Up @@ -198,39 +201,36 @@ func (h *Handler) BatchDelete(c droplet.Context) (interface{}, error) {
return nil, nil
}

type PatchInput struct {
ID string `auto_read:"id,path"`
SubPath string `auto_read:"path,path"`
Body []byte `auto_read:"@body"`
}

func (h *Handler) Patch(c droplet.Context) (interface{}, error) {
input := c.Input().(*UpdateInput)
arr := strings.Split(input.ID, "/")
var subPath string
if len(arr) > 1 {
input.ID = arr[0]
subPath = arr[1]
}
input := c.Input().(*PatchInput)
reqBody := input.Body
ID := input.ID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use variables started with letter in small case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. Thks

subPath := input.SubPath

stored, err := h.upstreamStore.Get(c.Context(), input.ID)
stored, err := h.upstreamStore.Get(c.Context(), ID)
if err != nil {
return handler.SpecCodeResponse(err), err
}

var patch jsonpatch.Patch
if subPath != "" {
patch = jsonpatch.Patch{
Operations: []jsonpatch.PatchOperation{
{Op: jsonpatch.Replace, Path: subPath, Value: c.Input()},
},
}
} else {
patch, err = jsonpatch.MakePatch(stored, input.Upstream)
if err != nil {
return handler.SpecCodeResponse(err), err
}
res, err := utils.MergePatch(stored, subPath, reqBody)

if err != nil {
return handler.SpecCodeResponse(err), err
}

if err := patch.Apply(&stored); err != nil {
var upstream entity.Upstream
err = json.Unmarshal(res, &upstream)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be merged into:

if err := json.Unmarshal(res, &upstream); err != nil {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. Thks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be merged into:

if err := json.Unmarshal(res, &upstream); err != nil {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. Thks

return handler.SpecCodeResponse(err), err
}

ret, err := h.upstreamStore.Update(c.Context(), &stored, false)
ret, err := h.upstreamStore.Update(c.Context(), &upstream, false)
if err != nil {
return handler.SpecCodeResponse(err), err
}
Expand Down Expand Up @@ -314,3 +314,4 @@ func (h *Handler) listUpstreamNames(c droplet.Context) (interface{}, error) {

return output, nil
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why keep an empty line here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my problem. Maybe I forgot to delete it when using vim. Done. Thks

61 changes: 61 additions & 0 deletions api/internal/handler/upstream/upstream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package upstream

import (
"encoding/json"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -240,3 +241,63 @@ func TestUpstream_Pass_Host(t *testing.T) {
assert.Nil(t, err)

}

func TestUpstream_Patch_Update(t *testing.T) {
//create
ctx := droplet.NewContext()
upstream := &entity.Upstream{}
reqBody := `{
"id": "3",
"nodes": [{
"host": "172.16.238.20",
"port": 1980,
"weight": 1
}],
"type": "roundrobin"
}`
err := json.Unmarshal([]byte(reqBody), upstream)
assert.Nil(t, err)
ctx.SetInput(upstream)
ret, err := upstreamHandler.Create(ctx)
assert.Nil(t, err)
objRet, ok := ret.(*entity.Upstream)
assert.True(t, ok)
assert.Equal(t, "3", objRet.ID)

//sleep
time.Sleep(time.Duration(20) * time.Millisecond)

reqBody1 := `{
"nodes": [{
"host": "172.16.238.20",
"port": 1981,
"weight": 1
}],
"type": "roundrobin"
}`
responesBody := `"nodes":[{"host":"172.16.238.20","port":1981,"weight":1}],"type":"roundrobin"}`

input2 := &PatchInput{}
input2.ID = "3"
input2.SubPath = ""
input2.Body = []byte(reqBody1)
ctx.SetInput(input2)

ret2, err := upstreamHandler.Patch(ctx)
assert.Nil(t, err)
_ret2, err := json.Marshal(ret2)
assert.Nil(t, err)
isContains := strings.Contains(string(_ret2), responesBody)
assert.True(t, isContains)

//delete test data
inputDel2 := &BatchDelete{}
reqBody = `{"ids": "3"}`
err = json.Unmarshal([]byte(reqBody), inputDel2)
assert.Nil(t, err)
ctx.SetInput(inputDel2)
_, err = upstreamHandler.BatchDelete(ctx)
assert.Nil(t, err)

}

82 changes: 82 additions & 0 deletions api/test/e2e/upstream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,3 +523,85 @@ func TestUpstream_Create_via_Post(t *testing.T) {
testCaseCheck(tc, t)
}
}

func TestUpstream_Update_Use_Patch_Method(t *testing.T) {
tests := []HttpTestCase{
{
Desc: "create upstream via POST",
Object: ManagerApiExpect(t),
Method: http.MethodPost,
Path: "/apisix/admin/upstreams",
Body: `{
"id": "u1",
"nodes": [{
"host": "172.16.238.20",
"port": 1980,
"weight": 1
}],
"type": "roundrobin"
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
ExpectBody: []string{"\"id\":\"u1\"", "\"type\":\"roundrobin\""},
},
{
Desc: "update upstream use patch method",
Object: ManagerApiExpect(t),
Method: http.MethodPatch,
Path: "/apisix/admin/upstreams/u1",
Body: `{
"nodes": [{
"host": "172.16.238.20",
"port": 1981,
"weight": 1
}],
"type": "roundrobin"
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
},
{
Desc: "get upstream data",
Object: ManagerApiExpect(t),
Method: http.MethodGet,
Path: "/apisix/admin/upstreams/u1",
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
ExpectBody: "nodes\":[{\"host\":\"172.16.238.20\",\"port\":1981,\"weight\":1}],\"type\":\"roundrobin\"}",
},
{
Desc: "Update upstream using path parameter patch method",
Object: ManagerApiExpect(t),
Method: http.MethodPatch,
Path: "/apisix/admin/upstreams/u1/nodes",
Body: `[{"host":"172.16.238.20","port": 1980,"weight":1}]`,
Headers: map[string]string{
"Authorization": token,
"Content-Type": "text/plain",
},
ExpectStatus: http.StatusOK,
},
{
Desc: "get upstream data",
Object: ManagerApiExpect(t),
Method: http.MethodGet,
Path: "/apisix/admin/upstreams/u1",
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
ExpectBody: "nodes\":[{\"host\":\"172.16.238.20\",\"port\":1980,\"weight\":1}],\"type\":\"roundrobin\"}",
},
{
Desc: "delete upstream",
Object: ManagerApiExpect(t),
Method: http.MethodDelete,
Path: "/apisix/admin/upstreams/u1",
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
},
}

for _, tc := range tests {
testCaseCheck(tc, t)
}
}