From cd0da5a16e405e9a53f0ab58278ee34c43b95649 Mon Sep 17 00:00:00 2001
From: Cam Saul <cam@geotip.com>
Date: Mon, 6 Jul 2015 15:48:05 -0700
Subject: [PATCH] Add some error checking to hydrate; complain when nested
hydration vectors are used incorrectly
---
src/metabase/api/meta/table.clj | 2 +-
src/metabase/models/hydrate.clj | 5 ++++-
test/metabase/models/hydrate_test.clj | 12 +++++++-----
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/metabase/api/meta/table.clj b/src/metabase/api/meta/table.clj
index 07e83da6196..552bac779f0 100644
--- a/src/metabase/api/meta/table.clj
+++ b/src/metabase/api/meta/table.clj
@@ -62,7 +62,7 @@
{include_sensitive_fields String->Boolean}
(->404 (sel :one Table :id id)
read-check
- (hydrate :db [:fields [:target]] :field_values)
+ (hydrate :db [:fields :target] :field_values)
(update-in [:fields] (if include_sensitive_fields
;; If someone passes include_sensitive_fields return hydrated :fields as-is
identity
diff --git a/src/metabase/models/hydrate.clj b/src/metabase/models/hydrate.clj
index 1c684efec9f..b5f81f4cc74 100644
--- a/src/metabase/models/hydrate.clj
+++ b/src/metabase/models/hydrate.clj
@@ -117,7 +117,10 @@
(defn- hydrate-vector
"Hydrate a nested hydration form (vector) by recursively calling `hydrate`."
- [results [k & more]]
+ [results [k & more :as vect]]
+ ;; TODO - it would be super snazzy if we could make this a compile-time check
+ (assert (> (count vect) 1)
+ (format "Replace '%s' with '%s'. Vectors are for nested hydration. There's no need to use one when you only have a single key." vect (first vect)))
(let [results (hydrate results k)]
(if-not (seq more) results
(counts-apply results k #(apply hydrate % more)))))
diff --git a/test/metabase/models/hydrate_test.clj b/test/metabase/models/hydrate_test.clj
index bf98426bc2b..af9098d703a 100644
--- a/test/metabase/models/hydrate_test.clj
+++ b/test/metabase/models/hydrate_test.clj
@@ -237,11 +237,13 @@
:b d2}
:b))
-;; specifying "nested" hydration with no "nested" keys should still work
-(expect {:a 1 :b 2}
- (hydrate {:a 1
- :b d2}
- [:b]))
+;; specifying "nested" hydration with no "nested" keys should throw an exception and tell you not to do it
+(expect "Assert failed: Replace '[:b]' with ':b'. Vectors are for nested hydration. There's no need to use one when you only have a single key.\n(> (count vect) 1)"
+ (try (hydrate {:a 1
+ :b d2}
+ [:b])
+ (catch Throwable e
+ (.getMessage e))))
;; check that returning an array works correctly
(expect {:c [1 2 3]}
--
GitLab