Merge pull request #124 from quickwit-oss/ddelemeny/improve-parsetime…

…-error Improve ParseTime, handle mismatched format and value
quickwit-oss · Apr 26, 2024 · 7bd9fd5 · 7bd9fd5
2 parents 8afe6fb + 8c6eafb
commit 7bd9fd5
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 38 deletions.
diff --git a/pkg/utils/parse_time.go b/pkg/utils/parse_time.go
@@ -1,7 +1,6 @@
 package utils
 
 import (
-	"errors"
 	"fmt"
 	"reflect"
 	"time"
@@ -26,33 +25,44 @@ const Rfc2822zLayout string = "%a, %d %b %Y %T %z"
 // Parses a value into Time given a timeOutputFormat. The conversion
 // only works with float64 as this is what we get when parsing a response.
 func ParseTime(value any, timeOutputFormat string) (time.Time, error) {
-	switch timeOutputFormat {
-	case Iso8601, Rfc3339:
+	switch value.(type) {
+	case string:
 		value_string := value.(string)
-		timeValue, err := time.Parse(time.RFC3339, value_string)
-		if err != nil {
-			return time.Time{}, err
-		}
-		return timeValue, nil
+		switch timeOutputFormat {
+		case Iso8601, Rfc3339:
+			timeValue, err := time.Parse(time.RFC3339, value_string)
+			if err != nil {
+				return time.Time{}, err
+			}
+			return timeValue, nil
 
-	case Rfc2822:
-		// XXX: the time package's layout for RFC2822 is bogus, don't use that.
-		value_string := value.(string)
-		timeValue, err := timefmt.Parse(value_string, Rfc2822Layout)
-		if err != nil {
-			return time.Time{}, err
-		}
-		return timeValue, nil
-	case Rfc2822z:
-		// XXX: the time package's layout for RFC2822 is bogus, don't use that.
-		value_string := value.(string)
-		timeValue, err := timefmt.Parse(value_string, Rfc2822zLayout)
-		if err != nil {
-			return time.Time{}, err
-		}
-		return timeValue, nil
+		case Rfc2822:
+			// XXX: the time package's layout for RFC2822 is bogus, don't use that.
+			timeValue, err := timefmt.Parse(value_string, Rfc2822Layout)
+			if err != nil {
+				return time.Time{}, err
+			}
+			return timeValue, nil
+
+		case Rfc2822z:
+			// XXX: the time package's layout for RFC2822 is bogus, don't use that.
+			timeValue, err := timefmt.Parse(value_string, Rfc2822zLayout)
+			if err != nil {
+				return time.Time{}, err
+			}
+			return timeValue, nil
+
+		case TimestampSecs, TimestampMillis, TimestampMicros, TimestampNanos:
+			return time.Time{}, fmt.Errorf("ParseTime received incoherent inputs: timeOutputFormat: %s value: %s (%s)", timeOutputFormat, fmt.Sprint(value), reflect.TypeOf(value))
 
-	case TimestampSecs, TimestampMillis, TimestampMicros, TimestampNanos:
+		default:
+			timeValue, err := timefmt.Parse(value_string, timeOutputFormat)
+			if err != nil {
+				return time.Time{}, err
+			}
+			return timeValue, nil
+		}
+	default:
 		var value_i64 int64
 		switch value.(type) {
 		case int, int8, int16, int32, int64:
@@ -61,25 +71,20 @@ func ParseTime(value any, timeOutputFormat string) (time.Time, error) {
 			value_f64 := reflect.ValueOf(value).Float()
 			value_i64 = int64(value_f64)
 		default:
-			return time.Time{}, errors.New("parseTime only accepts float64 or int64 values with timestamp based formats")
+			return time.Time{}, fmt.Errorf("ParseTime does not support values of type (%s)", reflect.TypeOf(value))
 		}
 
-		if timeOutputFormat == TimestampSecs {
+		switch timeOutputFormat {
+		case TimestampSecs:
 			return time.Unix(value_i64, 0), nil
-		} else if timeOutputFormat == TimestampMillis {
+		case TimestampMillis:
 			return time.Unix(0, value_i64*1_000_000), nil
-		} else if timeOutputFormat == TimestampMicros {
+		case TimestampMicros:
 			return time.Unix(0, value_i64*1_000), nil
-		} else if timeOutputFormat == TimestampNanos {
+		case TimestampNanos:
 			return time.Unix(0, value_i64), nil
+		default:
+			return time.Time{}, fmt.Errorf("ParseTime received incoherent inputs: timeOutputFormat: %s value: %s (%s)", timeOutputFormat, fmt.Sprint(value), reflect.TypeOf(value))
 		}
-	default:
-		value_string := value.(string)
-		timeValue, err := timefmt.Parse(value_string, timeOutputFormat)
-		if err != nil {
-			return time.Time{}, err
-		}
-		return timeValue, nil
 	}
-	return time.Time{}, fmt.Errorf("timeOutputFormat not supported yet %s", timeOutputFormat)
 }
diff --git a/pkg/utils/parse_time_test.go b/pkg/utils/parse_time_test.go
@@ -71,4 +71,12 @@ func TestParseTime(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("Error on incoherent format and value", func(t *testing.T) {
+		value := 1711629296987654321
+		format := "%Y-%m-%d %H:%M:%S.%f"
+		_, err := ParseTime(value, format)
+
+		assert.ErrorContains(err, fmt.Sprintf("ParseTime received incoherent inputs: timeOutputFormat: %s value: %s (%s)", format, fmt.Sprint(value), "int"))
+	})
 }