[GitHub] [apisix-dashboard] Jaycean commented on a change in pull request #1673: fix: unable to export route with nil methods field

2021-03-27 Thread GitBox


Jaycean commented on a change in pull request #1673:
URL: https://github.com/apache/apisix-dashboard/pull/1673#discussion_r602740981



##
File path: api/internal/handler/data_loader/route_export.go
##
@@ -245,8 +246,14 @@ func (h *Handler) RouteToOpenAPI3(c droplet.Context, 
routes []*entity.Route) (*o
path.RequestBody = &openapi3.RequestBodyRef{Value: requestBody}
path.Responses = openapi3.NewResponses()
 
-   for i := range route.Methods {
-   switch strings.ToUpper(route.Methods[i]) {
+   if route.Methods != nil && len(route.Methods) > 0 {
+   routeMethods = route.Methods
+   } else {
+   routeMethods = []string{http.MethodGet, 
http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, 
http.MethodHead, http.MethodConnect, http.MethodTrace, http.MethodOptions}

Review comment:
   Yeah,Because these methods such as "GET", "POST" and so on are 
immutable, the use here is more uniform, and "Get", "get" and "GET" will not 
appear.




-- 
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:
us...@infra.apache.org




[GitHub] [apisix-dashboard] Jaycean commented on a change in pull request #1673: fix: unable to export route with nil methods field

2021-03-27 Thread GitBox


Jaycean commented on a change in pull request #1673:
URL: https://github.com/apache/apisix-dashboard/pull/1673#discussion_r602740981



##
File path: api/internal/handler/data_loader/route_export.go
##
@@ -245,8 +246,14 @@ func (h *Handler) RouteToOpenAPI3(c droplet.Context, 
routes []*entity.Route) (*o
path.RequestBody = &openapi3.RequestBodyRef{Value: requestBody}
path.Responses = openapi3.NewResponses()
 
-   for i := range route.Methods {
-   switch strings.ToUpper(route.Methods[i]) {
+   if route.Methods != nil && len(route.Methods) > 0 {
+   routeMethods = route.Methods
+   } else {
+   routeMethods = []string{http.MethodGet, 
http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, 
http.MethodHead, http.MethodConnect, http.MethodTrace, http.MethodOptions}

Review comment:
   Yeah,Because these methods such as "GET", "POST" and so on are 
immutable, the use here is more uniform, and "Get", "get" and "GET" will not 
appear.
   They don't affect the output.




-- 
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:
us...@infra.apache.org




[GitHub] [apisix-dashboard] Jaycean commented on a change in pull request #1673: fix: unable to export route with nil methods field

2021-03-27 Thread GitBox


Jaycean commented on a change in pull request #1673:
URL: https://github.com/apache/apisix-dashboard/pull/1673#discussion_r602821325



##
File path: api/test/e2e/route_export_test.go
##
@@ -2505,6 +2503,499 @@ func TestRoute_Export_Equal_URI(t *testing.T) {
 
 }
 
+func TestRoute_Export_Methods_Feild_Empty(t *testing.T) {
+   exportStr := `{
+   "components": {},
+   "info": {
+   "title": "RoutesExport",
+   "version": "3.0.0"
+   },
+   "openapi": "3.0.0",
+   "paths": {
+   "/test-test": {
+   "connect": {
+   "operationId": "route_allCONNECT",
+   "requestBody": {},
+   "responses": {
+   "default": {
+   "description": ""
+   }
+   },
+   "summary": "所有",
+   "x-apisix-enable_websocket": false,
+   "x-apisix-hosts": ["test.com"],
+   "x-apisix-priority": 0,
+   "x-apisix-status": 1,
+   "x-apisix-upstream": {
+   "nodes": {
+   "172.16.238.20:1980": 1
+   },
+   "type": "roundrobin"
+   }
+   },
+   "delete": {
+   "operationId": "route_allDELETE",
+   "requestBody": {},
+   "responses": {
+   "default": {
+   "description": ""
+   }
+   },
+   "summary": "所有",
+   "x-apisix-enable_websocket": false,
+   "x-apisix-hosts": ["test.com"],
+   "x-apisix-priority": 0,
+   "x-apisix-status": 1,
+   "x-apisix-upstream": {
+   "nodes": {
+   "172.16.238.20:1980": 1
+   },
+   "type": "roundrobin"
+   }
+   },
+   "get": {
+   "operationId": "route_allGET",
+   "requestBody": {},
+   "responses": {
+   "default": {
+   "description": ""
+   }
+   },
+   "summary": "所有",
+   "x-apisix-enable_websocket": false,
+   "x-apisix-hosts": ["test.com"],
+   "x-apisix-priority": 0,
+   "x-apisix-status": 1,
+   "x-apisix-upstream": {
+   "nodes": {
+   "172.16.238.20:1980": 1
+   },
+   "type": "roundrobin"
+   }
+   },
+   "head": {
+   "operationId": "route_allHEAD",
+   "requestBody": {},
+   "responses": {
+   "default": {
+   "description": ""
+   }
+   },
+   "summary": "所有",
+   "x-apisix-enable_websocket": false,
+   "x-apisix-hosts": ["test.com"],
+   "x-apisix-priority": 0,
+   "x-apisix-status": 1,
+

[GitHub] [apisix-dashboard] Jaycean commented on a change in pull request #1673: fix: unable to export route with nil methods field

2021-03-27 Thread GitBox


Jaycean commented on a change in pull request #1673:
URL: https://github.com/apache/apisix-dashboard/pull/1673#discussion_r602821433



##
File path: web/cypress/fixtures/export-route-dataset.json
##
@@ -131,7 +129,6 @@
 }
   },
   "x-apisix-enable_websocket": false,
-  "x-apisix-plugins": {},

Review comment:
   The design of import and export is to export when there is data, and not 
export when there is no data. Here, no data is empty. I don't think it's 
necessary to export, so I delete 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:
us...@infra.apache.org




[GitHub] [apisix-dashboard] Jaycean commented on a change in pull request #1673: fix: unable to export route with nil methods field

2021-03-27 Thread GitBox


Jaycean commented on a change in pull request #1673:
URL: https://github.com/apache/apisix-dashboard/pull/1673#discussion_r602822008



##
File path: web/cypress/fixtures/export-route-dataset.json
##
@@ -131,7 +129,6 @@
 }
   },
   "x-apisix-enable_websocket": false,
-  "x-apisix-plugins": {},

Review comment:
   If it is not deleted, Fe ci will report an error.




-- 
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:
us...@infra.apache.org




[GitHub] [apisix-dashboard] Jaycean commented on a change in pull request #1673: fix: unable to export route with nil methods field

2021-03-28 Thread GitBox


Jaycean commented on a change in pull request #1673:
URL: https://github.com/apache/apisix-dashboard/pull/1673#discussion_r602878978



##
File path: web/cypress/fixtures/export-route-dataset.json
##
@@ -131,7 +129,6 @@
 }
   },
   "x-apisix-enable_websocket": false,
-  "x-apisix-plugins": {},

Review comment:
   Yes, most of the other test cases have data in `x-apisix-plugins`
   For example, this test case: 
   
https://github.com/apache/apisix-dashboard/blob/73e96357c64ddc92551854577879b86a4ecee341/api/internal/handler/data_loader/route_export_test.go#L34




-- 
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:
us...@infra.apache.org




[GitHub] [apisix-dashboard] Jaycean commented on a change in pull request #1673: fix: unable to export route with nil methods field

2021-03-28 Thread GitBox


Jaycean commented on a change in pull request #1673:
URL: https://github.com/apache/apisix-dashboard/pull/1673#discussion_r602977216



##
File path: api/internal/handler/data_loader/route_export.go
##
@@ -245,8 +246,14 @@ func (h *Handler) RouteToOpenAPI3(c droplet.Context, 
routes []*entity.Route) (*o
path.RequestBody = &openapi3.RequestBodyRef{Value: requestBody}
path.Responses = openapi3.NewResponses()
 
-   for i := range route.Methods {
-   switch strings.ToUpper(route.Methods[i]) {
+   if route.Methods != nil && len(route.Methods) > 0 {
+   routeMethods = route.Methods
+   } else {
+   routeMethods = []string{http.MethodGet, 
http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, 
http.MethodHead, http.MethodConnect, http.MethodTrace, http.MethodOptions}

Review comment:
   Sorry, I misunderstood.
   Done. Thks




-- 
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:
us...@infra.apache.org




[GitHub] [apisix-dashboard] Jaycean commented on a change in pull request #1673: fix: unable to export route with nil methods field

2021-03-29 Thread GitBox


Jaycean commented on a change in pull request #1673:
URL: https://github.com/apache/apisix-dashboard/pull/1673#discussion_r603712340



##
File path: api/internal/handler/data_loader/route_export.go
##
@@ -233,7 +235,7 @@ func (h *Handler) RouteToOpenAPI3(c droplet.Context, routes 
[]*entity.Route) (*o
return nil, err
}
 
-   if plugins != nil {
+   if len(plugins) > 0 {

Review comment:
   
   It was written in the 
past(commit:https://github.com/apache/apisix-dashboard/pull/1673/commits/6dd85785cd752539b0ec50652e68e1f34ef1cf56),
 but go-link will report error, so it was modified according to go-link.




-- 
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:
us...@infra.apache.org