You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by to...@apache.org on 2020/12/25 06:02:17 UTC
[apisix-dashboard] branch master updated: fix: delete POST method
in /apisix/admin/consumer (#1109)
This is an automated email from the ASF dual-hosted git repository.
tokers 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 b3463fd fix: delete POST method in /apisix/admin/consumer (#1109)
b3463fd is described below
commit b3463fda2c949833d5dba2c12c8fe3ab6a25399a
Author: Peter Zhu <st...@gmail.com>
AuthorDate: Fri Dec 25 14:02:09 2020 +0800
fix: delete POST method in /apisix/admin/consumer (#1109)
fixed #852
related #868
---
api/internal/handler/consumer/consumer.go | 38 +++---
api/internal/handler/consumer/consumer_test.go | 87 ++++++------
api/test/e2e/consumer_test.go | 179 +++++++++++++++++++++++++
api/test/e2e/route_online_debug_test.go | 2 +-
api/test/shell/cli_test.sh | 4 +-
web/src/pages/Consumer/service.ts | 2 +-
6 files changed, 248 insertions(+), 64 deletions(-)
diff --git a/api/internal/handler/consumer/consumer.go b/api/internal/handler/consumer/consumer.go
index 31b7af3..b3ea5a1 100644
--- a/api/internal/handler/consumer/consumer.go
+++ b/api/internal/handler/consumer/consumer.go
@@ -17,17 +17,21 @@
package consumer
import (
+ "fmt"
+ "net/http"
"reflect"
"strings"
"github.com/gin-gonic/gin"
"github.com/shiningrush/droplet"
+ "github.com/shiningrush/droplet/data"
"github.com/shiningrush/droplet/wrapper"
wgin "github.com/shiningrush/droplet/wrapper/gin"
"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"
)
type Handler struct {
@@ -45,12 +49,10 @@ func (h *Handler) ApplyRoute(r *gin.Engine) {
wrapper.InputType(reflect.TypeOf(GetInput{}))))
r.GET("/apisix/admin/consumers", wgin.Wraps(h.List,
wrapper.InputType(reflect.TypeOf(ListInput{}))))
- r.POST("/apisix/admin/consumers", wgin.Wraps(h.Create,
- wrapper.InputType(reflect.TypeOf(entity.Consumer{}))))
- r.PUT("/apisix/admin/consumers/:username", wgin.Wraps(h.Update,
- wrapper.InputType(reflect.TypeOf(UpdateInput{}))))
- r.PUT("/apisix/admin/consumers", wgin.Wraps(h.Update,
- wrapper.InputType(reflect.TypeOf(UpdateInput{}))))
+ r.PUT("/apisix/admin/consumers/:username", wgin.Wraps(h.Set,
+ wrapper.InputType(reflect.TypeOf(SetInput{}))))
+ r.PUT("/apisix/admin/consumers", wgin.Wraps(h.Set,
+ wrapper.InputType(reflect.TypeOf(SetInput{}))))
r.DELETE("/apisix/admin/consumers/:usernames", wgin.Wraps(h.BatchDelete,
wrapper.InputType(reflect.TypeOf(BatchDeleteInput{}))))
}
@@ -128,25 +130,17 @@ func (h *Handler) List(c droplet.Context) (interface{}, error) {
return ret, nil
}
-func (h *Handler) Create(c droplet.Context) (interface{}, error) {
- input := c.Input().(*entity.Consumer)
- input.ID = input.Username
-
- ensurePluginsDefValue(input.Plugins)
- if err := h.consumerStore.Create(c.Context(), input); err != nil {
- return handler.SpecCodeResponse(err), err
- }
-
- return nil, nil
-}
-
-type UpdateInput struct {
- Username string `auto_read:"username,path"`
+type SetInput struct {
entity.Consumer
+ Username string `auto_read:"username,path"`
}
-func (h *Handler) Update(c droplet.Context) (interface{}, error) {
- input := c.Input().(*UpdateInput)
+func (h *Handler) Set(c droplet.Context) (interface{}, error) {
+ input := c.Input().(*SetInput)
+ if input.ID != nil && utils.InterfaceToString(input.ID) != input.Username {
+ return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest},
+ fmt.Errorf("consumer's id and username must be a same value")
+ }
if input.Username != "" {
input.Consumer.Username = input.Username
}
diff --git a/api/internal/handler/consumer/consumer_test.go b/api/internal/handler/consumer/consumer_test.go
index 6a82154..5c80d90 100644
--- a/api/internal/handler/consumer/consumer_test.go
+++ b/api/internal/handler/consumer/consumer_test.go
@@ -20,14 +20,16 @@ package consumer
import (
"context"
"fmt"
- "github.com/apisix/manager-api/internal/core/entity"
- "github.com/apisix/manager-api/internal/core/store"
+ "net/http"
+ "testing"
+
"github.com/shiningrush/droplet"
"github.com/shiningrush/droplet/data"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
- "net/http"
- "testing"
+
+ "github.com/apisix/manager-api/internal/core/entity"
+ "github.com/apisix/manager-api/internal/core/store"
)
func TestHandler_Get(t *testing.T) {
@@ -172,31 +174,35 @@ func TestHandler_List(t *testing.T) {
func TestHandler_Create(t *testing.T) {
tests := []struct {
caseDesc string
- giveInput *entity.Consumer
+ giveInput *SetInput
giveCtx context.Context
giveErr error
wantErr error
- wantInput *entity.Consumer
+ wantInput *SetInput
wantRet interface{}
wantCalled bool
}{
{
caseDesc: "normal",
- giveInput: &entity.Consumer{
- Username: "name",
- Plugins: map[string]interface{}{
- "jwt-auth": map[string]interface{}{},
+ giveInput: &SetInput{
+ Consumer: entity.Consumer{
+ Username: "name",
+ Plugins: map[string]interface{}{
+ "jwt-auth": map[string]interface{}{},
+ },
},
},
giveCtx: context.WithValue(context.Background(), "test", "value"),
- wantInput: &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{
+ ID: "name",
+ },
+ Username: "name",
+ Plugins: map[string]interface{}{
+ "jwt-auth": map[string]interface{}{
+ "exp": 86400,
+ },
},
},
},
@@ -205,23 +211,27 @@ func TestHandler_Create(t *testing.T) {
},
{
caseDesc: "store create failed",
- giveInput: &entity.Consumer{
- Username: "name",
- Plugins: map[string]interface{}{
- "jwt-auth": map[string]interface{}{
- "exp": 5000,
+ giveInput: &SetInput{
+ Consumer: entity.Consumer{
+ Username: "name",
+ Plugins: map[string]interface{}{
+ "jwt-auth": map[string]interface{}{
+ "exp": 5000,
+ },
},
},
},
giveErr: fmt.Errorf("create failed"),
- wantInput: &entity.Consumer{
- BaseInfo: entity.BaseInfo{
- ID: "name",
- },
- Username: "name",
- Plugins: map[string]interface{}{
- "jwt-auth": map[string]interface{}{
- "exp": 5000,
+ wantInput: &SetInput{
+ Consumer: entity.Consumer{
+ BaseInfo: entity.BaseInfo{
+ ID: "name",
+ },
+ Username: "name",
+ Plugins: map[string]interface{}{
+ "jwt-auth": map[string]interface{}{
+ "exp": 5000,
+ },
},
},
},
@@ -237,17 +247,18 @@ func TestHandler_Create(t *testing.T) {
t.Run(tc.caseDesc, func(t *testing.T) {
methodCalled := true
mStore := &store.MockInterface{}
- mStore.On("Create", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
+ mStore.On("Update", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
methodCalled = true
assert.Equal(t, tc.giveCtx, args.Get(0))
- assert.Equal(t, tc.wantInput, args.Get(1))
+ assert.Equal(t, &tc.wantInput.Consumer, args.Get(1))
+ assert.True(t, args.Bool(2))
}).Return(tc.giveErr)
h := Handler{consumerStore: mStore}
ctx := droplet.NewContext()
ctx.SetInput(tc.giveInput)
ctx.SetContext(tc.giveCtx)
- ret, err := h.Create(ctx)
+ ret, err := h.Set(ctx)
assert.Equal(t, tc.wantCalled, methodCalled)
assert.Equal(t, tc.wantRet, ret)
assert.Equal(t, tc.wantErr, err)
@@ -258,7 +269,7 @@ func TestHandler_Create(t *testing.T) {
func TestHandler_Update(t *testing.T) {
tests := []struct {
caseDesc string
- giveInput *UpdateInput
+ giveInput *SetInput
giveCtx context.Context
giveErr error
wantErr error
@@ -268,7 +279,7 @@ func TestHandler_Update(t *testing.T) {
}{
{
caseDesc: "normal",
- giveInput: &UpdateInput{
+ giveInput: &SetInput{
Username: "name",
Consumer: entity.Consumer{
Plugins: map[string]interface{}{
@@ -295,7 +306,7 @@ func TestHandler_Update(t *testing.T) {
},
{
caseDesc: "store update failed",
- giveInput: &UpdateInput{
+ giveInput: &SetInput{
Username: "name",
Consumer: entity.Consumer{
Plugins: map[string]interface{}{
@@ -338,7 +349,7 @@ func TestHandler_Update(t *testing.T) {
ctx := droplet.NewContext()
ctx.SetInput(tc.giveInput)
ctx.SetContext(tc.giveCtx)
- ret, err := h.Update(ctx)
+ ret, err := h.Set(ctx)
assert.Equal(t, tc.wantCalled, methodCalled)
assert.Equal(t, tc.wantRet, ret)
assert.Equal(t, tc.wantErr, err)
diff --git a/api/test/e2e/consumer_test.go b/api/test/e2e/consumer_test.go
index d607957..2edfdf3 100644
--- a/api/test/e2e/consumer_test.go
+++ b/api/test/e2e/consumer_test.go
@@ -27,6 +27,185 @@ import (
"github.com/tidwall/gjson"
)
+func TestConsumer_Create_And_Get(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "check consumer is not exist",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers/consumer_1",
+ Method: http.MethodGet,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ ExpectBody: "data not found",
+ },
+ {
+ Desc: "check consumer is not exist",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers/consumer_2",
+ Method: http.MethodGet,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ ExpectBody: "data not found",
+ },
+ {
+ Desc: "create consumer by POST method",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers",
+ Method: http.MethodPost,
+ Body: `{
+ "username": "consumer_1",
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ },
+ "desc": "test description"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ ExpectBody: "404 page not found",
+ },
+ {
+ Desc: "create consumer by PUT method",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers",
+ Method: http.MethodPut,
+ Body: `{
+ "username": "consumer_2",
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ },
+ "desc": "test description"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "\"code\":0",
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "get consumer",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers/consumer_2",
+ Method: http.MethodGet,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "\"username\":\"consumer_2\"",
+ },
+ {
+ Desc: "create consumer without username",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers",
+ Method: http.MethodPut,
+ Body: `{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ },
+ "desc": "test description"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "\"code\":10000",
+ },
+ {
+ Desc: "delete consumer",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers/consumer_2",
+ Method: http.MethodDelete,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "\"code\":0",
+ },
+ }
+
+ for _, tc := range tests {
+ testCaseCheck(tc, t)
+ }
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "create consumer by PUT",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers",
+ Method: http.MethodPut,
+ Body: `{
+ "username": "consumer_3",
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ },
+ "desc": "test description"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "\"code\":0",
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "update consumer by PUT",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers/consumer_3",
+ Method: http.MethodPut,
+ Body: `{
+ "username": "consumer_3",
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 504,
+ "key": "remote_addr"
+ }
+ },
+ "desc": "test description"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "\"code\":0",
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "get consumer",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers/consumer_3",
+ Method: http.MethodGet,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "\"rejected_code\":504",
+ },
+ {
+ Desc: "delete consumer",
+ Object: ManagerApiExpect(t),
+ Path: "/apisix/admin/consumers/consumer_3",
+ Method: http.MethodDelete,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "\"code\":0",
+ },
+ }
+
+ for _, tc := range tests {
+ testCaseCheck(tc, t)
+ }
+}
+
func TestConsumer_with_key_auth(t *testing.T) {
tests := []HttpTestCase{
{
diff --git a/api/test/e2e/route_online_debug_test.go b/api/test/e2e/route_online_debug_test.go
index 377af3f..e8fc679 100644
--- a/api/test/e2e/route_online_debug_test.go
+++ b/api/test/e2e/route_online_debug_test.go
@@ -320,7 +320,7 @@ func TestRoute_Online_Debug_Route_With_Basic_Auth(t *testing.T) {
Desc: "create consumer",
Object: ManagerApiExpect(t),
Path: "/apisix/admin/consumers",
- Method: http.MethodPost,
+ Method: http.MethodPut,
Body: `{
"username": "jack",
"plugins": {
diff --git a/api/test/shell/cli_test.sh b/api/test/shell/cli_test.sh
index 411130b..da20c56 100755
--- a/api/test/shell/cli_test.sh
+++ b/api/test/shell/cli_test.sh
@@ -95,7 +95,7 @@ if [[ `grep -c "INFO" ./error.log` -eq '0' ]]; then
exit 1
fi
-# run on a different path
+# run on a different path
workDir=$(pwd)
rm -rf html
mkdir html
@@ -215,7 +215,7 @@ if [ -z "${token}" ]; then
fi
# more validation to make sure it's ok to access etcd
-resp=$(curl -ig http://127.0.0.1:9000/apisix/admin/consumers -i -H "Authorization: $token" -d '{"username":"etcd_basic_auth_test"}')
+resp=$(curl -ig -XPUT http://127.0.0.1:9000/apisix/admin/consumers -i -H "Authorization: $token" -d '{"username":"etcd_basic_auth_test"}')
respCode=$(echo "${resp}" | sed 's/{/\n/g'| sed 's/,/\n/g' | grep "code" | sed 's/:/\n/g' | sed '1d')
respMessage=$(echo "${resp}" | sed 's/{/\n/g'| sed 's/,/\n/g' | grep "message" | sed 's/:/\n/g' | sed '1d')
if [ "$respCode" != "0" ] || [ $respMessage != "\"\"" ]; then
diff --git a/web/src/pages/Consumer/service.ts b/web/src/pages/Consumer/service.ts
index f4289c6..f95aa31 100644
--- a/web/src/pages/Consumer/service.ts
+++ b/web/src/pages/Consumer/service.ts
@@ -33,7 +33,7 @@ export const fetchItem = (username: string) =>
export const create = (data: ConsumerModule.Entity) =>
request('/consumers', {
- method: 'POST',
+ method: 'PUT',
data,
});