Daily task to delete old inactive tables (#35443)

docs/configuring-metabase/environment-variables.md

@@ -520,6 +520,16 @@ Default: `null`
 
 Client ID for Google Auth SSO. If this is set, Google Auth is considered to be enabled.
 
+## MB_INACTIVE_TABLE_MAX_DAYS
+
+Type: integer<br>
+Default: `180`<br>
+Since: v48.0
+
+Sets the maximum number of days Metabase keep inactive tables.
+
+Once a day, Metabase will delete inactive tables older than this threshold.
+
 ### `MB_JDBC_DATA_WAREHOUSE_MAX_CONNECTION_POOL_SIZE`
 
 Type: integer<br>

src/metabase/models/table.clj

@@ -63,7 +63,7 @@
 
 (t2/define-before-delete :model/Table
   [{:keys [db_id schema id]}]
-  (t2/delete! Permissions :object [:like (str (perms/data-perms-path db_id schema id) "%")]))
+  (t2/delete! Permissions :object [:like (str "%" (perms/data-perms-path db_id schema id) "%")]))
 
 (defmethod mi/perms-objects-set :model/Table
   [{db-id :db_id, schema :schema, table-id :id, :as table} read-or-write]

src/metabase/task/delete_inactive_tables.clj

+(ns metabase.task.delete-inactive-tables
+  (:require
+   [clojurewerkz.quartzite.jobs :as jobs]
+   [clojurewerkz.quartzite.schedule.cron :as cron]
+   [clojurewerkz.quartzite.triggers :as triggers]
+   [java-time :as t]
+   [metabase.config :as config]
+   [metabase.models.task-history :as task-history]
+   [metabase.task :as task]
+   [metabase.util.i18n :refer [trs]]
+   [metabase.util.log :as log]
+   [toucan2.core :as t2]))
+
+(set! *warn-on-reflection* true)
+
+(def ^:private inactive-table-max-days
+  "The maximum number of days to keep inactive tables around for."
+  (or (config/config-int :inactive-table-max-days) 180))
+
+(def ^:private job-key     "metabase.task.delete-inactive-tables.job")
+(def ^:private trigger-key "metabase.task.delete-inactive-tables.trigger")
+
+(defn- delete-inactive-tables!
+  []
+  (log/debug (trs "Cleaning up Inactive Tables older than {0} days" inactive-table-max-days))
+  (task-history/with-task-history {:task "delete-inactive-tables"}
+    (t2/delete! :model/Table
+                :active false
+                :updated_at [:< (t/minus (t/zoned-date-time) (t/days inactive-table-max-days))])))
+
+(jobs/defjob
+  ^{:doc "Delete inactive Tables."}
+  DeleteInactiveTables [_]
+  (delete-inactive-tables!))
+
+(defmethod task/init! ::DeleteInactiveTables [_]
+  (let [job     (jobs/build
+                 (jobs/of-type DeleteInactiveTables)
+                 (jobs/with-identity (jobs/key job-key)))
+        trigger (triggers/build
+                 (triggers/with-identity (triggers/key trigger-key))
+                 (triggers/start-now)
+                 (triggers/with-schedule
+                   ;; run every day at 1 AM
+                   (cron/cron-schedule "0 30 1 * * ? *")))]
+    (task/schedule-task! job trigger)))

test/metabase/models/table_test.clj

@@ -3,6 +3,7 @@
    [clojure.java.jdbc :as jdbc]
    [clojure.test :refer :all]
    [metabase.models.database :refer [Database]]
+   [metabase.models.permissions :as perms]
    [metabase.models.serialization :as serdes]
    [metabase.models.table :as table :refer [Table]]
    [metabase.sync :as sync]
@@ -79,3 +80,17 @@
         (is (= "0395fe49"
                (serdes/raw-hash ["PUBLIC" "widget" db-hash])
                (serdes/identity-hash table)))))))
+
+(deftest cleanup-permissions-after-delete-table-test
+  (mt/with-temp
+    [:model/Database         {db-id :id}    {}
+     :model/PermissionsGroup {group-id :id} {}
+     :model/Table            table          {:db_id  db-id
+                                             :active true}]
+    (let [read-perm-id     (:id (first (perms/grant-permissions! group-id (perms/table-read-path table))))
+          download-perm-id (:id (first (perms/grant-permissions!
+                                        group-id (perms/feature-perms-path :download :limited (:db_id table) (:schema table) (:id table)))))]
+      (t2/delete! :model/Table (:id table))
+      (testing "table specific permissions are deleted when we delete the table"
+        (is (false? (t2/exists? :model/permissions read-perm-id)))
+        (is (false? (t2/exists? :model/permissions download-perm-id)))))))

test/metabase/task/delete_inactive_tables_test.clj

+(ns ^:mb/once metabase.task.delete-inactive-tables-test
+  (:require
+   [clojure.test :refer :all]
+   [java-time.api :as t]
+   [metabase.task.delete-inactive-tables :as delete-inactive-tables]
+   [metabase.test :as mt]
+   [toucan2.core :as t2]))
+
+(deftest delete-inactive-tables-test
+  (let [one-day-less-than-threshold (t/minus (t/zoned-date-time)
+                                             (t/days (dec @#'delete-inactive-tables/inactive-table-max-days)))
+        older-than-threshold        (t/minus (t/zoned-date-time)
+                                             (t/days (inc @#'delete-inactive-tables/inactive-table-max-days)))]
+    (mt/with-temp
+      [:model/Database {db-id :id}                 {}
+       :model/Table    {new-active-table-id :id}   {:db_id       db-id
+                                                    :active      true
+                                                    :updated_at  one-day-less-than-threshold}
+       :model/Table    {old-active-table-id :id}   {:db_id       db-id
+                                                    :active      true
+                                                    :updated_at  older-than-threshold}
+       :model/Table    {new-inactive-table-id :id} {:db_id       db-id
+                                                    :active      false
+                                                    :updated_at  one-day-less-than-threshold}
+       :model/Table    _old-inactive-table         {:db_id       db-id
+                                                    :active      false
+                                                    :updated_at  older-than-threshold}]
+
+      (#'delete-inactive-tables/delete-inactive-tables!)
+      (testing "inactive tables that are older than threshold should be deleted"
+        (is (= #{new-active-table-id old-active-table-id new-inactive-table-id}
+               (t2/select-pks-set :model/Table :db_id db-id)))))))