Skip to content

Commit

Permalink
fix: PATCH method bug (apache#1005)
Browse files Browse the repository at this point in the history
* fix: PATCH method bug

* test: use sub path patch in e2e test

* fix: lint

* fix: naming stype

* fix: according to review

* fix: style
  • Loading branch information
nic-chen authored and starsz committed Dec 14, 2020
1 parent 51316a8 commit c756b1c
Show file tree
Hide file tree
Showing 6 changed files with 358 additions and 31 deletions.
1 change: 1 addition & 0 deletions api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf // indirect
github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f // indirect
github.com/dgrijalva/jwt-go v3.2.0+incompatible
github.com/evanphx/json-patch/v5 v5.1.0
github.com/gin-contrib/pprof v1.3.0
github.com/gin-contrib/sessions v0.0.3
github.com/gin-contrib/static v0.0.0-20200916080430-d45d9a37d28e
Expand Down
4 changes: 4 additions & 0 deletions api/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZm
github.com/elazarl/go-bindata-assetfs v1.0.0/go.mod h1:v+YaWX3bdea5J/mo8dSETolEo7R71Vk1u8bnjau5yw4=
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/evanphx/json-patch v4.9.0+incompatible h1:kLcOMZeuLAJvL2BPWLMIj5oaZQobrkAqrL+WFZwQses=
github.com/evanphx/json-patch/v5 v5.1.0 h1:B0aXl1o/1cP8NbviYiBMkcHBtUjIJ1/Ccg6b+SwCLQg=
github.com/evanphx/json-patch/v5 v5.1.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
github.com/gin-contrib/pprof v1.3.0 h1:G9eK6HnbkSqDZBYbzG4wrjCsA4e+cvYAHUZw6W+W9K0=
github.com/gin-contrib/pprof v1.3.0/go.mod h1:waMjT1H9b179t3CxuG1cV3DHpga6ybizwfBaM5OXaB0=
github.com/gin-contrib/sessions v0.0.3 h1:PoBXki+44XdJdlgDqDrY5nDVe3Wk7wDV/UCOuLP6fBI=
Expand Down Expand Up @@ -77,6 +80,7 @@ github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+
github.com/gorilla/sessions v1.1.1/go.mod h1:8KCfur6+4Mqcc6S0FEfKuN15Vl5MgXW92AE8ovaJD0w=
github.com/gorilla/sessions v1.1.3 h1:uXoZdcdA5XdXF3QzuSlheVRUvjl+1rKY7zBXL68L9RU=
github.com/gorilla/sessions v1.1.3/go.mod h1:8KCfur6+4Mqcc6S0FEfKuN15Vl5MgXW92AE8ovaJD0w=
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/json-iterator/go v1.1.7/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
github.com/json-iterator/go v1.1.10/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
Expand Down
42 changes: 16 additions & 26 deletions api/internal/handler/ssl/ssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"reflect"
"strings"

"github.com/api7/go-jsonpatch"
"github.com/gin-gonic/gin"
"github.com/shiningrush/droplet"
"github.com/shiningrush/droplet/data"
Expand All @@ -38,6 +37,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 @@ -62,13 +62,14 @@ func (h *Handler) ApplyRoute(r *gin.Engine) {
wrapper.InputType(reflect.TypeOf(UpdateInput{}))))
r.PUT("/apisix/admin/ssl/:id", wgin.Wraps(h.Update,
wrapper.InputType(reflect.TypeOf(UpdateInput{}))))
r.PATCH("/apisix/admin/ssl/:id", wgin.Wraps(h.Patch,
wrapper.InputType(reflect.TypeOf(UpdateInput{}))))
r.DELETE("/apisix/admin/ssl/:ids", wgin.Wraps(h.BatchDelete,
wrapper.InputType(reflect.TypeOf(BatchDelete{}))))
r.POST("/apisix/admin/check_ssl_cert", wgin.Wraps(h.Validate,
wrapper.InputType(reflect.TypeOf(entity.SSL{}))))

r.PATCH("/apisix/admin/ssl/:id", consts.ErrorWrapper(Patch))
r.PATCH("/apisix/admin/ssl/:id/*path", consts.ErrorWrapper(Patch))

r.POST("/apisix/admin/check_ssl_exists", consts.ErrorWrapper(Exist))
}

Expand Down Expand Up @@ -216,40 +217,29 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
return nil, nil
}

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]
}
func Patch(c *gin.Context) (interface{}, error) {
reqBody, _ := c.GetRawData()
ID := c.Param("id")
subPath := c.Param("path")

stored, err := h.sslStore.Get(input.ID)
sslStore := store.GetStore(store.HubKeySsl)
stored, err := sslStore.Get(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.SSL)
if err != nil {
return handler.SpecCodeResponse(err), err
}
res, err := utils.MergePatch(stored, subPath, reqBody)
if err != nil {
return handler.SpecCodeResponse(err), err
}

err = patch.Apply(&stored)
var ssl entity.SSL
err = json.Unmarshal(res, &ssl)
if err != nil {
return handler.SpecCodeResponse(err), err
}

if err := h.sslStore.Update(c.Context(), &stored, false); err != nil {
if err := sslStore.Update(c, &ssl, false); err != nil {
return handler.SpecCodeResponse(err), err
}

Expand Down
75 changes: 75 additions & 0 deletions api/internal/utils/json_patch.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* 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 utils

import (
"encoding/json"
jsonpatch "github.com/evanphx/json-patch/v5"
)

func MergeJson(doc, patch []byte) ([]byte, error) {
out, err := jsonpatch.MergePatch(doc, patch)

if err != nil {
return nil, err
}
return out, nil
}

func PatchJson(doc []byte, path, val string) ([]byte, error) {
patch := []byte(`[ { "op": "replace", "path": "` + path + `", "value": ` + val + `}]`)
obj, err := jsonpatch.DecodePatch(patch)
if err != nil {
return nil, err
}

out, err := obj.Apply(doc)

if err != nil {
// try to add if field not exist
patch = []byte(`[ { "op": "add", "path": "` + path + `", "value": ` + val + `}]`)
obj, err = jsonpatch.DecodePatch(patch)
if err != nil {
return nil, err
}
out, err = obj.Apply(doc)
if err != nil {
return nil, err
}
}

return out, nil
}

func MergePatch(obj interface{}, subPath string, reqBody []byte) ([]byte, error) {
var res []byte
jsonBytes, err := json.Marshal(obj)
if err != nil {
return res, err
}

if subPath != "" {
res, err = PatchJson(jsonBytes, subPath, string(reqBody))
} else {
res, err = MergeJson(jsonBytes, reqBody)
}

if err != nil {
return res, err
}
return res, nil
}
211 changes: 211 additions & 0 deletions api/internal/utils/json_patch_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
/*
* 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 utils

import (
"bytes"
"encoding/json"
"reflect"
"testing"
)

func compareJSON(a, b string) bool {
var objA, objB interface{}
json.Unmarshal([]byte(a), &objA)
json.Unmarshal([]byte(b), &objB)

return reflect.DeepEqual(objA, objB)
}

func formatJSON(j string) string {
buf := new(bytes.Buffer)

json.Indent(buf, []byte(j), "", " ")

return buf.String()
}

func TestMergeJson(t *testing.T) {
cases := []struct {
doc, patch, result, desc string
}{
{
desc: "simple merge",
doc: `{
"id": "1",
"status": 1,
"key": "fake key",
"cert": "fake cert",
"create_time": 1,
"update_time": 2
}`,
patch: `{
"id": "1",
"status": 0,
"key": "fake key1",
"cert": "fake cert1"
}`,
result: `{
"id": "1",
"status": 0,
"key": "fake key1",
"cert": "fake cert1",
"create_time": 1,
"update_time": 2
}`,
},
{
desc: `array merge`,
doc: `{
"uri": "/index.html",
"upstream": {
"type": "roundrobin",
"nodes": [{
"host": "39.97.63.215",
"port": 80,
"weight" : 1
}]
}
}`,
patch: `{
"upstream": {
"nodes": [{
"host": "39.97.63.216",
"port": 80,
"weight" : 1
},{
"host": "39.97.63.217",
"port": 80,
"weight" : 1
}]
}
}`,
result: `{
"uri": "/index.html",
"upstream": {
"type": "roundrobin",
"nodes": [{
"host": "39.97.63.216",
"port": 80,
"weight" : 1
},{
"host": "39.97.63.217",
"port": 80,
"weight" : 1
}]
}
}`,
},
}
for _, c := range cases {
out, err := MergeJson([]byte(c.doc), []byte(c.patch))

if err != nil {
t.Errorf("Unable to merge patch: %s", err)
}

if !compareJSON(string(out), c.result) {
t.Errorf("Merge failed. Expected:\n%s\n\nActual:\n%s",
formatJSON(c.result), formatJSON(string(out)))
}
}
}

func TestPatchJson(t *testing.T) {
cases := []struct {
doc, path, value, result, desc string
}{
{
desc: "patch array",
doc: `{
"uri": "/index.html",
"upstream": {
"type": "roundrobin",
"nodes": [{
"host": "39.97.63.215",
"port": 80,
"weight" : 1
}]
}
}`,
path: `/upstream/nodes`,
value: `[{
"host": "39.97.63.216",
"port": 80,
"weight" : 1
},{
"host": "39.97.63.217",
"port": 80,
"weight" : 1
}]`,
result: `{
"uri": "/index.html",
"upstream": {
"type": "roundrobin",
"nodes": [{
"host": "39.97.63.216",
"port": 80,
"weight" : 1
},{
"host": "39.97.63.217",
"port": 80,
"weight" : 1
}]
}
}`,
},
{
desc: "patch field that non existent",
doc: `{
"uri": "/index.html",
"upstream": {
"type": "roundrobin",
"nodes": [{
"host": "39.97.63.215",
"port": 80,
"weight" : 1
}]
}
}`,
path: `/upstream/labels`,
value: `{"app": "test"}`,
result: `{
"uri": "/index.html",
"upstream": {
"type": "roundrobin",
"nodes": [{
"host": "39.97.63.215",
"port": 80,
"weight" : 1
}],
"labels": {"app": "test"}
}
}`,
},
}
for _, c := range cases {
out, err := PatchJson([]byte(c.doc), c.path, c.value)
if err != nil {
t.Errorf("Unable to patch: %s", err)
}

if !compareJSON(string(out), c.result) {
t.Errorf("Patch failed. Expected:\n%s\n\nActual:\n%s",
formatJSON(c.result), formatJSON(string(out)))
}
}
}
Loading

0 comments on commit c756b1c

Please sign in to comment.