diff --git a/project.clj b/project.clj
index c86d62e82ec1f63d633b751db5adac0cfbfc197d..edcfda7174218098ead35195ed7b933301a722c3 100644
--- a/project.clj
+++ b/project.clj
@@ -10,8 +10,7 @@
"test" ["with-profile" "+expectations" "expectations"]
"generate-sample-dataset" ["with-profile" "+generate-sample-dataset" "run"]
"profile" ["with-profile" "+profile" "run" "profile"]
- "h2" ["with-profile" "+h2-shell" "run" "-url" "jdbc:h2:./metabase.db" "-user" "" "-password" "" "-driver" "org.h2.Driver"]
- "validate-automagic-dashboards" ["with-profile" "+validate-automagic-dashboards" "run"]}
+ "h2" ["with-profile" "+h2-shell" "run" "-url" "jdbc:h2:./metabase.db" "-user" "" "-password" "" "-driver" "org.h2.Driver"]}
:dependencies [[org.clojure/clojure "1.8.0"]
[org.clojure/core.async "0.3.442"]
[org.clojure/core.match "0.3.0-alpha4"] ; optimized pattern matching library for Clojure
@@ -195,5 +194,4 @@
:profile {:jvm-opts ["-XX:+CITime" ; print time spent in JIT compiler
"-XX:+PrintGC"]} ; print a message when garbage collection takes place
;; get the H2 shell with 'lein h2'
- :h2-shell {:main org.h2.tools.Shell}
- :validate-automagic-dashboards {:main metabase.automagic-dashboards.rules}})
+ :h2-shell {:main org.h2.tools.Shell}})
diff --git a/src/metabase/api/automagic_dashboards.clj b/src/metabase/api/automagic_dashboards.clj
index acc12f9786b85fb87c17b898082285177bb55520..d46dce5a12d75d3ca3642d22d89ba9bdd59e5f66 100644
--- a/src/metabase/api/automagic_dashboards.clj
+++ b/src/metabase/api/automagic_dashboards.clj
@@ -30,20 +30,20 @@
(def ^:private Prefix
(su/with-api-error-message
- (->> ["table" "metric" "field"]
- (mapcat rules/load-rules)
- (filter :indepth)
- (map :rule)
- (apply s/enum))
+ (s/pred (fn [prefix]
+ (some #(not-empty (rules/get-rules [% prefix])) ["table" "metric" "field"])))
(tru "invalid value for prefix")))
(def ^:private Rule
(su/with-api-error-message
- (->> ["table" "metric" "field"]
- (mapcat rules/load-rules)
- (mapcat :indepth)
- (map :rule)
- (apply s/enum))
+ (s/pred (fn [rule]
+ (some (fn [toplevel]
+ (some (comp rules/get-rule
+ (fn [prefix]
+ [toplevel prefix rule])
+ :rule)
+ (rules/get-rules [toplevel])))
+ ["table" "metric" "field"])))
(tru "invalid value for rule name")))
(def ^:private ^{:arglists '([s])} decode-base64-json
@@ -51,13 +51,9 @@
(def ^:private Base64EncodedJSON
(su/with-api-error-message
- (s/pred decode-base64-json "valid base64 encoded json")
+ (s/pred decode-base64-json)
(tru "value couldn''t be parsed as base64 encoded JSON")))
-(defn- load-rule
- [entity prefix rule]
- (rules/load-rule (format "%s/%s/%s.yaml" entity prefix rule)))
-
(api/defendpoint GET "/database/:id/candidates"
"Return a list of candidates for automagic dashboards orderd by interestingness."
[id]
@@ -84,7 +80,7 @@
Table
api/check-404
(magic/automagic-analysis
- {:rule (load-rule "table" prefix rule)
+ {:rule ["table" prefix rule]
:show (keyword show)})))
(api/defendpoint GET "/segment/:id"
@@ -103,7 +99,7 @@
Segment
api/check-404
(magic/automagic-analysis
- {:rule (load-rule "table" prefix rule)
+ {:rule ["table" prefix rule]
:show (keyword show)})))
(api/defendpoint GET "/question/:id/cell/:cell-query"
@@ -130,7 +126,7 @@
Card
api/check-404
(magic/automagic-analysis {:show (keyword show)
- :rule (load-rule "table" prefix rule)
+ :rule ["table" prefix rule]
:cell-query (decode-base64-json cell-query)})))
(api/defendpoint GET "/metric/:id"
@@ -158,7 +154,7 @@
prefix Prefix
rule Rule}
(-> id Card api/check-404 (magic/automagic-analysis {:show (keyword show)
- :rule (load-rule "table" prefix rule)})))
+ :rule ["table" prefix rule]})))
(api/defendpoint GET "/adhoc/:query"
"Return an automagic dashboard analyzing ad hoc query."
@@ -181,7 +177,7 @@
decode-base64-json
query/adhoc-query
(magic/automagic-analysis {:show (keyword show)
- :rule (load-rule "table" prefix rule)})))
+ :rule ["table" prefix rule]})))
(api/defendpoint GET "/adhoc/:query/cell/:cell-query"
"Return an automagic dashboard analyzing ad hoc query."
@@ -211,7 +207,7 @@
query/adhoc-query
(magic/automagic-analysis {:show (keyword show)
:cell-query cell-query
- :rule (load-rule "table" prefix rule)}))))
+ :rule ["table" prefix rule]}))))
(def ^:private valid-comparison-pair?
#{["segment" "segment"]
diff --git a/src/metabase/automagic_dashboards/core.clj b/src/metabase/automagic_dashboards/core.clj
index 4f3ec4670ae0b9aa6fa3607d7cd47e09931dc8cd..861cc111e8157d2fa983c9b3fe7e2b9f8b649da6 100644
--- a/src/metabase/automagic_dashboards/core.clj
+++ b/src/metabase/automagic_dashboards/core.clj
@@ -568,7 +568,7 @@
[entity]
(let [root (->root entity)
rule (->> root
- (matching-rules (rules/load-rules (:rules-prefix root)))
+ (matching-rules (rules/get-rules [(:rules-prefix root)]))
first)
dashboard (make-dashboard root rule)]
{:url (:url root)
@@ -589,8 +589,7 @@
(defn- indepth
[root rule]
- (->> rule
- :indepth
+ (->> (rules/get-rules [(:rules-prefix root) (:rule rule)])
(keep (fn [indepth]
(when-let [[dashboard _] (apply-rule root indepth)]
{:title ((some-fn :short-title :title) dashboard)
@@ -609,9 +608,9 @@
"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 rule)
+ (apply-rule root (rules/get-rule rule))
(->> root
- (matching-rules (rules/load-rules rules-prefix))
+ (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).
@@ -765,7 +764,7 @@
interestingness of tables they contain (see above)."
([database] (candidate-tables database nil))
([database schema]
- (let [rules (rules/load-rules "table")]
+ (let [rules (rules/get-rules ["table"])]
(->> (apply db/select Table
(cond-> [:db_id (u/get-id database)
:visibility_type nil]
diff --git a/src/metabase/automagic_dashboards/rules.clj b/src/metabase/automagic_dashboards/rules.clj
index fb94ba1fe1093f2c0952be39386440a53325ab2a..21870cec9214866dca880ace49e77c472c013736 100644
--- a/src/metabase/automagic_dashboards/rules.clj
+++ b/src/metabase/automagic_dashboards/rules.clj
@@ -276,79 +276,73 @@
(def ^:private rules-dir "automagic_dashboards/")
-(def ^:private ^{:arglists '([f])} file->table-type
+(def ^:private ^{:arglists '([f])} file->entity-type
(comp (partial re-find #".+(?=\.yaml)") str (memfn ^Path getFileName)))
-(def ^:private ^{:arglists '([f])} file->parent-dir
- (comp last #(str/split % #"/") str (memfn ^Path getParent)))
-
-(defmacro ^:private with-resources
- [identifier & body]
- `(let [uri# (-> rules-dir io/resource .toURI)]
- (let [[fs# path#] (-> uri# .toString (str/split #"!" 2))]
- (if path#
- (with-open [^FileSystem ~identifier
- (-> fs#
- java.net.URI/create
- (FileSystems/newFileSystem (java.util.HashMap.)))]
- ~@body)
- (let [~identifier (FileSystems/getDefault)]
- ~@body)))))
-
-(defn- resource-path
- [^FileSystem fs path]
- (when-let [path (some->> path (str rules-dir) io/resource)]
- (let [path (if (-> path str (str/starts-with? "jar"))
- (-> path str (str/split #"!" 2) second)
- (.getPath path))]
- (.getPath fs path (into-array String [])))))
-
-(declare load-rules)
-
-(defn load-rule
- "Load and validate rule from file `f`."
- ([f]
- (with-resources fs
- (some->> f (resource-path fs) (load-rule fs))))
- ([fs ^Path f]
- (try
- (-> f
- .toUri
- slurp
- yaml/parse-string
- (assoc :rule (file->table-type f))
- (update :applies_to #(or % (file->table-type f)))
- rules-validator
- (assoc :indepth (load-rules fs (format "%s/%s"
- (file->parent-dir f)
- (file->table-type f)))))
- (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)))
- nil))))
-
-(defn load-rules
- "Load and validate all rules in dir."
- ([dir]
- (with-resources fs
- (load-rules fs dir)))
- ([fs dir]
- (when-let [dir (resource-path fs dir)]
- (with-open [ds (Files/newDirectoryStream dir)]
- (->> ds
- (filter #(str/ends-with? (.toString ^Path %) ".yaml"))
- (keep (partial load-rule fs))
- doall)))))
-
-(defn -main
- "Entry point for lein task `validate-automagic-dashboards`"
- [& _]
- (dorun (load-rules "tables"))
- (dorun (load-rules "metrics"))
- (dorun (load-rules "fields"))
- (System/exit 0))
+(defn- load-rule
+ [^Path f]
+ (try
+ (let [entity-type (file->entity-type f)]
+ (-> f
+ .toUri
+ slurp
+ yaml/parse-string
+ (assoc :rule entity-type)
+ (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)))
+ nil)))
+
+(defn- load-rule-dir
+ ([dir] (load-rule-dir dir [] {}))
+ ([dir path rules]
+ (with-open [ds (Files/newDirectoryStream dir)]
+ (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)
+
+ (file->entity-type f)
+ (assoc-in rules (concat path [(file->entity-type f) ::leaf]) (load-rule f))
+
+ :else
+ rules))
+ rules
+ ds))))
+
+(defmacro ^:private with-resource
+ [[identifier path] & body]
+ `(let [[jar# path#] (-> ~path .toString (str/split #"!" 2))]
+ (if path#
+ (with-open [^FileSystem fs# (-> jar#
+ java.net.URI/create
+ (FileSystems/newFileSystem (java.util.HashMap.)))]
+ (let [~identifier (.getPath fs# path# (into-array String []))]
+ ~@body))
+ (let [~identifier (.getPath (FileSystems/getDefault) (.getPath ~path) (into-array String []))]
+ ~@body))))
+
+(def ^:private rules (delay
+ (with-resource [path (-> rules-dir io/resource .toURI)]
+ (into {} (load-rule-dir path)))))
+
+(defn get-rules
+ "Get all rules with prefix `prefix`.
+ prefix is greedy, so [\"table\"] will match table/TransactionTable.yaml, but not
+ table/TransactionTable/ByCountry.yaml"
+ [prefix]
+ (->> prefix
+ (get-in @rules)
+ (keep (comp ::leaf val))))
+
+(defn get-rule
+ "Get rule at path `path`."
+ [path]
+ (get-in @rules (concat path [::leaf])))
diff --git a/test/metabase/automagic_dashboards/core_test.clj b/test/metabase/automagic_dashboards/core_test.clj
index 958899fbb09cbc90dc111658fd611695e8e68d0a..b19692f38655c4286ac0758029dc72f2a1bb09f4 100644
--- a/test/metabase/automagic_dashboards/core_test.clj
+++ b/test/metabase/automagic_dashboards/core_test.clj
@@ -50,7 +50,7 @@
(->> (data/id :users)
Table
(#'magic/->root)
- (#'magic/matching-rules (rules/load-rules "table"))
+ (#'magic/matching-rules (rules/get-rules ["table"]))
(map (comp first :applies_to))))
;; Test fallback to GenericTable
diff --git a/test/metabase/automagic_dashboards/rules_test.clj b/test/metabase/automagic_dashboards/rules_test.clj
index 67dffe23eca2f9dd1fe08aa46ff747719797e964..962e9428256c35bc4c9bd2df500253dec7eab0fd 100644
--- a/test/metabase/automagic_dashboards/rules_test.clj
+++ b/test/metabase/automagic_dashboards/rules_test.clj
@@ -14,9 +14,12 @@
(expect "ga:foo" (#'rules/->type "ga:foo"))
(expect :type/Foo (#'rules/->type "Foo"))
-(expect (every? some? (load-rules "table")))
-(expect (every? some? (load-rules "metrics")))
-(expect (every? some? (load-rules "fields")))
+;; This also tests that all the rules are valid (else there would be nils returned)
+(expect (every? some? (get-rules ["table"])))
+(expect (every? some? (get-rules ["metrics"])))
+(expect (every? some? (get-rules ["fields"])))
+
+(expect (some? (get-rules ["table" "GenericTable" "ByCountry"])))
(expect true (dimension-form? [:dimension "Foo"]))
(expect true (dimension-form? ["dimension" "Foo"]))