You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/12/19 05:07:08 UTC

[GitHub] [apisix-dashboard] tokers commented on a change in pull request #1064: fix: when enable or disable existing SSL, an error occurred

tokers commented on a change in pull request #1064:
URL: https://github.com/apache/apisix-dashboard/pull/1064#discussion_r546192246



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -160,9 +164,9 @@ func (h *Handler) List(c droplet.Context) (interface{}, error) {
 
 	//format respond
 	var list []interface{}
-	var ssl *entity.SSL
 	for _, item := range ret.Rows {
-		ssl = item.(*entity.SSL)
+		ssl := &entity.SSL{}

Review comment:
       Ditto.

##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -86,7 +86,11 @@ func (h *Handler) Get(c droplet.Context) (interface{}, error) {
 	}
 
 	//format respond
-	ssl := ret.(*entity.SSL)
+	ssl := &entity.SSL{}

Review comment:
       Do not use literal struct initialization.
   
   https://github.com/uber-go/guide/blob/master/style.md#toc40

##########
File path: api/internal/utils/consts/api_error.go
##########
@@ -27,7 +29,21 @@ func ErrorWrapper(handle WrapperHandle) gin.HandlerFunc {
 	return func(c *gin.Context) {
 		data, err := handle(c)
 		if err != nil {
-			apiError := err.(*ApiError)
+			apiError, ok := err.(*ApiError)
+			if !ok {
+				errMsg := err.Error()
+				if strings.Contains(errMsg, "required") ||

Review comment:
       Why not encapsulating these logics into a separate method in `ApiError`, like `ApiError.IsInvalidParam`?

##########
File path: api/internal/utils/utils.go
##########
@@ -97,6 +98,16 @@ func InterfaceToString(val interface{}) string {
 	return str
 }
 
+func ObjectClone(origin, copy interface{}) error {

Review comment:
       We'd better to add some comments for this function, such as the object should not have float number, the `json.Marshal` and `json.Unmarshal` will cause the precision loss.




----------------------------------------------------------------
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