From b36bd3f083d0b2c125f472c23caa39b035ee5f27 Mon Sep 17 00:00:00 2001 From: Antonio Rivero Martinez <38889534+Antonio-RiveroMartnez@users.noreply.github.com> Date: Mon, 26 Sep 2022 14:08:11 -0300 Subject: [PATCH] fix(databases): GSheets and Clickhouse DBs are not allowed to upload files (#21065) --- docs/static/resources/openapi.json | 13 + .../views/CRUD/data/database/DatabaseList.tsx | 8 +- .../database/DatabaseModal/ExtraOptions.tsx | 79 +++--- .../database/DatabaseModal/index.test.jsx | 197 +++++++++++++ .../data/database/DatabaseModal/index.tsx | 4 +- .../src/views/CRUD/data/database/types.ts | 5 + .../src/views/components/RightMenu.test.tsx | 268 ++++++++++++++++++ .../src/views/components/RightMenu.tsx | 18 +- .../src/views/components/SubMenu.tsx | 2 +- superset/databases/api.py | 10 + superset/db_engine_specs/base.py | 15 + superset/db_engine_specs/clickhouse.py | 2 + superset/db_engine_specs/gsheets.py | 2 + superset/models/core.py | 9 + superset/views/database/forms.py | 14 + .../integration_tests/databases/api_tests.py | 25 ++ 16 files changed, 625 insertions(+), 46 deletions(-) create mode 100644 superset-frontend/src/views/components/RightMenu.test.tsx diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index 29d9d7e8a9543..580b87d153015 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -3557,6 +3557,9 @@ }, "name": { "type": "string" + }, + "engine_information": { + "type": "object" } }, "type": "object" @@ -3738,6 +3741,9 @@ "sqlalchemy_uri": { "maxLength": 1024, "type": "string" + }, + "engine_information": { + "readOnly": true } }, "required": [ @@ -3817,6 +3823,9 @@ "id": { "format": "int32", "type": "integer" + }, + "engine_information": { + "readOnly": true } }, "required": [ @@ -13634,6 +13643,10 @@ "sqlalchemy_uri_placeholder": { "description": "Example placeholder for the SQLAlchemy URI", "type": "string" + }, + "engine_information": { + "description": "Object with properties we want to expose from our DB engine", + "type": "object" } }, "type": "object" diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx index f217a60ec450d..666c6e24101dd 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -229,7 +229,13 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { SupersetClient.get({ endpoint: `/api/v1/database/?q=${rison.encode(payload)}`, }).then(({ json }: Record) => { - setAllowUploads(json.count >= 1); + // There might be some existings Gsheets and Clickhouse DBs + // with allow_file_upload set as True which is not possible from now on + const allowedDatabasesWithFileUpload = + json?.result?.filter( + (database: any) => database?.engine_information?.supports_file_upload, + ) || []; + setAllowUploads(allowedDatabasesWithFileUpload?.length >= 1); }); }; diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx index 45504b3d5ee71..349264d6819fd 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx @@ -48,6 +48,8 @@ const ExtraOptions = ({ }) => { const expandableModalIsOpen = !!db?.expose_in_sqllab; const createAsOpen = !!(db?.allow_ctas || db?.allow_cvas); + const isFileUploadSupportedByEngine = + db?.engine_information?.supports_file_upload; return ( - -
- {t('Schemas allowed for CSV upload')} -
-
- -
-
- {t( - 'A comma-separated list of schemas that CSVs are allowed to upload to.', - )} -
- - +
- -
- - +
+ +
+ + )} + {isFileUploadSupportedByEngine && !!db?.allow_file_upload && ( + +
+ {t('Schemas allowed for File upload')} +
+
+ +
+
+ {t( + 'A comma-separated list of schemas that files are allowed to upload to.', )} - /> -
- +
+ + )} { const securityTab = screen.getByRole('tab', { name: /right security add extra connection information\./i, }); + const allowFileUploadCheckbox = screen.getByRole('checkbox', { + name: /Allow file uploads to database/i, + }); + const allowFileUploadText = screen.getByText( + /Allow file uploads to database/i, + ); + + const schemasForFileUploadText = screen.queryByText( + /Schemas allowed for File upload/i, + ); + + const visibleComponents = [ + closeButton, + advancedHeader, + basicHelper, + basicHeaderTitle, + basicHeaderSubtitle, + basicHeaderLink, + basicTab, + advancedTab, + sqlLabTab, + performanceTab, + securityTab, + allowFileUploadText, + ]; + // These components exist in the DOM but are not visible + const invisibleComponents = [allowFileUploadCheckbox]; // ---------- Assertions ---------- + visibleComponents.forEach(component => { + expect(component).toBeVisible(); + }); + invisibleComponents.forEach(component => { + expect(component).not.toBeVisible(); + }); + expect(schemasForFileUploadText).not.toBeInTheDocument(); + }); + + it('renders the "Advanced" - SECURITY tab correctly after selecting Allow file uploads', async () => { + // ---------- Components ---------- + // On step 1, click dbButton to access step 2 + userEvent.click( + screen.getByRole('button', { + name: /sqlite/i, + }), + ); + // Click the "Advanced" tab + userEvent.click(screen.getByRole('tab', { name: /advanced/i })); + // Click the "Security" tab + userEvent.click( + screen.getByRole('tab', { + name: /right security add extra connection information\./i, + }), + ); + // Click the "Allow file uploads" tab + + const allowFileUploadCheckbox = screen.getByRole('checkbox', { + name: /Allow file uploads to database/i, + }); + userEvent.click(allowFileUploadCheckbox); + + // ----- BEGIN STEP 2 (ADVANCED - SECURITY) + // - AntD header + const closeButton = screen.getByRole('button', { name: /close/i }); + const advancedHeader = screen.getByRole('heading', { + name: /connect a database/i, + }); + // - Connection header + const basicHelper = screen.getByText(/step 2 of 2/i); + const basicHeaderTitle = screen.getByText(/enter primary credentials/i); + const basicHeaderSubtitle = screen.getByText( + /need help\? learn how to connect your database \./i, + ); + const basicHeaderLink = within(basicHeaderSubtitle).getByRole('link', { + name: /here/i, + }); + // - Basic/Advanced tabs + const basicTab = screen.getByRole('tab', { name: /basic/i }); + const advancedTab = screen.getByRole('tab', { name: /advanced/i }); + // - Advanced tabs + const sqlLabTab = screen.getByRole('tab', { + name: /right sql lab adjust how this database will interact with sql lab\./i, + }); + const performanceTab = screen.getByRole('tab', { + name: /right performance adjust performance settings of this database\./i, + }); + const securityTab = screen.getByRole('tab', { + name: /right security add extra connection information\./i, + }); + const allowFileUploadText = screen.getByText( + /Allow file uploads to database/i, + ); + + const schemasForFileUploadText = screen.queryByText( + /Schemas allowed for File upload/i, + ); + const visibleComponents = [ closeButton, advancedHeader, @@ -775,11 +898,19 @@ describe('DatabaseModal', () => { sqlLabTab, performanceTab, securityTab, + allowFileUploadText, ]; + // These components exist in the DOM but are not visible + const invisibleComponents = [allowFileUploadCheckbox]; + // ---------- Assertions ---------- visibleComponents.forEach(component => { expect(component).toBeVisible(); }); + invisibleComponents.forEach(component => { + expect(component).not.toBeVisible(); + }); + expect(schemasForFileUploadText).toBeInTheDocument(); }); test('renders the "Advanced" - OTHER tab correctly', async () => { @@ -1072,4 +1203,70 @@ describe('DatabaseModal', () => { expect(step2of3text).toBeVisible(); }); }); + + describe('DatabaseModal w/ GSheet Engine', () => { + const renderAndWait = async () => { + const dbProps = { + show: true, + database_name: 'my database', + sqlalchemy_uri: 'gsheets://', + }; + const mounted = act(async () => { + render(, { + useRedux: true, + }); + }); + + return mounted; + }; + + beforeEach(async () => { + await renderAndWait(); + }); + + it('enters step 2 of 2 when proper database is selected', () => { + const step2of2text = screen.getByText(/step 2 of 2/i); + expect(step2of2text).toBeVisible(); + }); + + it('renders the "Advanced" - SECURITY tab without Allow File Upload Checkbox', async () => { + // Click the "Advanced" tab + userEvent.click(screen.getByRole('tab', { name: /advanced/i })); + // Click the "Security" tab + userEvent.click( + screen.getByRole('tab', { + name: /right security add extra connection information\./i, + }), + ); + + // ----- BEGIN STEP 2 (ADVANCED - SECURITY) + // - Advanced tabs + const impersonateLoggerUserCheckbox = screen.getByRole('checkbox', { + name: /impersonate logged in/i, + }); + const impersonateLoggerUserText = screen.getByText( + /impersonate logged in/i, + ); + const allowFileUploadText = screen.queryByText( + /Allow file uploads to database/i, + ); + const schemasForFileUploadText = screen.queryByText( + /Schemas allowed for File upload/i, + ); + + const visibleComponents = [impersonateLoggerUserText]; + // These components exist in the DOM but are not visible + const invisibleComponents = [impersonateLoggerUserCheckbox]; + + // ---------- Assertions ---------- + visibleComponents.forEach(component => { + expect(component).toBeVisible(); + }); + invisibleComponents.forEach(component => { + expect(component).not.toBeVisible(); + }); + expect(allowFileUploadText).not.toBeInTheDocument(); + expect(schemasForFileUploadText).not.toBeInTheDocument(); + }); + }); }); diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index d33a5cb521f9a..9fb55246ce510 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -175,6 +175,7 @@ type DBReducerActionType = database_name?: string; engine?: string; configuration_method: CONFIGURATION_METHOD; + engine_information?: {}; }; } | { @@ -718,7 +719,7 @@ const DatabaseModal: FunctionComponent = ({ const selectedDbModel = availableDbs?.databases.filter( (db: DatabaseObject) => db.name === database_name, )[0]; - const { engine, parameters } = selectedDbModel; + const { engine, parameters, engine_information } = selectedDbModel; const isDynamic = parameters !== undefined; setDB({ type: ActionType.dbSelected, @@ -728,6 +729,7 @@ const DatabaseModal: FunctionComponent = ({ configuration_method: isDynamic ? CONFIGURATION_METHOD.DYNAMIC_FORM : CONFIGURATION_METHOD.SQLALCHEMY_URI, + engine_information, }, }); } diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts index 92dd8e187851b..9a9386035eebc 100644 --- a/superset-frontend/src/views/CRUD/data/database/types.ts +++ b/superset-frontend/src/views/CRUD/data/database/types.ts @@ -101,6 +101,11 @@ export type DatabaseObject = { catalog?: Array; query_input?: string; extra?: string; + + // DB Engine Spec information + engine_information?: { + supports_file_upload?: boolean; + }; }; export type DatabaseForm = { diff --git a/superset-frontend/src/views/components/RightMenu.test.tsx b/superset-frontend/src/views/components/RightMenu.test.tsx new file mode 100644 index 0000000000000..f5ca82845d3c5 --- /dev/null +++ b/superset-frontend/src/views/components/RightMenu.test.tsx @@ -0,0 +1,268 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import * as reactRedux from 'react-redux'; +import fetchMock from 'fetch-mock'; +import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; +import { styledMount as mount } from 'spec/helpers/theming'; +import RightMenu from './RightMenu'; +import { RightMenuProps } from './types'; + +jest.mock('react-redux', () => ({ + ...jest.requireActual('react-redux'), + useSelector: jest.fn(), +})); + +const createProps = (): RightMenuProps => ({ + align: 'flex-end', + navbarRight: { + show_watermark: false, + bug_report_url: '/report/', + documentation_url: '/docs/', + languages: { + en: { + flag: 'us', + name: 'English', + url: '/lang/en', + }, + it: { + flag: 'it', + name: 'Italian', + url: '/lang/it', + }, + }, + show_language_picker: true, + user_is_anonymous: true, + user_info_url: '/users/userinfo/', + user_logout_url: '/logout/', + user_login_url: '/login/', + user_profile_url: '/profile/', + locale: 'en', + version_string: '1.0.0', + version_sha: 'randomSHA', + build_number: 'randomBuildNumber', + }, + settings: [ + { + name: 'Security', + icon: 'fa-cogs', + label: 'Security', + index: 1, + childs: [ + { + name: 'List Users', + icon: 'fa-user', + label: 'List Users', + url: '/users/list/', + index: 1, + }, + ], + }, + ], + isFrontendRoute: () => true, + environmentTag: { + color: 'error.base', + text: 'Development', + }, +}); + +const useSelectorMock = jest.spyOn(reactRedux, 'useSelector'); +const useStateMock = jest.spyOn(React, 'useState'); + +let setShowModal: any; +let setEngine: any; +let setAllowUploads: any; + +const mockNonGSheetsDBs = [...new Array(2)].map((_, i) => ({ + changed_by: { + first_name: `user`, + last_name: `${i}`, + }, + database_name: `db ${i}`, + backend: 'postgresql', + allow_run_async: true, + allow_dml: false, + allow_file_upload: true, + expose_in_sqllab: false, + changed_on_delta_humanized: `${i} day(s) ago`, + changed_on: new Date().toISOString, + id: i, + engine_information: { + supports_file_upload: true, + }, +})); + +const mockGsheetsDbs = [...new Array(2)].map((_, i) => ({ + changed_by: { + first_name: `user`, + last_name: `${i}`, + }, + database_name: `db ${i}`, + backend: 'gsheets', + allow_run_async: true, + allow_dml: false, + allow_file_upload: true, + expose_in_sqllab: false, + changed_on_delta_humanized: `${i} day(s) ago`, + changed_on: new Date().toISOString, + id: i, + engine_information: { + supports_file_upload: false, + }, +})); + +describe('RightMenu', () => { + const mockedProps = createProps(); + + beforeEach(async () => { + useSelectorMock.mockReset(); + useStateMock.mockReset(); + fetchMock.get( + 'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))', + { result: [], database_count: 0 }, + ); + // By default we get file extensions to be uploaded + useSelectorMock.mockReturnValue({ + CSV_EXTENSIONS: ['csv'], + EXCEL_EXTENSIONS: ['xls', 'xlsx'], + COLUMNAR_EXTENSIONS: ['parquet', 'zip'], + ALLOWED_EXTENSIONS: ['parquet', 'zip', 'xls', 'xlsx', 'csv'], + }); + setShowModal = jest.fn(); + setEngine = jest.fn(); + setAllowUploads = jest.fn(); + const mockSetStateModal: any = (x: any) => [x, setShowModal]; + const mockSetStateEngine: any = (x: any) => [x, setEngine]; + const mockSetStateAllow: any = (x: any) => [x, setAllowUploads]; + useStateMock.mockImplementationOnce(mockSetStateModal); + useStateMock.mockImplementationOnce(mockSetStateEngine); + useStateMock.mockImplementationOnce(mockSetStateAllow); + }); + afterEach(fetchMock.restore); + it('renders', async () => { + const wrapper = mount(); + await waitForComponentToPaint(wrapper); + expect(wrapper.find(RightMenu)).toExist(); + }); + it('If user has permission to upload files we query the existing DBs that has allow_file_upload as True', async () => { + useSelectorMock.mockReturnValueOnce({ + createdOn: '2021-04-27T18:12:38.952304', + email: 'admin', + firstName: 'admin', + isActive: true, + lastName: 'admin', + permissions: {}, + roles: { + Admin: [ + ['can_this_form_get', 'CsvToDatabaseView'], // So we can upload CSV + ], + }, + userId: 1, + username: 'admin', + }); + // Second call we get the dashboardId + useSelectorMock.mockReturnValueOnce('1'); + const wrapper = mount(); + await waitForComponentToPaint(wrapper); + const callsD = fetchMock.calls(/database\/\?q/); + expect(callsD).toHaveLength(1); + expect(callsD[0][0]).toMatchInlineSnapshot( + `"http://localhost/api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))"`, + ); + }); + it('If user has no permission to upload files the query API should not be called', async () => { + useSelectorMock.mockReturnValueOnce({ + createdOn: '2021-04-27T18:12:38.952304', + email: 'admin', + firstName: 'admin', + isActive: true, + lastName: 'admin', + permissions: {}, + roles: { + Admin: [['can_write', 'Chart']], // no file permissions + }, + userId: 1, + username: 'admin', + }); + // Second call we get the dashboardId + useSelectorMock.mockReturnValueOnce('1'); + const wrapper = mount(); + await waitForComponentToPaint(wrapper); + const callsD = fetchMock.calls(/database\/\?q/); + expect(callsD).toHaveLength(0); + }); + it('If user has permission to upload files but there are only gsheets and clickhouse DBs', async () => { + fetchMock.get( + 'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))', + { result: [...mockGsheetsDbs], database_count: 2 }, + { overwriteRoutes: true }, + ); + useSelectorMock.mockReturnValueOnce({ + createdOn: '2021-04-27T18:12:38.952304', + email: 'admin', + firstName: 'admin', + isActive: true, + lastName: 'admin', + permissions: {}, + roles: { + Admin: [ + ['can_this_form_get', 'CsvToDatabaseView'], // So we can upload CSV + ], + }, + userId: 1, + username: 'admin', + }); + // Second call we get the dashboardId + useSelectorMock.mockReturnValueOnce('1'); + const wrapper = mount(); + await waitForComponentToPaint(wrapper); + const callsD = fetchMock.calls(/database\/\?q/); + expect(callsD).toHaveLength(1); + expect(setAllowUploads).toHaveBeenCalledWith(false); + }); + it('If user has permission to upload files and some DBs with allow_file_upload are not gsheets nor clickhouse', async () => { + fetchMock.get( + 'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))', + { result: [...mockNonGSheetsDBs, ...mockGsheetsDbs], database_count: 2 }, + { overwriteRoutes: true }, + ); + useSelectorMock.mockReturnValueOnce({ + createdOn: '2021-04-27T18:12:38.952304', + email: 'admin', + firstName: 'admin', + isActive: true, + lastName: 'admin', + permissions: {}, + roles: { + Admin: [ + ['can_this_form_get', 'CsvToDatabaseView'], // So we can upload CSV + ], + }, + userId: 1, + username: 'admin', + }); + // Second call we get the dashboardId + useSelectorMock.mockReturnValueOnce('1'); + const wrapper = mount(); + await waitForComponentToPaint(wrapper); + const callsD = fetchMock.calls(/database\/\?q/); + expect(callsD).toHaveLength(1); + expect(setAllowUploads).toHaveBeenCalledWith(true); + }); +}); diff --git a/superset-frontend/src/views/components/RightMenu.tsx b/superset-frontend/src/views/components/RightMenu.tsx index 27f84a0be697d..cacd4089cb092 100644 --- a/superset-frontend/src/views/components/RightMenu.tsx +++ b/superset-frontend/src/views/components/RightMenu.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { Fragment, useState, useEffect } from 'react'; +import React, { Fragment, useEffect } from 'react'; import rison from 'rison'; import { useSelector } from 'react-redux'; import { Link } from 'react-router-dom'; @@ -118,8 +118,8 @@ const RightMenu = ({ ALLOWED_EXTENSIONS, HAS_GSHEETS_INSTALLED, } = useSelector(state => state.common.conf); - const [showModal, setShowModal] = useState(false); - const [engine, setEngine] = useState(''); + const [showModal, setShowModal] = React.useState(false); + const [engine, setEngine] = React.useState(''); const canSql = findPermission('can_sqllab', 'Superset', roles); const canDashboard = findPermission('can_write', 'Dashboard', roles); const canChart = findPermission('can_write', 'Chart', roles); @@ -135,7 +135,7 @@ const RightMenu = ({ ); const showActionDropdown = canSql || canChart || canDashboard; - const [allowUploads, setAllowUploads] = useState(false); + const [allowUploads, setAllowUploads] = React.useState(false); const isAdmin = isUserAdmin(user); const showUploads = allowUploads || isAdmin; const dropdownItems: MenuObjectProps[] = [ @@ -207,7 +207,13 @@ const RightMenu = ({ SupersetClient.get({ endpoint: `/api/v1/database/?q=${rison.encode(payload)}`, }).then(({ json }: Record) => { - setAllowUploads(json.count >= 1); + // There might be some existings Gsheets and Clickhouse DBs + // with allow_file_upload set as True which is not possible from now on + const allowedDatabasesWithFileUpload = + json?.result?.filter( + (database: any) => database?.engine_information?.supports_file_upload, + ) || []; + setAllowUploads(allowedDatabasesWithFileUpload?.length >= 1); }); }; @@ -241,7 +247,7 @@ const RightMenu = ({ const isDisabled = isAdmin && !allowUploads; const tooltipText = t( - "Enable 'Allow data upload' in any database's settings", + "Enable 'Allow file uploads to database' in any database's settings", ); const buildMenuItem = (item: Record) => { diff --git a/superset-frontend/src/views/components/SubMenu.tsx b/superset-frontend/src/views/components/SubMenu.tsx index e51c1e8eca8ce..6ace899bf6e48 100644 --- a/superset-frontend/src/views/components/SubMenu.tsx +++ b/superset-frontend/src/views/components/SubMenu.tsx @@ -302,7 +302,7 @@ const SubMenuComponent: React.FunctionComponent = props => { {item.label} diff --git a/superset/databases/api.py b/superset/databases/api.py index e61bce68db481..edf63392ee6f6 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -129,6 +129,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "server_cert", "sqlalchemy_uri", "is_managed_externally", + "engine_information", ] list_columns = [ "allow_file_upload", @@ -151,6 +152,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "force_ctas_schema", "id", "disable_data_preview", + "engine_information", ] add_columns = [ "database_name", @@ -1062,6 +1064,13 @@ def available(self) -> Response: parameters: description: JSON schema defining the needed parameters type: object + engine_information: + description: Dict with public properties form the DB Engine + type: object + properties: + supports_file_upload: + description: Whether the engine supports file uploads + type: boolean 400: $ref: '#/components/responses/400' 500: @@ -1078,6 +1087,7 @@ def available(self) -> Response: "engine": engine_spec.engine, "available_drivers": sorted(drivers), "preferred": engine_spec.engine_name in preferred_databases, + "engine_information": engine_spec.get_public_information(), } if engine_spec.default_driver: diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 28a6e2f5c3899..5a8354fd78dc7 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -361,6 +361,10 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]] ] = {} + # Whether the engine supports file uploads + # if True, database will be listed as option in the upload file form + supports_file_upload = True + @classmethod def supports_url(cls, url: URL) -> bool: """ @@ -1637,6 +1641,17 @@ def unmask_encrypted_extra(cls, old: str, new: str) -> str: """ return new + @classmethod + def get_public_information(cls) -> Dict[str, Any]: + """ + Construct a Dict with properties we want to expose. + + :returns: Dict with properties of our class like supports_file_upload + """ + return { + "supports_file_upload": cls.supports_file_upload, + } + # schema for adding a database by providing parameters instead of the # full SQLAlchemy URI diff --git a/superset/db_engine_specs/clickhouse.py b/superset/db_engine_specs/clickhouse.py index 4f34d2a5543c2..4531dca69860e 100644 --- a/superset/db_engine_specs/clickhouse.py +++ b/superset/db_engine_specs/clickhouse.py @@ -58,6 +58,8 @@ class ClickHouseEngineSpec(BaseEngineSpec): # pylint: disable=abstract-method _show_functions_column = "name" + supports_file_upload = False + @classmethod def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]: return {NewConnectionError: SupersetDBAPIDatabaseError} diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index acde55b480c2b..3ee284788c245 100644 --- a/superset/db_engine_specs/gsheets.py +++ b/superset/db_engine_specs/gsheets.py @@ -81,6 +81,8 @@ class GSheetsEngineSpec(SqliteEngineSpec): ), } + supports_file_upload = False + @classmethod def get_url_for_impersonation( cls, diff --git a/superset/models/core.py b/superset/models/core.py index a8ab4df6b02cf..8c1b58171d470 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -231,6 +231,7 @@ def data(self) -> Dict[str, Any]: "parameters": self.parameters, "disable_data_preview": self.disable_data_preview, "parameters_schema": self.parameters_schema, + "engine_information": self.engine_information, } @property @@ -312,6 +313,14 @@ def default_schemas(self) -> List[str]: def connect_args(self) -> Dict[str, Any]: return self.get_extra().get("engine_params", {}).get("connect_args", {}) + @property + def engine_information(self) -> Dict[str, Any]: + try: + engine_information = self.db_engine_spec.get_public_information() + except Exception: # pylint: disable=broad-except + engine_information = {} + return engine_information + @classmethod def get_password_masked_url_from_uri( # pylint: disable=invalid-name cls, uri: str diff --git a/superset/views/database/forms.py b/superset/views/database/forms.py index 4cf594c48705e..a44a412b483b0 100644 --- a/superset/views/database/forms.py +++ b/superset/views/database/forms.py @@ -52,6 +52,7 @@ def file_allowed_dbs() -> List[Database]: # type: ignore file_enabled_db for file_enabled_db in file_enabled_dbs if UploadToDatabaseForm.at_least_one_schema_is_allowed(file_enabled_db) + and UploadToDatabaseForm.is_engine_allowed_to_file_upl(file_enabled_db) ] @staticmethod @@ -89,6 +90,19 @@ def at_least_one_schema_is_allowed(database: Database) -> bool: return True return False + @staticmethod + def is_engine_allowed_to_file_upl(database: Database) -> bool: + """ + This method is mainly used for existing Gsheets and Clickhouse DBs + that have allow_file_upload set as True but they are no longer valid + DBs for file uploading. + New GSheets and Clickhouse DBs won't have the option to set + allow_file_upload set as True. + """ + if database.db_engine_spec.supports_file_upload: + return True + return False + class CsvToDatabaseForm(UploadToDatabaseForm): name = StringField( diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py index fab3708c9968d..4bd80279a0274 100644 --- a/tests/integration_tests/databases/api_tests.py +++ b/tests/integration_tests/databases/api_tests.py @@ -195,6 +195,7 @@ def test_get_items(self): "created_by", "database_name", "disable_data_preview", + "engine_information", "explore_database_id", "expose_in_sqllab", "extra", @@ -1941,6 +1942,9 @@ def test_available(self, app, get_available_engine_specs): }, "preferred": True, "sqlalchemy_uri_placeholder": "postgresql://user:password@host:port/dbname[?key=value&key=value...]", + "engine_information": { + "supports_file_upload": True, + }, }, { "available_drivers": ["bigquery"], @@ -1960,6 +1964,9 @@ def test_available(self, app, get_available_engine_specs): }, "preferred": True, "sqlalchemy_uri_placeholder": "bigquery://{project_id}", + "engine_information": { + "supports_file_upload": True, + }, }, { "available_drivers": ["psycopg2"], @@ -2008,6 +2015,9 @@ def test_available(self, app, get_available_engine_specs): }, "preferred": False, "sqlalchemy_uri_placeholder": "redshift+psycopg2://user:password@host:port/dbname[?key=value&key=value...]", + "engine_information": { + "supports_file_upload": True, + }, }, { "available_drivers": ["apsw"], @@ -2027,6 +2037,9 @@ def test_available(self, app, get_available_engine_specs): }, "preferred": False, "sqlalchemy_uri_placeholder": "gsheets://", + "engine_information": { + "supports_file_upload": False, + }, }, { "available_drivers": ["mysqlconnector", "mysqldb"], @@ -2075,12 +2088,18 @@ def test_available(self, app, get_available_engine_specs): }, "preferred": False, "sqlalchemy_uri_placeholder": "mysql://user:password@host:port/dbname[?key=value&key=value...]", + "engine_information": { + "supports_file_upload": True, + }, }, { "available_drivers": [""], "engine": "hana", "name": "SAP HANA", "preferred": False, + "engine_information": { + "supports_file_upload": True, + }, }, ] } @@ -2108,12 +2127,18 @@ def test_available_no_default(self, app, get_available_engine_specs): "engine": "mysql", "name": "MySQL", "preferred": True, + "engine_information": { + "supports_file_upload": True, + }, }, { "available_drivers": [""], "engine": "hana", "name": "SAP HANA", "preferred": False, + "engine_information": { + "supports_file_upload": True, + }, }, ] }