Added sync-new-table! (#27682)

Added new endpoint "/db/:id/new-table".

src/metabase/api/notify.clj

@@ -3,10 +3,15 @@
   (:require
    [compojure.core :refer [POST]]
    [metabase.api.common :as api]
+   [metabase.driver :as driver]
+   [metabase.driver.util :as driver.u]
    [metabase.models.database :refer [Database]]
    [metabase.models.table :refer [Table]]
    [metabase.sync :as sync]
    [metabase.sync.sync-metadata :as sync-metadata]
+   [metabase.sync.sync-metadata.tables :as sync-tables]
+   [metabase.sync.util :as sync-util]
+   [metabase.util.i18n :refer [trs]]
    [metabase.util.schema :as su]
    [schema.core :as s]
    [toucan.db :as db]))
@@ -36,5 +41,41 @@
         synchronous? deref)))
   {:success true})
 
+(defn- without-stacktrace [^Throwable throwable]
+  (doto throwable
+    (.setStackTrace (make-array StackTraceElement 0))))
+
+#_{:clj-kondo/ignore [:deprecated-var]}
+(api/defendpoint-schema POST "/db/:id/new-table"
+  "Sync a new table without running a full database sync. Requires `schema_name` and `table_name`. Will throw an error
+  if the table already exists in Metabase or cannot be found."
+  [id :as {{:keys [schema_name table_name]} :body}]
+  {schema_name su/NonBlankString
+   table_name  su/NonBlankString}
+  (api/let-404 [database (db/select-one Database :id id)]
+    (if-not (db/select-one Table :db_id id :name table_name :schema schema_name)
+      (let [driver (driver.u/database->driver database)
+            {db-tables :tables} (driver/describe-database driver database)]
+        (if-let [table (some (fn [table-in-db]
+                               (when (= (dissoc table-in-db :description)
+                                        {:schema schema_name :name table_name})
+                                 table-in-db))
+                             db-tables)]
+          (let [created (sync-tables/create-or-reactivate-table! database table)]
+            (doto created
+              sync/sync-table!
+              sync-util/set-initial-table-sync-complete!))
+          (throw (without-stacktrace
+                  (ex-info (trs "Unable to identify table ''{0}.{1}''"
+                                schema_name table_name)
+                           {:status-code 404
+                            :schema_name schema_name
+                            :table_name  table_name})))))
+      (throw (without-stacktrace
+              (ex-info (trs "Table ''{0}.{1}'' already exists"
+                            schema_name table_name)
+                       {:status-code 400
+                        :schema_name schema_name
+                        :table_name  table_name}))))))
 
 (api/define-routes)

src/metabase/sync/sync_metadata/tables.clj

@@ -9,7 +9,7 @@
    [metabase.models.interface :as mi]
    [metabase.models.permissions :as perms]
    [metabase.models.permissions-group :as perms-group]
-   [metabase.models.table :as table :refer [Table]]
+   [metabase.models.table :refer [Table]]
    [metabase.sync.fetch-metadata :as fetch-metadata]
    [metabase.sync.interface :as i]
    [metabase.sync.sync-metadata.metabase-metadata :as metabase-metadata]
@@ -90,8 +90,31 @@
   [database :- i/DatabaseInstance db-metadata :- i/DatabaseMetadata]
   (log/info (trs "Found new version for DB: {0}" (:version db-metadata)))
   (db/update! Database (u/the-id database)
-              :details
-              (assoc (:details database) :version (:version db-metadata))))
+    :details
+    (assoc (:details database) :version (:version db-metadata))))
+
+(defn create-or-reactivate-table!
+  "Create a single new table in the database, or mark it as active if it already exists."
+  [database {schema :schema, table-name :name, :as table}]
+  (if-let [existing-id (db/select-one-id Table
+                         :db_id (u/the-id database)
+                         :schema schema
+                         :name table-name
+                         :active false)]
+    ;; if the table already exists but is marked *inactive*, mark it as *active*
+    (db/update! Table existing-id
+      :active true)
+    ;; otherwise create a new Table
+    (let [is-crufty? (is-crufty-table? table)]
+      (db/insert! Table
+        :db_id (u/the-id database)
+        :schema schema
+        :name table-name
+        :display_name (humanization/name->human-readable-name table-name)
+        :active true
+        :visibility_type (when is-crufty? :cruft)
+        ;; if this is a crufty table, mark initial sync as complete since we'll skip the subsequent sync steps
+        :initial_sync_status (if is-crufty? "complete" "incomplete")))))
 
 ;; TODO - should we make this logic case-insensitive like it is for fields?
 
@@ -101,26 +124,8 @@
   (log/info (trs "Found new tables:")
             (for [table new-tables]
               (sync-util/name-for-logging (mi/instance Table table))))
-  (doseq [{schema :schema, table-name :name, :as table} new-tables]
-    (if-let [existing-id (db/select-one-id Table
-                           :db_id  (u/the-id database)
-                           :schema schema
-                           :name   table-name
-                           :active false)]
-      ;; if the table already exists but is marked *inactive*, mark it as *active*
-      (db/update! Table existing-id
-        :active true)
-      ;; otherwise create a new Table
-      (let [is-crufty? (is-crufty-table? table)]
-       (db/insert! Table
-         :db_id               (u/the-id database)
-         :schema              schema
-         :name                table-name
-         :display_name        (humanization/name->human-readable-name table-name)
-         :active              true
-         :visibility_type     (when is-crufty? :cruft)
-         ;; if this is a crufty table, mark initial sync as complete since we'll skip the subsequent sync steps
-         :initial_sync_status (if is-crufty? "complete" "incomplete"))))))
+  (doseq [table new-tables]
+    (create-or-reactivate-table! database table)))
 
 
 (s/defn ^:private retire-tables!

test/metabase/api/notify_test.clj

 (ns metabase.api.notify-test
   (:require
    [clj-http.client :as http]
+   [clojure.java.jdbc :as jdbc]
    [clojure.test :refer :all]
+   [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
    [metabase.http-client :as client]
-   [metabase.models.database :as database]
+   [metabase.models.database :as database :refer [Database]]
    [metabase.server.middleware.auth :as mw.auth]
    [metabase.server.middleware.util :as mw.util]
-   [metabase.sync]
+   [metabase.sync :as sync]
    [metabase.sync.sync-metadata]
    [metabase.test :as mt]
    [metabase.test.fixtures :as fixtures]
-   [metabase.util :as u]))
+   [metabase.util :as u]
+   [toucan.db :as db]))
 
 (use-fixtures :once (fixtures/initialize :db :web-server))
 
@@ -101,3 +104,59 @@
         (is (= {:errors
                 {:scan "value may be nil, or if non-nil, value must be one of: `full`, `schema`."}}
                (post {:scan :unrecognized} 400)))))))
+
+;; TODO - Consider generalizing this in the future. It was taken from `metabase.driver.postgres-test`
+;; Perhaps if there's another instance where it is used put it somewhere common.
+
+(defn- drop-if-exists-and-create-db!
+  "Drop a Postgres database named `db-name` if it already exists; then create a new empty one with that name."
+  [db-name]
+  (let [spec (sql-jdbc.conn/connection-details->spec :postgres (mt/dbdef->connection-details :postgres :server nil))]
+    ;; kill any open connections
+    (jdbc/query spec ["SELECT pg_terminate_backend(pg_stat_activity.pid)
+                       FROM pg_stat_activity
+                       WHERE pg_stat_activity.datname = ?;" db-name])
+    ;; create the DB
+    (jdbc/execute! spec [(format "DROP DATABASE IF EXISTS \"%s\";
+                                  CREATE DATABASE \"%s\";"
+                                 db-name db-name)]
+                   {:transaction? false})))
+
+(deftest add-new-table-sync-test
+  (mt/test-driver :postgres
+    (testing "Ensure we have the ability to add a single new table"
+      (let [db-name "add_new_table_sync_test_table"
+            details (mt/dbdef->connection-details :postgres :db {:database-name db-name})]
+        (drop-if-exists-and-create-db! db-name)
+        (mt/with-temp* [Database [database {:engine :postgres, :details (assoc details :dbname db-name)}]]
+          (let [spec     (sql-jdbc.conn/connection-details->spec :postgres details)
+                exec!    (fn [spec statements] (doseq [statement statements] (jdbc/execute! spec [statement])))
+                tableset #(set (map (fn [{:keys [schema name]}] (format "%s.%s" schema name)) (db/select 'Table :db_id (:id %))))
+                post     (fn post-api
+                           ([payload] (post-api payload 200))
+                           ([payload expected-code]
+                            (mt/with-temporary-setting-values [api-key "test-api-key"]
+                              (mt/client-full-response
+                               :post expected-code (format "notify/db/%d/new-table" (:id database))
+                               {:request-options api-headers}
+                               (merge {:synchronous? true}
+                                      payload)))))
+                sync!    #(sync/sync-database! database)]
+            ;; Create the initial table and sync it.
+            (exec! spec ["CREATE TABLE public.FOO (val bigint NOT NULL);"])
+            (sync!)
+            (let [tables (tableset database)]
+              (is (= #{"public.foo"} tables)))
+            ;; We can't add an existing table
+            (is (= 400 (:status (post {:schema_name "public" :table_name "foo"} 400))))
+            ;; We can't add a nonexistent table
+            (is (= 404 (:status (post {:schema_name "public" :table_name "bar"} 404))))
+            ;; Create two more tables that are not yet synced
+            (exec! spec ["CREATE TABLE public.BAR (val bigint NOT NULL);"
+                         "CREATE TABLE public.FERN (val bigint NOT NULL);"])
+            ;; This will add bar to metabase (but not fern).
+            (is (= 200 (:status (post {:schema_name "public" :table_name "bar"}))))
+            ;; Assert that only the synced tables are present.
+            (let [tables (tableset database)]
+              (is (= #{"public.foo" "public.bar"} tables))
+              (is (false? (contains? tables "public.fern"))))))))))