diff --git a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx index 94a568d1e317bc3fbd894c01bc73868f6f1b8223..0651978624adc7cfb125dea1857f0987b3e4dfe6 100644 --- a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx +++ b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx @@ -15,6 +15,7 @@ import ActionButton from "metabase/components/ActionButton"; import AddDatabaseHelpCard from "metabase/components/AddDatabaseHelpCard"; import Button from "metabase/components/Button"; import Breadcrumbs from "metabase/components/Breadcrumbs"; +import DriverWarning from "metabase/components/DriverWarning"; import Radio from "metabase/components/Radio"; import ModalWithTrigger from "metabase/components/ModalWithTrigger"; @@ -210,15 +211,20 @@ export default class DatabaseEditApp extends Component { )} </LoadingAndErrorWrapper> </Box> - {addingNewDatabase && ( - <Box> + <Box> + <DriverWarning + engine={selectedEngine} + ml={26} + data-testid="database-setup-driver-warning" + /> + {addingNewDatabase && ( <AddDatabaseHelpCard engine={selectedEngine} ml={26} data-testid="database-setup-help-card" /> - </Box> - )} + )} + </Box> </Flex> </div> </Box> diff --git a/frontend/src/metabase/components/DriverWarning/DriverWarning.jsx b/frontend/src/metabase/components/DriverWarning/DriverWarning.jsx new file mode 100644 index 0000000000000000000000000000000000000000..d91ad71189ce1ac080caecf6368aa69da463eeed --- /dev/null +++ b/frontend/src/metabase/components/DriverWarning/DriverWarning.jsx @@ -0,0 +1,97 @@ +import React from "react"; +import PropTypes from "prop-types"; + +import { t } from "ttag"; + +import { + allEngines, + engineSupersedesMap, +} from "metabase/entities/databases/forms"; + +import Warnings from "metabase/query_builder/components/Warnings"; + +import { + CardContent, + DriverWarningContainer, + IconContainer, +} from "./DriverWarning.styled"; +import ExternalLink from "metabase/components/ExternalLink"; +import MetabaseSettings from "metabase/lib/settings"; + +const propTypes = { + engine: PropTypes.string.isRequired, +}; + +const driverUpgradeHelpLink = MetabaseSettings.docsUrl( + "administration-guide/01-managing-databases", +); + +function getSupersedesWarningContent(newDriver, supersedesDriver) { + return ( + <div> + <p className="text-medium m0"> + {t`This is our new ${ + allEngines[newDriver]["driver-name"] + } driver, which is faster and more reliable.`} + </p> + <p>{t`The old driver has been deprecated and will be removed in the next release. If you really + need to use it, you can select ${ + allEngines[supersedesDriver]["driver-name"] + } now.`}</p> + </div> + ); +} + +function getSupersededByWarningContent(engine) { + return ( + <div> + <p className="text-medium m0"> + {t`This driver has been deprecated and will be removed in the next release.`} + </p> + <p className="text-medium m0"> + {t`We recommend that you upgrade to the new ${ + allEngines[engine]["driver-name"] + } driver, which is faster and more reliable.`} + </p> + <ExternalLink + href={driverUpgradeHelpLink} + className="text-brand text-bold" + > + {t`How to upgrade a driver`} + </ExternalLink> + </div> + ); +} + +function DriverWarning({ engine, ...props }) { + const supersededBy = engineSupersedesMap["superseded_by"][engine]; + const supersedes = engineSupersedesMap["supersedes"][engine]; + + if (!supersedes && !supersededBy) { + return null; + } + + const tooltipWarning = supersedes ? t`New driver` : t`Driver deprecated`; + const warningContent = supersedes + ? getSupersedesWarningContent(engine, supersedes) + : getSupersededByWarningContent(supersededBy); + + return ( + <DriverWarningContainer p={2} {...props}> + <IconContainer> + <Warnings + className="mx2 text-gold" + warnings={[tooltipWarning]} + size={20} + /> + </IconContainer> + <CardContent flexDirection="column" justify="center" className="ml2"> + {warningContent} + </CardContent> + </DriverWarningContainer> + ); +} + +DriverWarning.propTypes = propTypes; + +export default DriverWarning; diff --git a/frontend/src/metabase/components/DriverWarning/DriverWarning.styled.js b/frontend/src/metabase/components/DriverWarning/DriverWarning.styled.js new file mode 100644 index 0000000000000000000000000000000000000000..8d1f6436f2ae8caed6dd050c5284ce43a2d9f55c --- /dev/null +++ b/frontend/src/metabase/components/DriverWarning/DriverWarning.styled.js @@ -0,0 +1,17 @@ +import styled from "styled-components"; +import { Flex } from "grid-styled"; + +export const CardContent = styled(Flex)``; + +export const IconContainer = styled.div` + display: flex; + justify-content: center; + align-items: center; +`; + +export const DriverWarningContainer = styled(Flex)` + background-color: #f9fbfb; + border-radius: 10px; + width: 300px; + margin-bottom: 8px; +`; diff --git a/frontend/src/metabase/components/DriverWarning/index.js b/frontend/src/metabase/components/DriverWarning/index.js new file mode 100644 index 0000000000000000000000000000000000000000..80816f2cd1803d9d009355c6f2117b23e7ed93d5 --- /dev/null +++ b/frontend/src/metabase/components/DriverWarning/index.js @@ -0,0 +1 @@ +export { default } from "./DriverWarning"; diff --git a/frontend/src/metabase/entities/databases/forms.js b/frontend/src/metabase/entities/databases/forms.js index 9da262fe59c55370ca6865325cd51b9b312d67e2..f61bb41d397f09881f8ce34e2127063ebec0f4af 100644 --- a/frontend/src/metabase/entities/databases/forms.js +++ b/frontend/src/metabase/entities/databases/forms.js @@ -278,13 +278,51 @@ function getEngineFormFields(engine, details, id) { }); } -const ENGINE_OPTIONS = Object.entries(MetabaseSettings.get("engines") || {}) +const ENGINES = MetabaseSettings.get("engines", {}); +const ENGINE_OPTIONS = Object.entries(ENGINES) .map(([engine, info]) => ({ name: info["driver-name"], value: engine, })) .sort((a, b) => a.name.localeCompare(b.name)); +// use top level constant for engines so we only need to compute these maps once +const ENGINE_SUPERSEDES_MAPS = Object.keys(ENGINES).reduce( + (acc, engine) => { + const newEngine = ENGINES[engine]["superseded-by"]; + if (newEngine) { + acc.supersedes[newEngine] = engine; + acc.superseded_by[engine] = newEngine; + } + return acc; + }, + { supersedes: {}, superseded_by: {} }, +); + +/** + * Returns the options to show in the engines selection widget. An engine is available to be selected if either + * - it is not superseded by any other engine + * - it is the selected engine (i.e. someone is already using it) + * - it is superseded by some engine, which happens to be the currently selected one + * + * The idea behind this behavior is to only show someone a "legacy" driver if they have at least selected the one that + * will replace it first, at which point they can "fall back" on the legacy one if needed. + * + * @param currentEngine the current (selected engine) + * @returns the filtered engine options to be shown in the selection widget + */ +function getEngineOptions(currentEngine) { + return ENGINE_OPTIONS.filter(engine => { + const engineName = engine.value; + const newDriver = ENGINE_SUPERSEDES_MAPS["superseded_by"][engineName]; + return ( + typeof newDriver === "undefined" || + newDriver === currentEngine || + engineName === currentEngine + ); + }); +} + const forms = { details: { fields: ({ id, engine, details = {} } = {}) => [ @@ -292,7 +330,7 @@ const forms = { name: "engine", title: t`Database type`, type: "select", - options: ENGINE_OPTIONS, + options: getEngineOptions(engine), placeholder: t`Select a database`, }, { @@ -382,3 +420,5 @@ const SCHEDULING_FIELDS = new Set([ ]); export default forms; +export const engineSupersedesMap = ENGINE_SUPERSEDES_MAPS; +export const allEngines = ENGINES; diff --git a/frontend/src/metabase/setup/components/Setup.jsx b/frontend/src/metabase/setup/components/Setup.jsx index 18ec4319a8e901a8c568e9ed537d7dd47cd2cc52..ee7057acf7b50e0388099d3eecb095f4871a0d8b 100644 --- a/frontend/src/metabase/setup/components/Setup.jsx +++ b/frontend/src/metabase/setup/components/Setup.jsx @@ -8,6 +8,7 @@ import MetabaseAnalytics from "metabase/lib/analytics"; import MetabaseSettings from "metabase/lib/settings"; import AddDatabaseHelpCard from "metabase/components/AddDatabaseHelpCard"; +import DriverWarning from "metabase/components/DriverWarning"; import ExternalLink from "metabase/components/ExternalLink"; import LogoIcon from "metabase/components/LogoIcon"; import NewsletterForm from "metabase/components/NewsletterForm"; @@ -203,6 +204,11 @@ export default class Setup extends Component { </div> <AddDatabaseHelpCardHolder isVisible={isDatabaseHelpCardVisible}> + <DriverWarning + engine={selectedDatabaseEngine} + ml={26} + data-testid="database-setup-driver-warning" + /> <AddDatabaseHelpCard engine={selectedDatabaseEngine} hasCircle={false} diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj index 23f800d8d519efe4e20d8157f00f3ceb760bce8c..6492e0e8b282e3b66634f945321905da88c95c44 100644 --- a/src/metabase/driver.clj +++ b/src/metabase/driver.clj @@ -617,3 +617,19 @@ [_ db-details] ;; no normalization by default db-details) + +(defmulti superseded-by + "Returns the driver that supersedes the given `driver`. A non-nil return value means that the given `driver` is + deprecated in Metabase and will eventually be replaced by the returned driver, in some future version (at which point + any databases using it will be migrated to the new one). + + This is currently only used on the frontend for the purpose of showing/hiding deprecated drivers. A driver can make + use of this facility by adding a top-level `superseded-by` key to its plugin manifest YAML file, or (less preferred) + overriding this multimethod directly." + {:added "0.41.0" :arglists '([driver])} + dispatch-on-uninitialized-driver + :hierarchy #'hierarchy) + +(defmethod superseded-by :default + [_] + nil) diff --git a/src/metabase/driver/util.clj b/src/metabase/driver/util.clj index cb6111d21a92cce1c1ffb36ce219466be52628ea..860a6a66d6482f1cbdf1003391d3f1cd3c93d785 100644 --- a/src/metabase/driver/util.clj +++ b/src/metabase/driver/util.clj @@ -100,7 +100,8 @@ :when props] ;; TODO - maybe we should rename `details-fields` -> `connection-properties` on the FE as well? [driver {:details-fields props - :driver-name (driver/display-name driver)}]))) + :driver-name (driver/display-name driver) + :superseded-by (driver/superseded-by driver)}]))) ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | TLS Helpers | diff --git a/src/metabase/plugins/initialize.clj b/src/metabase/plugins/initialize.clj index 29bc6512ac9e1901d580a693fa2adec220833f67..666e5239a1cf0d88081986ccabc6c3a505e331d6 100644 --- a/src/metabase/plugins/initialize.clj +++ b/src/metabase/plugins/initialize.clj @@ -45,7 +45,7 @@ (@initialized-plugin-names plugin-name)) (s/defn init-plugin-with-info! - "Initiaize plugin using parsed info from a plugin maifest. Returns truthy if plugin was successfully initialized; + "Initialize plugin using parsed info from a plugin manifest. Returns truthy if plugin was successfully initialized; falsey otherwise." [info :- {:info {:name s/Str, :version s/Str, s/Keyword s/Any} s/Keyword s/Any}] diff --git a/src/metabase/plugins/lazy_loaded_driver.clj b/src/metabase/plugins/lazy_loaded_driver.clj index 8a519fcbc090d74ddf31069d14630d59dcb666b9..bae3c48a56f95b3e0e2f8afa582838ba7f82f77c 100644 --- a/src/metabase/plugins/lazy_loaded_driver.clj +++ b/src/metabase/plugins/lazy_loaded_driver.clj @@ -63,6 +63,7 @@ "Register a basic shell of a Metabase driver using the information from its Metabase plugin" [{:keys [add-to-classpath!] init-steps :init + superseded-by :superseded-by {driver-name :name, :keys [abstract display-name parent], :or {abstract false}, :as driver-info} :driver}] {:pre [(map? driver-info)]} (let [driver (keyword driver-name) @@ -81,7 +82,8 @@ (doseq [[^MultiFn multifn, f] {driver/initialize! (make-initialize! driver add-to-classpath! init-steps) driver/display-name (when display-name (constantly display-name)) - driver/connection-properties (constantly connection-props)}] + driver/connection-properties (constantly connection-props) + driver/superseded-by (constantly (keyword superseded-by))}] (when f (.addMethod multifn driver f))) ;; finally, register the Metabase driver diff --git a/test/metabase/driver/driver_deprecation_test_legacy.clj b/test/metabase/driver/driver_deprecation_test_legacy.clj new file mode 100644 index 0000000000000000000000000000000000000000..fe7aebbc0a720a0979b141f271943fbbf14731ec --- /dev/null +++ b/test/metabase/driver/driver_deprecation_test_legacy.clj @@ -0,0 +1,5 @@ +(ns metabase.driver.driver-deprecation-test-legacy + "Dummy driver for driver deprecation testing (legacy driver)" + (:require [metabase.driver :as driver])) + +(driver/register! :driver-deprecation-test-legacy, :parent :sql) diff --git a/test/metabase/driver/driver_deprecation_test_new.clj b/test/metabase/driver/driver_deprecation_test_new.clj new file mode 100644 index 0000000000000000000000000000000000000000..78ea7113cd7f654f41e2049a915d2893fe2e21b5 --- /dev/null +++ b/test/metabase/driver/driver_deprecation_test_new.clj @@ -0,0 +1,5 @@ +(ns metabase.driver.driver-deprecation-test-new + "Dummy driver for driver deprecation testing (new driver)" + (:require [metabase.driver :as driver])) + +(driver/register! :driver-deprecation-test-new, :parent :sql) diff --git a/test/metabase/plugins/driver_deprecation_test.clj b/test/metabase/plugins/driver_deprecation_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..e3c2da63cb4b6410ea9ee7a0fc3bcdb7f830ebe9 --- /dev/null +++ b/test/metabase/plugins/driver_deprecation_test.clj @@ -0,0 +1,13 @@ +(ns metabase.plugins.driver-deprecation-test + (:require [clojure.test :refer :all] + [metabase.models.setting :as setting] + [metabase.test :as mt] + [metabase.test.fixtures :as fixtures] + [metabase.util.i18n :as i18n :refer [tru]])) + +(use-fixtures :once (fixtures/initialize :plugins)) + +(deftest driver-deprecation-test + (mt/test-driver :driver-deprecation-test-legacy + (is (= :driver-deprecation-test-new + (get-in (setting/properties :public) [:engines :driver-deprecation-test-legacy :superseded-by]))))) diff --git a/test/metabase/test/data/driver_deprecation_test_legacy.clj b/test/metabase/test/data/driver_deprecation_test_legacy.clj new file mode 100644 index 0000000000000000000000000000000000000000..4d849b095ecbd297356b6dadb84785ef7959b355 --- /dev/null +++ b/test/metabase/test/data/driver_deprecation_test_legacy.clj @@ -0,0 +1,5 @@ +(ns metabase.test.data.driver-deprecation-test-legacy + "Dummy namespace for driver deprecation testing \"legacy\" driver, test data." + (:require [metabase.test.data.sql :as sql.tx])) + +(sql.tx/add-test-extensions! :driver-deprecation-test-legacy) diff --git a/test/metabase/test/data/driver_deprecation_test_new.clj b/test/metabase/test/data/driver_deprecation_test_new.clj new file mode 100644 index 0000000000000000000000000000000000000000..8ad92d734e03dd92d09f146a28e0892e051caf87 --- /dev/null +++ b/test/metabase/test/data/driver_deprecation_test_new.clj @@ -0,0 +1,5 @@ +(ns metabase.test.data.driver-deprecation-test-new + "Dummy namespace for driver deprecation testing \"new\" driver, test data." + (:require [metabase.test.data.sql :as sql.tx])) + +(sql.tx/add-test-extensions! :driver-deprecation-test-new) diff --git a/test/metabase/test/initialize/plugins.clj b/test/metabase/test/initialize/plugins.clj index 5794978769a0568d189c85682d5ac67946e7c4ea..eca64f5104b414faa49b2bbdb6fe302fc6145fb1 100644 --- a/test/metabase/test/initialize/plugins.clj +++ b/test/metabase/test/initialize/plugins.clj @@ -8,9 +8,23 @@ [yaml.core :as yaml])) (defn- driver-plugin-manifest [driver] - (let [manifest (io/file (format "modules/drivers/%s/resources/metabase-plugin.yaml" (name driver)))] - (when (.exists manifest) - (yaml/parse-string (slurp manifest))))) + (let [nm (name driver) + paths (mapv + #(format "%s/drivers/%s/resources/metabase-plugin.yaml" % nm) + ;; look for driver definition in both the regular modules directory, as well as in a top-level + ;; test_modules directory, specifically designed for test driver definitions + ["modules" "test_modules"])] + (first (filter some? + (for [path paths + :let [manifest (io/file path)] + :when (.exists manifest)] + (do + (println (u/format-color + 'green + "Loading plugin manifest (from %s) for driver as if it were a real plugin: %s" + path + nm)) + (yaml/parse-string (slurp manifest)))))))) (defn- driver-parents [driver] (let [parents-file (io/file (format "modules/drivers/%s/parents" (name driver)))] @@ -31,7 +45,6 @@ (doseq [driver drivers :let [info (driver-plugin-manifest driver)] :when info] - (println (u/format-color 'green "Loading plugin manifest for driver as if it were a real plugin: %s" driver)) (plugins.init/init-plugin-with-info! info) ;; ok, now we need to make sure we load any depenencies for those drivers as well (!) (load-plugin-manifests! (driver-parents driver))))) diff --git a/test_modules/drivers/driver-deprecation-test-legacy/resources/metabase-plugin.yaml b/test_modules/drivers/driver-deprecation-test-legacy/resources/metabase-plugin.yaml new file mode 100644 index 0000000000000000000000000000000000000000..0bbb3173bbc9bda1fdb52b0e976f7d63c6acbdac --- /dev/null +++ b/test_modules/drivers/driver-deprecation-test-legacy/resources/metabase-plugin.yaml @@ -0,0 +1,16 @@ +info: + name: Legacy Driver + version: 0.0.0-SNAPSHOT + description: Test plugin for driver deprecation (legacy) +driver: + name: driver-deprecation-test-legacy + display-name: Driver Deprecation Test Plugin (Legacy) + lazy-load: true + parent: sql + connection-properties: + - host + - port +superseded-by: driver-deprecation-test-new +init: + - step: load-namespace + namespace: metabase.driver.driver-deprecation-test-legacy diff --git a/test_modules/drivers/driver-deprecation-test-new/resources/metabase-plugin.yaml b/test_modules/drivers/driver-deprecation-test-new/resources/metabase-plugin.yaml new file mode 100644 index 0000000000000000000000000000000000000000..201a7981b3eeb1063049bbd19fe759f0da0e4596 --- /dev/null +++ b/test_modules/drivers/driver-deprecation-test-new/resources/metabase-plugin.yaml @@ -0,0 +1,15 @@ +info: + name: New Driver + version: 0.0.0-SNAPSHOT + description: Test plugin for driver deprecation (new) +driver: + name: driver-deprecation-test-new + display-name: Driver Deprecation Test Plugin (New) + lazy-load: true + parent: sql + connection-properties: + - host + - port +init: + - step: load-namespace + namespace: metabase.driver.driver-deprecation-test-new