use our new ->ISO8601DateTime protocol to simplify things a bit and create...

use our new ->ISO8601DateTime protocol to simplify things a bit and create unit tests specifically for our new QP middleware functions.

src/metabase/driver/query_processor.clj

@@ -149,29 +149,17 @@
         ;; Add :rows_truncated if we've hit the limit so the UI can let the user know
         (= num-results results-limit) (assoc-in [:data :rows_truncated] results-limit)))))
 
-(defn format-timestamp [ts & {:keys [tz]}]
-  (let [formatter (if tz
-                    (tf/formatter "yyyy-MM-dd'T'HH:mm:ss.SSSZ'" (t/time-zone-for-id tz))
-                    (tf/formatter "yyyy-MM-dd'T'HH:mm:ss.SSSZ'"))]
-    (tf/unparse formatter ts)))
-
-(defn- format-rows [{:keys [report_timezone]} rows]
-  (let [format-ts    #(format-timestamp % :tz report_timezone)
-        format-value (fn [v]
-                       (condp = (keyword (.getName (type v)))
-                         :java.sql.Timestamp (format-ts (tc/from-sql-time v))
-                         :java.sql.Date      (format-ts (tc/from-sql-date v))
-                         :java.util.Date     (format-ts (tc/from-date v))
-                         v))]
+(defn- format-rows [{:keys [report-timezone]} rows]
+  (let [format-value #(if (u/is-temporal? %)
+                       (u/->ISO8601DateTime % report-timezone)
+                       %)]
     (for [row rows]
       (map format-value row))))
 
 (defn- post-format-rows
-  "Format temporal values as strings."
+  "Format individual query result values as needed.  Ex: format temporal values as iso8601 strings w/ timezone."
   [qp]
   (fn [{:keys [settings] :as query}]
-    (log/info "query" query)
-    (log/info "settings" settings)
     (let [results (qp query)]
       (if-not (:rows results)
         results

test/metabase/driver/qp_middleware_test.clj

+(ns metabase.driver.qp-middleware-test
+  (:require [expectations :refer :all]
+            [clj-time.coerce :as tc]
+            [metabase.driver :as driver]
+            [metabase.driver.query-processor :refer :all]
+            [metabase.models.setting :as setting]
+            [metabase.test.util :refer [resolve-private-fns]]))
+
+(resolve-private-fns metabase.driver.query-processor post-format-rows pre-add-settings)
+
+
+(defrecord TestDriver []
+  clojure.lang.Named
+  (getName [_] "TestDriver"))
+
+(extend TestDriver
+  driver/IDriver
+  {:features (constantly #{:set-timezone})})
+
+
+;; pre-add-settings
+
+(expect
+  [{:settings {}}
+   {:settings {:report-timezone "US/Mountain"}}]
+  (let [original-tz (setting/get :report-timezone)
+        response1   ((pre-add-settings identity) {:driver (TestDriver.)})]
+    (setting/set :report-timezone "US/Mountain")
+    (let [response2 ((pre-add-settings identity) {:driver (TestDriver.)})]
+      (if original-tz
+        (setting/set :report-timezone original-tz)
+        (setting/delete :report-timezone))
+      [(dissoc response1 :driver)
+       (dissoc response2 :driver)])))
+
+
+;; post-format-rows
+
+(expect
+  [["2011-04-18T10:12:47.232Z"]
+   ["2011-04-18T00:00:00.000Z"]
+   ["2011-04-18T10:12:47.232Z"]]
+  ((post-format-rows (constantly {:rows [[(tc/to-sql-time 1303121567232)]
+                                         [(tc/to-sql-date "2011-04-18")] ; joda-time assumes this is UTC time when parsing it
+                                         [(tc/to-date 1303121567232)]]})) {:settings {}}))
+
+(expect
+  [["2011-04-18T19:12:47.232+09:00"]
+   ["2011-04-18T09:00:00.000+09:00"]
+   ["2011-04-18T19:12:47.232+09:00"]]
+  ((post-format-rows (constantly {:rows [[(tc/to-sql-time 1303121567232)]
+                                         [(tc/to-sql-date "2011-04-18")] ; joda-time assumes this is UTC time when parsing it
+                                         [(tc/to-date 1303121567232)]]})) {:settings {:report-timezone "Asia/Tokyo"}}))

test/metabase/driver_test.clj

+(ns metabase.driver-test
+  (:require [expectations :refer :all]
+            [metabase.driver :refer :all]))
+
+
+(defrecord TestDriver []
+  clojure.lang.Named
+  (getName [_] "TestDriver"))
+
+(extend TestDriver
+  IDriver
+  {:features (constantly #{:a})})
+
+
+;; driver-supports?
+
+(expect true (driver-supports? (TestDriver.) :a))
+(expect false (driver-supports? (TestDriver.) :b))