fix: Remedy logic for UpdateDatasetCommand uniqueness check

superset/commands/dataset/create.py

@@ -61,17 +61,16 @@ def run(self) -> Model:
  def validate(self) -> None:
  exceptions: list[ValidationError] = []
  database_id = self._properties["database"]
- table_name = self._properties["table_name"]
  schema = self._properties.get("schema")
  catalog = self._properties.get("catalog")
  sql = self._properties.get("sql")
  owner_ids: Optional[list[int]] = self._properties.get("owners")
 
- table = Table(table_name, schema, catalog)
+ table = Table(self._properties["table_name"], schema, catalog)
 
  # Validate uniqueness
  if not DatasetDAO.validate_uniqueness(database_id, table):
- exceptions.append(DatasetExistsValidationError(table_name))
+ exceptions.append(DatasetExistsValidationError(table))
 
  # Validate/Populate database
  database = DatasetDAO.get_database_by_id(database_id)
@@ -86,7 +85,7 @@ def validate(self) -> None:
  and not sql
  and not DatasetDAO.validate_table_exists(database, table)
  ):
- exceptions.append(TableNotFoundValidationError(table_name))
+ exceptions.append(TableNotFoundValidationError(table))
 
  if sql:
  try:

superset/commands/dataset/duplicate.py

@@ -37,7 +37,7 @@
 from superset.exceptions import SupersetErrorException
 from superset.extensions import db
 from superset.models.core import Database
-from superset.sql_parse import ParsedQuery
+from superset.sql_parse import ParsedQuery, Table
 
 logger = logging.getLogger(__name__)
 
@@ -124,7 +124,7 @@ def validate(self) -> None:
  exceptions.append(DatasourceTypeInvalidError())
 
  if DatasetDAO.find_one_or_none(table_name=duplicate_name):
- exceptions.append(DatasetExistsValidationError(table_name=duplicate_name))
+ exceptions.append(DatasetExistsValidationError(table=Table(duplicate_name)))
 
  try:
  owners = self.populate_owners()

superset/commands/dataset/exceptions.py

@@ -26,10 +26,10 @@
  ImportFailedError,
  UpdateFailedError,
 )
+from superset.sql_parse import Table
 
-
-def get_dataset_exist_error_msg(full_name: str) -> str:
- return _("Dataset %(name)s already exists", name=full_name)
+def get_dataset_exist_error_msg(table: Table) -> str:
+ return _("Dataset %(table)s already exists", table=table)
 
 
 class DatabaseNotFoundValidationError(ValidationError):
@@ -55,9 +55,9 @@ class DatasetExistsValidationError(ValidationError):
  Marshmallow validation error for dataset already exists
  """
 
- def __init__(self, table_name: str) -> None:
+ def __init__(self, table: Table) -> None:
  super().__init__(
- [get_dataset_exist_error_msg(table_name)], field_name="table_name"
+ [get_dataset_exist_error_msg(table)], field_name="table"
  )
 
 

superset/commands/dataset/update.py

@@ -86,15 +86,21 @@ def validate(self) -> None:
  except SupersetSecurityException as ex:
  raise DatasetForbiddenError() from ex
 
- database_id = self._properties.get("database", None)
- table_name = self._properties.get("table_name", None)
+ database_id = self._properties.get("database")
+
+ table = Table(
+ self._properties.get("table_name"),
+ self._properties.get("schema"),
+ self._model.catalog,
+ )
+
  # Validate uniqueness
  if not DatasetDAO.validate_update_uniqueness(
  self._model.database_id,
- Table(table_name, self._model.schema, self._model.catalog),
+ table,
  self._model_id,
  ):
- exceptions.append(DatasetExistsValidationError(table_name))
+ exceptions.append(DatasetExistsValidationError(table))
  # Validate/Populate database not allowed to change
  if database_id and database_id != self._model:
  exceptions.append(DatabaseChangeValidationError())

tests/integration_tests/datasets/commands_tests.py

@@ -601,7 +601,6 @@ def test_create_dataset_command_not_allowed(self):
  }
  ).run()
 
-
 class TestDatasetWarmUpCacheCommand(SupersetTestCase):
  def test_warm_up_cache_command_table_not_found(self):
  with self.assertRaises(WarmUpCacheTableNotFoundError):

tests/unit_tests/commands/dataset/test_update.py

@@ -0,0 +1,43 @@
+from unittest.mock import MagicMock
+
+import pytest
+from pytest_mock import MockFixture
+
+from superset import db
+from superset.commands.dataset.exceptions import DatasetInvalidError
+from superset.commands.dataset.update import UpdateDatasetCommand
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+
+@pytest.mark.usefixture("session")
+def test_update_uniqueness_error(mocker: MockFixture) -> None:
+ SqlaTable.metadata.create_all(db.session.get_bind())
+ database = Database(database_name="my_db", sqlalchemy_uri="sqlite://")
+ bar = SqlaTable(table_name="bar", schema="foo", database=database)
+ baz = SqlaTable(table_name="baz", schema="qux", database=database)
+ db.session.add_all([database, bar, baz])
+ db.session.commit()
+
+ mock_g = mocker.patch("superset.security.manager.g")
+ mock_g.user = MagicMock()
+
+ mocker.patch(
+ "superset.views.base.security_manager.can_access_all_datasources",
+ return_value=True,
+ )
+
+ mocker.patch(
+ "superset.commands.dataset.update.security_manager.raise_for_ownership",
+ return_value=None,
+ )
+
+ mocker.patch.object(UpdateDatasetCommand, "compute_owners", return_value=[])
+
+ with pytest.raises(DatasetInvalidError):
+ UpdateDatasetCommand(
+ bar.id,
+ {
+ "table_name": "baz",
+ "schema": "qux",
+ }
+ ).run()

tests/unit_tests/dao/dataset_test.py

@@ -51,7 +51,7 @@ def test_validate_update_uniqueness(session: Session) -> None:
  db.session.add_all([database, dataset1, dataset2])
  db.session.flush()
 
- # same table name, different schema
+ #
  assert (
  DatasetDAO.validate_update_uniqueness(
  database_id=database.id,
@@ -61,7 +61,6 @@ def test_validate_update_uniqueness(session: Session) -> None:
  is True
  )
 
- # duplicate schema and table name
  assert (
  DatasetDAO.validate_update_uniqueness(
  database_id=database.id,
@@ -71,7 +70,6 @@ def test_validate_update_uniqueness(session: Session) -> None:
  is False
  )
 
- # no schema
  assert (
  DatasetDAO.validate_update_uniqueness(
  database_id=database.id,