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 2021/03/17 02:57:43 UTC
[GitHub] [apisix-dashboard] nic-chen opened a new pull request #1606: fix: check name existed when creating a resource
nic-chen opened a new pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [x] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
- Related issues
#1595
___
### Bugfix
- Description
When creating route, server and upstream in the Dashboard, the FE queries whether the same name exists through API, and does not run the addition if it exists. But the backend lacks this limitation.
- How to fix?
Add a judgment on whether the name exists in the backend, if it exists, it is not allowed to create or update.
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595876271
##########
File path: api/internal/handler/route/route.go
##########
@@ -379,12 +380,25 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Create(c.Context(), input)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
Can't, because the name is not index.
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r596026656
##########
File path: api/internal/handler/handler.go
##########
@@ -104,3 +107,41 @@ func IDCompare(idOnPath string, idOnBody interface{}) error {
return nil
}
+
+func NameExistCheck(ctx context.Context, stg store.Interface, resource, name string, excludeID interface{}) (interface{}, error) {
+ ret, err := stg.List(ctx, store.ListInput{
+ Predicate: func(obj interface{}) bool {
+ var objName string
+ var objID interface{}
+ switch resource {
+ case "route":
+ objID = obj.(*entity.Route).ID
+ objName = obj.(*entity.Route).Name
+ case "service":
+ objID = obj.(*entity.Service).ID
+ objName = obj.(*entity.Service).Name
+ case "upstream":
+ objID = obj.(*entity.Upstream).ID
+ objName = obj.(*entity.Upstream).Name
+ }
Review comment:
fixed. @tokers
----------------------------------------------------------------
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 merged pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen merged pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606
----------------------------------------------------------------
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] Jaycean commented on a change in pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
Jaycean commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r596059021
##########
File path: api/test/e2enew/route/route_test.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 route
+
+import (
+ "net/http"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/extensions/table"
+
+ "github.com/apisix/manager-api/test/e2enew/base"
+)
+
+var _ = ginkgo.Describe("Route", func() {
+ table.DescribeTable("test route create and update",
+ func(tc base.HttpTestCase) {
+ base.RunTestCase(tc)
+ },
+ table.Entry("create route1 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route2 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name exists`,
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("update route2 failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name exists`,
+ }),
+ table.Entry("update route2 success, name not change", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("hit route1", base.HttpTestCase{
+ Object: base.APISIXExpect(),
+ Method: http.MethodGet,
+ Path: "/hello_",
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world",
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("hit route2", base.HttpTestCase{
+ Object: base.APISIXExpect(),
+ Method: http.MethodGet,
+ Path: "/hello_",
Review comment:
The path of route2 is updated to "/hello". This test case cannot be tested "hit route2".
##########
File path: api/test/e2enew/route/route_test.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 route
+
+import (
+ "net/http"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/extensions/table"
+
+ "github.com/apisix/manager-api/test/e2enew/base"
+)
+
+var _ = ginkgo.Describe("Route", func() {
+ table.DescribeTable("test route create and update",
+ func(tc base.HttpTestCase) {
+ base.RunTestCase(tc)
+ },
+ table.Entry("create route1 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route2 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name exists`,
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("update route2 failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name exists`,
+ }),
+ table.Entry("update route2 success, name not change", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("hit route1", base.HttpTestCase{
+ Object: base.APISIXExpect(),
+ Method: http.MethodGet,
+ Path: "/hello_",
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world",
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("hit route2", base.HttpTestCase{
+ Object: base.APISIXExpect(),
+ Method: http.MethodGet,
+ Path: "/hello_",
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world",
+ }),
+ table.Entry("delete route1", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodDelete,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("delete route2", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodDelete,
+ Path: "/apisix/admin/routes/r2",
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
Review comment:
IMHO, we should add test cases that verify the data is deleted after deleting route data.
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (3ed1a67) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/01c952803d256c53ac411ca52d1c90a2263879fc?el=desc) (01c9528) will **decrease** coverage by `20.80%`.
> The diff coverage is `26.08%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
===========================================
- Coverage 71.46% 50.65% -20.81%
===========================================
Files 132 37 -95
Lines 5375 2612 -2763
Branches 592 0 -592
===========================================
- Hits 3841 1323 -2518
+ Misses 1290 1090 -200
+ Partials 244 199 -45
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `?` | |
| backend-e2e-test-ginkgo | `?` | |
| backend-unit-test | `50.65% <26.08%> (-0.90%)` | :arrow_down: |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `55.81% <26.08%> (-22.43%)` | :arrow_down: |
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `58.40% <26.08%> (-33.09%)` | :arrow_down: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `57.93% <26.08%> (-30.85%)` | :arrow_down: |
| [api/internal/utils/version.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3ZlcnNpb24uZ28=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (-74.77%)` | :arrow_down: |
| [api/internal/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
| [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `0.00% <0.00%> (-55.47%)` | :arrow_down: |
| [api/internal/utils/consts/api\_error.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2NvbnN0cy9hcGlfZXJyb3IuZ28=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
| ... and [121 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [01c9528...3ed1a67](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r596024678
##########
File path: api/test/e2enew/route/route_test.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 route
+
+import (
+ "net/http"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/extensions/table"
+
+ "github.com/apisix/manager-api/test/e2enew/base"
+)
+
+var _ = ginkgo.Describe("Route", func() {
+ table.DescribeTable("test route create and update",
+ func(tc base.HttpTestCase) {
+ base.RunTestCase(tc)
+ },
+ table.Entry("create route1 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route2 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name is existed`,
Review comment:
fixed. @moonming
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r596024317
##########
File path: api/test/e2enew/route/route_test.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 route
+
+import (
+ "net/http"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/extensions/table"
+
+ "github.com/apisix/manager-api/test/e2enew/base"
+)
+
+var _ = ginkgo.Describe("Route", func() {
+ table.DescribeTable("test route create and update",
+ func(tc base.HttpTestCase) {
+ base.RunTestCase(tc)
+ },
+ table.Entry("create route1 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route2 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name is existed`,
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("update route2 failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route1",
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] codecov-io edited a comment on pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (c934178) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/01c952803d256c53ac411ca52d1c90a2263879fc?el=desc) (01c9528) will **decrease** coverage by `5.43%`.
> The diff coverage is `37.68%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
==========================================
- Coverage 71.46% 66.02% -5.44%
==========================================
Files 132 46 -86
Lines 5375 3076 -2299
Branches 592 0 -592
==========================================
- Hits 3841 2031 -1810
+ Misses 1290 782 -508
- Partials 244 263 +19
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `48.30% <33.33%> (-12.94%)` | :arrow_down: |
| backend-e2e-test-ginkgo | `?` | |
| backend-unit-test | `50.61% <26.08%> (-0.94%)` | :arrow_down: |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `77.87% <26.08%> (-13.62%)` | :arrow_down: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `69.37% <43.47%> (-8.87%)` | :arrow_down: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `78.57% <43.47%> (-10.22%)` | :arrow_down: |
| [...l/handler/route\_online\_debug/route\_online\_debug.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGVfb25saW5lX2RlYnVnL3JvdXRlX29ubGluZV9kZWJ1Zy5nbw==) | `5.88% <0.00%> (-68.63%)` | :arrow_down: |
| [api/internal/handler/healthz/healthz.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvaGVhbHRoei9oZWFsdGh6Lmdv) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `81.25% <0.00%> (-18.75%)` | :arrow_down: |
| [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `42.01% <0.00%> (-13.45%)` | :arrow_down: |
| [api/internal/core/storage/etcd.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmFnZS9ldGNkLmdv) | `41.81% <0.00%> (-9.10%)` | :arrow_down: |
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `67.28% <0.00%> (-7.48%)` | :arrow_down: |
| [api/internal/handler/plugin/plugin.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcGx1Z2luL3BsdWdpbi5nbw==) | `87.87% <0.00%> (-6.07%)` | :arrow_down: |
| ... and [93 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [01c9528...c934178](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r596103756
##########
File path: api/test/e2enew/route/route_test.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 route
+
+import (
+ "net/http"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/extensions/table"
+
+ "github.com/apisix/manager-api/test/e2enew/base"
+)
+
+var _ = ginkgo.Describe("Route", func() {
+ table.DescribeTable("test route create and update",
+ func(tc base.HttpTestCase) {
+ base.RunTestCase(tc)
+ },
+ table.Entry("create route1 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route2 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name exists`,
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("update route2 failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name exists`,
+ }),
+ table.Entry("update route2 success, name not change", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("hit route1", base.HttpTestCase{
+ Object: base.APISIXExpect(),
+ Method: http.MethodGet,
+ Path: "/hello_",
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world",
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("hit route2", base.HttpTestCase{
+ Object: base.APISIXExpect(),
+ Method: http.MethodGet,
+ Path: "/hello_",
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world",
+ }),
+ table.Entry("delete route1", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodDelete,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("delete route2", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodDelete,
+ Path: "/apisix/admin/routes/r2",
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
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] nic-chen commented on a change in pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595901346
##########
File path: api/internal/handler/route/route.go
##########
@@ -489,12 +503,32 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Update(c.Context(), &input.Route, true)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
@tokers @starsz please review again when you have time.
----------------------------------------------------------------
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] liuxiran commented on pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
liuxiran commented on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-801526692
> > Hi @nic-chen , I found that currently, fe called `/apisix/admin/notexist/routes` to check unique name of route, we can remove calling this api after this pr merged, right?
>
> I think we could keep it. In this way, the user can know whether the name is available on the first step of creating a route.
> > Hi @nic-chen , I found that currently, fe called `/apisix/admin/notexist/routes` to check unique name of route, we can remove calling this api after this pr merged, right?
>
> I think we could keep it. In this way, the user can know whether the name is available on the first step of creating a route.
ok
----------------------------------------------------------------
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] liuxiran commented on pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
liuxiran commented on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-801463611
Hi @nic-chen , I found that currently, fe called `/apisix/admin/notexist/routes` to check unique name of route, we can remove calling this api after this pr merged, right?
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595895661
##########
File path: api/internal/handler/route/route.go
##########
@@ -379,12 +380,25 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Create(c.Context(), input)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
We need to traverse the list to check if it exists.
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (8bccd65) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/01c952803d256c53ac411ca52d1c90a2263879fc?el=desc) (01c9528) will **decrease** coverage by `20.80%`.
> The diff coverage is `26.08%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
===========================================
- Coverage 71.46% 50.65% -20.81%
===========================================
Files 132 37 -95
Lines 5375 2612 -2763
Branches 592 0 -592
===========================================
- Hits 3841 1323 -2518
+ Misses 1290 1090 -200
+ Partials 244 199 -45
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `?` | |
| backend-e2e-test-ginkgo | `?` | |
| backend-unit-test | `50.65% <26.08%> (-0.90%)` | :arrow_down: |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `55.81% <26.08%> (-22.43%)` | :arrow_down: |
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `58.40% <26.08%> (-33.09%)` | :arrow_down: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `57.93% <26.08%> (-30.85%)` | :arrow_down: |
| [api/internal/utils/version.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3ZlcnNpb24uZ28=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (-74.77%)` | :arrow_down: |
| [api/internal/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
| [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `0.00% <0.00%> (-55.47%)` | :arrow_down: |
| [api/internal/utils/consts/api\_error.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2NvbnN0cy9hcGlfZXJyb3IuZ28=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
| ... and [121 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [01c9528...8bccd65](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (63a798f) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/4b8774fe10c809e333253a6b8c34a00c1ac71c90?el=desc) (4b8774f) will **decrease** coverage by `4.50%`.
> The diff coverage is `96.49%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
==========================================
- Coverage 71.37% 66.87% -4.51%
==========================================
Files 133 47 -86
Lines 5384 3073 -2311
Branches 592 0 -592
==========================================
- Hits 3843 2055 -1788
+ Misses 1297 775 -522
+ Partials 244 243 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `?` | |
| backend-e2e-test-ginkgo | `47.08% <92.98%> (+0.45%)` | :arrow_up: |
| backend-unit-test | `51.26% <43.85%> (-0.36%)` | :arrow_down: |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/handler.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvaGFuZGxlci5nbw==) | `77.77% <92.59%> (+14.81%)` | :arrow_up: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `78.77% <100.00%> (+0.53%)` | :arrow_up: |
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `92.00% <100.00%> (+0.51%)` | :arrow_up: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `89.38% <100.00%> (+0.59%)` | :arrow_up: |
| [api/internal/handler/data\_loader/route\_import.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvZGF0YV9sb2FkZXIvcm91dGVfaW1wb3J0Lmdv) | `35.08% <0.00%> (-29.84%)` | :arrow_down: |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
| [api/internal/handler/global\_rule/global\_rule.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvZ2xvYmFsX3J1bGUvZ2xvYmFsX3J1bGUuZ28=) | `66.12% <0.00%> (-17.75%)` | :arrow_down: |
| [api/internal/utils/utils.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3V0aWxzLmdv) | `54.11% <0.00%> (-11.77%)` | :arrow_down: |
| [api/internal/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3JvdXRlLmdv) | `77.77% <0.00%> (-8.34%)` | :arrow_down: |
| [api/cmd/managerapi.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2NtZC9tYW5hZ2VyYXBpLmdv) | `54.05% <0.00%> (-5.41%)` | :arrow_down: |
| ... and [93 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [4b8774f...63a798f](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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] tokers commented on a change in pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595954707
##########
File path: api/internal/handler/handler.go
##########
@@ -104,3 +107,41 @@ func IDCompare(idOnPath string, idOnBody interface{}) error {
return nil
}
+
+func NameExistCheck(ctx context.Context, stg store.Interface, resource, name string, excludeID interface{}) (interface{}, error) {
+ ret, err := stg.List(ctx, store.ListInput{
+ Predicate: func(obj interface{}) bool {
+ var objName string
+ var objID interface{}
+ switch resource {
+ case "route":
+ objID = obj.(*entity.Route).ID
+ objName = obj.(*entity.Route).Name
+ case "service":
+ objID = obj.(*entity.Service).ID
+ objName = obj.(*entity.Service).Name
+ case "upstream":
+ objID = obj.(*entity.Upstream).ID
+ objName = obj.(*entity.Upstream).Name
+ }
Review comment:
I think a default arm can be specified here:
```go
default:
panic("bad resource")
}
```
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595953565
##########
File path: api/test/e2enew/route/route_test.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 route
+
+import (
+ "net/http"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/extensions/table"
+
+ "github.com/apisix/manager-api/test/e2enew/base"
+)
+
+var _ = ginkgo.Describe("Route", func() {
+ table.DescribeTable("test route create and update",
+ func(tc base.HttpTestCase) {
+ base.RunTestCase(tc)
+ },
+ table.Entry("create route1 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route2 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name is existed`,
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("update route2 failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route1",
Review comment:
bad indentation
##########
File path: api/test/e2enew/route/route_test.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 route
+
+import (
+ "net/http"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/extensions/table"
+
+ "github.com/apisix/manager-api/test/e2enew/base"
+)
+
+var _ = ginkgo.Describe("Route", func() {
+ table.DescribeTable("test route create and update",
+ func(tc base.HttpTestCase) {
+ base.RunTestCase(tc)
+ },
+ table.Entry("create route1 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route2 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name is existed`,
Review comment:
I don't think the grammar is right
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r596103654
##########
File path: api/test/e2enew/route/route_test.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 route
+
+import (
+ "net/http"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/extensions/table"
+
+ "github.com/apisix/manager-api/test/e2enew/base"
+)
+
+var _ = ginkgo.Describe("Route", func() {
+ table.DescribeTable("test route create and update",
+ func(tc base.HttpTestCase) {
+ base.RunTestCase(tc)
+ },
+ table.Entry("create route1 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route2 success", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("create route failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name exists`,
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("update route2 failed, name existed", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route1",
+ "uri": "/hello_",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: `route name exists`,
+ }),
+ table.Entry("update route2 success, name not change", base.HttpTestCase{
+ Object: base.ManagerApiExpect(),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r2",
+ Body: `{
+ "name": "route2",
+ "uri": "/hello",
+ "upstream": {
+ "nodes": {
+ "` + base.UpstreamIp + `:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": base.GetToken()},
+ ExpectStatus: http.StatusOK,
+ }),
+ table.Entry("hit route1", base.HttpTestCase{
+ Object: base.APISIXExpect(),
+ Method: http.MethodGet,
+ Path: "/hello_",
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world",
+ Sleep: base.SleepTime,
+ }),
+ table.Entry("hit route2", base.HttpTestCase{
+ Object: base.APISIXExpect(),
+ Method: http.MethodGet,
+ Path: "/hello_",
Review comment:
fixed. thanks @Jaycean
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (3ed1a67) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/01c952803d256c53ac411ca52d1c90a2263879fc?el=desc) (01c9528) will **decrease** coverage by `5.20%`.
> The diff coverage is `37.68%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
==========================================
- Coverage 71.46% 66.25% -5.21%
==========================================
Files 132 46 -86
Lines 5375 3076 -2299
Branches 592 0 -592
==========================================
- Hits 3841 2038 -1803
+ Misses 1290 775 -515
- Partials 244 263 +19
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `48.50% <33.33%> (-12.75%)` | :arrow_down: |
| backend-e2e-test-ginkgo | `?` | |
| backend-unit-test | `50.65% <26.08%> (-0.90%)` | :arrow_down: |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `77.87% <26.08%> (-13.62%)` | :arrow_down: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `69.37% <43.47%> (-8.87%)` | :arrow_down: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `78.57% <43.47%> (-10.22%)` | :arrow_down: |
| [...l/handler/route\_online\_debug/route\_online\_debug.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGVfb25saW5lX2RlYnVnL3JvdXRlX29ubGluZV9kZWJ1Zy5nbw==) | `5.88% <0.00%> (-68.63%)` | :arrow_down: |
| [api/internal/handler/healthz/healthz.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvaGVhbHRoei9oZWFsdGh6Lmdv) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `81.25% <0.00%> (-18.75%)` | :arrow_down: |
| [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `42.01% <0.00%> (-13.45%)` | :arrow_down: |
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `67.28% <0.00%> (-7.48%)` | :arrow_down: |
| [api/internal/handler/plugin/plugin.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcGx1Z2luL3BsdWdpbi5nbw==) | `87.87% <0.00%> (-6.07%)` | :arrow_down: |
| ... and [93 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [01c9528...3ed1a67](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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] tokers commented on a change in pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595953547
##########
File path: api/internal/handler/route/route.go
##########
@@ -379,12 +380,25 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Create(c.Context(), input)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
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] nic-chen commented on a change in pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595895070
##########
File path: api/internal/handler/route/route.go
##########
@@ -489,12 +503,32 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Update(c.Context(), &input.Route, true)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
updated. thanks.
##########
File path: api/internal/handler/route/route.go
##########
@@ -379,12 +380,25 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Create(c.Context(), input)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
+ Predicate: func(obj interface{}) bool {
+ return obj.(*entity.Route).Name == input.Name
+ },
+ })
+ if err != nil {
+ return &data.SpecCodeResponse{StatusCode: http.StatusInternalServerError}, err
+ }
+ if ret.TotalSize > 0 {
+ return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest}, errors.New("route name is existed")
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595876692
##########
File path: api/internal/handler/route/route.go
##########
@@ -489,12 +503,32 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Update(c.Context(), &input.Route, true)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
Good idea.
##########
File path: api/internal/handler/route/route.go
##########
@@ -489,12 +503,32 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Update(c.Context(), &input.Route, true)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
We can use reflection to get the name filed.
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (c934178) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/01c952803d256c53ac411ca52d1c90a2263879fc?el=desc) (01c9528) will **decrease** coverage by `20.84%`.
> The diff coverage is `26.08%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
===========================================
- Coverage 71.46% 50.61% -20.85%
===========================================
Files 132 37 -95
Lines 5375 2612 -2763
Branches 592 0 -592
===========================================
- Hits 3841 1322 -2519
+ Misses 1290 1091 -199
+ Partials 244 199 -45
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `?` | |
| backend-e2e-test-ginkgo | `?` | |
| backend-unit-test | `50.61% <26.08%> (-0.94%)` | :arrow_down: |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `55.81% <26.08%> (-22.43%)` | :arrow_down: |
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `58.40% <26.08%> (-33.09%)` | :arrow_down: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `57.93% <26.08%> (-30.85%)` | :arrow_down: |
| [api/internal/utils/version.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3ZlcnNpb24uZ28=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (-74.77%)` | :arrow_down: |
| [api/internal/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
| [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `0.00% <0.00%> (-55.47%)` | :arrow_down: |
| [api/internal/utils/consts/api\_error.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2NvbnN0cy9hcGlfZXJyb3IuZ28=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
| ... and [121 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [01c9528...c934178](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (3ed1a67) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/01c952803d256c53ac411ca52d1c90a2263879fc?el=desc) (01c9528) will **decrease** coverage by `2.46%`.
> The diff coverage is `37.68%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
==========================================
- Coverage 71.46% 68.99% -2.47%
==========================================
Files 132 132
Lines 5375 5432 +57
Branches 592 592
==========================================
- Hits 3841 3748 -93
- Misses 1290 1421 +131
- Partials 244 263 +19
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `48.50% <33.33%> (-12.75%)` | :arrow_down: |
| backend-e2e-test-ginkgo | `?` | |
| backend-unit-test | `50.65% <26.08%> (-0.90%)` | :arrow_down: |
| frontend-e2e-test | `72.58% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `77.87% <26.08%> (-13.62%)` | :arrow_down: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `69.37% <43.47%> (-8.87%)` | :arrow_down: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `78.57% <43.47%> (-10.22%)` | :arrow_down: |
| [...l/handler/route\_online\_debug/route\_online\_debug.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGVfb25saW5lX2RlYnVnL3JvdXRlX29ubGluZV9kZWJ1Zy5nbw==) | `5.88% <0.00%> (-68.63%)` | :arrow_down: |
| [api/internal/handler/healthz/healthz.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvaGVhbHRoei9oZWFsdGh6Lmdv) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `81.25% <0.00%> (-18.75%)` | :arrow_down: |
| [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `42.01% <0.00%> (-13.45%)` | :arrow_down: |
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `67.28% <0.00%> (-7.48%)` | :arrow_down: |
| [api/internal/handler/plugin/plugin.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcGx1Z2luL3BsdWdpbi5nbw==) | `87.87% <0.00%> (-6.07%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [01c9528...3ed1a67](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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] tokers commented on a change in pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#discussion_r595835029
##########
File path: api/internal/handler/route/route.go
##########
@@ -379,12 +380,25 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Create(c.Context(), input)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
+ Predicate: func(obj interface{}) bool {
+ return obj.(*entity.Route).Name == input.Name
+ },
+ })
+ if err != nil {
+ return &data.SpecCodeResponse{StatusCode: http.StatusInternalServerError}, err
+ }
+ if ret.TotalSize > 0 {
+ return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest}, errors.New("route name is existed")
Review comment:
typo: "route name is existed" => "route name exists".
##########
File path: api/internal/handler/route/route.go
##########
@@ -489,12 +503,32 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Update(c.Context(), &input.Route, true)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
All these logics are similar, can we abstract them to a common interface/method, so that we can reduce the code complexity and redundency.
##########
File path: api/internal/handler/route/route.go
##########
@@ -379,12 +380,25 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
}
}
- ret, err := h.routeStore.Create(c.Context(), input)
+ // check name existed
+ ret, err := h.routeStore.List(c.Context(), store.ListInput{
Review comment:
Can we use interface like `Get` so that the check can be faster.
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (63a798f) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/4b8774fe10c809e333253a6b8c34a00c1ac71c90?el=desc) (4b8774f) will **decrease** coverage by `20.11%`.
> The diff coverage is `43.85%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
===========================================
- Coverage 71.37% 51.26% -20.12%
===========================================
Files 133 38 -95
Lines 5384 2608 -2776
Branches 592 0 -592
===========================================
- Hits 3843 1337 -2506
+ Misses 1297 1083 -214
+ Partials 244 188 -56
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `?` | |
| backend-e2e-test-ginkgo | `?` | |
| backend-unit-test | `51.26% <43.85%> (-0.36%)` | :arrow_down: |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/handler.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvaGFuZGxlci5nbw==) | `42.59% <25.92%> (-20.38%)` | :arrow_down: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `58.77% <60.00%> (-19.47%)` | :arrow_down: |
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `66.00% <60.00%> (-25.49%)` | :arrow_down: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `64.60% <60.00%> (-24.19%)` | :arrow_down: |
| [api/internal/utils/version.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3ZlcnNpb24uZ28=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (-71.03%)` | :arrow_down: |
| [api/internal/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
| [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `0.00% <0.00%> (-55.47%)` | :arrow_down: |
| ... and [123 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [4b8774f...63a798f](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (63a798f) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/4b8774fe10c809e333253a6b8c34a00c1ac71c90?el=desc) (4b8774f) will **decrease** coverage by `0.47%`.
> The diff coverage is `96.49%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
==========================================
- Coverage 71.37% 70.90% -0.48%
==========================================
Files 133 47 -86
Lines 5384 3073 -2311
Branches 592 0 -592
==========================================
- Hits 3843 2179 -1664
+ Misses 1297 651 -646
+ Partials 244 243 -1
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `61.56% <92.98%> (+0.50%)` | :arrow_up: |
| backend-e2e-test-ginkgo | `47.08% <92.98%> (+0.45%)` | :arrow_up: |
| backend-unit-test | `51.26% <43.85%> (-0.36%)` | :arrow_down: |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/handler.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvaGFuZGxlci5nbw==) | `77.77% <92.59%> (+14.81%)` | :arrow_up: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `78.77% <100.00%> (+0.53%)` | :arrow_up: |
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `92.00% <100.00%> (+0.51%)` | :arrow_up: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `89.38% <100.00%> (+0.59%)` | :arrow_up: |
| [...eb/src/pages/PluginTemplate/components/Preview.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-d2ViL3NyYy9wYWdlcy9QbHVnaW5UZW1wbGF0ZS9jb21wb25lbnRzL1ByZXZpZXcudHN4) | | |
| [web/src/components/Plugin/data.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-d2ViL3NyYy9jb21wb25lbnRzL1BsdWdpbi9kYXRhLnRzeA==) | | |
| [web/src/access.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-d2ViL3NyYy9hY2Nlc3MudHM=) | | |
| [web/src/components/PluginOrchestration/service.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-d2ViL3NyYy9jb21wb25lbnRzL1BsdWdpbk9yY2hlc3RyYXRpb24vc2VydmljZS50cw==) | | |
| [web/src/pages/User/Login.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-d2ViL3NyYy9wYWdlcy9Vc2VyL0xvZ2luLnRzeA==) | | |
| [web/src/pages/Route/transform.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-d2ViL3NyYy9wYWdlcy9Sb3V0ZS90cmFuc2Zvcm0udHM=) | | |
| ... and [81 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [4b8774f...63a798f](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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 pull request #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-801516802
> Hi @nic-chen , I found that currently, fe called `/apisix/admin/notexist/routes` to check unique name of route, we can remove calling this api after this pr merged, right?
I think we could keep it. In this way, the user can know whether the name is available on the first step of creating a route.
----------------------------------------------------------------
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 #1606: fix: check name existed when creating or updating a resource
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1606:
URL: https://github.com/apache/apisix-dashboard/pull/1606#issuecomment-800759425
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=h1) Report
> Merging [#1606](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=desc) (fc02009) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/01c952803d256c53ac411ca52d1c90a2263879fc?el=desc) (01c9528) will **decrease** coverage by `24.10%`.
> The diff coverage is `96.36%`.
> :exclamation: Current head fc02009 differs from pull request most recent head 8d201d3. Consider uploading reports for the commit 8d201d3 to get more accurate results
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1606 +/- ##
===========================================
- Coverage 71.46% 47.35% -24.11%
===========================================
Files 132 46 -86
Lines 5375 3062 -2313
Branches 592 0 -592
===========================================
- Hits 3841 1450 -2391
- Misses 1290 1328 +38
- Partials 244 284 +40
```
| Flag | Coverage Δ | |
|---|---|---|
| backend-e2e-test | `?` | |
| backend-e2e-test-ginkgo | `47.35% <96.36%> (+0.58%)` | :arrow_up: |
| backend-unit-test | `?` | |
| frontend-e2e-test | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/handler/handler.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvaGFuZGxlci5nbw==) | `71.15% <92.00%> (+8.19%)` | :arrow_up: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `50.20% <100.00%> (-28.04%)` | :arrow_down: |
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `55.00% <100.00%> (-36.49%)` | :arrow_down: |
| [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `76.10% <100.00%> (-12.68%)` | :arrow_down: |
| [api/internal/handler/label/label.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvbGFiZWwvbGFiZWwuZ28=) | `9.43% <0.00%> (-72.65%)` | :arrow_down: |
| [api/internal/handler/data\_loader/route\_export.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvZGF0YV9sb2FkZXIvcm91dGVfZXhwb3J0Lmdv) | `3.38% <0.00%> (-65.79%)` | :arrow_down: |
| [api/internal/utils/runtime/runtime.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3J1bnRpbWUvcnVudGltZS5nbw==) | `0.00% <0.00%> (-64.29%)` | :arrow_down: |
| [api/internal/handler/global\_rule/global\_rule.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvZ2xvYmFsX3J1bGUvZ2xvYmFsX3J1bGUuZ28=) | `27.41% <0.00%> (-56.46%)` | :arrow_down: |
| [api/internal/core/store/validate\_mock.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGVfbW9jay5nbw==) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
| [api/internal/filter/authentication.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9hdXRoZW50aWNhdGlvbi5nbw==) | `36.11% <0.00%> (-41.67%)` | :arrow_down: |
| ... and [109 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1606/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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/1606?src=pr&el=footer). Last update [01c9528...8d201d3](https://codecov.io/gh/apache/apisix-dashboard/pull/1606?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