Repository: sacloud/usacloud (go)

Issue:
フラグから--format-fileと--query-fileを除去する
common flags(`pkg/cmd/cflag`)の出力関連オプションから`--format-file`と`--query-file`を除去し
`--format`と`--query`でファイルパスも受け入れ可能にする。

Pull request description:
Remove --format-file and --query-file
closes #634 



Patch:
diff --git a/pkg/cli/context.go b/pkg/cli/context.go
index b14b244a..1d60d419 100644
--- a/pkg/cli/context.go
+++ b/pkg/cli/context.go
@@ -193,17 +193,12 @@ func getOutputWriter(io IO, columnDefs []output.ColumnDef, rawOptions interface{
 	if options.QuietFlagValue() {
 		return output.NewIDOutput(out, err)
 	}
-	if options.FormatFlagValue() != "" || options.FormatFileFlagValue() != "" {
+	if options.FormatFlagValue() != "" {
 		return output.NewFreeOutput(out, err, options)
 	}
 	switch options.OutputTypeFlagValue() {
 	case "json":
-		query := options.QueryFlagValue()
-		if query == "" {
-			bQuery, _ := ioutil.ReadFile(options.QueryFileFlagValue()) // nolint: err was already checked
-			query = string(bQuery)
-		}
-		return output.NewJSONOutput(out, err, query)
+		return output.NewJSONOutput(out, err, options.QueryFlagValue())
 	case "yaml":
 		return output.NewYAMLOutput(out, err)
 	default:
diff --git a/pkg/cli/validators.go b/pkg/cli/validators.go
index 7628aaef..74b06fda 100644
--- a/pkg/cli/validators.go
+++ b/pkg/cli/validators.go
@@ -16,30 +16,12 @@ package cli
 
 import (
 	"fmt"
-	"io/ioutil"
-	"strings"
 
 	"github.com/jmespath/go-jmespath"
 	"github.com/sacloud/usacloud/pkg/output"
-	"github.com/sacloud/usacloud/pkg/schema"
 	"github.com/sacloud/usacloud/pkg/util"
 )
 
-func ValidateInStrValues(fieldName string, object interface{}, allowValues ...string) []error {
-	return schema.ValidateInStrValues(allowValues...)(fieldName, object)
-}
-
-func ValidateRequired(fieldName string, object interface{}) []error {
-	if util.IsEmpty(object) {
-		return []error{fmt.Errorf("%q: is required", fieldName)}
-	}
-	return []error{}
-}
-
-func ValidateSakuraID(fieldName string, object interface{}) []error {
-	return schema.ValidateSakuraID()(fieldName, object)
-}
-
 func ValidateSetProhibited(fieldName string, object interface{}) []error {
 	if !util.IsEmpty(object) {
 		return []error{fmt.Errorf("%q: can't set on current context", fieldName)}
@@ -47,106 +29,17 @@ func ValidateSetProhibited(fieldName string, object interface{}) []error {
 	return []error{}
 }
 
-func ValidateIPv4Address(fieldName string, object interface{}) []error {
-	return schema.ValidateIPv4Address()(fieldName, object)
-}
-
-func ValidateIPv6Address(fieldName string, object interface{}) []error {
-	return schema.ValidateIPv6Address()(fieldName, object)
-}
-
-func ValidateExistsFileOrStdIn(fieldName string, object interface{}) []error {
-	return schema.ValidateMultiOr(schema.ValidateFileExists(), schema.ValidateStdinExists())(fieldName, object)
-}
-
-func ValidateConflicts(fieldName string, object interface{}, values map[string]interface{}) []error {
-	if !util.IsEmpty(object) {
-		for _, v := range values {
-			if !util.IsEmpty(v) {
-				keys := []string{}
-				for k := range values {
-					keys = append(keys, fmt.Sprintf("%q", k))
-				}
-				return []error{fmt.Errorf("%q: is conflict with %s", fieldName, strings.Join(keys, " or "))}
-			}
-		}
-	}
-	return []error{}
-}
-
-func ValidateConflictValues(fieldName string, object interface{}, values map[string]interface{}) []error {
-	if !util.IsEmpty(object) {
-		for _, v := range values {
-			if !util.IsEmpty(v) {
-				keys := []string{}
-				for k := range values {
-					keys = append(keys, fmt.Sprintf("%q", k))
-				}
-				return []error{fmt.Errorf("%q(%#v): is conflict with %s", fieldName, object, strings.Join(keys, " or "))}
-			}
-		}
-	}
-	return []error{}
-}
-
-func ValidateBetween(fieldName string, object interface{}, min int, max int) []error {
-	if object == nil {
-		object = []int64{}
-	}
-
-	isSlice := func(object interface{}) bool {
-		_, ok1 := object.([]int64)
-		_, ok2 := object.([]string)
-
-		return ok1 || ok2
-	}
-
-	if isSlice(object) {
-		sliceLen := 0
-		if s, ok := object.([]int64); ok {
-			sliceLen = len(s)
-		} else {
-			s := object.([]string)
-			sliceLen = len(s)
-		}
-
-		if max <= 0 {
-			if sliceLen < min {
-				return []error{fmt.Errorf("%q: slice length must be %d or more", fieldName, min)}
-			}
-		} else {
-			if !(min <= sliceLen && sliceLen <= max) {
-				return []error{fmt.Errorf("%q: slice length must be beetween %d and %d", fieldName, min, max)}
-			}
-		}
-	}
-
-	return []error{}
-}
-
 func ValidateOutputOption(o output.Option, defaultOutputType string) []error {
 	outputType := o.OutputTypeFlagValue()
 	//columns := o.GetColumn()
 	format := o.FormatFlagValue()
-	formatFile := o.FormatFileFlagValue()
 	quiet := o.QuietFlagValue()
 	query := o.QueryFlagValue()
-	queryFile := o.QueryFileFlagValue()
-
-	// format and format-file
-	if format != "" && formatFile != "" {
-		return []error{fmt.Errorf("%q: can't set with --format-file", "--format")}
-	}
-	// query and query-file
-	if query != "" && queryFile != "" {
-		return []error{fmt.Errorf("%q: can't set with --query-file", "--query")}
-	}
 
 	// format(or format-file) with output-type
 	if outputType != defaultOutputType {
 		cannotWith := map[string]string{
-			"--format":      format,
-			"--format-file": formatFile,
+			"--format": format,
 		}
 		for k, v := range cannotWith {
 			if v != "" {
@@ -155,19 +48,10 @@ func ValidateOutputOption(o output.Option, defaultOutputType string) []error {
 		}
 	}
 
-	// format-file is exists?
-	if formatFile != "" {
-		errs := schema.ValidateFileExists()("--format-file", formatFile)
-		if len(errs) > 0 {
-			return errs
-		}
-	}
-
 	// with quiet
 	if quiet {
 		cannotWith := map[string]string{
-			"--format":      format,
-			"--format-file": formatFile,
+			"--format": format,
 		}
 		for k, v := range cannotWith {
 			if v != "" {
@@ -181,20 +65,10 @@ func ValidateOutputOption(o output.Option, defaultOutputType string) []error {
 		}
 	}
 
-	// query-file is exists?
-	if queryFile != "" {
-		errs := schema.ValidateFileExists()("--query-file", formatFile)
-		if len(errs) > 0 {
-			return errs
-		}
-	}
 	// query only allow when outputType is json
 	if outputType != "json" && len(query) > 0 {
 		return []error{fmt.Errorf("%q: can't set when --output-type is not json", "--query")}
 	}
-	if outputType != "json" && len(queryFile) > 0 {
-		return []error{fmt.Errorf("%q: can't set when --output-type is not json", "--query-file")}
-	}
 
 	if outputType == "json" && len(query) > 0 {
 		_, err := jmespath.Compile(query)
@@ -203,16 +77,5 @@ func ValidateOutputOption(o output.Option, defaultOutputType string) []error {
 		}
 	}
 
-	if outputType == "json" && len(queryFile) > 0 {
-		bQuery, err := ioutil.ReadFile(queryFile)
-		if err != nil {
-			return []error{fmt.Errorf("%q: can't open query file: %s", "--query-file", err)}
-		}
-		_, err = jmespath.Compile(string(bQuery))
-		if err != nil {
-			return []error{fmt.Errorf("%q: invalid JMESPath: %s", "--query-file", err)}
-		}
-	}
-
 	return []error{}
 }
diff --git a/pkg/cmd/cflag/output_parameter.go b/pkg/cmd/cflag/output_parameter.go
index 7fccab32..71820a36 100644
--- a/pkg/cmd/cflag/output_parameter.go
+++ b/pkg/cmd/cflag/output_parameter.go
@@ -35,14 +35,6 @@ func (p *OutputParameter) FormatFlagValue() string {
 	return p.Format
 }
 
-func (p *OutputParameter) FormatFileFlagValue() string {
-	return p.FormatFile
-}
-
 func (p *OutputParameter) QueryFlagValue() string {
 	return p.Query
 }
-
-func (p *OutputParameter) QueryFileFlagValue() string {
-	return p.QueryFile
-}
diff --git a/pkg/output/free_output.go b/pkg/output/free_output.go
index dbd9b24a..f0496889 100644
--- a/pkg/output/free_output.go
+++ b/pkg/output/free_output.go
@@ -18,7 +18,6 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 
 	"github.com/fatih/structs"
@@ -26,18 +25,16 @@ import (
 )
 
 type freeOutput struct {
-	Out        io.Writer
-	Err        io.Writer
-	Format     string
-	FormatFile string
+	Out    io.Writer
+	Err    io.Writer
+	Format string
 }
 
 func NewFreeOutput(out io.Writer, err io.Writer, option Option) Output {
 	return &freeOutput{
-		Out:        out,
-		Err:        err,
-		Format:     option.FormatFlagValue(),
-		FormatFile: option.FormatFileFlagValue(),
+		Out:    out,
+		Err:    err,
+		Format: option.FormatFlagValue(),
 	}
 }
 
@@ -54,17 +51,14 @@ func (o *freeOutput) Print(contents Contents) error {
 		return nil
 	}
 
-	if o.FormatFile != "" {
-		format, err := ioutil.ReadFile(o.FormatFile)
-		if err != nil {
-			return fmt.Errorf("FreeOutput:Print: read format-file is failed: %s", err)
-		}
-		o.Format = string(format)
+	format, err := util.StringFromPathOrContent(o.Format)
+	if err != nil {
+		return fmt.Errorf("FreeOutput:Print: load format from %q failed failed: %s", o.Format, err)
 	}
 
-	t, err := newTemplate().Parse(o.Format)
+	t, err := newTemplate().Parse(format)
 	if err != nil {
-		return fmt.Errorf("invalid output format %q: %s", o.Format, err)
+		return fmt.Errorf("invalid output format %q: %s", format, err)
 	}
 
 	for i, v := range targets {
diff --git a/pkg/output/json_output.go b/pkg/output/json_output.go
index 5480611a..e49e296e 100644
--- a/pkg/output/json_output.go
+++ b/pkg/output/json_output.go
@@ -53,7 +53,12 @@ func (o *jsonOutput) Print(contents Contents) error {
 	}
 
 	if o.query != "" {
-		v, err := o.searchByJMESPath(targets)
+		query, err := util.StringFromPathOrContent(o.query)
+		if err != nil {
+			return fmt.Errorf("JSONOutput:Query: loading query from %q Failed: %s", o.query, err)
+		}
+
+		v, err := o.searchByJMESPath(targets, query)
 		if err != nil {
 			return fmt.Errorf("JSONOutput:Query: jmespath.Search is Failed: %s", err)
 		}
@@ -100,7 +105,7 @@ func (o *jsonOutput) Print(contents Contents) error {
 	return nil
 }
 
-func (o *jsonOutput) searchByJMESPath(v interface{}) (result interface{}, err error) {
+func (o *jsonOutput) searchByJMESPath(v interface{}, query string) (result interface{}, err error) {
 	defer func() {
 		ret := recover()
 		if ret != nil {
@@ -108,6 +113,6 @@ func (o *jsonOutput) searchByJMESPath(v interface{}) (result interface{}, err er
 			err = fmt.Errorf("jmespath.Search failed: %s", ret)
 		}
 	}()
-	result, err = jmespath.Search(o.query, v)
+	result, err = jmespath.Search(query, v)
 	return
 }
diff --git a/pkg/output/output.go b/pkg/output/output.go
index 41e7d28b..6f874aa3 100644
--- a/pkg/output/output.go
+++ b/pkg/output/output.go
@@ -39,8 +39,6 @@ func (f *DefaultFormatter) ColumnDefs() []ColumnDef {
 type Option interface {
 	OutputTypeFlagValue() string
 	FormatFlagValue() string
-	FormatFileFlagValue() string
 	QuietFlagValue() bool
 	QueryFlagValue() string
-	QueryFileFlagValue() string
 }


Test patch:
diff --git a/pkg/cli/validators_test.go b/pkg/cli/validators_test.go
index 3c62b0fa..8dbfb96e 100644
--- a/pkg/cli/validators_test.go
+++ b/pkg/cli/validators_test.go
@@ -26,19 +26,15 @@ import (
 type dummyOption struct {
 	outputType        string
 	format            string
-	formatFile        string
 	quiet             bool
 	query             string
-	queryFile         string
 	defaultOutputType string
 }
 
 func (o *dummyOption) OutputTypeFlagValue() string { return o.outputType }
 func (o *dummyOption) FormatFlagValue() string     { return o.format }
-func (o *dummyOption) FormatFileFlagValue() string { return o.formatFile }
 func (o *dummyOption) QuietFlagValue() bool        { return o.quiet }
 func (o dummyOption) QueryFlagValue() string       { return o.query }
-func (o dummyOption) QueryFileFlagValue() string   { return o.queryFile }
 
 func TestValidateOutputOption(t *testing.T) {
 	expects := []struct {
@@ -60,24 +56,6 @@ func TestValidateOutputOption(t *testing.T) {
 			},
 			expect: false,
 		},
-		{
-			testName: "Should get error when OutputType is json and have format-file",
-			option: &dummyOption{
-				defaultOutputType: "table",
-				outputType:        "json",
-				formatFile:        "/etc/hosts",
-			},
-			expect: false,
-		},
-		// format and format-file
-		{
-			testName: "Should get error with format and format-file",
-			option: &dummyOption{
-				format:     "a",
-				formatFile: "b",
-			},
-			expect: false,
-		},
 		{
 			testName: "Should get no error when have format only",
 			option: &dummyOption{
@@ -86,14 +64,6 @@ func TestValidateOutputOption(t *testing.T) {
 			},
 			expect: true,
 		},
-		{
-			testName: "Should get no error when have format-file only",
-			option: &dummyOption{
-				outputType: "table",
-				formatFile: "/etc/hosts",
-			},
-			expect: true,
-		},
 		// quiet with format/format-file
 		{
 			testName: "Should get error with format and quiet",
@@ -103,31 +73,6 @@ func TestValidateOutputOption(t *testing.T) {
 			},
 			expect: false,
 		},
-		{
-			testName: "Should get error with format-file and quiet",
-			option: &dummyOption{
-				formatFile: "/etc/hosts",
-				quiet:      true,
-			},
-			expect: false,
-		},
-		// format-file
-		{
-			testName: "Should get error when format-file is not exists",
-			option: &dummyOption{
-				outputType: "table",
-				formatFile: "dummy-not-exists-format-file",
-			},
-			expect: false,
-		},
-		{
-			testName: "Should get no error when format-file is exists",
-			option: &dummyOption{
-				outputType: "table",
-				formatFile: "/etc/hosts",
-			},
-			expect: true,
-		},
 		{
 			testName: "Should get no error when have query and output-type is json",
 			option: &dummyOption{