refactor Input Data GetTimeStamp functionality into a method (#1)

* refactor Input Data GetTimeStamp functionality into a method

* Comment time format tests

handler.go

@@ -1661,22 +1661,35 @@ func (h *Handler) InputJSONDataParser(req map[string]interface{}, index *Index,
 	if err != nil {
 		return nil, err
 	}
-	// if field in input data is not in defined definition, return error
-	var columnLabel string
+	// If field in input data is not in defined definition, return error.
+	var colValue uint64
 	validFields := make(map[string]bool)
-	timestampFrame := make(map[string]string)
+	timestampFrame := make(map[string]int64)
 	for _, field := range inputDef.Fields() {
 		validFields[field.Name] = true
 		if field.PrimaryKey {
-			columnLabel = field.Name
+			columnLabel := field.Name
+			value, ok := req[columnLabel]
+			if !ok {
+				return nil, fmt.Errorf("columnLabel required")
+			}
+			rawValue, ok := value.(float64) // The default JSON marshalling will interpret this as a float
+			if !ok {
+				return nil, fmt.Errorf("float64 require, got value:%s, type: %s", value, reflect.TypeOf(value))
+			}
+			colValue = uint64(rawValue)
 		}
-		// finding frame that need to add timestamp
+		// Find frame that need to add timestamp.
 		for _, action := range field.Actions {
 			if action.ValueDestination == InputSetTimestamp {
-				timestampFrame[action.Frame] = field.Name
+				timestampFrame[action.Frame], err = GetTimeStamp(req, field.Name)
+				if err != nil {
+					return nil, err
+				}
 			}
 		}
 	}
+
 	for key := range req {
 		_, ok := validFields[key]
 		if !ok {
@@ -1691,35 +1704,12 @@ func (h *Handler) InputJSONDataParser(req map[string]interface{}, index *Index,
 		if _, ok := req[field.Name]; !ok {
 			continue
 		}
-		value, ok := req[columnLabel]
-		if !ok {
-			return nil, fmt.Errorf("columnLabel required")
-		}
-		colValue, ok := value.(float64)
-		if !ok {
-			return nil, fmt.Errorf("float64 require, got value:%s, type: %s", value, reflect.TypeOf(value))
-		}
 
 		// Looking into timestampFrame map and set timestamp to the whole frame
-		var timestamp string
 		for _, action := range field.Actions {
 			frame := action.Frame
-			timeField, ok := timestampFrame[action.Frame]
-			if !ok {
-				timestamp = ""
-			} else {
-				tmstamp, ok := req[timeField]
-				if !ok {
-					timestamp = ""
-				} else {
-					timestamp, ok = tmstamp.(string)
-					if !ok {
-						return nil, fmt.Errorf("set-timestamp value must be in time format: YYYY-MM-DD, having: %v", req[timeField])
-					}
-				}
-			}
-
-			bit, err := HandleAction(action, req[field.Name], uint64(colValue), timestamp)
+			timestamp := timestampFrame[action.Frame]
+			bit, err := HandleAction(action, req[field.Name], colValue, timestamp)
 			if err != nil {
 				return nil, fmt.Errorf("error handling action: %s, err: %s", action.ValueDestination, err)
 			}
@@ -1730,3 +1720,23 @@ func (h *Handler) InputJSONDataParser(req map[string]interface{}, index *Index,
 	}
 	return setBits, nil
 }
+
+// GetTimeStamp retrieves unix timestamp from Input data.
+func GetTimeStamp(data map[string]interface{}, timeField string) (int64, error) {
+	tmstamp, ok := data[timeField]
+	if !ok {
+		return 0, nil
+	}
+
+	timestamp, ok := tmstamp.(string)
+	if !ok {
+		return 0, fmt.Errorf("set-timestamp value must be in time format: YYYY-MM-DD, has: %v", data[timeField])
+	}
+
+	v, err := time.Parse(TimeFormat, timestamp)
+	if err != nil {
+		return 0, err
+	}
+
+	return v.Unix(), nil
+}

handler_test.go

@@ -1422,7 +1422,7 @@ func TestInput_JSON(t *testing.T) {
 				"distanceMiles": 8,
 				"time_value": 12345
 				}]`,
-			err: "set-timestamp value must be in time format: YYYY-MM-DD, having: 12345"},
+			err: "set-timestamp value must be in time format: YYYY-MM-DD, has: 12345"},
 	}
 	h := test.NewHandler()
 	h.Holder = hldr.Holder
@@ -1446,3 +1446,33 @@ func EncodeInputDef(name string, body []byte) (*internal.InputDefinition, error)
 	def.Name = name
 	return def, nil
 }
+
+func TestHandler_GetTimeStamp(t *testing.T) {
+	data := make(map[string]interface{})
+	timeField := "time"
+	data["time"] = "2017-03-20T19:35"
+	val, err := pilosa.GetTimeStamp(data, timeField)
+	if val != 1490038500 {
+		t.Fatalf("Timestamp is not set correctly for %s", data["time"])
+	}
+
+	// Verify that an integer is not a valid time format.
+	data["int"] = 1490000000
+	val, err = pilosa.GetTimeStamp(data, "int")
+	if !strings.Contains(err.Error(), "set-timestamp value must be in time format") {
+		t.Fatalf("Expected set-timestamp value must be in time format error, actual error: %s", err)
+	}
+
+	// Verify reversing month and year is not valid time format.
+	data["time"] = "03-2017-20T19:35"
+	val, err = pilosa.GetTimeStamp(data, timeField)
+	if !strings.Contains(err.Error(), "cannot parse") {
+		t.Fatalf("Expected Timestamp is not set correctly, actual error: %s", err)
+	}
+
+	// Handle time fields that do not exist.
+	val, err = pilosa.GetTimeStamp(data, "test")
+	if val != 0 {
+		t.Fatalf("Expected Ignore nonexistent fields")
+	}
+}

input_definition.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/gogo/protobuf/proto"
 	"github.com/pilosa/pilosa/internal"
-	"time"
 )
 
 // Action types.
@@ -341,17 +340,11 @@ func (i *InputDefinition) AddFrame(frame InputFrame) error {
 // HandleAction Process the input data with its action and return a bit to be imported later
 // Note: if the Bit should not be set then nil is returned with no error
 // From the JSON marshalling the possible types are: float64, boolean, string
-func HandleAction(a Action, value interface{}, colID uint64, timestamp string) (*Bit, error) {
+func HandleAction(a Action, value interface{}, colID uint64, timestamp int64) (*Bit, error) {
 	var err error
 	var bit Bit
 	bit.ColumnID = colID
-	if timestamp != "" {
-		v, err := time.Parse(TimeFormat, timestamp)
-		if err != nil {
-			return nil, err
-		}
-		bit.Timestamp = v.Unix()
-	}
+	bit.Timestamp = timestamp
 
 	switch a.ValueDestination {
 	case InputMapping:

input_definition_test.go

@@ -23,7 +23,6 @@ import (
 	"github.com/pilosa/pilosa"
 	"github.com/pilosa/pilosa/internal"
 	"github.com/pilosa/pilosa/test"
-	"time"
 )
 
 func TestInputDefinition_Open(t *testing.T) {
@@ -197,7 +196,7 @@ func TestHandleAction(t *testing.T) {
 	colID := uint64(0)
 	rowID := uint64(100)
 	action := pilosa.Action{ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
-	timestamp := ""
+	timestamp := int64(0)
 
 	value = 1
 	b, err := pilosa.HandleAction(action, value, colID, timestamp)
@@ -270,21 +269,9 @@ func TestHandleAction(t *testing.T) {
 		t.Fatalf("Expected Ignore values that are not type string")
 	}
 
-	action.ValueDestination = pilosa.InputSetTimestamp
-	timestamp = "2017-03-20T19:35"
-	parsedTime, _ := time.Parse(pilosa.TimeFormat, timestamp)
-	b, err = pilosa.HandleAction(action, value, colID, timestamp)
-	if b == nil {
-		t.Fatalf("Expected return bit")
-	} else if b.Timestamp != parsedTime.Unix() {
-		t.Fatalf("Timestamp is not set correctly")
-	}
-
 	action.ValueDestination = "test"
-	timestamp = ""
 	b, err = pilosa.HandleAction(action, value, colID, timestamp)
 	if !strings.Contains(err.Error(), "Unrecognized Value Destination") {
 		t.Fatalf("Expected Unrecognized Value Destination error, actual error: %s", err)
 	}
-
 }