Merge pull request #11 from dadosfera/updating-metabase-to-solve-issues

Updating metabase to solve issues

bin/build/src/build/uberjar.clj

@@ -1,6 +1,7 @@
 (ns build.uberjar
   (:require
    [clojure.java.io :as io]
+   [clojure.set :as set]
    [clojure.tools.build.api :as b]
    [clojure.tools.build.util.zip :as build.zip]
    [clojure.tools.namespace.dependency :as ns.deps]
@@ -67,11 +68,14 @@
   (let [ns-decls   (mapcat
                     (comp ns.find/find-ns-decls-in-dir io/file)
                     (all-paths basis))
-        ns-symbols (set (map ns.parse/name-from-ns-decl ns-decls))]
-    (->> (dependencies-graph ns-decls)
-         ns.deps/topo-sort
-         (filter ns-symbols)
-         (cons 'metabase.bootstrap))))
+        ns-symbols (set (map ns.parse/name-from-ns-decl ns-decls))
+        sorted     (->> (dependencies-graph ns-decls)
+                        ns.deps/topo-sort
+                        (filter ns-symbols))
+        orphans    (set/difference (set ns-symbols) (set sorted))]
+    (concat
+     orphans
+     sorted)))
 
 (defn compile-sources! [basis]
   (u/step "Compile Clojure source files"
@@ -88,17 +92,18 @@
 
 (defn copy-resources! [edition basis]
   (u/step "Copy resources"
-    ;; technically we don't NEED to copy the Clojure source files but it doesn't really hurt anything IMO.
     (doseq [path (all-paths basis)]
-      (u/step (format "Copy %s" path)
-        (b/copy-dir {:target-dir class-dir, :src-dirs [path]})))))
+     (when (not (#{"src" "shared/src" "enterprise/backend/src"} path))
+       (u/step (format "Copy %s" path)
+         (b/copy-dir {:target-dir class-dir, :src-dirs [path]}))))))
 
 (defn create-uberjar! [basis]
   (u/step "Create uberjar"
     (with-duration-ms [duration-ms]
       (depstar/uber {:class-dir class-dir
                      :uber-file uberjar-filename
-                     :basis     basis})
+                     :basis     basis
+                     :exclude   [".*metabase.*.clj[c|s]?$"]})
       (u/announce "Created uberjar in %.1f seconds." (/ duration-ms 1000.0)))))
 
 (def manifest-entries

e2e/runner/cypress-runner-backend.js

@@ -36,6 +36,7 @@ const CypressBackend = {
         MB_JETTY_HOST: "0.0.0.0",
         MB_JETTY_PORT: server.port,
         MB_ENABLE_TEST_ENDPOINTS: "true",
+        MB_DANGEROUS_UNSAFE_ENABLE_TESTING_H2_CONNECTIONS_DO_NOT_ENABLE: "true",
         MB_PREMIUM_EMBEDDING_TOKEN:
           (process.env["MB_EDITION"] === "ee" &&
             process.env["MB_PREMIUM_EMBEDDING_TOKEN"]) ||

e2e/runner/cypress-runner-generate-snapshots.js

@@ -4,7 +4,6 @@ const { parseArguments, args } = require("./cypress-runner-utils");
 
 const getConfig = baseUrl => {
   return {
-    browser: "chrome",
     configFile: "e2e/support/cypress-snapshots.config.js",
     config: {
       baseUrl,

e2e/runner/cypress-runner-run-tests.js

@@ -22,7 +22,6 @@ const runCypress = async (baseUrl, exitFunction) => {
   });
 
   const defaultConfig = {
-    browser: "chrome",
     configFile: "e2e/support/cypress.config.js",
     config: {
       baseUrl,

e2e/test/scenarios/admin/databases/add-new-database.cy.spec.js

@@ -24,7 +24,8 @@ describe("admin > database > add", () => {
     cy.findByLabelText("Database type").click();
   });
 
-  it("should add a new database", () => {
+  // Skipping as we are removing H2
+  it.skip("should add a new database", () => {
     popover().within(() => {
       if (isEE) {
         // EE should ship with Oracle and Vertica as options
@@ -56,6 +57,14 @@ describe("admin > database > add", () => {
 
   describe("external databases", { tags: "@external" }, () => {
     it("should add Postgres database and redirect to listing (metabase#12972, metabase#14334, metabase#17450)", () => {
+      popover().within(() => {
+        if (isEE) {
+          // EE should ship with Oracle and Vertica as options
+          cy.findByText("Oracle");
+          cy.findByText("Vertica");
+        }
+      });
+
       cy.contains("PostgreSQL").click({ force: true });
 
       cy.findByText("Show advanced options").click();

e2e/test/scenarios/filters/view.cy.spec.js

@@ -18,7 +18,11 @@ describe("scenarios > question > view", () => {
     beforeEach(() => {
       // All users upgraded to collection view access
       cy.visit("/admin/permissions/collections/root");
-      cy.icon("close").first().click();
+      cy.findByText("All Users")
+        .closest("tr")
+        .within(() => {
+          cy.icon("close").click();
+        });
       cy.findAllByRole("option").contains("View").click();
       cy.findByText("Save changes").click();
       cy.findByText("Yes").click();

e2e/test/scenarios/onboarding/setup/setup.cy.spec.js

@@ -114,20 +114,9 @@ describe("scenarios > setup", () => {
       cy.findByText("SQLite").click();
       cy.findByText("Need help connecting?");
 
-      // add h2 database
+      // remove database
       cy.findByLabelText("Remove database").click();
-      cy.findByText("Show more options").click();
-      cy.findByText("H2").click();
-      cy.findByLabelText("Display name").type("Metabase H2");
-      cy.findByText("Connect database").closest("button").should("be.disabled");
-
-      const dbFilename = "e2e/runner/empty.db";
-      const dbPath = Cypress.config("fileServerFolder") + "/" + dbFilename;
-      cy.findByLabelText("Connection String").type(`file:${dbPath}`);
-      cy.findByText("Connect database")
-        .closest("button")
-        .should("not.be.disabled")
-        .click();
+      cy.findByText("I'll add my data later").click();
 
       // test database setup help card is hidden on the next step
       cy.findByText("Need help connecting?").should("not.be.visible");
@@ -185,16 +174,7 @@ describe("scenarios > setup", () => {
 
     // Database
     cy.findByText("Add your data");
-    cy.findByText("I'll add my data later");
-
-    cy.findByText("Show more options").click();
-    cy.findByText("H2").click();
-    cy.findByLabelText("Display name").type("Metabase H2");
-
-    const dbFilename = "e2e/runner/empty.db";
-    const dbPath = Cypress.config("fileServerFolder") + "/" + dbFilename;
-    cy.findByLabelText("Connection String").type(`file:${dbPath}`);
-    cy.button("Connect database").click();
+    cy.findByText("I'll add my data later").click();
 
     // Turns off anonymous data collection
     cy.findByLabelText(

e2e/test/scenarios/visualizations/reproductions/11435-time-tooltip-native.cy.spec.js

@@ -30,11 +30,11 @@ describe("issue 11435", () => {
 
   it("should use time formatting settings in tooltips for native questions (metabase#11435)", () => {
     cy.createNativeQuestion(questionDetails, { visitQuestion: true });
-    clickLineDot({ index: 1 });
+    hoverLineDot({ index: 1 });
     popover().findByTextEnsureVisible("March 11, 2019, 8:45:17.010 PM");
   });
 });
 
-const clickLineDot = ({ index } = {}) => {
-  cy.get(".Visualization .dot").eq(index).click({ force: true });
+const hoverLineDot = ({ index } = {}) => {
+  cy.get(".Visualization .dot").eq(index).realHover();
 };

enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj

@@ -3,14 +3,16 @@
    [clojure.test :refer :all]
    [metabase-enterprise.advanced-config.file :as advanced-config.file]
    [metabase.db.connection :as mdb.connection]
+   [metabase.driver.h2 :as h2]
    [metabase.models :refer [Database Table]]
    [metabase.public-settings.premium-features-test :as premium-features-test]
    [metabase.test :as mt]
    [metabase.util :as u]
    [toucan.db :as db]))
 
 (use-fixtures :each (fn [thunk]
-                      (binding [advanced-config.file/*supported-versions* {:min 1, :max 1}]
+                      (binding [advanced-config.file/*supported-versions* {:min 1, :max 1}
+                                h2/*allow-testing-h2-connections*         true]
                         (premium-features-test/with-premium-features #{:advanced-config}
                           (thunk)))))
 

frontend/src/metabase/admin/app/selectors.ts

@@ -16,7 +16,10 @@ export const isNoticeEnabled = (state: State): boolean => {
 };
 
 export const hasDeprecatedDatabase = (state: State, props: Props): boolean => {
-  return props.databases?.some(d => isDeprecatedEngine(d.engine)) ?? false;
+  return (
+    props.databases?.some(d => !d.is_sample && isDeprecatedEngine(d.engine)) ??
+    false
+  );
 };
 
 export const getAdminPaths = (state: State) => {

src/metabase/api/database.clj

@@ -878,13 +878,16 @@
    settings           (s/maybe su/Map)}
   ;; TODO - ensure that custom schedules and let-user-control-scheduling go in lockstep
   (let [existing-database (api/write-check (db/select-one Database :id id))
-        details           (driver.u/db-details-client->server engine details)
-        details           (upsert-sensitive-fields existing-database details)
-        conn-error        (when (some? details)
-                            (assert (some? engine))
-                            (test-database-connection engine details))
-        full-sync?        (when-not (nil? is_full_sync)
-                            (boolean is_full_sync))]
+        details           (some->> details
+                                   (driver.u/db-details-client->server (or engine (:engine existing-database)))
+                                   (upsert-sensitive-fields existing-database))
+        ;; verify that we can connect to the database if `:details` OR `:engine` have changed.
+        details-changed?  (some-> details (not= (:details existing-database)))
+        engine-changed?   (some-> engine (not= (:engine existing-database)))
+        conn-error        (when (or details-changed? engine-changed?)
+                            (test-database-connection (or engine (:engine existing-database))
+                                                      (or details (:details existing-database))))
+        full-sync?        (some-> is_full_sync boolean)]
     (if conn-error
       ;; failed to connect, return error
       {:status 400

src/metabase/api/setup.clj

@@ -93,6 +93,8 @@
     (when-not (some-> (u/ignore-exceptions (driver/the-driver driver)) driver/available?)
       (let [msg (tru "Cannot create Database: cannot find driver {0}." driver)]
         (throw (ex-info msg {:errors {:database {:engine msg}}, :status-code 400}))))
+    (when-let [error (api.database/test-database-connection driver details)]
+      (throw (ex-info (:message error (tru "Cannot connect to Database")) (assoc error :status-code 400))))
     (db/insert! Database
       (merge
        {:name name, :engine driver, :details details, :creator_id creator-id}
@@ -180,6 +182,9 @@
   [:as {{{:keys [engine details]} :details, token :token} :body}]
   {token  SetupToken
    engine DBEngineString}
+  (when (setup/has-user-setup)
+    (throw (ex-info (tru "Instance already initialized")
+                    {:status-code 400})))
   (let [engine       (keyword engine)
         error-or-nil (api.database/test-database-connection engine details)]
     (when error-or-nil

src/metabase/api/table.clj

@@ -6,6 +6,7 @@
    [metabase.api.common :as api]
    [metabase.db.query :as mdb.query]
    [metabase.driver :as driver]
+   [metabase.driver.h2 :as h2]
    [metabase.driver.util :as driver.u]
    [metabase.models.card :refer [Card]]
    [metabase.models.field :refer [Field]]
@@ -15,7 +16,7 @@
    [metabase.related :as related]
    [metabase.sync :as sync]
    [metabase.sync.concurrent :as sync.concurrent]
-   #_:clj-kondo/ignore
+   #_{:clj-kondo/ignore [:consistent-alias]}
    [metabase.sync.field-values :as sync.field-values]
    [metabase.types :as types]
    [metabase.util :as u]
@@ -75,7 +76,10 @@
     (sync.concurrent/submit-task
      (fn []
        (let [database (table/database (first newly-unhidden))]
-         (if (driver.u/can-connect-with-details? (:engine database) (:details database))
+         ;; it's okay to allow testing H2 connections during sync. We only want to disallow you from testing them for the
+         ;; purposes of creating a new H2 database.
+         (if (binding [h2/*allow-testing-h2-connections* true]
+               (driver.u/can-connect-with-details? (:engine database) (:details database)))
            (doseq [table newly-unhidden]
              (log/info (u/format-color 'green (trs "Table ''{0}'' is now visible. Resyncing." (:name table))))
              (sync/sync-table! table))

src/metabase/cmd/copy.clj

@@ -163,18 +163,33 @@
       (log/error (with-out-str (jdbc/print-sql-exception-chain e)))
       (throw e))))
 
-(def ^:private table-select-fragments
-  {;; ensure ID order to ensure that parent fields are inserted before children
-   "metabase_field" "ORDER BY id ASC"})
+(def ^:dynamic *allow-loading-h2-databases*
+  "Whether `load-from-h2` should allow loading H2 databases. Normally disabled for security reasons. This is only here
+  so we can disable this check for tests."
+  false)
+
+(defn- table-select-fragment [table-name]
+  (cond
+    ;; ensure ID order to ensure that parent fields are inserted before children
+    (= table-name (name (t2/table-name Field)))
+    "ORDER BY id ASC"
+
+    ;; don't copy H2 Databases, since we don't allow people to create them for security reasons.
+    (when-not *allow-loading-h2-databases*
+      (= table-name (name (t2/table-name Database))))
+    "WHERE engine <> 'h2'"))
+
+(defn- sql-for-selecting-instances-from-source-db [table-name]
+  (let [fragment (table-select-fragment (u/lower-case-en (name table-name)))]
+    (str "SELECT * FROM "
+         (name table-name)
+         (when fragment (str " " fragment)))))
 
 (defn- copy-data! [^javax.sql.DataSource source-data-source target-db-type target-db-conn-spec]
   (with-open [source-conn (.getConnection source-data-source)]
     (doseq [entity entities
             :let   [table-name (t2/table-name entity)
-                    fragment   (table-select-fragments (u/lower-case-en (name table-name)))
-                    sql        (str "SELECT * FROM "
-                                    (name table-name)
-                                    (when fragment (str " " fragment)))
+                    sql        (sql-for-selecting-instances-from-source-db table-name)
                     results    (jdbc/reducible-query {:connection source-conn} sql)]]
       (transduce
        (partition-all chunk-size)

src/metabase/driver.clj

@@ -7,11 +7,13 @@
   these drivers define additional multimethods that child drivers should implement; see [[metabase.driver.sql]] and
   [[metabase.driver.sql-jdbc]] for more details."
   (:require
+   [clojure.set :as set]
    [clojure.string :as str]
    [java-time :as t]
    [metabase.driver.impl :as driver.impl]
    [metabase.models.setting :as setting :refer [defsetting]]
    [metabase.plugins.classloader :as classloader]
+   [metabase.util :as u]
    [metabase.util.i18n :refer [deferred-tru trs tru]]
    [metabase.util.log :as log]
    [potemkin :as p]
@@ -258,14 +260,28 @@
   [_]
   nil)
 
+(defn dispatch-on-initialized-driver-safe-keys
+  "Dispatch on initialized driver, except checks for `classname`,
+  `subprotocol`, `connection-uri` in the details map in order to
+  prevent a mismatch in spec type vs driver."
+  [driver details-map]
+  (let [invalid-keys #{"classname" "subprotocol" "connection-uri"}
+        ks           (->> details-map keys
+                          (map name)
+                          (map u/lower-case-en) set)]
+    (when (seq (set/intersection ks invalid-keys))
+      (throw (ex-info "Cannot specify subname, protocol, or connection-uri in details map"
+                      {:invalid-keys (set/intersection ks invalid-keys)})))
+    (dispatch-on-initialized-driver driver)))
+
 (defmulti can-connect?
   "Check whether we can connect to a `Database` with `details-map` and perform a simple query. For example, a SQL
   database might try running a query like `SELECT 1;`. This function should return truthy if a connection to the DB
   can be made successfully, otherwise it should return falsey or throw an appropriate Exception. Exceptions if a
   connection cannot be made. Throw an `ex-info` containing a truthy `::can-connect-message?` in `ex-data`
   in order to suppress logging expected driver validation messages during setup."
   {:arglists '([driver details])}
-  dispatch-on-initialized-driver
+  dispatch-on-initialized-driver-safe-keys
   :hierarchy #'hierarchy)
 
 (defmulti dbms-version