Skip to content
Snippets Groups Projects
Unverified Commit 0de45854 authored by Sameer Al-Sakran's avatar Sameer Al-Sakran Committed by GitHub
Browse files

Merge pull request #7631 from metabase/fix-rule-loading 

Fix rule loading
parents 746b2c1f 9d0666b2
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/automagic_dashboards/core.clj
+ 65
− 58
View file @ 0de45854
......@@ -30,8 +30,9 @@
[metabase.related :as related]
[metabase.sync.analyze.classify :as classify]
[metabase.util :as u]
[puppetlabs.i18n.core :as i18n :refer [tru]]
[puppetlabs.i18n.core :as i18n :refer [tru trs]]
[ring.util.codec :as codec]
[schema.core :as s]
[toucan.db :as db]))
(def ^:private public-endpoint "/auto/dashboard/")
......@@ -50,7 +51,7 @@
:source table
:database (:db_id table)
:url (format "%stable/%s" public-endpoint (u/get-id table))
:rules-prefix "table"})
:rules-prefix ["table"]})
(defmethod ->root (type Segment)
[segment]
......@@ -61,7 +62,7 @@
:database (:db_id table)
:query-filter (-> segment :definition :filter)
:url (format "%ssegment/%s" public-endpoint (u/get-id segment))
:rules-prefix "table"}))
:rules-prefix ["table"]}))
(defmethod ->root (type Metric)
[metric]
......@@ -71,7 +72,7 @@
:source table
:database (:db_id table)
:url (format "%smetric/%s" public-endpoint (u/get-id metric))
:rules-prefix "metric"}))
:rules-prefix ["metric"]}))
(defmethod ->root (type Field)
[field]
......@@ -81,7 +82,7 @@
:source table
:database (:db_id table)
:url (format "%sfield/%s" public-endpoint (u/get-id field))
:rules-prefix "field"}))
:rules-prefix ["field"]}))
(defmulti
^{:doc "Get a reference for a given model to be injected into a template
......@@ -420,14 +421,15 @@
(assoc :score score
:dataset_query query))))))))
(def ^:private ^{:arglists '([rule])} rule-specificity
(comp (partial transduce (map (comp count ancestors)) +) :applies_to))
(s/defn ^:private rule-specificity
[rule :- rules/Rule]
(transduce (map (comp count ancestors)) + (:applies_to rule)))
(defn- matching-rules
(s/defn ^:private matching-rules
"Return matching rules orderd by specificity.
Most specific is defined as entity type specification the longest ancestor
chain."
[rules {:keys [source entity]}]
[rules :- [rules/Rule], {:keys [source entity]}]
(let [table-type (or (:entity_type source) :entity/GenericTable)]
(->> rules
(filter (fn [{:keys [applies_to]}]
......@@ -479,8 +481,8 @@
[context _]
context)
(defn- make-context
[root rule]
(s/defn ^:private make-context
[root, rule :- rules/Rule]
{:pre [(:source root)]}
(let [source (:source root)
tables (concat [source] (when (instance? (type Table) source)
......@@ -524,10 +526,10 @@
vals
(apply concat)))
(defn- make-dashboard
([root rule]
(s/defn ^:private make-dashboard
([root, rule :- rules/Rule]
(make-dashboard root rule {:tables [(:source root)]}))
([root rule context]
([root, rule :- rules/Rule, context]
(let [this {"this" (-> root
:entity
(assoc :full-name (:full-name root)))}]
......@@ -540,8 +542,8 @@
(update :groups (partial fill-templates :string context {}))
(assoc :refinements (:cell-query root))))))
(defn- apply-rule
[root rule]
(s/defn ^:private apply-rule
[root, rule :- rules/Rule]
(let [context (make-context root rule)
dashboard (make-dashboard root rule context)
filters (->> rule
......@@ -568,7 +570,7 @@
[entity]
(let [root (->root entity)
rule (->> root
(matching-rules (rules/get-rules [(:rules-prefix root)]))
(matching-rules (rules/get-rules (:rules-prefix root)))
first)
dashboard (make-dashboard root rule)]
{:url (:url root)
......@@ -587,9 +589,9 @@
(take n)
(map ->related-entity)))))
(defn- indepth
[root rule]
(->> (rules/get-rules [(:rules-prefix root) (:rule rule)])
(s/defn ^:private indepth
[root, rule :- rules/Rule]
(->> (rules/get-rules (concat (:rules-prefix root) [(:rule rule)]))
(keep (fn [indepth]
(when-let [[dashboard _] (apply-rule root indepth)]
{:title ((some-fn :short-title :title) dashboard)
......@@ -598,8 +600,8 @@
(:rule indepth))})))
(take max-related)))
(defn- related
[root rule]
(s/defn ^:private related
[root, rule :- rules/Rule]
(let [indepth (indepth root rule)]
{:indepth indepth
:tables (take (- max-related (count indepth)) (others root))}))
......@@ -607,40 +609,45 @@
(defn- automagic-dashboard
"Create dashboards for table `root` using the best matching heuristics."
[{:keys [rule show rules-prefix query-filter cell-query full-name] :as root}]
(when-let [[dashboard rule] (if rule
(apply-rule root (rules/get-rule rule))
(->> root
(matching-rules (rules/get-rules [rules-prefix]))
(keep (partial apply-rule root))
;; `matching-rules` returns an `ArraySeq` (via `sort-by`) so
;; `first` realises one element at a time (no chunking).
first))]
(log/info (format "Applying heuristic %s to %s." (:rule rule) full-name))
(log/info (format "Dimensions bindings:\n%s"
(->> dashboard
:context
:dimensions
(m/map-vals #(update % :matches (partial map :name)))
u/pprint-to-str)))
(log/info (format "Using definitions:\nMetrics:\n%s\nFilters:\n%s"
(-> dashboard :context :metrics u/pprint-to-str)
(-> dashboard :context :filters u/pprint-to-str)))
(-> (cond-> dashboard
(or query-filter cell-query)
(assoc :title (str (tru "A closer look at ") full-name)))
(populate/create-dashboard (or show max-cards))
(assoc :related (-> (related root rule)
(assoc :more (if (and (-> dashboard
:cards
count
(> max-cards))
(not= show :all))
[{:title (tru "Show more about this")
:description nil
:table (:source root)
:url (format "%s#show=all"
(:url root))}]
[])))))))
(if-let [[dashboard rule] (if rule
(apply-rule root (rules/get-rule rule))
(->> root
(matching-rules (rules/get-rules rules-prefix))
(keep (partial apply-rule root))
;; `matching-rules` returns an `ArraySeq` (via `sort-by`) so
;; `first` realises one element at a time (no chunking).
first))]
(do
(log/infof (trs "Applying heuristic %s to %s.") (:rule rule) full-name)
(log/infof (trs "Dimensions bindings:\n%s")
(->> dashboard
:context
:dimensions
(m/map-vals #(update % :matches (partial map :name)))
u/pprint-to-str))
(log/infof (trs "Using definitions:\nMetrics:\n%s\nFilters:\n%s")
(-> dashboard :context :metrics u/pprint-to-str)
(-> dashboard :context :filters u/pprint-to-str))
(-> (cond-> dashboard
(or query-filter cell-query)
(assoc :title (str (tru "A closer look at ") full-name)))
(populate/create-dashboard (or show max-cards))
(assoc :related (-> (related root rule)
(assoc :more (if (and (-> dashboard
:cards
count
(> max-cards))
(not= show :all))
[{:title (tru "Show more about this")
:description nil
:table (:source root)
:url (format "%s#show=all"
(:url root))}]
[]))))))
(throw (ex-info (format (trs "Can't create dashboard for %s") full-name)
{:root root
:available-rules (map :rule (or (some-> rule rules/get-rule vector)
(rules/get-rules rules-prefix)))}))))
(def ^:private ^{:arglists '([card])} table-like?
(comp empty? #(qp.util/get-in-normalized % [:dataset_query :query :aggregation])))
......@@ -698,7 +705,7 @@
(u/get-id card)
(encode-base64-json cell-query))
(format "%squestion/%s" public-endpoint (u/get-id card)))
:rules-prefix "table"}
:rules-prefix ["table"]}
opts)))
nil))
......@@ -731,7 +738,7 @@
(encode-base64-json cell-query))
(format "%sadhoc/%s" public-endpoint
(encode-base64-json query)))
:rules-prefix "table"}
:rules-prefix ["table"]}
(update opts :cell-query merge-filter-clauses
(qp.util/get-in-normalized query [:dataset_query :query :filter])))))
nil))
......
src/metabase/automagic_dashboards/populate.clj
+ 5
− 4
View file @ 0de45854
......@@ -6,6 +6,7 @@
[metabase.automagic-dashboards.filters :as magic.filters]
[metabase.models.card :as card]
[metabase.query-processor.util :as qp.util]
[puppetlabs.i18n.core :as i18n :refer [trs]]
[toucan.db :as db]))
(def ^Long ^:const grid-width
......@@ -252,10 +253,10 @@
;; Height doesn't need to be precise, just some
;; safe upper bound.
(make-grid grid-width (* n grid-width))]))]
(log/info (format "Adding %s cards to dashboard %s:\n%s"
(count cards)
title
(str/join "; " (map :title cards))))
(log/infof (trs "Adding %s cards to dashboard %s:\n%s")
(count cards)
title
(str/join "; " (map :title cards)))
(cond-> dashboard
(not-empty filters) (magic.filters/add-filters filters max-filters)))))
......
src/metabase/automagic_dashboards/rules.clj
+ 29
− 21
View file @ 0de45854
......@@ -7,19 +7,20 @@
[metabase.types]
[metabase.util :as u]
[metabase.util.schema :as su]
[puppetlabs.i18n.core :as i18n :refer [trs]]
[schema
[coerce :as sc]
[core :as s]]
[yaml.core :as yaml])
(:import java.nio.file.Path java.nio.file.FileSystems java.nio.file.FileSystem
java.nio.file.Files ))
java.nio.file.Files))
(def ^Long ^:const max-score
"Maximal (and default) value for heuristics scores."
100)
(def ^:private Score (s/constrained s/Int #(<= 0 % max-score)
(str "0 <= score <= " max-score)))
(format (trs "0 <= score <= %s") max-score)))
(def ^:private MBQL [s/Any])
......@@ -80,7 +81,7 @@
(def ^:private Visualization [(s/one s/Str "visualization") su/Map])
(def ^:private Width (s/constrained s/Int #(<= 1 % populate/grid-width)
(format "1 <= width <= %s"
(format (trs "1 <= width <= %s")
populate/grid-width)))
(def ^:private Height (s/constrained s/Int pos?))
......@@ -186,7 +187,8 @@
schema
(partition 2 constraints)))
(def ^:private Rules
(def Rule
"Rules defining an automagic dashboard."
(constrained-all
{(s/required-key :title) s/Str
(s/required-key :dimensions) [Dimension]
......@@ -201,13 +203,13 @@
(s/optional-key :groups) Groups
(s/optional-key :indepth) [s/Any]
(s/optional-key :dashboard_filters) [s/Str]}
valid-metrics-references? "Valid metrics references"
valid-filters-references? "Valid filters references"
valid-group-references? "Valid group references"
valid-order-by-references? "Valid order_by references"
valid-dashboard-filters-references? "Valid dashboard filters references"
valid-dimension-references? "Valid dimension references"
valid-breakout-dimension-references? "Valid card dimension references"))
valid-metrics-references? (trs "Valid metrics references")
valid-filters-references? (trs "Valid filters references")
valid-group-references? (trs "Valid group references")
valid-order-by-references? (trs "Valid order_by references")
valid-dashboard-filters-references? (trs "Valid dashboard filters references")
valid-dimension-references? (trs "Valid dimension references")
valid-breakout-dimension-references? (trs "Valid card dimension references")))
(defn- with-defaults
[defaults]
......@@ -234,7 +236,7 @@
(def ^:private rules-validator
(sc/coercer!
Rules
Rule
{[s/Str] ensure-seq
[OrderByPair] ensure-seq
OrderByPair (fn [x]
......@@ -277,7 +279,7 @@
(def ^:private rules-dir "automagic_dashboards/")
(def ^:private ^{:arglists '([f])} file->entity-type
(comp (partial re-find #".+(?=\.yaml)") str (memfn ^Path getFileName)))
(comp (partial re-find #".+(?=\.yaml$)") str (memfn ^Path getFileName)))
(defn- load-rule
[^Path f]
......@@ -291,15 +293,21 @@
(update :applies_to #(or % entity-type))
rules-validator))
(catch Exception e
(log/error (format "Error parsing %s:\n%s"
(.getFileName f)
(or (some-> e
ex-data
(select-keys [:error :value])
u/pprint-to-str)
e)))
(log/errorf (trs "Error parsing %s:\n%s")
(.getFileName f)
(or (some-> e
ex-data
(select-keys [:error :value])
u/pprint-to-str)
e))
nil)))
(defn- trim-trailing-slash
[s]
(if (str/ends-with? s "/")
(subs s 0 (-> s count dec))
s))
(defn- load-rule-dir
([dir] (load-rule-dir dir [] {}))
([dir path rules]
......@@ -307,7 +315,7 @@
(reduce (fn [rules ^Path f]
(cond
(Files/isDirectory f (into-array java.nio.file.LinkOption []))
(load-rule-dir f (conj path (str (.getFileName f))) rules)
(load-rule-dir f (->> f (.getFileName) str trim-trailing-slash (conj path)) rules)
(file->entity-type f)
(assoc-in rules (concat path [(file->entity-type f) ::leaf]) (load-rule f))
......
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