diff --git a/src/metabase/analytics/stats.clj b/src/metabase/analytics/stats.clj index 270c5626ed867d9d6166dfe502f727744dee1899..e61d3f413ae1de3ff2b244391dfd93eb0ef99850 100644 --- a/src/metabase/analytics/stats.clj +++ b/src/metabase/analytics/stats.clj @@ -19,6 +19,7 @@ PermissionsGroup Pulse PulseCard PulseChannel QueryCache Segment Table User]] [metabase.models.humanization :as humanization] + [metabase.models.interface :as mi] [metabase.public-settings :as public-settings] [metabase.util :as u] [metabase.util.honey-sql-2 :as h2x] @@ -179,8 +180,8 @@ "Get metrics based on questions TODO characterize by # executions and avg latency" [] - (let [cards (t2/select [Card :query_type :public_uuid :enable_embedding :embedding_params :dataset_query] - :creator_id [:not= config/internal-mb-user-id])] + (let [cards (t2/select [:model/Card :query_type :public_uuid :enable_embedding :embedding_params :dataset_query] + {:where (mi/exclude-internal-content-hsql :model/Card)})] {:questions (merge-count-maps (for [card cards] (let [native? (= (keyword (:query_type card)) :native)] {:total 1 @@ -204,11 +205,12 @@ "Get metrics based on dashboards TODO characterize by # of revisions, and created by an admin" [] - (let [dashboards (t2/select [Dashboard :creator_id :public_uuid :parameters :enable_embedding :embedding_params] :creator_id [:not= config/internal-mb-user-id]) + (let [dashboards (t2/select [:model/Dashboard :creator_id :public_uuid :parameters :enable_embedding :embedding_params] + {:where (mi/exclude-internal-content-hsql :model/Dashboard)}) dashcards (t2/query {:select :dc.* :from [[(t2/table-name DashboardCard) :dc]] :join [[(t2/table-name Dashboard) :d] [:= :d.id :dc.dashboard_id]] - :where [:not [:= :d.creator_id config/internal-mb-user-id]]})] + :where (mi/exclude-internal-content-hsql :model/Dashboard :table-alias :d)})] {:dashboards (count dashboards) :with_params (count (filter (comp seq :parameters) dashboards)) :num_dashs_per_user (medium-histogram dashboards :creator_id) @@ -302,8 +304,8 @@ (defn- collection-metrics "Get metrics on Collection usage." [] - (let [collections (t2/select Collection :type [:not= "instance-analytics"] :is_sample false) - cards (t2/select [Card :collection_id] :creator_id [:not= config/internal-mb-user-id])] + (let [collections (t2/select Collection {:where (mi/exclude-internal-content-hsql :model/Collection)}) + cards (t2/select [Card :collection_id] {:where (mi/exclude-internal-content-hsql :model/Card)})] {:collections (count collections) :cards_in_collections (count (filter :collection_id cards)) :cards_not_in_collections (count (remove :collection_id cards)) @@ -313,7 +315,8 @@ (defn- database-metrics "Get metrics based on Databases." [] - (let [databases (t2/select [Database :is_full_sync :engine :dbms_version])] + (let [databases (t2/select [:model/Database :is_full_sync :engine :dbms_version] + {:where (mi/exclude-internal-content-hsql :model/Database)})] {:databases (merge-count-maps (for [{is-full-sync? :is_full_sync} databases] {:total 1 :analyzed is-full-sync?})) @@ -327,7 +330,10 @@ (defn- table-metrics "Get metrics based on Tables." [] - (let [tables (t2/select [Table :db_id :schema])] + (let [tables (t2/query {:select [:t.db_id :t.schema] + :from [[(t2/table-name :model/Table) :t]] + :join [[(t2/table-name :model/Database) :d] [:= :d.id :t.db_id]] + :where (mi/exclude-internal-content-hsql :model/Database :table-alias :d)})] {:tables (count tables) :num_per_database (medium-histogram tables :db_id) :num_per_schema (medium-histogram tables :schema)})) @@ -335,7 +341,11 @@ (defn- field-metrics "Get metrics based on Fields." [] - (let [fields (t2/select [Field :table_id])] + (let [fields (t2/query {:select [:f.table_id] + :from [[(t2/table-name Field) :f]] + :join [[(t2/table-name Table) :t] [:= :t.id :f.table_id] + [(t2/table-name Database) :d] [:= :d.id :t.db_id]] + :where (mi/exclude-internal-content-hsql :model/Database :table-alias :d)})] {:fields (count fields) :num_per_table (medium-histogram fields :table_id)})) diff --git a/src/metabase/api/setup.clj b/src/metabase/api/setup.clj index 0998eb47823d8275b2f6bf6a710a377dd9b751f3..b5a39307fb0f34d7aab2c847c4845bdc035f704f 100644 --- a/src/metabase/api/setup.clj +++ b/src/metabase/api/setup.clj @@ -10,15 +10,10 @@ [metabase.email :as email] [metabase.events :as events] [metabase.integrations.slack :as slack] - [metabase.models.card :refer [Card]] - [metabase.models.collection :refer [Collection]] - [metabase.models.dashboard :refer [Dashboard]] - [metabase.models.database :refer [Database]] + [metabase.models.interface :as mi] [metabase.models.permissions-group :as perms-group] - [metabase.models.pulse :refer [Pulse]] [metabase.models.session :refer [Session]] [metabase.models.setting.cache :as setting.cache] - [metabase.models.table :refer [Table]] [metabase.models.user :as user :refer [User]] [metabase.public-settings :as public-settings] [metabase.public-settings.premium-features :as premium-features] @@ -170,15 +165,22 @@ :hosted? (premium-features/is-hosted?) :configured {:email (email/email-configured?) :slack (slack/slack-configured?)} - :counts {:user (t2/count User) - :card (t2/count Card) - :table (t2/count Table)} - :exists {:non-sample-db (t2/exists? Database, :is_sample false) - :dashboard (t2/exists? Dashboard) - :pulse (t2/exists? Pulse) - :hidden-table (t2/exists? Table, :visibility_type [:not= nil]) - :collection (t2/exists? Collection) - :model (t2/exists? Card :type :model)}}) + :counts {:user (t2/count :model/User {:where (mi/exclude-internal-content-hsql :model/User)}) + :card (t2/count :model/Card {:where (mi/exclude-internal-content-hsql :model/Card)}) + :table (val (ffirst (t2/query {:select [:%count.*] + :from [[(t2/table-name :model/Table) :t]] + :join [[(t2/table-name :model/Database) :d] [:= :d.id :t.db_id]] + :where (mi/exclude-internal-content-hsql :model/Database :table-alias :d)})))} + :exists {:non-sample-db (t2/exists? :model/Database {:where (mi/exclude-internal-content-hsql :model/Database)}) + :dashboard (t2/exists? :model/Dashboard {:where (mi/exclude-internal-content-hsql :model/Dashboard)}) + :pulse (t2/exists? :model/Pulse) + :hidden-table (t2/exists? :model/Table {:where [:and + [:not= :visibility_type nil] + (mi/exclude-internal-content-hsql :model/Table)]}) + :collection (t2/exists? :model/Collection {:where (mi/exclude-internal-content-hsql :model/Collection)}) + :model (t2/exists? :model/Card {:where [:and + [:= :type "model"] + (mi/exclude-internal-content-hsql :model/Card)]})}}) (defn- get-connected-tasks [{:keys [configured counts exists] :as _info}] diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index 651073b3a528cf536c2d4bf0fd7c4b22bdf8d14d..a4943421c73529de764565f97c942acc9d89844e 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -43,6 +43,7 @@ [metabase.shared.util.i18n :refer [trs]] [metabase.util :as u] [metabase.util.embed :refer [maybe-populate-initially-published-at]] + [metabase.util.honey-sql-2 :as h2x] [metabase.util.i18n :refer [tru]] [metabase.util.log :as log] [metabase.util.malli :as mu] @@ -570,6 +571,10 @@ [_card] [:name (serdes/hydrated-hash :collection) :created_at]) +(defmethod mi/exclude-internal-content-hsql :model/Card + [_model & {:keys [table-alias]}] + [:not= (h2x/identifier :field table-alias :creator_id) config/internal-mb-user-id]) + ;;; ----------------------------------------------- Creating Cards ---------------------------------------------------- (mu/defn result-metadata-async :- (ms/InstanceOfClass ManyToManyChannel) diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj index 15a6ef0c84e5d36b23325226bd272211bd8bac47..4dae37fa50543b0158a1d9f88ea5db66b5fbb22a 100644 --- a/src/metabase/models/collection.clj +++ b/src/metabase/models/collection.clj @@ -944,6 +944,17 @@ :read (perms/collection-read-path collection-or-id) :write (perms/collection-readwrite-path collection-or-id))}))) +(def ^:private instance-analytics-collection-type + "The value of the `:type` field for the `instance-analytics` Collection created in [[metabase-enterprise.audit-db]]" + "instance-analytics") + +(defmethod mi/exclude-internal-content-hsql :model/Collection + [_model & {:keys [table-alias]}] + (let [maybe-alias #(h2x/identifier :field (some-> table-alias name) %)] + [:and + [:not= (maybe-alias :type) [:inline instance-analytics-collection-type]] + [:not (maybe-alias :is_sample)]])) + (defn- parent-identity-hash [coll] (let [parent-id (-> coll (t2/hydrate :parent_id) diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index 41913dcf0c2bd1173ff489c2a50841cdb65d7c83..bb401b5e7c0fe3a54456d579a49057b8ab5baf17 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -6,6 +6,7 @@ [clojure.string :as str] [medley.core :as m] [metabase.automagic-dashboards.populate :as populate] + [metabase.config :as config] [metabase.db.query :as mdb.query] [metabase.events :as events] [metabase.models.audit-log :as audit-log] @@ -29,6 +30,7 @@ [metabase.query-processor.async :as qp.async] [metabase.util :as u] [metabase.util.embed :refer [maybe-populate-initially-published-at]] + [metabase.util.honey-sql-2 :as h2x] [metabase.util.i18n :as i18n :refer [deferred-tru deferred-trun tru]] [metabase.util.log :as log] [metabase.util.malli :as mu] @@ -569,6 +571,10 @@ (let [dashboards-with-cards (t2/hydrate dashboards [:dashcards :card])] (map #(assoc %1 k %2) dashboards (map dashboard->resolved-params dashboards-with-cards)))) +(defmethod mi/exclude-internal-content-hsql :model/Dashboard + [_model & {:keys [table-alias]}] + [:not= (h2x/identifier :field table-alias :creator_id) config/internal-mb-user-id]) + ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | SERIALIZATION | ;;; +----------------------------------------------------------------------------------------------------------------+ diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index 455a36fc17ec1b436b6d0dde66c7988c5ea556cc..0311fcf2ffbecda9dfe570066df75e47806a2eb7 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -22,6 +22,7 @@ :refer [defenterprise]] [metabase.sync.schedules :as sync.schedules] [metabase.util :as u] + [metabase.util.honey-sql-2 :as h2x] [metabase.util.i18n :refer [deferred-tru trs]] [metabase.util.log :as log] [methodical.core :as methodical] @@ -323,6 +324,11 @@ :visibility :public :database-local :only) +(defmethod mi/exclude-internal-content-hsql :model/Database + [_model & {:keys [table-alias]}] + (let [maybe-alias #(h2x/identifier :field table-alias %)] + [:not [:or (maybe-alias :is_sample) (maybe-alias :is_audit)]])) + ;;; ---------------------------------------------- Hydration / Util Fns ---------------------------------------------- ;; only used in tests diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj index 4808d42a271ffe9058e08d667e074d42d5adc2d6..2ea69d3cadbc8a7bdaae3e4f7b40473681af7ff0 100644 --- a/src/metabase/models/interface.clj +++ b/src/metabase/models/interface.clj @@ -720,3 +720,14 @@ (let [key->hydrated-items (instance-key->hydrated-data-fn)] (for [item instances] (assoc item hydration-key (get key->hydrated-items (get item instance-key) default)))))) + +(defmulti exclude-internal-content-hsql + "Returns a HoneySQL expression to exclude instances of the model that were created automatically as part of internally + used content, such as Metabase Analytics, the sample database, or the sample dashboard. If a `table-alias` (string + or keyword) is provided any columns will have a table alias in the returned expression." + {:arglists '([model & {:keys [table-alias]}])} + dispatch-on-model) + +(defmethod exclude-internal-content-hsql :default + [_model & _] + [:= [:inline 1] [:inline 1]]) diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj index 0861835f53f6b3bf797b8b49005bb17554692ea2..25f76b51a93b99fa2c9ab9a2f9624667dfced93f 100644 --- a/src/metabase/models/user.clj +++ b/src/metabase/models/user.clj @@ -23,6 +23,7 @@ [metabase.public-settings.premium-features :as premium-features] [metabase.setup :as setup] [metabase.util :as u] + [metabase.util.honey-sql-2 :as h2x] [metabase.util.i18n :as i18n :refer [deferred-tru trs tru]] [metabase.util.log :as log] [metabase.util.malli :as mu] @@ -237,6 +238,10 @@ ;; is_group_manager only included if `advanced-permissions` is enabled [:is_group_manager {:optional true} :boolean]]) +(defmethod mi/exclude-internal-content-hsql :model/User + [_model & {:keys [table-alias]}] + [:and [:not= (h2x/identifier :field table-alias :type) [:inline "internal"]]]) + ;;; -------------------------------------------------- Permissions --------------------------------------------------- (defn permissions-set diff --git a/test/metabase/analytics/stats_test.clj b/test/metabase/analytics/stats_test.clj index 7f998ed1bfbc76a9d3b315caefbabca7a8db34b3..469cffc0120b6f062ff25cfe83742a868a44bcd8 100644 --- a/test/metabase/analytics/stats_test.clj +++ b/test/metabase/analytics/stats_test.clj @@ -3,6 +3,7 @@ [clojure.test :refer :all] [java-time.api :as t] [metabase.analytics.stats :as stats :refer [anonymous-usage-stats]] + [metabase.core :as mbc] [metabase.db :as mdb] [metabase.email :as email] [metabase.integrations.slack :as slack] @@ -286,15 +287,28 @@ ["6-10" [:int {:min 1}]]]]] (#'stats/alert-metrics))))) -(deftest sample-content-metrics-test - (testing "Sample content doesn't contribute to stats" +(deftest internal-content-metrics-test + (testing "Internal content doesn't contribute to stats" (mt/with-temp-empty-app-db [_conn :h2] (mdb/setup-db! :create-sample-content? true) - (testing "sense check: Collection, Dashboard, and Cards exist" + (mbc/ensure-audit-db-installed!) + (testing "sense check: internal content exists" + (is (true? (t2/exists? :model/User))) + (is (true? (t2/exists? :model/Database))) + (is (true? (t2/exists? :model/Table))) + (is (true? (t2/exists? :model/Field))) (is (true? (t2/exists? :model/Collection))) (is (true? (t2/exists? :model/Dashboard))) (is (true? (t2/exists? :model/Card)))) (testing "All metrics should be empty" + (is (= {:users {}} + (#'stats/user-metrics))) + (is (= {:databases {}, :dbms_versions {}} + (#'stats/database-metrics))) + (is (= {:tables 0, :num_per_database {}, :num_per_schema {}} + (#'stats/table-metrics))) + (is (= {:num_per_table {}, :fields 0} + (#'stats/field-metrics))) (is (= {:collections 0, :cards_in_collections 0, :cards_not_in_collections 0, :num_cards_per_collection {}} (#'stats/collection-metrics))) (is (= {:questions {}, :public {}, :embedded {}} diff --git a/test/metabase/api/setup_test.clj b/test/metabase/api/setup_test.clj index 96f296845f26d8cd70497b23084b7852d01d1282..970e0d7bc9ec64c074ce47e2595dae376df99519 100644 --- a/test/metabase/api/setup_test.clj +++ b/test/metabase/api/setup_test.clj @@ -7,6 +7,8 @@ [metabase.analytics.snowplow-test :as snowplow-test] [metabase.api.setup :as api.setup] [metabase.config :as config] + [metabase.core :as mbc] + [metabase.db :as mdb] [metabase.driver.h2 :as h2] [metabase.events :as events] [metabase.http-client :as client] @@ -180,8 +182,8 @@ (s/def ::setup!-args (s/cat :expected-status (s/? integer?) - :f any? - :args (s/* any?))) + :f any? + :args (s/* any?))) (defn- setup! {:arglists '([expected-status? f & args])} @@ -416,6 +418,34 @@ (is (= "You don't have permissions to do that." (mt/user-http-request :rasta :get 403 "setup/admin_checklist")))))) +(deftest internal-content-setup-test + (testing "Internally created state like Metabase Analytics shouldn't affect the checklist" + (mt/with-temp-empty-app-db [_conn :h2] + (mdb/setup-db! :create-sample-content? true) + (mbc/ensure-audit-db-installed!) + (testing "Sense check: internal content exists" + (is (true? (t2/exists? :model/User))) + (is (true? (t2/exists? :model/Database))) + (is (true? (t2/exists? :model/Table))) + (is (true? (t2/exists? :model/Field))) + (is (true? (t2/exists? :model/Collection))) + (is (true? (t2/exists? :model/Dashboard))) + (is (true? (t2/exists? :model/Card)))) + (testing "All state should be empty" + (is (=? {:db-type :h2 + :configured {:email false + :slack false} + :counts {:user 0 + :card 0 + :table 0} + :exists {:non-sample-db false + :dashboard false + :pulse false + :hidden-table false + :collection false + :model false}} + (#'api.setup/state-for-checklist))))))) + (deftest annotate-test (testing "identifies next step" (is (partial= [{:group "first"