You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by va...@apache.org on 2025/02/27 22:20:11 UTC
(superset) branch feat/async-db-perm-sync updated: New approach after syncing with Beto
This is an automated email from the ASF dual-hosted git repository.
vavila pushed a commit to branch feat/async-db-perm-sync
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/feat/async-db-perm-sync by this push:
new 4b72e2ac43 New approach after syncing with Beto
4b72e2ac43 is described below
commit 4b72e2ac43550af1835b4a360c4c3a443b26fc8f
Author: Vitor Avila <vi...@preset.io>
AuthorDate: Thu Feb 27 19:19:57 2025 -0300
New approach after syncing with Beto
---
superset/commands/database/sync_permissions.py | 339 ++++++++++++++--
superset/commands/database/update.py | 2 +-
superset/tasks/permissions.py | 259 ------------
tests/unit_tests/commands/databases/conftest.py | 90 +++++
.../commands/databases/sync_permissions_test.py | 293 +++++++++++---
tests/unit_tests/commands/databases/update_test.py | 263 ++++++-------
tests/unit_tests/tasks/permissions_test.py | 438 ---------------------
7 files changed, 761 insertions(+), 923 deletions(-)
diff --git a/superset/commands/database/sync_permissions.py b/superset/commands/database/sync_permissions.py
index ba1f056889..f7a4000b6c 100644
--- a/superset/commands/database/sync_permissions.py
+++ b/superset/commands/database/sync_permissions.py
@@ -18,6 +18,14 @@ from __future__ import annotations
import logging
from functools import partial
+from typing import Iterable
+
+from flask import current_app, g
+from flask_appbuilder.security.sqla.models import (
+ Permission,
+ PermissionView,
+ ViewMenu,
+)
from superset import app, security_manager
from superset.commands.base import BaseCommand
@@ -29,9 +37,13 @@ from superset.commands.database.exceptions import (
)
from superset.commands.database.utils import ping
from superset.daos.database import DatabaseDAO
+from superset.daos.dataset import DatasetDAO
from superset.databases.ssh_tunnel.models import SSHTunnel
+from superset.db_engine_specs.base import GenericDBException
+from superset.exceptions import OAuth2RedirectError
+from superset.extensions import celery_app, db
from superset.models.core import Database
-from superset.tasks.permissions import sync_database_permissions
+from superset.security.manager import SupersetSecurityManager
from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
@@ -40,6 +52,10 @@ logger = logging.getLogger(__name__)
class SyncPermissionsCommand(BaseCommand):
"""
Command to sync database permissions.
+
+ This command can be called either via its dedicated endpoint, or as part of
+ another command. If async mode is enabled, the command is executed through
+ a celery task, otherwise it's executed synchronously.
"""
def __init__(
@@ -55,19 +71,33 @@ class SyncPermissionsCommand(BaseCommand):
"""
self.db_connection_id = model_id
self.username = username
- self.old_db_connection_name: str | None = old_db_connection_name
- self.db_connection: Database | None = db_connection
+ self._old_db_connection_name: str | None = old_db_connection_name
+ self._db_connection: Database | None = db_connection
self.db_connection_ssh_tunnel: SSHTunnel | None = ssh_tunnel
self.async_mode: bool = app.config["SYNC_DB_PERMISSIONS_IN_ASYNC_MODE"]
+ @property
+ def db_connection(self) -> Database:
+ if not self._db_connection:
+ raise DatabaseNotFoundError()
+ return self._db_connection
+
+ @property
+ def old_db_connection_name(self) -> str:
+ return (
+ self._old_db_connection_name
+ if self._old_db_connection_name is not None
+ else self.db_connection.database_name
+ )
+
def validate(self) -> None:
- self.db_connection = (
- self.db_connection
- if self.db_connection
+ self._db_connection = (
+ self._db_connection
+ if self._db_connection
else DatabaseDAO.find_by_id(self.db_connection_id)
)
- if not self.db_connection:
+ if not self._db_connection:
raise DatabaseNotFoundError()
if not self.db_connection_ssh_tunnel:
@@ -75,12 +105,6 @@ class SyncPermissionsCommand(BaseCommand):
self.db_connection_id
)
- self.old_db_connection_name = (
- self.old_db_connection_name
- if self.old_db_connection_name
- else self.db_connection.database_name
- )
-
# Need user info to impersonate for OAuth2 connections
if not self.username or not security_manager.get_user_by_username(
self.username
@@ -98,21 +122,286 @@ class SyncPermissionsCommand(BaseCommand):
if not alive:
raise DatabaseConnectionFailedError()
- def _run(self) -> None:
+ def run(self) -> None:
"""
Triggers the perm sync in sync or async mode.
"""
self.validate()
- args = [self.db_connection_id, self.username, self.old_db_connection_name]
if self.async_mode:
- sync_database_permissions.delay(*args)
- else:
- sync_database_permissions(*args)
-
- # This command can be called either via its dedicated endpoint, or as part of
- # another command. In the latter, we don't want to start a nested transaction
- # to ensure an atomic operation, so ``run_without_transaction`` should be used.
- run = transaction(
+ sync_database_permissions_task.delay(
+ self.db_connection_id, self.username, self.old_db_connection_name
+ )
+ return
+
+ self.sync_database_permissions()
+
+ @transaction(
on_error=partial(on_error, reraise=DatabaseConnectionSyncPermissionsError)
- )(_run)
- run_without_transaction = _run
+ )
+ def sync_database_permissions(self) -> None:
+ """
+ Syncs the permissions for a DB connection.
+ """
+ catalogs = (
+ self._get_catalog_names()
+ if self.db_connection.db_engine_spec.supports_catalog
+ else [None]
+ )
+
+ for catalog in catalogs:
+ try:
+ schemas = self._get_schema_names(catalog)
+
+ if catalog:
+ perm = security_manager.get_catalog_perm(
+ self.old_db_connection_name,
+ catalog,
+ )
+ existing_pvm = security_manager.find_permission_view_menu(
+ "catalog_access",
+ perm,
+ )
+ if not existing_pvm:
+ # new catalog
+ add_pvm(
+ security_manager,
+ "catalog_access",
+ security_manager.get_catalog_perm(
+ self.db_connection.database_name,
+ catalog,
+ ),
+ )
+ for schema in schemas:
+ add_pvm(
+ security_manager,
+ "schema_access",
+ security_manager.get_schema_perm(
+ self.db_connection.database_name,
+ catalog,
+ schema,
+ ),
+ )
+ continue
+ except DatabaseConnectionFailedError:
+ logger.warning("Error processing catalog %s", catalog or "(default)")
+ continue
+
+ # add possible new schemas in catalog
+ self._refresh_schemas(catalog, schemas)
+
+ if self.old_db_connection_name != self.db_connection.database_name:
+ self._rename_database_in_permissions(catalog, schemas)
+
+ def _get_catalog_names(self) -> set[str]:
+ """
+ Helper method to load catalogs.
+ """
+ try:
+ return self.db_connection.get_all_catalog_names(
+ force=True,
+ ssh_tunnel=self.db_connection_ssh_tunnel,
+ )
+ except OAuth2RedirectError:
+ # raise OAuth2 exceptions as-is
+ raise
+ except GenericDBException as ex:
+ raise DatabaseConnectionFailedError() from ex
+
+ def _get_schema_names(self, catalog: str | None) -> set[str]:
+ """
+ Helper method to load schemas.
+ """
+ try:
+ return self.db_connection.get_all_schema_names(
+ force=True,
+ catalog=catalog,
+ ssh_tunnel=self.db_connection_ssh_tunnel,
+ )
+ except OAuth2RedirectError:
+ # raise OAuth2 exceptions as-is
+ raise
+ except GenericDBException as ex:
+ raise DatabaseConnectionFailedError() from ex
+
+ def _refresh_schemas(self, catalog: str | None, schemas: Iterable[str]) -> None:
+ """
+ Add new schemas that don't have permissions yet.
+ """
+ for schema in schemas:
+ perm = security_manager.get_schema_perm(
+ self.old_db_connection_name,
+ catalog,
+ schema,
+ )
+ existing_pvm = security_manager.find_permission_view_menu(
+ "schema_access",
+ perm,
+ )
+ if not existing_pvm:
+ new_name = security_manager.get_schema_perm(
+ self.db_connection.name,
+ catalog,
+ schema,
+ )
+ add_pvm(security_manager, "schema_access", new_name)
+
+ def _rename_database_in_permissions(
+ self, catalog: str | None, schemas: Iterable[str]
+ ) -> None:
+ # rename existing catalog permission
+ if catalog:
+ new_catalog_perm_name = security_manager.get_catalog_perm(
+ self.db_connection.name,
+ catalog,
+ )
+ new_catalog_vm = add_vm(security_manager, new_catalog_perm_name)
+ perm = security_manager.get_catalog_perm(
+ self.old_db_connection_name,
+ catalog,
+ )
+ existing_pvm = security_manager.find_permission_view_menu(
+ "catalog_access",
+ perm,
+ )
+ if existing_pvm:
+ existing_pvm.view_menu = new_catalog_vm
+
+ for schema in schemas:
+ new_schema_perm_name = security_manager.get_schema_perm(
+ self.db_connection.name,
+ catalog,
+ schema,
+ )
+
+ # rename existing schema permission
+ perm = security_manager.get_schema_perm(
+ self.old_db_connection_name,
+ catalog,
+ schema,
+ )
+ existing_pvm = security_manager.find_permission_view_menu(
+ "schema_access",
+ perm,
+ )
+ if existing_pvm:
+ existing_pvm.view_menu.name = new_schema_perm_name
+
+ # rename permissions on datasets and charts
+ for dataset in DatabaseDAO.get_datasets(
+ self.db_connection_id,
+ catalog=catalog,
+ schema=schema,
+ ):
+ dataset.catalog_perm = new_catalog_perm_name
+ dataset.schema_perm = new_schema_perm_name
+ for chart in DatasetDAO.get_related_objects(dataset.id)["charts"]:
+ chart.catalog_perm = new_catalog_perm_name
+ chart.schema_perm = new_schema_perm_name
+
+
+def add_vm(
+ security_manager: SupersetSecurityManager,
+ view_menu_name: str | None,
+) -> ViewMenu:
+ """
+ Similar to security_manager.add_view_menu, but without commit.
+
+ This ensures an atomic operation.
+ """
+ if view_menu := security_manager.find_view_menu(view_menu_name):
+ return view_menu
+
+ view_menu = security_manager.viewmenu_model()
+ view_menu.name = view_menu_name
+ db.session.add(view_menu)
+ return view_menu
+
+
+def add_perm(
+ security_manager: SupersetSecurityManager,
+ permission_name: str | None,
+) -> Permission:
+ """
+ Similar to security_manager.add_permission, but without commit.
+
+ This ensures an atomic operation.
+ """
+ if perm := security_manager.find_permission(permission_name):
+ return perm
+
+ perm = security_manager.permission_model()
+ perm.name = permission_name
+ db.session.add(perm)
+ return perm
+
+
+def add_pvm(
+ security_manager: SupersetSecurityManager,
+ permission_name: str | None,
+ view_menu_name: str | None,
+) -> PermissionView | None:
+ """
+ Similar to security_manager.add_permission_view_menu, but without commit.
+
+ This ensures an atomic operation.
+ """
+ if not (permission_name and view_menu_name):
+ return None
+
+ if pv := security_manager.find_permission_view_menu(
+ permission_name, view_menu_name
+ ):
+ return pv
+
+ vm = add_vm(security_manager, view_menu_name)
+ perm = add_perm(security_manager, permission_name)
+ pv = security_manager.permissionview_model()
+ pv.view_menu, pv.permission = vm, perm
+ db.session.add(pv)
+
+ return pv
+
+
+@celery_app.task(name="sync_database_permissions", soft_time_limit=600)
+def sync_database_permissions_task(
+ database_id: int, username: str, old_db_connection_name: str
+) -> None:
+ """
+ Celery task that triggers the SyncPermissionsCommand in async mode.
+ """
+ with current_app.test_request_context():
+ try:
+ user = security_manager.get_user_by_username(username)
+ if not user:
+ raise UserNotFoundInSessionError()
+ g.user = user
+ logger.info(
+ "Syncing permissions for DB connection %s while impersonating user %s",
+ database_id,
+ user.id,
+ )
+
+ db_connection = DatabaseDAO.find_by_id(database_id)
+ if not db_connection:
+ raise DatabaseNotFoundError()
+ ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database_id)
+
+ SyncPermissionsCommand(
+ database_id,
+ username,
+ old_db_connection_name=old_db_connection_name,
+ db_connection=db_connection,
+ ssh_tunnel=ssh_tunnel,
+ ).sync_database_permissions()
+
+ logger.info(
+ "Successfully synced permissions for DB connection %s",
+ database_id,
+ )
+
+ except Exception:
+ logger.error(
+ "An error occurred while syncing permissions for DB connection ID %s",
+ database_id,
+ exc_info=True,
+ )
diff --git a/superset/commands/database/update.py b/superset/commands/database/update.py
index e6cfcfda52..a1accf4df1 100644
--- a/superset/commands/database/update.py
+++ b/superset/commands/database/update.py
@@ -94,7 +94,7 @@ class UpdateDatabaseCommand(BaseCommand):
old_db_connection_name=original_database_name,
db_connection=database,
ssh_tunnel=ssh_tunnel,
- ).run_without_transaction()
+ ).run()
except OAuth2RedirectError:
pass
diff --git a/superset/tasks/permissions.py b/superset/tasks/permissions.py
deleted file mode 100644
index 052fe69a3a..0000000000
--- a/superset/tasks/permissions.py
+++ /dev/null
@@ -1,259 +0,0 @@
-# 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.
-from __future__ import annotations
-
-import logging
-from typing import Iterable
-
-from flask import current_app, g
-
-from superset import security_manager
-from superset.commands.database.exceptions import (
- DatabaseConnectionFailedError,
- DatabaseNotFoundError,
- UserNotFoundInSessionError,
-)
-from superset.daos.database import DatabaseDAO
-from superset.daos.dataset import DatasetDAO
-from superset.databases.ssh_tunnel.models import SSHTunnel
-from superset.db_engine_specs.base import GenericDBException
-from superset.exceptions import OAuth2RedirectError
-from superset.extensions import celery_app
-from superset.models.core import Database
-
-logger = logging.getLogger(__name__)
-
-
-@celery_app.task(name="sync_database_permissions", soft_time_limit=600)
-def sync_database_permissions(
- database_id: int, username: str, old_db_connection_name: str
-) -> None:
- with current_app.test_request_context():
- try:
- user = security_manager.get_user_by_username(username)
- if not user:
- raise UserNotFoundInSessionError()
- g.user = user
- logger.info(
- "Syncing permissions for DB connection %s while impersonating user %s",
- database_id,
- user.id,
- )
-
- db_connection = DatabaseDAO.find_by_id(database_id)
- if not db_connection:
- raise DatabaseNotFoundError()
- ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database_id)
-
- sync_permissions(db_connection, ssh_tunnel, old_db_connection_name)
-
- except Exception:
- logger.error(
- "An error occurred while syncing permissions for DB connection ID %s",
- database_id,
- exc_info=True,
- )
- raise
-
-
-def sync_permissions(
- db_connection: Database,
- ssh_tunnel: SSHTunnel | None,
- old_db_connection_name: str,
-) -> None:
- """
- Syncs the permissions for a DB connection.
- """
- catalogs = (
- _get_catalog_names(db_connection, ssh_tunnel)
- if db_connection.db_engine_spec.supports_catalog
- else [None]
- )
- new_db_connection_name = db_connection.database_name
-
- for catalog in catalogs:
- try:
- schemas = _get_schema_names(db_connection, ssh_tunnel, catalog)
-
- if catalog:
- perm = security_manager.get_catalog_perm(
- old_db_connection_name,
- catalog,
- )
- existing_pvm = security_manager.find_permission_view_menu(
- "catalog_access",
- perm,
- )
- if not existing_pvm:
- # new catalog
- security_manager.add_permission_view_menu(
- "catalog_access",
- security_manager.get_catalog_perm(
- new_db_connection_name,
- catalog,
- ),
- )
- for schema in schemas:
- security_manager.add_permission_view_menu(
- "schema_access",
- security_manager.get_schema_perm(
- new_db_connection_name,
- catalog,
- schema,
- ),
- )
- continue
- except DatabaseConnectionFailedError:
- logger.warning("Error processing catalog %s", catalog or "(default)")
- continue
-
- # add possible new schemas in catalog
- _refresh_schemas(
- old_db_connection_name, new_db_connection_name, catalog, schemas
- )
-
- if old_db_connection_name != new_db_connection_name:
- _rename_database_in_permissions(
- db_connection.id,
- old_db_connection_name,
- new_db_connection_name,
- catalog,
- schemas,
- )
-
-
-def _get_catalog_names(
- db_connection: Database, ssh_tunnel: SSHTunnel | None
-) -> set[str]:
- """
- Helper method to load catalogs.
- """
- try:
- return db_connection.get_all_catalog_names(
- force=True,
- ssh_tunnel=ssh_tunnel,
- )
- except OAuth2RedirectError:
- # raise OAuth2 exceptions as-is
- raise
- except GenericDBException as ex:
- raise DatabaseConnectionFailedError() from ex
-
-
-def _get_schema_names(
- db_connection: Database, ssh_tunnel: SSHTunnel | None, catalog: str | None
-) -> set[str]:
- """
- Helper method to load schemas.
- """
- try:
- return db_connection.get_all_schema_names(
- force=True,
- catalog=catalog,
- ssh_tunnel=ssh_tunnel,
- )
- except OAuth2RedirectError:
- # raise OAuth2 exceptions as-is
- raise
- except GenericDBException as ex:
- raise DatabaseConnectionFailedError() from ex
-
-
-def _refresh_schemas(
- old_db_connection_name: str,
- new_db_connection_name: str,
- catalog: str | None,
- schemas: Iterable[str],
-) -> None:
- """
- Add new schemas that don't have permissions yet.
- """
- for schema in schemas:
- perm = security_manager.get_schema_perm(
- old_db_connection_name,
- catalog,
- schema,
- )
- existing_pvm = security_manager.find_permission_view_menu(
- "schema_access",
- perm,
- )
- if not existing_pvm:
- new_name = security_manager.get_schema_perm(
- new_db_connection_name,
- catalog,
- schema,
- )
- security_manager.add_permission_view_menu("schema_access", new_name)
-
-
-def _rename_database_in_permissions(
- db_connection_id: int,
- old_db_connection_name: str,
- new_db_connection_name: str,
- catalog: str | None,
- schemas: Iterable[str],
-) -> None:
- new_catalog_perm_name = security_manager.get_catalog_perm(
- new_db_connection_name,
- catalog,
- )
-
- # rename existing catalog permission
- if catalog:
- perm = security_manager.get_catalog_perm(
- old_db_connection_name,
- catalog,
- )
- existing_pvm = security_manager.find_permission_view_menu(
- "catalog_access",
- perm,
- )
- if existing_pvm:
- existing_pvm.view_menu.name = new_catalog_perm_name
-
- for schema in schemas:
- new_schema_perm_name = security_manager.get_schema_perm(
- new_db_connection_name,
- catalog,
- schema,
- )
-
- # rename existing schema permission
- perm = security_manager.get_schema_perm(
- old_db_connection_name,
- catalog,
- schema,
- )
- existing_pvm = security_manager.find_permission_view_menu(
- "schema_access",
- perm,
- )
- if existing_pvm:
- existing_pvm.view_menu.name = new_schema_perm_name
-
- # rename permissions on datasets and charts
- for dataset in DatabaseDAO.get_datasets(
- db_connection_id,
- catalog=catalog,
- schema=schema,
- ):
- dataset.catalog_perm = new_catalog_perm_name
- dataset.schema_perm = new_schema_perm_name
- for chart in DatasetDAO.get_related_objects(dataset.id)["charts"]:
- chart.catalog_perm = new_catalog_perm_name
- chart.schema_perm = new_schema_perm_name
diff --git a/tests/unit_tests/commands/databases/conftest.py b/tests/unit_tests/commands/databases/conftest.py
new file mode 100644
index 0000000000..49da52daf9
--- /dev/null
+++ b/tests/unit_tests/commands/databases/conftest.py
@@ -0,0 +1,90 @@
+# 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.
+
+from unittest.mock import MagicMock
+
+import pytest
+from pytest_mock import MockerFixture
+
+from superset.db_engine_specs.base import BaseEngineSpec
+from superset.exceptions import OAuth2RedirectError
+from superset.utils import json
+
+oauth2_client_info = {
+ "id": "client_id",
+ "secret": "client_secret",
+ "scope": "scope-a",
+ "redirect_uri": "redirect_uri",
+ "authorization_request_uri": "auth_uri",
+ "token_request_uri": "token_uri",
+ "request_content_type": "json",
+}
+
+
+@pytest.fixture
+def database_with_catalog(mocker: MockerFixture) -> MagicMock:
+ """
+ Mock a database with catalogs and schemas.
+ """
+ database = mocker.MagicMock()
+ database.database_name = "my_db"
+ database.db_engine_spec.__name__ = "test_engine"
+ database.db_engine_spec.supports_catalog = True
+ database.get_all_catalog_names.return_value = ["catalog1", "catalog2"]
+ database.get_all_schema_names.side_effect = [
+ ["schema1", "schema2"],
+ ["schema3", "schema4"],
+ ]
+ database.get_default_catalog.return_value = "catalog2"
+
+ return database
+
+
+@pytest.fixture
+def database_without_catalog(mocker: MockerFixture) -> MagicMock:
+ """
+ Mock a database without catalogs.
+ """
+ database = mocker.MagicMock()
+ database.database_name = "my_db"
+ database.db_engine_spec.__name__ = "test_engine"
+ database.db_engine_spec.supports_catalog = False
+ database.get_all_schema_names.return_value = ["schema1", "schema2"]
+
+ return database
+
+
+@pytest.fixture
+def database_needs_oauth2(mocker: MockerFixture) -> MagicMock:
+ """
+ Mock a database without catalogs that needs OAuth2.
+ """
+ database = mocker.MagicMock()
+ database.database_name = "my_db"
+ database.db_engine_spec.__name__ = "test_engine"
+ database.db_engine_spec.supports_catalog = False
+ database.get_all_schema_names.side_effect = OAuth2RedirectError(
+ "url",
+ "tab_id",
+ "redirect_uri",
+ )
+ database.encrypted_extra = json.dumps({"oauth2_client_info": oauth2_client_info})
+ database.db_engine_spec.unmask_encrypted_extra = (
+ BaseEngineSpec.unmask_encrypted_extra
+ )
+
+ return database
diff --git a/tests/unit_tests/commands/databases/sync_permissions_test.py b/tests/unit_tests/commands/databases/sync_permissions_test.py
index 2e639c4cbc..b23c3a7c25 100644
--- a/tests/unit_tests/commands/databases/sync_permissions_test.py
+++ b/tests/unit_tests/commands/databases/sync_permissions_test.py
@@ -16,6 +16,8 @@
# under the License.
from __future__ import annotations
+from unittest.mock import MagicMock
+
import pytest
from pytest_mock import MockerFixture
@@ -25,82 +27,89 @@ from superset.commands.database.exceptions import (
UserNotFoundInSessionError,
)
from superset.commands.database.sync_permissions import SyncPermissionsCommand
+from superset.db_engine_specs.base import GenericDBException
+from superset.exceptions import OAuth2RedirectError
+from superset.extensions import security_manager
from tests.conftest import with_config
@with_config({"SYNC_DB_PERMISSIONS_IN_ASYNC_MODE": False})
-def test_sync_permissions_command_sync_mode(mocker: MockerFixture):
+def test_sync_permissions_command_sync_mode(
+ mocker: MockerFixture,
+ database_with_catalog: MagicMock,
+):
"""
Test ``SyncPermissionsCommand`` in sync mode.
"""
- mock_db = mocker.MagicMock()
- mock_db.database_name = "current name"
mock_ssh = mocker.MagicMock()
- mock_database_dao = mocker.patch(
- "superset.commands.database.sync_permissions.DatabaseDAO"
- )
- mock_database_dao.find_by_id.return_value = mock_db
- mock_database_dao.get_ssh_tunnel.return_value = mock_ssh
user_mock = mocker.patch(
"superset.commands.database.sync_permissions.security_manager.get_user_by_username"
)
- sync_mock = mocker.patch(
- "superset.commands.database.sync_permissions.sync_database_permissions"
- )
mocker.patch("superset.commands.database.sync_permissions.ping", return_value=True)
+ find_pvm_mock = mocker.patch(
+ "superset.commands.database.sync_permissions.security_manager.find_permission_view_menu"
+ )
+ find_pvm_mock.side_effect = [mocker.MagicMock(), None]
+ add_pvm_mock = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
+
+ cmmd = SyncPermissionsCommand(
+ 1, "admin", db_connection=database_with_catalog, ssh_tunnel=mock_ssh
+ )
+ mock_refresh_schemas = mocker.patch.object(cmmd, "_refresh_schemas")
+ mock_rename_db_perm = mocker.patch.object(cmmd, "_rename_database_in_permissions")
- cmmd = SyncPermissionsCommand(1, "admin")
cmmd.run()
- assert cmmd.db_connection == mock_db
- assert cmmd.old_db_connection_name == "current name"
+ assert cmmd.db_connection == database_with_catalog
+ assert cmmd.old_db_connection_name == "my_db"
assert cmmd.db_connection_ssh_tunnel == mock_ssh
- mock_database_dao.find_by_id.assert_called_once_with(1)
- mock_database_dao.get_ssh_tunnel.assert_called_once_with(1)
user_mock.assert_called_once_with("admin")
- sync_mock.assert_called_once_with(1, "admin", "current name")
+ add_pvm_mock.assert_has_calls(
+ [
+ mocker.call(security_manager, "catalog_access", "[my_db].[catalog2]"),
+ mocker.call(
+ security_manager, "schema_access", "[my_db].[catalog2].[schema3]"
+ ),
+ mocker.call(
+ security_manager, "schema_access", "[my_db].[catalog2].[schema4]"
+ ),
+ ]
+ )
+ mock_refresh_schemas.assert_called_once_with("catalog1", ["schema1", "schema2"])
+ mock_rename_db_perm.assert_not_called()
@with_config({"SYNC_DB_PERMISSIONS_IN_ASYNC_MODE": True})
-def test_sync_permissions_command_async_mode(mocker: MockerFixture) -> None:
+def test_sync_permissions_command_async_mode(
+ mocker: MockerFixture, database_with_catalog: MagicMock
+) -> None:
"""
Test ``SyncPermissionsCommand`` in async mode.
"""
- mock_db = mocker.MagicMock()
- mock_db.database_name = "current name"
- mock_ssh = mocker.MagicMock()
mock_database_dao = mocker.patch(
"superset.commands.database.sync_permissions.DatabaseDAO"
)
- mock_database_dao.find_by_id.return_value = mock_db
- mock_database_dao.get_ssh_tunnel.return_value = mock_ssh
- user_mock = mocker.patch(
+ mock_database_dao.find_by_id.return_value = database_with_catalog
+ mocker.patch(
"superset.commands.database.sync_permissions.security_manager.get_user_by_username"
)
- sync_mock = mocker.patch(
- "superset.commands.database.sync_permissions.sync_database_permissions"
+ async_task_mock = mocker.patch(
+ "superset.commands.database.sync_permissions.sync_database_permissions_task"
)
mocker.patch("superset.commands.database.sync_permissions.ping", return_value=True)
cmmd = SyncPermissionsCommand(1, "admin")
cmmd.run()
-
- assert cmmd.db_connection == mock_db
- assert cmmd.old_db_connection_name == "current name"
- assert cmmd.db_connection_ssh_tunnel == mock_ssh
- mock_database_dao.find_by_id.assert_called_once_with(1)
- mock_database_dao.get_ssh_tunnel.assert_called_once_with(1)
- user_mock.assert_called_once_with("admin")
- sync_mock.delay.assert_called_once_with(1, "admin", "current name")
+ async_task_mock.delay.assert_called_once_with(1, "admin", "my_db")
@with_config({"SYNC_DB_PERMISSIONS_IN_ASYNC_MODE": False})
-def test_sync_permissions_command_passing_all_values(mocker: MockerFixture):
+def test_sync_permissions_command_passing_all_values(
+ mocker: MockerFixture, database_with_catalog: MagicMock
+):
"""
Test ``SyncPermissionsCommand`` when providing all arguments to the constructor.
"""
- mock_db = mocker.MagicMock()
- mock_db.database_name = "current name"
mock_ssh = mocker.MagicMock()
mock_database_dao = mocker.patch(
"superset.commands.database.sync_permissions.DatabaseDAO"
@@ -108,21 +117,19 @@ def test_sync_permissions_command_passing_all_values(mocker: MockerFixture):
mocker.patch(
"superset.commands.database.sync_permissions.security_manager.get_user_by_username"
)
- mocker.patch(
- "superset.commands.database.sync_permissions.sync_database_permissions"
- )
mocker.patch("superset.commands.database.sync_permissions.ping", return_value=True)
cmmd = SyncPermissionsCommand(
1,
"admin",
old_db_connection_name="old name",
- db_connection=mock_db,
+ db_connection=database_with_catalog,
ssh_tunnel=mock_ssh,
)
+ mocker.patch.object(cmmd, "sync_database_permissions")
cmmd.run()
- assert cmmd.db_connection == mock_db
+ assert cmmd.db_connection == database_with_catalog
assert cmmd.old_db_connection_name == "old name"
assert cmmd.db_connection_ssh_tunnel == mock_ssh
mock_database_dao.find_by_id.assert_not_called()
@@ -142,9 +149,6 @@ def test_sync_permissions_command_raise(mocker: MockerFixture):
mock_user = mocker.patch(
"superset.commands.database.sync_permissions.security_manager.get_user_by_username"
)
- mocker.patch(
- "superset.commands.database.sync_permissions.sync_database_permissions"
- )
# Connection issues
mock_ping = mocker.patch(
@@ -173,28 +177,203 @@ def test_sync_permissions_command_raise(mocker: MockerFixture):
@with_config({"SYNC_DB_PERMISSIONS_IN_ASYNC_MODE": False})
-def test_sync_permissions_command_new_db_name(mocker: MockerFixture):
+def test_sync_permissions_command_new_db_name(
+ mocker: MockerFixture, database_with_catalog: MagicMock
+):
"""
Test ``SyncPermissionsCommand`` when the database name changed.
"""
- mock_db = mocker.MagicMock()
- mock_db.database_name = "New Name"
-
mocker.patch(
"superset.commands.database.sync_permissions.security_manager.get_user_by_username"
)
+ cmmd = SyncPermissionsCommand(
+ 1,
+ "admin",
+ old_db_connection_name="Old Name",
+ db_connection=database_with_catalog,
+ )
+ cmmd.run()
+
+ assert cmmd.old_db_connection_name == "Old Name"
+
+
+@with_config({"SYNC_DB_PERMISSIONS_IN_ASYNC_MODE": True})
+def test_sync_permissions_command_async_mode_new_db_name(
+ mocker: MockerFixture, database_with_catalog: MagicMock
+):
+ """
+ Test ``SyncPermissionsCommand`` in async mode when the
+ database name changed.
+ """
mocker.patch(
- "superset.commands.database.sync_permissions.sync_database_permissions"
+ "superset.commands.database.sync_permissions.security_manager.get_user_by_username"
)
- sync_mock = mocker.patch(
- "superset.commands.database.sync_permissions.sync_database_permissions"
+ async_task_mock = mocker.patch(
+ "superset.commands.database.sync_permissions.sync_database_permissions_task"
)
-
- SyncPermissionsCommand(
+ cmmd = SyncPermissionsCommand(
1,
"admin",
old_db_connection_name="Old Name",
- db_connection=mock_db,
- ).run()
+ db_connection=database_with_catalog,
+ )
+ cmmd.run()
+
+ async_task_mock.delay.assert_called_once_with(1, "admin", "Old Name")
+
+
+def test_resync_permissions_command_get_catalogs(database_with_catalog: MagicMock):
+ """
+ Test the ``_get_catalog_names`` method.
+ """
+ cmmd = SyncPermissionsCommand(1, None, db_connection=database_with_catalog)
+ assert cmmd._get_catalog_names() == ["catalog1", "catalog2"]
+
+
+@pytest.mark.parametrize(
+ ("inner_exception, outer_exception"),
+ [
+ (
+ OAuth2RedirectError("Missing token", "mock_tab", "mock_url"),
+ OAuth2RedirectError,
+ ),
+ (GenericDBException, DatabaseConnectionFailedError),
+ ],
+)
+def test_resync_permissions_command_raise_on_getting_catalogs(
+ inner_exception: Exception,
+ outer_exception: Exception,
+ database_with_catalog: MagicMock,
+):
+ """
+ Test the ``_get_catalog_names`` method when raising an exception.
+ """
+ database_with_catalog.get_all_catalog_names.side_effect = inner_exception
+ cmmd = SyncPermissionsCommand(1, None, db_connection=database_with_catalog)
+ with pytest.raises(outer_exception):
+ cmmd._get_catalog_names()
+
+
+def test_resync_permissions_command_get_schemas(database_with_catalog: MagicMock):
+ """
+ Test the ``_get_schema_names`` method.
+ """
+ cmmd = SyncPermissionsCommand(1, None, db_connection=database_with_catalog)
+ assert cmmd._get_schema_names("catalog1") == ["schema1", "schema2"]
+ assert cmmd._get_schema_names("catalog2") == ["schema3", "schema4"]
+
- sync_mock.assert_called_once_with(1, "admin", "Old Name")
+@pytest.mark.parametrize(
+ ("inner_exception, outer_exception"),
+ [
+ (
+ OAuth2RedirectError("Missing token", "mock_tab", "mock_url"),
+ OAuth2RedirectError,
+ ),
+ (GenericDBException, DatabaseConnectionFailedError),
+ ],
+)
+def test_resync_permissions_command_raise_on_getting_schemas(
+ inner_exception: Exception,
+ outer_exception: Exception,
+ database_with_catalog: MagicMock,
+):
+ """
+ Test the ``_get_schema_names`` method when raising an exception.
+ """
+ database_with_catalog.get_all_schema_names.side_effect = inner_exception
+ cmmd = SyncPermissionsCommand(1, None, db_connection=database_with_catalog)
+ with pytest.raises(outer_exception):
+ cmmd._get_schema_names("blah")
+
+
+def test_resync_permissions_command_refresh_schemas(
+ mocker: MockerFixture, database_with_catalog: MagicMock
+):
+ """
+ Test the ``_refresh_schemas`` method.
+ """
+ find_pvm_mock = mocker.patch(
+ "superset.commands.database.sync_permissions.security_manager.find_permission_view_menu"
+ )
+ find_pvm_mock.side_effect = [mocker.MagicMock(), None]
+ add_pvm_mock = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
+
+ cmmd = SyncPermissionsCommand(1, None, db_connection=database_with_catalog)
+ cmmd._refresh_schemas("catalog1", ["schema1", "schema2"])
+
+ add_pvm_mock.assert_called_once_with(
+ security_manager,
+ "schema_access",
+ f"[{database_with_catalog.name}].[catalog1].[schema2]",
+ )
+
+
+def test_resync_permissions_command_rename_db_in_perms(
+ mocker: MockerFixture, database_with_catalog: MagicMock
+):
+ """
+ Test the ``_rename_database_in_permissions`` method.
+ """
+ find_pvm_mock = mocker.patch(
+ "superset.commands.database.sync_permissions.security_manager.find_permission_view_menu"
+ )
+ mock_catalog_perm = mocker.MagicMock()
+ mock_catalog_perm.view_menu.name = "[old_name].[catalog]"
+ mock_schema_perm = mocker.MagicMock()
+ mock_schema_perm.view_menu.name = "[old_name].[catalog].[schema1]"
+ find_pvm_mock.side_effect = [
+ mock_catalog_perm,
+ mock_schema_perm,
+ None,
+ ]
+
+ mock_dataset = mocker.MagicMock()
+ mock_dataset.id = 1
+ mock_dataset.catalog_perm = "[old_name].[catalog1]"
+ mock_dataset.schema_perm = "[old_name].[catalog1].[schema1]"
+ mock_chart = mocker.MagicMock()
+ mock_chart.catalog_perm = "[old_name].[catalog1]"
+ mock_chart.schema_perm = "[old_name].[catalog1].[schema1]"
+
+ mock_database_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
+ mock_database_dao.get_datasets.side_effect = [
+ [mock_dataset],
+ [],
+ ]
+ mock_dataset_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatasetDAO"
+ )
+ mock_dataset_dao.get_related_objects.return_value = {"charts": [mock_chart]}
+
+ cmmd = SyncPermissionsCommand(
+ 1, None, old_db_connection_name="old_name", db_connection=database_with_catalog
+ )
+ cmmd._rename_database_in_permissions("catalog1", ["schema1", "schema2"])
+
+ find_pvm_mock.assert_has_calls(
+ [
+ mocker.call("catalog_access", "[old_name].[catalog1]"),
+ mocker.call("schema_access", "[old_name].[catalog1].[schema1]"),
+ mocker.call("schema_access", "[old_name].[catalog1].[schema2]"),
+ ]
+ )
+
+ assert (
+ mock_catalog_perm.view_menu.name == f"[{database_with_catalog.name}].[catalog1]"
+ )
+ assert (
+ mock_schema_perm.view_menu.name
+ == f"[{database_with_catalog.name}].[catalog1].[schema1]"
+ )
+ assert mock_dataset.catalog_perm == f"[{database_with_catalog.name}].[catalog1]"
+ assert (
+ mock_dataset.schema_perm
+ == f"[{database_with_catalog.name}].[catalog1].[schema1]"
+ )
+ assert mock_chart.catalog_perm == f"[{database_with_catalog.name}].[catalog1]"
+ assert (
+ mock_chart.schema_perm == f"[{database_with_catalog.name}].[catalog1].[schema1]"
+ )
diff --git a/tests/unit_tests/commands/databases/update_test.py b/tests/unit_tests/commands/databases/update_test.py
index dcd4d5d4db..da8baa6a39 100644
--- a/tests/unit_tests/commands/databases/update_test.py
+++ b/tests/unit_tests/commands/databases/update_test.py
@@ -17,84 +17,18 @@
from unittest.mock import MagicMock
-import pytest
from pytest_mock import MockerFixture
from superset.commands.database.update import UpdateDatabaseCommand
-from superset.db_engine_specs.base import BaseEngineSpec
-from superset.exceptions import OAuth2RedirectError
from superset.extensions import security_manager
from superset.utils import json
-
-oauth2_client_info = {
- "id": "client_id",
- "secret": "client_secret",
- "scope": "scope-a",
- "redirect_uri": "redirect_uri",
- "authorization_request_uri": "auth_uri",
- "token_request_uri": "token_uri",
- "request_content_type": "json",
-}
-
-
-@pytest.fixture
-def database_with_catalog(mocker: MockerFixture) -> MagicMock:
- """
- Mock a database with catalogs and schemas.
- """
- database = mocker.MagicMock()
- database.database_name = "my_db"
- database.db_engine_spec.__name__ = "test_engine"
- database.db_engine_spec.supports_catalog = True
- database.get_all_catalog_names.return_value = ["catalog1", "catalog2"]
- database.get_all_schema_names.side_effect = [
- ["schema1", "schema2"],
- ["schema3", "schema4"],
- ]
- database.get_default_catalog.return_value = "catalog2"
-
- return database
-
-
-@pytest.fixture
-def database_without_catalog(mocker: MockerFixture) -> MagicMock:
- """
- Mock a database without catalogs.
- """
- database = mocker.MagicMock()
- database.database_name = "my_db"
- database.db_engine_spec.__name__ = "test_engine"
- database.db_engine_spec.supports_catalog = False
- database.get_all_schema_names.return_value = ["schema1", "schema2"]
-
- return database
-
-
-@pytest.fixture
-def database_needs_oauth2(mocker: MockerFixture) -> MagicMock:
- """
- Mock a database without catalogs that needs OAuth2.
- """
- database = mocker.MagicMock()
- database.database_name = "my_db"
- database.db_engine_spec.__name__ = "test_engine"
- database.db_engine_spec.supports_catalog = False
- database.get_all_schema_names.side_effect = OAuth2RedirectError(
- "url",
- "tab_id",
- "redirect_uri",
- )
- database.encrypted_extra = json.dumps({"oauth2_client_info": oauth2_client_info})
- database.db_engine_spec.unmask_encrypted_extra = (
- BaseEngineSpec.unmask_encrypted_extra
- )
-
- return database
+from tests.conftest import with_config
+from tests.unit_tests.commands.databases.conftest import oauth2_client_info
def test_update_with_catalog(
mocker: MockerFixture,
- database_with_catalog: MockerFixture,
+ database_with_catalog: MagicMock,
) -> None:
"""
Test that permissions are updated correctly.
@@ -114,7 +48,9 @@ def test_update_with_catalog(
database_dao = mocker.patch("superset.commands.database.update.DatabaseDAO")
database_dao.find_by_id.return_value = database_with_catalog
database_dao.update.return_value = database_with_catalog
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_with_catalog
mocker.patch("superset.commands.database.update.get_username")
mocker.patch("superset.security_manager.get_user_by_username")
@@ -132,25 +68,57 @@ def test_update_with_catalog(
None,
None,
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
UpdateDatabaseCommand(1, {}).run()
- add_permission_view_menu.assert_has_calls(
+ add_pvm.assert_has_calls(
[
# first catalog is added with all schemas
- mocker.call("catalog_access", "[my_db].[catalog1]"),
- mocker.call("schema_access", "[my_db].[catalog1].[schema1]"),
- mocker.call("schema_access", "[my_db].[catalog1].[schema2]"),
+ mocker.call(security_manager, "catalog_access", "[my_db].[catalog1]"),
+ mocker.call(
+ security_manager, "schema_access", "[my_db].[catalog1].[schema1]"
+ ),
+ mocker.call(
+ security_manager, "schema_access", "[my_db].[catalog1].[schema2]"
+ ),
# second catalog already exists, only `schema4` is added
- mocker.call("schema_access", "[my_db].[catalog2].[schema4]"),
+ mocker.call(
+ security_manager,
+ "schema_access",
+ f"[{database_with_catalog.name}].[catalog2].[schema4]",
+ ),
],
)
+@with_config({"SYNC_DB_PERMISSIONS_IN_ASYNC_MODE": True})
+def test_update_sync_perms_in_async_mode(
+ mocker: MockerFixture,
+ database_with_catalog: MagicMock,
+) -> None:
+ """
+ Test that updating a DB connection with async mode enables
+ triggers the celery task to syn perms.
+ """
+ database_dao = mocker.patch("superset.commands.database.update.DatabaseDAO")
+ database_dao.find_by_id.return_value = database_with_catalog
+ database_dao.update.return_value = database_with_catalog
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
+ sync_db_perms_dao.find_by_id.return_value = database_with_catalog
+ sync_task = mocker.patch(
+ "superset.commands.database.sync_permissions.sync_database_permissions_task.delay"
+ )
+ mocker.patch("superset.commands.database.update.get_username", return_value="admin")
+ mocker.patch("superset.security_manager.get_user_by_username")
+
+ UpdateDatabaseCommand(1, {}).run()
+
+ sync_task.assert_called_once_with(1, "admin", "my_db")
+
+
def test_update_without_catalog(
mocker: MockerFixture,
database_without_catalog: MockerFixture,
@@ -169,7 +137,9 @@ def test_update_without_catalog(
database_dao = mocker.patch("superset.commands.database.update.DatabaseDAO")
database_dao.find_by_id.return_value = database_without_catalog
database_dao.update.return_value = database_without_catalog
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_without_catalog
mocker.patch("superset.commands.database.update.get_username")
mocker.patch("superset.security_manager.get_user_by_username")
@@ -182,22 +152,20 @@ def test_update_without_catalog(
None, # schema1 has no permissions
"[my_db].[schema2]", # second schema already exists
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
UpdateDatabaseCommand(1, {}).run()
- add_permission_view_menu.assert_called_with(
+ add_pvm.assert_called_with(
+ security_manager,
"schema_access",
- "[my_db].[schema1]",
+ f"[{database_without_catalog.name}].[schema1]",
)
def test_rename_with_catalog(
mocker: MockerFixture,
- database_with_catalog: MockerFixture,
+ database_with_catalog: MagicMock,
) -> None:
"""
Test that permissions are renamed correctly.
@@ -221,7 +189,9 @@ def test_rename_with_catalog(
database_dao.find_by_id.return_value = original_database
database_with_catalog.database_name = "my_other_db"
database_dao.update.return_value = database_with_catalog
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_with_catalog
mocker.patch("superset.commands.database.update.get_username")
mocker.patch("superset.security_manager.get_user_by_username")
@@ -229,7 +199,7 @@ def test_rename_with_catalog(
dataset = mocker.MagicMock()
chart = mocker.MagicMock()
sync_db_perms_dao.get_datasets.return_value = [dataset]
- dataset_dao = mocker.patch("superset.tasks.permissions.DatasetDAO")
+ dataset_dao = mocker.patch("superset.commands.database.sync_permissions.DatasetDAO")
dataset_dao.get_related_objects.return_value = {"charts": [chart]}
find_permission_view_menu = mocker.patch.object(
@@ -237,7 +207,9 @@ def test_rename_with_catalog(
"find_permission_view_menu",
)
catalog2_pvm = mocker.MagicMock()
+ catalog2_pvm.view_menu.name = "[my_db].[catalog2]"
catalog2_schema3_pvm = mocker.MagicMock()
+ catalog2_schema3_pvm.view_menu.name = "[my_db].[catalog2].[schema3]"
find_permission_view_menu.side_effect = [
# these are called when adding the permissions:
None, # first catalog is new
@@ -249,31 +221,40 @@ def test_rename_with_catalog(
catalog2_schema3_pvm, # old [my_db].[catalog2].[schema3]
None, # [my_db].[catalog2].[schema4] doesn't exist
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
+ add_vm = mocker.patch("superset.commands.database.sync_permissions.add_vm")
UpdateDatabaseCommand(1, {}).run()
- add_permission_view_menu.assert_has_calls(
+ add_pvm.assert_has_calls(
[
# first catalog is added with all schemas with the new DB name
- mocker.call("catalog_access", "[my_other_db].[catalog1]"),
- mocker.call("schema_access", "[my_other_db].[catalog1].[schema1]"),
- mocker.call("schema_access", "[my_other_db].[catalog1].[schema2]"),
+ mocker.call(security_manager, "catalog_access", "[my_other_db].[catalog1]"),
+ mocker.call(
+ security_manager, "schema_access", "[my_other_db].[catalog1].[schema1]"
+ ),
+ mocker.call(
+ security_manager, "schema_access", "[my_other_db].[catalog1].[schema2]"
+ ),
# second catalog already exists, only `schema4` is added
- mocker.call("schema_access", "[my_other_db].[catalog2].[schema4]"),
+ mocker.call(
+ security_manager,
+ "schema_access",
+ f"[{database_with_catalog.name}].[catalog2].[schema4]",
+ ),
],
)
- assert catalog2_pvm.view_menu.name == "[my_other_db].[catalog2]"
- assert catalog2_schema3_pvm.view_menu.name == "[my_other_db].[catalog2].[schema3]"
+ assert catalog2_pvm.view_menu == add_vm.return_value
+ assert (
+ catalog2_schema3_pvm.view_menu.name
+ == f"[{database_with_catalog.name}].[catalog2].[schema3]"
+ )
- assert dataset.catalog_perm == "[my_other_db].[catalog2]"
- assert dataset.schema_perm == "[my_other_db].[catalog2].[schema4]"
- assert chart.catalog_perm == "[my_other_db].[catalog2]"
- assert chart.schema_perm == "[my_other_db].[catalog2].[schema4]"
+ assert dataset.catalog_perm == f"[{database_with_catalog.name}].[catalog2]"
+ assert dataset.schema_perm == f"[{database_with_catalog.name}].[catalog2].[schema4]"
+ assert chart.catalog_perm == f"[{database_with_catalog.name}].[catalog2]"
+ assert chart.schema_perm == f"[{database_with_catalog.name}].[catalog2].[schema4]"
def test_rename_without_catalog(
@@ -297,7 +278,9 @@ def test_rename_without_catalog(
database_without_catalog.database_name = "my_other_db"
database_dao.update.return_value = database_without_catalog
database_dao.find_by_id.return_value = original_database
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_without_catalog
sync_db_perms_dao.get_datasets.return_value = []
mocker.patch("superset.commands.database.update.get_username")
@@ -308,25 +291,24 @@ def test_rename_without_catalog(
"find_permission_view_menu",
)
schema2_pvm = mocker.MagicMock()
+ schema2_pvm.view_menu.name = "[my_db].[schema2]"
find_permission_view_menu.side_effect = [
None, # schema1 has no permissions
"[my_db].[schema2]", # second schema already exists
None, # [my_db].[schema1] doesn't exist
schema2_pvm, # old [my_db].[schema2]
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
UpdateDatabaseCommand(1, {}).run()
- add_permission_view_menu.assert_called_with(
+ add_pvm.assert_called_with(
+ security_manager,
"schema_access",
- "[my_other_db].[schema1]",
+ f"[{database_without_catalog.name}].[schema1]",
)
- assert schema2_pvm.view_menu.name == "[my_other_db].[schema2]"
+ assert schema2_pvm.view_menu.name == f"[{database_without_catalog.name}].[schema2]"
def test_update_with_oauth2(
@@ -339,7 +321,9 @@ def test_update_with_oauth2(
database_dao = mocker.patch("superset.commands.database.update.DatabaseDAO")
database_dao.find_by_id.return_value = database_needs_oauth2
database_dao.update.return_value = database_needs_oauth2
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_needs_oauth2
mocker.patch("superset.commands.database.update.get_username")
mocker.patch("superset.security_manager.get_user_by_username")
@@ -352,14 +336,11 @@ def test_update_with_oauth2(
None, # schema1 has no permissions
"[my_db].[schema2]", # second schema already exists
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
UpdateDatabaseCommand(1, {}).run()
- add_permission_view_menu.assert_not_called()
+ add_pvm.assert_not_called()
database_needs_oauth2.purge_oauth2_tokens.assert_not_called()
@@ -373,7 +354,9 @@ def test_update_with_oauth2_changed(
database_dao = mocker.patch("superset.commands.database.update.DatabaseDAO")
database_dao.find_by_id.return_value = database_needs_oauth2
database_dao.update.return_value = database_needs_oauth2
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_needs_oauth2
mocker.patch("superset.commands.database.update.get_username")
mocker.patch("superset.security_manager.get_user_by_username")
@@ -386,10 +369,7 @@ def test_update_with_oauth2_changed(
None, # schema1 has no permissions
"[my_db].[schema2]", # second schema already exists
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
modified_oauth2_client_info = oauth2_client_info.copy()
modified_oauth2_client_info["scope"] = "scope-b"
@@ -403,7 +383,7 @@ def test_update_with_oauth2_changed(
},
).run()
- add_permission_view_menu.assert_not_called()
+ add_pvm.assert_not_called()
database_needs_oauth2.purge_oauth2_tokens.assert_called()
@@ -417,7 +397,9 @@ def test_remove_oauth_config_purges_tokens(
database_dao = mocker.patch("superset.commands.database.update.DatabaseDAO")
database_dao.find_by_id.return_value = database_needs_oauth2
database_dao.update.return_value = database_needs_oauth2
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_needs_oauth2
mocker.patch("superset.commands.database.update.get_username")
mocker.patch("superset.security_manager.get_user_by_username")
@@ -430,19 +412,16 @@ def test_remove_oauth_config_purges_tokens(
None,
"[my_db].[schema2]",
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
UpdateDatabaseCommand(1, {"masked_encrypted_extra": None}).run()
- add_permission_view_menu.assert_not_called()
+ add_pvm.assert_not_called()
database_needs_oauth2.purge_oauth2_tokens.assert_called()
UpdateDatabaseCommand(1, {"masked_encrypted_extra": "{}"}).run()
- add_permission_view_menu.assert_not_called()
+ add_pvm.assert_not_called()
database_needs_oauth2.purge_oauth2_tokens.assert_called()
@@ -456,7 +435,9 @@ def test_update_oauth2_removes_masked_encrypted_extra_key(
database_dao = mocker.patch("superset.commands.database.update.DatabaseDAO")
database_dao.find_by_id.return_value = database_needs_oauth2
database_dao.update.return_value = database_needs_oauth2
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_needs_oauth2
mocker.patch("superset.commands.database.update.get_username")
mocker.patch("superset.security_manager.get_user_by_username")
@@ -469,10 +450,7 @@ def test_update_oauth2_removes_masked_encrypted_extra_key(
None,
"[my_db].[schema2]",
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
modified_oauth2_client_info = oauth2_client_info.copy()
modified_oauth2_client_info["scope"] = "scope-b"
@@ -486,7 +464,7 @@ def test_update_oauth2_removes_masked_encrypted_extra_key(
},
).run()
- add_permission_view_menu.assert_not_called()
+ add_pvm.assert_not_called()
database_needs_oauth2.purge_oauth2_tokens.assert_called()
database_dao.update.assert_called_with(
database_needs_oauth2,
@@ -509,7 +487,9 @@ def test_update_other_fields_dont_affect_oauth(
database_dao = mocker.patch("superset.commands.database.update.DatabaseDAO")
database_dao.find_by_id.return_value = database_needs_oauth2
database_dao.update.return_value = database_needs_oauth2
- sync_db_perms_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
+ sync_db_perms_dao = mocker.patch(
+ "superset.commands.database.sync_permissions.DatabaseDAO"
+ )
sync_db_perms_dao.find_by_id.return_value = database_needs_oauth2
mocker.patch("superset.commands.database.update.get_username")
mocker.patch("superset.security_manager.get_user_by_username")
@@ -522,12 +502,9 @@ def test_update_other_fields_dont_affect_oauth(
None,
"[my_db].[schema2]",
]
- add_permission_view_menu = mocker.patch.object(
- security_manager,
- "add_permission_view_menu",
- )
+ add_pvm = mocker.patch("superset.commands.database.sync_permissions.add_pvm")
UpdateDatabaseCommand(1, {"database_name": "New DB name"}).run()
- add_permission_view_menu.assert_not_called()
+ add_pvm.assert_not_called()
database_needs_oauth2.purge_oauth2_tokens.assert_not_called()
diff --git a/tests/unit_tests/tasks/permissions_test.py b/tests/unit_tests/tasks/permissions_test.py
deleted file mode 100644
index 49db280da8..0000000000
--- a/tests/unit_tests/tasks/permissions_test.py
+++ /dev/null
@@ -1,438 +0,0 @@
-# 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.
-from __future__ import annotations
-
-from unittest.mock import MagicMock
-
-import pytest
-from pytest_mock import MockerFixture
-
-from superset.commands.database.exceptions import (
- DatabaseConnectionFailedError,
- DatabaseNotFoundError,
- UserNotFoundInSessionError,
-)
-from superset.db_engine_specs.base import GenericDBException
-from superset.exceptions import OAuth2RedirectError
-from superset.tasks.permissions import (
- _get_catalog_names,
- _get_schema_names,
- _refresh_schemas,
- _rename_database_in_permissions,
- sync_database_permissions,
- sync_permissions,
-)
-
-
-def test__get_catalog_names(mocker: MockerFixture):
- """
- Test the ``_get_catalog_names`` method.
- """
- mock_db = mocker.MagicMock()
- mock_db.get_all_catalog_names.return_value = {"first_catalog", "second_catalog"}
-
- result = _get_catalog_names(mock_db, None)
-
- assert result == {"first_catalog", "second_catalog"}
- mock_db.get_all_catalog_names.assert_called_once_with(
- force=True,
- ssh_tunnel=None,
- )
-
-
-@pytest.mark.parametrize(
- ("exception_side_effect", "exception_raised"),
- [
- (
- OAuth2RedirectError("Missing token", "mock_tab", "mock_url"),
- OAuth2RedirectError,
- ),
- (GenericDBException, DatabaseConnectionFailedError),
- ],
-)
-def test__get_catalog_names_raise(
- mocker: MockerFixture,
- exception_side_effect: Exception,
- exception_raised: Exception,
-):
- """
- Test the ``_get_catalog_names`` method when an exception happens.
- """
- mock_db = mocker.MagicMock()
- mock_db.get_all_catalog_names.side_effect = exception_side_effect
- with pytest.raises(exception_raised):
- _get_catalog_names(mock_db, None)
-
-
-@pytest.mark.parametrize(
- ("catalog", "ssh_tunnel"),
- [
- (None, None),
- ("my_catalog", None),
- ("my_catalog", MagicMock()),
- (None, MagicMock()),
- ],
-)
-def test__get_schema_names(
- mocker: MockerFixture, catalog: str, ssh_tunnel: MagicMock | None
-):
- """
- Test the ``_get_schema_names`` method.
- """
- mock_db = mocker.MagicMock()
- mock_db.get_all_schema_names.return_value = {"first_schema", "second_schema"}
-
- result = _get_schema_names(mock_db, ssh_tunnel, catalog)
- assert result == {"first_schema", "second_schema"}
- mock_db.get_all_schema_names.assert_called_once_with(
- force=True,
- catalog=catalog,
- ssh_tunnel=ssh_tunnel,
- )
-
-
-@pytest.mark.parametrize(
- ("exception_side_effect", "exception_raised"),
- [
- (
- OAuth2RedirectError("Missing token", "mock_tab", "mock_url"),
- OAuth2RedirectError,
- ),
- (GenericDBException, DatabaseConnectionFailedError),
- ],
-)
-def test__get_schema_names_raise(
- mocker: MockerFixture,
- exception_side_effect: Exception,
- exception_raised: Exception,
-):
- """
- Test the ``_get_schema_names`` method when an exception happens.
- """
- mock_db = mocker.MagicMock()
- mock_db.get_all_schema_names.side_effect = exception_side_effect
- with pytest.raises(exception_raised):
- _get_schema_names(mock_db, None, None)
-
-
-def test__refresh_schemas(mocker: MockerFixture):
- """
- Test the ``_refresh_schemas`` method.
- """
- get_schem_perm_mock = mocker.patch(
- "superset.tasks.permissions.security_manager.get_schema_perm"
- )
- get_schem_perm_mock.side_effect = [
- "[same_name].[catalog].[schema1]",
- None,
- "[same_name].[catalog].[schema2]",
- ]
- find_pvm_mock = mocker.patch(
- "superset.tasks.permissions.security_manager.find_permission_view_menu"
- )
- find_pvm_mock.side_effect = [mocker.MagicMock(), None]
- add_pvm_mock = mocker.patch(
- "superset.commands.database.sync_permissions.security_manager.add_permission_view_menu"
- )
-
- _refresh_schemas("same name", "same name", "catalog", ["schema1", "schema2"])
-
- add_pvm_mock.assert_called_once_with(
- "schema_access", "[same_name].[catalog].[schema2]"
- )
-
-
-def test__rename_database_in_permissions(mocker: MockerFixture):
- """
- Test the ``_rename_database_in_permissions`` method.
- """
- get_schema_perm_mock = mocker.patch(
- "superset.tasks.permissions.security_manager.get_schema_perm"
- )
- find_pvm_mock = mocker.patch(
- "superset.tasks.permissions.security_manager.find_permission_view_menu"
- )
- mock_catalog_perm = mocker.MagicMock()
- mock_catalog_perm.view_menu.name = "[old_name].[catalog]"
- mock_schema_perm = mocker.MagicMock()
- mock_schema_perm.view_menu.name = "[old_name].[catalog].[schema1]"
- find_pvm_mock.side_effect = [
- mock_catalog_perm,
- mock_schema_perm,
- None,
- ]
- get_schema_perm_mock.side_effect = [
- "[new_name].[catalog].[schema1]",
- "[old_name].[catalog].[schema1]",
- "[new_name].[catalog].[schema2]",
- "[old_name].[catalog].[schema2]",
- ]
-
- mock_dataset = mocker.MagicMock()
- mock_dataset.id = 1
- mock_dataset.catalog_perm = "[old_name].[catalog]"
- mock_dataset.schema_perm = "[old_name].[catalog].[schema1]"
- mock_chart = mocker.MagicMock()
- mock_chart.catalog_perm = "[old_name].[catalog]"
- mock_chart.schema_perm = "[old_name].[catalog].[schema1]"
-
- mock_database_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
- mock_database_dao.get_datasets.side_effect = [
- [mock_dataset],
- [],
- ]
- mock_dataset_dao = mocker.patch("superset.tasks.permissions.DatasetDAO")
- mock_dataset_dao.get_related_objects.return_value = {"charts": [mock_chart]}
-
- _rename_database_in_permissions(
- 1, "old_name", "new_name", "catalog", ["schema1", "schema2"]
- )
-
- find_pvm_mock.assert_has_calls(
- [
- mocker.call("catalog_access", "[old_name].[catalog]"),
- mocker.call("schema_access", "[old_name].[catalog].[schema1]"),
- mocker.call("schema_access", "[old_name].[catalog].[schema2]"),
- ]
- )
-
- assert mock_catalog_perm.view_menu.name == "[new_name].[catalog]"
- assert mock_schema_perm.view_menu.name == "[new_name].[catalog].[schema1]"
- assert mock_dataset.catalog_perm == "[new_name].[catalog]"
- assert mock_dataset.schema_perm == "[new_name].[catalog].[schema1]"
- assert mock_chart.catalog_perm == "[new_name].[catalog]"
- assert mock_chart.schema_perm == "[new_name].[catalog].[schema1]"
-
-
-def test_sync_permissions(mocker: MockerFixture):
- """
- Test the ``sync_permissions`` method.
- """
- mock_db = mocker.MagicMock()
- mock_db.database_name = "same_name"
- mock_db.db_engine_spec.supports_catalog = True
- mocker.patch(
- "superset.tasks.permissions._get_catalog_names",
- return_value=["catalog1", "catalog2"],
- )
- mocker.patch(
- "superset.tasks.permissions._get_schema_names",
- side_effect=[
- ["schema1_catalog_1", "schema2_catalog_1"],
- ["schema1_catalog_2", "schema2_catalog_2"],
- ],
- )
- find_pvm_mock = mocker.patch(
- "superset.tasks.permissions.security_manager.find_permission_view_menu"
- )
- add_pvm_mock = mocker.patch(
- "superset.tasks.permissions.security_manager.add_permission_view_menu"
- )
- find_pvm_mock.side_effect = [mocker.MagicMock(), None]
- mock_refresh_schemas = mocker.patch("superset.tasks.permissions._refresh_schemas")
- mock_rename_db_perm = mocker.patch(
- "superset.tasks.permissions._rename_database_in_permissions"
- )
-
- sync_permissions(mock_db, None, "same_name")
-
- add_pvm_mock.assert_has_calls(
- [
- mocker.call("catalog_access", "[same_name].[catalog2]"),
- mocker.call("schema_access", "[same_name].[catalog2].[schema1_catalog_2]"),
- mocker.call("schema_access", "[same_name].[catalog2].[schema2_catalog_2]"),
- ]
- )
- mock_refresh_schemas.assert_called_once_with(
- "same_name",
- "same_name",
- "catalog1",
- ["schema1_catalog_1", "schema2_catalog_1"],
- )
- mock_rename_db_perm.assert_not_called()
-
-
-def test_sync_permissions_error_on_getting_schemas(mocker: MockerFixture):
- """
- Test the ``sync_permissions`` method when an exception is raised on
- getting the schemas for the catalog.
- """
- mock_db = mocker.MagicMock()
- mock_db.database_name = "same_name"
- mock_db.db_engine_spec.supports_catalog = True
- mocker.patch(
- "superset.tasks.permissions._get_catalog_names",
- return_value=["catalog1", "catalog2"],
- )
- mocker.patch(
- "superset.tasks.permissions._get_schema_names",
- side_effect=[
- DatabaseConnectionFailedError,
- ["schema1_catalog_2", "schema2_catalog_2"],
- ],
- )
- find_pvm_mock = mocker.patch(
- "superset.tasks.permissions.security_manager.find_permission_view_menu"
- )
- add_pvm_mock = mocker.patch(
- "superset.tasks.permissions.security_manager.add_permission_view_menu"
- )
- find_pvm_mock.return_value = mocker.MagicMock()
- mock_refresh_schemas = mocker.patch("superset.tasks.permissions._refresh_schemas")
- mock_rename_db_perm = mocker.patch(
- "superset.tasks.permissions._rename_database_in_permissions"
- )
-
- sync_permissions(mock_db, None, "same_name")
-
- add_pvm_mock.assert_not_called()
- mock_refresh_schemas.assert_called_once_with(
- "same_name",
- "same_name",
- "catalog2",
- ["schema1_catalog_2", "schema2_catalog_2"],
- )
- mock_rename_db_perm.assert_not_called()
-
-
-def test_sync_permissions_new_db_name(mocker: MockerFixture):
- """
- Test the ``sync_permissions`` method when the database name has changed.
- """
- mock_db = mocker.MagicMock()
- mock_db.database_name = "New Name"
- mock_db.id = 1
- mock_db.db_engine_spec.supports_catalog = True
- mocker.patch(
- "superset.tasks.permissions._get_catalog_names", return_value=["catalog"]
- )
- mocker.patch(
- "superset.tasks.permissions._get_schema_names", return_value=["schema"]
- )
- mocker.patch(
- "superset.tasks.permissions.security_manager.find_permission_view_menu",
- return_value=mocker.MagicMock(),
- )
- mocker.patch("superset.tasks.permissions._refresh_schemas")
- mock_rename_db_perm = mocker.patch(
- "superset.tasks.permissions._rename_database_in_permissions"
- )
-
- sync_permissions(mock_db, None, "Old Name")
-
- mock_rename_db_perm.assert_called_once_with(
- 1, "Old Name", "New Name", "catalog", ["schema"]
- )
-
-
-def test_sync_permissions_no_catalog(mocker: MockerFixture):
- """
- Test the ``sync_permissions`` method when the DB connection
- does not supports catalogs.
- """
- mock_db = mocker.MagicMock()
- mock_db.database_name = "Name"
- mock_db.db_engine_spec.supports_catalog = False
- mock_get_catalog = mocker.patch("superset.tasks.permissions._get_catalog_names")
- mocker.patch(
- "superset.tasks.permissions._get_schema_names", return_value=["schema"]
- )
- mock_refresh_schemas = mocker.patch("superset.tasks.permissions._refresh_schemas")
-
- sync_permissions(mock_db, None, "Name")
-
- mock_get_catalog.assert_not_called()
- mock_refresh_schemas.assert_called_once_with("Name", "Name", None, ["schema"])
-
-
-def test_sync_permissions_no_catalog_new_db_name(mocker: MockerFixture):
- """
- Test the ``sync_permissions`` method when the database name has changed
- and the DB connection does not support catalog.
- """
- mock_db = mocker.MagicMock()
- mock_db.database_name = "New Name"
- mock_db.id = 2
- mock_db.db_engine_spec.supports_catalog = False
- mock_get_catalog = mocker.patch("superset.tasks.permissions._get_catalog_names")
- mocker.patch(
- "superset.tasks.permissions._get_schema_names", return_value=["schema"]
- )
- mock_refresh_schemas = mocker.patch("superset.tasks.permissions._refresh_schemas")
- mock_rename_db_perm = mocker.patch(
- "superset.tasks.permissions._rename_database_in_permissions"
- )
-
- sync_permissions(mock_db, None, "Name")
-
- mock_get_catalog.assert_not_called()
- mock_refresh_schemas.assert_called_once_with("Name", "New Name", None, ["schema"])
- mock_rename_db_perm.assert_called_once_with(2, "Name", "New Name", None, ["schema"])
-
-
-def test_sync_database_permissions(mocker: MockerFixture):
- """
- Test the ``sync_database_permissions`` method.
- """
- mock_db = mocker.MagicMock()
- mock_ssh = mocker.MagicMock()
- mock_database_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
- mock_database_dao.find_by_id.return_value = mock_db
- mock_database_dao.get_ssh_tunnel.return_value = mock_ssh
- mocker.patch(
- "superset.commands.database.sync_permissions.security_manager.get_user_by_username"
- )
- mock_sync = mocker.patch("superset.tasks.permissions.sync_permissions")
-
- sync_database_permissions(1, "admin", "old_db_name")
-
- mock_sync.assert_called_once_with(mock_db, mock_ssh, "old_db_name")
-
-
-def test_sync_database_permissions_raise(mocker: MockerFixture):
- """
- Test the ``sync_database_permissions`` method when there's
- an exception.
- """
- mock_db = mocker.MagicMock()
- mock_database_dao = mocker.patch("superset.tasks.permissions.DatabaseDAO")
- mock_database_dao.find_by_id.return_value = mock_db
- mock_database_dao.get_ssh_tunnel.return_value = None
- mock_user = mocker.patch(
- "superset.commands.database.sync_permissions.security_manager.get_user_by_username"
- )
- mock_sync = mocker.patch("superset.tasks.permissions.sync_permissions")
-
- # Error during sync
- mock_sync.side_effect = DatabaseConnectionFailedError
- with pytest.raises(DatabaseConnectionFailedError):
- sync_database_permissions(1, "admin", "old_db_name")
- mock_database_dao.reset_mock()
-
- # DB not found
- mock_database_dao.reset_mock()
- mock_database_dao.find_by_id.return_value = None
- with pytest.raises(DatabaseNotFoundError):
- sync_database_permissions(1, "admin", "old_db_name")
- mock_database_dao.reset_mock()
- mock_database_dao.find_by_id.return_value = mock_db
-
- # User not found
- mock_user.reset_mock()
- mock_user.return_value = None
- with pytest.raises(UserNotFoundInSessionError):
- sync_database_permissions(1, "admin", "old_db_name")