Fix rule loading

src/metabase/automagic_dashboards/core.clj

@@ -32,6 +32,7 @@
             [metabase.util :as u]
             [puppetlabs.i18n.core :as i18n :refer [tru]]
             [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,48 @@
 (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/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))}]
+                                             []))))))
+    (throw (ex-info (format "Can't create dashboard for %s" full-name)
+             {:path            (or rule rules-prefix)
+              :path-type       (if rule
+                                 :path
+                                 :prefix)
+              :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 +708,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 +741,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/rules.clj

@@ -12,7 +12,7 @@
              [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."
@@ -186,7 +186,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]
@@ -234,7 +235,7 @@
 
 (def ^:private rules-validator
   (sc/coercer!
-   Rules
+   Rule
    {[s/Str]         ensure-seq
     [OrderByPair]   ensure-seq
     OrderByPair     (fn [x]
@@ -277,7 +278,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]
@@ -300,6 +301,12 @@
                              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 +314,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))