Fix parsing ga:yearWeek in Google Analytics results [ci googleanalytics] (#11493)

modules/drivers/googleanalytics/src/metabase/driver/googleanalytics/query_processor.clj

@@ -2,6 +2,7 @@
   "The Query Processor is responsible for translating the Metabase Query Language into Google Analytics request format.
   See https://developers.google.com/analytics/devguides/reporting/core/v3"
   (:require [clojure.string :as str]
+            [clojure.tools.logging :as log]
             [clojure.tools.reader.edn :as edn]
             [java-time :as t]
             [metabase.mbql.util :as mbql.u]
@@ -10,8 +11,10 @@
              [date-2 :as u.date]
              [i18n :as ui18n :refer [deferred-tru tru]]
              [schema :as su]]
-            [metabase.util.date-2.parse :as u.date.parse]
-            [metabase.util.date-2.parse.builder :as u.date.parse.builder]
+            [metabase.util.date-2
+             [common :as u.date.common]
+             [parse :as u.date.parse]]
+            [metabase.util.date-2.parse.builder :as u.date.builder]
             [schema.core :as s])
   (:import [com.google.api.services.analytics.model GaData GaData$ColumnHeaders]
            java.time.DayOfWeek
@@ -415,27 +418,40 @@
   (edn/read-string (str/replace s #"^0+(.+)$" "$1")))
 
 (def ^:private ^DateTimeFormatter iso-year-week-formatter
-  (u.date.parse.builder/formatter
-   (u.date.parse.builder/value :iso/week-based-year 4)
-   (u.date.parse.builder/value :iso/week-of-week-based-year 2)))
+  (u.date.builder/formatter
+   (u.date.builder/value :iso/week-based-year 4)
+   (u.date.builder/value :iso/week-of-week-based-year 2)))
 
 (defn- parse-iso-year-week [^String s]
   (when s
     (-> (YearWeek/from (.parse iso-year-week-formatter s))
         (.atDay DayOfWeek/MONDAY))))
 
+(def ^:private ^DateTimeFormatter year-week
+  (u.date.builder/formatter
+   (u.date.builder/value :week-fields/week-based-year 4)
+   (u.date.builder/value :week-fields/week-of-week-based-year 2)))
+
+(defn- parse-year-week [^String s]
+  (when s
+    (let [parsed (.parse year-week s)
+          year   (.getLong parsed (u.date.common/temporal-field :week-fields/week-based-year))
+          week   (.getLong parsed (u.date.common/temporal-field :week-fields/week-of-week-based-year))]
+      (t/adjust (t/local-date year 1 1) (u.date/adjuster :week-of-year week)))))
+
 (def ^:private ga-dimension->date-format-fn
-  {"ga:minute"         parse-number
-   "ga:dateHour"       (partial u.date.parse/parse-with-formatter "yyyyMMddHH")
-   "ga:hour"           parse-number
-   "ga:date"           (partial u.date.parse/parse-with-formatter "yyyyMMdd")
-   "ga:dayOfWeek"      (comp inc parse-number)
+  {"ga:date"           "yyyyMMdd"
+   "ga:dateHour"       "yyyyMMddHH"
    "ga:day"            parse-number
+   "ga:dayOfWeek"      (comp inc parse-number)
+   "ga:hour"           parse-number
    "ga:isoYearIsoWeek" parse-iso-year-week
-   "ga:week"           parse-number
-   "ga:yearMonth"      (partial u.date.parse/parse-with-formatter "yyyyMM")
+   "ga:minute"         parse-number
    "ga:month"          parse-number
-   "ga:year"           parse-number})
+   "ga:week"           parse-number
+   "ga:year"           parse-number
+   "ga:yearMonth"      "yyyyMM"
+   "ga:yearWeek"       parse-year-week})
 
 (defn- header->column [^GaData$ColumnHeaders header]
   (let [date-parser (ga-dimension->date-format-fn (.getName header))]
@@ -447,11 +463,15 @@
 
 (defn- header->getter-fn [^GaData$ColumnHeaders header]
   (let [date-parser (ga-dimension->date-format-fn (.getName header))
-        base-type   (ga-type->base-type (.getDataType header))]
-    (cond
-      date-parser                   date-parser
-      (isa? base-type :type/Number) edn/read-string
-      :else                         identity)))
+        base-type   (ga-type->base-type (.getDataType header))
+        parser      (cond
+                      date-parser                   date-parser
+                      (isa? base-type :type/Number) edn/read-string
+                      :else                         identity)]
+    (log/tracef "Parsing result column %s with %s" (.getName header) (pr-str parser))
+    (if (string? parser)
+      (partial u.date.parse/parse-with-formatter parser)
+      parser)))
 
 (defn execute-query
   "Execute a `query` using the provided `do-query` function, and return the results in the usual format."

modules/drivers/googleanalytics/test/metabase/driver/googleanalytics/query_processor_test.clj

@@ -16,10 +16,19 @@
     (is (= "ga::B"
            (#'ga.qp/built-in-segment {:filter [:and [:segment 100] [:segment "ga::B"]]})))))
 
-(deftest iso-year-iso-week-test
+(deftest parse-year-week-test
   (testing "Make sure we properly parse isoYearIsoWeeks (#9244)"
-    (is (= #t "2018-12-31"
-           ((#'ga.qp/ga-dimension->date-format-fn "ga:isoYearIsoWeek") "201901")))))
+    (let [f (#'ga.qp/ga-dimension->date-format-fn "ga:isoYearIsoWeek")]
+      (is (= #t "2018-12-31"
+             (f "201901")))
+      (is (= #t "2019-12-09"
+             (f "201950")))))
+  (testing "Make sure we properly parse (non-ISO) yearWeeks"
+    (let [f (#'ga.qp/ga-dimension->date-format-fn "ga:yearWeek")]
+      (is (= #t "2018-12-30"
+             (f "201901")))
+      (is (= #t "2019-12-08"
+             (f "201950"))))))
 
 (deftest filter-test
   (testing "absolute datetimes"

src/metabase/util/date_2.clj

@@ -148,24 +148,51 @@
              :quarter-of-year  :quarter-of-year
              :year             :year))))
 
-(def ^:private adjusters*
-  {:first-day-of-week
-   (reify TemporalAdjuster
-     (adjustInto [_ t]
-       (t/adjust t :previous-or-same-day-of-week :sunday)))
-
-   :first-day-of-iso-week
-   (reify TemporalAdjuster
-     (adjustInto [_ t]
-       (t/adjust t :previous-or-same-day-of-week :monday)))
-
-   :first-day-of-quarter
-   (reify TemporalAdjuster
-     (adjustInto [_ t]
-       (.with t (.atDay (t/year-quarter t) 1))))})
-
-(defn- adjusters ^TemporalAdjuster [k]
-  (get adjusters* k))
+(defmulti ^TemporalAdjuster adjuster
+  "Get the custom `TemporalAdjuster` named by `k`.
+
+    ;; adjust 2019-12-10T17:26 to the second week of the year
+    (t/adjust #t \"2019-12-10T17:26\" (u.date/adjuster :week-of-year 2)) ;; -> #t \"2019-01-06T17:26\""
+  {:arglists '([k & args])}
+  (fn [k & _] (keyword k)))
+
+(defmethod adjuster :default
+  [k]
+  (throw (Exception. (tru "No temporal adjuster named {0}" k))))
+
+(defmethod adjuster :first-day-of-week
+  [_]
+  (reify TemporalAdjuster
+    (adjustInto [_ t]
+      (t/adjust t :previous-or-same-day-of-week :sunday))))
+
+(defmethod adjuster :first-day-of-iso-week
+  [_]
+  (reify TemporalAdjuster
+    (adjustInto [_ t]
+      (t/adjust t :previous-or-same-day-of-week :monday))))
+
+(defmethod adjuster :first-day-of-quarter
+  [_]
+  (reify TemporalAdjuster
+    (adjustInto [_ t]
+      (.with t (.atDay (t/year-quarter t) 1)))))
+
+(defmethod adjuster :first-week-of-year
+  [_]
+  (reify TemporalAdjuster
+    (adjustInto [_ t]
+      (-> t
+          (t/adjust :first-day-of-year)
+          (t/adjust (adjuster :first-day-of-week))))))
+
+(defmethod adjuster :week-of-year
+  [_ week-of-year]
+  (reify TemporalAdjuster
+    (adjustInto [_ t]
+      (-> t
+          (t/adjust (adjuster :first-week-of-year))
+          (t/plus (t/weeks (dec week-of-year)))))))
 
 ;; if you attempt to truncate a `LocalDate` to `:day` or anything smaller we can go ahead and return it as is
 (extend-protocol t.core/Truncatable
@@ -196,10 +223,10 @@
      :minute      (t/truncate-to t :minutes)
      :hour        (t/truncate-to t :hours)
      :day         (t/truncate-to t :days)
-     :week        (-> (.with t (adjusters :first-day-of-week))     (t/truncate-to :days))
-     :iso-week    (-> (.with t (adjusters :first-day-of-iso-week)) (t/truncate-to :days))
+     :week        (-> (.with t (adjuster :first-day-of-week))     (t/truncate-to :days))
+     :iso-week    (-> (.with t (adjuster :first-day-of-iso-week)) (t/truncate-to :days))
      :month       (-> (t/adjust t :first-day-of-month)             (t/truncate-to :days))
-     :quarter     (-> (.with t (adjusters :first-day-of-quarter))  (t/truncate-to :days))
+     :quarter     (-> (.with t (adjuster :first-day-of-quarter))  (t/truncate-to :days))
      :year        (-> (t/adjust t :first-day-of-year)              (t/truncate-to :days)))))
 
 (s/defn bucket :- (s/cond-pre Number Temporal)

src/metabase/util/date_2/parse/builder.clj

@@ -98,7 +98,8 @@
     field))
 
 (defn value
-  "Define a section for a specific field such as `:hour-of-day` or `:minute-of-hour`."
+  "Define a section for a specific field such as `:hour-of-day` or `:minute-of-hour`. Refer to
+  `metabase.util.date-2.common/temporal-field` for all possible temporal fields names."
   ([temporal-field-name]
    (fn [^DateTimeFormatterBuilder builder]
      (.appendValue builder (temporal-field temporal-field-name))))

test/metabase/util/date_2_test.clj

@@ -153,6 +153,22 @@
            (u.date/format-sql (t/zoned-date-time "2019-11-01T18:39:00-07:00[US/Pacific]")))
         "should get formatted as the same way as an OffsetDateTime")))
 
+(deftest adjuster-test
+  (let [now (t/zoned-date-time "2019-12-10T17:17:00-08:00[US/Pacific]")]
+    (testing "adjust temporal value to first day of week (Sunday)"
+      (is (= (t/zoned-date-time "2019-12-08T17:17-08:00[US/Pacific]")
+             (t/adjust now (u.date/adjuster :first-day-of-week)))))
+    (testing "adjust temporal value to first day of ISO week (Monday)"
+      (is (= (t/zoned-date-time "2019-12-09T17:17-08:00[US/Pacific]")
+             (t/adjust now (u.date/adjuster :first-day-of-iso-week)))))
+    (testing "adjust temporal value to first day of first week of year (previous or same Sunday as first day of year)"
+      (is (= (t/zoned-date-time "2018-12-30T17:17-08:00[US/Pacific]")
+             (t/adjust now (u.date/adjuster :first-week-of-year))
+             (t/adjust now (u.date/adjuster :week-of-year 1)))))
+    (testing "adjust temporal value to the 50th week of the year"
+      (is (= (t/zoned-date-time "2019-12-08T17:17-08:00[US/Pacific]")
+             (t/adjust now (u.date/adjuster :week-of-year 50)))))))
+
 (deftest extract-test
   (testing "u.date/extract with 2 args"
     ;; everything is at `Sunday October 27th 2019 2:03:40.555 PM` or subset thereof