Support hhmm timezone format (#46172)

* Add testcase from https://github.com/metabase/metabase/issues/43915



* Try a custom formatter if the default formatter for OffsetDateTime doesn't work

* Add testcase

* Make auxillary formatter private

* Catch a more specific exception

* Throw IllegalArgumentException on invalid dates

* Use strict resolver and fix test

* Empty commit to assign credit where it's due

---------

Co-authored-by: Chris Truter <chris@metabase.com>

src/metabase/upload/parsing.clj

@@ -6,8 +6,8 @@
    [metabase.util.i18n :refer [tru]])
   (:import
    (java.text NumberFormat ParsePosition)
-   (java.time LocalDate)
-   (java.time.format DateTimeFormatter DateTimeFormatterBuilder ResolverStyle)
+   (java.time LocalDate OffsetDateTime)
+   (java.time.format DateTimeFormatter DateTimeFormatterBuilder DateTimeParseException ResolverStyle)
    (java.util Locale)))
 
 (set! *warn-on-reflection* true)
@@ -105,6 +105,25 @@
           (throw (IllegalArgumentException.
                   (tru "''{0}'' is not a recognizable datetime" s))))))))
 
+(def ^:private auxillary-offset-datetime-formatter
+  (-> (DateTimeFormatterBuilder.)
+    (.parseCaseInsensitive)
+    (.append DateTimeFormatter/ISO_LOCAL_DATE_TIME)
+    (.optionalStart)
+    (.appendPattern "ss")
+    (.optionalEnd)
+    (.optionalStart)
+    (.appendPattern ".SSS")
+    (.optionalEnd)
+    (.optionalStart)
+    (.appendZoneOrOffsetId)
+    (.optionalEnd)
+    (.optionalStart)
+    (.appendOffset "+HHMM", "Z")
+    (.optionalEnd)
+    (.toFormatter)
+    (.withResolverStyle ResolverStyle/STRICT)))
+
 (defn parse-offset-datetime
   "Parses a string representing an offset datetime into an OffsetDateTime.
 
@@ -119,13 +138,18 @@
       - +HH or -HH
       - +HH:mm or -HH:mm
       - +HH:mm:ss or -HH:mm:ss
+      - +HHmm (see auxillary-offset-datetime-formatter)
 
   Parsing is case-insensitive."
   [s]
-  (try
-    (-> s (str/replace \space \T) t/offset-date-time)
-    (catch Exception _
-      (throw (IllegalArgumentException. (tru "''{0}'' is not a recognizable zoned datetime" s))))))
+  (let [ss (str/replace s \space \T)]
+    (try
+      (try
+        (OffsetDateTime/parse ss)
+        (catch DateTimeParseException _
+          (OffsetDateTime/parse ss auxillary-offset-datetime-formatter)))
+      (catch Exception _
+        (throw (IllegalArgumentException. (tru "''{0}'' is not a recognizable zoned datetime" s)))))))
 
 (defn- remove-currency-signs
   "Remove any recognized currency signs from the string (c.f. [[currency-regex]])."

test/metabase/upload/types_test.clj

@@ -152,7 +152,8 @@
            [" 2022-01-01 01:00:00.00-07:00 "  #t "2022-01-01T01:00-07:00" offset-dt-type]
            [" 2022-01-01T01:00:00.00Z "       (t/offset-date-time "2022-01-01T01:00+00:00") offset-dt-type]
            [" 2022-01-01t01:00:00.00Z "       (t/offset-date-time "2022-01-01T01:00+00:00") offset-dt-type]
-           [" 2022-01-01 01:00:00.00Z "       (t/offset-date-time "2022-01-01T01:00+00:00") offset-dt-type]]]
+           [" 2022-01-01 01:00:00.00Z "       (t/offset-date-time "2022-01-01T01:00+00:00") offset-dt-type]
+           [" 2024-06-10T07:55:59+0000 "      (t/offset-date-time "2024-06-10T07:55:59+00:00") offset-dt-type]]]
     (let [settings    {:number-separators (or separators ".,")}
           type->check (#'upload-types/settings->type->check settings)
           value-type  (#'upload-types/value->type type->check string-value)

test/metabase/upload_test.clj

@@ -236,14 +236,17 @@
 
 (deftest ^:parallel detect-schema-offset-datetimes-test
   (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
-    (testing "Dates"
-      (is (=? {:offset_datetime offset-dt-type
-               :not_datetime   vchar-type}
-              (detect-schema-with-csv-rows
-               ["Offset Datetime,Not Datetime"
-                "2022-01-01T00:00:00-01:00,2023-02-28T00:00:00-01:00"
-                "2022-01-01T00:00:00-01:00,2023-02-29T00:00:00-01:00"
-                "2022-01-01T00:00:00Z,2023-02-29T00:00:00-01:00"]))))))
+    (let [good-versus-bad-rows ["2022-01-01T00:00:00-01:00,2023-02-29T00:00:00-01:00"
+                                "2022-01-01T00:00:00-0100,2023-02-29T00:00:00-0100"
+                                "2022-01-01T00:00:00Z,2023-02-29T00:00:00-01:00"]]
+      (testing "Dates"
+        (doseq [additional-row good-versus-bad-rows]
+          (is (=? {:offset_datetime offset-dt-type
+                   :not_datetime   vchar-type}
+                  (detect-schema-with-csv-rows
+                   (conj ["Offset Datetime,Not Datetime"
+                          "2022-01-01T00:00:00-01:00,2023-02-28T00:00:00-01:00"]
+                         additional-row)))))))))
 
 (deftest ^:parallel unique-table-name-test
   (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
@@ -662,7 +665,8 @@
                                                 ["2022-01-01T12:00:00-00:00" "2022-01-01T12:00:00Z"]
                                                 ["2022-01-01T12:00:00+07"    "2022-01-01T05:00:00Z"]
                                                 ["2022-01-01T12:00:00+07:00" "2022-01-01T05:00:00Z"]
-                                                ["2022-01-01T12:00:00+07:30" "2022-01-01T04:30:00Z"]])]
+                                                ["2022-01-01T12:00:00+07:30" "2022-01-01T04:30:00Z"]
+                                                ["2022-01-01T12:00:00+0730"  "2022-01-01T04:30:00Z"]])]
             (testing "Fields exists after sync"
               (with-upload-table!
                 [table (create-from-csv-and-sync-with-defaults!