diff --git a/src/metabase/api/dataset.clj b/src/metabase/api/dataset.clj index 196d61793d318a3102c5e79b0e433dfbee3c831a..c13db9ccfd54c3881738d1421c8ded44420f0b6c 100644 --- a/src/metabase/api/dataset.clj +++ b/src/metabase/api/dataset.clj @@ -42,8 +42,10 @@ (let [source-card-id (query->source-card-id query)] (api/cancellable-json-response (fn [] - (qp/process-query-and-save-with-max! query {:executed-by api/*current-user-id*, :context :ad-hoc, - :card-id source-card-id, :nested? (boolean source-card-id)}))))) + (qp/process-query-and-save-with-max! + query + {:executed-by api/*current-user-id*, :context :ad-hoc, + :card-id source-card-id, :nested? (boolean source-card-id)}))))) ;;; ----------------------------------- Downloading Query Results in Other Formats ----------------------------------- diff --git a/src/metabase/api/field.clj b/src/metabase/api/field.clj index f0a9768a72fde7b33f4c0c46fdc2f508ea11d2af..634e9bdfe7378eff65360da70fb93b303d6fcaa7 100644 --- a/src/metabase/api/field.clj +++ b/src/metabase/api/field.clj @@ -1,5 +1,9 @@ (ns metabase.api.field (:require [compojure.core :refer [DELETE GET POST PUT]] + [metabase + [query-processor :as qp] + [related :as related] + [util :as u]] [metabase.api.common :as api] [metabase.db.metadata-queries :as metadata] [metabase.models @@ -7,9 +11,6 @@ [field :as field :refer [Field]] [field-values :as field-values :refer [FieldValues]] [table :refer [Table]]] - [metabase.query-processor :as qp] - [metabase.related :as related] - [metabase.util :as u] [metabase.util.schema :as su] [schema.core :as s] [toucan diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj index 395f2fc15e50c5f8c55eac3c67f0307c1f13e7c9..8c7052f8990b4fb6b2e4013fb0c27332ff5cd3b2 100644 --- a/src/metabase/driver/generic_sql/query_processor.clj +++ b/src/metabase/driver/generic_sql/query_processor.clj @@ -34,7 +34,7 @@ "The outer query currently being processed." nil) -(def ^:private ^:dynamic *nested-query-level* +(def ^:dynamic *nested-query-level* "How many levels deep are we into nested queries? (0 = top level.) We keep track of this so we know what level to find referenced aggregations (otherwise something like [:aggregation 0] could be ambiguous in a nested query). Each nested query increments this counter by 1." diff --git a/src/metabase/email/messages.clj b/src/metabase/email/messages.clj index 59c746b67cfe93b8e54eab1f34dc7de6158fb2eb..b4a74f38b52d62d14efe5d3ca3c7c3e70d28db1d 100644 --- a/src/metabase/email/messages.clj +++ b/src/metabase/email/messages.clj @@ -23,6 +23,8 @@ [toucan.db :as db]) (:import [java.io File IOException])) +(alter-meta! #'stencil.core/render-file assoc :style/indent 1) + ;; Dev only -- disable template caching (when config/is-dev? (stencil-loader/set-cache (cache/ttl-cache-factory {} :ttl 0))) @@ -47,7 +49,7 @@ :link "https://www.metabase.com/feedback/inactive"}) (defn- follow-up-context [] - {:heading (str (trs "We hope you've been enjoying Metabase.")) + {:heading (str (trs "We hope you''ve been enjoying Metabase.")) :callToAction (str (trs "Would you mind taking a fast 6 question survey to tell us how it’s going?")) :link "https://www.metabase.com/feedback/active"}) @@ -90,10 +92,9 @@ {:pre [(map? new-user)]} (let [recipients (all-admin-recipients)] (email/send-message! - :subject (format (if google-auth? - "%s created a Metabase account" - "%s accepted their Metabase invite") - (:common_name new-user)) + :subject (str (if google-auth? + (trs "{0} created a Metabase account" (:common_name new-user)) + (trs "{0} accepted their Metabase invite" (:common_name new-user)))) :recipients recipients :message-type :html :message (stencil/render-file "metabase/email/user_joined_notification" @@ -121,7 +122,7 @@ :passwordResetUrl password-reset-url :logoHeader true})] (email/send-message! - :subject "[Metabase] Password Reset Request" + :subject (str (trs "[Metabase] Password Reset Request")) :recipients [email] :message-type :html :message message-body))) @@ -160,7 +161,7 @@ (random-quote-context)) message-body (stencil/render-file "metabase/email/notification" context)] (email/send-message! - :subject "[Metabase] Notification" + :subject (str (trs "[Metabase] Notification")) :recipients [email] :message-type :html :message message-body))) @@ -169,9 +170,9 @@ "Format and send an email to the system admin following up on the installation." [email msg-type] {:pre [(u/email? email) (contains? #{"abandon" "follow-up"} msg-type)]} - (let [subject (if (= "abandon" msg-type) - "[Metabase] Help make Metabase better." - "[Metabase] Tell us how things are going.") + (let [subject (str (if (= "abandon" msg-type) + (trs "[Metabase] Help make Metabase better.") + (trs "[Metabase] Tell us how things are going."))) context (merge notification-context (random-quote-context) (if (= "abandon" msg-type) diff --git a/src/metabase/mbql/schema.clj b/src/metabase/mbql/schema.clj index 13732aae57643ac674921d180e6e2189d193cd8c..9b5027e851878a7db3c326440fe43e29d26a3414 100644 --- a/src/metabase/mbql/schema.clj +++ b/src/metabase/mbql/schema.clj @@ -492,7 +492,7 @@ ;;; ----------------------------------------------- MBQL [Inner] Query ----------------------------------------------- -(declare MBQLQuery) +(declare Query MBQLQuery) (def ^:private SourceQuery "Schema for a valid value for a `:source-query` clause." @@ -518,10 +518,10 @@ (def JoinQueryInfo "Schema for information about about a JOIN (or equivalent) that should be performed using a recursive MBQL or native - query. " - {:join-alias su/NonBlankString - ;; TODO - put a proper schema in here once I figure out what it is. I think it's (s/recursive #'Query)? - :query s/Any}) + query." + ;; Similar to a `JoinTable` but instead of referencing a table, it references a query expression + (assoc JoinTableInfo + :query (s/recursive #'Query))) (def JoinInfo "Schema for information about a JOIN (or equivalent) that needs to be performed, either `JoinTableInfo` or diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index 9adf79213b13774bd52a1a615b6a4ec3de6f5754..7890c40543c9398135a5d9bf73c54b2e524ec920 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -56,17 +56,17 @@ :Segment (extract-ids :segment inner-query)}))) -;;; -------------------------------------------------- Revisions -------------------------------------------------- +;;; --------------------------------------------------- Revisions ---------------------------------------------------- (defn serialize-instance "Serialize a `Card` for use in a `Revision`." ([instance] (serialize-instance nil nil instance)) ([_ _ instance] - (dissoc instance :created_at :updated_at))) + (dissoc instance :created_at :updated_at :result_metadata))) -;;; -------------------------------------------------- Lifecycle -------------------------------------------------- +;;; --------------------------------------------------- Lifecycle ---------------------------------------------------- (defn populate-query-fields "Lift `database_id`, `table_id`, and `query_type` from query definition." diff --git a/src/metabase/models/permissions.clj b/src/metabase/models/permissions.clj index 76ad328003c60944fabe203c5ec49ced692aad3a..0664eb6f0a2a45428ac0f2b76dd9766e24380553 100644 --- a/src/metabase/models/permissions.clj +++ b/src/metabase/models/permissions.clj @@ -102,6 +102,7 @@ (assert-not-admin-group permissions) (assert-valid-object permissions)) + ;;; ------------------------------------------------- Path Util Fns -------------------------------------------------- (def ^:private MapOrID @@ -569,8 +570,10 @@ [current-revision old new] (when *current-user-id* (db/insert! PermissionsRevision - :id (inc current-revision) ; manually specify ID here so if one was somehow inserted in the meantime in the fraction of a second - :before old ; since we called `check-revision-numbers` the PK constraint will fail and the transaction will abort + ;; manually specify ID here so if one was somehow inserted in the meantime in the fraction of a second since we + ;; called `check-revision-numbers` the PK constraint will fail and the transaction will abort + :id (inc current-revision) + :before old :after new :user_id *current-user-id*))) diff --git a/src/metabase/models/revision/diff.clj b/src/metabase/models/revision/diff.clj index 769dad5ac4302da2db3d410f571c8189a2cab2b2..cbe12bc7bd433150faf5769468f3ff916256d9b6 100644 --- a/src/metabase/models/revision/diff.clj +++ b/src/metabase/models/revision/diff.clj @@ -16,6 +16,9 @@ [:updated_at _ _] nil + [:result_metadata _ _] + nil + [:dataset_query _ _] "modified the query" diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj index 45b21fe2b811de06fc8925973983feb4fe02c2b3..01354c9c157971f8d09a2662856c2c308d918d5d 100644 --- a/src/metabase/public_settings.clj +++ b/src/metabase/public_settings.clj @@ -1,5 +1,5 @@ (ns metabase.public-settings - (:require [clojure.string :as s] + (:require [clojure.string :as str] [metabase [config :as config] [types :as types]] @@ -47,8 +47,8 @@ (tru "The base URL of this Metabase instance, e.g. \"http://metabase.my-company.com\".") :setter (fn [new-value] (setting/set-string! :site-url (when new-value - (cond->> (s/replace new-value #"/$" "") - (not (s/starts-with? new-value "http")) (str "http://")))))) + (cond->> (str/replace new-value #"/$" "") + (not (str/starts-with? new-value "http")) (str "http://")))))) (defsetting site-locale (str (tru "The default language for this Metabase instance.") diff --git a/src/metabase/public_settings/metastore.clj b/src/metabase/public_settings/metastore.clj index f7f98d416be40582119db45f2454481040fcbc1a..130246cc9a0a82dd4e1c09586cfe8a2dfc17547d 100644 --- a/src/metabase/public_settings/metastore.clj +++ b/src/metabase/public_settings/metastore.clj @@ -118,7 +118,7 @@ (try (when (seq new-value) (when (s/check ValidToken new-value) - (throw (ex-info (tru "Token format is invalid. Token should be 64 hexadecimal characters.") + (throw (ex-info (str (tru "Token format is invalid. Token should be 64 hexadecimal characters.")) {:status-code 400}))) (valid-token->features new-value) (log/info (trs "Token is valid."))) diff --git a/src/metabase/query_processor/middleware/add_query_throttle.clj b/src/metabase/query_processor/middleware/add_query_throttle.clj index 09fd88cdf7d65e3bdd92b9447f9199b59306bcc2..859351121a82b8dbceb50e1bb8e3b7ef0c13a354 100644 --- a/src/metabase/query_processor/middleware/add_query_throttle.clj +++ b/src/metabase/query_processor/middleware/add_query_throttle.clj @@ -18,7 +18,7 @@ (defn- throw-503-unavailable [] - (throw (ex-info (tru "Max concurrent query limit reached") + (throw (ex-info (str (tru "Max concurrent query limit reached")) {:type ::concurrent-query-limit-reached :status-code 503}))) diff --git a/src/metabase/query_processor/util.clj b/src/metabase/query_processor/util.clj index c47482e887ac59b3a97eee8314f6359685d4eb70..88a2c4439908f0f120cebd16b49f480b22c5dae5 100644 --- a/src/metabase/query_processor/util.clj +++ b/src/metabase/query_processor/util.clj @@ -46,9 +46,12 @@ (str/replace #"_" "-") keyword)) +;; TODO - rename this to `get-in-mbql-query-recursive` or something like that? (defn get-in-query - "Similar to `get-in` but will look in either `:query` or recursively in `[:query :source-query]`. Using - this function will avoid having to check if there's a nested query vs. top-level query." + "Similar to `get-in` but will look in either `:query` or recursively in `[:query :source-query]`. Using this function + will avoid having to check if there's a nested query vs. top-level query. Results in deeper levels of nesting are + preferred; i.e. if a key is present in both a `:source-query` and the top-level query, the value from the source + query will be returned." ([m ks] (get-in-query m ks nil)) ([m ks not-found] diff --git a/src/metabase/util.clj b/src/metabase/util.clj index f210a1b5cf5e7a8e5a4059320443e07c5036db8e..fc89d07d9b900c00fe01f3e2870db3c83f536ead 100644 --- a/src/metabase/util.clj +++ b/src/metabase/util.clj @@ -18,9 +18,9 @@ [java.text Normalizer Normalizer$Form] java.util.concurrent.TimeoutException)) -;; This is the very first log message that will get printed. It's here because this is one of the very first -;; namespaces that gets loaded, and the first that has access to the logger It shows up a solid 10-15 seconds before -;; the "Starting Metabase in STANDALONE mode" message because so many other namespaces need to get loaded +;; This is the very first log message that will get printed. +;; It's here because this is one of the very first namespaces that gets loaded, and the first that has access to the logger +;; It shows up a solid 10-15 seconds before the "Starting Metabase in STANDALONE mode" message because so many other namespaces need to get loaded (log/info (trs "Loading Metabase...")) ;; Set the default width for pprinting to 200 instead of 72. The default width is too narrow and wastes a lot of space @@ -571,6 +571,11 @@ (when-let [[_ java-major-version-str] (re-matches #"^(?:1\.)?(\d+).*$" java-version-str)] (>= (Integer/parseInt java-major-version-str) 9)))) +(defn hexadecimal-string? + "Returns truthy if `new-value` is a hexadecimal-string" + [new-value] + (and (string? new-value) + (re-matches #"[0-9a-f]{64}" new-value))) (defn snake-key "Convert a keyword or string `k` from `lisp-case` to `snake-case`." diff --git a/src/metabase/util/embed.clj b/src/metabase/util/embed.clj index 51ec4130c6bca09cb1dbf8f08800554ae554b2d1..ba51a73a17120f71e5a19e7f8869278425eb60da 100644 --- a/src/metabase/util/embed.clj +++ b/src/metabase/util/embed.clj @@ -7,12 +7,14 @@ [cheshire.core :as json] [clojure.string :as str] [hiccup.core :refer [html]] + [metabase + [public-settings :as public-settings] + [util :as u]] [metabase.models.setting :as setting] - [metabase.public-settings :as public-settings] - [metabase.util.i18n :refer [tru]] + [metabase.util.i18n :refer [trs tru]] [ring.util.codec :as codec])) -;;; ------------------------------------------------------------ PUBLIC LINKS UTIL FNS ------------------------------------------------------------ +;;; --------------------------------------------- PUBLIC LINKS UTIL FNS ---------------------------------------------- (defn- oembed-url "Return an oEmbed URL for the RELATIVE-PATH. @@ -51,14 +53,14 @@ :frameborder 0}])) -;;; ------------------------------------------------------------ EMBEDDING UTIL FNS ------------------------------------------------------------ +;;; ----------------------------------------------- EMBEDDING UTIL FNS ----------------------------------------------- (setting/defsetting ^:private embedding-secret-key (tru "Secret key used to sign JSON Web Tokens for requests to `/api/embed` endpoints.") :setter (fn [new-value] (when (seq new-value) - (assert (re-matches #"[0-9a-f]{64}" new-value) - "Invalid embedding-secret-key! Secret key must be a hexadecimal-encoded 256-bit key (i.e., a 64-character string).")) + (assert (u/hexadecimal-string? new-value) + (tru "Invalid embedding-secret-key! Secret key must be a hexadecimal-encoded 256-bit key (i.e., a 64-character string)."))) (setting/set-string! :embedding-secret-key new-value))) (defn- jwt-header @@ -68,28 +70,30 @@ (json/parse-string (codecs/bytes->str (codec/base64-decode header)) keyword))) (defn- check-valid-alg - "Check that the JWT `alg` isn't `none`. `none` is valid per the standard, but for obvious reasons we want to make sure our keys are signed. - Unfortunately, I don't think there's an easy way to do this with the JWT library we use, so manually parse the token and check the alg." + "Check that the JWT `alg` isn't `none`. `none` is valid per the standard, but for obvious reasons we want to make sure + our keys are signed. Unfortunately, I don't think there's an easy way to do this with the JWT library we use, so + manually parse the token and check the alg." [^String message] (let [{:keys [alg]} (jwt-header message)] (when-not alg - (throw (Exception. "JWT is missing `alg`."))) + (throw (Exception. (str (trs "JWT is missing `alg`."))))) (when (= alg "none") - (throw (Exception. "JWT `alg` cannot be `none`."))))) + (throw (Exception. (str (trs "JWT `alg` cannot be `none`."))))))) (defn unsign - "Parse a \"signed\" (base-64 encoded) JWT and return a Clojure representation. - Check that the signature is valid (i.e., check that it was signed with `embedding-secret-key`) - and it's otherwise a valid JWT (e.g., not expired), or throw an Exception." + "Parse a \"signed\" (base-64 encoded) JWT and return a Clojure representation. Check that the signature is + valid (i.e., check that it was signed with `embedding-secret-key`) and it's otherwise a valid JWT (e.g., not + expired), or throw an Exception." [^String message] (when (seq message) (try (check-valid-alg message) (jwt/unsign message (or (embedding-secret-key) - (throw (ex-info "The embedding secret key has not been set." {:status-code 400}))) - ;; The library will reject tokens with a created at timestamp in the future, so to account for clock skew tell the library - ;; that "now" is actually two minutes ahead of whatever the system time is so tokens don't get inappropriately rejected + (throw (ex-info (str (tru "The embedding secret key has not been set.")) {:status-code 400}))) + ;; The library will reject tokens with a created at timestamp in the future, so to account for clock + ;; skew tell the library that "now" is actually two minutes ahead of whatever the system time is so + ;; tokens don't get inappropriately rejected {:now (+ (buddy-util/now) 120)}) ;; if `jwt/unsign` throws an Exception rethrow it in a format that's friendlier to our API (catch Throwable e @@ -99,4 +103,4 @@ "Find KEYSEQ in the UNSIGNED-TOKEN (a JWT token decoded by `unsign`) or throw a 400." [unsigned-token keyseq] (or (get-in unsigned-token keyseq) - (throw (ex-info (str "Token is missing value for keypath" keyseq) {:status-code 400})))) + (throw (ex-info (str (tru "Token is missing value for keypath") " " keyseq) {:status-code 400})))) diff --git a/test/metabase/automagic_dashboards/comparison_test.clj b/test/metabase/automagic_dashboards/comparison_test.clj index 52cdbe40514bb55c4e1ceaf632fb4f502c796492..6aeea018267c224d033d305ca975d25e5c7c6d2b 100644 --- a/test/metabase/automagic_dashboards/comparison_test.clj +++ b/test/metabase/automagic_dashboards/comparison_test.clj @@ -1,15 +1,16 @@ (ns metabase.automagic-dashboards.comparison-test (:require [expectations :refer :all] [metabase.automagic-dashboards - [comparison :refer :all :as c] + [comparison :as c :refer :all] [core :refer [automagic-analysis]]] [metabase.models [card :refer [Card]] [query :as query] [segment :refer [Segment]] - [table :refer [Table] :as table]] - [metabase.test.data :as data] - [metabase.test.automagic-dashboards :refer :all] + [table :as table :refer [Table]]] + [metabase.test + [automagic-dashboards :refer :all] + [data :as data]] [toucan.util.test :as tt])) (def ^:private segment diff --git a/test/metabase/cmd/load_from_h2_test.clj b/test/metabase/cmd/load_from_h2_test.clj index cbe27d04f86283e02162690a23ac8cd178cb10d1..61351f1aaca7ff6c334b0385f1d963b923f17b99 100644 --- a/test/metabase/cmd/load_from_h2_test.clj +++ b/test/metabase/cmd/load_from_h2_test.clj @@ -12,8 +12,7 @@ (def ^:private models-to-exclude "Models that should *not* be migrated in `load-from-h2`." - #{"LegacyQueryExecution" - "Query" + #{"Query" "QueryCache" "QueryExecution"}) diff --git a/test/metabase/events/revision_test.clj b/test/metabase/events/revision_test.clj index 3919fbe07fd4c4b04ab4d17a50c08f259504c09a..6c80d24f1f8d739353cd50fe855c9bc6aa65b4c0 100644 --- a/test/metabase/events/revision_test.clj +++ b/test/metabase/events/revision_test.clj @@ -45,8 +45,7 @@ :cache_ttl nil :query_type :query :table_id (data/id :categories) - :visualization_settings {} - :result_metadata nil}) + :visualization_settings {}}) (defn- dashboard->revision-object [dashboard] {:description nil diff --git a/test/metabase/query_processor/util_test.clj b/test/metabase/query_processor/util_test.clj index 2c3c18c6f53fc568ad626d57eb20b6bc853776e0..b9d8cc9c6309a107b977bdc15f624c9a10a9de70 100644 --- a/test/metabase/query_processor/util_test.clj +++ b/test/metabase/query_processor/util_test.clj @@ -108,17 +108,6 @@ :native {:query "SELECT pg_sleep(15), 2 AS two"}}))) -(defrecord ^:private TestRecord1 [x]) -(defrecord ^:private TestRecord2 [x]) - -(def ^:private test-tree - {:a {:aa (TestRecord1. 1) - :ab (TestRecord2. 1)} - :b (TestRecord1. 1) - :c (TestRecord2. 1) - :d [1 2 3 4]}) - - (def ^:private test-inner-map {:test {:value 10}}) diff --git a/test/metabase/query_processor_test/failure_test.clj b/test/metabase/query_processor_test/failure_test.clj index 6bec2791803c0e3571a06e251d6aea7d26fe1304..8e7f9558075c183f85315ffc039b6c4c061da080 100644 --- a/test/metabase/query_processor_test/failure_test.clj +++ b/test/metabase/query_processor_test/failure_test.clj @@ -2,8 +2,8 @@ "Tests for how the query processor as a whole handles failures." (:require [expectations :refer [expect]] [metabase.query-processor :as qp] - [metabase.test.data :as data] - [metabase.query-processor.interface :as qp.i]) + [metabase.query-processor.interface :as qp.i] + [metabase.test.data :as data]) (:import metabase.driver.h2.H2Driver)) (defn- bad-query []