Make sure query templates containing newlines are parsed properly (#11532)

Make sure query templates containing newlines are parsed properly [ci drivers]

src/metabase/driver/common/parameters/parse.clj

@@ -61,7 +61,7 @@
     ["]]" :optional-end]
     ;; param-begin should only match the last two opening brackets in a sequence of > 2, e.g.
     ;; [{$match: {{{x}}, field: 1}}] should parse to ["[$match: {" (param "x") ", field: 1}}]"]
-    [#"(.*?)(\{\{(?!\{))(.*)" :param-begin]
+    [#"(?s)(.*?)(\{\{(?!\{))(.*)" :param-begin]
     ["}}" :param-end]]))
 
 (defn- param [& [k & more]]

test/metabase/driver/common/parameters/parse_test.clj

@@ -70,6 +70,10 @@
            (let [query "SELECT array_dims(1 || '[0:1]={2,3}'::int[])"]
              {query [query]})
 
+           "Queries with newlines (#11526)"
+           {"SELECT count(*)\nFROM products\nWHERE category = {{category}}"
+            ["SELECT count(*)\nFROM products\nWHERE category = " (param "category")]}
+
            "JSON queries that contain non-param fragments like '}}'"
            {"{x: {y: \"{{param}}\"}}"         ["{x: {y: \"" (param "param") "\"}}"]
             "{$match: {{{date}}, field: 1}}}" ["{$match: {" (param "date") ", field: 1}}}"]}}]

test/metabase/driver/sql/parameters/substitute_test.clj

@@ -698,6 +698,21 @@
                                                      :default      "2017-11-14"
                                                      :widget-type  :date/all-options}}}}))
 
+(deftest newlines-test
+  (testing "Make sure queries with newlines are parsed correctly (#11526)"
+    (is (= [[1]]
+           (mt/rows
+             (qp/process-query
+               {:database   (mt/id)
+                :type       "native"
+                :native     {:query         "SELECT count(*)\nFROM venues\n WHERE name = {{name}}"
+                             :template-tags {:name {:name         "name"
+                                                    :display_name "Name"
+                                                    :type         "text"
+                                                    :required     true
+                                                    :default      "Fred 62"}}}
+                :parameters []}))))))
+
 
 ;;; ------------------------------- Multiple Value Support (comma-separated or array) --------------------------------
 
@@ -705,15 +720,15 @@
   (testing "Make sure using commas in numeric params treats them as separate IDs (#5457)"
     (is (= "SELECT * FROM USERS where id IN (1, 2, 3)"
            (-> (qp/process-query
-                 {:database   (mt/id)
-                  :type       "native"
-                  :native     {:query         "SELECT * FROM USERS [[where id IN ({{ids_list}})]]"
-                               :template-tags {"ids_list" {:name         "ids_list"
-                                                           :display-name "Ids list"
-                                                           :type         :number}}}
-                  :parameters [{:type   "category"
-                                :target [:variable [:template-tag "ids_list"]]
-                                :value  "1,2,3"}]})
+                {:database   (mt/id)
+                 :type       "native"
+                 :native     {:query         "SELECT * FROM USERS [[where id IN ({{ids_list}})]]"
+                              :template-tags {"ids_list" {:name         "ids_list"
+                                                          :display-name "Ids list"
+                                                          :type         :number}}}
+                 :parameters [{:type   "category"
+                               :target [:variable [:template-tag "ids_list"]]
+                               :value  "1,2,3"}]})
                :data :native_form :query))))
   (testing "make sure you can now also pass multiple values in by passing an array of values"
     (is (= {:query  "SELECT * FROM CATEGORIES where name IN (?, ?, ?)"

test/metabase/query_processor_test/parameters_test.clj

@@ -78,44 +78,56 @@
                     {:$project {"_id" false, "count" true}}])
      :collection  (name table)}))
 
-(defn- count-query [table field->type+value]
+(defn- count-query [table field->type+value {:keys [defaults?]}]
   {:database   (mt/id)
    :type       :native
    :native     (assoc (native-count-query driver/*driver* table field->type+value)
-                      :template-tags (into {} (for [[field [param-type]] field->type+value]
-                                                [field {:name         (name field)
-                                                        :display-name (name field)
-                                                        :type         (or (namespace param-type)
-                                                                          (name param-type))}])))
-   :parameters (for [[field [param-type v]] field->type+value]
-                 {:type   param-type
-                  :target [:variable [:template-tag (name field)]]
-                  :value  v})})
-
-(defn- count= [expected table field->type+value]
-  (let [query (count-query table field->type+value)]
-    (testing (str "\nquery =\n" (u/pprint-to-str query))
-      (is (= expected
-             (ffirst
-              (mt/formatted-rows [int]
-                (qp/process-query query))))
-          (format "count with of %s with %s should be %d"
-                  (name table)
-                  (str/join " and " (for [[field [_ v]] field->type+value]
-                                      (format "%s = %s" (name field) v)))
-                  expected)))))
+                      :template-tags (into {} (for [[field [param-type v]] field->type+value]
+                                                [field (cond-> {:name         (name field)
+                                                                :display-name (name field)
+                                                                :type         (or (namespace param-type)
+                                                                                  (name param-type))}
+                                                         defaults? (assoc :default v))])))
+   :parameters (when-not defaults?
+                 (for [[field [param-type v]] field->type+value]
+                   {:type   param-type
+                    :target [:variable [:template-tag (name field)]]
+                    :value  v}))})
 
 (deftest param-test
   (mt/test-drivers (mt/normal-drivers-with-feature :native-parameters)
-    (testing "text params"
-      (count= 1
-              :venues {:name [:text "In-N-Out Burger"]}))
-    (testing "number params"
-      (count= 22
-              :venues {:price [:number "1"]}))
-    ;; FIXME — This is not currently working on SQLite, probably because SQLite's implementation of temporal types is
-    ;; wacko.
-    (when-not (= driver/*driver* :sqlite)
-      (testing "date params"
-        (count= 1
-                :users {:last_login [:date/single "2014-08-02T09:30Z"]})))))
+    (doseq [[message {:keys [expected-count table param-name param-type value exclude-drivers]}]
+            {"text params"   {:expected-count 1
+                              :table          :venues
+                              :param-name     :name
+                              :param-type     :text
+                              :value          "In-N-Out Burger"}
+             "number params" {:expected-count 22
+                              :table          :venues
+                              :param-name     :price
+                              :param-type     :number
+                              :value          "1"}
+             "date params"   {:expected-count  1
+                              ;; FIXME — This is not currently working on SQLite, probably because SQLite's
+                              ;; implementation of temporal types is wacko.
+                              :exclude-drivers #{:sqlite}
+                              :table           :users
+                              :param-name      :last_login
+                              :param-type      :date/single
+                              :value           "2014-08-02T09:30Z"}}
+            :when (not (contains? exclude-drivers driver/*driver*))]
+      (testing (str "\n" message)
+        (doseq [[message options] {"Query with all supplied parameters" nil
+                                   "Query using default values"         {:defaults? true}}]
+          (testing (str "\n" message)
+            (let [query (count-query table {param-name [param-type value]} options)]
+              (testing (str "\nquery =\n" (u/pprint-to-str query))
+                (is (= expected-count
+                       (ffirst
+                        (mt/formatted-rows [int]
+                          (qp/process-query query))))
+                    (format "count with of %s with %s = %s should be %d"
+                            (name table)
+                            (name param-name)
+                            value
+                            expected-count))))))))))