Skip to content
Snippets Groups Projects
Unverified Commit f0794ae9 authored by Cam Saul's avatar Cam Saul
Browse files

Code cleanup :shower:

parent edf6efa3
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 341 additions and 278 deletions
.dir-locals.el
+ 0
− 26
View file @ f0794ae9
......@@ -10,37 +10,11 @@
;; Define custom indentation for functions inside metabase.
;; This list isn't complete; add more forms as we come across them.
(define-clojure-indent
(api-let 2)
(assert 1)
(assoc 1)
(auto-parse 1)
(catch-api-exceptions 0)
(check 1)
(checkp 1)
(context 2)
(create-database-definition 1)
(ex-info 1)
(execute-query 1)
(execute-sql! 2)
(expect 0)
(expect-with-all-engines 0)
(expect-with-engine 1)
(expect-with-engines 1)
(let-400 1)
(let-404 1)
(let-500 1)
(match 1)
(match-$ 1)
(merge-with 1)
(post-select 1)
(pre-delete 1)
(pre-insert 1)
(pre-update 1)
(project 1)
(qp-expect-with-engines 1)
(render-file 1)
(resolve-private-vars 1)
(select 1)
(sync-in-context 2)
(when-testing-engine 1)
(with-redefs-fn 1)))))))
src/metabase/api/activity.clj
+ 6
− 3
View file @ f0794ae9
......@@ -24,15 +24,18 @@
(apply merge-with set/union (for [{:keys [model model_id], :as activity} activities
:when model]
(merge {model #{model_id}}
;; pull the referenced card IDs out of the dashcards for dashboard activites that involve adding/removing cards
;; pull the referenced card IDs out of the dashcards for dashboard activites
;; that involve adding/removing cards
(when (dashcard-activity? activity)
{"card" (set (for [dashcard (get-in activity [:details :dashcards])]
(:card_id dashcard)))})))))
(defn- referenced-objects->existing-objects
"Given a map of existing objects like the one returned by `activities->referenced-objects`, return a similar map of models to IDs of objects *that exist*.
"Given a map of existing objects like the one returned by `activities->referenced-objects`, return a similar map of
models to IDs of objects *that exist*.
(referenced-objects->existing-objects {\"dashboard\" #{41 42 43}, \"card\" #{100 101}, ...}) -> {\"dashboard\" #{41 43}, \"card\" #{101}, ...}"
(referenced-objects->existing-objects {\"dashboard\" #{41 42 43}, \"card\" #{100 101}, ...})
;; -> {\"dashboard\" #{41 43}, \"card\" #{101}, ...}"
[referenced-objects]
(into {} (for [[model ids] referenced-objects
:when (seq ids)]
......
src/metabase/api/collection.clj
+ 18
− 9
View file @ f0794ae9
......@@ -17,14 +17,18 @@
(api/defendpoint GET "/"
"Fetch a list of all Collections that the current user has read permissions for.
This includes `:can_write`, which means whether the current user is allowed to add or remove Cards to this Collection; keep in mind
that regardless of this status you must be a superuser to modify properties of Collections themselves.
This includes `:can_write`, which means whether the current user is allowed to add or remove Cards to this
Collection; keep in mind that regardless of this status you must be a superuser to modify properties of Collections
themselves.
By default, this returns non-archived Collections, but instead you can show archived ones by passing `?archived=true`."
By default, this returns non-archived Collections, but instead you can show archived ones by passing
`?archived=true`."
[archived]
{archived (s/maybe su/BooleanString)}
(-> (filterv mi/can-read? (db/select Collection :archived (Boolean/parseBoolean archived) {:order-by [[:%lower.name :asc]]}))
(hydrate :can_write)))
(as-> (db/select Collection :archived (Boolean/parseBoolean archived)
{:order-by [[:%lower.name :asc]]}) collections
(filter mi/can-read? collections)
(hydrate collections :can_write)))
(api/defendpoint GET "/:id"
"Fetch a specific (non-archived) Collection, including cards that belong to it."
......@@ -45,8 +49,12 @@
(api/defendpoint PUT "/:id"
"Modify an existing Collection, including archiving or unarchiving it."
[id, :as {{:keys [name color description archived]} :body}]
{name su/NonBlankString, color collection/hex-color-regex, description (s/maybe su/NonBlankString), archived (s/maybe s/Bool)}
;; you have to be a superuser to modify a Collection itself, but `/collection/:id/` perms are sufficient for adding/removing Cards
{name su/NonBlankString
color collection/hex-color-regex
description (s/maybe su/NonBlankString)
archived (s/maybe s/Bool)}
;; you have to be a superuser to modify a Collection itself, but `/collection/:id/` perms are sufficient for
;; adding/removing Cards
(api/check-superuser)
(api/api-let [404 "Not Found"] [collection-before-update (Collection id)]
(db/update! Collection id
......@@ -68,7 +76,7 @@
(Collection id))
;;; ------------------------------------------------------------ GRAPH ENDPOINTS ------------------------------------------------------------
;;; ------------------------------------------------ GRAPH ENDPOINTS -------------------------------------------------
(api/defendpoint GET "/graph"
"Fetch a graph of all Collection Permissions."
......@@ -88,7 +96,8 @@
{(->int group-id) (dejsonify-collections collections)})))
(defn- dejsonify-graph
"Fix the types in the graph when it comes in from the API, e.g. converting things like `\"none\"` to `:none` and parsing object keys as integers."
"Fix the types in the graph when it comes in from the API, e.g. converting things like `\"none\"` to `:none` and
parsing object keys as integers."
[graph]
(update graph :groups dejsonify-groups))
......
src/metabase/api/common.clj
+ 76
− 54
View file @ f0794ae9
......@@ -19,7 +19,7 @@
(declare check-403 check-404)
;;; ------------------------------------------------------------ DYNAMIC VARIABLES ------------------------------------------------------------
;;; ----------------------------------------------- DYNAMIC VARIABLES ------------------------------------------------
;; These get bound by middleware for each HTTP request.
(def ^:dynamic ^Integer *current-user-id*
......@@ -40,15 +40,17 @@
(atom #{}))
;;; ------------------------------------------------------------ Precondition checking helper fns ------------------------------------------------------------
;;; ---------------------------------------- Precondition checking helper fns ----------------------------------------
(defn check
"Assertion mechanism for use inside API functions.
Checks that TEST is true, or throws an `ExceptionInfo` with STATUS-CODE and MESSAGE.
Checks that TEST is true, or throws an `ExceptionInfo` with STATUS-CODE and MESSAGE.
MESSAGE can be either a plain string error message, or a map including the key `:message` and any additional details, such as an `:error_code`.
MESSAGE can be either a plain string error message, or a map including the key `:message` and any additional
details, such as an `:error_code`.
This exception is automatically caught in the body of `defendpoint` functions, and the appropriate HTTP response is generated.
This exception is automatically caught in the body of `defendpoint` functions, and the appropriate HTTP response is
generated.
`check` can be called with the form
......@@ -62,6 +64,7 @@
(check test1 code1 message1
test2 code2 message2)"
{:style/indent 1}
([tst code-or-code-message-pair & rest-args]
(let [[[code message] rest-args] (if (vector? code-or-code-message-pair)
[code-or-code-message-pair rest-args]
......@@ -86,7 +89,8 @@
(check-403 *is-superuser?*))
;;; #### checkp- functions: as in "check param". These functions expect that you pass a symbol so they can throw exceptions w/ relevant error messages.
;; checkp- functions: as in "check param". These functions expect that you pass a symbol so they can throw exceptions
;; w/ relevant error messages.
(defn throw-invalid-param-exception
"Throw an `ExceptionInfo` that contains information about an invalid API params in the expected format."
......@@ -97,13 +101,15 @@
(defn checkp
"Assertion mechanism for use inside API functions that validates individual input params.
Checks that TEST is true, or throws an `ExceptionInfo` with FIELD-NAME and MESSAGE.
Checks that TEST is true, or throws an `ExceptionInfo` with FIELD-NAME and MESSAGE.
This exception is automatically caught in the body of `defendpoint` functions, and the appropriate HTTP response is generated.
This exception is automatically caught in the body of `defendpoint` functions, and the appropriate HTTP response is
generated.
`checkp` can be called with the form
(checkp test field-name message)"
{:style/indent 1}
([tst field-name message]
(when-not tst
(throw-invalid-param-exception (str field-name) message))))
......@@ -142,7 +148,7 @@
(checkp-with (partial contains? valid-values-set) symb value (str "must be one of: " valid-values-set)))
;;; ------------------------------------------------------------ api-let, api->, etc. ------------------------------------------------------------
;;; ---------------------------------------------- api-let, api->, etc. ----------------------------------------------
;; The following all work exactly like the corresponding Clojure versions
;; but take an additional arg at the beginning called RESPONSE-PAIR.
......@@ -156,7 +162,7 @@
(api-let [404 \"Not found.\"] [user @*current-user*]
(:id user))"
{:arglists '([[status-code message] [binding test] & body])}
{:arglists '([[status-code message] [binding test] & body]), :style/indent 2}
[response-pair [binding test & more] & body]
(if (seq more)
`(api-let ~response-pair ~[binding test]
......@@ -168,74 +174,89 @@
~@more]
~@body))))
(defmacro api->
"If TEST is true, thread the result using `->` through BODY.
(api-> [404 \"Not found\"] @*current-user*
:id)"
[response-pair tst & body]
`(api-let ~response-pair [result# ~tst]
(-> result#
~@body)))
(defmacro api->>
"Like `api->`, but threads result using `->>`."
[response-pair tst & body]
`(api-let ~response-pair [result# ~tst]
(->> result#
~@body)))
;;; ### GENERIC RESPONSE HELPERS
;; These are basically the same as the `api-` versions but with RESPONSE-PAIR already bound
;; #### GENERIC 400 RESPONSE HELPERS
(def ^:private ^:const generic-400 [400 "Invalid Request."])
(defn check-400 "Throw a `400` if ARG is `false` or `nil`, otherwise return as-is." [arg] (check arg generic-400))
(defmacro let-400 "Bind a form as with `let`; throw a 400 if it is `nil` or `false`." [& body] `(api-let ~generic-400 ~@body))
(defmacro ->400 "If form is `nil` or `false`, throw a 400; otherwise thread it through BODY via `->`." [& body] `(api-> ~generic-400 ~@body))
(defmacro ->>400 "If form is `nil` or `false`, throw a 400; otherwise thread it through BODY via `->>`." [& body] `(api->> ~generic-400 ~@body))
(def ^:private generic-400
[400 "Invalid Request."])
(defn check-400
"Throw a `400` if ARG is `false` or `nil`, otherwise return as-is."
[arg]
(check arg generic-400))
(defmacro let-400
"Bind a form as with `let`; throw a 400 if it is `nil` or `false`."
{:style/indent 1}
[& body]
`(api-let ~generic-400 ~@body))
;; #### GENERIC 404 RESPONSE HELPERS
(def ^:private ^:const generic-404 [404 "Not found."])
(defn check-404 "Throw a `404` if ARG is `false` or `nil`, otherwise return as-is." [arg] (check arg generic-404))
(defmacro let-404 "Bind a form as with `let`; throw a 404 if it is `nil` or `false`." [& body] `(api-let ~generic-404 ~@body))
(defmacro ->404 "If form is `nil` or `false`, throw a 404; otherwise thread it through BODY via `->`." [& body] `(api-> ~generic-404 ~@body))
(defmacro ->>404 "If form is `nil` or `false`, throw a 404; otherwise thread it through BODY via `->>`." [& body] `(api->> ~generic-404 ~@body))
(def ^:private generic-404
[404 "Not found."])
(defn check-404
"Throw a `404` if ARG is `false` or `nil`, otherwise return as-is."
[arg]
(check arg generic-404))
(defmacro let-404
"Bind a form as with `let`; throw a 404 if it is `nil` or `false`."
{:style/indent 1}
[& body]
`(api-let ~generic-404 ~@body))
;; #### GENERIC 403 RESPONSE HELPERS
;; If you can't be bothered to write a custom error message
(def ^:private ^:const generic-403 [403 "You don't have permissions to do that."])
(defn check-403 "Throw a `403` if ARG is `false` or `nil`, otherwise return as-is." [arg] (check arg generic-403))
(defmacro let-403 "Bind a form as with `let`; throw a 403 if it is `nil` or `false`." [& body] `(api-let ~generic-403 ~@body))
(defmacro ->403 "If form is `nil` or `false`, throw a 403; otherwise thread it through BODY via `->`." [& body] `(api-> ~generic-403 ~@body))
(defmacro ->>403 "If form is `nil` or `false`, throw a 403; otherwise thread it through BODY via `->>`." [& body] `(api->> ~generic-403 ~@body))
(def ^:private generic-403
[403 "You don't have permissions to do that."])
(defn check-403
"Throw a `403` if ARG is `false` or `nil`, otherwise return as-is."
[arg]
(check arg generic-403))
(defmacro let-403
"Bind a form as with `let`; throw a 403 if it is `nil` or `false`."
{:style/indent 1}
[& body]
`(api-let ~generic-403 ~@body))
;; #### GENERIC 500 RESPONSE HELPERS
;; For when you don't feel like writing something useful
(def ^:private ^:const generic-500 [500 "Internal server error."])
(defn check-500 "Throw a `500` if ARG is `false` or `nil`, otherwise return as-is." [arg] (check arg generic-500))
(defmacro let-500 "Bind a form as with `let`; throw a 500 if it is `nil` or `false`." [& body] `(api-let ~generic-500 ~@body))
(defmacro ->500 "If form is `nil` or `false`, throw a 500; otherwise thread it through BODY via `->`." [& body] `(api-> ~generic-500 ~@body))
(defmacro ->>500 "If form is `nil` or `false`, throw a 500; otherwise thread it through BODY via `->>`." [& body] `(api->> ~generic-500 ~@body))
(def ^:private generic-500
[500 "Internal server error."])
(defn check-500
"Throw a `500` if ARG is `false` or `nil`, otherwise return as-is."
[arg]
(check arg generic-500))
(defmacro let-500
"Bind a form as with `let`; throw a 500 if it is `nil` or `false`."
{:style/indent 1}
[& body]
`(api-let ~generic-500 ~@body))
(def ^:const generic-204-no-content
"A 'No Content' response for `DELETE` endpoints to return."
{:status 204, :body nil})
;;; ------------------------------------------------------------ DEFENDPOINT AND RELATED FUNCTIONS ------------------------------------------------------------
;;; --------------------------------------- DEFENDPOINT AND RELATED FUNCTIONS ----------------------------------------
;; TODO - several of the things `defendpoint` does could and should just be done by custom Ring middleware instead
(defmacro defendpoint
"Define an API function.
This automatically does several things:
- calls `auto-parse` to automatically parse certain args. e.g. `id` is converted from `String` to `Integer` via `Integer/parseInt`
- calls `auto-parse` to automatically parse certain args. e.g. `id` is converted from `String` to `Integer` via
`Integer/parseInt`
- converts ROUTE from a simple form like `\"/:id\"` to a typed one like `[\"/:id\" :id #\"[0-9]+\"]`
- sequentially applies specified annotation functions on args to validate them.
- executes BODY inside a `try-catch` block that handles exceptions; if exception is an instance of `ExceptionInfo` and includes a `:status-code`,
that code will be returned
- executes BODY inside a `try-catch` block that handles exceptions; if exception is an instance of `ExceptionInfo`
and includes a `:status-code`, that code will be returned
- automatically calls `wrap-response-if-needed` on the result of BODY
- tags function's metadata in a way that subsequent calls to `define-routes` (see below)
will automatically include the function in the generated `defroutes` form.
......@@ -252,7 +273,8 @@
(when-not docstr
(log/warn (format "Warning: endpoint %s/%s does not have a docstring." (ns-name *ns*) fn-name)))
`(def ~(vary-meta fn-name assoc
;; eval the vals in arg->schema to make sure the actual schemas are resolved so we can document their API error messages
;; eval the vals in arg->schema to make sure the actual schemas are resolved so we can document
;; their API error messages
:doc (route-dox method route docstr args (m/map-vals eval arg->schema) body)
:is-endpoint? true)
(~method ~route ~args
......@@ -277,7 +299,7 @@
~@additional-routes ~@api-routes)))
;;; ------------------------------------------------------------ PERMISSIONS CHECKING HELPER FNS ------------------------------------------------------------
;;; ---------------------------------------- PERMISSIONS CHECKING HELPER FNS -----------------------------------------
(defn read-check
"Check whether we can read an existing OBJ, or ENTITY with ID.
......@@ -308,7 +330,7 @@
(write-check (apply db/select-one entity :id id other-conditions))))
;;; ------------------------------------------------------------ OTHER HELPER FNS ------------------------------------------------------------
;;; ------------------------------------------------ OTHER HELPER FNS ------------------------------------------------
(defn check-public-sharing-enabled
"Check that the `public-sharing-enabled` Setting is `true`, or throw a `400`."
......
src/metabase/api/common/internal.clj
+ 60
− 52
View file @ f0794ae9
......@@ -10,9 +10,9 @@
[schema.core :as s])
(:import java.sql.SQLException))
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; | DOCSTRING GENERATION |
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | DOCSTRING GENERATION |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- endpoint-name
"Generate a string like `GET /api/meta/db/:id` for a defendpoint route."
......@@ -37,9 +37,8 @@
:else [form]))
(defn- args-form-symbols
"Return a map of arg -> nil for args taken from the arguments vector.
This map is merged with the ones found in the schema validation map to build a complete map of args used by the
endpoint."
"Return a map of arg -> nil for args taken from the arguments vector. This map is merged with the ones found in the
schema validation map to build a complete map of args used by the endpoint."
[form]
(into {} (for [arg (args-form-flatten form)
:when (and (symbol? arg)
......@@ -47,8 +46,8 @@
{arg nil})))
(defn- dox-for-schema
"Look up the docstring for SCHEMA for use in auto-generated API documentation.
In most cases this is defined by wrapping the schema with `with-api-error-message`."
"Look up the docstring for SCHEMA for use in auto-generated API documentation. In most cases this is defined by
wrapping the schema with `with-api-error-message`."
[schema]
(if-not schema
""
......@@ -59,7 +58,7 @@
(defn- param-name
"Return the appropriate name for this PARAM-SYMB based on its SCHEMA. Usually this is just the name of the
PARAM-SYMB, but if the schema used a call to `su/api-param` we;ll use that name instead."
PARAM-SYMB, but if the schema used a call to `su/api-param` we;ll use that name instead."
[param-symb schema]
(or (when (record? schema)
(:api-param-name schema))
......@@ -99,13 +98,13 @@
param->schema)))
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; | AUTO-PARSING + ROUTE TYPING |
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | AUTO-PARSING + ROUTE TYPING |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn parse-int
"Parse VALUE (presumabily a string) as an Integer, or throw a 400 exception.
Used to automatically to parse `id` parameters in `defendpoint` functions."
"Parse VALUE (presumabily a string) as an Integer, or throw a 400 exception. Used to automatically to parse `id`
parameters in `defendpoint` functions."
[^String value]
(try (Integer/parseInt value)
(catch NumberFormatException _
......@@ -122,8 +121,8 @@
:parser nil}})
(def ^:private ^:const auto-parse-arg-name-patterns
"Sequence of `[param-pattern parse-type]` pairs.
A param with name matching PARAM-PATTERN should be considered to be of AUTO-PARSE-TYPE."
"Sequence of `[param-pattern parse-type]` pairs. A param with name matching PARAM-PATTERN should be considered to be
of AUTO-PARSE-TYPE."
[[#"^uuid$" :uuid]
[#"^session_id$" :uuid]
[#"^[\w-_]*id$" :int]])
......@@ -142,8 +141,8 @@
;;; ## TYPIFY-ROUTE
(defn route-param-regex
"If keyword ARG has a matching type, return a pair like `[arg route-param-regex]`,
where ROUTE-PARAM-REGEX is the regex that this param that arg must match.
"If keyword ARG has a matching type, return a pair like `[arg route-param-regex]`,where ROUTE-PARAM-REGEX is the
regex that this param that arg must match.
(route-param-regex :id) -> [:id #\"[0-9]+\"]"
[arg]
......@@ -162,16 +161,16 @@
(map keyword)))
(defn typify-args
"Given a sequence of keyword ARGS, return a sequence of `[:arg pattern :arg pattern ...]`
for args that have matching types."
"Given a sequence of keyword ARGS, return a sequence of `[:arg pattern :arg pattern ...]` for args that have
matching types."
[args]
(->> args
(mapcat route-param-regex)
(filterv identity)))
(defn typify-route
"Expand a ROUTE string like \"/:id\" into a Compojure route form that uses regexes to match
parameters whose name matches a regex from `auto-parse-arg-name-patterns`.
"Expand a ROUTE string like \"/:id\" into a Compojure route form that uses regexes to match parameters whose name
matches a regex from `auto-parse-arg-name-patterns`.
(typify-route \"/:id/card\") -> [\"/:id/card\" :id #\"[0-9]+\"]"
[route]
......@@ -186,8 +185,7 @@
;;; ## ROUTE ARG AUTO PARSING
(defn let-form-for-arg
"Given an ARG-SYMBOL like `id`, return a pair like `[id (Integer/parseInt id)]`
that can be used in a `let` form."
"Given an ARG-SYMBOL like `id`, return a pair like `[id (Integer/parseInt id)]` that can be used in a `let` form."
[arg-symbol]
(when (symbol? arg-symbol)
(some-> (arg-type arg-symbol) ; :int
......@@ -197,7 +195,9 @@
((partial vector arg-symbol))))) ; [id (Integer/parseInt id)]
(defmacro auto-parse
"Create a `let` form that applies corresponding parse-fn for any symbols in ARGS that are present in `*auto-parse-types*`."
"Create a `let` form that applies corresponding parse-fn for any symbols in ARGS that are present in
`*auto-parse-types*`."
{:style/indent 1}
[args & body]
(let [let-forms (->> args
(mapcat let-form-for-arg)
......@@ -206,9 +206,9 @@
~@body)))
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; | EXCEPTION HANDLING |
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | EXCEPTION HANDLING |
;;; +----------------------------------------------------------------------------------------------------------------+
;; TODO - this SHOULD all be implemented as middleware instead
(defn- api-exception-response
......@@ -222,22 +222,27 @@
;; Exceptions that include a status code *and* other info are things like Field validation exceptions.
;; Return those as is
(and status-code
(seq other-info)) other-info
;; If status code was specified but other data wasn't, it's something like a 404. Return message as the body.
status-code message
;; Otherwise it's a 500. Return a body that includes exception & filtered stacktrace for debugging purposes
:else (let [stacktrace (u/filtered-stacktrace e)]
(merge (assoc other-info
:message message
:stacktrace stacktrace)
(when (instance? SQLException e)
{:sql-exception-chain (str/split (with-out-str (jdbc/print-sql-exception-chain e))
#"\s*\n\s*")}))))}))
(seq other-info))
other-info
;; If status code was specified but other data wasn't, it's something like a 404. Return message as the
;; body.
status-code
message
;; Otherwise it's a 500. Return a body that includes exception & filtered stacktrace for debugging
;; purposes
:else
(let [stacktrace (u/filtered-stacktrace e)]
(merge (assoc other-info
:message message
:stacktrace stacktrace)
(when (instance? SQLException e)
{:sql-exception-chain (str/split (with-out-str (jdbc/print-sql-exception-chain e))
#"\s*\n\s*")}))))}))
(def ^:dynamic ^Boolean *automatically-catch-api-exceptions*
"Should API exceptions automatically be caught? By default, this is `true`, but this can be disabled when we want to catch
Exceptions and return something generic to avoid leaking information, e.g. with the `api/public` and `api/embed` endpoints.
generic exceptions"
"Should API exceptions automatically be caught? By default, this is `true`, but this can be disabled when we want to
catch Exceptions and return something generic to avoid leaking information, e.g. with the `api/public` and
`api/embed` endpoints. generic exceptions"
true)
(defn do-with-caught-api-exceptions
......@@ -251,13 +256,14 @@
(defmacro catch-api-exceptions
"Execute BODY, and if an exception is thrown, return the appropriate HTTP response."
{:style/indent 0}
[& body]
`(do-with-caught-api-exceptions (fn [] ~@body)))
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; | PARAM VALIDATION |
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | PARAM VALIDATION |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn validate-param
"Validate a parameter against its respective schema, or throw an Exception."
......@@ -277,16 +283,18 @@
`(validate-param '~param ~param ~schema)))
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; | MISC. OTHER FNS USED BY DEFENDPOINT |
;;; +------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | MISC. OTHER FNS USED BY DEFENDPOINT |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn route-fn-name
"Generate a symbol suitable for use as the name of an API endpoint fn.
Name is just METHOD + ROUTE with slashes replaced by underscores.
`(route-fn-name GET \"/:id\") -> GET_:id`"
"Generate a symbol suitable for use as the name of an API endpoint fn. Name is just METHOD + ROUTE with slashes
replaced by underscores.
(route-fn-name GET \"/:id\") ;-> GET_:id"
[method route]
(let [route (if (vector? route) (first route) route)] ; if we were passed a vector like [":id" :id #"[0-9+]"] only use first part
;; if we were passed a vector like [":id" :id #"[0-9+]"] only use first part
(let [route (if (vector? route) (first route) route)]
(-> (str (name method) route)
(^String .replace "/" "_")
symbol)))
......
src/metabase/api/dashboard.clj
+ 53
− 33
View file @ f0794ae9
......@@ -11,7 +11,7 @@
[metabase.models
[card :refer [Card]]
[dashboard :as dashboard :refer [Dashboard]]
[dashboard-card :refer [create-dashboard-card! DashboardCard delete-dashboard-card! update-dashboard-card!]]
[dashboard-card :refer [DashboardCard delete-dashboard-card!]]
[dashboard-favorite :refer [DashboardFavorite]]
[interface :as mi]
[query :as query :refer [Query]]
......@@ -70,7 +70,7 @@
(events/publish-event! :dashboard-create)))
;;; ------------------------------------------------------------ Hiding Unreadable Cards ------------------------------------------------------------
;;; -------------------------------------------- Hiding Unreadable Cards ---------------------------------------------
(defn- hide-unreadable-card
"If CARD is unreadable, replace it with an object containing only its `:id`."
......@@ -89,26 +89,36 @@
(update :series (partial mapv hide-unreadable-card))))))))
;;; ------------------------------------------------------------ Query Average Duration Info ------------------------------------------------------------
;;; ------------------------------------------ Query Average Duration Info -------------------------------------------
;; Adding the average execution time to all of the Cards in a Dashboard efficiently is somewhat involved. There are a few things that make this tricky:
;; Adding the average execution time to all of the Cards in a Dashboard efficiently is somewhat involved. There are a
;; few things that make this tricky:
;;
;; 1. Queries are usually executed with `:constraints` that different from how they're actually definied, but not always. This means we should look
;; up hashes for both the query as-is and for the query with `default-query-constraints` and use whichever one we find
;; 2. The structure of DashCards themselves is complicated. It has a top-level `:card` property and (optionally) a sequence of additional Cards under `:series`
;; 3. Query hashes are byte arrays, and two idential byte arrays aren't equal to each other in Java; thus they don't work as one would expect when being used as map keys
;; 1. Queries are usually executed with `:constraints` that different from how they're actually definied, but not
;; always. This means we should look up hashes for both the query as-is and for the query with
;; `default-query-constraints` and use whichever one we find
;;
;; 2. The structure of DashCards themselves is complicated. It has a top-level `:card` property and (optionally) a
;; sequence of additional Cards under `:series`
;;
;; 3. Query hashes are byte arrays, and two idential byte arrays aren't equal to each other in Java; thus they don't
;; work as one would expect when being used as map keys
;;
;; Here's an overview of the approach used to efficiently add the info:
;;
;; 1. Build a sequence of query hashes (both as-is and with default constraints) for every card and series in the dashboard cards
;; 2. Fetch all matching entires from Query in the DB and build a map of hash (converted to a Clojure vector) -> average execution time
;; 3. Iterate back over each card and look for matching entries in the `hash-vec->avg-time` for either the normal hash or the hash with default constraints,
;; and add the result as `:average_execution_time`
;; 1. Build a sequence of query hashes (both as-is and with default constraints) for every card and series in the
;; dashboard cards
;;
;; 2. Fetch all matching entires from Query in the DB and build a map of hash (converted to a Clojure vector) ->
;; average execution time
;;
;; 3. Iterate back over each card and look for matching entries in the `hash-vec->avg-time` for either the normal hash
;; or the hash with default constraints, and add the result as `:average_execution_time`
(defn- card->query-hashes
"Return a tuple of possible hashes that would be associated with executions of CARD.
The first is the hash of the query dictionary as-is; the second is one with the `default-query-constraints`,
which is how it will most likely be run."
"Return a tuple of possible hashes that would be associated with executions of CARD. The first is the hash of the
query dictionary as-is; the second is one with the `default-query-constraints`, which is how it will most likely be
run."
[{:keys [dataset_query]}]
(u/ignore-exceptions
[(qp-util/query-hash dataset_query)
......@@ -130,15 +140,16 @@
(defn- hashes->hash-vec->avg-time
"Given some query HASHES, return a map of hashes (as normal Clojure vectors) to the average query durations.
(The hashes are represented as normal Clojure vectors because identical byte arrays aren't considered equal to one another, and thus do not
work as one would expect when used as map keys.)"
(The hashes are represented as normal Clojure vectors because identical byte arrays aren't considered equal to one
another, and thus do not work as one would expect when used as map keys.)"
[hashes]
(when (seq hashes)
(into {} (for [[k v] (db/select-field->field :query_hash :average_execution_time Query :query_hash [:in hashes])]
{(vec k) v}))))
(defn- add-query-average-duration-to-card
"Add `:query_average_duration` info to a CARD (i.e., the `:card` property of a DashCard or an entry in its `:series` array)."
"Add `:query_average_duration` info to a CARD (i.e., the `:card` property of a DashCard or an entry in its `:series`
array)."
[card hash-vec->avg-time]
(assoc card :query_average_duration (some (fn [query-hash]
(hash-vec->avg-time (vec query-hash)))
......@@ -162,7 +173,8 @@
(update dashboard :ordered_cards add-query-average-duration-to-dashcards))
;;; ------------------------------------------------------------------------------------------------------------------------------------------------------
;;; --------------------------------------------- Fetching/Updating/Etc. ---------------------------------------------
(api/defendpoint GET "/:id"
"Get `Dashboard` with ID."
......@@ -179,9 +191,11 @@
(api/defendpoint PUT "/:id"
"Update a `Dashboard`.
Usually, you just need write permissions for this Dashboard to do this (which means you have appropriate permissions for the Cards belonging to this Dashboard),
but to change the value of `enable_embedding` you must be a superuser."
[id :as {{:keys [description name parameters caveats points_of_interest show_in_getting_started enable_embedding embedding_params position archived], :as dashboard} :body}]
Usually, you just need write permissions for this Dashboard to do this (which means you have appropriate
permissions for the Cards belonging to this Dashboard), but to change the value of `enable_embedding` you must be a
superuser."
[id :as {{:keys [description name parameters caveats points_of_interest show_in_getting_started enable_embedding
embedding_params position archived], :as dashboard} :body}]
{name (s/maybe su/NonBlankString)
description (s/maybe s/Str)
caveats (s/maybe s/Str)
......@@ -193,7 +207,8 @@
position (s/maybe su/IntGreaterThanZero)
archived (s/maybe s/Bool)}
(let [dash (api/write-check Dashboard id)]
;; you must be a superuser to change the value of `enable_embedding` or `embedding_params`. Embedding must be enabled
;; you must be a superuser to change the value of `enable_embedding` or `embedding_params`. Embedding must be
;; enabled
(when (or (and (some? enable_embedding)
(not= enable_embedding (:enable_embedding dash)))
(and embedding_params
......@@ -205,17 +220,20 @@
;; description, position are allowed to be `nil`. Everything else must be non-nil
(u/select-keys-when dashboard
:present #{:description :position}
:non-nil #{:name :parameters :caveats :points_of_interest :show_in_getting_started :enable_embedding :embedding_params :archived})))
:non-nil #{:name :parameters :caveats :points_of_interest :show_in_getting_started :enable_embedding
:embedding_params :archived})))
;; now publish an event and return the updated Dashboard
(u/prog1 (Dashboard id)
(events/publish-event! :dashboard-update (assoc <> :actor_id api/*current-user-id*))))
;; TODO - We can probably remove this in the near future since it should no longer be needed now that we're going to be setting `:archived` to `true` via the `PUT` endpoint instead
;; TODO - We can probably remove this in the near future since it should no longer be needed now that we're going to
;; be setting `:archived` to `true` via the `PUT` endpoint instead
(api/defendpoint DELETE "/:id"
"Delete a `Dashboard`."
[id]
(log/warn "DELETE /api/dashboard/:id is deprecated. Instead of deleting a Dashboard, you should change its `archived` value via PUT /api/dashboard/:id.")
(log/warn (str "DELETE /api/dashboard/:id is deprecated. Instead of deleting a Dashboard, you should change its "
"`archived` value via PUT /api/dashboard/:id."))
(let [dashboard (api/write-check Dashboard id)]
(db/delete! Dashboard :id id)
(events/publish-event! :dashboard-delete (assoc dashboard :actor_id api/*current-user-id*)))
......@@ -283,7 +301,7 @@
:revision-id revision_id))
;;; ------------------------------------------------------------ Favoriting ------------------------------------------------------------
;;; --------------------------------------------------- Favoriting ---------------------------------------------------
(api/defendpoint POST "/:id/favorite"
"Favorite a Dashboard."
......@@ -301,12 +319,12 @@
api/generic-204-no-content)
;;; ------------------------------------------------------------ Sharing is Caring ------------------------------------------------------------
;;; ----------------------------------------------- Sharing is Caring ------------------------------------------------
(api/defendpoint POST "/:dashboard-id/public_link"
"Generate publically-accessible links for this Dashboard. Returns UUID to be used in public links.
(If this Dashboard has already been shared, it will return the existing public link rather than creating a new one.)
Public sharing must be enabled."
"Generate publically-accessible links for this Dashboard. Returns UUID to be used in public links. (If this
Dashboard has already been shared, it will return the existing public link rather than creating a new one.) Public
sharing must be enabled."
[dashboard-id]
(api/check-superuser)
(api/check-public-sharing-enabled)
......@@ -329,14 +347,16 @@
{:status 204, :body nil})
(api/defendpoint GET "/public"
"Fetch a list of Dashboards with public UUIDs. These dashboards are publically-accessible *if* public sharing is enabled."
"Fetch a list of Dashboards with public UUIDs. These dashboards are publically-accessible *if* public sharing is
enabled."
[]
(api/check-superuser)
(api/check-public-sharing-enabled)
(db/select [Dashboard :name :id :public_uuid], :public_uuid [:not= nil], :archived false))
(api/defendpoint GET "/embeddable"
"Fetch a list of Dashboards where `enable_embedding` is `true`. The dashboards can be embedded using the embedding endpoints and a signed JWT."
"Fetch a list of Dashboards where `enable_embedding` is `true`. The dashboards can be embedded using the embedding
endpoints and a signed JWT."
[]
(api/check-superuser)
(api/check-embedding-enabled)
......
src/metabase/api/database.clj
+ 24
− 12
View file @ f0794ae9
......@@ -150,7 +150,7 @@
[]))
;;; ------------------------------------------------------------ GET /api/database/:id ------------------------------------------------------------
;;; --------------------------------------------- GET /api/database/:id ----------------------------------------------
(def ExpandedSchedulesMap
"Schema for the `:schedules` key we add to the response containing 'expanded' versions of the CRON schedules.
......@@ -167,7 +167,8 @@
:metadata_sync (cron-util/cron-string->schedule-map (:metadata_sync_schedule db))})
(defn- add-expanded-schedules
"Add 'expanded' versions of the cron schedules strings for DB in a format that is appropriate for frontend consumption."
"Add 'expanded' versions of the cron schedules strings for DB in a format that is appropriate for frontend
consumption."
[db]
(assoc db :schedules (expanded-schedules db)))
......@@ -177,7 +178,7 @@
(add-expanded-schedules (api/read-check Database id)))
;;; ------------------------------------------------------------ GET /api/database/:id/metadata ------------------------------------------------------------
;;; ----------------------------------------- GET /api/database/:id/metadata -----------------------------------------
;; Since the normal `:id` param in the normal version of the endpoint will never match with negative numbers
;; we'll create another endpoint to specifically match the ID of the 'virtual' database. The `defendpoint` macro
......@@ -207,7 +208,7 @@
(db-metadata id))
;;; ------------------------------------------------------------ GET /api/database/:id/autocomplete_suggestions ------------------------------------------------------------
;;; --------------------------------- GET /api/database/:id/autocomplete_suggestions ---------------------------------
(defn- autocomplete-tables [db-id prefix]
(db/select [Table :id :db_id :schema :name]
......@@ -296,23 +297,34 @@
field m
:message m})
(defn- test-database-connection
(defn test-database-connection
"Try out the connection details for a database and useful error message if connection fails, returns `nil` if
connection succeeds."
[engine {:keys [host port] :as details}]
[engine {:keys [host port] :as details}, & {:keys [invalid-response-handler]
:or {invalid-response-handler invalid-connection-response}}]
;; This test is disabled for testing so we can save invalid databases, I guess (?) Not sure why this is this way :/
(when-not config/is-test?
(let [engine (keyword engine)
details (assoc details :engine engine)]
(try
(cond
(driver/can-connect-with-details? engine details :rethrow-exceptions) nil
(and host port (u/host-port-up? host port)) (invalid-connection-response :dbname (format "Connection to '%s:%d' successful, but could not connect to DB." host port))
(and host (u/host-up? host)) (invalid-connection-response :port (format "Connection to '%s' successful, but port %d is invalid." port))
host (invalid-connection-response :host (format "'%s' is not reachable" host))
:else (invalid-connection-response :db "Unable to connect to database."))
(driver/can-connect-with-details? engine details :rethrow-exceptions)
nil
(and host port (u/host-port-up? host port))
(invalid-response-handler :dbname (format "Connection to '%s:%d' successful, but could not connect to DB."
host port))
(and host (u/host-up? host))
(invalid-response-handler :port (format "Connection to '%s' successful, but port %d is invalid." port))
host
(invalid-response-handler :host (format "'%s' is not reachable" host))
:else
(invalid-response-handler :db "Unable to connect to database."))
(catch Throwable e
(invalid-connection-response :dbname (.getMessage e)))))))
(invalid-response-handler :dbname (.getMessage e)))))))
;; TODO - Just make `:ssl` a `feature`
(defn- supports-ssl?
......
src/metabase/api/embed.clj
+ 5
− 2
View file @ f0794ae9
......@@ -242,8 +242,11 @@
(api/check (:enable_embedding object)
[400 "Embedding is not enabled for this object."])))
(def ^:private ^{:arglists '([dashboard-id])} check-embedding-enabled-for-dashboard (partial check-embedding-enabled-for-object Dashboard))
(def ^:private ^{:arglists '([card-id])} check-embedding-enabled-for-card (partial check-embedding-enabled-for-object Card))
(def ^:private ^{:arglists '([dashboard-id])} check-embedding-enabled-for-dashboard
(partial check-embedding-enabled-for-object Dashboard))
(def ^:private ^{:arglists '([card-id])} check-embedding-enabled-for-card
(partial check-embedding-enabled-for-object Card))
;;; ------------------------------------------- /api/embed/card endpoints --------------------------------------------
......
src/metabase/api/field.clj
+ 16
− 17
View file @ f0794ae9
(ns metabase.api.field
(:require [compojure.core :refer [GET POST PUT DELETE]]
(:require [compojure.core :refer [DELETE GET POST PUT]]
[metabase.api.common :as api]
[metabase.db.metadata-queries :as metadata]
[metabase.models
[dimension :refer [Dimension]]
[field :as field :refer [Field]]
[field-values :as field-values :refer [create-field-values-if-needed! field-should-have-field-values? field-values->pairs FieldValues]]]
[field-values :as field-values :refer [FieldValues]]]
[metabase.util :as u]
[metabase.util.schema :as su]
[schema.core :as s]
......@@ -103,14 +103,14 @@
{:values []})
(api/defendpoint GET "/:id/values"
"If `Field`'s special type derives from `type/Category`, or its base type is `type/Boolean`, return
all distinct values of the field, and a map of human-readable values defined by the user."
"If `Field`'s special type derives from `type/Category`, or its base type is `type/Boolean`, return all distinct
values of the field, and a map of human-readable values defined by the user."
[id]
(let [field (api/read-check Field id)]
(if-let [field-values (and (field-should-have-field-values? field)
(create-field-values-if-needed! field))]
(if-let [field-values (and (field-values/field-should-have-field-values? field)
(field-values/create-field-values-if-needed! field))]
(-> field-values
(assoc :values (field-values->pairs field-values))
(assoc :values (field-values/field-values->pairs field-values))
(dissoc :human_readable_values))
{:values []})))
......@@ -147,17 +147,15 @@
;; match things like GET /field-literal%2Ccreated_at%2Ctype%2FDatetime/values
;; (this is how things like [field-literal,created_at,type/Datetime] look when URL-encoded)
(api/defendpoint GET "/field-literal%2C:field-name%2Ctype%2F:field-type/values"
"Implementation of the field values endpoint for fields in the Saved Questions 'virtual' DB.
This endpoint is just a convenience to simplify the frontend code. It just returns the standard
'empty' field values response."
"Implementation of the field values endpoint for fields in the Saved Questions 'virtual' DB. This endpoint is just a
convenience to simplify the frontend code. It just returns the standard 'empty' field values response."
;; we don't actually care what field-name or field-type are, so they're ignored
[_ _]
empty-field-values)
(defn- validate-human-readable-pairs
"Human readable values are optional, but if present they must be
present for each field value. Throws if invalid, returns a boolean
indicating whether human readable values were found."
"Human readable values are optional, but if present they must be present for each field value. Throws if invalid,
returns a boolean indicating whether human readable values were found."
[value-pairs]
(let [human-readable-missing? #(= ::not-found (get % 1 ::not-found))
has-human-readable-values? (not-any? human-readable-missing? value-pairs)]
......@@ -183,13 +181,14 @@
(map second value-pairs)))))
(api/defendpoint POST "/:id/values"
"Update the fields values and human-readable values for a `Field` whose special type is `category`/`city`/`state`/`country`
or whose base type is `type/Boolean`. The human-readable values are optional."
"Update the fields values and human-readable values for a `Field` whose special type is
`category`/`city`/`state`/`country` or whose base type is `type/Boolean`. The human-readable values are optional."
[id :as {{value-pairs :values} :body}]
{value-pairs [[(s/one s/Num "value") (s/optional su/NonBlankString "human readable value")]]}
(let [field (api/write-check Field id)]
(api/check (field-should-have-field-values? field)
[400 "You can only update the human readable values of a mapped values of a Field whose 'special_type' is 'category'/'city'/'state'/'country' or whose 'base_type' is 'type/Boolean'."])
(api/check (field-values/field-should-have-field-values? field)
[400 (str "You can only update the human readable values of a mapped values of a Field whose 'special_type' "
"is 'category'/'city'/'state'/'country' or whose 'base_type' is 'type/Boolean'.")])
(if-let [field-value-id (db/select-one-id FieldValues, :field_id id)]
(update-field-values! field-value-id value-pairs)
(create-field-values! field value-pairs)))
......
src/metabase/api/geojson.clj
+ 12
− 5
View file @ f0794ae9
......@@ -10,14 +10,16 @@
(defn- valid-json?
"Does this URL-OR-RESOURCE point to valid JSON?
URL-OR-RESOURCE should be something that can be passed to `slurp`, like an HTTP URL or a `java.net.URL` (which is what `io/resource` returns below)."
URL-OR-RESOURCE should be something that can be passed to `slurp`, like an HTTP URL or a `java.net.URL` (which is
what `io/resource` returns below)."
[url-or-resource]
(u/with-timeout 5000
(json/parse-string (slurp url-or-resource)))
true)
(defn- valid-json-resource?
"Does this RELATIVE-PATH point to a valid local JSON resource? (RELATIVE-PATH is something like \"app/assets/geojson/us-states.json\".)"
"Does this RELATIVE-PATH point to a valid local JSON resource? (RELATIVE-PATH is something like
\"app/assets/geojson/us-states.json\".)"
[relative-path]
(when-let [^java.net.URI uri (u/ignore-exceptions (java.net.URI. relative-path))]
(when-not (.isAbsolute uri)
......@@ -40,7 +42,10 @@
(def ^:private CustomGeoJSON
{s/Keyword {:name s/Str
:url (s/constrained s/Str valid-json-url-or-resource? "URL must point to a valid JSON file.")
:url (s/constrained
s/Str
valid-json-url-or-resource?
"URL must point to a valid JSON file.")
:region_key (s/maybe s/Str)
:region_name (s/maybe s/Str)
(s/optional-key :builtin) s/Bool}})
......@@ -58,7 +63,8 @@
:builtin true}})
(defsetting custom-geojson
"JSON containing information about custom GeoJSON files for use in map visualizations instead of the default US State or World GeoJSON."
"JSON containing information about custom GeoJSON files for use in map visualizations instead of the default US
State or World GeoJSON."
:type :json
:default {}
:getter (fn [] (merge (setting/get-json :custom-geojson) builtin-geojson))
......@@ -69,7 +75,8 @@
(defendpoint GET "/:key"
"Fetch a custom GeoJSON file as defined in the `custom-geojson` setting. (This just acts as a simple proxy for the file specified for KEY)."
"Fetch a custom GeoJSON file as defined in the `custom-geojson` setting. (This just acts as a simple proxy for the
file specified for KEY)."
[key]
{key su/NonBlankString}
(let [url (or (get-in (custom-geojson) [(keyword key) :url])
......
src/metabase/api/label.clj
+ 2
− 1
View file @ f0794ae9
......@@ -12,7 +12,8 @@
(defn warn-about-labels-being-deprecated
"Print a warning message about Labels-related endpoints being deprecated."
[]
(log/warn (u/format-color 'yellow "Labels are deprecated, and this API endpoint will be removed in a future version of Metabase.")))
(log/warn (u/format-color 'yellow
"Labels are deprecated, and this API endpoint will be removed in a future version of Metabase.")))
(defendpoint GET "/"
"[DEPRECATED] List all `Labels`. :label:"
......
src/metabase/api/ldap.clj
+ 1
− 0
View file @ f0794ae9
......@@ -108,4 +108,5 @@
{:status 500
:body (humanize-error-messages results)})))
(define-routes)
src/metabase/api/metric.clj
+ 2
− 1
View file @ f0794ae9
......@@ -56,7 +56,8 @@
(api/check-superuser)
(api/write-check Metric id)
(metric/update-metric!
(assoc (select-keys body #{:caveats :definition :description :how_is_this_calculated :name :points_of_interest :revision_message :show_in_getting_started})
(assoc (select-keys body #{:caveats :definition :description :how_is_this_calculated :name :points_of_interest
:revision_message :show_in_getting_started})
:id id)
api/*current-user-id*))
......
src/metabase/api/permissions.clj
+ 29
− 26
View file @ f0794ae9
......@@ -14,11 +14,11 @@
[db :as db]
[hydrate :refer [hydrate]]]))
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; | PERMISSIONS GRAPH ENDPOINTS |
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | PERMISSIONS GRAPH ENDPOINTS |
;;; +----------------------------------------------------------------------------------------------------------------+
;;; ---------------------------------------- DeJSONifaction ----------------------------------------
;;; ------------------------------------------------- DeJSONifaction -------------------------------------------------
(defn- ->int [id] (Integer/parseInt (name id)))
......@@ -44,12 +44,13 @@
{(->int group-id) (dejsonify-dbs dbs)})))
(defn- dejsonify-graph
"Fix the types in the graph when it comes in from the API, e.g. converting things like `\"none\"` to `:none` and parsing object keys as integers."
"Fix the types in the graph when it comes in from the API, e.g. converting things like `\"none\"` to `:none` and
parsing object keys as integers."
[graph]
(update graph :groups dejsonify-groups))
;;; ---------------------------------------- Endpoints ----------------------------------------
;;; --------------------------------------------------- Endpoints ----------------------------------------------------
(api/defendpoint GET "/graph"
"Fetch a graph of all Permissions."
......@@ -59,14 +60,14 @@
(api/defendpoint PUT "/graph"
"Do a batch update of Permissions by passing in a modified graph. This should return the same graph,
in the same format, that you got from `GET /api/permissions/graph`, with any changes made in the wherever neccesary.
This modified graph must correspond to the `PermissionsGraph` schema.
If successful, this endpoint returns the updated permissions graph; use this as a base for any further modifications.
Revisions to the permissions graph are tracked. If you fetch the permissions graph and some other third-party modifies it before you can submit
you revisions, the endpoint will instead make no changes andr eturn a 409 (Conflict) response. In this case, you should fetch the updated graph
and make desired changes to that."
"Do a batch update of Permissions by passing in a modified graph. This should return the same graph, in the same
format, that you got from `GET /api/permissions/graph`, with any changes made in the wherever neccesary. This
modified graph must correspond to the `PermissionsGraph` schema. If successful, this endpoint returns the updated
permissions graph; use this as a base for any further modifications.
Revisions to the permissions graph are tracked. If you fetch the permissions graph and some other third-party
modifies it before you can submit you revisions, the endpoint will instead make no changes and return a
409 (Conflict) response. In this case, you should fetch the updated graph and make desired changes to that."
[:as {body :body}]
{body su/Map}
(api/check-superuser)
......@@ -74,19 +75,20 @@
(perms/graph))
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; | PERMISSIONS GROUP ENDPOINTS |
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | PERMISSIONS GROUP ENDPOINTS |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- group-id->num-members
"Return a map of `PermissionsGroup` ID -> number of members in the group.
(This doesn't include entries for empty groups.)"
"Return a map of `PermissionsGroup` ID -> number of members in the group. (This doesn't include entries for empty
groups.)"
[]
(into {} (for [{:keys [group_id members]} (db/query {:select [[:pgm.group_id :group_id] [:%count.pgm.id :members]]
:from [[:permissions_group_membership :pgm]]
:left-join [[:core_user :user] [:= :pgm.user_id :user.id]]
:where [:= :user.is_active true]
:group-by [:pgm.group_id]})]
(into {} (for [{:keys [group_id members]} (db/query
{:select [[:pgm.group_id :group_id] [:%count.pgm.id :members]]
:from [[:permissions_group_membership :pgm]]
:left-join [[:core_user :user] [:= :pgm.user_id :user.id]]
:where [:= :user.is_active true]
:group-by [:pgm.group_id]})]
{group_id members})))
(defn- ordered-groups
......@@ -141,7 +143,7 @@
api/generic-204-no-content)
;;; ---------------------------------------- Group Membership Endpoints ----------------------------------------
;;; ------------------------------------------- Group Membership Endpoints -------------------------------------------
(api/defendpoint GET "/membership"
"Fetch a map describing the group memberships of various users.
......@@ -162,7 +164,8 @@
(db/insert! PermissionsGroupMembership
:group_id group_id
:user_id user_id)
;; TODO - it's a bit silly to return the entire list of members for the group, just return the newly created one and let the frontend add it ass appropriate
;; TODO - it's a bit silly to return the entire list of members for the group, just return the newly created one and
;; let the frontend add it ass appropriate
(group/members {:id group_id}))
(api/defendpoint DELETE "/membership/:id"
......
src/metabase/api/preview_embed.clj
+ 2
− 1
View file @ f0794ae9
......@@ -4,7 +4,8 @@
1. Require admin access
2. Ignore the values of `:enabled_embedding` for Cards/Dashboards
3. Ignore the `:embed_params` whitelist for Card/Dashboards, instead using a field called `:_embedding_params` in the JWT token itself.
3. Ignore the `:embed_params` whitelist for Card/Dashboards, instead using a field called `:_embedding_params` in
the JWT token itself.
Refer to the documentation for those endpoints for further details."
(:require [compojure.core :refer [GET]]
......
src/metabase/api/public.clj
+ 1
− 1
View file @ f0794ae9
......@@ -90,7 +90,7 @@
(dataset-api/as-format export-format
(run-query-for-card-with-public-uuid uuid parameters, :constraints nil)))
;;; ----------------------------------------------- Public Dashboards -----------------------------------------------
;;; ----------------------------------------------- Public Dashboards ------------------------------------------------
(defn public-dashboard
"Return a public Dashboard matching key-value CONDITIONS, removing all fields that should not be visible to the
......
src/metabase/api/routes.clj
+ 4
− 3
View file @ f0794ae9
......@@ -38,12 +38,13 @@
[metabase.middleware :as middleware]))
(def ^:private +generic-exceptions
"Wrap ROUTES so any Exception thrown is just returned as a generic 400, to prevent details from leaking in public endpoints."
"Wrap ROUTES so any Exception thrown is just returned as a generic 400, to prevent details from leaking in public
endpoints."
middleware/genericize-exceptions)
(def ^:private +message-only-exceptions
"Wrap ROUTES so any Exception thrown is just returned as a 400 with only the message from the original Exception (i.e., remove
the original stacktrace), to prevent details from leaking in public endpoints."
"Wrap ROUTES so any Exception thrown is just returned as a 400 with only the message from the original
Exception (i.e., remove the original stacktrace), to prevent details from leaking in public endpoints."
middleware/message-only-exceptions)
(def ^:private +apikey
......
src/metabase/api/segment.clj
+ 2
− 1
View file @ f0794ae9
......@@ -46,7 +46,8 @@
(api/check-superuser)
(api/write-check Segment id)
(segment/update-segment!
(assoc (select-keys body #{:name :description :caveats :points_of_interest :show_in_getting_started :definition :revision_message})
(assoc (select-keys body #{:name :description :caveats :points_of_interest :show_in_getting_started :definition
:revision_message})
:id id)
api/*current-user-id*))
......
src/metabase/api/session.clj
+ 19
− 13
View file @ f0794ae9
......@@ -38,10 +38,12 @@
(def ^:private login-throttlers
{:username (throttle/make-throttler :username)
:ip-address (throttle/make-throttler :username, :attempts-threshold 50)}) ; IP Address doesn't have an actual UI field so just show error by username
;; IP Address doesn't have an actual UI field so just show error by username
:ip-address (throttle/make-throttler :username, :attempts-threshold 50)})
(defn- ldap-login
"If LDAP is enabled and a matching user exists return a new Session for them, or `nil` if they couldn't be authenticated."
"If LDAP is enabled and a matching user exists return a new Session for them, or `nil` if they couldn't be
authenticated."
[username password]
(when (ldap/ldap-configured?)
(try
......@@ -86,10 +88,10 @@
(db/delete! Session :id session_id)
api/generic-204-no-content)
;; Reset tokens:
;; We need some way to match a plaintext token with the a user since the token stored in the DB is hashed.
;; So we'll make the plaintext token in the format USER-ID_RANDOM-UUID, e.g. "100_8a266560-e3a8-4dc1-9cd1-b4471dcd56d7", before hashing it.
;; "Leaking" the ID this way is ok because the plaintext token is only sent in the password reset email to the user in question.
;; Reset tokens: We need some way to match a plaintext token with the a user since the token stored in the DB is
;; hashed. So we'll make the plaintext token in the format USER-ID_RANDOM-UUID, e.g.
;; "100_8a266560-e3a8-4dc1-9cd1-b4471dcd56d7", before hashing it. "Leaking" the ID this way is ok because the
;; plaintext token is only sent in the password reset email to the user in question.
;;
;; There's also no need to salt the token because it's already random <3
......@@ -120,7 +122,8 @@
[^String token]
(when-let [[_ user-id] (re-matches #"(^\d+)_.+$" token)]
(let [user-id (Integer/parseInt user-id)]
(when-let [{:keys [reset_token reset_triggered], :as user} (db/select-one [User :id :last_login :reset_triggered :reset_token], :id user-id, :is_active true)]
(when-let [{:keys [reset_token reset_triggered], :as user} (db/select-one [User :id :last_login :reset_triggered :reset_token]
:id user-id, :is_active true)]
;; Make sure the plaintext token matches up with the hashed one for this user
(when (u/ignore-exceptions
(creds/bcrypt-verify token reset_token))
......@@ -159,10 +162,11 @@
(public-settings/public-settings))
;;; ------------------------------------------------------------ GOOGLE AUTH ------------------------------------------------------------
;;; -------------------------------------------------- GOOGLE AUTH ---------------------------------------------------
;; TODO - The more I look at all this code the more I think it should go in its own namespace. `metabase.integrations.google-auth` would be appropriate,
;; or `metabase.integrations.auth.google` if we decide to add more 3rd-party SSO options
;; TODO - The more I look at all this code the more I think it should go in its own namespace.
;; `metabase.integrations.google-auth` would be appropriate, or `metabase.integrations.auth.google` if we decide to
;; add more 3rd-party SSO options
(defsetting google-auth-client-id
"Client ID for Google Auth SSO. If this is set, Google Auth is considered to be enabled.")
......@@ -192,14 +196,16 @@
(defn- check-autocreate-user-allowed-for-email [email]
(when-not (autocreate-user-allowed-for-email? email)
;; Use some wacky status code (428 - Precondition Required) so we will know when to so the error screen specific to this situation
;; Use some wacky status code (428 - Precondition Required) so we will know when to so the error screen specific
;; to this situation
(throw (ex-info "You'll need an administrator to create a Metabase account before you can use Google to log in."
{:status-code 428}))))
(defn- google-auth-create-new-user! [first-name last-name email]
(check-autocreate-user-allowed-for-email email)
;; this will just give the user a random password; they can go reset it if they ever change their mind and want to log in without Google Auth;
;; this lets us keep the NOT NULL constraints on password / salt without having to make things hairy and only enforce those for non-Google Auth users
;; this will just give the user a random password; they can go reset it if they ever change their mind and want to
;; log in without Google Auth; this lets us keep the NOT NULL constraints on password / salt without having to make
;; things hairy and only enforce those for non-Google Auth users
(user/create-new-google-auth-user! first-name last-name email))
(defn- google-auth-fetch-or-create-user! [first-name last-name email]
......
src/metabase/api/setup.clj
+ 9
− 18
View file @ f0794ae9
......@@ -54,8 +54,10 @@
;; set a couple preferences
(public-settings/site-name site_name)
(public-settings/admin-email email)
(public-settings/anon-tracking-enabled (or (nil? allow_tracking) ; default to `true` if allow_tracking isn't specified
allow_tracking)) ; the setting will set itself correctly whether a boolean or boolean string is specified
;; default to `true` if allow_tracking isn't specified. The setting will set itself correctly whether a boolean or
;; boolean string is specified
(public-settings/anon-tracking-enabled (or (nil? allow_tracking)
allow_tracking))
;; setup database (if needed)
(when (driver/is-engine? engine)
(let [db (db/insert! Database
......@@ -83,25 +85,14 @@
(api/defendpoint POST "/validate"
"Validate that we can connect to a database given a set of details."
[:as {{{:keys [engine] {:keys [host port] :as details} :details} :details, token :token} :body}]
[:as {{{:keys [engine details]} :details, token :token} :body}]
{token SetupToken
engine DBEngineString}
(let [engine (keyword engine)
details (assoc details :engine engine)
response-invalid (fn [field m] {:status 400 :body (if (= :general field)
{:message m}
{:errors {field m}})})]
;; TODO - as @atte mentioned this should just use the same logic as we use in POST /api/database/, which tries with
;; both SSL and non-SSL.
(try
(cond
(driver/can-connect-with-details? engine details :rethrow-exceptions) {:valid true}
(and host port (u/host-port-up? host port)) (response-invalid :dbname (format "Connection to '%s:%d' successful, but could not connect to DB." host port))
(and host (u/host-up? host)) (response-invalid :port (format "Connection to '%s' successful, but port %d is invalid." port))
host (response-invalid :host (format "'%s' is not reachable" host))
:else (response-invalid :general "Unable to connect to database."))
(catch Throwable e
(response-invalid :general (.getMessage e))))))
invalid-response (fn [field m] {:status 400, :body (if (#{:dbname :port :host} field)
{:errors {field m}}
{:message m})})]
(database-api/test-database-connection engine details :invalid-response-handler invalid-response)))
;;; Admin Checklist
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment