You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/25 02:34:29 UTC

[GitHub] [apisix-dashboard] starsz opened a new pull request #868: fix: delete POST method in /apisix/admin/customer

starsz opened a new pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [x] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   
   - Related issues
   
   fix #852
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530175649



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",

Review comment:
       OK, Fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733534053


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=h1) Report
   > Merging [#868](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=desc) (d214251) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/1a38fa6cf6540bc966098f6467a160312fb6b0fd?el=desc) (1a38fa6) will **increase** coverage by `0.32%`.
   > The diff coverage is `20.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/868/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #868      +/-   ##
   ==========================================
   + Coverage   43.03%   43.35%   +0.32%     
   ==========================================
     Files          18       18              
     Lines        1271     1257      -14     
   ==========================================
   - Hits          547      545       -2     
   + Misses        632      622      -10     
   + Partials       92       90       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `40.81% <20.00%> (+5.89%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=footer). Last update [1a38fa6...d214251](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733534053


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=h1) Report
   > Merging [#868](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=desc) (c855483) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/1a38fa6cf6540bc966098f6467a160312fb6b0fd?el=desc) (1a38fa6) will **increase** coverage by `0.32%`.
   > The diff coverage is `20.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/868/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #868      +/-   ##
   ==========================================
   + Coverage   43.03%   43.35%   +0.32%     
   ==========================================
     Files          18       18              
     Lines        1271     1257      -14     
   ==========================================
   - Hits          547      545       -2     
   + Misses        632      622      -10     
   + Partials       92       90       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `40.81% <20.00%> (+5.89%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=footer). Last update [1a38fa6...c855483](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530742418



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,

Review comment:
       @nic-chen Hi, ~~Why we should use the same `sleepTime`, In my opinion, only create and update consumer need to sleep for a while.~~
   
   Got it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530453605



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",

Review comment:
       useless?

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",

Review comment:
       useless ?

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,

Review comment:
       i  think 100~200ms should be enough

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",

Review comment:
       ditto 

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc: "update consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,

Review comment:
       ditto 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530744803



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc: "update consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_3",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"rejected_code\":504",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_3",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},

Review comment:
       OK, Fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530744710



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,

Review comment:
       OK, Fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: WIP: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r531108884



##########
File path: api/internal/handler/consumer/consumer.go
##########
@@ -97,35 +95,13 @@ 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)
-	if input.ID != "" && input.ID != input.Username {
-		return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest},
-			fmt.Errorf("consumer's id and username must be a same value")
-	}
-	input.ID = input.Username
-
-	if _, ok := input.Plugins["jwt-auth"]; ok {
-		jwt := input.Plugins["jwt-auth"].(map[string]interface{})
-		jwt["exp"] = 86400
-
-		input.Plugins["jwt-auth"] = jwt
-	}
-
-	if err := h.consumerStore.Create(c.Context(), input); err != nil {
-		return handler.SpecCodeResponse(err), err
-	}
-
-	return nil, nil
-}
-
-type UpdateInput struct {
+type SetInput struct {
 	Username string `auto_read:"username,path"`
 	entity.Consumer

Review comment:
       Good style.Fiexd.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530176500



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(t),
+			Path:     "/apisix/admin/consumers",
+			Method:   http.MethodPost,
+			Body: `{
+              "username": "consumer_1",

Review comment:
       Fixed.

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "update consumer by PUT",
+			Object:   MangerApiExpect(t),
+			Path:     "/apisix/admin/consumers/consumer_2",
+			Method:   http.MethodPut,
+			Body: `{
+              "username": "consumer_2",

Review comment:
       OK. Fixed

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(t),
+			Path:     "/apisix/admin/consumers",
+			Method:   http.MethodPut,
+			Body: `{
+              "username": "consumer_2",

Review comment:
       OK fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] moonming commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530680829



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,

Review comment:
       The response code is not enough, we need to check in data plane 

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,

Review comment:
       How to check the delete is successful?

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc: "update consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_3",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"rejected_code\":504",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_3",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},

Review comment:
       Ditto 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530124851



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(t),
+			Path:     "/apisix/admin/consumers",
+			Method:   http.MethodPost,
+			Body: `{
+              "username": "consumer_1",

Review comment:
       bad indentation

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "update consumer by PUT",
+			Object:   MangerApiExpect(t),
+			Path:     "/apisix/admin/consumers/consumer_2",
+			Method:   http.MethodPut,
+			Body: `{
+              "username": "consumer_2",

Review comment:
       ditto




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] membphis commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-742994415


   > I think it would be better to merge after #859 merged, we could not maintain handler's integration test cases anymore.
   
   @ShiningRush I think we can merge this PR and #859 now. what do you think?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] codecov-io commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733534053


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=h1) Report
   > Merging [#868](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=desc) (ae69c21) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/1a38fa6cf6540bc966098f6467a160312fb6b0fd?el=desc) (1a38fa6) will **increase** coverage by `0.32%`.
   > The diff coverage is `20.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/868/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #868      +/-   ##
   ==========================================
   + Coverage   43.03%   43.35%   +0.32%     
   ==========================================
     Files          18       18              
     Lines        1271     1257      -14     
   ==========================================
   - Hits          547      545       -2     
   + Misses        632      622      -10     
   + Partials       92       90       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `40.81% <20.00%> (+5.89%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=footer). Last update [1a38fa6...47e2905](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530745021



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",

Review comment:
       Fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530742418



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,

Review comment:
       @nic-chen Hi, Why we should use the same `sleepTime`, In my opinion, only create and update consumer need to sleep for a while.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] tokers commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530990653



##########
File path: api/internal/handler/consumer/consumer.go
##########
@@ -97,35 +95,13 @@ 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)
-	if input.ID != "" && input.ID != input.Username {
-		return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest},
-			fmt.Errorf("consumer's id and username must be a same value")
-	}
-	input.ID = input.Username
-
-	if _, ok := input.Plugins["jwt-auth"]; ok {
-		jwt := input.Plugins["jwt-auth"].(map[string]interface{})
-		jwt["exp"] = 86400
-
-		input.Plugins["jwt-auth"] = jwt
-	}
-
-	if err := h.consumerStore.Create(c.Context(), input); err != nil {
-		return handler.SpecCodeResponse(err), err
-	}
-
-	return nil, nil
-}
-
-type UpdateInput struct {
+type SetInput struct {
 	Username string `auto_read:"username,path"`
 	entity.Consumer

Review comment:
       Embed members should be put at the top.

##########
File path: api/test/e2e/base.go
##########
@@ -32,6 +33,11 @@ import (
 
 var token string
 
+var MExpect *httpexpect.Expect

Review comment:
       Use var (
   ......
   )
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733723620


   > @starsz Can the E2E case file use another name? because `consumer_test.go` is used by #830 ...
   
   Hi, I don't think it's a problem. Next PR can rebase the master branch if my PR is merged first.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733735036


   > CI failed. please have a look. thanks.
   
   OK. CI had passed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530175839



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "update consumer by PUT",
+			Object:   MangerApiExpect(t),
+			Path:     "/apisix/admin/consumers/consumer_2",
+			Method:   http.MethodPut,
+			Body: `{
+              "username": "consumer_2",
+              "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,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"rejected_code\":504",
+		},

Review comment:
       OK. Deleted




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733723620


   > @starsz Can the E2E case file use another name? because `consumer_test.go` is used by #830 ...
   
   I don't think it's a problem. Next PR can rebase the master branch if my PR is merged first.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733534053


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=h1) Report
   > Merging [#868](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=desc) (9a509f6) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/1a38fa6cf6540bc966098f6467a160312fb6b0fd?el=desc) (1a38fa6) will **increase** coverage by `0.24%`.
   > The diff coverage is `20.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/868/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #868      +/-   ##
   ==========================================
   + Coverage   43.03%   43.27%   +0.24%     
   ==========================================
     Files          18       18              
     Lines        1271     1257      -14     
   ==========================================
   - Hits          547      544       -3     
   + Misses        632      623       -9     
   + Partials       92       90       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `40.81% <20.00%> (+5.89%)` | :arrow_up: |
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.28% <0.00%> (-0.66%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=footer). Last update [1a38fa6...9a509f6](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733534053


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=h1) Report
   > Merging [#868](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=desc) (3f1d822) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/1a38fa6cf6540bc966098f6467a160312fb6b0fd?el=desc) (1a38fa6) will **increase** coverage by `0.32%`.
   > The diff coverage is `20.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/868/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #868      +/-   ##
   ==========================================
   + Coverage   43.03%   43.35%   +0.32%     
   ==========================================
     Files          18       18              
     Lines        1271     1257      -14     
   ==========================================
   - Hits          547      545       -2     
   + Misses        632      622      -10     
   + Partials       92       90       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `40.81% <20.00%> (+5.89%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=footer). Last update [1a38fa6...3f1d822](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] idbeta commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
idbeta commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-734038254


   > > @starsz Can the E2E case file use another name? because `consumer_test.go` is used by #830 ...
   > 
   > Hi, I don't think it's a problem. Next PR can rebase the master branch if my PR is merged first.
   
   got it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733534053


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=h1) Report
   > Merging [#868](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=desc) (1a0ae30) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/1a38fa6cf6540bc966098f6467a160312fb6b0fd?el=desc) (1a38fa6) will **increase** coverage by `0.32%`.
   > The diff coverage is `20.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/868/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #868      +/-   ##
   ==========================================
   + Coverage   43.03%   43.35%   +0.32%     
   ==========================================
     Files          18       18              
     Lines        1271     1257      -14     
   ==========================================
   - Hits          547      545       -2     
   + Misses        632      622      -10     
   + Partials       92       90       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `40.81% <20.00%> (+5.89%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=footer). Last update [1a38fa6...d214251](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] idbeta edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
idbeta edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733611666


   @starsz Can the E2E case file use another name?  because `consumer_test.go` is used by #830 ...


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-740697857


   Hi. Ping @membphis @moonming. Please have a look.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733534053


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=h1) Report
   > Merging [#868](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=desc) (47e2905) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/1a38fa6cf6540bc966098f6467a160312fb6b0fd?el=desc) (1a38fa6) will **increase** coverage by `0.32%`.
   > The diff coverage is `20.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/868/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #868      +/-   ##
   ==========================================
   + Coverage   43.03%   43.35%   +0.32%     
   ==========================================
     Files          18       18              
     Lines        1271     1257      -14     
   ==========================================
   - Hits          547      545       -2     
   + Misses        632      622      -10     
   + Partials       92       90       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `40.81% <20.00%> (+5.89%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=footer). Last update [1a38fa6...47e2905](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530678239



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,

Review comment:
       we could use the same 'sleepTime'




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: WIP: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r531114297



##########
File path: api/test/e2e/base.go
##########
@@ -32,6 +33,11 @@ import (
 
 var token string
 
+var MExpect *httpexpect.Expect

Review comment:
       Deleted those codes.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530153697



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(t),
+			Path:     "/apisix/admin/consumers",
+			Method:   http.MethodPut,
+			Body: `{
+              "username": "consumer_2",

Review comment:
       bad indentation
   
   

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",

Review comment:
       Need to test to get consumer first to ensure that the consumer has not been created before
   

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "update consumer by PUT",
+			Object:   MangerApiExpect(t),
+			Path:     "/apisix/admin/consumers/consumer_2",
+			Method:   http.MethodPut,
+			Body: `{
+              "username": "consumer_2",
+              "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,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"rejected_code\":504",
+		},

Review comment:
       Need to delete consumers just created.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] membphis merged pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
membphis merged pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] ShiningRush commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
ShiningRush commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-741572737


   I think it would be better to merge after #859 merged, we could not maintain handler's integration test cases anymore.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] nic-chen commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733610854


   CI failed. please have a look. thanks.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733534053


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=h1) Report
   > Merging [#868](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=desc) (51c9161) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/1a38fa6cf6540bc966098f6467a160312fb6b0fd?el=desc) (1a38fa6) will **increase** coverage by `0.32%`.
   > The diff coverage is `20.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/868/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #868      +/-   ##
   ==========================================
   + Coverage   43.03%   43.35%   +0.32%     
   ==========================================
     Files          18       18              
     Lines        1271     1257      -14     
   ==========================================
   - Hits          547      545       -2     
   + Misses        632      622      -10     
   + Partials       92       90       -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/868/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `40.81% <20.00%> (+5.89%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=footer). Last update [1a38fa6...51c9161](https://codecov.io/gh/apache/apisix-dashboard/pull/868?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530754229



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,

Review comment:
       Hi, @membphis, When I reduce the sleep time, It will cause CI to fail.
   So I think maybe it's not enough.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] idbeta commented on pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
idbeta commented on pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#issuecomment-733611666


   @starsz Can the E2E use case file use another name?  because `consumer_test.go` is used by #830 ...


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] starsz commented on a change in pull request #868: fix: delete POST method in /apisix/admin/consumer

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #868:
URL: https://github.com/apache/apisix-dashboard/pull/868#discussion_r530744940



##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",

Review comment:
       Fixed.

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",

Review comment:
       Fixed.

##########
File path: api/test/e2e/consumer_test.go
##########
@@ -0,0 +1,202 @@
+/*
+ * 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 e2e
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestConsumer_Create_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_1",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc:     "check consumer is not exist",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusNotFound,
+			ExpectBody:   "",
+		},
+		{
+			caseDesc: "create consumer by POST",
+			Object:   MangerApiExpect(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",
+		},
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc:     "get consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodGet,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "\"username\":\"consumer_2\"",
+		},
+		{
+			caseDesc: "create consumer without username",
+			Object:   MangerApiExpect(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:   "",
+		},
+		{
+			caseDesc:     "delete consumer",
+			Object:       MangerApiExpect(t),
+			Path:         "/apisix/admin/consumers/consumer_2",
+			Method:       http.MethodDelete,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "",
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+}
+
+func TestConsumer_Update_And_Get(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,
+		},
+		{
+			caseDesc: "update consumer by PUT",
+			Object:   MangerApiExpect(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:   "",
+			Sleep:        2 * time.Second,

Review comment:
       Fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org