Merge pull request #187 in YUNIONIO/onecloud from ~ZHOUYOUSONG/onecloud:bugfix/yousong-minors to release/2.1.0

* commit 'af0603d6db82f884552644b3b78f02c8e6ffae56':
  mcclient: options: accomodate time.Time type
  mcclient: options: allow listing all resources including pending_deleted
  validators: rework error handling
  validators: StringMultiChoicesValidator: add test case

pkg/cloudcommon/validators/errors.go

@@ -1,9 +1,14 @@
 package validators
 
 import (
+	"database/sql"
 	"fmt"
+
+	"yunion.io/x/onecloud/pkg/httperrors"
 )
 
+var returnHttpError = true
+
 type ErrType uintptr
 
 const (
@@ -21,7 +26,7 @@ const (
 var errTypeToString = map[ErrType]string{
 	ERR_SUCCESS:         "No error",
 	ERR_GENERAL:         "General error",
-	ERR_MISSING_KEY:     "Missing_key error",
+	ERR_MISSING_KEY:     "Missing key error",
 	ERR_INVALID_TYPE:    "Invalid type error",
 	ERR_INVALID_CHOICE:  "Invalid choice error",
 	ERR_NOT_IN_RANGE:    "Not in range error",
@@ -85,16 +90,30 @@ func newModelManagerError(modelKeyword string) error {
 }
 
 func newModelNotFoundError(modelKeyword, idOrName string, err error) error {
-	msg := fmt.Sprintf("cannot find %q with id/name %q: %s",
-		modelKeyword, idOrName, err)
+	msg := fmt.Sprintf("cannot find %q with id/name %q",
+		modelKeyword, idOrName)
+	if err != sql.ErrNoRows {
+		msg += ": " + err.Error()
+	}
 	return newError(ERR_MODEL_NOT_FOUND, msg)
 }
 
 func newError(typ ErrType, msg string) error {
-	return &ValidateError{
+	err := &ValidateError{
 		ErrType: typ,
 		Msg:     msg,
 	}
+	if returnHttpError {
+		switch typ {
+		case ERR_SUCCESS:
+			return nil
+		case ERR_GENERAL, ERR_MODEL_MANAGER:
+			return httperrors.NewInternalServerError(msg)
+		default:
+			return httperrors.NewInputParameterError(msg)
+		}
+	}
+	return err
 }
 
 func IsModelNotFoundError(err error) bool {

pkg/cloudcommon/validators/validators_test.go

@@ -47,6 +47,8 @@ type C struct {
 }
 
 func testS(t *testing.T, v IValidator, c *C) {
+	returnHttpError = false
+
 	j, _ := jsonutils.ParseString(c.In)
 	jd := j.(*jsonutils.JSONDict)
 	err := v.Validate(jd)
@@ -143,6 +145,91 @@ func TestStringChoicesValidator(t *testing.T) {
 	}
 }
 
+func TestStringMultiChoicesValidator(t *testing.T) {
+	type MultiChoicesC struct {
+		*C
+		KeepDup bool
+	}
+	choices := NewChoices("choice0", "choice1")
+	cases := []*MultiChoicesC{
+		{
+			C: &C{
+				Name:      "missing non-optional",
+				In:        `{}`,
+				Out:       `{}`,
+				Optional:  false,
+				Err:       ERR_MISSING_KEY,
+				ValueWant: "",
+			},
+		},
+		{
+			C: &C{
+				Name:      "missing optional",
+				In:        `{}`,
+				Out:       `{}`,
+				Optional:  true,
+				ValueWant: "",
+			},
+		},
+		{
+			C: &C{
+				Name:      "missing with default",
+				In:        `{}`,
+				Out:       `{s: "choice0,choice1"}`,
+				Default:   "choice0,choice1",
+				ValueWant: "choice0,choice1",
+			},
+		},
+		{
+			C: &C{
+				Name:      "good choices",
+				In:        `{"s": "choice0,choice1"}`,
+				Out:       `{"s": "choice0,choice1"}`,
+				ValueWant: "choice0,choice1",
+			},
+		},
+		{
+			C: &C{
+				Name:      "keep dup",
+				In:        `{"s": "choice0,choice0,choice1,choice0"}`,
+				Out:       `{"s": "choice0,choice0,choice1,choice0"}`,
+				ValueWant: "choice0,choice0,choice1,choice0",
+			},
+			KeepDup: true,
+		},
+		{
+			C: &C{
+				Name:      "strip dup",
+				In:        `{"s": "choice0,choice0,choice1,choice0"}`,
+				Out:       `{"s": "choice0,choice1"}`,
+				ValueWant: "choice0,choice1",
+			},
+		},
+		{
+			C: &C{
+				Name:      "invalid choice",
+				In:        `{"s": "choice0,choicex"}`,
+				Out:       `{"s": "choice0,choicex"}`,
+				Err:       ERR_INVALID_CHOICE,
+				ValueWant: "",
+			},
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.Name, func(t *testing.T) {
+			v := NewStringMultiChoicesValidator("s", choices).Sep(",").KeepDup(c.KeepDup)
+			if c.Default != nil {
+				s := c.Default.(string)
+				v.Default(s)
+			}
+			if c.Optional {
+				v.Optional(true)
+			}
+			testS(t, v, c.C)
+		})
+	}
+}
+
 func TestBoolValidator(t *testing.T) {
 	cases := []*C{
 		{

pkg/mcclient/options/base.go

@@ -3,6 +3,7 @@ package options
 import (
 	"fmt"
 	"reflect"
+	"time"
 
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/pkg/gotypes"
@@ -88,6 +89,11 @@ func optionsStructRvToParams(rv reflect.Value) (*jsonutils.JSONDict, error) {
 			if ft.Anonymous {
 				continue
 			}
+			if f.Type() == gotypes.TimeType {
+				t := f.Interface().(time.Time)
+				p.Set(name, jsonutils.NewTimeString(t))
+				continue
+			}
 			// TODO
 			msg := fmt.Sprintf("do not know what to do with non-anonymous struct field: %s", ft.Name)
 			panic(msg)
@@ -142,23 +148,24 @@ func ListStructToParams(v interface{}) (*jsonutils.JSONDict, error) {
 }
 
 type BaseListOptions struct {
-	Limit         *int     `default:"20" help:"Page limit"`
-	Offset        *int     `default:"0" help:"Page offset"`
-	OrderBy       []string `help:"Name of the field to be ordered by"`
-	Order         string   `help:"List order" choices:"desc|asc"`
-	Details       *bool    `help:"Show more details" default:"false"`
-	Search        string   `help:"Filter results by a simple keyword search"`
-	Meta          *bool    `help:"Piggyback metadata information"`
-	Filter        []string `help:"Filters"`
-	JointFilter   []string `help:"Filters with joint table col; joint_tbl.related_key(origin_key).filter_col.filter_cond(filters)"`
-	FilterAny     *bool    `help:"If true, match if any of the filters matches; otherwise, match if all of the filters match"`
-	Admin         *bool    `help:"Is an admin call?"`
-	Tenant        string   `help:"Tenant ID or Name"`
-	User          string   `help:"User ID or Name"`
-	System        *bool    `help:"Show system resource"`
-	PendingDelete *bool    `help:"Show pending deleted resource"`
-	Field         []string `help:"Show only specified fields"`
-	ShowEmulated  *bool    `help:"Show all resources including the emulated resources"`
+	Limit            *int     `default:"20" help:"Page limit"`
+	Offset           *int     `default:"0" help:"Page offset"`
+	OrderBy          []string `help:"Name of the field to be ordered by"`
+	Order            string   `help:"List order" choices:"desc|asc"`
+	Details          *bool    `help:"Show more details" default:"false"`
+	Search           string   `help:"Filter results by a simple keyword search"`
+	Meta             *bool    `help:"Piggyback metadata information"`
+	Filter           []string `help:"Filters"`
+	JointFilter      []string `help:"Filters with joint table col; joint_tbl.related_key(origin_key).filter_col.filter_cond(filters)"`
+	FilterAny        *bool    `help:"If true, match if any of the filters matches; otherwise, match if all of the filters match"`
+	Admin            *bool    `help:"Is an admin call?"`
+	Tenant           string   `help:"Tenant ID or Name"`
+	User             string   `help:"User ID or Name"`
+	System           *bool    `help:"Show system resource"`
+	PendingDelete    *bool    `help:"Show only pending deleted resource"`
+	PendingDeleteAll *bool    `help:"Show all resources including pending deleted" json:"-"`
+	Field            []string `help:"Show only specified fields"`
+	ShowEmulated     *bool    `help:"Show all resources including the emulated resources"`
 }
 
 func (opts *BaseListOptions) Params() (*jsonutils.JSONDict, error) {
@@ -169,10 +176,16 @@ func (opts *BaseListOptions) Params() (*jsonutils.JSONDict, error) {
 	if len(opts.Filter) == 0 {
 		params.Remove("filter_any")
 	}
+	if BoolV(opts.PendingDeleteAll) {
+		params.Set("pending_delete", jsonutils.NewString("all"))
+	}
 	if opts.Admin == nil {
-		requiresSystem := len(opts.Tenant) > 0 || BoolV(opts.System) || BoolV(opts.PendingDelete)
+		requiresSystem := len(opts.Tenant) > 0 ||
+			BoolV(opts.System) ||
+			BoolV(opts.PendingDelete) ||
+			BoolV(opts.PendingDeleteAll)
 		if requiresSystem {
-			params.Set("admin", jsonutils.NewBool(true))
+			params.Set("admin", jsonutils.JSONTrue)
 		}
 	}
 	return params, nil