From d8b4b5b824f1745e576d1409c96cba6319acd324 Mon Sep 17 00:00:00 2001 From: Shengyao Qian Date: Mon, 1 May 2017 16:45:31 -0700 Subject: [PATCH] Override _delete instead of post_delete --- superset/connectors/druid/models.py | 2 +- superset/connectors/druid/views.py | 81 ++++++++++++++----- superset/connectors/sqla/models.py | 29 ++----- superset/connectors/sqla/views.py | 48 ++++++++--- ..._add_datasource_name_to_construct_perms.py | 64 --------------- superset/views/core.py | 61 ++++++++------ 6 files changed, 138 insertions(+), 147 deletions(-) delete mode 100644 superset/migrations/versions/a2e7ece571a9_add_datasource_name_to_construct_perms.py diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index f842531ab2eb7..4ad6a6f951914 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -361,7 +361,7 @@ def schema(self): @property def schema_perm(self): """Returns schema permission if present, cluster one otherwise.""" - return utils.get_schema_perm(self.cluster_name, self.schema) + return utils.get_schema_perm(self.cluster, self.schema) def get_perm(self): return ( diff --git a/superset/connectors/druid/views.py b/superset/connectors/druid/views.py index a8e9f23df1404..c8a4f15ebef51 100644 --- a/superset/connectors/druid/views.py +++ b/superset/connectors/druid/views.py @@ -135,16 +135,36 @@ def pre_add(self, cluster): def pre_update(self, cluster): self.pre_add(cluster) - def post_delete(self, cluster): - logging.info("Clean up permission views for {}".format(cluster)) - - view_menu = sm.find_view_menu(cluster.get_perm()) - pvs = sm.get_session.query(sm.permissionview_model).filter_by( - view_menu=view_menu).all() - for pv in pvs: - sm.get_session.delete(pv) - sm.get_session.delete(view_menu) - sm.get_session.commit() + def _delete(self, pk): + """ + Delete function logic, override to implement diferent logic + deletes the record with primary_key = pk + + :param pk: + record primary key to delete + """ + cluster = self.datamodel.get(pk, self._base_filters) + if not cluster: + abort(404) + try: + self.pre_delete(cluster) + except Exception as e: + flash(str(e), "danger") + else: + view_menu = sm.find_view_menu(cluster.get_perm()) + pvs = sm.get_session.query(sm.permissionview_model).filter_by( + view_menu=view_menu).all() + + if self.datamodel.delete(cluster): + self.post_delete(cluster) + + for pv in pvs: + sm.get_session.delete(pv) + sm.get_session.delete(view_menu) + sm.get_session.commit() + + flash(*self.datamodel.message) + self.update_redirect() appbuilder.add_view( DruidClusterModelView, @@ -237,17 +257,36 @@ def post_add(self, datasource): def post_update(self, datasource): self.post_add(datasource) - def post_delete(self, datasource): - logging.info("Clean up permission views for {}".format(datasource)) - - view_menu = sm.find_view_menu(datasource.get_perm()) - pvs = sm.get_session.query(sm.permissionview_model).filter_by( - view_menu=view_menu).all() - for pv in pvs: - sm.get_session.delete(pv) - sm.get_session.delete(view_menu) - sm.get_session.commit() - + def _delete(self, pk): + """ + Delete function logic, override to implement diferent logic + deletes the record with primary_key = pk + + :param pk: + record primary key to delete + """ + datasource = self.datamodel.get(pk, self._base_filters) + if not datasource: + abort(404) + try: + self.pre_delete(datasource) + except Exception as e: + flash(str(e), "danger") + else: + view_menu = sm.find_view_menu(datasource.get_perm()) + pvs = sm.get_session.query(sm.permissionview_model).filter_by( + view_menu=view_menu).all() + + if self.datamodel.delete(cluster): + self.post_delete(cluster) + + for pv in pvs: + sm.get_session.delete(pv) + sm.get_session.delete(view_menu) + sm.get_session.commit() + + flash(*self.datamodel.message) + self.update_redirect() appbuilder.add_view( DruidDatasourceModelView, diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index fc53ecdbb544c..1ed45023fbc97 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -1,7 +1,6 @@ from datetime import datetime import logging import sqlparse -from past.builtins import basestring import pandas as pd @@ -172,18 +171,12 @@ class SqlaTable(Model, BaseDatasource): 'User', backref='tables', foreign_keys=[user_id]) - database_name = Column(String(250)) database = relationship( 'Database', backref=backref('tables', cascade='all, delete-orphan'), foreign_keys=[database_id]) schema = Column(String(255)) sql = Column(Text) - slices = relationship( - 'Slice', - primaryjoin=( - "SqlaTable.id == foreign(Slice.datasource_id) and " - "Slice.datasource_type == 'table'")) baselink = "tablemodelview" export_fields = ( @@ -212,11 +205,11 @@ def link(self): @property def schema_perm(self): """Returns schema permission if present, database one otherwise.""" - return utils.get_schema_perm(self.database_name, self.schema) + return utils.get_schema_perm(self.database, self.schema) def get_perm(self): return ( - "[{obj.database_name}].[{obj.table_name}]" + "[{obj.database}].[{obj.table_name}]" "(id:{obj.id})").format(obj=self) @property @@ -469,19 +462,9 @@ def visit_column(element, compiler, **kw): col_obj = cols.get(col) if col_obj: if op in ('in', 'not in'): - values = [] - for v in eq: - # For backwards compatibility and edge cases - # where a column data type might have changed - if isinstance(v, basestring): - v = v.strip("'").strip('"') - if col_obj.is_num: - v = utils.string_to_num(v) - - # Removing empty strings and non numeric values - # targeting numeric columns - if v is not None: - values.append(v) + values = [types.strip("'").strip('"') for types in eq] + if col_obj.is_num: + values = [utils.js_string_to_num(s) for s in values] cond = col_obj.sqla_col.in_(values) if op == 'not in': cond = ~cond @@ -586,8 +569,6 @@ def fetch_metadata(self): "Table doesn't seem to exist in the specified database, " "couldn't fetch column information") - self.database_name = self.database.database_name - TC = TableColumn # noqa shortcut to class M = SqlMetric # noqa metrics = [] diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 3bcacd6ea2c60..9416e74939f61 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -239,25 +239,47 @@ def post_add(self, table, flash_message=True): def post_update(self, table): self.post_add(table, flash_message=False) - def post_delete(self, table): - logging.info("Clean up permission views for {}".format(table)) + def _delete(self, pk): + """ + Delete function logic, override to implement diferent logic + deletes the record with primary_key = pk - table_view_menu = sm.find_view_menu(table.get_perm()) - pvs = sm.get_session.query(sm.permissionview_model).filter_by( - view_menu=table_view_menu).all() + :param pk: + record primary key to delete + """ + table = self.datamodel.get(pk, self._base_filters) + if not table: + abort(404) + try: + self.pre_delete(table) + except Exception as e: + flash(str(e), "danger") + else: + table_view_menu = sm.find_view_menu(table.get_perm()) + pvs = sm.get_session.query(sm.permissionview_model).filter_by( + view_menu=table_view_menu).all() + + schema_view_menu = sm.find_view_menu(table.schema_perm) + + pvs.extend(sm.get_session.query(sm.permissionview_model).filter_by( + view_menu=schema_view_menu).all()) + + if self.datamodel.delete(table): + self.post_delete(table) - schema_view_menu = sm.find_view_menu(table.schema_perm) + for pv in pvs: + sm.get_session.delete(pv) - pvs.extend(sm.get_session.query(sm.permissionview_model).filter_by( - view_menu=schema_view_menu).all()) + if table_view_menu: + sm.get_session.delete(table_view_menu) - for pv in pvs: - sm.get_session.delete(pv) + if schema_view_menu: + sm.get_session.delete(schema_view_menu) - sm.get_session.delete(table_view_menu) - sm.get_session.delete(schema_view_menu) + sm.get_session.commit() - sm.get_session.commit() + flash(*self.datamodel.message) + self.update_redirect() @expose('/edit/', methods=['GET', 'POST']) @has_access diff --git a/superset/migrations/versions/a2e7ece571a9_add_datasource_name_to_construct_perms.py b/superset/migrations/versions/a2e7ece571a9_add_datasource_name_to_construct_perms.py deleted file mode 100644 index ea0fa7a4811ed..0000000000000 --- a/superset/migrations/versions/a2e7ece571a9_add_datasource_name_to_construct_perms.py +++ /dev/null @@ -1,64 +0,0 @@ -"""Add datasource name to construct perms - -Revision ID: a2e7ece571a9 -Revises: 2fcdcb35e487 -Create Date: 2017-04-25 10:53:00.445273 - -""" - -# revision identifiers, used by Alembic. -revision = 'a2e7ece571a9' -down_revision = '2fcdcb35e487' - -from alembic import op -import sqlalchemy as sa -from superset import db -from sqlalchemy.ext.declarative import declarative_base -from sqlalchemy import ( - Column, Integer, String, ForeignKey) -from sqlalchemy.orm import backref, relationship - - -Base = declarative_base() - - -class SqlaTable(Base): - """Declarative class to do query in upgrade""" - __tablename__ = 'tables' - id = Column(Integer, primary_key=True) - database_id = Column(Integer, ForeignKey('dbs.id'), nullable=False) - database_name = Column(String(250)) - database = relationship( - 'Database', - backref=backref('tables', cascade='all, delete-orphan'), - foreign_keys=[database_id]) - - -class Database(Base): - - """An ORM object that stores Database related information""" - - __tablename__ = 'dbs' - type = "table" - id = Column(Integer, primary_key=True) - database_name = Column(String(250), unique=True) - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('tables', sa.Column('database_name', sa.String(length=250), nullable=True)) - # ### end Alembic commands ### - bind = op.get_bind() - session = db.Session(bind=bind) - - tables = session.query(SqlaTable).all() - for table in tables: - table.database_name = table.database.database_name - session.commit() - session.close() - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - with op.batch_alter_table('tables') as batch_op: - batch_op.drop_column('database_name') - # ### end Alembic commands ### diff --git a/superset/views/core.py b/superset/views/core.py index 5b5f9832c597f..b790f0eed942a 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -248,33 +248,46 @@ def pre_add(self, db): def pre_update(self, db): self.pre_add(db) - def post_delete(self, db): - logging.info("Clean up permission views for {}".format(db)) - - db_view_menu = sm.find_view_menu(db.perm) - pvs = sm.get_session.query(sm.permissionview_model).filter_by( - view_menu=db_view_menu).all() - - for pv in pvs: - sm.get_session.delete(pv) - - sm.get_session.delete(db_view_menu) - - for schema in db.all_schema_names(): - schema_view_menu = sm.find_view_menu( - utils.get_schema_perm(db, schema)) - - schema_pvs = sm.get_session.query( - sm.permissionview_model).filter_by( - view_menu=schema_view_menu).all() - - for pv in schema_pvs: - sm.get_session.delete(pv) + def _delete(self, pk): + """ + Delete function logic, override to implement diferent logic + deletes the record with primary_key = pk - sm.get_session.delete(schema_view_menu) + :param pk: + record primary key to delete + """ + db = self.datamodel.get(pk, self._base_filters) + if not db: + abort(404) + try: + self.pre_delete(db) + except Exception as e: + flash(str(e), "danger") + else: + db_view_menu = sm.find_view_menu(db.perm) + pvs = sm.get_session.query(sm.permissionview_model).filter_by( + view_menu=db_view_menu).all() + db_schema_menus = db.all_schema_names() + for schema in db_schema_menus: + pvs.extend(sm.get_session.query(sm.permissionview_model).filter_by( + view_menu=db_schema_menus).all()) + + if self.datamodel.delete(db): + self.post_delete(db) + + for pv in pvs: + sm.get_session.delete(pv) + + if db_view_menu: + sm.get_session.delete(db_view_menu) + if db_schema_menus: + for schema_view_menu in db_schema_menus: + sm.get_session.delete(schema_view_menu) - sm.get_session.commit() + sm.get_session.commit() + flash(*self.datamodel.message) + self.update_redirect() appbuilder.add_link( 'Import Dashboards',