Skip to content
Snippets Groups Projects
Unverified Commit 19f6d315 authored by dpsutton's avatar dpsutton Committed by GitHub
Browse files

Suppress some route logs on startup (#35120)

Annoying warnings on startup

;#### Before

```shell
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/rule/:prefix/:dashboard-template [prefix Prefix]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/rule/:prefix/:dashboard-template [dashboard-template DashboardTemplate]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/cell/:cell-query [cell-query Base64EncodedJSON]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/cell/:cell-query/rule/:prefix/:dashboard-template [prefix Prefix]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/cell/:cell-query/rule/:prefix/:dashboard-template [dashboard-template DashboardTemplate]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/cell/:cell-query/rule/:prefix/:dashboard-template [cell-query Base64EncodedJSON]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/rule/:prefix/:dashboard-template/compare/:comparison-entity/:comparison-entity-id-or-query [prefix Prefix]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/rule/:prefix/:dashboard-template/compare/:comparison-entity/:comparison-entity-id-or-query [dashboard-template DashboardTemplate]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/cell/:cell-query/compare/:comparison-entity/:comparison-entity-id-or-query [cell-query Base64EncodedJSON]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/cell/:cell-query/rule/:prefix/:dashboard-template/compare/:comparison-entity/:comparison-entity-id-or-query [prefix Prefix]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/cell/:cell-query/rule/:prefix/:dashboard-template/compare/:comparison-entity/:comparison-entity-id-or-query [dashboard-template DashboardTemplate]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:entity/:entity-id-or-query/cell/:cell-query/rule/:prefix/:dashboard-template/compare/:comparison-entity/:comparison-entity-id-or-query [cell-query Base64EncodedJSON]
 Either add :fn to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:key [key kebab-cased-keyword]
 Either add :keyword to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:key [key kebab-cased-keyword]
 Either add :keyword to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:zoom/:x/:y/:lat-field/:lon-field [lat-field :string]
 Either add :string to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
 Warning: missing route-param regex for schema: /:zoom/:x/:y/:lat-field/:lon-field [lon-field :string]
 Either add :string to metabase.api.common.internal/->matching-regex or metabase.api.common.internal/no-regex-schemas.
```

;#### After

;#### Fixes

;###### :fn

We logged lots of startups that we didn't have a regex for the `:fn` type validator in routes. And this is generally not possible. So we should generally just ignore function based validators, as we're not going to write a regex for what a function validates/accepts. All of these `:fn` matchers are in the x-ray namespace matching base64 encoded json, an existing dashboard template prefix (`(malli.core/validate Prefix "TransactionTable")`),

;###### `:keyword`: Add a regex `#"[\S]+"` that just looks for non space characters

;###### String matches `:string`

```clojure
(def ^:private kebab-cased-keyword
  "Keyword that can be transformed from \"a_b\" -> :a-b"
  [:keyword {:decode/json #(keyword (u/->kebab-case-en %))}])

...

(api/defendpoint GET "/:key"
  "Fetch a single `Setting`."
  [key]
  {key kebab-cased-keyword}
  (with-setting-access-control
    (setting/user-facing-value key)))
```

We don't want a regex for this. Whatever we are currently accepting in the route matching is our desired behavior. Arbitrary strings don't have a good regex and we don't want to restrict what the route currently does.

```clojure
(api/defendpoint GET "/:zoom/:x/:y/:lat-field/:lon-field"
  "This endpoints provides an image with the appropriate pins rendered given a MBQL `query` (passed as a GET query
  string param). We evaluate the query and find the set of lat/lon pairs which are relevant and then render the
  appropriate ones. It's expected that to render a full map view several calls will be made to this endpoint in
  parallel."
  [zoom x y lat-field lon-field query]
  {zoom        ms/Int
   x           ms/Int
   y           ms/Int
   lat-field   :string
   lon-field   :string
   query       ms/JSONString} ...)
```
parent 54485cc8
Branches
Tags
No related merge requests found
......@@ -206,6 +206,7 @@
:re (first (try (mc/children schema)
(catch clojure.lang.ExceptionInfo _
(mc/children #_:clj-kondo/ignore (eval schema)))))
:keyword #"[\S]+"
'pos-int? #"[0-9]+"
:int #"-?[0-9]+"
'int? #"-?[0-9]+"
......@@ -215,7 +216,9 @@
(def ^:private no-regex-schemas #{(mc/type ms/NonBlankString)
(mc/type (mc/schema [:maybe ms/PositiveInt]))
(mc/type [:enum "a" "b"])})
(mc/type [:enum "a" "b"])
:fn
:string})
(defn add-route-param-schema
"Expand a `route` string like \"/:id\" into a Compojure route form with regexes to match parameters based on
......@@ -227,25 +230,25 @@
(if (vector? route)
route
(let [[wildcard & wildcards]
(->> "metabase.api.common.internal/no-regex-schemas."
(str "Either add " (pr-str schema-type) " to "
"metabase.api.common.internal/->matching-regex or ")
colorize/green
log/warn
(when (and config/is-dev? (not (contains? no-regex-schemas schema-type)))
(log/warn (colorize/red (str "Warning: missing route-param regex for schema: "
route " " [k schema]))))
(if re
[route (keyword k) re])
(for [[k schema] arg->schema
:when (re-find (re-pattern (str ":" k)) route)
:let [[schema-type re] (->matching-regex schema)]])
(->> (for [[k schema] arg->schema
:when (re-find (re-pattern (str ":" k)) route)
:let [[schema-type re] (->matching-regex schema)]]
(if re
[route (keyword k) re]
(when (and config/is-dev? (not (contains? no-regex-schemas schema-type)))
(let [overview (str "Warning: missing route-param regex for schema: "
route " " [k schema])
fix (str "Either add `" (pr-str schema-type) "` to "
"metabase.api.common.internal/->matching-regex or "
"metabase.api.common.internal/no-regex-schemas.")]
(log/warn (colorize/red overview))
(log/warn (colorize/green fix))))))
(remove nil?))]
(cond
;; multiple hits -> tack them onto the original route shape.
wildcards (vec (reduce into wildcard (mapv #(drop 1 %) wildcards)))
wildcard wildcard
:else route))))
wildcard wildcard
:else route))))
;;; ## ROUTE ARG AUTO PARSING
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment