diff --git a/frontend/src/metabase/lib/core.js b/frontend/src/metabase/lib/core.js
index 3249404b8d8e51d5962c6df8c8e36487ff02ba62..06cedb96b4c76f49b4a5c5bddf54d26f8f976523 100644
--- a/frontend/src/metabase/lib/core.js
+++ b/frontend/src/metabase/lib/core.js
@@ -88,20 +88,6 @@ export const field_special_types = [{
export const field_special_types_map = field_special_types
.reduce((map, type) => Object.assign({}, map, {[type.id]: type}), {});
-export const field_field_types = [{
- 'id': 'info',
- 'name': 'Information',
- 'description': 'Non-numerical value that is not meant to be used.'
-}, {
- 'id': 'metric',
- 'name': 'Metric',
- 'description': 'A number that can be added, graphed, etc.'
-}, {
- 'id': 'dimension',
- 'name': 'Dimension',
- 'description': 'A high or low-cardinality numerical string value that is meant to be used as a grouping.'
-}];
-
export const field_visibility_types = [{
'id': 'normal',
'name': 'Everywhere',
diff --git a/frontend/src/metabase/lib/query.js b/frontend/src/metabase/lib/query.js
index c70be0e9a1de6fe3a17338e12f22d695fe62a33e..7e4ed6eb86cee0848b37bfc48519383695c76842 100644
--- a/frontend/src/metabase/lib/query.js
+++ b/frontend/src/metabase/lib/query.js
@@ -521,7 +521,6 @@ var Query = {
operators_lookup: {},
valid_operators: [],
active: true,
- field_type: "info",
fk_target_field_id: null,
parent_id: null,
preview_display: true,
diff --git a/resources/migrations/041_drop_field_field_type.yaml b/resources/migrations/041_drop_field_field_type.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..8b40de6dc7421bdf5a0e790d2d2f4e537a93ac48
--- /dev/null
+++ b/resources/migrations/041_drop_field_field_type.yaml
@@ -0,0 +1,24 @@
+databaseChangeLog:
+ - changeSet:
+ id: 41
+ author: camsaul
+ changes:
+ - dropColumn:
+ tableName: metabase_field
+ columnName: field_type
+ - addDefaultValue:
+ tableName: metabase_field
+ columnName: active
+ defaultValueBoolean: true
+ - addDefaultValue:
+ tableName: metabase_field
+ columnName: preview_display
+ defaultValueBoolean: true
+ - addDefaultValue:
+ tableName: metabase_field
+ columnName: position
+ defaultValueNumeric: 0
+ - addDefaultValue:
+ tableName: metabase_field
+ columnName: visibility_type
+ defaultValue: "normal"
diff --git a/resources/migrations/liquibase.json b/resources/migrations/liquibase.json
index c79840f627546fc2c799e04c64392c0c34da854a..6cfa2675026dc0274b60cb3a54900735ce04a797 100644
--- a/resources/migrations/liquibase.json
+++ b/resources/migrations/liquibase.json
@@ -37,6 +37,7 @@
{"include": {"file": "migrations/036_add_dashboard_filters_columns.yaml"}},
{"include": {"file": "migrations/037_add_query_hash_and_indexes.yaml"}},
{"include": {"file": "migrations/038_getting_started_guide.yaml"}},
- {"include": {"file": "migrations/039_add_user_google_auth_column.yaml"}}
+ {"include": {"file": "migrations/039_add_user_google_auth_column.yaml"}},
+ {"include": {"file": "migrations/041_drop_field_field_type.yaml"}}
]
}
diff --git a/sample_dataset/metabase/sample_dataset/generate.clj b/sample_dataset/metabase/sample_dataset/generate.clj
index c39c6c6fe6005a7d94a6ff34d3d1d8f38122631a..eb5614e7a6f459cf5499c5ebdb9156c1c0642c3f 100644
--- a/sample_dataset/metabase/sample_dataset/generate.clj
+++ b/sample_dataset/metabase/sample_dataset/generate.clj
@@ -337,8 +337,7 @@
:latitude {:description "This is the latitude of the user on sign-up. It might be updated in the future to the last seen location."}
:longitude {:description "This is the longitude of the user on sign-up. It might be updated in the future to the last seen location."}
:name {:description "The name of the user who owns an account"}
- :password {:description "This is the salted password of the user. It should not be visible"
- :field_type :sensitive}
+ :password {:description "This is the salted password of the user. It should not be visible"}
:source {:description "The channel through which we acquired this user. Valid values include: Affiliate, Facebook, Google, Organic and Twitter"}
:state {:description "The state or province of the account’s billing address"}
:zip {:description "The postal code of the account’s billing address"
diff --git a/src/metabase/db/migrations.clj b/src/metabase/db/migrations.clj
index 24a69821af86a5c52de2728af4159559459f653c..1272e2f1d8df9e08e94599efefa2fb10b31245a6 100644
--- a/src/metabase/db/migrations.clj
+++ b/src/metabase/db/migrations.clj
@@ -135,11 +135,6 @@
(db/update-where! Field {:visibility_type "unset"
:active false}
:visibility_type "retired")
- ;; anything that is active with field_type = :sensitive gets visibility_type :sensitive
- (db/update-where! Field {:visibility_type "unset"
- :active true
- :field_type "sensitive"}
- :visibility_type "sensitive")
;; if field is active but preview_display = false then it becomes :details-only
(db/update-where! Field {:visibility_type "unset"
:active true
diff --git a/src/metabase/driver/druid.clj b/src/metabase/driver/druid.clj
index 56d14e27f58e68781ef83fb6280e162ed5b3f2df..31a0589f14d2bf0a0701a349a32f91536aeeaa66 100644
--- a/src/metabase/driver/druid.clj
+++ b/src/metabase/driver/druid.clj
@@ -65,12 +65,12 @@
;;; ### Sync
(defn- describe-table-field [druid-field-type field-name]
- (merge {:name field-name}
- ;; all dimensions are Strings, and all metrics as JS Numbers, I think (?)
- ;; string-encoded booleans + dates are treated as strings (!)
- (if (= :metric druid-field-type)
- {:field-type :metric, :base-type :FloatField}
- {:field-type :dimension, :base-type :TextField})))
+ ;; all dimensions are Strings, and all metrics as JS Numbers, I think (?)
+ ;; string-encoded booleans + dates are treated as strings (!)
+ {:name field-name
+ :base-type (if (= :metric druid-field-type)
+ :FloatField
+ :TextField)})
(defn- describe-table [database table]
(let [details (:details database)
@@ -81,7 +81,6 @@
;; every Druid table is an event stream w/ a timestamp field
[{:name "timestamp"
:base-type :DateTimeField
- :field-type :dimension
:pk? true}]
(map (partial describe-table-field :dimension) dimensions)
(map (partial describe-table-field :metric) metrics)))}))
diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj
index 43a788fce4a1d9c71e60ef0cbe94b1d250218535..958c6ac992c82962d2f0555aa39d2a41053b1164 100644
--- a/src/metabase/models/field.clj
+++ b/src/metabase/models/field.clj
@@ -8,6 +8,9 @@
[interface :as i])
[metabase.util :as u]))
+
+;;; ------------------------------------------------------------ Type Mappings ------------------------------------------------------------
+
(def ^:const special-types
"Possible values for `Field.special_type`."
#{:avatar
@@ -46,13 +49,6 @@
:UUIDField ; e.g. a Postgres 'UUID' column
:UnknownField})
-(def ^:const ^:deprecated field-types
- "Possible values for `Field.field_type`.
- `field_type` is deprecated, and issue #2454 is open for removing it entirely. It's no longer displayed in the UI, nor is it possible to change its value from the API."
- #{:metric ; A number that can be added, graphed, etc.
- :dimension ; A high or low-cardinality numerical string value that is meant to be used as a grouping
- :info}) ; Non-numerical value that is not meant to be used
-
(def ^:const visibility-types
"Possible values for `Field.visibility_type`."
#{:normal ; Default setting. field has no visibility restrictions.
@@ -77,15 +73,13 @@
(contains? valid-base-types (keyword base-type)))))
+
+;;; ------------------------------------------------------------ Entity & Lifecycle ------------------------------------------------------------
+
(i/defentity Field :metabase_field)
(defn- pre-insert [field]
- (let [defaults {:active true
- :preview_display true
- :field_type :info
- :visibility_type :normal
- :position 0
- :display_name (humanization/name->human-readable-name (:name field))}]
+ (let [defaults {:display_name (humanization/name->human-readable-name (:name field))}]
(merge defaults field)))
(defn- pre-cascade-delete [{:keys [id]}]
@@ -93,6 +87,22 @@
(db/cascade-delete! 'FieldValues :field_id id)
(db/cascade-delete! 'MetricImportantField :field_id id))
+(u/strict-extend (class Field)
+ i/IEntity (merge i/IEntityDefaults
+ {:hydration-keys (constantly [:destination :field :origin])
+ :types (constantly {:base_type :keyword
+ :special_type :keyword
+ :visibility_type :keyword
+ :description :clob})
+ :timestamped? (constantly true)
+ :can-read? (constantly true)
+ :can-write? i/superuser?
+ :pre-insert pre-insert
+ :pre-cascade-delete pre-cascade-delete}))
+
+
+;;; ------------------------------------------------------------ Hydration / Util Fns ------------------------------------------------------------
+
(defn target
"Return the FK target `Field` that this `Field` points to."
@@ -153,19 +163,8 @@
[{:keys [table_id]}]
(db/select-one 'Table, :id table_id))
-(u/strict-extend (class Field)
- i/IEntity (merge i/IEntityDefaults
- {:hydration-keys (constantly [:destination :field :origin])
- :types (constantly {:base_type :keyword
- :field_type :keyword
- :special_type :keyword
- :visibility_type :keyword
- :description :clob})
- :timestamped? (constantly true)
- :can-read? (constantly true)
- :can-write? i/superuser?
- :pre-insert pre-insert
- :pre-cascade-delete pre-cascade-delete}))
+
+;;; ------------------------------------------------------------ Sync Util Type Inference Fns ------------------------------------------------------------
(def ^:private ^:const pattern+base-types+special-type
"Tuples of [pattern set-of-valid-base-types special-type]
@@ -219,7 +218,7 @@
(assert (every? (partial contains? base-types) base-types))
(assert (contains? special-types special-type)))
-(defn infer-field-special-type
+(defn- infer-field-special-type
"If `name` and `base-type` matches a known pattern, return the `special_type` we should assign to it."
[field-name base_type]
(when (and (string? field-name)
@@ -234,6 +233,8 @@
(last matching-pattern)))))
+;;; ------------------------------------------------------------ Sync Util CRUD Fns ------------------------------------------------------------
+
(defn update-field!
"Update an existing `Field` from the given FIELD-DEF."
[{:keys [id], :as existing-field} {field-name :name, :keys [base-type special-type pk? parent-id]}]
diff --git a/src/metabase/query_processor/expand.clj b/src/metabase/query_processor/expand.clj
index de4a00b814b7ec2bbd70400cd82eb1fd94b098bd..e532a5ae14935d9c398f8204bd69f25be5a24ece 100644
--- a/src/metabase/query_processor/expand.clj
+++ b/src/metabase/query_processor/expand.clj
@@ -9,8 +9,8 @@
[metabase.db :as db]
[metabase.models.table :refer [Table]]
[metabase.query-processor.interface :refer [*driver*], :as i]
- [metabase.util :as u])
-
+ [metabase.util :as u]
+ [metabase.util.schema :as su])
(:import (metabase.query_processor.interface AgFieldRef
BetweenFilter
ComparisonFilter
@@ -29,7 +29,7 @@
(s/defn ^:private ^:always-validate normalize-token :- s/Keyword
"Convert a string or keyword in various cases (`lisp-case`, `snake_case`, or `SCREAMING_SNAKE_CASE`) to a lisp-cased keyword."
- [token :- (s/named (s/cond-pre s/Keyword s/Str) "Valid token (keyword or string)")]
+ [token :- su/KeywordOrString]
(-> (name token)
str/lower-case
(str/replace #"_" "-")
@@ -48,7 +48,7 @@
(s/defn ^:ql ^:always-validate field-id :- FieldPlaceholder
"Create a generic reference to a `Field` with ID."
- [id :- i/IntGreaterThanZero]
+ [id :- su/IntGreaterThanZero]
(i/map->FieldPlaceholder {:field-id id}))
(s/defn ^:private ^:always-validate field :- i/AnyField
@@ -108,7 +108,7 @@
(s/defn ^:ql ^:always-validate expression :- ExpressionRef
{:added "0.17.0"}
- [expression-name :- (s/cond-pre s/Str s/Keyword)]
+ [expression-name :- su/KeywordOrString]
(i/strict-map->ExpressionRef {:expression-name (name expression-name)}))
diff --git a/src/metabase/query_processor/interface.clj b/src/metabase/query_processor/interface.clj
index 44c49f5ba89e90b400a9c51095de485462169005..77ae08e67539b414d4739a5a9d5184114d33ab52 100644
--- a/src/metabase/query_processor/interface.clj
+++ b/src/metabase/query_processor/interface.clj
@@ -4,7 +4,8 @@
should go in `metabase.query-processor.expand`."
(:require [schema.core :as s]
[metabase.models.field :as field]
- [metabase.util :as u])
+ [metabase.util :as u]
+ [metabase.util.schema :as su])
(:import clojure.lang.Keyword
java.sql.Timestamp))
@@ -34,21 +35,15 @@
;; These are just used by the QueryExpander to record information about how joins should occur.
-(def IntGreaterThanZero
- "Schema representing an `s/Int` than must also be greater than zero."
- (s/constrained s/Int
- (partial < 0)
- "Integer greater than zero"))
-
-(s/defrecord JoinTableField [field-id :- IntGreaterThanZero
- field-name :- s/Str])
+(s/defrecord JoinTableField [field-id :- su/IntGreaterThanZero
+ field-name :- su/NonBlankString])
(s/defrecord JoinTable [source-field :- JoinTableField
pk-field :- JoinTableField
- table-id :- IntGreaterThanZero
- table-name :- s/Str
- schema :- (s/maybe s/Str)
- join-alias :- s/Str])
+ table-id :- su/IntGreaterThanZero
+ table-name :- su/NonBlankString
+ schema :- (s/maybe su/NonBlankString)
+ join-alias :- su/NonBlankString])
;;; # ------------------------------------------------------------ PROTOCOLS ------------------------------------------------------------
@@ -61,19 +56,19 @@
;;; # ------------------------------------------------------------ "RESOLVED" TYPES: FIELD + VALUE ------------------------------------------------------------
;; Field is the expansion of a Field ID in the standard QL
-(s/defrecord Field [field-id :- IntGreaterThanZero
- field-name :- s/Str
- field-display-name :- s/Str
+(s/defrecord Field [field-id :- su/IntGreaterThanZero
+ field-name :- su/NonBlankString
+ field-display-name :- su/NonBlankString
base-type :- (apply s/enum field/base-types)
special-type :- (s/maybe (apply s/enum field/special-types))
visibility-type :- (apply s/enum field/visibility-types)
- table-id :- IntGreaterThanZero
- schema-name :- (s/maybe s/Str)
- table-name :- (s/maybe s/Str) ; TODO - Why is this `maybe` ?
- position :- (s/maybe s/Int) ; TODO - int >= 0
+ table-id :- su/IntGreaterThanZero
+ schema-name :- (s/maybe su/NonBlankString)
+ table-name :- (s/maybe su/NonBlankString) ; TODO - Why is this `maybe` ?
+ position :- (s/maybe su/IntGreaterThanZero)
fk-field-id :- (s/maybe s/Int)
- description :- (s/maybe s/Str)
- parent-id :- (s/maybe IntGreaterThanZero)
+ description :- (s/maybe su/NonBlankString)
+ parent-id :- (s/maybe su/IntGreaterThanZero)
;; Field once its resolved; FieldPlaceholder before that
parent :- s/Any]
clojure.lang.Named
@@ -116,11 +111,7 @@
clojure.lang.Named
(getName [_] (name field)))
-(def NonEmptyString
- "Schema for a non-empty string."
- (s/constrained s/Str seq "non-empty string")) ; TODO - should this be used elsewhere as well, for things like `Field`?
-
-(s/defrecord ExpressionRef [expression-name :- NonEmptyString]
+(s/defrecord ExpressionRef [expression-name :- su/NonBlankString]
clojure.lang.Named
(getName [_] expression-name)
IField
@@ -130,7 +121,7 @@
;; Value is the expansion of a value within a QL clause
;; Information about the associated Field is included for convenience
-(s/defrecord Value [value :- (s/maybe (s/cond-pre s/Bool s/Num s/Str))
+(s/defrecord Value [value :- (s/maybe (s/cond-pre s/Bool s/Num su/NonBlankString))
field :- (s/named (s/cond-pre Field ExpressionRef) ; TODO - Value doesn't need the whole field, just the relevant type info / units
"field or expression reference")])
@@ -162,8 +153,8 @@
;;; # ------------------------------------------------------------ PLACEHOLDER TYPES: FIELDPLACEHOLDER + VALUEPLACEHOLDER ------------------------------------------------------------
;; Replace Field IDs with these during first pass
-(s/defrecord FieldPlaceholder [field-id :- IntGreaterThanZero
- fk-field-id :- (s/maybe (s/constrained IntGreaterThanZero
+(s/defrecord FieldPlaceholder [field-id :- su/IntGreaterThanZero
+ fk-field-id :- (s/maybe (s/constrained su/IntGreaterThanZero
(fn [_] (or (assert-driver-supports :foreign-keys) true))
"foreign-keys is not supported by this driver."))
datetime-unit :- (s/maybe (apply s/enum datetime-field-units))])
@@ -203,7 +194,7 @@
(def LiteralDatetimeString
"Schema for an MBQL datetime string literal, in ISO-8601 format."
- (s/constrained s/Str u/date-string? "Valid ISO-8601 datetime string literal"))
+ (s/constrained su/NonBlankString u/date-string? "Valid ISO-8601 datetime string literal"))
(def LiteralDatetime
"Schema for an MBQL literal datetime value: and ISO-8601 string or `java.sql.Date`."
@@ -219,7 +210,7 @@
(def AnyValue
"Schema for anything that is a considered a valid value in MBQL - `nil`, a `Boolean`, `Number`, `String`, or relative datetime form."
- (s/named (s/maybe (s/cond-pre s/Bool s/Str OrderableValue)) "Valid value (must be nil, boolean, number, string, or a relative-datetime form)"))
+ (s/named (s/maybe (s/cond-pre s/Bool su/NonBlankString OrderableValue)) "Valid value (must be nil, boolean, number, string, or a relative-datetime form)"))
;; Replace values with these during first pass over Query.
;; Include associated Field ID so appropriate the info can be found during Field resolution
@@ -316,8 +307,8 @@
(def Page
"Schema for the top-level `page` clause in a MBQL query."
- (s/named {:page IntGreaterThanZero
- :items IntGreaterThanZero}
+ (s/named {:page su/IntGreaterThanZero
+ :items su/IntGreaterThanZero}
"Valid page clause"))
(def Query
@@ -326,8 +317,8 @@
(s/optional-key :breakout) [FieldPlaceholderOrExpressionRef]
(s/optional-key :fields) [AnyField]
(s/optional-key :filter) Filter
- (s/optional-key :limit) IntGreaterThanZero
+ (s/optional-key :limit) su/IntGreaterThanZero
(s/optional-key :order-by) [OrderBy]
(s/optional-key :page) Page
(s/optional-key :expressions) {s/Keyword Expression}
- :source-table IntGreaterThanZero})
+ :source-table su/IntGreaterThanZero})
diff --git a/src/metabase/sync_database/interface.clj b/src/metabase/sync_database/interface.clj
index 2b3dffdeeb8b57c475d4eb584f6ba667494f9b67..047151cd116064764016fec22988013b368ddafd 100644
--- a/src/metabase/sync_database/interface.clj
+++ b/src/metabase/sync_database/interface.clj
@@ -20,7 +20,6 @@
"Schema for a given Field as provided in `describe-table` or `analyze-table`."
{:name schema/Str
:base-type (apply schema/enum field/base-types)
- (schema/optional-key :field-type) (apply schema/enum field/field-types)
(schema/optional-key :special-type) (apply schema/enum field/special-types)
(schema/optional-key :pk?) schema/Bool
(schema/optional-key :nested-fields) #{(schema/recursive #'DescribeTableField)}
diff --git a/src/metabase/util/schema.clj b/src/metabase/util/schema.clj
new file mode 100644
index 0000000000000000000000000000000000000000..f6b39d289a451dfc9589f4a1cacd17df2a759c17
--- /dev/null
+++ b/src/metabase/util/schema.clj
@@ -0,0 +1,16 @@
+(ns metabase.util.schema
+ "Various schemas that are useful throughout the app."
+ (:require [clojure.string :as str]
+ [schema.core :as s]))
+
+(def NonBlankString
+ "Schema for a string that cannot be blank."
+ (s/constrained s/Str (complement str/blank?) "Non-blank string"))
+
+(def IntGreaterThanZero
+ "Schema representing an integer than must also be greater than zero."
+ (s/constrained s/Int (partial < 0) "Integer greater than zero"))
+
+(def KeywordOrString
+ "Schema for something that can be either a `Keyword` or a `String`."
+ (s/named (s/cond-pre s/Keyword s/Str) "Keyword or string"))
diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj
index b74e625f414007ee79a9c238f263405d1ce308cf..affa2cb2da88c0f4d04ac20fa40ae5be78b1e095 100644
--- a/test/metabase/api/database_test.clj
+++ b/test/metabase/api/database_test.clj
@@ -266,7 +266,6 @@
:active true
:id $
:raw_column_id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -289,7 +288,6 @@
:active true
:id $
:raw_column_id $
- :field_type "info"
:position 0
:target nil
:preview_display true
diff --git a/test/metabase/api/field_test.clj b/test/metabase/api/field_test.clj
index 0cebd9f94091fd4843934383df9227d0e0aecb0b..b73e12f0f62cb78104de71d9bdbe27e58269b196 100644
--- a/test/metabase/api/field_test.clj
+++ b/test/metabase/api/field_test.clj
@@ -62,7 +62,6 @@
:last_analyzed $
:active true
:id (id :users :name)
- :field_type "info"
:visibility_type "normal"
:position 0
:preview_display true
@@ -144,13 +143,6 @@
((user->client :crowberto) :put 200 (str "field/" field-id) {:special_type :timestamp_seconds})
(db/select-one-field :special_type Field, :id field-id)))
-;; check that a Field with a :base_type of :sensitive can still be modified as normal.
-;; This was a bug introduced when :visibility-type was added -- see issue #2678
-(expect
- (tu/with-temp* [Field [{field-id :id} {:field_type "sensitive"}]]
- (boolean ((user->client :crowberto) :put 200 (str "field/" field-id) {:special_type :avatar}))))
-
-
(defn- field->field-values
"Fetch the `FieldValues` object that corresponds to a given `Field`."
diff --git a/test/metabase/api/table_test.clj b/test/metabase/api/table_test.clj
index 3ae47aa9c05b2fbdd59f1e2780d2ad05e89ea11d..a8f32ff2470c3a06ced16178ffa408110b3cacf1 100644
--- a/test/metabase/api/table_test.clj
+++ b/test/metabase/api/table_test.clj
@@ -122,7 +122,6 @@
:updated_at $
:active true
:id (id :categories :id)
- :field_type "info"
:position 0
:preview_display true
:created_at $
@@ -145,7 +144,6 @@
:updated_at $
:active true
:id (id :categories :name)
- :field_type "info"
:position 0
:preview_display true
:created_at $
@@ -181,7 +179,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -203,7 +200,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -269,7 +265,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -291,7 +286,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -313,7 +307,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -335,7 +328,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -399,7 +391,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -421,7 +412,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -443,7 +433,6 @@
:updated_at $
:active true
:id $
- :field_type "info"
:position 0
:target nil
:preview_display true
@@ -547,7 +536,6 @@
:visibility_type "normal"
:preview_display $
:position $
- :field_type "info"
:active true
:special_type "fk"
:fk_target_field_id $
@@ -587,7 +575,6 @@
:visibility_type "normal"
:preview_display $
:position $
- :field_type "info"
:active true
:special_type "id"
:fk_target_field_id $
diff --git a/test/metabase/models/field_test.clj b/test/metabase/models/field_test.clj
index 045e556aaed3d7f26e78f05836730d98c5a1fa1f..4bd22bbc54521ce049da46c22d7cf17e8ed03e2a 100644
--- a/test/metabase/models/field_test.clj
+++ b/test/metabase/models/field_test.clj
@@ -2,7 +2,8 @@
(:require [expectations :refer :all]
(metabase.models [field :refer :all]
[field-values :refer :all])
- [metabase.test.data :refer :all]))
+ [metabase.test.data :refer :all]
+ [metabase.test.util :as tu]))
;; valid-special-type-for-base-type?
@@ -80,7 +81,10 @@
:visibility_type :normal}))
-;; infer-field-special-type
+;;; infer-field-special-type
+
+(tu/resolve-private-fns metabase.models.field infer-field-special-type)
+
(expect nil (infer-field-special-type nil nil))
(expect nil (infer-field-special-type "id" nil))
(expect nil (infer-field-special-type nil :IntegerField))
diff --git a/test/metabase/sync_database/sync_dynamic_test.clj b/test/metabase/sync_database/sync_dynamic_test.clj
index 184698c1d2848b8e5dc4569d9290c772ac06dea6..46e0b3b725371d675398ef10a46ea2d7c389fa41 100644
--- a/test/metabase/sync_database/sync_dynamic_test.clj
+++ b/test/metabase/sync_database/sync_dynamic_test.clj
@@ -287,7 +287,7 @@
(let [get-fields (fn []
(for [field (db/select Field, :table_id table-id, {:order-by [:id]})]
(dissoc (tu/boolean-ids-and-timestamps field)
- :active :field_type :position :preview_display)))]
+ :active :position :preview_display)))]
;; start with no fields
[(get-fields)
;; first sync will add all the fields
diff --git a/test/metabase/sync_database/sync_test.clj b/test/metabase/sync_database/sync_test.clj
index 5983e9a48ce3ce4c31c2fc392e63b7deb776f127..59ef6f78e24aec5de0e717bcdfcfbcdba9a33810 100644
--- a/test/metabase/sync_database/sync_test.clj
+++ b/test/metabase/sync_database/sync_test.clj
@@ -289,7 +289,7 @@
(let [get-fields #(->> (db/select Field, :table_id table-id, {:order-by [:id]})
(mapv tu/boolean-ids-and-timestamps)
(mapv (fn [m]
- (dissoc m :active :field_type :position :preview_display))))
+ (dissoc m :active :position :preview_display))))
initial-fields (get-fields)
first-sync (do
(save-table-fields! tbl)
diff --git a/test/metabase/sync_database_test.clj b/test/metabase/sync_database_test.clj
index a234865123b94e7fd582bd48f5c1c56efe49326d..7970b39be0d8dbf29bd5667e8a3704896346eed7 100644
--- a/test/metabase/sync_database_test.clj
+++ b/test/metabase/sync_database_test.clj
@@ -91,7 +91,6 @@
:name "id"
:active true
:parent_id false
- :field_type :info
:position 0
:preview_display true
:display_name "ID"
@@ -111,7 +110,6 @@
:name "studio"
:active true
:parent_id false
- :field_type :info
:position 0
:preview_display true
:display_name "Studio"
@@ -131,7 +129,6 @@
:name "title"
:active true
:parent_id false
- :field_type :info
:position 0
:preview_display true
:display_name "Title"
@@ -168,7 +165,6 @@
:name "name"
:active true
:parent_id false
- :field_type :info
:position 0
:preview_display true
:display_name "Name"
@@ -188,7 +184,6 @@
:name "studio"
:active true
:parent_id false
- :field_type :info
:position 0
:preview_display true
:display_name "Studio"
@@ -236,7 +231,6 @@
:name "id"
:active true
:parent_id false
- :field_type :info
:position 0
:preview_display true
:display_name "ID"
@@ -256,7 +250,6 @@
:name "studio"
:active true
:parent_id false
- :field_type :info
:position 0
:preview_display true
:display_name "Studio"
@@ -276,7 +269,6 @@
:name "title"
:active true
:parent_id false
- :field_type :info
:position 0
:preview_display true
:display_name "Title"
diff --git a/test/metabase/test/data.clj b/test/metabase/test/data.clj
index bb944bc9b8946c0d05c52af27ce024fdb805a61a..905218634bf9059f3e77c629389cb11712f29100 100644
--- a/test/metabase/test/data.clj
+++ b/test/metabase/test/data.clj
@@ -188,7 +188,7 @@
;; Sync the database
(sync-database/sync-database! db)
- ;; Add extra metadata like Field field-type, base-type, etc.
+ ;; Add extra metadata like Field base-type, etc.
(doseq [^TableDefinition table-definition (:table-definitions database-definition)]
(let [table-name (:table-name table-definition)
table (delay (or (i/metabase-instance table-definition db)
@@ -196,14 +196,11 @@
table-name
(u/pprint-to-str (dissoc table-definition :rows))
(u/pprint-to-str (db/select [Table :schema :name], :db_id (:id db))))))))]
- (doseq [{:keys [field-name field-type visibility-type special-type], :as field-definition} (:field-definitions table-definition)]
+ (doseq [{:keys [field-name visibility-type special-type], :as field-definition} (:field-definitions table-definition)]
(let [field (delay (or (i/metabase-instance field-definition @table)
(throw (Exception. (format "Field '%s' not loaded from definition:\n"
field-name
(u/pprint-to-str field-definition))))))]
- (when field-type
- (log/debug (format "SET FIELD TYPE %s.%s -> %s" table-name field-name field-type))
- (db/update! Field (:id @field) :field_type (name field-type)))
(when visibility-type
(log/debug (format "SET VISIBILITY TYPE %s.%s -> %s" table-name field-name visibility-type))
(db/update! Field (:id @field) :visibility_type (name visibility-type)))
diff --git a/test/metabase/test/data/interface.clj b/test/metabase/test/data/interface.clj
index 996af1c691a8a55a0cb301148fc00030297243d3..07d5660f2f73db46a2cb5f47991708fa54aa9e39 100644
--- a/test/metabase/test/data/interface.clj
+++ b/test/metabase/test/data/interface.clj
@@ -3,36 +3,39 @@
Objects that implement `IDatasetLoader` know how to load a `DatabaseDefinition` into an
actual physical RDMS database. This functionality allows us to easily test with multiple datasets."
- (:require [clojure.string :as s]
+ (:require [clojure.string :as str]
+ [schema.core :as s]
(metabase [db :as db]
[driver :as driver])
(metabase.models [database :refer [Database]]
[field :refer [Field] :as field]
[table :refer [Table]])
- [metabase.util :as u])
+ [metabase.util :as u]
+ [metabase.util.schema :as su])
(:import clojure.lang.Keyword))
-(defrecord FieldDefinition [^String field-name
- ^Keyword base-type
- ^Keyword field-type
- ^Keyword special-type
- ^Keyword visibility-type
- ^Keyword fk])
+(s/defrecord FieldDefinition [field-name :- su/NonBlankString
+ base-type :- (s/cond-pre {:native su/NonBlankString}
+ (apply s/enum field/base-types))
+ special-type :- (s/maybe (apply s/enum field/special-types))
+ visibility-type :- (s/maybe (apply s/enum field/visibility-types))
+ fk :- (s/maybe s/Keyword)])
-(defrecord TableDefinition [^String table-name
- field-definitions
- rows])
+(s/defrecord TableDefinition [table-name :- su/NonBlankString
+ field-definitions :- [FieldDefinition]
+ rows :- [[s/Any]]])
-(defrecord DatabaseDefinition [^String database-name
- table-definitions
- ;; Optional. Set this to non-nil to let dataset loaders know that we don't intend to keep it
- ;; for long -- they can adjust connection behavior, e.g. choosing simple connections instead of creating pools.
- ^Boolean short-lived?])
+(s/defrecord DatabaseDefinition [database-name :- su/NonBlankString
+ table-definitions :- [TableDefinition]
+ ;; Optional. Set this to non-nil to let dataset loaders know that we don't intend to keep it
+ ;; for long -- they can adjust connection behavior, e.g. choosing simple connections instead of creating pools.
+ ;; TODO - not sure this is still used now that we create connection pools directly via C3P0; we might be able to remove it
+ short-lived? :- (s/maybe s/Bool)])
(defn escaped-name
"Return escaped version of database name suitable for use as a filename / database name / etc."
^String [^DatabaseDefinition database-definition]
- (s/replace (:database-name database-definition) #"\s+" "_"))
+ (str/replace (:database-name database-definition) #"\s+" "_"))
(defn db-qualified-table-name
"Return a combined table name qualified with the name of its database, suitable for use as an identifier.
@@ -41,7 +44,7 @@
^String [^String database-name, ^String table-name]
{:pre [(string? database-name) (string? table-name)]}
;; take up to last 30 characters because databases like Oracle have limits on the lengths of identifiers
- (apply str (take-last 30 (s/replace (s/lower-case (str database-name \_ table-name)) #"-" "_"))))
+ (apply str (take-last 30 (str/replace (str/lower-case (str database-name \_ table-name)) #"-" "_"))))
(defprotocol IMetabaseInstance
@@ -53,19 +56,19 @@
(extend-protocol IMetabaseInstance
FieldDefinition
(metabase-instance [this table]
- (Field :table_id (:id table), :%lower.name (s/lower-case (:field-name this))))
+ (Field :table_id (:id table), :%lower.name (str/lower-case (:field-name this))))
TableDefinition
(metabase-instance [this database]
;; Look first for an exact table-name match; otherwise allow DB-qualified table names for drivers that need them like Oracle
- (or (Table :db_id (:id database), :%lower.name (s/lower-case (:table-name this)))
+ (or (Table :db_id (:id database), :%lower.name (str/lower-case (:table-name this)))
(Table :db_id (:id database), :%lower.name (db-qualified-table-name (:name database) (:table-name this)))))
DatabaseDefinition
(metabase-instance [{:keys [database-name]} engine-kw]
(assert (string? database-name))
(assert (keyword? engine-kw))
- (db/setup-db-if-needed :auto-migrate true)
+ (db/setup-db-if-needed, :auto-migrate true)
(Database :name database-name, :engine (name engine-kw))))
@@ -128,35 +131,22 @@
(defn create-field-definition
"Create a new `FieldDefinition`; verify its values."
- ^FieldDefinition [{:keys [field-name base-type field-type special-type visibility-type fk], :as field-definition-map}]
- (assert (or (contains? field/base-types base-type)
- (and (map? base-type)
- (string? (:native base-type))))
- (str (format "Invalid field base type: '%s'\n" base-type)
- "Field base-type should be either a valid base type like :TextField or be some native type wrapped in a map, like {:native \"JSON\"}."))
- (when field-type
- (assert (contains? field/field-types field-type)))
- (when visibility-type
- (assert (contains? field/visibility-types visibility-type)))
- (when special-type
- (assert (contains? field/special-types special-type)))
- (map->FieldDefinition field-definition-map))
+ ^FieldDefinition [field-definition-map]
+ (s/validate FieldDefinition (map->FieldDefinition field-definition-map)))
(defn create-table-definition
"Convenience for creating a `TableDefinition`."
- ^TableDefinition [^String table-name field-definition-maps rows]
- (map->TableDefinition {:table-name table-name
- :rows rows
- :field-definitions (mapv create-field-definition field-definition-maps)}))
+ ^TableDefinition [^String table-name, field-definition-maps rows]
+ (s/validate TableDefinition (map->TableDefinition {:table-name table-name
+ :rows rows
+ :field-definitions (mapv create-field-definition field-definition-maps)})))
(defn create-database-definition
"Convenience for creating a new `DatabaseDefinition`."
^DatabaseDefinition [^String database-name & table-name+field-definition-maps+rows]
- {:pre [(string? database-name)
- (not (s/blank? database-name))]}
- (map->DatabaseDefinition {:database-name database-name
- :table-definitions (mapv (partial apply create-table-definition)
- table-name+field-definition-maps+rows)}))
+ (s/validate DatabaseDefinition (map->DatabaseDefinition {:database-name database-name
+ :table-definitions (mapv (partial apply create-table-definition)
+ table-name+field-definition-maps+rows)})))
(defmacro def-database-definition
"Convenience for creating a new `DatabaseDefinition` named by the symbol DATASET-NAME."
@@ -221,8 +211,8 @@
field-name
(let [[_ fk-table fk-dest-name] field-name]
(-> fk-table
- (s/replace #"ies$" "y")
- (s/replace #"s$" "")
+ (str/replace #"ies$" "y")
+ (str/replace #"s$" "")
(str \_ (flatten-field-name fk-dest-name))))))
(defn flatten-dbdef
diff --git a/test/metabase/test/mock/moviedb.clj b/test/metabase/test/mock/moviedb.clj
index 9c59a0e1eb599f01dbb497bad90a6303d2b67617..e2ca8b3799403fc04ff1676e696cb3f133f094de 100644
--- a/test/metabase/test/mock/moviedb.clj
+++ b/test/metabase/test/mock/moviedb.clj
@@ -266,7 +266,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -286,7 +285,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -323,7 +321,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -343,7 +340,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -363,7 +359,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -400,7 +395,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -420,7 +414,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -440,7 +433,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -477,7 +469,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -497,7 +488,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -517,7 +507,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -537,7 +526,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -557,7 +545,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
diff --git a/test/metabase/test/mock/schema_per_customer.clj b/test/metabase/test/mock/schema_per_customer.clj
index cf7e6d917c32acbe4391929406dc633d0a07e0fc..03600191cc75d4e0f8aa287c1a2bffcf7c32ba47 100644
--- a/test/metabase/test/mock/schema_per_customer.clj
+++ b/test/metabase/test/mock/schema_per_customer.clj
@@ -483,7 +483,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -503,7 +502,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -540,7 +538,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -560,7 +557,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -597,7 +593,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -617,7 +612,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -638,7 +632,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -658,7 +651,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -696,7 +688,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -717,7 +708,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -737,7 +727,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -774,7 +763,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -794,7 +782,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -831,7 +818,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -851,7 +837,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -872,7 +857,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -892,7 +876,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -930,7 +913,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -951,7 +933,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -971,7 +952,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1008,7 +988,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1028,7 +1007,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1065,7 +1043,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1085,7 +1062,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1106,7 +1082,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1126,7 +1101,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1164,7 +1138,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1185,7 +1158,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -1205,7 +1177,6 @@
:id true
:raw_column_id true
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
diff --git a/test/metabase/test/mock/toucanery.clj b/test/metabase/test/mock/toucanery.clj
index 2d15914f27caaaa6c473af1fdc6ca6f13c908596..33f66b846f08fd4c45658ad9531e95ef828a93b7 100644
--- a/test/metabase/test/mock/toucanery.clj
+++ b/test/metabase/test/mock/toucanery.clj
@@ -103,7 +103,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -123,7 +122,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -160,7 +158,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -180,7 +177,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -200,7 +196,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -220,7 +215,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -240,7 +234,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -260,7 +253,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -280,7 +272,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -300,7 +291,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -320,7 +310,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
@@ -340,7 +329,6 @@
:id true
:raw_column_id false
:last_analyzed false
- :field_type :info
:position 0
:visibility_type :normal
:preview_display true
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index 1a3ce45b3029a3a8eaa4d3ed9e7eee3351ed7047..531f5695503452abada3f4fb65e7d2b6fda46d4e 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -123,13 +123,10 @@
(u/strict-extend (class Field)
WithTempDefaults
- {:with-temp-defaults (fn [_] {:active true
- :base_type :TextField
- :field_type :info
- :name (random-name)
- :position 1
- :preview_display true
- :table_id (data/id :venues)})})
+ {:with-temp-defaults (fn [_] {:base_type :TextField
+ :name (random-name)
+ :position 1
+ :table_id (data/id :venues)})})
(u/strict-extend (class Metric)
WithTempDefaults