You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by li...@apache.org on 2021/02/01 03:26:18 UTC

[apisix-dashboard] branch master updated: fix(be): Modify the upstream to use the new JSON patch package (#1395)

This is an automated email from the ASF dual-hosted git repository.

liuxiran pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-dashboard.git


The following commit(s) were added to refs/heads/master by this push:
     new 94d952f  fix(be): Modify the upstream to use the new JSON patch package (#1395)
94d952f is described below

commit 94d952f9a43a9d6507b2966b58791f789e5b74a6
Author: JinChen <36...@users.noreply.github.com>
AuthorDate: Mon Feb 1 11:26:08 2021 +0800

    fix(be): Modify the upstream to use the new JSON patch package (#1395)
    
    
    Co-authored-by: nic-chen <jo...@163.com>
---
 api/go.mod                                     |  2 -
 api/go.sum                                     |  4 --
 api/internal/handler/upstream/upstream.go      | 47 ++++++++-------
 api/internal/handler/upstream/upstream_test.go | 61 +++++++++++++++++++
 api/test/docker/Dockerfile-apisix              |  2 -
 api/test/e2e/base.go                           |  3 +
 api/test/e2e/upstream_test.go                  | 82 ++++++++++++++++++++++++++
 7 files changed, 169 insertions(+), 32 deletions(-)

diff --git a/api/go.mod b/api/go.mod
index e0b9457..e714d8a 100644
--- a/api/go.mod
+++ b/api/go.mod
@@ -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
diff --git a/api/go.sum b/api/go.sum
index d8234a3..5bc205f 100644
--- a/api/go.sum
+++ b/api/go.sum
@@ -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=
@@ -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=
diff --git a/api/internal/handler/upstream/upstream.go b/api/internal/handler/upstream/upstream.go
index faab379..eb78034 100644
--- a/api/internal/handler/upstream/upstream.go
+++ b/api/internal/handler/upstream/upstream.go
@@ -17,11 +17,11 @@
 package upstream
 
 import (
+	"encoding/json"
 	"net/http"
 	"reflect"
 	"strings"
 
-	"github.com/api7/go-jsonpatch"
 	"github.com/gin-gonic/gin"
 	"github.com/shiningrush/droplet"
 	"github.com/shiningrush/droplet/data"
@@ -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"
 )
 
@@ -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{}))))
 
@@ -198,39 +201,35 @@ 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
+	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
+	if err := json.Unmarshal(res, &upstream); err != nil {
 		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
 	}
diff --git a/api/internal/handler/upstream/upstream_test.go b/api/internal/handler/upstream/upstream_test.go
index 103fe3c..c22d99a 100644
--- a/api/internal/handler/upstream/upstream_test.go
+++ b/api/internal/handler/upstream/upstream_test.go
@@ -19,6 +19,7 @@ package upstream
 
 import (
 	"encoding/json"
+	"strings"
 	"testing"
 	"time"
 
@@ -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)
+
+}
+
diff --git a/api/test/docker/Dockerfile-apisix b/api/test/docker/Dockerfile-apisix
index 5c4bd31..bd1ad0a 100644
--- a/api/test/docker/Dockerfile-apisix
+++ b/api/test/docker/Dockerfile-apisix
@@ -31,8 +31,6 @@ RUN set -x \
     git \
     && luarocks install https://github.com/apache/apisix/raw/master/rockspec/apisix-${APISIX_VERSION}-0.rockspec --tree=/usr/local/apisix/deps \
     && cp -v /usr/local/apisix/deps/lib/luarocks/rocks-5.1/apisix/${APISIX_VERSION}-0/bin/apisix /usr/bin/ \
-    && bin='#! /usr/local/openresty/luajit/bin/luajit\npackage.path = "/usr/local/apisix/?.lua;" .. package.path' \
-    && sed -i "1s@.*@$bin@" /usr/bin/apisix \
     && mv /usr/local/apisix/deps/share/lua/5.1/apisix /usr/local/apisix \
     && apk del .builddeps build-base make unzip
 
diff --git a/api/test/e2e/base.go b/api/test/e2e/base.go
index 0d9a333..8d3385e 100644
--- a/api/test/e2e/base.go
+++ b/api/test/e2e/base.go
@@ -147,6 +147,9 @@ func BatchTestServerPort(t *testing.T, times int) map[string]int {
 	for i := 0; i < times; i++ {
 		client = &http.Client{}
 		resp, err = client.Do(req)
+		if err != nil {
+			fmt.Printf("err: %s", err)
+		}
 		assert.Nil(t, err)
 
 		bodyByte, err = ioutil.ReadAll(resp.Body)
diff --git a/api/test/e2e/upstream_test.go b/api/test/e2e/upstream_test.go
index 3936c11..be2dd6f 100644
--- a/api/test/e2e/upstream_test.go
+++ b/api/test/e2e/upstream_test.go
@@ -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)
+	}
+}
+