update clj-time to 0.12.0, add a bunch more unit tests for parameters date...

update clj-time to 0.12.0, add a bunch more unit tests for parameters date range calculationsa along with a couple fixes.

project.clj

@@ -30,7 +30,7 @@
                   :exclusions [commons-codec
                                commons-io
                                slingshot]]
-                 [clj-time "0.11.0"]                                  ; library for dealing with date/time
+                 [clj-time "0.12.0"]                                  ; library for dealing with date/time
                  [clojurewerkz/quartzite "2.0.0"]                     ; scheduling library
                  [colorize "0.1.1" :exclusions [org.clojure/clojure]] ; string output with ANSI color codes (for logging)
                  [com.cemerick/friend "0.2.1"                         ; auth library

resources/log4j.properties

@@ -18,7 +18,6 @@ log4j.appender.metabase=metabase.logger.Appender
 
 # customizations to logging by package
 log4j.logger.metabase=INFO
-log4j.logger.metabase.db=DEBUG
 log4j.logger.metabase.driver=DEBUG
 log4j.logger.metabase.query-processor=DEBUG
 log4j.logger.metabase.sync-database=DEBUG

src/metabase/query_processor/parameters.clj

@@ -29,8 +29,9 @@
 
 (defn- week-range [dt]
   ;; weeks always start on SUNDAY and end on SATURDAY
+  ;; NOTE: in Joda the week starts on Monday and ends on Sunday, so to get the right Sunday we rollback 1 week
   {:end   (.withDayOfWeek dt DateTimeConstants/SATURDAY)
-   :start (.withDayOfWeek dt DateTimeConstants/SUNDAY)})
+   :start (.withDayOfWeek (t/minus dt (t/weeks 1)) DateTimeConstants/SUNDAY)})
 
 (defn- month-range [dt]
   {:end   (t/last-day-of-the-month dt)
@@ -50,19 +51,51 @@
    Supported formats:
       \"2014-05-10~2014-05-16\"
       \"Q1-2016\"
-      \"2016-04\""
+      \"2016-04\"
+      \"2016-04-12\""
   [value]
   (if (s/includes? value "~")
     ;; these values are already expected to be iso8601 strings, so we are done
     (zipmap [:start :end] (s/split value #"~" 2))
     ;; these cases represent fixed date ranges, but we need to calculate start/end still
-    (->> (if (s/starts-with? value "Q")
-           (let [[quarter year] (s/split value #"-" 2)]
-             (quarter-range (start-of-quarter quarter (Integer/parseInt year))))
-           (month-range (tf/parse (tf/formatters :year-month) value)))
+    (->> (cond
+           ;; quarter-year (Q1-2016)
+           (s/starts-with? value "Q") (let [[quarter year] (s/split value #"-" 2)]
+                                        (quarter-range (start-of-quarter quarter (Integer/parseInt year))))
+           ;; year-month (2016-04)
+           (= (count value) 7)        (month-range (tf/parse (tf/formatters :year-month) value))
+           ;; default is to assume a single day (2016-04-18).  we still parse just to validate.
+           :else                      (let [dt (tf/parse (tf/formatters :year-month-day) value)]
+                                        {:start dt, :end dt}))
          (m/map-vals (partial tf/unparse (tf/formatters :year-month-day))))))
 
 
+(defn- relative-date->range
+  "Take a given string description of a relative date range such as 'lastmonth' and return a MAP with a given
+   `:start` and `:end` as iso8601 string formatted dates.  Values should be appropriate for the given REPORT-TIMEZONE."
+  [value report-timezone]
+  (let [tz        (t/time-zone-for-id report-timezone)
+        formatter (tf/formatter "YYYY-MM-dd" tz)
+        today     (.withTimeAtStartOfDay (t/to-time-zone (t/now) tz))]
+    (->> (condp = value
+           "past7days"  {:end   (t/minus today (t/days 1))
+                         :start (t/minus today (t/days 7))}
+           "past30days" {:end   (t/minus today (t/days 1))
+                         :start (t/minus today (t/days 30))}
+           "thisweek"   (week-range today)
+           "thismonth"  (month-range today)
+           "thisyear"   (year-range today)
+           "lastweek"   (week-range (t/minus today (t/weeks 1)))
+           "lastmonth"  (month-range (t/minus today (t/months 1)))
+           "lastyear"   (year-range (t/minus today (t/years 1)))
+           "yesterday"  {:end   (t/minus today (t/days 1))
+                         :start (t/minus today (t/days 1))}
+           "today"      {:end   today
+                         :start today})
+         ;; the above values are JodaTime objects, so unparse them to iso8601 strings
+         (m/map-vals (partial tf/unparse formatter)))))
+
+
 ;;; +-------------------------------------------------------------------------------------------------------+
 ;;; |                                           MBQL QUERIES                                             |
 ;;; +-------------------------------------------------------------------------------------------------------+
@@ -113,31 +146,6 @@
 ;;; +-------------------------------------------------------------------------------------------------------+
 
 
-(defn- relative-date->range
-  "Take a given string description of a relative date range such as 'lastmonth' and return a MAP with a given
-   `:start` and `:end` as iso8601 string formatted dates.  Values should be appropriate for the given REPORT-TIMEZONE."
-  [value report-timezone]
-  (let [tz        (t/time-zone-for-id report-timezone)
-        formatter (tf/formatter "YYYY-MM-dd" tz)
-        today     (t/today-at-midnight tz)]
-    (->> (condp = value
-           "past7days"  {:end   (t/minus today (t/days 1))
-                         :start (t/minus today (t/days 7))}
-           "past30days" {:end   (t/minus today (t/days 1))
-                         :start (t/minus today (t/days 30))}
-           "thisweek"   (week-range today)
-           "thismonth"  (month-range today)
-           "thisyear"   (year-range today)
-           "lastweek"   (week-range (t/minus today (t/weeks 1)))
-           "lastmonth"  (month-range (t/minus today (t/months 1)))
-           "lastyear"   (year-range (t/minus today (t/years 1)))
-           "yesterday"  {:end   (t/minus today (t/days 1))
-                         :start (t/minus today (t/days 1))}
-           "today"      {:end   today
-                         :start today})
-         ;; the above values are JodaTime objects, so unparse them to iso8601 strings
-         (m/map-vals (partial tf/unparse formatter)))))
-
 (defn- extract-dates [value report-timezone]
   (if-not (contains? relative-dates value)
     ;; absolute date range

test/metabase/query_processor/parameters_test.clj

@@ -12,9 +12,31 @@
             [metabase.test.util :as tu]))
 
 (tu/resolve-private-fns metabase.query-processor.parameters
-  expand-date-range-param)
+  absolute-date->range expand-date-range-param relative-date->range)
+
+(expect {:end "2016-03-31", :start "2016-01-01"} (absolute-date->range "Q1-2016"))
+(expect {:end "2016-02-29", :start "2016-02-01"} (absolute-date->range "2016-02"))
+(expect {:end "2016-04-18", :start "2016-04-18"} (absolute-date->range "2016-04-18"))
+(expect {:end "2016-04-23", :start "2016-04-18"} (absolute-date->range "2016-04-18~2016-04-23"))
+
+;; we hard code "now" to a specific point in time so that we can control the test output
+(defn- test-relative [value]
+  (with-redefs-fn {#'clj-time.core/now (fn [] (t/date-time 2016 06 07 12 0 0))}
+    #(relative-date->range value nil)))
+
+(expect {:end "2016-06-06", :start "2016-05-31"} (test-relative "past7days"))
+(expect {:end "2016-06-06", :start "2016-05-08"} (test-relative "past30days"))
+(expect {:end "2016-06-11", :start "2016-06-05"} (test-relative "thisweek"))
+(expect {:end "2016-06-30", :start "2016-06-01"} (test-relative "thismonth"))
+(expect {:end "2016-12-31", :start "2016-01-01"} (test-relative "thisyear"))
+(expect {:end "2016-06-04", :start "2016-05-29"} (test-relative "lastweek"))
+(expect {:end "2016-05-31", :start "2016-05-01"} (test-relative "lastmonth"))
+(expect {:end "2015-12-31", :start "2015-01-01"} (test-relative "lastyear"))
+(expect {:end "2016-06-06", :start "2016-06-06"} (test-relative "yesterday"))
+(expect {:end "2016-06-07", :start "2016-06-07"} (test-relative "today"))
 
 ;; TODO: Casting of values into appropriate types (structured queries)
+;; TODO: test for value escaping to prevent sql injection (native queries)
 
 ;;; +-------------------------------------------------------------------------------------------------------+
 ;;; |                                           NATIVE QUERIES                                             |
@@ -126,15 +148,13 @@
 (expect
   [{:type "date", :target ["parameter" "foo:start"], :value "2014-05-10"}
    {:type "date", :target ["parameter" "foo:end"], :value "2014-05-16"}]
-  (expand-date-range-param nil {:target ["parameter" "foo"], :type "date", :value "2014-05-10,2014-05-16"}))
+  (expand-date-range-param nil {:target ["parameter" "foo"], :type "date", :value "2014-05-10~2014-05-16"}))
 
 (expect
   [{:type "date", :target ["parameter" "foo:start"], :value (tf/unparse (tf/formatters :year-month-day) (t/yesterday))}
    {:type "date", :target ["parameter" "foo:end"], :value (tf/unparse (tf/formatters :year-month-day) (t/yesterday))}]
   (expand-date-range-param nil {:target ["parameter" "foo"], :type "date", :value "yesterday"}))
 
-;; TODO: test for value escaping to prevent sql injection
-
 ;; if driver doesn't support :native-parameters then we skip any substitutions
 (expect
   {:database   1
@@ -291,4 +311,4 @@
                                     :name   "foo"
                                     :type   "date"
                                     :target ["dimension" ["field-id" 123]]
-                                    :value  "2014-05-10,2014-05-16"}]}))
+                                    :value  "2014-05-10~2014-05-16"}]}))