From a7d19ebe9bb72396a70e2fb71d313ee670199e9c Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Wed, 27 Jul 2022 12:12:29 -0400 Subject: [PATCH 01/34] Added back-end dataset tagging code --- superset/common/tags.py | 100 ++++++++++++++++++++++++++--- superset/connectors/sqla/models.py | 7 ++ superset/models/tags.py | 26 ++++++-- superset/views/tags.py | 28 ++++++++ 4 files changed, 146 insertions(+), 15 deletions(-) diff --git a/superset/common/tags.py b/superset/common/tags.py index 74c882cf92f69..95a477925c09f 100644 --- a/superset/common/tags.py +++ b/superset/common/tags.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from sqlalchemy import Metadata +from sqlalchemy import MetaData from sqlalchemy.engine import Engine from sqlalchemy.exc import IntegrityError from sqlalchemy.sql import and_, func, functions, join, literal, select @@ -22,7 +22,7 @@ from superset.models.tags import ObjectTypes, TagTypes -def add_types(engine: Engine, metadata: Metadata) -> None: +def add_types(engine: Engine, metadata: MetaData) -> None: """ Tag every object according to its type: @@ -67,6 +67,20 @@ def add_types(engine: Engine, metadata: Metadata) -> None: AND tagged_object.object_id = saved_query.id AND tagged_object.object_type = 'query' WHERE tagged_object.tag_id IS NULL; + + INSERT INTO tagged_object (tag_id, object_id, object_type) + SELECT + tag.id AS tag_id, + tables.id AS object_id, + 'dataset' AS object_type + FROM tables + JOIN tag + ON tag.name = 'type:dataset' + LEFT OUTER JOIN tagged_object + ON tagged_object.tag_id = tag.id + AND tagged_object.object_id = tables.id + AND tagged_object.object_type = 'dataset' + WHERE tagged_object.tag_id IS NULL; """ @@ -75,6 +89,7 @@ def add_types(engine: Engine, metadata: Metadata) -> None: slices = metadata.tables["slices"] dashboards = metadata.tables["dashboards"] saved_query = metadata.tables["saved_query"] + tables = metadata.tables["tables"] columns = ["tag_id", "object_id", "object_type"] # add a tag for each object type @@ -163,8 +178,34 @@ def add_types(engine: Engine, metadata: Metadata) -> None: query = tagged_object.insert().from_select(columns, saved_queries) engine.execute(query) + datasets = ( + select( + [ + tag.c.id.label("tag_id"), + tables.c.id.label("object_id"), + literal(ObjectTypes.dataset.name).label("object_type"), + ] + ) + .select_from( + join( + join(tables, tag, tag.c.name == "type:dataset"), + tagged_object, + and_( + tagged_object.c.tag_id == tag.c.id, + tagged_object.c.object_id == tables.c.id, + tagged_object.c.object_type == "dataset", + ), + isouter=True, + full=False, + ) + ) + .where(tagged_object.c.tag_id.is_(None)) + ) + query = tagged_object.insert().from_select(columns, datasets) + engine.execute(query) + -def add_owners(engine: Engine, metadata: Metadata) -> None: +def add_owners(engine: Engine, metadata: MetaData) -> None: """ Tag every object according to its owner: @@ -208,6 +249,19 @@ def add_owners(engine: Engine, metadata: Metadata) -> None: AND tagged_object.object_type = 'query' WHERE tagged_object.tag_id IS NULL; + SELECT + tag.id AS tag_id, + tables.id AS object_id, + 'dataset' AS object_type + FROM tables + JOIN tag + ON tag.name = CONCAT('owner:', tables.created_by_fk) + LEFT OUTER JOIN tagged_object + ON tagged_object.tag_id = tag.id + AND tagged_object.object_id = tables.id + AND tagged_object.object_type = 'dataset' + WHERE tagged_object.tag_id IS NULL; + """ tag = metadata.tables["tag"] @@ -216,6 +270,7 @@ def add_owners(engine: Engine, metadata: Metadata) -> None: slices = metadata.tables["slices"] dashboards = metadata.tables["dashboards"] saved_query = metadata.tables["saved_query"] + tables = metadata.tables["tables"] columns = ["tag_id", "object_id", "object_type"] # create a custom tag for each user @@ -240,7 +295,7 @@ def add_owners(engine: Engine, metadata: Metadata) -> None: join( slices, tag, - tag.c.name == functions.concat("owner:", slices.c.created_by_fk), + tag.c.name == "owner:" + slices.c.created_by_fk, ), tagged_object, and_( @@ -271,7 +326,7 @@ def add_owners(engine: Engine, metadata: Metadata) -> None: dashboards, tag, tag.c.name - == functions.concat("owner:", dashboards.c.created_by_fk), + == "owner:" + dashboards.c.created_by_fk, ), tagged_object, and_( @@ -302,7 +357,7 @@ def add_owners(engine: Engine, metadata: Metadata) -> None: saved_query, tag, tag.c.name - == functions.concat("owner:", saved_query.c.created_by_fk), + == "owner:" + saved_query.c.created_by_fk, ), tagged_object, and_( @@ -319,8 +374,37 @@ def add_owners(engine: Engine, metadata: Metadata) -> None: query = tagged_object.insert().from_select(columns, saved_queries) engine.execute(query) + datasets = ( + select( + [ + tag.c.id.label("tag_id"), + tables.c.id.label("object_id"), + literal(ObjectTypes.dataset.name).label("object_type"), + ] + ) + .select_from( + join( + join( + tables, + tag, + tag.c.name == "owner:" + tables.c.created_by_fk, + ), + tagged_object, + and_( + tagged_object.c.tag_id == tag.c.id, + tagged_object.c.object_id == tables.c.id, + tagged_object.c.object_type == "dataset", + ), + isouter=True, + full=False, + ) + ) + .where(tagged_object.c.tag_id.is_(None)) + ) + query = tagged_object.insert().from_select(columns, datasets) + engine.execute(query) -def add_favorites(engine: Engine, metadata: Metadata) -> None: +def add_favorites(engine: Engine, metadata: MetaData) -> None: """ Tag every object that was favorited: @@ -368,7 +452,7 @@ def add_favorites(engine: Engine, metadata: Metadata) -> None: join( favstar, tag, - tag.c.name == functions.concat("favorited_by:", favstar.c.user_id), + tag.c.name == "favorited_by:" + favstar.c.user_id, ), tagged_object, and_( diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 237a71ab7b76a..0d8a75ac8bc7b 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -108,6 +108,7 @@ clone_model, QueryResult, ) +from superset.models.tags import DatasetUpdater from superset.sql_parse import ( extract_table_references, ParsedQuery, @@ -2540,6 +2541,12 @@ def write_shadow_dataset( sa.event.listen(TableColumn, "after_update", SqlaTable.update_column) sa.event.listen(TableColumn, "after_delete", TableColumn.after_delete) +# events for updating tags +if is_feature_enabled("TAGGING_SYSTEM"): + sa.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) + sa.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) + sa.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) + RLSFilterRoles = Table( "rls_filter_roles", metadata, diff --git a/superset/models/tags.py b/superset/models/tags.py index 528206e672f62..722d8440e0630 100644 --- a/superset/models/tags.py +++ b/superset/models/tags.py @@ -32,6 +32,7 @@ from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import Query + from superset.connectors.sqla.models import SqlaTable Session = sessionmaker(autoflush=False) @@ -41,7 +42,7 @@ class TagTypes(enum.Enum): """ Types for tags. - Objects (queries, charts and dashboards) will have with implicit tags based + Objects (queries, charts, dashboards, and datasets) will have with implicit tags based on metadata: types, owners and who favorited them. This way, user "alice" can find all their objects by querying for the tag `owner:alice`. """ @@ -64,11 +65,12 @@ class ObjectTypes(enum.Enum): query = 1 chart = 2 dashboard = 3 + dataset = 4 class Tag(Model, AuditMixinNullable): - """A tag attached to an object (query, chart or dashboard).""" + """A tag attached to an object (query, chart, dashboard, or dataset).""" __tablename__ = "tag" id = Column(Integer, primary_key=True) @@ -103,6 +105,7 @@ def get_object_type(class_name: str) -> ObjectTypes: "slice": ObjectTypes.chart, "dashboard": ObjectTypes.dashboard, "query": ObjectTypes.query, + "dataset": ObjectTypes.dataset, } try: return mapping[class_name.lower()] @@ -116,13 +119,13 @@ class ObjectUpdater: @classmethod def get_owners_ids( - cls, target: Union["Dashboard", "FavStar", "Slice"] + cls, target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"] ) -> List[int]: raise NotImplementedError("Subclass should implement `get_owners_ids`") @classmethod def _add_owners( - cls, session: Session, target: Union["Dashboard", "FavStar", "Slice"] + cls, session: Session, target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"] ) -> None: for owner_id in cls.get_owners_ids(target): name = "owner:{0}".format(owner_id) @@ -137,7 +140,7 @@ def after_insert( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice"], + target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"], ) -> None: session = Session(bind=connection) @@ -158,7 +161,7 @@ def after_update( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice"], + target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"], ) -> None: session = Session(bind=connection) @@ -187,7 +190,7 @@ def after_delete( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice"], + target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"], ) -> None: session = Session(bind=connection) @@ -227,6 +230,15 @@ def get_owners_ids(cls, target: "Query") -> List[int]: return [target.user_id] +class DatasetUpdater(ObjectUpdater): + + object_type = "dataset" + + @classmethod + def get_owners_ids(cls, target: "SqlaTable") -> List[int]: + return [owner.id for owner in target.owners] + + class FavStarUpdater: @classmethod def after_insert( diff --git a/superset/views/tags.py b/superset/views/tags.py index 8ab2798f5d84c..b0b5fb11786b0 100644 --- a/superset/views/tags.py +++ b/superset/views/tags.py @@ -32,6 +32,7 @@ from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import SavedQuery +from superset.connectors.sqla.models import SqlaQuery from superset.models.tags import ObjectTypes, Tag, TaggedObject, TagTypes from superset.superset_typing import FlaskResponse @@ -238,4 +239,31 @@ def tagged_objects(self) -> FlaskResponse: # pylint: disable=no-self-use for obj in saved_queries ) + # datasets + if not types or "dataset" in types: + datasets = ( + db.session.query(SqlaQuery) + .join( + TaggedObject, + and_( + TaggedObject.object_id == SqlaQuery.id, + TaggedObject.object_type == ObjectTypes.dataset, + ), + ) + .join(Tag, TaggedObject.tag_id == Tag.id) + .filter(Tag.name.in_(tags)) + ) + results.extend( + { + "id": obj.id, + "type": ObjectTypes.dataset.name, + "name": obj.table_name, + "url": obj.sql_url(), + "changed_on": obj.changed_on, + "created_by": obj.created_by_fk, + "creator": obj.creator(), + } + for obj in datasets + ) + return json_success(json.dumps(results, default=utils.core.json_int_dttm_ser)) From 9d9dce50da1eef23a0545dfb097cd2d16035999c Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Wed, 27 Jul 2022 12:12:29 -0400 Subject: [PATCH 02/34] Added back-end dataset tagging code From ba5df773d47973f456b182dadc79d57a670063ae Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 28 Jul 2022 08:17:53 -0400 Subject: [PATCH 03/34] Ran pre-commit hook --- superset/common/tags.py | 9 ++++----- superset/models/tags.py | 6 ++++-- superset/views/tags.py | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/superset/common/tags.py b/superset/common/tags.py index 95a477925c09f..b9a1783029190 100644 --- a/superset/common/tags.py +++ b/superset/common/tags.py @@ -67,7 +67,7 @@ def add_types(engine: Engine, metadata: MetaData) -> None: AND tagged_object.object_id = saved_query.id AND tagged_object.object_type = 'query' WHERE tagged_object.tag_id IS NULL; - + INSERT INTO tagged_object (tag_id, object_id, object_type) SELECT tag.id AS tag_id, @@ -325,8 +325,7 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: join( dashboards, tag, - tag.c.name - == "owner:" + dashboards.c.created_by_fk, + tag.c.name == "owner:" + dashboards.c.created_by_fk, ), tagged_object, and_( @@ -356,8 +355,7 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: join( saved_query, tag, - tag.c.name - == "owner:" + saved_query.c.created_by_fk, + tag.c.name == "owner:" + saved_query.c.created_by_fk, ), tagged_object, and_( @@ -404,6 +402,7 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: query = tagged_object.insert().from_select(columns, datasets) engine.execute(query) + def add_favorites(engine: Engine, metadata: MetaData) -> None: """ Tag every object that was favorited: diff --git a/superset/models/tags.py b/superset/models/tags.py index 722d8440e0630..475207ebf6027 100644 --- a/superset/models/tags.py +++ b/superset/models/tags.py @@ -28,11 +28,11 @@ from superset.models.helpers import AuditMixinNullable if TYPE_CHECKING: + from superset.connectors.sqla.models import SqlaTable from superset.models.core import FavStar from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import Query - from superset.connectors.sqla.models import SqlaTable Session = sessionmaker(autoflush=False) @@ -125,7 +125,9 @@ def get_owners_ids( @classmethod def _add_owners( - cls, session: Session, target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"] + cls, + session: Session, + target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"], ) -> None: for owner_id in cls.get_owners_ids(target): name = "owner:{0}".format(owner_id) diff --git a/superset/views/tags.py b/superset/views/tags.py index b0b5fb11786b0..838bdb73edab9 100644 --- a/superset/views/tags.py +++ b/superset/views/tags.py @@ -28,11 +28,11 @@ from werkzeug.exceptions import NotFound from superset import db, is_feature_enabled, utils +from superset.connectors.sqla.models import SqlaTable from superset.jinja_context import ExtraCache from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import SavedQuery -from superset.connectors.sqla.models import SqlaQuery from superset.models.tags import ObjectTypes, Tag, TaggedObject, TagTypes from superset.superset_typing import FlaskResponse @@ -242,11 +242,11 @@ def tagged_objects(self) -> FlaskResponse: # pylint: disable=no-self-use # datasets if not types or "dataset" in types: datasets = ( - db.session.query(SqlaQuery) + db.session.query(SqlaTable) .join( TaggedObject, and_( - TaggedObject.object_id == SqlaQuery.id, + TaggedObject.object_id == SqlaTable.id, TaggedObject.object_type == ObjectTypes.dataset, ), ) From 1c9e70ad2cf8014df7ad624e476a20205e76e923 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 28 Jul 2022 09:40:42 -0400 Subject: [PATCH 04/34] Broke up large function into multiple smaller functions so that the linter passes --- superset/common/tags.py | 250 ++++++++++++++++++++++++---------------- 1 file changed, 151 insertions(+), 99 deletions(-) diff --git a/superset/common/tags.py b/superset/common/tags.py index b9a1783029190..7f4ca10161e84 100644 --- a/superset/common/tags.py +++ b/superset/common/tags.py @@ -14,91 +14,20 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import Any, List + from sqlalchemy import MetaData from sqlalchemy.engine import Engine from sqlalchemy.exc import IntegrityError -from sqlalchemy.sql import and_, func, functions, join, literal, select +from sqlalchemy.sql import and_, func, join, literal, select from superset.models.tags import ObjectTypes, TagTypes -def add_types(engine: Engine, metadata: MetaData) -> None: - """ - Tag every object according to its type: - - INSERT INTO tagged_object (tag_id, object_id, object_type) - SELECT - tag.id AS tag_id, - slices.id AS object_id, - 'chart' AS object_type - FROM slices - JOIN tag - ON tag.name = 'type:chart' - LEFT OUTER JOIN tagged_object - ON tagged_object.tag_id = tag.id - AND tagged_object.object_id = slices.id - AND tagged_object.object_type = 'chart' - WHERE tagged_object.tag_id IS NULL; - - INSERT INTO tagged_object (tag_id, object_id, object_type) - SELECT - tag.id AS tag_id, - dashboards.id AS object_id, - 'dashboard' AS object_type - FROM dashboards - JOIN tag - ON tag.name = 'type:dashboard' - LEFT OUTER JOIN tagged_object - ON tagged_object.tag_id = tag.id - AND tagged_object.object_id = dashboards.id - AND tagged_object.object_type = 'dashboard' - WHERE tagged_object.tag_id IS NULL; - - INSERT INTO tagged_object (tag_id, object_id, object_type) - SELECT - tag.id AS tag_id, - saved_query.id AS object_id, - 'query' AS object_type - FROM saved_query - JOIN tag - ON tag.name = 'type:query'; - LEFT OUTER JOIN tagged_object - ON tagged_object.tag_id = tag.id - AND tagged_object.object_id = saved_query.id - AND tagged_object.object_type = 'query' - WHERE tagged_object.tag_id IS NULL; - - INSERT INTO tagged_object (tag_id, object_id, object_type) - SELECT - tag.id AS tag_id, - tables.id AS object_id, - 'dataset' AS object_type - FROM tables - JOIN tag - ON tag.name = 'type:dataset' - LEFT OUTER JOIN tagged_object - ON tagged_object.tag_id = tag.id - AND tagged_object.object_id = tables.id - AND tagged_object.object_type = 'dataset' - WHERE tagged_object.tag_id IS NULL; - - """ - - tag = metadata.tables["tag"] - tagged_object = metadata.tables["tagged_object"] +def add_types_to_charts( + engine: Engine, metadata: MetaData, tag: Any, tagged_object: Any, columns: List[str] +) -> None: slices = metadata.tables["slices"] - dashboards = metadata.tables["dashboards"] - saved_query = metadata.tables["saved_query"] - tables = metadata.tables["tables"] - columns = ["tag_id", "object_id", "object_type"] - - # add a tag for each object type - insert = tag.insert() - for type_ in ObjectTypes.__members__: - try: - engine.execute(insert, name=f"type:{type_}", type=TagTypes.type) - except IntegrityError: - pass # already exists charts = ( select( @@ -126,21 +55,27 @@ def add_types(engine: Engine, metadata: MetaData) -> None: query = tagged_object.insert().from_select(columns, charts) engine.execute(query) + +def add_types_to_dashboards( + engine: Engine, metadata: MetaData, tag: Any, tagged_object: Any, columns: List[str] +) -> None: + dashboard_table = metadata.tables["dashboards"] + dashboards = ( select( [ tag.c.id.label("tag_id"), - dashboards.c.id.label("object_id"), + dashboard_table.c.id.label("object_id"), literal(ObjectTypes.dashboard.name).label("object_type"), ] ) .select_from( join( - join(dashboards, tag, tag.c.name == "type:dashboard"), + join(dashboard_table, tag, tag.c.name == "type:dashboard"), tagged_object, and_( tagged_object.c.tag_id == tag.c.id, - tagged_object.c.object_id == dashboards.c.id, + tagged_object.c.object_id == dashboard_table.c.id, tagged_object.c.object_type == "dashboard", ), isouter=True, @@ -152,6 +87,12 @@ def add_types(engine: Engine, metadata: MetaData) -> None: query = tagged_object.insert().from_select(columns, dashboards) engine.execute(query) + +def add_types_to_saved_queries( + engine: Engine, metadata: MetaData, tag: Any, tagged_object: Any, columns: List[str] +) -> None: + saved_query = metadata.tables["saved_query"] + saved_queries = ( select( [ @@ -178,6 +119,12 @@ def add_types(engine: Engine, metadata: MetaData) -> None: query = tagged_object.insert().from_select(columns, saved_queries) engine.execute(query) + +def add_types_to_datasets( + engine: Engine, metadata: MetaData, tag: Any, tagged_object: Any, columns: List[str] +) -> None: + tables = metadata.tables["tables"] + datasets = ( select( [ @@ -205,9 +152,9 @@ def add_types(engine: Engine, metadata: MetaData) -> None: engine.execute(query) -def add_owners(engine: Engine, metadata: MetaData) -> None: +def add_types(engine: Engine, metadata: MetaData) -> None: """ - Tag every object according to its owner: + Tag every object according to its type: INSERT INTO tagged_object (tag_id, object_id, object_type) SELECT @@ -216,46 +163,49 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: 'chart' AS object_type FROM slices JOIN tag - ON tag.name = CONCAT('owner:', slices.created_by_fk) + ON tag.name = 'type:chart' LEFT OUTER JOIN tagged_object ON tagged_object.tag_id = tag.id AND tagged_object.object_id = slices.id AND tagged_object.object_type = 'chart' WHERE tagged_object.tag_id IS NULL; + INSERT INTO tagged_object (tag_id, object_id, object_type) SELECT tag.id AS tag_id, dashboards.id AS object_id, 'dashboard' AS object_type FROM dashboards JOIN tag - ON tag.name = CONCAT('owner:', dashboards.created_by_fk) + ON tag.name = 'type:dashboard' LEFT OUTER JOIN tagged_object ON tagged_object.tag_id = tag.id AND tagged_object.object_id = dashboards.id AND tagged_object.object_type = 'dashboard' WHERE tagged_object.tag_id IS NULL; + INSERT INTO tagged_object (tag_id, object_id, object_type) SELECT tag.id AS tag_id, saved_query.id AS object_id, 'query' AS object_type FROM saved_query JOIN tag - ON tag.name = CONCAT('owner:', saved_query.created_by_fk) + ON tag.name = 'type:query'; LEFT OUTER JOIN tagged_object ON tagged_object.tag_id = tag.id AND tagged_object.object_id = saved_query.id AND tagged_object.object_type = 'query' WHERE tagged_object.tag_id IS NULL; + INSERT INTO tagged_object (tag_id, object_id, object_type) SELECT tag.id AS tag_id, tables.id AS object_id, 'dataset' AS object_type FROM tables JOIN tag - ON tag.name = CONCAT('owner:', tables.created_by_fk) + ON tag.name = 'type:dataset' LEFT OUTER JOIN tagged_object ON tagged_object.tag_id = tag.id AND tagged_object.object_id = tables.id @@ -266,22 +216,27 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: tag = metadata.tables["tag"] tagged_object = metadata.tables["tagged_object"] - users = metadata.tables["ab_user"] - slices = metadata.tables["slices"] - dashboards = metadata.tables["dashboards"] - saved_query = metadata.tables["saved_query"] - tables = metadata.tables["tables"] columns = ["tag_id", "object_id", "object_type"] - # create a custom tag for each user - ids = select([users.c.id]) + # add a tag for each object type insert = tag.insert() - for (id_,) in engine.execute(ids): + for type_ in ObjectTypes.__members__: try: - engine.execute(insert, name=f"owner:{id_}", type=TagTypes.owner) + engine.execute(insert, name=f"type:{type_}", type=TagTypes.type) except IntegrityError: pass # already exists + add_types_to_charts(engine, metadata, tag, tagged_object, columns) + add_types_to_dashboards(engine, metadata, tag, tagged_object, columns) + add_types_to_saved_queries(engine, metadata, tag, tagged_object, columns) + add_types_to_datasets(engine, metadata, tag, tagged_object, columns) + + +def add_owners_to_charts( + engine: Engine, metadata: MetaData, tag: Any, tagged_object: Any, columns: List[str] +) -> None: + slices = metadata.tables["slices"] + charts = ( select( [ @@ -312,25 +267,31 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: query = tagged_object.insert().from_select(columns, charts) engine.execute(query) + +def add_owners_to_dashboards( + engine: Engine, metadata: MetaData, tag: Any, tagged_object: Any, columns: List[str] +) -> None: + dashboard_table = metadata.tables["dashboards"] + dashboards = ( select( [ tag.c.id.label("tag_id"), - dashboards.c.id.label("object_id"), + dashboard_table.c.id.label("object_id"), literal(ObjectTypes.dashboard.name).label("object_type"), ] ) .select_from( join( join( - dashboards, + dashboard_table, tag, - tag.c.name == "owner:" + dashboards.c.created_by_fk, + tag.c.name == "owner:" + dashboard_table.c.created_by_fk, ), tagged_object, and_( tagged_object.c.tag_id == tag.c.id, - tagged_object.c.object_id == dashboards.c.id, + tagged_object.c.object_id == dashboard_table.c.id, tagged_object.c.object_type == "dashboard", ), isouter=True, @@ -342,6 +303,12 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: query = tagged_object.insert().from_select(columns, dashboards) engine.execute(query) + +def add_owners_to_saved_queries( + engine: Engine, metadata: MetaData, tag: Any, tagged_object: Any, columns: List[str] +) -> None: + saved_query = metadata.tables["saved_query"] + saved_queries = ( select( [ @@ -372,6 +339,12 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: query = tagged_object.insert().from_select(columns, saved_queries) engine.execute(query) + +def add_owners_to_datasets( + engine: Engine, metadata: MetaData, tag: Any, tagged_object: Any, columns: List[str] +) -> None: + tables = metadata.tables["tables"] + datasets = ( select( [ @@ -403,6 +376,85 @@ def add_owners(engine: Engine, metadata: MetaData) -> None: engine.execute(query) +def add_owners(engine: Engine, metadata: MetaData) -> None: + """ + Tag every object according to its owner: + + INSERT INTO tagged_object (tag_id, object_id, object_type) + SELECT + tag.id AS tag_id, + slices.id AS object_id, + 'chart' AS object_type + FROM slices + JOIN tag + ON tag.name = CONCAT('owner:', slices.created_by_fk) + LEFT OUTER JOIN tagged_object + ON tagged_object.tag_id = tag.id + AND tagged_object.object_id = slices.id + AND tagged_object.object_type = 'chart' + WHERE tagged_object.tag_id IS NULL; + + SELECT + tag.id AS tag_id, + dashboards.id AS object_id, + 'dashboard' AS object_type + FROM dashboards + JOIN tag + ON tag.name = CONCAT('owner:', dashboards.created_by_fk) + LEFT OUTER JOIN tagged_object + ON tagged_object.tag_id = tag.id + AND tagged_object.object_id = dashboards.id + AND tagged_object.object_type = 'dashboard' + WHERE tagged_object.tag_id IS NULL; + + SELECT + tag.id AS tag_id, + saved_query.id AS object_id, + 'query' AS object_type + FROM saved_query + JOIN tag + ON tag.name = CONCAT('owner:', saved_query.created_by_fk) + LEFT OUTER JOIN tagged_object + ON tagged_object.tag_id = tag.id + AND tagged_object.object_id = saved_query.id + AND tagged_object.object_type = 'query' + WHERE tagged_object.tag_id IS NULL; + + SELECT + tag.id AS tag_id, + tables.id AS object_id, + 'dataset' AS object_type + FROM tables + JOIN tag + ON tag.name = CONCAT('owner:', tables.created_by_fk) + LEFT OUTER JOIN tagged_object + ON tagged_object.tag_id = tag.id + AND tagged_object.object_id = tables.id + AND tagged_object.object_type = 'dataset' + WHERE tagged_object.tag_id IS NULL; + + """ + + tag = metadata.tables["tag"] + tagged_object = metadata.tables["tagged_object"] + users = metadata.tables["ab_user"] + columns = ["tag_id", "object_id", "object_type"] + + # create a custom tag for each user + ids = select([users.c.id]) + insert = tag.insert() + for (id_,) in engine.execute(ids): + try: + engine.execute(insert, name=f"owner:{id_}", type=TagTypes.owner) + except IntegrityError: + pass # already exists + + add_owners_to_charts(engine, metadata, tag, tagged_object, columns) + add_owners_to_dashboards(engine, metadata, tag, tagged_object, columns) + add_owners_to_saved_queries(engine, metadata, tag, tagged_object, columns) + add_owners_to_datasets(engine, metadata, tag, tagged_object, columns) + + def add_favorites(engine: Engine, metadata: MetaData) -> None: """ Tag every object that was favorited: From 12322cd3f9362336111bf05485d7cf569f277426 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 26 Aug 2022 09:57:09 -0400 Subject: [PATCH 05/34] Added integration tests to make sure that the code which was added worked as intended --- tests/integration_tests/tagging_tests.py | 139 +++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 9ae8764d40d55..a87b3f58e2cbd 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -15,11 +15,32 @@ # specific language governing permissions and limitations # under the License. +from sqlalchemy import func + +from superset.connectors.sqla.models import SqlaTable +from superset.extensions import db +from superset.models.core import FavStar +from superset.models.dashboard import Dashboard +from superset.models.slice import Slice +from superset.models.tags import Tag, TaggedObject +from superset.utils.core import DatasourceType +from superset.utils.database import get_main_database from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.conftest import with_feature_flags class TestTagging(SupersetTestCase): + def query_tagged_object_table(self): + query = ( + db.session.query(TaggedObject) + .join(Tag) + .with_entities(TaggedObject.tag_id, Tag.name) + .group_by(TaggedObject.tag_id, Tag.name) + .order_by(func.count().desc()) + .all() + ) + return [{"id": id, "name": name} for id, name in query] + @with_feature_flags(TAGGING_SYSTEM=False) def test_tag_view_disabled(self): self.login("admin") @@ -31,3 +52,121 @@ def test_tag_view_enabled(self): self.login("admin") response = self.client.get("/tagview/tags/suggestions/") self.assertNotEqual(404, response.status_code) + + @with_feature_flags(TAGGING_SYSTEM=True) + def test_dataset_tagging(self): + """ + Test to make sure that when a new dataset is created, + a corresponding tag in the tagged_objects table + is created + """ + + # Test to make sure nothing is in the tagged_object table + self.assertEqual([], self.query_tagged_object_table()) + + # Create a dataset and add it to the db + test_dataset = SqlaTable( + table_name="foo", + schema=None, + owners=[], + database=get_main_database(), + sql=None, + extra='{"certification": 1}', + ) + db.session.add(test_dataset) + db.session.commit() + + # Test to make sure that a dataset tag was added to the tagged_object table + tags = self.query_tagged_object_table() + self.assertEqual(1, len(tags)) + self.assertEqual("type:dataset", tags[0].get("name")) + + # Cleanup the db + db.session.delete(test_dataset) + db.session.commit() + + @with_feature_flags(TAGGING_SYSTEM=True) + def test_chart_tagging(self): + """ + Test to make sure that when a new chart is created, + a corresponding tag in the tagged_objects table + is created + """ + + # Test to make sure nothing is in the tagged_object table + self.assertEqual([], self.query_tagged_object_table()) + + # Create a chart and add it to the db + test_chart = Slice( + slice_name="test_chart", + datasource_type=DatasourceType.TABLE, + viz_type="bubble", + datasource_id=1, + id=1, + ) + db.session.add(test_chart) + db.session.commit() + + # Test to make sure that a chart tag was added to the tagged_object table + tags = self.query_tagged_object_table() + self.assertEqual(1, len(tags)) + self.assertEqual("type:chart", tags[0].get("name")) + + # Cleanup the db + db.session.delete(test_chart) + db.session.commit() + + @with_feature_flags(TAGGING_SYSTEM=True) + def test_dashboard_tagging(self): + """ + Test to make sure that when a new dashboard is created, + a corresponding tag in the tagged_objects table + is created + """ + + # Test to make sure nothing is in the tagged_object table + self.assertEqual([], self.query_tagged_object_table()) + + # Create a dashboard and add it to the db + test_dashboard = Dashboard() + test_dashboard.dashboard_title = "test_dashboard" + test_dashboard.slug = "test_slug" + test_dashboard.slices = [] + test_dashboard.published = True + + db.session.add(test_dashboard) + db.session.commit() + + # Test to make sure that a dashboard tag was added to the tagged_object table + tags = self.query_tagged_object_table() + self.assertEqual(1, len(tags)) + self.assertEqual("type:dashboard", tags[0].get("name")) + + # Cleanup the db + db.session.delete(test_dashboard) + db.session.commit() + + @with_feature_flags(TAGGING_SYSTEM=True) + def test_saved_query_tagging(self): + """ + Test to make sure that when a new saved query is + created, a corresponding tag in the tagged_objects + table is created + """ + + # Test to make sure nothing is in the tagged_object table + self.assertEqual([], self.query_tagged_object_table()) + + # Create a saved query and add it to the db + test_saved_query = FavStar(user_id=1, class_name="slice", obj_id=1) + db.session.add(test_saved_query) + db.session.commit() + + # Test to make sure that a saved query tag was added to the tagged_object table + tags = self.query_tagged_object_table() + self.assertEqual(1, len(tags)) + self.assertEqual("favorited_by:1", tags[0].get("name")) + + # Cleanup the db + db.session.delete(test_saved_query) + db.session.commit() From d0e3ccad735eb2adba047d6bcde6e07b45c575b6 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 26 Aug 2022 12:06:25 -0400 Subject: [PATCH 06/34] Fixed integration tests which were failing --- tests/integration_tests/tagging_tests.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index a87b3f58e2cbd..c81432d0a855e 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -40,6 +40,11 @@ def query_tagged_object_table(self): .all() ) return [{"id": id, "name": name} for id, name in query] + + def cleanup_tables(self): + db.session.query(Tag).delete() + db.session.query(TaggedObject).delete() + db.session.commit() @with_feature_flags(TAGGING_SYSTEM=False) def test_tag_view_disabled(self): @@ -61,6 +66,9 @@ def test_dataset_tagging(self): is created """ + # Make sure the tagged_object and tag tables are empty + self.cleanup_tables() + # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -82,6 +90,7 @@ def test_dataset_tagging(self): self.assertEqual("type:dataset", tags[0].get("name")) # Cleanup the db + self.cleanup_tables() db.session.delete(test_dataset) db.session.commit() @@ -93,6 +102,9 @@ def test_chart_tagging(self): is created """ + # Make sure the tagged_object and tag tables are empty + self.cleanup_tables() + # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -113,6 +125,7 @@ def test_chart_tagging(self): self.assertEqual("type:chart", tags[0].get("name")) # Cleanup the db + self.cleanup_tables() db.session.delete(test_chart) db.session.commit() @@ -124,6 +137,9 @@ def test_dashboard_tagging(self): is created """ + # Make sure the tagged_object and tag tables are empty + self.cleanup_tables() + # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -143,6 +159,7 @@ def test_dashboard_tagging(self): self.assertEqual("type:dashboard", tags[0].get("name")) # Cleanup the db + self.cleanup_tables() db.session.delete(test_dashboard) db.session.commit() @@ -154,6 +171,9 @@ def test_saved_query_tagging(self): table is created """ + # Make sure the tagged_object and tag tables are empty + self.cleanup_tables() + # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -168,5 +188,6 @@ def test_saved_query_tagging(self): self.assertEqual("favorited_by:1", tags[0].get("name")) # Cleanup the db + self.cleanup_tables() db.session.delete(test_saved_query) db.session.commit() From 907e7bb7bdd456f91e4fac1e78ff333a6a64487d Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 26 Aug 2022 12:15:49 -0400 Subject: [PATCH 07/34] Ran pre-commit hook --- tests/integration_tests/tagging_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index c81432d0a855e..1f72074d22fff 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -40,7 +40,7 @@ def query_tagged_object_table(self): .all() ) return [{"id": id, "name": name} for id, name in query] - + def cleanup_tables(self): db.session.query(Tag).delete() db.session.query(TaggedObject).delete() From 0ee29eb0e4224286c8f3706ffbb5fbc799e07940 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 26 Aug 2022 13:28:00 -0400 Subject: [PATCH 08/34] Reverse which table has entries deleted first --- tests/integration_tests/tagging_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 1f72074d22fff..051c97c7047b6 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -42,8 +42,8 @@ def query_tagged_object_table(self): return [{"id": id, "name": name} for id, name in query] def cleanup_tables(self): - db.session.query(Tag).delete() db.session.query(TaggedObject).delete() + db.session.query(Tag).delete() db.session.commit() @with_feature_flags(TAGGING_SYSTEM=False) From bd43570d524dc7679b1c4da5aed7dae9916ce284 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 26 Aug 2022 13:41:03 -0400 Subject: [PATCH 09/34] Keep what is in the tag table, as without it, the tests do not work --- tests/integration_tests/tagging_tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 051c97c7047b6..adefe84273bd6 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -43,7 +43,6 @@ def query_tagged_object_table(self): def cleanup_tables(self): db.session.query(TaggedObject).delete() - db.session.query(Tag).delete() db.session.commit() @with_feature_flags(TAGGING_SYSTEM=False) From 69523041765b913981b23a877074087011115b5f Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 26 Aug 2022 13:56:13 -0400 Subject: [PATCH 10/34] Removing the code that clears the tagged_object table, going to try testing using another avenue --- tests/integration_tests/tagging_tests.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index adefe84273bd6..a87b3f58e2cbd 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -41,10 +41,6 @@ def query_tagged_object_table(self): ) return [{"id": id, "name": name} for id, name in query] - def cleanup_tables(self): - db.session.query(TaggedObject).delete() - db.session.commit() - @with_feature_flags(TAGGING_SYSTEM=False) def test_tag_view_disabled(self): self.login("admin") @@ -65,9 +61,6 @@ def test_dataset_tagging(self): is created """ - # Make sure the tagged_object and tag tables are empty - self.cleanup_tables() - # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -89,7 +82,6 @@ def test_dataset_tagging(self): self.assertEqual("type:dataset", tags[0].get("name")) # Cleanup the db - self.cleanup_tables() db.session.delete(test_dataset) db.session.commit() @@ -101,9 +93,6 @@ def test_chart_tagging(self): is created """ - # Make sure the tagged_object and tag tables are empty - self.cleanup_tables() - # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -124,7 +113,6 @@ def test_chart_tagging(self): self.assertEqual("type:chart", tags[0].get("name")) # Cleanup the db - self.cleanup_tables() db.session.delete(test_chart) db.session.commit() @@ -136,9 +124,6 @@ def test_dashboard_tagging(self): is created """ - # Make sure the tagged_object and tag tables are empty - self.cleanup_tables() - # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -158,7 +143,6 @@ def test_dashboard_tagging(self): self.assertEqual("type:dashboard", tags[0].get("name")) # Cleanup the db - self.cleanup_tables() db.session.delete(test_dashboard) db.session.commit() @@ -170,9 +154,6 @@ def test_saved_query_tagging(self): table is created """ - # Make sure the tagged_object and tag tables are empty - self.cleanup_tables() - # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -187,6 +168,5 @@ def test_saved_query_tagging(self): self.assertEqual("favorited_by:1", tags[0].get("name")) # Cleanup the db - self.cleanup_tables() db.session.delete(test_saved_query) db.session.commit() From 9babd50796728865f8a8353c68ba66bc9d7a4c4c Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Tue, 30 Aug 2022 14:29:19 -0400 Subject: [PATCH 11/34] Modified integration tests --- tests/integration_tests/tagging_tests.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index a87b3f58e2cbd..730e907549a02 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -31,15 +31,8 @@ class TestTagging(SupersetTestCase): def query_tagged_object_table(self): - query = ( - db.session.query(TaggedObject) - .join(Tag) - .with_entities(TaggedObject.tag_id, Tag.name) - .group_by(TaggedObject.tag_id, Tag.name) - .order_by(func.count().desc()) - .all() - ) - return [{"id": id, "name": name} for id, name in query] + query = (db.session.query(TaggedObject).all()) + return query @with_feature_flags(TAGGING_SYSTEM=False) def test_tag_view_disabled(self): @@ -79,7 +72,8 @@ def test_dataset_tagging(self): # Test to make sure that a dataset tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) - self.assertEqual("type:dataset", tags[0].get("name")) + self.assertEqual("ObjectTypes.dataset", str(tags[0].object_type)) + self.assertEqual(test_dataset.id, tags[0].object_id) # Cleanup the db db.session.delete(test_dataset) @@ -110,7 +104,8 @@ def test_chart_tagging(self): # Test to make sure that a chart tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) - self.assertEqual("type:chart", tags[0].get("name")) + self.assertEqual("ObjectTypes.chart", str(tags[0].object_type)) + self.assertEqual(test_chart.id, tags[0].object_id) # Cleanup the db db.session.delete(test_chart) @@ -140,7 +135,8 @@ def test_dashboard_tagging(self): # Test to make sure that a dashboard tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) - self.assertEqual("type:dashboard", tags[0].get("name")) + self.assertEqual("ObjectTypes.dashboard", str(tags[0].object_type)) + self.assertEqual(test_dashboard.id, tags[0].object_id) # Cleanup the db db.session.delete(test_dashboard) @@ -165,7 +161,8 @@ def test_saved_query_tagging(self): # Test to make sure that a saved query tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) - self.assertEqual("favorited_by:1", tags[0].get("name")) + self.assertEqual("ObjectTypes.chart", str(tags[0].object_type)) + self.assertEqual(test_saved_query.obj_id, tags[0].object_id) # Cleanup the db db.session.delete(test_saved_query) From b988747ca9b9beeca5e57f71929b908e8000f128 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Wed, 31 Aug 2022 14:48:09 -0400 Subject: [PATCH 12/34] Clear tagged_object table before running the tests --- tests/integration_tests/tagging_tests.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 730e907549a02..0b1eedeeda4bf 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -31,9 +31,13 @@ class TestTagging(SupersetTestCase): def query_tagged_object_table(self): - query = (db.session.query(TaggedObject).all()) + query = db.session.query(TaggedObject).all() return query + def clear_tagged_object_table(self): + db.session.query(TaggedObject).delete() + db.session.commit() + @with_feature_flags(TAGGING_SYSTEM=False) def test_tag_view_disabled(self): self.login("admin") @@ -54,6 +58,9 @@ def test_dataset_tagging(self): is created """ + # Remove all existing rows in the tagged_object table + self.clear_tagged_object_table() + # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -87,6 +94,9 @@ def test_chart_tagging(self): is created """ + # Remove all existing rows in the tagged_object table + self.clear_tagged_object_table() + # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -119,6 +129,9 @@ def test_dashboard_tagging(self): is created """ + # Remove all existing rows in the tagged_object table + self.clear_tagged_object_table() + # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -150,6 +163,9 @@ def test_saved_query_tagging(self): table is created """ + # Remove all existing rows in the tagged_object table + self.clear_tagged_object_table() + # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) From 5aa32eb6ba884c407010561028f348d8fdc00f95 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 1 Sep 2022 09:02:41 -0400 Subject: [PATCH 13/34] Run the sync_tags function so that the tags are applied to the new objects --- tests/integration_tests/tagging_tests.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 0b1eedeeda4bf..e99fd2db1e4c2 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -17,6 +17,7 @@ from sqlalchemy import func +from superset.cli.update import sync_tags from superset.connectors.sqla.models import SqlaTable from superset.extensions import db from superset.models.core import FavStar @@ -76,6 +77,9 @@ def test_dataset_tagging(self): db.session.add(test_dataset) db.session.commit() + # Run the sync tags command so that the new dataset is tagged + sync_tags() + # Test to make sure that a dataset tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) @@ -111,6 +115,9 @@ def test_chart_tagging(self): db.session.add(test_chart) db.session.commit() + # Run the sync tags command so that the new chart is tagged + sync_tags() + # Test to make sure that a chart tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) @@ -145,6 +152,9 @@ def test_dashboard_tagging(self): db.session.add(test_dashboard) db.session.commit() + # Run the sync tags command so that the new dashboard is tagged + sync_tags() + # Test to make sure that a dashboard tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) @@ -174,6 +184,9 @@ def test_saved_query_tagging(self): db.session.add(test_saved_query) db.session.commit() + # Run the sync tags command so that the new saved query is tagged + sync_tags() + # Test to make sure that a saved query tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) From 26a53f6ab1f08827b2f327686fd8e967399c9664 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 1 Sep 2022 11:16:44 -0400 Subject: [PATCH 14/34] Testing with integration tests --- tests/integration_tests/tagging_tests.py | 29 ++++++------------------ 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index e99fd2db1e4c2..db6616ae53f95 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -15,9 +15,6 @@ # specific language governing permissions and limitations # under the License. -from sqlalchemy import func - -from superset.cli.update import sync_tags from superset.connectors.sqla.models import SqlaTable from superset.extensions import db from superset.models.core import FavStar @@ -35,9 +32,9 @@ def query_tagged_object_table(self): query = db.session.query(TaggedObject).all() return query - def clear_tagged_object_table(self): - db.session.query(TaggedObject).delete() - db.session.commit() + # def clear_tagged_object_table(self): + # db.session.query(TaggedObject).delete() + # db.session.commit() @with_feature_flags(TAGGING_SYSTEM=False) def test_tag_view_disabled(self): @@ -60,7 +57,7 @@ def test_dataset_tagging(self): """ # Remove all existing rows in the tagged_object table - self.clear_tagged_object_table() + # self.clear_tagged_object_table() # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -77,9 +74,6 @@ def test_dataset_tagging(self): db.session.add(test_dataset) db.session.commit() - # Run the sync tags command so that the new dataset is tagged - sync_tags() - # Test to make sure that a dataset tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) @@ -99,7 +93,7 @@ def test_chart_tagging(self): """ # Remove all existing rows in the tagged_object table - self.clear_tagged_object_table() + # self.clear_tagged_object_table() # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -115,9 +109,6 @@ def test_chart_tagging(self): db.session.add(test_chart) db.session.commit() - # Run the sync tags command so that the new chart is tagged - sync_tags() - # Test to make sure that a chart tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) @@ -137,7 +128,7 @@ def test_dashboard_tagging(self): """ # Remove all existing rows in the tagged_object table - self.clear_tagged_object_table() + # self.clear_tagged_object_table() # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -152,9 +143,6 @@ def test_dashboard_tagging(self): db.session.add(test_dashboard) db.session.commit() - # Run the sync tags command so that the new dashboard is tagged - sync_tags() - # Test to make sure that a dashboard tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) @@ -174,7 +162,7 @@ def test_saved_query_tagging(self): """ # Remove all existing rows in the tagged_object table - self.clear_tagged_object_table() + # self.clear_tagged_object_table() # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -184,9 +172,6 @@ def test_saved_query_tagging(self): db.session.add(test_saved_query) db.session.commit() - # Run the sync tags command so that the new saved query is tagged - sync_tags() - # Test to make sure that a saved query tag was added to the tagged_object table tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) From dc0bfd94eede395dc3f3ba264ada365fd31da392 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 1 Sep 2022 13:54:34 -0400 Subject: [PATCH 15/34] Modifying tests --- tests/integration_tests/tagging_tests.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index db6616ae53f95..1b50fa9da1ee6 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -32,9 +32,9 @@ def query_tagged_object_table(self): query = db.session.query(TaggedObject).all() return query - # def clear_tagged_object_table(self): - # db.session.query(TaggedObject).delete() - # db.session.commit() + def clear_tagged_object_table(self): + db.session.query(TaggedObject).delete() + db.session.commit() @with_feature_flags(TAGGING_SYSTEM=False) def test_tag_view_disabled(self): @@ -57,7 +57,7 @@ def test_dataset_tagging(self): """ # Remove all existing rows in the tagged_object table - # self.clear_tagged_object_table() + self.clear_tagged_object_table() # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -93,7 +93,7 @@ def test_chart_tagging(self): """ # Remove all existing rows in the tagged_object table - # self.clear_tagged_object_table() + self.clear_tagged_object_table() # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -128,7 +128,7 @@ def test_dashboard_tagging(self): """ # Remove all existing rows in the tagged_object table - # self.clear_tagged_object_table() + self.clear_tagged_object_table() # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) @@ -162,7 +162,7 @@ def test_saved_query_tagging(self): """ # Remove all existing rows in the tagged_object table - # self.clear_tagged_object_table() + self.clear_tagged_object_table() # Test to make sure nothing is in the tagged_object table self.assertEqual([], self.query_tagged_object_table()) From bfb0aa6edf2529c64f98d1a30507b70080089743 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 2 Sep 2022 07:49:31 -0400 Subject: [PATCH 16/34] Updated the way we mock the feature flag --- tests/integration_tests/tagging_tests.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 1b50fa9da1ee6..df7bc53d2c94c 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. +from unittest import mock from superset.connectors.sqla.models import SqlaTable from superset.extensions import db from superset.models.core import FavStar @@ -48,7 +49,10 @@ def test_tag_view_enabled(self): response = self.client.get("/tagview/tags/suggestions/") self.assertNotEqual(404, response.status_code) - @with_feature_flags(TAGGING_SYSTEM=True) + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + TAGGING_SYSTEM=True, + ) def test_dataset_tagging(self): """ Test to make sure that when a new dataset is created, @@ -84,7 +88,10 @@ def test_dataset_tagging(self): db.session.delete(test_dataset) db.session.commit() - @with_feature_flags(TAGGING_SYSTEM=True) + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + TAGGING_SYSTEM=True, + ) def test_chart_tagging(self): """ Test to make sure that when a new chart is created, @@ -119,7 +126,10 @@ def test_chart_tagging(self): db.session.delete(test_chart) db.session.commit() - @with_feature_flags(TAGGING_SYSTEM=True) + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + TAGGING_SYSTEM=True, + ) def test_dashboard_tagging(self): """ Test to make sure that when a new dashboard is created, @@ -153,7 +163,10 @@ def test_dashboard_tagging(self): db.session.delete(test_dashboard) db.session.commit() - @with_feature_flags(TAGGING_SYSTEM=True) + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + TAGGING_SYSTEM=True, + ) def test_saved_query_tagging(self): """ Test to make sure that when a new saved query is From 8eb510d5b99ba14114333408c25f1a90794b1fa6 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 8 Sep 2022 09:19:28 -0400 Subject: [PATCH 17/34] Change where the SQLAlchemy event listeners are placed --- superset/connectors/sqla/models.py | 6 ----- superset/initialization/__init__.py | 31 ++++++++++++++++++++++++ superset/models/core.py | 6 ----- superset/models/dashboard.py | 6 ----- superset/models/slice.py | 6 ----- tests/integration_tests/tagging_tests.py | 21 ++++------------ 6 files changed, 36 insertions(+), 40 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 800726c3ff689..5e4fc9f7d0a42 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -2561,12 +2561,6 @@ def write_shadow_dataset( sa.event.listen(TableColumn, "after_update", SqlaTable.update_column) sa.event.listen(TableColumn, "after_delete", TableColumn.after_delete) -# events for updating tags -if is_feature_enabled("TAGGING_SYSTEM"): - sa.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) - sa.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) - sa.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) - RLSFilterRoles = Table( "rls_filter_roles", metadata, diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 12d3692ac9fd9..3ba4d3dc986a3 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -583,8 +583,39 @@ def configure_db_encrypt(self) -> None: encrypted_field_factory.init_app(self.superset_app) def setup_db(self) -> None: + with self.superset_app.app_context(): + import sqlalchemy as sqla + + from superset.connectors.sqla.models import SqlaTable + from superset.models.core import FavStar + from superset.models.dashboard import Dashboard + from superset.models.slice import Slice + from superset.models.tags import ( + ChartUpdater, + DashboardUpdater, + DatasetUpdater, + FavStarUpdater, + ) + db.init_app(self.superset_app) + # events for updating tags + if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): + sqla.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) + sqla.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) + sqla.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) + + sqla.event.listen(Slice, "after_insert", ChartUpdater.after_insert) + sqla.event.listen(Slice, "after_update", ChartUpdater.after_update) + sqla.event.listen(Slice, "after_delete", ChartUpdater.after_delete) + + sqla.event.listen(Dashboard, "after_insert", DashboardUpdater.after_insert) + sqla.event.listen(Dashboard, "after_update", DashboardUpdater.after_update) + sqla.event.listen(Dashboard, "after_delete", DashboardUpdater.after_delete) + + sqla.event.listen(FavStar, "after_insert", FavStarUpdater.after_insert) + sqla.event.listen(FavStar, "after_delete", FavStarUpdater.after_delete) + with self.superset_app.app_context(): pessimistic_connection_handling(db.engine) diff --git a/superset/models/core.py b/superset/models/core.py index 822191cb72ff2..58705a322d602 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -852,9 +852,3 @@ class FavStar(Model): # pylint: disable=too-few-public-methods class_name = Column(String(50)) obj_id = Column(Integer) dttm = Column(DateTime, default=datetime.utcnow) - - -# events for updating tags -if is_feature_enabled("TAGGING_SYSTEM"): - sqla.event.listen(FavStar, "after_insert", FavStarUpdater.after_insert) - sqla.event.listen(FavStar, "after_delete", FavStarUpdater.after_delete) diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index b8dbf37e7d6ff..e2a3a3c0a30f3 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -454,12 +454,6 @@ def id_or_slug_filter(id_or_slug: Union[int, str]) -> BinaryExpression: OnDashboardChange = Callable[[Mapper, Connection, Dashboard], Any] -# events for updating tags -if is_feature_enabled("TAGGING_SYSTEM"): - sqla.event.listen(Dashboard, "after_insert", DashboardUpdater.after_insert) - sqla.event.listen(Dashboard, "after_update", DashboardUpdater.after_update) - sqla.event.listen(Dashboard, "after_delete", DashboardUpdater.after_delete) - if is_feature_enabled("THUMBNAILS_SQLA_LISTENERS"): update_thumbnail: OnDashboardChange = lambda _, __, dash: dash.update_thumbnail() sqla.event.listen(Dashboard, "after_insert", update_thumbnail) diff --git a/superset/models/slice.py b/superset/models/slice.py index de0f3df59684f..8bac21492bcd6 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -367,12 +367,6 @@ def event_after_chart_changed( sqla.event.listen(Slice, "before_insert", set_related_perm) sqla.event.listen(Slice, "before_update", set_related_perm) -# events for updating tags -if is_feature_enabled("TAGGING_SYSTEM"): - sqla.event.listen(Slice, "after_insert", ChartUpdater.after_insert) - sqla.event.listen(Slice, "after_update", ChartUpdater.after_update) - sqla.event.listen(Slice, "after_delete", ChartUpdater.after_delete) - # events for updating tags if is_feature_enabled("THUMBNAILS_SQLA_LISTENERS"): sqla.event.listen(Slice, "after_insert", event_after_chart_changed) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index df7bc53d2c94c..c89feacc5d9bc 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -16,6 +16,7 @@ # under the License. from unittest import mock + from superset.connectors.sqla.models import SqlaTable from superset.extensions import db from superset.models.core import FavStar @@ -49,10 +50,7 @@ def test_tag_view_enabled(self): response = self.client.get("/tagview/tags/suggestions/") self.assertNotEqual(404, response.status_code) - @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", - TAGGING_SYSTEM=True, - ) + @with_feature_flags(TAGGING_SYSTEM=True) def test_dataset_tagging(self): """ Test to make sure that when a new dataset is created, @@ -88,10 +86,7 @@ def test_dataset_tagging(self): db.session.delete(test_dataset) db.session.commit() - @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", - TAGGING_SYSTEM=True, - ) + @with_feature_flags(TAGGING_SYSTEM=True) def test_chart_tagging(self): """ Test to make sure that when a new chart is created, @@ -126,10 +121,7 @@ def test_chart_tagging(self): db.session.delete(test_chart) db.session.commit() - @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", - TAGGING_SYSTEM=True, - ) + @with_feature_flags(TAGGING_SYSTEM=True) def test_dashboard_tagging(self): """ Test to make sure that when a new dashboard is created, @@ -163,10 +155,7 @@ def test_dashboard_tagging(self): db.session.delete(test_dashboard) db.session.commit() - @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", - TAGGING_SYSTEM=True, - ) + @with_feature_flags(TAGGING_SYSTEM=True) def test_saved_query_tagging(self): """ Test to make sure that when a new saved query is From fffc790c8a066dc4bd27491b95ae9121b35fc6f3 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 9 Sep 2022 08:19:01 -0400 Subject: [PATCH 18/34] Removing un-needed import --- superset/connectors/sqla/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 5e4fc9f7d0a42..a72d07f2d0741 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -110,7 +110,6 @@ clone_model, QueryResult, ) -from superset.models.tags import DatasetUpdater from superset.sql_parse import ( extract_table_references, ParsedQuery, From c4136df1f67bca324f824012304f41dc1d138e94 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 9 Sep 2022 08:43:47 -0400 Subject: [PATCH 19/34] Adding back code which does not appear to have been properly tracked in git --- superset/connectors/sqla/models.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index a72d07f2d0741..800726c3ff689 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -110,6 +110,7 @@ clone_model, QueryResult, ) +from superset.models.tags import DatasetUpdater from superset.sql_parse import ( extract_table_references, ParsedQuery, @@ -2560,6 +2561,12 @@ def write_shadow_dataset( sa.event.listen(TableColumn, "after_update", SqlaTable.update_column) sa.event.listen(TableColumn, "after_delete", TableColumn.after_delete) +# events for updating tags +if is_feature_enabled("TAGGING_SYSTEM"): + sa.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) + sa.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) + sa.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) + RLSFilterRoles = Table( "rls_filter_roles", metadata, From c8deabbed83e8ead414b18806dc5daa74882e615 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 9 Sep 2022 08:46:25 -0400 Subject: [PATCH 20/34] Removing code which does not appear to have been properly tracked in git --- superset/connectors/sqla/models.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 800726c3ff689..a72d07f2d0741 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -110,7 +110,6 @@ clone_model, QueryResult, ) -from superset.models.tags import DatasetUpdater from superset.sql_parse import ( extract_table_references, ParsedQuery, @@ -2561,12 +2560,6 @@ def write_shadow_dataset( sa.event.listen(TableColumn, "after_update", SqlaTable.update_column) sa.event.listen(TableColumn, "after_delete", TableColumn.after_delete) -# events for updating tags -if is_feature_enabled("TAGGING_SYSTEM"): - sa.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) - sa.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) - sa.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) - RLSFilterRoles = Table( "rls_filter_roles", metadata, From f9bb93cba737d223e210ea8758a20a65ae33b891 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Wed, 14 Sep 2022 08:06:47 -0400 Subject: [PATCH 21/34] Moving event listeners into their own function --- superset/initialization/__init__.py | 38 +++++++++++++++-------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 3ba4d3dc986a3..234840cf5a827 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -419,6 +419,7 @@ def init_app_in_ctx(self) -> None: self.configure_data_sources() self.configure_auth_provider() self.configure_async_queries() + self.configure_event_listeners() # Hook that provides administrators a handle on the Flask APP # after initialization @@ -583,6 +584,25 @@ def configure_db_encrypt(self) -> None: encrypted_field_factory.init_app(self.superset_app) def setup_db(self) -> None: + db.init_app(self.superset_app) + + with self.superset_app.app_context(): + pessimistic_connection_handling(db.engine) + + migrate.init_app(self.superset_app, db=db, directory=APP_DIR + "/migrations") + + def configure_wtf(self) -> None: + if self.config["WTF_CSRF_ENABLED"]: + csrf.init_app(self.superset_app) + csrf_exempt_list = self.config["WTF_CSRF_EXEMPT_LIST"] + for ex in csrf_exempt_list: + csrf.exempt(ex) + + def configure_async_queries(self) -> None: + if feature_flag_manager.is_feature_enabled("GLOBAL_ASYNC_QUERIES"): + async_query_manager.init_app(self.superset_app) + + def configure_event_listeners(self) -> None: with self.superset_app.app_context(): import sqlalchemy as sqla @@ -597,8 +617,6 @@ def setup_db(self) -> None: FavStarUpdater, ) - db.init_app(self.superset_app) - # events for updating tags if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): sqla.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) @@ -616,22 +634,6 @@ def setup_db(self) -> None: sqla.event.listen(FavStar, "after_insert", FavStarUpdater.after_insert) sqla.event.listen(FavStar, "after_delete", FavStarUpdater.after_delete) - with self.superset_app.app_context(): - pessimistic_connection_handling(db.engine) - - migrate.init_app(self.superset_app, db=db, directory=APP_DIR + "/migrations") - - def configure_wtf(self) -> None: - if self.config["WTF_CSRF_ENABLED"]: - csrf.init_app(self.superset_app) - csrf_exempt_list = self.config["WTF_CSRF_EXEMPT_LIST"] - for ex in csrf_exempt_list: - csrf.exempt(ex) - - def configure_async_queries(self) -> None: - if feature_flag_manager.is_feature_enabled("GLOBAL_ASYNC_QUERIES"): - async_query_manager.init_app(self.superset_app) - def register_blueprints(self) -> None: for bp in self.config["BLUEPRINTS"]: try: From 0b51f50b6cb474ad992a78c60f48b990f8cd2999 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 15 Sep 2022 10:10:42 -0400 Subject: [PATCH 22/34] Created functions to register and unregister event listeners, and created a new fixture to use in the integration tests --- superset/initialization/__init__.py | 33 ----------- superset/models/core.py | 1 - superset/models/dashboard.py | 1 - superset/models/slice.py | 2 - superset/models/sql_lab.py | 7 --- superset/tags/core.py | 74 ++++++++++++++++++++++++ tests/integration_tests/fixtures/tags.py | 29 ++++++++++ tests/integration_tests/tagging_tests.py | 54 +++++++++++++++-- 8 files changed, 151 insertions(+), 50 deletions(-) create mode 100644 superset/tags/core.py create mode 100644 tests/integration_tests/fixtures/tags.py diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 234840cf5a827..12d3692ac9fd9 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -419,7 +419,6 @@ def init_app_in_ctx(self) -> None: self.configure_data_sources() self.configure_auth_provider() self.configure_async_queries() - self.configure_event_listeners() # Hook that provides administrators a handle on the Flask APP # after initialization @@ -602,38 +601,6 @@ def configure_async_queries(self) -> None: if feature_flag_manager.is_feature_enabled("GLOBAL_ASYNC_QUERIES"): async_query_manager.init_app(self.superset_app) - def configure_event_listeners(self) -> None: - with self.superset_app.app_context(): - import sqlalchemy as sqla - - from superset.connectors.sqla.models import SqlaTable - from superset.models.core import FavStar - from superset.models.dashboard import Dashboard - from superset.models.slice import Slice - from superset.models.tags import ( - ChartUpdater, - DashboardUpdater, - DatasetUpdater, - FavStarUpdater, - ) - - # events for updating tags - if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): - sqla.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) - sqla.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) - sqla.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) - - sqla.event.listen(Slice, "after_insert", ChartUpdater.after_insert) - sqla.event.listen(Slice, "after_update", ChartUpdater.after_update) - sqla.event.listen(Slice, "after_delete", ChartUpdater.after_delete) - - sqla.event.listen(Dashboard, "after_insert", DashboardUpdater.after_insert) - sqla.event.listen(Dashboard, "after_update", DashboardUpdater.after_update) - sqla.event.listen(Dashboard, "after_delete", DashboardUpdater.after_delete) - - sqla.event.listen(FavStar, "after_insert", FavStarUpdater.after_insert) - sqla.event.listen(FavStar, "after_delete", FavStarUpdater.after_delete) - def register_blueprints(self) -> None: for bp in self.config["BLUEPRINTS"]: try: diff --git a/superset/models/core.py b/superset/models/core.py index 58705a322d602..000ca75889e46 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -59,7 +59,6 @@ from superset.db_engine_specs.base import MetricType, TimeGrain from superset.extensions import cache_manager, encrypted_field_factory, security_manager from superset.models.helpers import AuditMixinNullable, ImportExportMixin -from superset.models.tags import FavStarUpdater from superset.result_set import SupersetResultSet from superset.utils import cache as cache_util, core as utils from superset.utils.core import get_username diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index e2a3a3c0a30f3..57567e61641c4 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -53,7 +53,6 @@ from superset.models.filter_set import FilterSet from superset.models.helpers import AuditMixinNullable, ImportExportMixin from superset.models.slice import Slice -from superset.models.tags import DashboardUpdater from superset.models.user_attributes import UserAttribute from superset.tasks.thumbnails import cache_dashboard_thumbnail from superset.utils import core as utils diff --git a/superset/models/slice.py b/superset/models/slice.py index 8bac21492bcd6..d644e7b7472ee 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -42,7 +42,6 @@ from superset import db, is_feature_enabled, security_manager from superset.legacy import update_time_range from superset.models.helpers import AuditMixinNullable, ImportExportMixin -from superset.models.tags import ChartUpdater from superset.tasks.thumbnails import cache_chart_thumbnail from superset.utils import core as utils from superset.utils.hashing import md5_sha_from_str @@ -367,7 +366,6 @@ def event_after_chart_changed( sqla.event.listen(Slice, "before_insert", set_related_perm) sqla.event.listen(Slice, "before_update", set_related_perm) -# events for updating tags if is_feature_enabled("THUMBNAILS_SQLA_LISTENERS"): sqla.event.listen(Slice, "after_insert", event_after_chart_changed) sqla.event.listen(Slice, "after_update", event_after_chart_changed) diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py index d12af490818d5..408bc708dfb81 100644 --- a/superset/models/sql_lab.py +++ b/superset/models/sql_lab.py @@ -49,7 +49,6 @@ ExtraJSONMixin, ImportExportMixin, ) -from superset.models.tags import QueryUpdater from superset.sql_parse import CtasMethod, ParsedQuery, Table from superset.sqllab.limiting_factor import LimitingFactor from superset.superset_typing import ResultSetColumnType @@ -509,9 +508,3 @@ def to_dict(self) -> Dict[str, Any]: "description": description, "expanded": self.expanded, } - - -# events for updating tags -sqla.event.listen(SavedQuery, "after_insert", QueryUpdater.after_insert) -sqla.event.listen(SavedQuery, "after_update", QueryUpdater.after_update) -sqla.event.listen(SavedQuery, "after_delete", QueryUpdater.after_delete) diff --git a/superset/tags/core.py b/superset/tags/core.py new file mode 100644 index 0000000000000..bed4bb1a91e54 --- /dev/null +++ b/superset/tags/core.py @@ -0,0 +1,74 @@ +# 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 sqlalchemy as sqla + +from superset import is_feature_enabled +from superset.connectors.sqla.models import SqlaTable +from superset.models.core import FavStar +from superset.models.dashboard import Dashboard +from superset.models.slice import Slice +from superset.models.sql_lab import SavedQuery +from superset.models.tags import ( + ChartUpdater, + DashboardUpdater, + DatasetUpdater, + FavStarUpdater, + QueryUpdater, +) + +def register_sqla_event_listeners() -> None: + if is_feature_enabled("TAGGING_SYSTEM"): + sqla.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) + sqla.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) + sqla.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) + + sqla.event.listen(Slice, "after_insert", ChartUpdater.after_insert) + sqla.event.listen(Slice, "after_update", ChartUpdater.after_update) + sqla.event.listen(Slice, "after_delete", ChartUpdater.after_delete) + + sqla.event.listen(Dashboard, "after_insert", DashboardUpdater.after_insert) + sqla.event.listen(Dashboard, "after_update", DashboardUpdater.after_update) + sqla.event.listen(Dashboard, "after_delete", DashboardUpdater.after_delete) + + sqla.event.listen(FavStar, "after_insert", FavStarUpdater.after_insert) + sqla.event.listen(FavStar, "after_delete", FavStarUpdater.after_delete) + + sqla.event.listen(SavedQuery, "after_insert", QueryUpdater.after_insert) + sqla.event.listen(SavedQuery, "after_update", QueryUpdater.after_update) + sqla.event.listen(SavedQuery, "after_delete", QueryUpdater.after_delete) + +def clear_sqla_event_listeners() -> None: + if is_feature_enabled("TAGGING_SYSTEM"): + sqla.event.remove(SqlaTable, "after_insert", DatasetUpdater.after_insert) + sqla.event.remove(SqlaTable, "after_update", DatasetUpdater.after_update) + sqla.event.remove(SqlaTable, "after_delete", DatasetUpdater.after_delete) + + sqla.event.remove(Slice, "after_insert", ChartUpdater.after_insert) + sqla.event.remove(Slice, "after_update", ChartUpdater.after_update) + sqla.event.remove(Slice, "after_delete", ChartUpdater.after_delete) + + sqla.event.remove(Dashboard, "after_insert", DashboardUpdater.after_insert) + sqla.event.remove(Dashboard, "after_update", DashboardUpdater.after_update) + sqla.event.remove(Dashboard, "after_delete", DashboardUpdater.after_delete) + + sqla.event.remove(FavStar, "after_insert", FavStarUpdater.after_insert) + sqla.event.remove(FavStar, "after_delete", FavStarUpdater.after_delete) + + sqla.event.remove(SavedQuery, "after_insert", QueryUpdater.after_insert) + sqla.event.remove(SavedQuery, "after_update", QueryUpdater.after_update) + sqla.event.remove(SavedQuery, "after_delete", QueryUpdater.after_delete) \ No newline at end of file diff --git a/tests/integration_tests/fixtures/tags.py b/tests/integration_tests/fixtures/tags.py new file mode 100644 index 0000000000000..36609f47bee9e --- /dev/null +++ b/tests/integration_tests/fixtures/tags.py @@ -0,0 +1,29 @@ +# 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 pytest +from superset.tags.core import clear_sqla_event_listeners, register_sqla_event_listeners +from tests.integration_tests.test_app import app + +@pytest.fixture +def tag_sqla_event_listeners(): + with app.app_context(): + app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] = True + register_sqla_event_listeners() + yield + app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] = False + clear_sqla_event_listeners() \ No newline at end of file diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index c89feacc5d9bc..29196dc80c462 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -17,16 +17,20 @@ from unittest import mock +import pytest + from superset.connectors.sqla.models import SqlaTable from superset.extensions import db from superset.models.core import FavStar from superset.models.dashboard import Dashboard from superset.models.slice import Slice -from superset.models.tags import Tag, TaggedObject +from superset.models.sql_lab import SavedQuery +from superset.models.tags import TaggedObject from superset.utils.core import DatasourceType from superset.utils.database import get_main_database from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.conftest import with_feature_flags +from tests.integration_tests.fixtures.tags import tag_sqla_event_listeners class TestTagging(SupersetTestCase): @@ -50,7 +54,7 @@ def test_tag_view_enabled(self): response = self.client.get("/tagview/tags/suggestions/") self.assertNotEqual(404, response.status_code) - @with_feature_flags(TAGGING_SYSTEM=True) + @pytest.mark.usefixtures("tag_sqla_event_listeners") def test_dataset_tagging(self): """ Test to make sure that when a new dataset is created, @@ -86,7 +90,7 @@ def test_dataset_tagging(self): db.session.delete(test_dataset) db.session.commit() - @with_feature_flags(TAGGING_SYSTEM=True) + @pytest.mark.usefixtures("tag_sqla_event_listeners") def test_chart_tagging(self): """ Test to make sure that when a new chart is created, @@ -121,7 +125,7 @@ def test_chart_tagging(self): db.session.delete(test_chart) db.session.commit() - @with_feature_flags(TAGGING_SYSTEM=True) + @pytest.mark.usefixtures("tag_sqla_event_listeners") def test_dashboard_tagging(self): """ Test to make sure that when a new dashboard is created, @@ -155,7 +159,7 @@ def test_dashboard_tagging(self): db.session.delete(test_dashboard) db.session.commit() - @with_feature_flags(TAGGING_SYSTEM=True) + @pytest.mark.usefixtures("tag_sqla_event_listeners") def test_saved_query_tagging(self): """ Test to make sure that when a new saved query is @@ -170,12 +174,50 @@ def test_saved_query_tagging(self): self.assertEqual([], self.query_tagged_object_table()) # Create a saved query and add it to the db - test_saved_query = FavStar(user_id=1, class_name="slice", obj_id=1) + test_saved_query = SavedQuery(id=1, label="test saved query") db.session.add(test_saved_query) db.session.commit() # Test to make sure that a saved query tag was added to the tagged_object table tags = self.query_tagged_object_table() + + self.assertEqual(2, len(tags)) + + self.assertEqual("ObjectTypes.query", str(tags[0].object_type)) + self.assertEqual("owner:None", str(tags[0].tag.name)) + self.assertEqual("TagTypes.owner", str(tags[0].tag.type)) + self.assertEqual(test_saved_query.id, tags[0].object_id) + + self.assertEqual("ObjectTypes.query", str(tags[1].object_type)) + self.assertEqual("type:query", str(tags[1].tag.name)) + self.assertEqual("TagTypes.type", str(tags[1].tag.type)) + self.assertEqual(test_saved_query.id, tags[1].object_id) + + # Cleanup the db + db.session.delete(test_saved_query) + db.session.commit() + + @with_feature_flags(TAGGING_SYSTEM=True) + def test_favorite_tagging(self): + """ + Test to make sure that when a new favorite object is + created, a corresponding tag in the tagged_objects + table is created + """ + + # Remove all existing rows in the tagged_object table + self.clear_tagged_object_table() + + # Test to make sure nothing is in the tagged_object table + self.assertEqual([], self.query_tagged_object_table()) + + # Create a favorited object and add it to the db + test_saved_query = FavStar(user_id=1, class_name="slice", obj_id=1) + db.session.add(test_saved_query) + db.session.commit() + + # Test to make sure that a favorited object tag was added to the tagged_object table + tags = self.query_tagged_object_table() self.assertEqual(1, len(tags)) self.assertEqual("ObjectTypes.chart", str(tags[0].object_type)) self.assertEqual(test_saved_query.obj_id, tags[0].object_id) From aaae5aa34f80ee10cc30d391eabc27551871a26c Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 15 Sep 2022 11:14:27 -0400 Subject: [PATCH 23/34] Moving and renaming file from superset.models.tags.py -> superset.tags.models.py --- superset/common/tags.py | 2 +- .../2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py | 2 +- superset/tags/core.py | 6 ++++-- superset/{models/tags.py => tags/models.py} | 0 superset/tasks/cache.py | 2 +- superset/utils/url_map_converters.py | 2 +- superset/views/tags.py | 2 +- tests/integration_tests/fixtures/tags.py | 4 +++- tests/integration_tests/strategy_tests.py | 2 +- tests/integration_tests/tagging_tests.py | 2 +- 10 files changed, 14 insertions(+), 10 deletions(-) rename superset/{models/tags.py => tags/models.py} (100%) diff --git a/superset/common/tags.py b/superset/common/tags.py index 7f4ca10161e84..d85a33b84e140 100644 --- a/superset/common/tags.py +++ b/superset/common/tags.py @@ -21,7 +21,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.sql import and_, func, join, literal, select -from superset.models.tags import ObjectTypes, TagTypes +from superset.tags.models import ObjectTypes, TagTypes def add_types_to_charts( diff --git a/superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py b/superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py index 8a1a5f989ec1b..0179ba7d0348e 100644 --- a/superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py +++ b/superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py @@ -33,7 +33,7 @@ from sqlalchemy import Column, DateTime, Enum, ForeignKey, Integer, String from sqlalchemy.ext.declarative import declarative_base, declared_attr -from superset.models.tags import ObjectTypes, TagTypes +from superset.tags.models import ObjectTypes, TagTypes from superset.utils.core import get_user_id Base = declarative_base() diff --git a/superset/tags/core.py b/superset/tags/core.py index bed4bb1a91e54..0180658bf873c 100644 --- a/superset/tags/core.py +++ b/superset/tags/core.py @@ -23,7 +23,7 @@ from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import SavedQuery -from superset.models.tags import ( +from superset.tags.models import ( ChartUpdater, DashboardUpdater, DatasetUpdater, @@ -31,6 +31,7 @@ QueryUpdater, ) + def register_sqla_event_listeners() -> None: if is_feature_enabled("TAGGING_SYSTEM"): sqla.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) @@ -52,6 +53,7 @@ def register_sqla_event_listeners() -> None: sqla.event.listen(SavedQuery, "after_update", QueryUpdater.after_update) sqla.event.listen(SavedQuery, "after_delete", QueryUpdater.after_delete) + def clear_sqla_event_listeners() -> None: if is_feature_enabled("TAGGING_SYSTEM"): sqla.event.remove(SqlaTable, "after_insert", DatasetUpdater.after_insert) @@ -71,4 +73,4 @@ def clear_sqla_event_listeners() -> None: sqla.event.remove(SavedQuery, "after_insert", QueryUpdater.after_insert) sqla.event.remove(SavedQuery, "after_update", QueryUpdater.after_update) - sqla.event.remove(SavedQuery, "after_delete", QueryUpdater.after_delete) \ No newline at end of file + sqla.event.remove(SavedQuery, "after_delete", QueryUpdater.after_delete) diff --git a/superset/models/tags.py b/superset/tags/models.py similarity index 100% rename from superset/models/tags.py rename to superset/tags/models.py diff --git a/superset/tasks/cache.py b/superset/tasks/cache.py index 137ec068e8843..0bda1e70845e8 100644 --- a/superset/tasks/cache.py +++ b/superset/tasks/cache.py @@ -28,7 +28,7 @@ from superset.models.core import Log from superset.models.dashboard import Dashboard from superset.models.slice import Slice -from superset.models.tags import Tag, TaggedObject +from superset.tags.models import Tag, TaggedObject from superset.utils.date_parser import parse_human_datetime from superset.utils.machine_auth import MachineAuthProvider diff --git a/superset/utils/url_map_converters.py b/superset/utils/url_map_converters.py index c6a14f3fd8048..c5eaf3b359f06 100644 --- a/superset/utils/url_map_converters.py +++ b/superset/utils/url_map_converters.py @@ -18,7 +18,7 @@ from werkzeug.routing import BaseConverter, Map -from superset.models.tags import ObjectTypes +from superset.tags.models import ObjectTypes class RegexConverter(BaseConverter): diff --git a/superset/views/tags.py b/superset/views/tags.py index 838bdb73edab9..985d26179fe28 100644 --- a/superset/views/tags.py +++ b/superset/views/tags.py @@ -33,8 +33,8 @@ from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import SavedQuery -from superset.models.tags import ObjectTypes, Tag, TaggedObject, TagTypes from superset.superset_typing import FlaskResponse +from superset.tags.models import ObjectTypes, Tag, TaggedObject, TagTypes from .base import BaseSupersetView, json_success diff --git a/tests/integration_tests/fixtures/tags.py b/tests/integration_tests/fixtures/tags.py index 36609f47bee9e..41bd909e094cb 100644 --- a/tests/integration_tests/fixtures/tags.py +++ b/tests/integration_tests/fixtures/tags.py @@ -16,9 +16,11 @@ # under the License. import pytest + from superset.tags.core import clear_sqla_event_listeners, register_sqla_event_listeners from tests.integration_tests.test_app import app + @pytest.fixture def tag_sqla_event_listeners(): with app.app_context(): @@ -26,4 +28,4 @@ def tag_sqla_event_listeners(): register_sqla_event_listeners() yield app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] = False - clear_sqla_event_listeners() \ No newline at end of file + clear_sqla_event_listeners() diff --git a/tests/integration_tests/strategy_tests.py b/tests/integration_tests/strategy_tests.py index f31489bb04569..e54ae865e3c15 100644 --- a/tests/integration_tests/strategy_tests.py +++ b/tests/integration_tests/strategy_tests.py @@ -35,7 +35,7 @@ from superset import db from superset.models.core import Log -from superset.models.tags import get_tag, ObjectTypes, TaggedObject, TagTypes +from superset.tags.models import get_tag, ObjectTypes, TaggedObject, TagTypes from superset.tasks.cache import ( DashboardTagsStrategy, TopNDashboardsStrategy, diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 29196dc80c462..95d77249d15d7 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -25,7 +25,7 @@ from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import SavedQuery -from superset.models.tags import TaggedObject +from superset.tags.models import TaggedObject from superset.utils.core import DatasourceType from superset.utils.database import get_main_database from tests.integration_tests.base_tests import SupersetTestCase From 4ad8d8a39e10523039a8ff716ff15a90e9e04d1d Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 15 Sep 2022 11:17:46 -0400 Subject: [PATCH 24/34] Added the 'Query' object to some of the methods where it was required --- superset/tags/models.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/superset/tags/models.py b/superset/tags/models.py index 475207ebf6027..04d1345d2d0d2 100644 --- a/superset/tags/models.py +++ b/superset/tags/models.py @@ -119,7 +119,7 @@ class ObjectUpdater: @classmethod def get_owners_ids( - cls, target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"] + cls, target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"] ) -> List[int]: raise NotImplementedError("Subclass should implement `get_owners_ids`") @@ -127,7 +127,7 @@ def get_owners_ids( def _add_owners( cls, session: Session, - target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"], + target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"], ) -> None: for owner_id in cls.get_owners_ids(target): name = "owner:{0}".format(owner_id) @@ -142,7 +142,7 @@ def after_insert( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"], + target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"], ) -> None: session = Session(bind=connection) @@ -163,7 +163,7 @@ def after_update( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"], + target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"], ) -> None: session = Session(bind=connection) @@ -192,7 +192,7 @@ def after_delete( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice", "SqlaTable"], + target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"], ) -> None: session = Session(bind=connection) From 33ee25b080c3be69a11e2693ad1a320870233600 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 15 Sep 2022 11:48:24 -0400 Subject: [PATCH 25/34] Added a test to verify that if the 'TAGGING_SYSTEM' flag is set to False, you can no longer tag objects --- tests/integration_tests/tagging_tests.py | 62 +++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 95d77249d15d7..9029d5976c2c8 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -197,7 +197,7 @@ def test_saved_query_tagging(self): db.session.delete(test_saved_query) db.session.commit() - @with_feature_flags(TAGGING_SYSTEM=True) + @pytest.mark.usefixtures("tag_sqla_event_listeners") def test_favorite_tagging(self): """ Test to make sure that when a new favorite object is @@ -225,3 +225,63 @@ def test_favorite_tagging(self): # Cleanup the db db.session.delete(test_saved_query) db.session.commit() + + @with_feature_flags(TAGGING_SYSTEM=False) + def test_tagging_system(self): + """ + Test to make sure that when the TAGGING_SYSTEM + feature flag is false, that no tags are created + """ + + # Remove all existing rows in the tagged_object table + self.clear_tagged_object_table() + + # Test to make sure nothing is in the tagged_object table + self.assertEqual([], self.query_tagged_object_table()) + + # Create a dataset and add it to the db + test_dataset = SqlaTable( + table_name="foo", + schema=None, + owners=[], + database=get_main_database(), + sql=None, + extra='{"certification": 1}', + ) + + # Create a chart and add it to the db + test_chart = Slice( + slice_name="test_chart", + datasource_type=DatasourceType.TABLE, + viz_type="bubble", + datasource_id=1, + id=1, + ) + + # Create a dashboard and add it to the db + test_dashboard = Dashboard() + test_dashboard.dashboard_title = "test_dashboard" + test_dashboard.slug = "test_slug" + test_dashboard.slices = [] + test_dashboard.published = True + + # Create a saved query and add it to the db + test_saved_query = SavedQuery(id=1, label="test saved query") + + # Create a favorited object and add it to the db + test_saved_query = FavStar(user_id=1, class_name="slice", obj_id=1) + + db.session.add(test_dataset) + db.session.add(test_chart) + db.session.add(test_dashboard) + db.session.add(test_saved_query) + db.session.add(test_saved_query) + db.session.commit() + + # Test to make sure that no tags were added to the tagged_object table + tags = self.query_tagged_object_table() + self.assertEqual(0, len(tags)) + + # Cleanup the db + db.session.delete(test_dashboard) + db.session.commit() From 2fdedfaac8c9130882f5102a2d8c4eef6cad2a5b Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 16 Sep 2022 09:01:03 -0400 Subject: [PATCH 26/34] Moved import statements and where the feature flag check occurs --- superset/initialization/__init__.py | 4 ++ superset/tags/core.py | 103 +++++++++++++++------------- 2 files changed, 61 insertions(+), 46 deletions(-) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 12d3692ac9fd9..598cf94e055e0 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -49,6 +49,7 @@ ) from superset.security import SupersetSecurityManager from superset.superset_typing import FlaskResponse +from superset.tags.core import register_sqla_event_listeners from superset.utils.core import pessimistic_connection_handling from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value @@ -426,6 +427,9 @@ def init_app_in_ctx(self) -> None: if flask_app_mutator: flask_app_mutator(self.superset_app) + if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): + register_sqla_event_listeners() + self.init_views() def check_secret_key(self) -> None: diff --git a/superset/tags/core.py b/superset/tags/core.py index 0180658bf873c..c92d789e3adef 100644 --- a/superset/tags/core.py +++ b/superset/tags/core.py @@ -15,62 +15,73 @@ # specific language governing permissions and limitations # under the License. -import sqlalchemy as sqla - -from superset import is_feature_enabled -from superset.connectors.sqla.models import SqlaTable -from superset.models.core import FavStar -from superset.models.dashboard import Dashboard -from superset.models.slice import Slice -from superset.models.sql_lab import SavedQuery -from superset.tags.models import ( - ChartUpdater, - DashboardUpdater, - DatasetUpdater, - FavStarUpdater, - QueryUpdater, -) +def register_sqla_event_listeners() -> None: + import sqlalchemy as sqla + from superset.connectors.sqla.models import SqlaTable + from superset.models.core import FavStar + from superset.models.dashboard import Dashboard + from superset.models.slice import Slice + from superset.models.sql_lab import SavedQuery + from superset.tags.models import ( + ChartUpdater, + DashboardUpdater, + DatasetUpdater, + FavStarUpdater, + QueryUpdater, + ) -def register_sqla_event_listeners() -> None: - if is_feature_enabled("TAGGING_SYSTEM"): - sqla.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) - sqla.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) - sqla.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) + sqla.event.listen(SqlaTable, "after_insert", DatasetUpdater.after_insert) + sqla.event.listen(SqlaTable, "after_update", DatasetUpdater.after_update) + sqla.event.listen(SqlaTable, "after_delete", DatasetUpdater.after_delete) - sqla.event.listen(Slice, "after_insert", ChartUpdater.after_insert) - sqla.event.listen(Slice, "after_update", ChartUpdater.after_update) - sqla.event.listen(Slice, "after_delete", ChartUpdater.after_delete) + sqla.event.listen(Slice, "after_insert", ChartUpdater.after_insert) + sqla.event.listen(Slice, "after_update", ChartUpdater.after_update) + sqla.event.listen(Slice, "after_delete", ChartUpdater.after_delete) - sqla.event.listen(Dashboard, "after_insert", DashboardUpdater.after_insert) - sqla.event.listen(Dashboard, "after_update", DashboardUpdater.after_update) - sqla.event.listen(Dashboard, "after_delete", DashboardUpdater.after_delete) + sqla.event.listen(Dashboard, "after_insert", DashboardUpdater.after_insert) + sqla.event.listen(Dashboard, "after_update", DashboardUpdater.after_update) + sqla.event.listen(Dashboard, "after_delete", DashboardUpdater.after_delete) - sqla.event.listen(FavStar, "after_insert", FavStarUpdater.after_insert) - sqla.event.listen(FavStar, "after_delete", FavStarUpdater.after_delete) + sqla.event.listen(FavStar, "after_insert", FavStarUpdater.after_insert) + sqla.event.listen(FavStar, "after_delete", FavStarUpdater.after_delete) - sqla.event.listen(SavedQuery, "after_insert", QueryUpdater.after_insert) - sqla.event.listen(SavedQuery, "after_update", QueryUpdater.after_update) - sqla.event.listen(SavedQuery, "after_delete", QueryUpdater.after_delete) + sqla.event.listen(SavedQuery, "after_insert", QueryUpdater.after_insert) + sqla.event.listen(SavedQuery, "after_update", QueryUpdater.after_update) + sqla.event.listen(SavedQuery, "after_delete", QueryUpdater.after_delete) def clear_sqla_event_listeners() -> None: - if is_feature_enabled("TAGGING_SYSTEM"): - sqla.event.remove(SqlaTable, "after_insert", DatasetUpdater.after_insert) - sqla.event.remove(SqlaTable, "after_update", DatasetUpdater.after_update) - sqla.event.remove(SqlaTable, "after_delete", DatasetUpdater.after_delete) + import sqlalchemy as sqla + + from superset.connectors.sqla.models import SqlaTable + from superset.models.core import FavStar + from superset.models.dashboard import Dashboard + from superset.models.slice import Slice + from superset.models.sql_lab import SavedQuery + from superset.tags.models import ( + ChartUpdater, + DashboardUpdater, + DatasetUpdater, + FavStarUpdater, + QueryUpdater, + ) + + sqla.event.remove(SqlaTable, "after_insert", DatasetUpdater.after_insert) + sqla.event.remove(SqlaTable, "after_update", DatasetUpdater.after_update) + sqla.event.remove(SqlaTable, "after_delete", DatasetUpdater.after_delete) - sqla.event.remove(Slice, "after_insert", ChartUpdater.after_insert) - sqla.event.remove(Slice, "after_update", ChartUpdater.after_update) - sqla.event.remove(Slice, "after_delete", ChartUpdater.after_delete) + sqla.event.remove(Slice, "after_insert", ChartUpdater.after_insert) + sqla.event.remove(Slice, "after_update", ChartUpdater.after_update) + sqla.event.remove(Slice, "after_delete", ChartUpdater.after_delete) - sqla.event.remove(Dashboard, "after_insert", DashboardUpdater.after_insert) - sqla.event.remove(Dashboard, "after_update", DashboardUpdater.after_update) - sqla.event.remove(Dashboard, "after_delete", DashboardUpdater.after_delete) + sqla.event.remove(Dashboard, "after_insert", DashboardUpdater.after_insert) + sqla.event.remove(Dashboard, "after_update", DashboardUpdater.after_update) + sqla.event.remove(Dashboard, "after_delete", DashboardUpdater.after_delete) - sqla.event.remove(FavStar, "after_insert", FavStarUpdater.after_insert) - sqla.event.remove(FavStar, "after_delete", FavStarUpdater.after_delete) + sqla.event.remove(FavStar, "after_insert", FavStarUpdater.after_insert) + sqla.event.remove(FavStar, "after_delete", FavStarUpdater.after_delete) - sqla.event.remove(SavedQuery, "after_insert", QueryUpdater.after_insert) - sqla.event.remove(SavedQuery, "after_update", QueryUpdater.after_update) - sqla.event.remove(SavedQuery, "after_delete", QueryUpdater.after_delete) + sqla.event.remove(SavedQuery, "after_insert", QueryUpdater.after_insert) + sqla.event.remove(SavedQuery, "after_update", QueryUpdater.after_update) + sqla.event.remove(SavedQuery, "after_delete", QueryUpdater.after_delete) From a94cacaa765096ba8a3f6d6ba5edb1e8e7d4a1bf Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 16 Sep 2022 09:03:07 -0400 Subject: [PATCH 27/34] Ran pre-commit hook --- superset/tags/core.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset/tags/core.py b/superset/tags/core.py index c92d789e3adef..f1f832c7a5509 100644 --- a/superset/tags/core.py +++ b/superset/tags/core.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. + def register_sqla_event_listeners() -> None: import sqlalchemy as sqla @@ -66,7 +67,7 @@ def clear_sqla_event_listeners() -> None: FavStarUpdater, QueryUpdater, ) - + sqla.event.remove(SqlaTable, "after_insert", DatasetUpdater.after_insert) sqla.event.remove(SqlaTable, "after_update", DatasetUpdater.after_update) sqla.event.remove(SqlaTable, "after_delete", DatasetUpdater.after_delete) From fd9bd21f5884d38920a9d6edeb3a0b0c08b3779b Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 16 Sep 2022 09:39:33 -0400 Subject: [PATCH 28/34] Renamed fixture --- tests/integration_tests/fixtures/tags.py | 2 +- tests/integration_tests/tagging_tests.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/integration_tests/fixtures/tags.py b/tests/integration_tests/fixtures/tags.py index 41bd909e094cb..2d74ef1eca92b 100644 --- a/tests/integration_tests/fixtures/tags.py +++ b/tests/integration_tests/fixtures/tags.py @@ -22,7 +22,7 @@ @pytest.fixture -def tag_sqla_event_listeners(): +def with_tagging_system_feature(): with app.app_context(): app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] = True register_sqla_event_listeners() diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 9029d5976c2c8..c8d80ac716c8b 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -30,7 +30,7 @@ from superset.utils.database import get_main_database from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.conftest import with_feature_flags -from tests.integration_tests.fixtures.tags import tag_sqla_event_listeners +from tests.integration_tests.fixtures.tags import with_tagging_system_feature class TestTagging(SupersetTestCase): @@ -54,7 +54,7 @@ def test_tag_view_enabled(self): response = self.client.get("/tagview/tags/suggestions/") self.assertNotEqual(404, response.status_code) - @pytest.mark.usefixtures("tag_sqla_event_listeners") + @pytest.mark.usefixtures("with_tagging_system_feature") def test_dataset_tagging(self): """ Test to make sure that when a new dataset is created, @@ -90,7 +90,7 @@ def test_dataset_tagging(self): db.session.delete(test_dataset) db.session.commit() - @pytest.mark.usefixtures("tag_sqla_event_listeners") + @pytest.mark.usefixtures("with_tagging_system_feature") def test_chart_tagging(self): """ Test to make sure that when a new chart is created, @@ -125,7 +125,7 @@ def test_chart_tagging(self): db.session.delete(test_chart) db.session.commit() - @pytest.mark.usefixtures("tag_sqla_event_listeners") + @pytest.mark.usefixtures("with_tagging_system_feature") def test_dashboard_tagging(self): """ Test to make sure that when a new dashboard is created, @@ -159,7 +159,7 @@ def test_dashboard_tagging(self): db.session.delete(test_dashboard) db.session.commit() - @pytest.mark.usefixtures("tag_sqla_event_listeners") + @pytest.mark.usefixtures("with_tagging_system_feature") def test_saved_query_tagging(self): """ Test to make sure that when a new saved query is @@ -197,7 +197,7 @@ def test_saved_query_tagging(self): db.session.delete(test_saved_query) db.session.commit() - @pytest.mark.usefixtures("tag_sqla_event_listeners") + @pytest.mark.usefixtures("with_tagging_system_feature") def test_favorite_tagging(self): """ Test to make sure that when a new favorite object is From d8a5e114751c36d42b845ace7067b22861486940 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 16 Sep 2022 09:48:17 -0400 Subject: [PATCH 29/34] Removed quotation marks after importing annotations from '__future__ --- superset/tags/models.py | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/superset/tags/models.py b/superset/tags/models.py index 04d1345d2d0d2..89505146e2598 100644 --- a/superset/tags/models.py +++ b/superset/tags/models.py @@ -14,7 +14,13 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from __future__ import absolute_import, division, print_function, unicode_literals +from __future__ import ( + absolute_import, + annotations, + division, + print_function, + unicode_literals, +) import enum from typing import List, Optional, TYPE_CHECKING, Union @@ -119,7 +125,7 @@ class ObjectUpdater: @classmethod def get_owners_ids( - cls, target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"] + cls, target: Union[Dashboard, FavStar, Slice, Query, SqlaTable] ) -> List[int]: raise NotImplementedError("Subclass should implement `get_owners_ids`") @@ -127,7 +133,7 @@ def get_owners_ids( def _add_owners( cls, session: Session, - target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"], + target: Union[Dashboard, FavStar, Slice, Query, SqlaTable], ) -> None: for owner_id in cls.get_owners_ids(target): name = "owner:{0}".format(owner_id) @@ -142,7 +148,7 @@ def after_insert( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"], + target: Union[Dashboard, FavStar, Slice, Query, SqlaTable], ) -> None: session = Session(bind=connection) @@ -163,7 +169,7 @@ def after_update( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"], + target: Union[Dashboard, FavStar, Slice, Query, SqlaTable], ) -> None: session = Session(bind=connection) @@ -192,7 +198,7 @@ def after_delete( cls, _mapper: Mapper, connection: Connection, - target: Union["Dashboard", "FavStar", "Slice", "Query", "SqlaTable"], + target: Union[Dashboard, FavStar, Slice, Query, SqlaTable], ) -> None: session = Session(bind=connection) @@ -210,7 +216,7 @@ class ChartUpdater(ObjectUpdater): object_type = "chart" @classmethod - def get_owners_ids(cls, target: "Slice") -> List[int]: + def get_owners_ids(cls, target: Slice) -> List[int]: return [owner.id for owner in target.owners] @@ -219,7 +225,7 @@ class DashboardUpdater(ObjectUpdater): object_type = "dashboard" @classmethod - def get_owners_ids(cls, target: "Dashboard") -> List[int]: + def get_owners_ids(cls, target: Dashboard) -> List[int]: return [owner.id for owner in target.owners] @@ -228,7 +234,7 @@ class QueryUpdater(ObjectUpdater): object_type = "query" @classmethod - def get_owners_ids(cls, target: "Query") -> List[int]: + def get_owners_ids(cls, target: Query) -> List[int]: return [target.user_id] @@ -237,14 +243,14 @@ class DatasetUpdater(ObjectUpdater): object_type = "dataset" @classmethod - def get_owners_ids(cls, target: "SqlaTable") -> List[int]: + def get_owners_ids(cls, target: SqlaTable) -> List[int]: return [owner.id for owner in target.owners] class FavStarUpdater: @classmethod def after_insert( - cls, _mapper: Mapper, connection: Connection, target: "FavStar" + cls, _mapper: Mapper, connection: Connection, target: FavStar ) -> None: session = Session(bind=connection) name = "favorited_by:{0}".format(target.user_id) @@ -260,7 +266,7 @@ def after_insert( @classmethod def after_delete( - cls, _mapper: Mapper, connection: Connection, target: "FavStar" + cls, _mapper: Mapper, connection: Connection, target: FavStar ) -> None: session = Session(bind=connection) name = "favorited_by:{0}".format(target.user_id) From 079cb7edfb8c56cf16923a4bac070da5bee4dfca Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 16 Sep 2022 10:57:03 -0400 Subject: [PATCH 30/34] Remove unused import --- superset/models/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/models/core.py b/superset/models/core.py index 000ca75889e46..354b8fb5c6c09 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -53,7 +53,7 @@ from sqlalchemy.schema import UniqueConstraint from sqlalchemy.sql import expression, Select -from superset import app, db_engine_specs, is_feature_enabled +from superset import app, db_engine_specs from superset.constants import PASSWORD_MASK from superset.databases.utils import make_url_safe from superset.db_engine_specs.base import MetricType, TimeGrain From 160c95d466b27c2d47277ce9419f974f4903e006 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 16 Sep 2022 13:46:29 -0400 Subject: [PATCH 31/34] Commenting out a test to see if it is causing another integration test to fail --- tests/integration_tests/tagging_tests.py | 68 ++++++++++++------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index c8d80ac716c8b..2054e73e08f39 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -90,40 +90,40 @@ def test_dataset_tagging(self): db.session.delete(test_dataset) db.session.commit() - @pytest.mark.usefixtures("with_tagging_system_feature") - def test_chart_tagging(self): - """ - Test to make sure that when a new chart is created, - a corresponding tag in the tagged_objects table - is created - """ - - # Remove all existing rows in the tagged_object table - self.clear_tagged_object_table() - - # Test to make sure nothing is in the tagged_object table - self.assertEqual([], self.query_tagged_object_table()) - - # Create a chart and add it to the db - test_chart = Slice( - slice_name="test_chart", - datasource_type=DatasourceType.TABLE, - viz_type="bubble", - datasource_id=1, - id=1, - ) - db.session.add(test_chart) - db.session.commit() - - # Test to make sure that a chart tag was added to the tagged_object table - tags = self.query_tagged_object_table() - self.assertEqual(1, len(tags)) - self.assertEqual("ObjectTypes.chart", str(tags[0].object_type)) - self.assertEqual(test_chart.id, tags[0].object_id) - - # Cleanup the db - db.session.delete(test_chart) - db.session.commit() + # @pytest.mark.usefixtures("with_tagging_system_feature") + # def test_chart_tagging(self): + # """ + # Test to make sure that when a new chart is created, + # a corresponding tag in the tagged_objects table + # is created + # """ + + # # Remove all existing rows in the tagged_object table + # self.clear_tagged_object_table() + + # # Test to make sure nothing is in the tagged_object table + # self.assertEqual([], self.query_tagged_object_table()) + + # # Create a chart and add it to the db + # test_chart = Slice( + # slice_name="test_chart", + # datasource_type=DatasourceType.TABLE, + # viz_type="bubble", + # datasource_id=1, + # id=1, + # ) + # db.session.add(test_chart) + # db.session.commit() + + # # Test to make sure that a chart tag was added to the tagged_object table + # tags = self.query_tagged_object_table() + # self.assertEqual(1, len(tags)) + # self.assertEqual("ObjectTypes.chart", str(tags[0].object_type)) + # self.assertEqual(test_chart.id, tags[0].object_id) + + # # Cleanup the db + # db.session.delete(test_chart) + # db.session.commit() @pytest.mark.usefixtures("with_tagging_system_feature") def test_dashboard_tagging(self): From 461b6d57ed57c360ad3888bc0f131e99a9bb94a3 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Tue, 20 Sep 2022 08:38:28 -0400 Subject: [PATCH 32/34] Removing newly created objects from the db when they are no longer needed --- tests/integration_tests/tagging_tests.py | 76 +++++++++++++----------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 2054e73e08f39..6f6d8ee0d70c0 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -90,40 +90,40 @@ def test_dataset_tagging(self): db.session.delete(test_dataset) db.session.commit() - # @pytest.mark.usefixtures("with_tagging_system_feature") - # def test_chart_tagging(self): - # """ - # Test to make sure that when a new chart is created, - # a corresponding tag in the tagged_objects table - # is created - # """ - - # # Remove all existing rows in the tagged_object table - # self.clear_tagged_object_table() - - # # Test to make sure nothing is in the tagged_object table - # self.assertEqual([], self.query_tagged_object_table()) - - # # Create a chart and add it to the db - # test_chart = Slice( - # slice_name="test_chart", - # datasource_type=DatasourceType.TABLE, - # viz_type="bubble", - # datasource_id=1, - # id=1, - # ) - # db.session.add(test_chart) - # db.session.commit() - - # # Test to make sure that a chart tag was added to the tagged_object table - # tags = self.query_tagged_object_table() - # self.assertEqual(1, len(tags)) - # self.assertEqual("ObjectTypes.chart", str(tags[0].object_type)) - # self.assertEqual(test_chart.id, tags[0].object_id) - - # # Cleanup the db - # db.session.delete(test_chart) - # db.session.commit() + @pytest.mark.usefixtures("with_tagging_system_feature") + def test_chart_tagging(self): + """ + Test to make sure that when a new chart is created, + a corresponding tag in the tagged_objects table + is created + """ + + # Remove all existing rows in the tagged_object table + self.clear_tagged_object_table() + + # Test to make sure nothing is in the tagged_object table + self.assertEqual([], self.query_tagged_object_table()) + + # Create a chart and add it to the db + test_chart = Slice( + slice_name="test_chart", + datasource_type=DatasourceType.TABLE, + viz_type="bubble", + datasource_id=1, + id=1, + ) + db.session.add(test_chart) + db.session.commit() + + # Test to make sure that a chart tag was added to the tagged_object table + tags = self.query_tagged_object_table() + self.assertEqual(1, len(tags)) + self.assertEqual("ObjectTypes.chart", str(tags[0].object_type)) + self.assertEqual(test_chart.id, tags[0].object_id) + + # Cleanup the db + db.session.delete(test_chart) + db.session.commit() @pytest.mark.usefixtures("with_tagging_system_feature") def test_dashboard_tagging(self): @@ -269,13 +269,13 @@ def test_tagging_system(self): test_saved_query = SavedQuery(id=1, label="test saved query") # Create a favorited object and add it to the db - test_saved_query = FavStar(user_id=1, class_name="slice", obj_id=1) + test_favorited_object = FavStar(user_id=1, class_name="slice", obj_id=1) db.session.add(test_dataset) db.session.add(test_chart) db.session.add(test_dashboard) db.session.add(test_saved_query) - db.session.add(test_saved_query) + db.session.add(test_favorited_object) db.session.commit() # Test to make sure that no tags were added to the tagged_object table @@ -283,5 +283,9 @@ def test_tagging_system(self): self.assertEqual(0, len(tags)) # Cleanup the db + db.session.delete(test_dataset) + db.session.delete(test_chart) db.session.delete(test_dashboard) + db.session.delete(test_saved_query) + db.session.delete(test_favorited_object) db.session.commit() From 2375ab151d6ad281edac343fdd4ac8711e192d15 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Tue, 20 Sep 2022 09:09:18 -0400 Subject: [PATCH 33/34] Added a check to make sure that each tag was deleted when the associated object was deleted --- tests/integration_tests/tagging_tests.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/integration_tests/tagging_tests.py b/tests/integration_tests/tagging_tests.py index 6f6d8ee0d70c0..4ee10041d2c53 100644 --- a/tests/integration_tests/tagging_tests.py +++ b/tests/integration_tests/tagging_tests.py @@ -90,6 +90,9 @@ def test_dataset_tagging(self): db.session.delete(test_dataset) db.session.commit() + # Test to make sure the tag is deleted when the associated object is deleted + self.assertEqual([], self.query_tagged_object_table()) + @pytest.mark.usefixtures("with_tagging_system_feature") def test_chart_tagging(self): """ @@ -125,6 +128,9 @@ def test_chart_tagging(self): db.session.delete(test_chart) db.session.commit() + # Test to make sure the tag is deleted when the associated object is deleted + self.assertEqual([], self.query_tagged_object_table()) + @pytest.mark.usefixtures("with_tagging_system_feature") def test_dashboard_tagging(self): """ @@ -159,6 +165,9 @@ def test_dashboard_tagging(self): db.session.delete(test_dashboard) db.session.commit() + # Test to make sure the tag is deleted when the associated object is deleted + self.assertEqual([], self.query_tagged_object_table()) + @pytest.mark.usefixtures("with_tagging_system_feature") def test_saved_query_tagging(self): """ @@ -197,6 +206,9 @@ def test_saved_query_tagging(self): db.session.delete(test_saved_query) db.session.commit() + # Test to make sure the tag is deleted when the associated object is deleted + self.assertEqual([], self.query_tagged_object_table()) + @pytest.mark.usefixtures("with_tagging_system_feature") def test_favorite_tagging(self): """ @@ -226,6 +238,9 @@ def test_favorite_tagging(self): db.session.delete(test_saved_query) db.session.commit() + # Test to make sure the tag is deleted when the associated object is deleted + self.assertEqual([], self.query_tagged_object_table()) + @with_feature_flags(TAGGING_SYSTEM=False) def test_tagging_system(self): """ @@ -289,3 +304,6 @@ def test_tagging_system(self): db.session.delete(test_saved_query) db.session.delete(test_favorited_object) db.session.commit() + + # Test to make sure all the tags are deleted when the associated objects are deleted + self.assertEqual([], self.query_tagged_object_table()) From 36115793981c094f088149311b188f5dfb5fe2d1 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Tue, 20 Sep 2022 11:04:21 -0400 Subject: [PATCH 34/34] Update tests/integration_tests/fixtures/tags.py Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- tests/integration_tests/fixtures/tags.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/integration_tests/fixtures/tags.py b/tests/integration_tests/fixtures/tags.py index 2d74ef1eca92b..57fd4ec7196e2 100644 --- a/tests/integration_tests/fixtures/tags.py +++ b/tests/integration_tests/fixtures/tags.py @@ -24,8 +24,10 @@ @pytest.fixture def with_tagging_system_feature(): with app.app_context(): - app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] = True - register_sqla_event_listeners() - yield - app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] = False - clear_sqla_event_listeners() + is_enabled = app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] + if not is_enabled: + app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] = True + register_sqla_event_listeners() + yield + app.config["DEFAULT_FEATURE_FLAGS"]["TAGGING_SYSTEM"] = False + clear_sqla_event_listeners()