TOOLS-474: error when JSON type is used with invalid flags

Former-commit-id: b148ebf35842ba09c1f94a773955e533e47b78cb

4 files changed, 121 insertions, 37 deletions

diff --git a/mongoimport/mongoimport.go b/mongoimport/mongoimport.go
index e2f8f2e4155..80ab2578b66 100644
--- a/mongoimport/mongoimport.go
+++ b/mongoimport/mongoimport.go
@@ -114,23 +114,37 @@ func (mongoImport *MongoImport) ValidateSettings(args []string) error {
 	if mongoImport.InputOptions.Type == CSV ||
 		mongoImport.InputOptions.Type == TSV {
 		if !mongoImport.InputOptions.HeaderLine {
-			if mongoImport.InputOptions.Fields == "" &&
-				mongoImport.InputOptions.FieldFile == "" {
-				return fmt.Errorf("You need to specify fields or have a " +
-					"header line to import this file type")
+			if mongoImport.InputOptions.Fields == nil &&
+				mongoImport.InputOptions.FieldFile == nil {
+				return fmt.Errorf("must specify --fields, --fieldFile or --headerline to import this file type")
 			}
-			if mongoImport.InputOptions.Fields != "" &&
-				mongoImport.InputOptions.FieldFile != "" {
+			if mongoImport.InputOptions.FieldFile != nil &&
+				*mongoImport.InputOptions.FieldFile == "" {
+				return fmt.Errorf("--fieldFile can not be empty string")
+			}
+			if mongoImport.InputOptions.Fields != nil &&
+				mongoImport.InputOptions.FieldFile != nil {
 				return fmt.Errorf("incompatible options: --fields and --fieldFile")
 			}
 		} else {
-			if mongoImport.InputOptions.Fields != "" {
+			if mongoImport.InputOptions.Fields != nil {
 				return fmt.Errorf("incompatible options: --fields and --headerline")
 			}
-			if mongoImport.InputOptions.FieldFile != "" {
+			if mongoImport.InputOptions.FieldFile != nil {
 				return fmt.Errorf("incompatible options: --fieldFile and --headerline")
 			}
 		}
+	} else {
+		// input type is JSON
+		if mongoImport.InputOptions.HeaderLine {
+			return fmt.Errorf("can not use --headerline when input type is JSON")
+		}
+		if mongoImport.InputOptions.Fields != nil {
+			return fmt.Errorf("can not use --fields when input type is JSON")
+		}
+		if mongoImport.InputOptions.FieldFile != nil {
+			return fmt.Errorf("can not use --fieldFile when input type is JSON")
+		}
 	}
 
 	// set the number of decoding workers to use for imports
@@ -520,10 +534,10 @@ func (mongoImport *MongoImport) ingester(documents []bson.Raw, collection *mgo.C
 func (mongoImport *MongoImport) getInputReader(in io.Reader) (InputReader, error) {
 	var fields []string
 	var err error
-	if len(mongoImport.InputOptions.Fields) != 0 {
-		fields = strings.Split(strings.Trim(mongoImport.InputOptions.Fields, " "), ",")
-	} else if mongoImport.InputOptions.FieldFile != "" {
-		fields, err = util.GetFieldsFromFile(mongoImport.InputOptions.FieldFile)
+	if mongoImport.InputOptions.Fields != nil {
+		fields = strings.Split(*mongoImport.InputOptions.Fields, ",")
+	} else if mongoImport.InputOptions.FieldFile != nil {
+		fields, err = util.GetFieldsFromFile(*mongoImport.InputOptions.FieldFile)
 		if err != nil {
 			return nil, err
 		}
diff --git a/mongoimport/mongoimport_test.go b/mongoimport/mongoimport_test.go
index 0fc0aa9667e..c4f3dc1117b 100644
--- a/mongoimport/mongoimport_test.go
+++ b/mongoimport/mongoimport_test.go
@@ -117,7 +117,8 @@ func TestMongoImportValidateSettings(t *testing.T) {
 			"but --fields is supplied", func() {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
-			mongoImport.InputOptions.Fields = "a,b,c"
+			fields := "a,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.InputOptions.Type = CSV
 			So(mongoImport.ValidateSettings([]string{}), ShouldBeNil)
 		})
@@ -125,23 +126,74 @@ func TestMongoImportValidateSettings(t *testing.T) {
 		Convey("no error should be thrown if no input type is supplied", func() {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
-			mongoImport.InputOptions.Fields = "a,b,c"
 			So(mongoImport.ValidateSettings([]string{}), ShouldBeNil)
 		})
 
+		Convey("an error should be thrown if --headerline is used with JSON input", func() {
+			mongoImport, err := NewMongoImport()
+			So(err, ShouldBeNil)
+			mongoImport.InputOptions.HeaderLine = true
+			So(mongoImport.ValidateSettings([]string{}), ShouldNotBeNil)
+		})
+
+		Convey("an error should be thrown if --fields is used with JSON input", func() {
+			mongoImport, err := NewMongoImport()
+			So(err, ShouldBeNil)
+			fields := ""
+			mongoImport.InputOptions.Fields = &fields
+			So(mongoImport.ValidateSettings([]string{}), ShouldNotBeNil)
+			fields = "a,b,c"
+			mongoImport.InputOptions.Fields = &fields
+			So(mongoImport.ValidateSettings([]string{}), ShouldNotBeNil)
+		})
+
+		Convey("an error should be thrown if --fieldFile is used with JSON input", func() {
+			mongoImport, err := NewMongoImport()
+			So(err, ShouldBeNil)
+			fieldFile := ""
+			mongoImport.InputOptions.FieldFile = &fieldFile
+			So(mongoImport.ValidateSettings([]string{}), ShouldNotBeNil)
+			fieldFile = "test.csv"
+			mongoImport.InputOptions.FieldFile = &fieldFile
+			So(mongoImport.ValidateSettings([]string{}), ShouldNotBeNil)
+		})
+
 		Convey("no error should be thrown if --headerline is not supplied "+
 			"but --fieldFile is supplied", func() {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
-			mongoImport.InputOptions.FieldFile = "test.csv"
+			fieldFile := "test.csv"
+			mongoImport.InputOptions.FieldFile = &fieldFile
+			mongoImport.InputOptions.Type = CSV
+			So(mongoImport.ValidateSettings([]string{}), ShouldBeNil)
+		})
+
+		Convey("no error should be thrown if --fields is supplied with CSV import", func() {
+			mongoImport, err := NewMongoImport()
+			So(err, ShouldBeNil)
+			fields := "a,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.InputOptions.Type = CSV
 			So(mongoImport.ValidateSettings([]string{}), ShouldBeNil)
+			fields = ""
+			mongoImport.InputOptions.Fields = &fields
+			So(mongoImport.ValidateSettings([]string{}), ShouldBeNil)
+		})
+
+		Convey("an error should be thrown if empty --fieldFile is used with CSV input", func() {
+			mongoImport, err := NewMongoImport()
+			So(err, ShouldBeNil)
+			fieldFile := ""
+			mongoImport.InputOptions.Type = CSV
+			mongoImport.InputOptions.FieldFile = &fieldFile
+			So(mongoImport.ValidateSettings([]string{}), ShouldNotBeNil)
 		})
 
 		Convey("an error should be thrown if no collection and no file is supplied", func() {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
-			mongoImport.InputOptions.FieldFile = "test.csv"
+			fieldFile := "test.csv"
+			mongoImport.InputOptions.FieldFile = &fieldFile
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.ToolOptions.Namespace.Collection = ""
 			So(mongoImport.ValidateSettings([]string{}), ShouldNotBeNil)
@@ -152,7 +204,8 @@ func TestMongoImportValidateSettings(t *testing.T) {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.File = "input"
-			mongoImport.InputOptions.FieldFile = "test.csv"
+			fieldFile := "test.csv"
+			mongoImport.InputOptions.FieldFile = &fieldFile
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.ToolOptions.Namespace.Collection = ""
 			So(mongoImport.ValidateSettings([]string{}), ShouldBeNil)
@@ -165,7 +218,8 @@ func TestMongoImportValidateSettings(t *testing.T) {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.File = "/path/to/input/file/dot/input.txt"
-			mongoImport.InputOptions.FieldFile = "test.csv"
+			fieldFile := "test.csv"
+			mongoImport.InputOptions.FieldFile = &fieldFile
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.ToolOptions.Namespace.Collection = ""
 			So(mongoImport.ValidateSettings([]string{}), ShouldBeNil)
@@ -222,7 +276,8 @@ func TestGetInputReader(t *testing.T) {
 		Convey("no error should be thrown if --fields is used", func() {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
-			mongoImport.InputOptions.Fields = "a,b,c"
+			fields := "a,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.InputOptions.File = "/path/to/input/file/dot/input.txt"
 			_, err = mongoImport.getInputReader(&os.File{})
 			So(err, ShouldBeNil)
@@ -231,7 +286,8 @@ func TestGetInputReader(t *testing.T) {
 			"references a valid file", func() {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
-			mongoImport.InputOptions.FieldFile = "testdata/test_array.json"
+			fieldFile := "testdata/test_array.json"
+			mongoImport.InputOptions.FieldFile = &fieldFile
 			_, err = mongoImport.getInputReader(&os.File{})
 			So(err, ShouldBeNil)
 		})
@@ -239,7 +295,8 @@ func TestGetInputReader(t *testing.T) {
 			"references an invalid file", func() {
 			mongoImport, err := NewMongoImport()
 			So(err, ShouldBeNil)
-			mongoImport.InputOptions.FieldFile = "/path/to/input/file/dot/input.txt"
+			fieldFile := "/path/to/input/file/dot/input.txt"
+			mongoImport.InputOptions.FieldFile = &fieldFile
 			_, err = mongoImport.getInputReader(&os.File{})
 			So(err, ShouldNotBeNil)
 		})
@@ -289,7 +346,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test.csv"
-			mongoImport.InputOptions.Fields = "a,b,c"
+			fields := "a,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.WriteConcern = "majority"
 			numImported, err := mongoImport.ImportDocuments()
 			So(err, ShouldBeNil)
@@ -310,7 +368,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test_blanks.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.IgnoreBlanks = true
 
 			numImported, err := mongoImport.ImportDocuments()
@@ -328,7 +387,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test_blanks.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			numImported, err := mongoImport.ImportDocuments()
 			So(err, ShouldBeNil)
 			So(numImported, ShouldEqual, 3)
@@ -344,7 +404,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.Upsert = true
 			mongoImport.IngestOptions.MaintainInsertionOrder = true
 			numImported, err := mongoImport.ImportDocuments()
@@ -363,7 +424,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.StopOnError = true
 			mongoImport.IngestOptions.MaintainInsertionOrder = true
 			mongoImport.IngestOptions.WriteConcern = "majority"
@@ -383,7 +445,8 @@ func TestImportDocuments(t *testing.T) {
 
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test_duplicate.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.StopOnError = false
 			numImported, err := mongoImport.ImportDocuments()
 			So(err, ShouldBeNil)
@@ -403,7 +466,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.Drop = true
 			mongoImport.IngestOptions.MaintainInsertionOrder = true
 			mongoImport.IngestOptions.WriteConcern = "majority"
@@ -422,7 +486,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.InputOptions.HeaderLine = true
 			numImported, err := mongoImport.ImportDocuments()
 			So(err, ShouldBeNil)
@@ -437,7 +502,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = csvFile.Name()
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.InputOptions.HeaderLine = true
 			numImported, err := mongoImport.ImportDocuments()
 			So(err, ShouldEqual, io.EOF)
@@ -449,7 +515,8 @@ func TestImportDocuments(t *testing.T) {
 
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test.csv"
-			mongoImport.InputOptions.Fields = "_id,c,b"
+			fields := "_id,c,b"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.Upsert = true
 			mongoImport.IngestOptions.UpsertFields = "_id"
 			mongoImport.IngestOptions.MaintainInsertionOrder = true
@@ -469,7 +536,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test_duplicate.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.Upsert = true
 			mongoImport.IngestOptions.UpsertFields = "_id"
 			numImported, err := mongoImport.ImportDocuments()
@@ -490,7 +558,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test_duplicate.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.StopOnError = true
 			mongoImport.IngestOptions.WriteConcern = "1"
 			mongoImport.IngestOptions.MaintainInsertionOrder = true
@@ -508,7 +577,8 @@ func TestImportDocuments(t *testing.T) {
 			So(err, ShouldBeNil)
 			mongoImport.InputOptions.Type = CSV
 			mongoImport.InputOptions.File = "testdata/test_bad.csv"
-			mongoImport.InputOptions.Fields = "_id,b,c"
+			fields := "_id,b,c"
+			mongoImport.InputOptions.Fields = &fields
 			mongoImport.IngestOptions.StopOnError = true
 			mongoImport.IngestOptions.WriteConcern = "1"
 			mongoImport.IngestOptions.MaintainInsertionOrder = true
diff --git a/mongoimport/options/options.go b/mongoimport/options/options.go
index f201b55aced..a1508ff761c 100644
--- a/mongoimport/options/options.go
+++ b/mongoimport/options/options.go
@@ -2,10 +2,10 @@ package options
 
 type InputOptions struct {
 	// Fields is an option to directly specify comma-separated fields to import to CSV
-	Fields string `long:"fields" short:"f" description:"comma separated list of field names e.g. -f name,age"`
+	Fields *string `long:"fields" short:"f" description:"comma separated list of field names e.g. -f name,age"`
 
 	// FieldFile is a filename that refers to a list of fields to import, 1 per line
-	FieldFile string `long:"fieldFile" description:"file with field names - 1 per line"`
+	FieldFile *string `long:"fieldFile" description:"file with field names - 1 per line"`
 
 	// JSONArray if set will import the documents an array of JSON doccuments
 	JSONArray bool `long:"jsonArray" description:"output to a JSON array rather than one object per line"`
diff --git a/mongoimport/tsv.go b/mongoimport/tsv.go
index 202f50f5ff9..4f31f993efc 100644
--- a/mongoimport/tsv.go
+++ b/mongoimport/tsv.go
@@ -72,7 +72,7 @@ func (tsvInputReader *TSVInputReader) ReadHeaderFromSource() ([]string, error) {
 	}
 	tokenizedHeaders := strings.Split(stringHeaders, tokenSeparator)
 	for _, header := range tokenizedHeaders {
-		unsortedHeaders = append(unsortedHeaders, strings.TrimSpace(header))
+		unsortedHeaders = append(unsortedHeaders, strings.TrimRight(header, "\r\n"))
 	}
 	return unsortedHeaders, nil
 }