Skip to content

Commit

Permalink
even better name and test cleanup via rollback
Browse files Browse the repository at this point in the history
  • Loading branch information
bogdan-dbx committed Aug 8, 2022
1 parent fc63ff2 commit ab59aa4
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 19 deletions.
4 changes: 2 additions & 2 deletions superset/dao/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ def find_by_id(
cls,
model_id: Union[str, int],
session: Session = None,
skip_filter: bool = False,
skip_base_filter: bool = False,
) -> Optional[Model]:
"""
Find a model by id, if defined applies `base_filter`
"""
session = session or db.session
query = session.query(cls.model_cls)
if cls.base_filter and not skip_filter:
if cls.base_filter and not skip_base_filter:
data_model = SQLAInterface(cls.model_cls, session)
query = cls.base_filter( # pylint: disable=not-callable
cls.id_column_name, data_model
Expand Down
6 changes: 3 additions & 3 deletions superset/explore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
def check_dataset_access(dataset_id: int) -> Optional[bool]:
if dataset_id:
# Access checks below, no need to validate them twice as they can be expensive.
dataset = DatasetDAO.find_by_id(dataset_id, skip_filter=True)
dataset = DatasetDAO.find_by_id(dataset_id, skip_base_filter=True)
if dataset:
can_access_datasource = security_manager.can_access_datasource(dataset)
if can_access_datasource:
Expand All @@ -51,7 +51,7 @@ def check_dataset_access(dataset_id: int) -> Optional[bool]:
def check_query_access(query_id: int) -> Optional[bool]:
if query_id:
# Access checks below, no need to validate them twice as they can be expensive.
query = QueryDAO.find_by_id(query_id, skip_filter=True)
query = QueryDAO.find_by_id(query_id, skip_base_filter=True)
if query:
security_manager.raise_for_access(query=query)
return True
Expand Down Expand Up @@ -84,7 +84,7 @@ def check_access(
if not chart_id:
return True
# Access checks below, no need to validate them twice as they can be expensive.
chart = ChartDAO.find_by_id(chart_id, skip_filter=True)
chart = ChartDAO.find_by_id(chart_id, skip_base_filter=True)
if chart:
can_access_chart = security_manager.is_owner(
chart
Expand Down
15 changes: 8 additions & 7 deletions tests/unit_tests/charts/dao/dao_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,29 @@ def session_with_data(session: Session) -> Iterator[Session]:
)

session.add(slice_obj)
session.flush()
yield session
session.delete(slice_obj)
session.commit()
yield session
session.rollback()


def test_slice_find_by_id_skip_filter(session_with_data: Session) -> None:
def test_slice_find_by_id_skip_base_filter(session_with_data: Session) -> None:
from superset.charts.dao import ChartDAO
from superset.models.slice import Slice

result = ChartDAO.find_by_id(1, session=session_with_data, skip_filter=True)
result = ChartDAO.find_by_id(1, session=session_with_data, skip_base_filter=True)

assert result
assert 1 == result.id
assert "slice_name" == result.slice_name
assert isinstance(result, Slice)


def test_datasource_find_by_id_skip_filter_not_found(
def test_datasource_find_by_id_skip_base_filter_not_found(
session_with_data: Session,
) -> None:
from superset.charts.dao import ChartDAO

result = ChartDAO.find_by_id(125326326, session=session_with_data, skip_filter=True)
result = ChartDAO.find_by_id(
125326326, session=session_with_data, skip_base_filter=True
)
assert result is None
12 changes: 5 additions & 7 deletions tests/unit_tests/datasets/dao/dao_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,17 @@ def session_with_data(session: Session) -> Iterator[Session]:
session.add(sqla_table)
session.flush()
yield session
session.delete(sqla_table)
session.delete(db)
session.commit()
session.rollback()


def test_datasource_find_by_id_skip_filter(session_with_data: Session) -> None:
def test_datasource_find_by_id_skip_base_filter(session_with_data: Session) -> None:
from superset.connectors.sqla.models import SqlaTable
from superset.datasets.dao import DatasetDAO

result = DatasetDAO.find_by_id(
1,
session=session_with_data,
skip_filter=True,
skip_base_filter=True,
)

assert result
Expand All @@ -62,14 +60,14 @@ def test_datasource_find_by_id_skip_filter(session_with_data: Session) -> None:
assert isinstance(result, SqlaTable)


def test_datasource_find_by_id_skip_filter_not_found(
def test_datasource_find_by_id_skip_base_filter_not_found(
session_with_data: Session,
) -> None:
from superset.datasets.dao import DatasetDAO

result = DatasetDAO.find_by_id(
125326326,
session=session_with_data,
skip_filter=True,
skip_base_filter=True,
)
assert result is None

0 comments on commit ab59aa4

Please sign in to comment.