diff --git a/src/metabase/api/database.clj b/src/metabase/api/database.clj
index 49ea7918e3556534fe4503ffaaaa750406526bcb..e6e3808ee17a9df05d497388f9de07d93ddcf9bd 100644
--- a/src/metabase/api/database.clj
+++ b/src/metabase/api/database.clj
@@ -246,6 +246,7 @@
(sample-data/add-sample-dataset!)
(Database :is_sample true))
+
;;; ------------------------------------------------------------ PUT /api/database/:id ------------------------------------------------------------
(api/defendpoint PUT "/:id"
diff --git a/src/metabase/api/public.clj b/src/metabase/api/public.clj
index ba3690a093f01c1281cd6926ef798c7383186510..996fbd43e7a0a959c7bb7aa722790da0ad29a051 100644
--- a/src/metabase/api/public.clj
+++ b/src/metabase/api/public.clj
@@ -88,11 +88,11 @@
(card-with-uuid uuid))
-
(defn run-query-for-card-with-id
"Run the query belonging to Card with CARD-ID with PARAMETERS and other query options (e.g. `:constraints`)."
[card-id parameters & options]
(u/prog1 (-> (let [parameters (if (string? parameters) (json/parse-string parameters keyword) parameters)]
+ ;; run this query with full superuser perms
(binding [api/*current-user-permissions-set* (atom #{"/"})
qp/*allow-queries-with-no-executor-id* true]
(apply card-api/run-query-for-card card-id, :parameters parameters, :context :public-question, options)))
diff --git a/src/metabase/api/table.clj b/src/metabase/api/table.clj
index 91909a555838eaa3ca3740920e2aeec92877faae..83354618e2af92ea5db48f8b3557830cbb0e4777 100644
--- a/src/metabase/api/table.clj
+++ b/src/metabase/api/table.clj
@@ -2,6 +2,7 @@
"/api/table endpoints."
(:require [clojure.tools.logging :as log]
[compojure.core :refer [GET PUT]]
+ [medley.core :as m]
[metabase
[sync-database :as sync-database]
[util :as u]]
@@ -87,6 +88,7 @@
{include_sensitive_fields (s/maybe su/BooleanString)}
(-> (api/read-check Table id)
(hydrate :db [:fields :target] :field_values :segments :metrics)
+ (m/dissoc-in [:db :details])
(update-in [:fields] (if (Boolean/parseBoolean include_sensitive_fields)
;; If someone passes include_sensitive_fields return hydrated :fields as-is
identity
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 80a4ff3bc61e8d445519c40c5af83f2af8784799..3eb090d1542e8e926eab35e62504005828fa29cf 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -323,6 +323,17 @@
(log/warn (format "Don't know how to map class '%s' to a Field base_type, falling back to :type/*." klass))
:type/*))
+(defn values->base-type
+ "Given a sequence of VALUES, return the most common base type."
+ [values]
+ (->> values
+ (filter (complement nil?)) ; filter out `nil` values
+ (take 1000) ; take up to 1000 values
+ (group-by (comp class->base-type class)) ; now group by their base-type
+ (sort-by (comp (partial * -1) count second)) ; sort the map into pairs of [base-type count] with highest count as first pair
+ ffirst)) ; take the base-type from the first pair
+
+
;; ## Driver Lookup
(defn engine->driver
diff --git a/src/metabase/driver/generic_sql.clj b/src/metabase/driver/generic_sql.clj
index 4d97cf543544b5688e00f50391b8d984e642c061..a2157fa9076811d95f4403b76c97ff81cd1f7d8d 100644
--- a/src/metabase/driver/generic_sql.clj
+++ b/src/metabase/driver/generic_sql.clj
@@ -220,6 +220,7 @@
(defn honeysql-form->sql+args
"Convert HONEYSQL-FORM to a vector of SQL string and params, like you'd pass to JDBC."
+ {:style/indent 1}
[driver honeysql-form]
{:pre [(map? honeysql-form)]}
(let [[sql & args] (try (binding [hformat/*subquery?* false]
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index 73e8c0623f9094d942fd950e367013bde8a276a7..e8c254dd6856d5a4a02953c76f42941590c1fee2 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -243,7 +243,7 @@
(h/limit items)
(h/offset (* items (dec page)))))
-(defn- apply-source-table [_ honeysql-form {{table-name :name, schema :schema} :source-table}]
+(defn- apply-source-table [honeysql-form {{table-name :name, schema :schema} :source-table}]
{:pre [table-name]}
(h/from honeysql-form (hx/qualify-and-escape-dots schema table-name)))
@@ -252,7 +252,7 @@
;; will get swapped around and we'll be left with old version of the function that nobody implements
;; 2) This is a vector rather than a map because the order the clauses get handled is important for some drivers.
;; For example, Oracle needs to wrap the entire query in order to apply its version of limit (`WHERE ROWNUM`).
- [:source-table apply-source-table
+ [:source-table (u/drop-first-arg apply-source-table)
:aggregation #'sql/apply-aggregation
:breakout #'sql/apply-breakout
:fields #'sql/apply-fields
diff --git a/src/metabase/driver/googleanalytics/query_processor.clj b/src/metabase/driver/googleanalytics/query_processor.clj
index 1424d4d7ae115dd6d4e63ad5d0af27b6be086d13..cc0990c380f4708b0309793958bdfd670b8c1f31 100644
--- a/src/metabase/driver/googleanalytics/query_processor.clj
+++ b/src/metabase/driver/googleanalytics/query_processor.clj
@@ -3,7 +3,7 @@
(:require [clojure.string :as s]
[clojure.tools.reader.edn :as edn]
[medley.core :as m]
- [metabase.query-processor.expand :as ql]
+ [metabase.query-processor.util :as qputil]
[metabase.util :as u])
(:import [com.google.api.services.analytics.model GaData GaData$ColumnHeaders]
[metabase.query_processor.interface AgFieldRef DateTimeField DateTimeValue Field RelativeDateTimeValue Value]))
@@ -251,7 +251,7 @@
[{query :query}]
(let [[aggregation-type metric-name] (first-aggregation query)]
(when (and aggregation-type
- (= :metric (ql/normalize-token aggregation-type))
+ (= :metric (qputil/normalize-token aggregation-type))
(string? metric-name))
metric-name)))
@@ -266,7 +266,7 @@
(defn- filter-type ^clojure.lang.Keyword [filter-clause]
(when (and (sequential? filter-clause)
(u/string-or-keyword? (first filter-clause)))
- (ql/normalize-token (first filter-clause))))
+ (qputil/normalize-token (first filter-clause))))
(defn- compound-filter? [filter-clause]
(contains? #{:and :or :not} (filter-type filter-clause)))
diff --git a/src/metabase/driver/mongo/util.clj b/src/metabase/driver/mongo/util.clj
index ce830438fdc750382d7300e46555b3cb9c8f79aa..c7a153a1a7c532606a3eb0f07749caca1b30182f 100644
--- a/src/metabase/driver/mongo/util.clj
+++ b/src/metabase/driver/mongo/util.clj
@@ -137,26 +137,3 @@
(if *mongo-connection*
(f# *mongo-connection*)
(-with-mongo-connection f# ~database))))
-
-;; TODO - this isn't neccesarily Mongo-specific; consider moving
-(defn values->base-type
- "Given a sequence of values, return `Field.base_type` in the most ghetto way possible.
- This just gets counts the types of *every* value and returns the `base_type` for class whose count was highest."
- [values-seq]
- {:pre [(sequential? values-seq)]}
- (or (->> values-seq
- ;; TODO - why not do a query to return non-nil values of this column instead
- (filter identity)
- ;; it's probably fine just to consider the first 1,000 *non-nil* values when trying to type a column instead
- ;; of iterating over the whole collection. (VALUES-SEQ should be up to 10,000 values, but we don't know how many are
- ;; nil)
- (take 1000)
- (group-by type)
- ;; create tuples like [Integer count].
- (map (fn [[klass valus]]
- [klass (count valus)]))
- (sort-by second)
- last ; last result will be tuple with highest count
- first ; keep just the type
- driver/class->base-type) ; convert to Field base_type
- :type/*))
diff --git a/src/metabase/middleware.clj b/src/metabase/middleware.clj
index 5e2467e44bff706c5269cb29b60395e045bfa05a..9ca52e30797b1ab6cade62ca6eda95ef22a6581a 100644
--- a/src/metabase/middleware.clj
+++ b/src/metabase/middleware.clj
@@ -100,17 +100,12 @@
"Return User ID and superuser status for Session with SESSION-ID if it is valid and not expired."
[session-id]
(when (and session-id (init-status/complete?))
- (when-let [session (or (session-with-id session-id)
- (println "no matching session with ID") ; DEBUG
- )]
- (if (session-expired? session)
- (printf "session-is-expired! %d min / %d min\n" (session-age-minutes session) (config/config-int :max-session-age)) ; DEBUG
+ (when-let [session (session-with-id session-id)]
+ (when-not (session-expired? session)
{:metabase-user-id (:user_id session)
:is-superuser? (:is_superuser session)}))))
(defn- add-current-user-info [{:keys [metabase-session-id], :as request}]
- (when-not (init-status/complete?)
- (println "Metabase is not initialized yet!")) ; DEBUG
(merge request (current-user-info-for-session metabase-session-id)))
(defn wrap-current-user-id
@@ -347,7 +342,7 @@
(try (binding [*automatically-catch-api-exceptions* false]
(handler request))
(catch Throwable e
- (log/error (.getMessage e))
+ (log/warn (.getMessage e))
{:status 400, :body "An error occurred."}))))
(defn message-only-exceptions
diff --git a/src/metabase/query_processor/annotate.clj b/src/metabase/query_processor/annotate.clj
index 8200086f5f84f5c2c9945c3748d3cadc4d79765b..734e4bfc8ddeb102d6598eb8128380ab3dbeac06 100644
--- a/src/metabase/query_processor/annotate.clj
+++ b/src/metabase/query_processor/annotate.clj
@@ -134,16 +134,19 @@
(expression-aggregate-field-info ag)
(aggregate-field-info ag))))))
-
(defn- generic-info-for-missing-key
- "Return a set of bare-bones metadata for a Field named K when all else fails."
- [k]
- {:base-type :type/*
+ "Return a set of bare-bones metadata for a Field named K when all else fails.
+ Scan the INITIAL-VALUES of K in an attempt to determine the `base-type`."
+ [k & [initial-values]]
+ {:base-type (if (seq initial-values)
+ (driver/values->base-type initial-values)
+ :type/*)
:preview-display true
:special-type nil
:field-name k
:field-display-name k})
+
(defn- info-for-duplicate-field
"The Clojure JDBC driver automatically appends suffixes like `count_2` to duplicate columns if multiple columns come back with the same name;
since at this time we can't resolve those normally (#1786) fall back to using the metadata for the first column (e.g., `count`).
@@ -159,14 +162,14 @@
(defn- info-for-missing-key
"Metadata for a field named K, which we weren't able to resolve normally.
If possible, we work around This defaults to generic information "
- [fields k]
+ [fields k initial-values]
(or (info-for-duplicate-field fields k)
- (generic-info-for-missing-key k)))
+ (generic-info-for-missing-key k initial-values)))
(defn- add-unknown-fields-if-needed
"When create info maps for any fields we didn't expect to come back from the query.
Ideally, this should never happen, but on the off chance it does we still want to return it in the results."
- [actual-keys fields]
+ [actual-keys initial-rows fields]
{:pre [(set? actual-keys) (every? keyword? actual-keys)]}
(let [expected-keys (u/prog1 (set (map :field-name fields))
(assert (every? keyword? <>)))
@@ -175,7 +178,7 @@
(log/warn (u/format-color 'yellow "There are fields we weren't expecting in the results: %s\nExpected: %s\nActual: %s"
missing-keys expected-keys actual-keys)))
(concat fields (for [k missing-keys]
- (info-for-missing-key fields k)))))
+ (info-for-missing-key fields k (map k initial-rows))))))
(defn- convert-field-to-expected-format
"Rename keys, provide default values, etc. for FIELD so it is in the format expected by the frontend."
@@ -238,14 +241,14 @@
(defn- resolve-sort-and-format-columns
"Collect the Fields referenced in QUERY, sort them according to the rules at the top
of this page, format them as expected by the frontend, and return the results."
- [query result-keys]
+ [query result-keys initial-rows]
{:pre [(set? result-keys)]}
(when (seq result-keys)
(->> (collect-fields (dissoc query :expressions))
(map qualify-field-name)
(add-aggregate-fields-if-needed query)
(map (u/rpartial update :field-name keyword))
- (add-unknown-fields-if-needed result-keys)
+ (add-unknown-fields-if-needed result-keys initial-rows)
(sort/sort-fields query)
(map convert-field-to-expected-format)
(filter (comp (partial contains? result-keys) :name))
@@ -261,7 +264,7 @@
[query {:keys [columns rows], :as results}]
(let [row-maps (for [row rows]
(zipmap columns row))
- cols (resolve-sort-and-format-columns (:query query) (set columns))
+ cols (resolve-sort-and-format-columns (:query query) (set columns) (take 10 row-maps))
columns (mapv :name cols)]
(assoc results
:cols (vec (for [col cols]
diff --git a/src/metabase/query_processor/expand.clj b/src/metabase/query_processor/expand.clj
index a6a1d3c7bc8db4c8efebcfa0dd683ba1ab1b6191..97d22d5bc9a0a8d372d610c86f29b8393b6a6a32 100644
--- a/src/metabase/query_processor/expand.clj
+++ b/src/metabase/query_processor/expand.clj
@@ -2,25 +2,16 @@
"Converts a Query Dict as received by the API into an *expanded* one that contains extra information that will be needed to
construct the appropriate native Query, and perform various post-processing steps such as Field ordering."
(:refer-clojure :exclude [< <= > >= = != and or not filter count distinct sum min max + - / *])
- (:require [clojure
- [core :as core]
- [string :as str]]
+ (:require [clojure.core :as core]
[clojure.tools.logging :as log]
- [metabase.query-processor.interface :as i]
+ [metabase.query-processor
+ [interface :as i]
+ [util :as qputil]]
[metabase.util :as u]
[metabase.util.schema :as su]
[schema.core :as s])
- (:import [metabase.query_processor.interface AgFieldRef BetweenFilter ComparisonFilter CompoundFilter Expression ExpressionRef FieldPlaceholder RelativeDatetime StringFilter ValuePlaceholder]))
-
-;;; # ------------------------------------------------------------ Token dispatch ------------------------------------------------------------
-
-(s/defn ^:always-validate normalize-token :- s/Keyword
- "Convert a string or keyword in various cases (`lisp-case`, `snake_case`, or `SCREAMING_SNAKE_CASE`) to a lisp-cased keyword."
- [token :- su/KeywordOrString]
- (-> (name token)
- str/lower-case
- (str/replace #"_" "-")
- keyword))
+ (:import [metabase.query_processor.interface AgFieldRef BetweenFilter ComparisonFilter CompoundFilter Expression ExpressionRef
+ FieldPlaceholder RelativeDatetime StringFilter Value ValuePlaceholder]))
;;; # ------------------------------------------------------------ Clause Handlers ------------------------------------------------------------
@@ -38,7 +29,7 @@
[id :- su/IntGreaterThanZero]
(i/map->FieldPlaceholder {:field-id id}))
-(s/defn ^:private ^:always-validate field :- i/AnyFieldOrExpression
+(s/defn ^:private ^:always-validate field :- i/AnyField
"Generic reference to a `Field`. F can be an integer Field ID, or various other forms like `fk->` or `aggregation`."
[f]
(if (integer? f)
@@ -58,7 +49,7 @@
([f _ unit] (log/warn (u/format-color 'yellow (str "The syntax for datetime-field has changed in MBQL '98. [:datetime-field <field> :as <unit>] is deprecated. "
"Prefer [:datetime-field <field> <unit>] instead.")))
(datetime-field f unit))
- ([f unit] (assoc (field f) :datetime-unit (normalize-token unit))))
+ ([f unit] (assoc (field f) :datetime-unit (qputil/normalize-token unit))))
(s/defn ^:ql ^:always-validate fk-> :- FieldPlaceholder
"Reference to a `Field` that belongs to another `Table`. DEST-FIELD-ID is the ID of this Field, and FK-FIELD-ID is the ID of the foreign key field
@@ -72,11 +63,12 @@
(i/map->FieldPlaceholder {:fk-field-id fk-field-id, :field-id dest-field-id}))
-(s/defn ^:private ^:always-validate value :- ValuePlaceholder
+(s/defn ^:private ^:always-validate value :- (s/cond-pre Value ValuePlaceholder)
"Literal value. F is the `Field` it relates to, and V is `nil`, or a boolean, string, numerical, or datetime value."
[f v]
(cond
(instance? ValuePlaceholder v) v
+ (instance? Value v) v
:else (i/map->ValuePlaceholder {:field-placeholder (field f), :value v})))
(s/defn ^:private ^:always-validate field-or-value
@@ -94,11 +86,11 @@
(relative-datetime :current)
(relative-datetime -31 :day)"
- ([n] (s/validate (s/eq :current) (normalize-token n))
+ ([n] (s/validate (s/eq :current) (qputil/normalize-token n))
(relative-datetime 0 nil))
([n :- s/Int, unit] (i/map->RelativeDatetime {:amount n, :unit (if (nil? unit)
:day ; give :unit a default value so we can simplify the schema a bit and require a :unit
- (normalize-token unit))})))
+ (qputil/normalize-token unit))})))
(s/defn ^:ql ^:always-validate expression :- ExpressionRef
{:added "0.17.0"}
@@ -170,8 +162,8 @@
;; make sure the ag map is still typed correctly
(u/prog1 (cond
(:operator ag) (i/map->Expression ag)
- (:field ag) (i/map->AggregationWithField (update ag :aggregation-type normalize-token))
- :else (i/map->AggregationWithoutField (update ag :aggregation-type normalize-token)))
+ (:field ag) (i/map->AggregationWithField (update ag :aggregation-type qputil/normalize-token))
+ :else (i/map->AggregationWithoutField (update ag :aggregation-type qputil/normalize-token)))
(s/validate i/Aggregation <>)))))))
;; also handle varargs for convenience
@@ -288,7 +280,7 @@
(filter {} (time-interval (field-id 100) :current :day)) "
[f n unit]
(if-not (integer? n)
- (case (normalize-token n)
+ (case (qputil/normalize-token n)
:current (recur f 0 unit)
:last (recur f -1 unit)
:next (recur f 1 unit))
@@ -353,7 +345,7 @@
(map? subclause) subclause ; already parsed by `asc` or `desc`
(vector? subclause) (let [[f direction] subclause]
(log/warn (u/format-color 'yellow "The syntax for order-by has changed in MBQL '98. [<field> :ascending/:descending] is deprecated. Prefer [:asc/:desc <field>] instead."))
- (order-by-subclause (normalize-token direction) f))))
+ (order-by-subclause (qputil/normalize-token direction) f))))
(defn ^:ql order-by
"Specify how ordering should be done for this query.
@@ -432,7 +424,7 @@
(fn-for-token :starts-with) -> #'starts-with"
[token]
- (let [token (normalize-token token)]
+ (let [token (qputil/normalize-token token)]
(core/or (token->ql-fn token)
(throw (Exception. (str "Illegal clause (no matching fn found): " token))))))
@@ -506,4 +498,4 @@
(is-clause? :field-id [\"FIELD-ID\" 2000]) ; -> true"
[clause-keyword clause]
(core/and (sequential? clause)
- (core/= (normalize-token (first clause)) clause-keyword)))
+ (core/= (qputil/normalize-token (first clause)) clause-keyword)))
diff --git a/src/metabase/query_processor/interface.clj b/src/metabase/query_processor/interface.clj
index 05397e945ad9f5d7f28868ee2469e99540cca81a..c2aa5bee4fcdd20904c9fcf9a6d568332c1fe420 100644
--- a/src/metabase/query_processor/interface.clj
+++ b/src/metabase/query_processor/interface.clj
@@ -33,13 +33,21 @@
Not neccesarily bound when using various functions like `fk->` in the REPL."
nil)
+(defn driver-supports?
+ "Does the currently bound `*driver*` support FEATURE?
+ (This returns `nil` if `*driver*` is unbound. `*driver*` is always bound when running queries the normal way,
+ but may not be when calling this function directly from the REPL.)"
+ [feature]
+ (when *driver*
+ ((resolve 'metabase.driver/driver-supports?) *driver* feature)))
+
;; `assert-driver-supports` doesn't run check when `*driver*` is unbound (e.g., when used in the REPL)
;; Allows flexibility when composing queries for tests or interactive development
(defn assert-driver-supports
"When `*driver*` is bound, assert that is supports keyword FEATURE."
[feature]
(when *driver*
- (when-not (contains? ((resolve 'metabase.driver/features) *driver*) feature)
+ (when-not (driver-supports? feature)
(throw (Exception. (str (name feature) " is not supported by this driver."))))))
;; Expansion Happens in a Few Stages:
@@ -70,9 +78,11 @@
"Return a vector of name components of the form `[table-name parent-names... field-name]`"))
-;;; # ------------------------------------------------------------ "RESOLVED" TYPES: FIELD + VALUE ------------------------------------------------------------
+;;; +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
+;;; | FIELDS |
+;;; +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
-;; Field is the expansion of a Field ID in the standard QL
+;; Field is the "expanded" form of a Field ID (field reference) in MBQL
(s/defrecord Field [field-id :- su/IntGreaterThanZero
field-name :- su/NonBlankString
field-display-name :- su/NonBlankString
@@ -98,6 +108,7 @@
[table-name])
field-name)))
+;;; DateTimeField
(def ^:const datetime-field-units
"Valid units for a `DateTimeField`."
@@ -122,7 +133,7 @@
(contains? relative-datetime-value-units (keyword unit)))
-;; wrapper around Field
+;; DateTimeField is just a simple wrapper around Field
(s/defrecord DateTimeField [field :- Field
unit :- DatetimeFieldUnit]
clojure.lang.Named
@@ -136,78 +147,53 @@
[nil expression-name]))
-;; Value is the expansion of a value within a QL clause
-;; Information about the associated Field is included for convenience
-(s/defrecord Value [value :- (s/maybe (s/cond-pre s/Bool s/Num su/NonBlankString))
- field :- (s/named (s/cond-pre Field ExpressionRef) ; TODO - Value doesn't need the whole field, just the relevant type info / units
- "field or expression reference")])
-
-;; e.g. an absolute point in time (literal)
-(s/defrecord DateTimeValue [value :- Timestamp
- field :- DateTimeField])
-
-(s/defrecord RelativeDateTimeValue [amount :- s/Int
- unit :- DatetimeValueUnit
- field :- DateTimeField])
-
-(defprotocol ^:private IDateTimeValue
- (unit [this]
- "Get the `unit` associated with a `DateTimeValue` or `RelativeDateTimeValue`.")
-
- (add-date-time-units [this n]
- "Return a new `DateTimeValue` or `RelativeDateTimeValue` with N `units` added to it."))
-
-(extend-protocol IDateTimeValue
- DateTimeValue
- (unit [this] (:unit (:field this)))
- (add-date-time-units [this n] (assoc this :value (u/relative-date (unit this) n (:value this))))
-
- RelativeDateTimeValue
- (unit [this] (:unit this))
- (add-date-time-units [this n] (update this :amount (partial + n))))
-
-
-;;; # ------------------------------------------------------------ PLACEHOLDER TYPES: FIELDPLACEHOLDER + VALUEPLACEHOLDER ------------------------------------------------------------
+;;; Placeholder Types
;; Replace Field IDs with these during first pass
(s/defrecord FieldPlaceholder [field-id :- su/IntGreaterThanZero
fk-field-id :- (s/maybe (s/constrained su/IntGreaterThanZero
- (fn [_] (or (assert-driver-supports :foreign-keys) true))
- "foreign-keys is not supported by this driver."))
+ (fn [_] (or (assert-driver-supports :foreign-keys) true)) ; assert-driver-supports will throw Exception if driver is bound
+ "foreign-keys is not supported by this driver.")) ; and driver does not support foreign keys
datetime-unit :- (s/maybe (apply s/enum datetime-field-units))])
(s/defrecord AgFieldRef [index :- s/Int])
-
;; TODO - add a method to get matching expression from the query?
-(def FieldPlaceholderOrAgRef
- "Schema for either a `FieldPlaceholder` or `AgFieldRef`."
- (s/named (s/cond-pre FieldPlaceholder AgFieldRef) "Valid field (not a field ID or aggregate field reference)"))
+
+
(def FieldPlaceholderOrExpressionRef
"Schema for either a `FieldPlaceholder` or `ExpressionRef`."
(s/named (s/cond-pre FieldPlaceholder ExpressionRef)
"Valid field or expression reference."))
-
(s/defrecord RelativeDatetime [amount :- s/Int
unit :- DatetimeValueUnit])
-
-(declare RValue Aggregation)
+(declare Aggregation AnyField AnyValueLiteral)
(def ^:private ExpressionOperator (s/named (s/enum :+ :- :* :/) "Valid expression operator"))
(s/defrecord Expression [operator :- ExpressionOperator
- args :- [(s/cond-pre (s/recursive #'RValue)
+ args :- [(s/cond-pre (s/recursive #'AnyValueLiteral)
+ (s/recursive #'AnyField)
(s/recursive #'Aggregation))]
custom-name :- (s/maybe su/NonBlankString)])
-(def AnyFieldOrExpression
- "Schema for a `FieldPlaceholder`, `AgRef`, or `Expression`."
- (s/named (s/cond-pre ExpressionRef Expression FieldPlaceholderOrAgRef)
- "Valid field, ag field reference, expression, or expression reference."))
+(def AnyField
+ "Schema for a anything that is considered a valid 'field'."
+ (s/named (s/cond-pre Field
+ FieldPlaceholder
+ AgFieldRef
+ Expression
+ ExpressionRef)
+ "AnyField: field, ag field reference, expression, expression reference, or field literal."))
+
+
+;;; +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
+;;; | VALUES |
+;;; +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
(def LiteralDatetimeString
"Schema for an MBQL datetime string literal, in ISO-8601 format."
@@ -225,14 +211,48 @@
"Schema for something that is orderable value in MBQL (either a number or datetime)."
(s/named (s/cond-pre s/Num Datetime) "Valid orderable value (must be number or datetime)"))
-(def AnyValue
- "Schema for anything that is a considered a valid value in MBQL - `nil`, a `Boolean`, `Number`, `String`, or relative datetime form."
+(def AnyValueLiteral
+ "Schema for anything that is a considered a valid value literal in MBQL - `nil`, a `Boolean`, `Number`, `String`, or relative datetime form."
(s/named (s/maybe (s/cond-pre s/Bool su/NonBlankString OrderableValue)) "Valid value (must be nil, boolean, number, string, or a relative-datetime form)"))
+
+;; Value is the expansion of a value within a QL clause
+;; Information about the associated Field is included for convenience
+;; TODO - Value doesn't need the whole field, just the relevant type info / units
+(s/defrecord Value [value :- AnyValueLiteral
+ field :- (s/recursive #'AnyField)])
+
+;; e.g. an absolute point in time (literal)
+(s/defrecord DateTimeValue [value :- Timestamp
+ field :- DateTimeField])
+
+(s/defrecord RelativeDateTimeValue [amount :- s/Int
+ unit :- DatetimeValueUnit
+ field :- DateTimeField])
+
+(defprotocol ^:private IDateTimeValue
+ (unit [this]
+ "Get the `unit` associated with a `DateTimeValue` or `RelativeDateTimeValue`.")
+
+ (add-date-time-units [this n]
+ "Return a new `DateTimeValue` or `RelativeDateTimeValue` with N `units` added to it."))
+
+(extend-protocol IDateTimeValue
+ DateTimeValue
+ (unit [this] (:unit (:field this)))
+ (add-date-time-units [this n] (assoc this :value (u/relative-date (unit this) n (:value this))))
+
+ RelativeDateTimeValue
+ (unit [this] (:unit this))
+ (add-date-time-units [this n] (update this :amount (partial + n))))
+
+
+;;; Placeholder Types
+
;; Replace values with these during first pass over Query.
;; Include associated Field ID so appropriate the info can be found during Field resolution
(s/defrecord ValuePlaceholder [field-placeholder :- FieldPlaceholderOrExpressionRef
- value :- AnyValue])
+ value :- AnyValueLiteral])
(def OrderableValuePlaceholder
"`ValuePlaceholder` schema with the additional constraint that the value be orderable (a number or datetime)."
@@ -242,21 +262,16 @@
"`ValuePlaceholder` schema with the additional constraint that the value be a string/"
(s/constrained ValuePlaceholder (comp string? :value) ":value must be a string"))
-(def FieldOrAnyValue
- "Schema that accepts either a `FieldPlaceholder` or `ValuePlaceholder`."
- (s/named (s/cond-pre FieldPlaceholder ValuePlaceholder) "Field or value"))
-
-;; (def FieldOrOrderableValue (s/named (s/cond-pre FieldPlaceholder OrderableValuePlaceholder) "Field or orderable value (number or datetime)"))
-;; (def FieldOrStringValue (s/named (s/cond-pre FieldPlaceholder StringValuePlaceholder) "Field or string literal"))
+(def AnyFieldOrValue
+ "Schema that accepts anything normally considered a field (including expressions and literals) *or* a value or value placehoder."
+ (s/named (s/cond-pre AnyField Value ValuePlaceholder) "Field or value"))
-(def RValue
- "Schema for anything that can be an [RValue](https://github.com/metabase/metabase/wiki/Query-Language-'98#rvalues) -
- a `Field`, `Value`, or `Expression`."
- (s/named (s/cond-pre AnyValue FieldPlaceholderOrExpressionRef Expression)
- "RValue"))
+;;; +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
+;;; | CLAUSES |
+;;; +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
-;;; # ------------------------------------------------------------ CLAUSE SCHEMAS ------------------------------------------------------------
+;;; aggregation
(s/defrecord AggregationWithoutField [aggregation-type :- (s/named (s/enum :count :cumulative-count)
"Valid aggregation type")
@@ -281,9 +296,11 @@
"standard-deviation-aggregations is not supported by this driver."))
+;;; filter
+
(s/defrecord EqualityFilter [filter-type :- (s/enum := :!=)
field :- FieldPlaceholderOrExpressionRef
- value :- FieldOrAnyValue])
+ value :- AnyFieldOrValue])
(s/defrecord ComparisonFilter [filter-type :- (s/enum :< :<= :> :>=)
field :- FieldPlaceholderOrExpressionRef
@@ -316,28 +333,38 @@
(s/named (s/cond-pre SimpleFilterClause NotFilter CompoundFilter)
"Valid filter clause"))
+
+;;; order-by
+
(def OrderByDirection
"Schema for the direction in an `OrderBy` subclause."
(s/named (s/enum :ascending :descending) "Valid order-by direction"))
(def OrderBy
"Schema for top-level `order-by` clause in an MBQL query."
- (s/named {:field AnyFieldOrExpression
+ (s/named {:field AnyField
:direction OrderByDirection}
"Valid order-by subclause"))
+;;; page
+
(def Page
"Schema for the top-level `page` clause in a MBQL query."
(s/named {:page su/IntGreaterThanZero
:items su/IntGreaterThanZero}
"Valid page clause"))
+
+;;; +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
+;;; | QUERY |
+;;; +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
+
(def Query
"Schema for an MBQL query."
{(s/optional-key :aggregation) [Aggregation]
(s/optional-key :breakout) [FieldPlaceholderOrExpressionRef]
- (s/optional-key :fields) [AnyFieldOrExpression]
+ (s/optional-key :fields) [AnyField]
(s/optional-key :filter) Filter
(s/optional-key :limit) su/IntGreaterThanZero
(s/optional-key :order-by) [OrderBy]
diff --git a/src/metabase/query_processor/middleware/permissions.clj b/src/metabase/query_processor/middleware/permissions.clj
index e4c32e1b8c43c9c93e4394fe08eb2f360bc8dbf2..61cccf4a5d02f95d50b7c372c3ff112890ecbf22 100644
--- a/src/metabase/query_processor/middleware/permissions.clj
+++ b/src/metabase/query_processor/middleware/permissions.clj
@@ -5,12 +5,14 @@
[metabase.util :as u]))
(defn- check-query-permissions* [query]
- ;; TODO - should we do anything if there is no *current-user-id* (for something like a pulse?)
(u/prog1 query
(when *current-user-id*
(perms/check-query-permissions *current-user-id* query))))
(defn check-query-permissions
- "Middleware that check that the current user has permissions to run the current query."
+ "Middleware that check that the current user has permissions to run the current query.
+ This only applies if `*current-user-id*` is bound. In other cases, like when running
+ public Cards or sending pulses, permissions need to be checked separately before allowing
+ the relevant objects to be create (e.g., when saving a new Pulse or 'publishing' a Card)."
[qp]
(comp qp check-query-permissions*))
diff --git a/src/metabase/query_processor/permissions.clj b/src/metabase/query_processor/permissions.clj
index 5dd16462d78d2b1b618fb25cbe38cd50545cd68c..bbf2a888d435edb628c84e99f82c124eb27717b0 100644
--- a/src/metabase/query_processor/permissions.clj
+++ b/src/metabase/query_processor/permissions.clj
@@ -62,6 +62,7 @@
(defn- ^:deprecated table-id [source-or-join-table]
+ {:post [(integer? %)]}
(or (:id source-or-join-table)
(:table-id source-or-join-table)))
@@ -75,6 +76,7 @@
(or (user-can-run-query-referencing-table? user-id (table-id table))
(throw-permissions-exception "You do not have permissions to run queries referencing table '%s'." (table-identifier table))))
+;; TODO - why is this the only function here that takes `user-id`?
(defn- throw-if-cannot-run-query
"Throw an exception if USER-ID doesn't have permissions to run QUERY."
[user-id {:keys [source-table join-tables]}]
@@ -118,8 +120,8 @@
(defn check-query-permissions
"Check that User with USER-ID has permissions to run QUERY, or throw an exception."
- [user-id {query-type :type, database :database, query :query, {card-id :card-id} :info}]
- {:pre [(integer? user-id)]}
+ [user-id {query-type :type, database :database, query :query, {card-id :card-id} :info, :as outer-query}]
+ {:pre [(integer? user-id) (map? outer-query)]}
(let [native? (= (keyword query-type) :native)
collection-id (db/select-one-field :collection_id 'Card :id card-id)]
(cond
diff --git a/src/metabase/query_processor/resolve.clj b/src/metabase/query_processor/resolve.clj
index 1d1747d15df622cbecd0e6ae2281097218a87285..33fcc695de7d22f22f57002f615b6123898b679d 100644
--- a/src/metabase/query_processor/resolve.clj
+++ b/src/metabase/query_processor/resolve.clj
@@ -194,7 +194,7 @@
;; Otherwise fetch + resolve the Fields in question
(let [fields (->> (u/key-by :id (db/select [field/Field :name :display_name :base_type :special_type :visibility_type :table_id :parent_id :description :id]
:visibility_type [:not= "sensitive"]
- :id [:in field-ids]))
+ :id [:in field-ids]))
(m/map-vals rename-mb-field-keys)
(m/map-vals #(assoc % :parent (when-let [parent-id (:parent-id %)]
(i/map->FieldPlaceholder {:field-id parent-id})))))]
@@ -207,7 +207,11 @@
;; Recurse in case any new (nested) unresolved fields were found.
(recur (dec max-iterations))))))))
-(defn- fk-field-ids->info [source-table-id fk-field-ids]
+(defn- fk-field-ids->info
+ "Given a SOURCE-TABLE-ID and collection of FK-FIELD-IDS, return a sequence of maps containing IDs and identifiers for those FK fields and their target tables and fields.
+ FK-FIELD-IDS are IDs of fields that belong to the source table. For example, SOURCE-TABLE-ID might be 'checkins' and FK-FIELD-IDS might have the IDs for 'checkins.user_id'
+ and the like."
+ [source-table-id fk-field-ids]
(when (seq fk-field-ids)
(db/query {:select [[:source-fk.name :source-field-name]
[:source-fk.id :source-field-id]
@@ -219,8 +223,8 @@
:from [[field/Field :source-fk]]
:left-join [[field/Field :target-pk] [:= :source-fk.fk_target_field_id :target-pk.id]
[Table :target-table] [:= :target-pk.table_id :target-table.id]]
- :where [:and [:in :source-fk.id (set fk-field-ids)]
- [:= :source-fk.table_id source-table-id]
+ :where [:and [:in :source-fk.id (set fk-field-ids)]
+ [:= :source-fk.table_id source-table-id]
(mdb/isa :source-fk.special_type :type/FK)]})))
(defn- fk-field-ids->joined-tables
diff --git a/src/metabase/query_processor/util.clj b/src/metabase/query_processor/util.clj
index 47f80946e2abff02fbc651b1287212716bfc57de..304527c8b2991153b26c81ec1ef879541da647d3 100644
--- a/src/metabase/query_processor/util.clj
+++ b/src/metabase/query_processor/util.clj
@@ -3,7 +3,11 @@
(:require [buddy.core
[codecs :as codecs]
[hash :as hash]]
- [cheshire.core :as json]))
+ [cheshire.core :as json]
+ [clojure.string :as str]
+ [metabase.util :as u]
+ [metabase.util.schema :as su]
+ [schema.core :as s]))
(defn mbql-query?
"Is the given query an MBQL query?"
@@ -33,6 +37,69 @@
(format ":: userID: %s queryType: %s queryHash: %s" executed-by query-type (codecs/bytes->hex query-hash)))))
+;;; ------------------------------------------------------------ Normalization ------------------------------------------------------------
+
+;; The following functions make it easier to deal with MBQL queries, which are case-insensitive, string/keyword insensitive, and underscore/hyphen insensitive.
+;; These should be preferred instead of assuming the frontend will always pass in clauses the same way, since different variation are all legal under MBQL '98.
+
+;; TODO - In the future it might make sense to simply walk the entire query and normalize the whole thing when it comes in. I've tried implementing middleware
+;; to do that but it ended up breaking a few things that wrongly assume different clauses will always use a certain case (e.g. SQL `:template_tags`). Fixing
+;; all of that is out-of-scope for the nested queries PR but should possibly be revisited in the future.
+
+(s/defn ^:always-validate normalize-token :- s/Keyword
+ "Convert a string or keyword in various cases (`lisp-case`, `snake_case`, or `SCREAMING_SNAKE_CASE`) to a lisp-cased keyword."
+ [token :- su/KeywordOrString]
+ (-> (name token)
+ str/lower-case
+ (str/replace #"_" "-")
+ keyword))
+
+(defn get-normalized
+ "Get the value for normalized key K in map M, regardless of how the key was specified in M,
+ whether string or keyword, lisp-case, snake_case, or SCREAMING_SNAKE_CASE.
+
+ (get-normalized {\"NUM_TOUCANS\" 2} :num-toucans) ; -> 2"
+ ([m k]
+ {:pre [(or (u/maybe? map? m)
+ (println "Not a map:" m))]}
+ (let [k (normalize-token k)]
+ (some (fn [[map-k v]]
+ (when (= k (normalize-token map-k))
+ v))
+ m)))
+ ([m k not-found]
+ (or (get-normalized m k)
+ not-found)))
+
+(defn get-in-normalized
+ "Like `get-normalized`, but accepts a sequence of keys KS, like `get-in`.
+
+ (get-in-normalized {\"NUM_BIRDS\" {\"TOUCANS\" 2}} [:num-birds :toucans]) ; -> 2"
+ ([m ks]
+ {:pre [(u/maybe? sequential? ks)]}
+ (loop [m m, [k & more] ks]
+ (if-not k
+ m
+ (recur (get-normalized m k) more))))
+ ([m ks not-found]
+ (or (get-in-normalized m ks)
+ not-found)))
+
+(defn dissoc-normalized
+ "Remove all matching keys from map M regardless of case, string/keyword, or hypens/underscores.
+
+ (dissoc-normalized {\"NUM_TOUCANS\" 3} :num-toucans) ; -> {}"
+ [m k]
+ {:pre [(or (u/maybe? map? m)
+ (println "Not a map:" m))]}
+ (let [k (normalize-token k)]
+ (loop [m m, [map-k & more, :as ks] (keys m)]
+ (cond
+ (not (seq ks)) m
+ (= k (normalize-token map-k)) (recur (dissoc m map-k) more)
+ :else (recur m more)))))
+
+
;;; ------------------------------------------------------------ Hashing ------------------------------------------------------------
(defn- select-keys-for-hashing
diff --git a/src/metabase/sync_database/analyze.clj b/src/metabase/sync_database/analyze.clj
index 87985f8b62283d70ebda6a42180d0b3e6a0fdefa..8881ba24d65522e8bac4223f19a0b242d2bd43fa 100644
--- a/src/metabase/sync_database/analyze.clj
+++ b/src/metabase/sync_database/analyze.clj
@@ -44,7 +44,7 @@
(try
(queries/table-row-count table)
(catch Throwable e
- (log/error (u/format-color 'red "Unable to determine row count for '%s': %s\n%s" (:name table) (.getMessage e) (u/pprint-to-str (u/filtered-stacktrace e)))))))
+ (log/warn (u/format-color 'red "Unable to determine row count for '%s': %s\n%s" (:name table) (.getMessage e) (u/pprint-to-str (u/filtered-stacktrace e)))))))
(defn test-for-cardinality?
"Should FIELD should be tested for cardinality?"
diff --git a/src/metabase/util/honeysql_extensions.clj b/src/metabase/util/honeysql_extensions.clj
index 70bfe6de28eaa0af1de1aae991c085fa5f54f685..2b08b2a006c3e7af4f63ae30e15d199a49183d5a 100644
--- a/src/metabase/util/honeysql_extensions.clj
+++ b/src/metabase/util/honeysql_extensions.clj
@@ -84,6 +84,7 @@
(rest sql-string-or-vector))))))
+;; Single-quoted string literal
(defrecord Literal [literal]
ToSql
(to-sql [_]
diff --git a/test/metabase/http_client.clj b/test/metabase/http_client.clj
index 8324c12a618f39b3f284c2908aab2b54df5e6561..b16790230d69800d76db42b769cca561eeb139bc 100644
--- a/test/metabase/http_client.clj
+++ b/test/metabase/http_client.clj
@@ -67,7 +67,6 @@
or throw an Exception if that fails."
[{:keys [email password], :as credentials}]
{:pre [(string? email) (string? password)]}
- (println "Authenticating" email) ; DEBUG
(try
(:id (client :post 200 "session" credentials))
(catch Throwable e
diff --git a/test/metabase/models/card_test.clj b/test/metabase/models/card_test.clj
index 3f6cb57eddcb316acca67407fbcd5ade8f251864..0d7fc226e149a9d3122a0b6293c5e14053d1a114 100644
--- a/test/metabase/models/card_test.clj
+++ b/test/metabase/models/card_test.clj
@@ -9,8 +9,8 @@
[permissions :as perms]]
[metabase.query-processor.expand :as ql]
[metabase.test
- [data :refer [id]]
- [util :as tu :refer [random-name]]]
+ [data :as data]
+ [util :as tu]]
[metabase.test.data.users :refer :all]
[metabase.util :as u]
[toucan.db :as db]
@@ -26,9 +26,9 @@
(let [get-dashboard-count (fn [] (dashboard-count (Card card-id)))]
[(get-dashboard-count)
- (do (db/insert! DashboardCard :card_id card-id, :dashboard_id (:id (create-dash! (random-name))), :parameter_mappings [])
+ (do (db/insert! DashboardCard :card_id card-id, :dashboard_id (:id (create-dash! (tu/random-name))), :parameter_mappings [])
(get-dashboard-count))
- (do (db/insert! DashboardCard :card_id card-id, :dashboard_id (:id (create-dash! (random-name))), :parameter_mappings [])
+ (do (db/insert! DashboardCard :card_id card-id, :dashboard_id (:id (create-dash! (tu/random-name))), :parameter_mappings [])
(get-dashboard-count))])))
@@ -60,25 +60,25 @@
(expect
false
- (tt/with-temp Card [card {:dataset_query {:database (id), :type "native"}}]
+ (tt/with-temp Card [card {:dataset_query {:database (data/id), :type "native"}}]
(binding [*current-user-permissions-set* (delay #{})]
(mi/can-read? card))))
(expect
- (tt/with-temp Card [card {:dataset_query {:database (id), :type "native"}}]
- (binding [*current-user-permissions-set* (delay #{(perms/native-read-path (id))})]
+ (tt/with-temp Card [card {:dataset_query {:database (data/id), :type "native"}}]
+ (binding [*current-user-permissions-set* (delay #{(perms/native-read-path (data/id))})]
(mi/can-read? card))))
;; in order to *write* a native card user should need native readwrite access
(expect
false
- (tt/with-temp Card [card {:dataset_query {:database (id), :type "native"}}]
- (binding [*current-user-permissions-set* (delay #{(perms/native-read-path (id))})]
+ (tt/with-temp Card [card {:dataset_query {:database (data/id), :type "native"}}]
+ (binding [*current-user-permissions-set* (delay #{(perms/native-read-path (data/id))})]
(mi/can-write? card))))
(expect
- (tt/with-temp Card [card {:dataset_query {:database (id), :type "native"}}]
- (binding [*current-user-permissions-set* (delay #{(perms/native-readwrite-path (id))})]
+ (tt/with-temp Card [card {:dataset_query {:database (data/id), :type "native"}}]
+ (binding [*current-user-permissions-set* (delay #{(perms/native-readwrite-path (data/id))})]
(mi/can-write? card))))
@@ -100,24 +100,24 @@
(defn- mbql [query]
- {:database (id)
+ {:database (data/id)
:type :query
:query query})
;; MBQL w/o JOIN
(expect
- #{(perms/object-path (id) "PUBLIC" (id :venues))}
+ #{(perms/object-path (data/id) "PUBLIC" (data/id :venues))}
(query-perms-set (mbql (ql/query
- (ql/source-table (id :venues))))
+ (ql/source-table (data/id :venues))))
:read))
;; MBQL w/ JOIN
(expect
- #{(perms/object-path (id) "PUBLIC" (id :checkins))
- (perms/object-path (id) "PUBLIC" (id :venues))}
+ #{(perms/object-path (data/id) "PUBLIC" (data/id :checkins))
+ (perms/object-path (data/id) "PUBLIC" (data/id :venues))}
(query-perms-set (mbql (ql/query
- (ql/source-table (id :checkins))
- (ql/order-by (ql/asc (ql/fk-> (id :checkins :venue_id) (id :venues :name))))))
+ (ql/source-table (data/id :checkins))
+ (ql/order-by (ql/asc (ql/fk-> (data/id :checkins :venue_id) (data/id :venues :name))))))
:read))
;; invalid/legacy card should return perms for something that doesn't exist so no one gets to see it
diff --git a/test/metabase/permissions_collection_test.clj b/test/metabase/permissions_collection_test.clj
index fec9b5777ecfe7888f3d879046745e689f52f1dc..77c78293e23804bbfd6c6fa94c9239d5f68902e5 100644
--- a/test/metabase/permissions_collection_test.clj
+++ b/test/metabase/permissions_collection_test.clj
@@ -18,9 +18,6 @@
;; but not Rasta (all-users)
(defn- api-call-was-successful? {:style/indent 0} [response]
- (when (and (string? response)
- (not= response "You don't have permissions to do that."))
- (println "RESPONSE:" response)) ; DEBUG
(and (not= response "You don't have permissions to do that.")
(not= response "Unauthenticated")))
diff --git a/test/metabase/permissions_test.clj b/test/metabase/permissions_test.clj
index 43262069da6ce9d416f0daa4a6bfb8b31ab421aa..5171091191b20c4c4f8b67275217dc3a378c1c1d 100644
--- a/test/metabase/permissions_test.clj
+++ b/test/metabase/permissions_test.clj
@@ -129,7 +129,7 @@
:table_id (u/get-id table)
:dataset_query {:database (u/get-id db)
:type "native"
- :query (format "SELECT count(*) FROM \"%s\";" (:name table))}}))
+ :native {:query (format "SELECT count(*) FROM \"%s\";" (:name table))}}}))
(def ^:dynamic *card:db1-count-of-venues*)
diff --git a/test/metabase/query_processor/util_test.clj b/test/metabase/query_processor/util_test.clj
index 357583558a259029c48d0101d354c459d7c69542..eea0cdd83e2330b1a3097b7e79db11fd24f8ab31 100644
--- a/test/metabase/query_processor/util_test.clj
+++ b/test/metabase/query_processor/util_test.clj
@@ -105,3 +105,53 @@
(qputil/query-hash {:database 2
:type "native"
:native {:query "SELECT pg_sleep(15), 2 AS two"}})))
+
+
+;;; ------------------------------------------------------------ Tests for get-normalized and get-in-normalized ------------------------------------------------------------
+
+(expect 2 (qputil/get-normalized {"num_toucans" 2} :num-toucans))
+(expect 2 (qputil/get-normalized {"NUM_TOUCANS" 2} :num-toucans))
+(expect 2 (qputil/get-normalized {"num-toucans" 2} :num-toucans))
+(expect 2 (qputil/get-normalized {:num_toucans 2} :num-toucans))
+(expect 2 (qputil/get-normalized {:NUM_TOUCANS 2} :num-toucans))
+(expect 2 (qputil/get-normalized {:num-toucans 2} :num-toucans))
+
+(expect
+ nil
+ (qputil/get-normalized nil :num-toucans))
+
+(expect 2 (qputil/get-in-normalized {"BIRDS" {"NUM_TOUCANS" 2}} [:birds :num-toucans]))
+(expect 2 (qputil/get-in-normalized {"birds" {"num_toucans" 2}} [:birds :num-toucans]))
+(expect 2 (qputil/get-in-normalized {"birds" {"num-toucans" 2}} [:birds :num-toucans]))
+(expect 2 (qputil/get-in-normalized {:BIRDS {:NUM_TOUCANS 2}} [:birds :num-toucans]))
+(expect 2 (qputil/get-in-normalized {:birds {:num_toucans 2}} [:birds :num-toucans]))
+(expect 2 (qputil/get-in-normalized {:birds {:num-toucans 2}} [:birds :num-toucans]))
+
+(expect
+ 2
+ (qputil/get-in-normalized {:num-toucans 2} [:num-toucans]))
+
+(expect
+ nil
+ (qputil/get-in-normalized nil [:birds :num-toucans]))
+
+(expect
+ 10
+ (qputil/get-in-normalized
+ {"dataset_query" {"query" {"source_table" 10}}}
+ [:dataset-query :query :source-table]))
+
+(expect {} (qputil/dissoc-normalized {"NUM_TOUCANS" 3} :num-toucans))
+(expect {} (qputil/dissoc-normalized {"num_toucans" 3} :num-toucans))
+(expect {} (qputil/dissoc-normalized {"num-toucans" 3} :num-toucans))
+(expect {} (qputil/dissoc-normalized {:NUM_TOUCANS 3} :num-toucans))
+(expect {} (qputil/dissoc-normalized {:num_toucans 3} :num-toucans))
+(expect {} (qputil/dissoc-normalized {:num-toucans 3} :num-toucans))
+
+(expect
+ {}
+ (qputil/dissoc-normalized {:num-toucans 3, "NUM_TOUCANS" 3, "num_toucans" 3} :num-toucans))
+
+(expect
+ nil
+ (qputil/dissoc-normalized nil :num-toucans))
diff --git a/test/metabase/test/data.clj b/test/metabase/test/data.clj
index 83606ed5b3989ead19d4db53ab52ebc458300ecf..867e0ea04402d8e9ccab401ccd09c2d99275ea7a 100644
--- a/test/metabase/test/data.clj
+++ b/test/metabase/test/data.clj
@@ -120,7 +120,12 @@
`(run-query* (query ~table ~@forms)))
-(defn format-name [nm]
+(defn format-name
+ "Format a SQL schema, table, or field identifier in the correct way for the current database by calling the
+ driver's implementation of `format-name`.
+ (Most databases use the default implementation of `identity`; H2 uses `clojure.string/upper-case`.)
+ This function DOES NOT quote the identifier."
+ [nm]
(i/format-name *driver* (name nm)))
(defn- get-table-id-or-explode [db-id table-name]
diff --git a/test/metabase/test/data/users.clj b/test/metabase/test/data/users.clj
index 773f80d5f9641d901d2acc7c3d60c54b7e246f9e..a1415e76d1298cdd4ff7b1e0eafd169f4de90d9d 100644
--- a/test/metabase/test/data/users.clj
+++ b/test/metabase/test/data/users.clj
@@ -70,7 +70,6 @@
{:pre [(string? email) (string? first) (string? last) (string? password) (m/boolean? superuser) (m/boolean? active)]}
(wait-for-initiailization)
(or (User :email email)
- (println "Creating test user:" email) ; DEBUG
(db/insert! User
:email email
:first_name first
@@ -139,7 +138,6 @@
(when-not (= status-code 401)
(throw e))
;; If we got a 401 unauthenticated clear the tokens cache + recur
- (printf "Got 401 (Unauthenticated) for %s. Clearing cached auth tokens and retrying request.\n" username) ; DEBUG
(reset! tokens {})
(apply client-fn username args)))))
diff --git a/test_resources/log4j.properties b/test_resources/log4j.properties
index 993488ed722d55bcad76dbc98f3f8c197a2989a4..161945c47bd0cc0b9aece51e130d6ff59e51c407 100644
--- a/test_resources/log4j.properties
+++ b/test_resources/log4j.properties
@@ -16,8 +16,8 @@ log4j.appender.file.layout.ConversionPattern=%d [%t] %-5p%c - %m%n
# customizations to logging by package
log4j.logger.com.mchange=ERROR
+log4j.logger.org.eclipse.jetty.server.HttpChannel=ERROR
log4j.logger.metabase=ERROR
-log4j.logger.metabase.middleware=INFO
log4j.logger.metabase.test-setup=INFO
log4j.logger.metabase.test.data.datasets=INFO
log4j.logger.metabase.util.encryption=INFO