From 060a96a525359dc975e8e259d2dc314dc96f1edc Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 15 Jun 2020 04:00:03 +0100 Subject: [PATCH 01/22] refactor: interface query on m-m joins and select specific columns --- flask_appbuilder/models/sqla/interface.py | 176 ++++++++++++++++------ 1 file changed, 134 insertions(+), 42 deletions(-) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index af337aebc6..60ddc83616 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -5,7 +5,7 @@ from flask_sqlalchemy import BaseQuery import sqlalchemy as sa -from sqlalchemy import func +from sqlalchemy import asc, desc from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import aliased, contains_eager, Load, load_only from sqlalchemy.orm.descriptor_props import SynonymProperty @@ -92,12 +92,18 @@ def _apply_query_order( if hasattr(self.obj, order_column): if hasattr(getattr(self.obj, order_column), "_col_name"): order_column = getattr(self._get_attr(order_column), "_col_name") + _order_column = self._get_attr(order_column) or order_column if order_direction == "asc": - query = query.order_by(self._get_attr(order_column).asc()) + query = query.order_by(asc(_order_column)) else: - query = query.order_by(self._get_attr(order_column).desc()) + query = query.order_by(desc(_order_column)) return query + def apply_inner_order_by( + self, query: BaseQuery, order_column: str, order_direction: str + ) -> BaseQuery: + return self._apply_query_order(query, order_column, order_direction) + def _get_base_query( self, query=None, filters=None, order_column="", order_direction="" ): @@ -184,15 +190,113 @@ def _query_select_options( query = query.options(*tuple(load_options)) return query - def _get_non_dotted_filters(self, filters): - dotted_filters = Filters(self.filter_converter_class, self, [], []) + def apply_inner_pagination( + self, query: BaseQuery, page: int, page_size: int + ) -> BaseQuery: + if page and page_size: + query = query.offset(page * page_size) + if page_size: + query = query.limit(page_size) + return query + + def apply_inner_filters(self, query: BaseQuery, filters: Filters) -> BaseQuery: + if filters: + return filters.apply_all(query) + return query + + def apply_inner_select_joins( + self, query: BaseQuery, select_columns: List[str] = None + ) -> BaseQuery: + """ + Add select load options to query. The goal + is to only SQL select what is requested and join all the necessary + models when dotted notation is used. Inner implies non dotted columns + and one to many and one to one + + :param query: + :param select_columns: + :return: + """ + if not select_columns: + return query + load_options = list() + joined_models = list() + for column in select_columns: + if is_column_dotted(column): + root_relation = get_column_root_relation(column) + leaf_column = get_column_leaf(column) + if self.is_relation_many_to_one( + root_relation + ) or self.is_relation_one_to_one(root_relation): + if root_relation not in joined_models: + query = self._query_join_relation(query, root_relation) + related_model = self.get_related_model(root_relation) + query = query.add_entity(related_model) + joined_models.append(root_relation) + load_options.append( + (contains_eager(root_relation).load_only(leaf_column)) + ) + else: + if not self.is_relation(column) and not self.is_property_or_function( + column + ): + load_options.append(Load(self.obj).load_only(column)) + query = query.options(*tuple(load_options)) + return query + + def apply_outer_select_joins( + self, query: BaseQuery, select_columns: List[str] = None + ) -> BaseQuery: + if not select_columns: + return query + load_options = list() + for column in select_columns: + if is_column_dotted(column): + root_relation = get_column_root_relation(column) + leaf_column = get_column_leaf(column) + if self.is_relation_many_to_many( + root_relation + ) or self.is_relation_one_to_many(root_relation): + load_options.append( + ( + Load(self.obj) + .joinedload(root_relation) + .load_only(leaf_column) + ) + ) + else: + related_model = self.get_related_model(root_relation) + load_options.append((Load(related_model).load_only(leaf_column))) + else: + if not self.is_relation(column) and not self.is_property_or_function( + column + ): + load_options.append(Load(self.obj).load_only(column)) + query = query.options(*tuple(load_options)) + return query + + def get_inner_filters(self, filters: Filters) -> Filters: + """ + Inner filters are non dotted columns and + one to many or one to one relations + + :param filters: All filters + :return: New filtered filters to apply to an inner query + """ + inner_filters = Filters(self.filter_converter_class, self, [], []) _filters = [] if filters: for flt, value in zip(filters.filters, filters.values): + print(f"F1: {flt.column_name}") if not is_column_dotted(flt.column_name): _filters.append((flt.column_name, flt.__class__, value)) - dotted_filters.add_filter_list(_filters) - return dotted_filters + elif self.is_relation_many_to_one( + flt.column_name + ) or self.is_relation_one_to_one(flt.column_name): + _filters.append((flt.column_name, flt.__class__, value)) + print(f"FILTERS: {_filters}") + inner_filters.add_filter_list(_filters) + return inner_filters def query( self, @@ -218,46 +322,34 @@ def query( the current page size """ query = self.session.query(self.obj) - query_count = self.session.query(func.count("*")).select_from(self.obj) - - query_count = self._get_base_query(query=query_count, filters=filters) - - # MSSQL exception page/limit must have an order by - if ( - page - and page_size - and not order_column - and self.session.bind.dialect.name == "mssql" - ): - pk_name = self.get_pk_name() - query = query.order_by(pk_name) - - # If order by is not dotted (related) we need to apply it first - if not is_column_dotted(order_column): - query = self._get_non_dotted_filters(filters).apply_all(query) - query = self._apply_query_order(query, order_column, order_direction) - - # Pagination comes first - if page and page_size: - query = query.offset(page * page_size) - if page_size: - query = query.limit(page_size) + + inner_filters = self.get_inner_filters(filters) + inner_query = self.apply_inner_select_joins(query, select_columns) + inner_query = self.apply_inner_filters(inner_query, inner_filters) + + count = inner_query.count() + inner_query = self.apply_inner_order_by( + inner_query, order_column, order_direction + ) + inner_query = self.apply_inner_pagination(inner_query, page, page_size) + + outer_query = inner_query.from_self() if select_columns and order_column: - # Use from self strategy select_columns = select_columns + [order_column] - # Everything uses an inner query because of joins to m/m m/1 - query = self._query_select_options(query.from_self(), select_columns) - - query = self._get_base_query( - query=query, - filters=filters, - order_column=order_column, - order_direction=order_direction, + outer_query = self.apply_outer_select_joins(outer_query, select_columns) + outer_query = self.apply_inner_order_by( + outer_query, order_column, order_direction ) - count = query_count.scalar() - return count, query.all() + result = list() + results = outer_query.all() + for item in results: + if hasattr(item, self.obj.__name__): + result.append(getattr(item, self.obj.__name__)) + else: + return count, results + return count, result def query_simple_group( self, group_by="", aggregate_func=None, aggregate_col=None, filters=None From dd8b5593b118e9fbdb5fa307dd91b964b3b50dc0 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 15 Jun 2020 09:58:37 +0100 Subject: [PATCH 02/22] mssql hack --- flask_appbuilder/models/sqla/interface.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index 60ddc83616..dbd002e00a 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -99,6 +99,20 @@ def _apply_query_order( query = query.order_by(desc(_order_column)) return query + def apply_engine_specific_hack( + self, query: BaseQuery, page, page_size, order_column + ) -> BaseQuery: + # MSSQL exception page/limit must have an order by + if ( + page + and page_size + and not order_column + and self.session.bind.dialect.name == "mssql" + ): + pk_name = self.get_pk_name() + return query.order_by(pk_name) + return query + def apply_inner_order_by( self, query: BaseQuery, order_column: str, order_direction: str ) -> BaseQuery: @@ -328,6 +342,10 @@ def query( inner_query = self.apply_inner_filters(inner_query, inner_filters) count = inner_query.count() + + inner_query = self.apply_engine_specific_hack( + inner_query, page, page_size, order_column + ) inner_query = self.apply_inner_order_by( inner_query, order_column, order_direction ) From 65d1759dd50670f3bea515c51257fc6016f9daed Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 18 Jun 2020 15:43:45 +0100 Subject: [PATCH 03/22] add tests with many children --- flask_appbuilder/tests/const.py | 2 + flask_appbuilder/tests/sqla/models.py | 15 ++++--- flask_appbuilder/tests/test_api.py | 64 ++++++++++++++++++++++----- 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/flask_appbuilder/tests/const.py b/flask_appbuilder/tests/const.py index 5321fb0ac2..bdc26c6be8 100644 --- a/flask_appbuilder/tests/const.py +++ b/flask_appbuilder/tests/const.py @@ -1,5 +1,7 @@ MODEL1_DATA_SIZE = 30 MODEL2_DATA_SIZE = 30 +MODELOMCHILD_DATA_SIZE = 30 + USERNAME_ADMIN = "testadmin" PASSWORD_ADMIN = "password" MAX_PAGE_SIZE = 25 diff --git a/flask_appbuilder/tests/sqla/models.py b/flask_appbuilder/tests/sqla/models.py index a01a27a1ed..2506187e85 100644 --- a/flask_appbuilder/tests/sqla/models.py +++ b/flask_appbuilder/tests/sqla/models.py @@ -18,6 +18,8 @@ ) from sqlalchemy.orm import backref, relationship +from ..const import MODELOMCHILD_DATA_SIZE + def validate_name(n): if n[0] != "A": @@ -35,8 +37,9 @@ def __repr__(self): return str(self.field_string) def full_concat(self): - return "{}.{}.{}.{}".format( - self.field_string, self.field_integer, self.field_float, self.field_date + return ( + f"{self.field_string}.{self.field_integer}" + f".{self.field_float}.{self.field_date}" ) @@ -320,18 +323,18 @@ def insert_data(session, count): session.add(model) session.commit() - model_oo_parents = list() + model_om_parents = list() for i in range(count): model = ModelOMParent() model.field_string = f"text{i}" session.add(model) session.commit() - model_oo_parents.append(model) + model_om_parents.append(model) for i in range(count): - for j in range(1, 4): + for j in range(1, MODELOMCHILD_DATA_SIZE): model = ModelOMChild() model.field_string = f"text{i}.{j}" - model.parent = model_oo_parents[i] + model.parent = model_om_parents[i] session.add(model) session.commit() diff --git a/flask_appbuilder/tests/test_api.py b/flask_appbuilder/tests/test_api.py index 6f59d08b9f..427b8062a8 100644 --- a/flask_appbuilder/tests/test_api.py +++ b/flask_appbuilder/tests/test_api.py @@ -39,6 +39,7 @@ MAX_PAGE_SIZE, MODEL1_DATA_SIZE, MODEL2_DATA_SIZE, + MODELOMCHILD_DATA_SIZE, PASSWORD_ADMIN, PASSWORD_READONLY, USERNAME_ADMIN, @@ -289,6 +290,13 @@ class ModelOMParentApi(ModelRestApi): self.appbuilder.add_api(ModelOMParentApi) + class ModelDottedOMParentApi(ModelRestApi): + datamodel = SQLAInterface(ModelOMParent) + list_columns = ["field_string", "children.field_string"] + show_columns = ["field_string", "children.field_string"] + + self.appbuilder.add_api(ModelDottedOMParentApi) + class ModelMMRequiredApi(ModelRestApi): datamodel = SQLAInterface(ModelMMParentRequired) @@ -682,9 +690,9 @@ def test_get_item_select_cols(self): ) self.assertEqual(rv.status_code, 200) - def test_get_item_dotted_notation(self): + def test_get_item_dotted_mo_notation(self): """ - REST Api: Test get item with dotted notation + REST Api: Test get item with dotted M-O related field """ client = self.app.test_client() token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) @@ -771,9 +779,9 @@ def test_get_item_base_filters(self): rv = self.auth_client_get(client, token, f"api/v1/model1apifiltered/{pk}") self.assertEqual(rv.status_code, 200) - def test_get_item_1m_field(self): + def test_get_item_mo_field(self): """ - REST Api: Test get item with 1-N related field + REST Api: Test get item with M-O related field """ client = self.app.test_client() token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) @@ -801,7 +809,7 @@ def test_get_item_1m_field(self): def test_get_item_mm_field(self): """ - REST Api: Test get item with N-N related field + REST Api: Test get item with M-M related field """ client = self.app.test_client() token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) @@ -820,7 +828,7 @@ def test_get_item_mm_field(self): def test_get_item_dotted_mm_field(self): """ - REST Api: Test get item with dotted N-N related field + REST Api: Test get item with dotted M-M related field """ client = self.app.test_client() token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) @@ -855,7 +863,8 @@ def test_get_item_om_field(self): data = json.loads(rv.data.decode("utf-8")) self.assertEqual(rv.status_code, 200) expected_rel_field = [ - {"field_string": f"text0.{i}", "id": i} for i in range(1, 4) + {"field_string": f"text0.{i}", "id": i} + for i in range(1, MODELOMCHILD_DATA_SIZE) ] self.assertEqual(data[API_RESULT_RES_KEY]["children"], expected_rel_field) @@ -874,9 +883,9 @@ def test_get_list(self): # Tests data result default page size self.assertEqual(len(data[API_RESULT_RES_KEY]), self.model1api.page_size) - def test_get_list_dotted_notation(self): + def test_get_list_dotted_mo_field(self): """ - REST Api: Test get list with dotted notation + REST Api: Test get list with dotted M-O related field """ client = self.app.test_client() token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) @@ -897,9 +906,44 @@ def test_get_list_dotted_notation(self): {"field_string": "test0", "group": {"field_string": "test0"}}, ) + def test_get_list_om_field(self): + """ + REST Api: Test get list with O-M related field + """ + client = self.app.test_client() + token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) + + rv = self.auth_client_get(client, token, "api/v1/modelomparentapi/") + data = json.loads(rv.data.decode("utf-8")) + self.assertEqual(rv.status_code, 200) + self.assertEqual(data["count"], MODEL1_DATA_SIZE) + self.assertEqual(len(data[API_RESULT_RES_KEY]), self.model1api.page_size) + expected_rel_field = [ + {"field_string": f"text0.{i}", "id": i} + for i in range(1, MODELOMCHILD_DATA_SIZE) + ] + self.assertEqual(data[API_RESULT_RES_KEY][0]["children"], expected_rel_field) + + def test_get_list_dotted_om_field(self): + """ + REST Api: Test get list with dotted O-M related field + """ + client = self.app.test_client() + token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) + + rv = self.auth_client_get(client, token, "api/v1/modeldottedomparentapi/") + data = json.loads(rv.data.decode("utf-8")) + self.assertEqual(rv.status_code, 200) + self.assertEqual(data["count"], MODEL1_DATA_SIZE) + self.assertEqual(len(data[API_RESULT_RES_KEY]), self.model1api.page_size) + expected_rel_field = [ + {"field_string": f"text0.{i}"} for i in range(1, MODELOMCHILD_DATA_SIZE) + ] + self.assertEqual(data[API_RESULT_RES_KEY][0]["children"], expected_rel_field) + def test_get_list_dotted_mm_field(self): """ - REST Api: Test get list with dotted N-N related field + REST Api: Test get list with dotted M-M related field """ client = self.app.test_client() token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) From c3e772e6b2d7de4f3c6896684871e85a1def3126 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 18 Jun 2020 16:38:15 +0100 Subject: [PATCH 04/22] a bit of refactor on the refactor --- flask_appbuilder/models/sqla/interface.py | 50 ++++++++++------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index dbd002e00a..da33e9574f 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -113,7 +113,7 @@ def apply_engine_specific_hack( return query.order_by(pk_name) return query - def apply_inner_order_by( + def apply_order_by( self, query: BaseQuery, order_column: str, order_direction: str ) -> BaseQuery: return self._apply_query_order(query, order_column, order_direction) @@ -204,7 +204,7 @@ def _query_select_options( query = query.options(*tuple(load_options)) return query - def apply_inner_pagination( + def apply_pagination( self, query: BaseQuery, page: int, page_size: int ) -> BaseQuery: if page and page_size: @@ -213,11 +213,17 @@ def apply_inner_pagination( query = query.limit(page_size) return query - def apply_inner_filters(self, query: BaseQuery, filters: Filters) -> BaseQuery: + def apply_filters(self, query: BaseQuery, filters: Filters) -> BaseQuery: if filters: return filters.apply_all(query) return query + def _apply_normal_col_select_option(self, query, column) -> BaseQuery: + if not self.is_relation(column) and not self.is_property_or_function( + column + ): + return query.options(Load(self.obj).load_only(column)) + def apply_inner_select_joins( self, query: BaseQuery, select_columns: List[str] = None ) -> BaseQuery: @@ -233,7 +239,6 @@ def apply_inner_select_joins( """ if not select_columns: return query - load_options = list() joined_models = list() for column in select_columns: if is_column_dotted(column): @@ -247,15 +252,11 @@ def apply_inner_select_joins( related_model = self.get_related_model(root_relation) query = query.add_entity(related_model) joined_models.append(root_relation) - load_options.append( + query = query.options( (contains_eager(root_relation).load_only(leaf_column)) ) else: - if not self.is_relation(column) and not self.is_property_or_function( - column - ): - load_options.append(Load(self.obj).load_only(column)) - query = query.options(*tuple(load_options)) + query = self._apply_normal_col_select_option(query, column) return query def apply_outer_select_joins( @@ -263,7 +264,6 @@ def apply_outer_select_joins( ) -> BaseQuery: if not select_columns: return query - load_options = list() for column in select_columns: if is_column_dotted(column): root_relation = get_column_root_relation(column) @@ -271,22 +271,16 @@ def apply_outer_select_joins( if self.is_relation_many_to_many( root_relation ) or self.is_relation_one_to_many(root_relation): - load_options.append( - ( - Load(self.obj) - .joinedload(root_relation) - .load_only(leaf_column) - ) + query = query.options( + Load(self.obj).joinedload(root_relation).load_only(leaf_column) ) else: related_model = self.get_related_model(root_relation) - load_options.append((Load(related_model).load_only(leaf_column))) + query = query.options( + Load(related_model).load_only(leaf_column) + ) else: - if not self.is_relation(column) and not self.is_property_or_function( - column - ): - load_options.append(Load(self.obj).load_only(column)) - query = query.options(*tuple(load_options)) + query = self._apply_normal_col_select_option(query, column) return query def get_inner_filters(self, filters: Filters) -> Filters: @@ -339,24 +333,22 @@ def query( inner_filters = self.get_inner_filters(filters) inner_query = self.apply_inner_select_joins(query, select_columns) - inner_query = self.apply_inner_filters(inner_query, inner_filters) + inner_query = self.apply_filters(inner_query, inner_filters) count = inner_query.count() inner_query = self.apply_engine_specific_hack( inner_query, page, page_size, order_column ) - inner_query = self.apply_inner_order_by( - inner_query, order_column, order_direction - ) - inner_query = self.apply_inner_pagination(inner_query, page, page_size) + inner_query = self.apply_order_by(inner_query, order_column, order_direction) + inner_query = self.apply_pagination(inner_query, page, page_size) outer_query = inner_query.from_self() if select_columns and order_column: select_columns = select_columns + [order_column] outer_query = self.apply_outer_select_joins(outer_query, select_columns) - outer_query = self.apply_inner_order_by( + outer_query = self.apply_order_by( outer_query, order_column, order_direction ) From a119c811ca921861f21f3d164c0453be0f643588 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 18 Jun 2020 16:40:42 +0100 Subject: [PATCH 05/22] remove prints --- flask_appbuilder/models/sqla/interface.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index da33e9574f..21311b5b5b 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -295,14 +295,12 @@ def get_inner_filters(self, filters: Filters) -> Filters: _filters = [] if filters: for flt, value in zip(filters.filters, filters.values): - print(f"F1: {flt.column_name}") if not is_column_dotted(flt.column_name): _filters.append((flt.column_name, flt.__class__, value)) elif self.is_relation_many_to_one( flt.column_name ) or self.is_relation_one_to_one(flt.column_name): _filters.append((flt.column_name, flt.__class__, value)) - print(f"FILTERS: {_filters}") inner_filters.add_filter_list(_filters) return inner_filters From ddcd26b529436a23062f66e288fdc18876858e90 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 18 Jun 2020 17:20:45 +0100 Subject: [PATCH 06/22] fix and cleanup --- flask_appbuilder/models/sqla/interface.py | 118 ++++++---------------- 1 file changed, 32 insertions(+), 86 deletions(-) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index 21311b5b5b..10f99d8a79 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -7,7 +7,7 @@ import sqlalchemy as sa from sqlalchemy import asc, desc from sqlalchemy.exc import IntegrityError -from sqlalchemy.orm import aliased, contains_eager, Load, load_only +from sqlalchemy.orm import aliased, contains_eager, Load from sqlalchemy.orm.descriptor_props import SynonymProperty from sqlalchemy.sql.elements import BinaryExpression from sqlalchemy_utils.types.uuid import UUIDType @@ -83,47 +83,12 @@ def model_name(self): def is_model_already_joined(query, model): return model in [mapper.class_ for mapper in query._join_entities] - def _apply_query_order( - self, query, order_column: str, order_direction: str - ) -> BaseQuery: - if order_column != "": - # if Model has custom decorator **renders('')** - # this decorator will add a property to the method named *_col_name* - if hasattr(self.obj, order_column): - if hasattr(getattr(self.obj, order_column), "_col_name"): - order_column = getattr(self._get_attr(order_column), "_col_name") - _order_column = self._get_attr(order_column) or order_column - if order_direction == "asc": - query = query.order_by(asc(_order_column)) - else: - query = query.order_by(desc(_order_column)) - return query - - def apply_engine_specific_hack( - self, query: BaseQuery, page, page_size, order_column - ) -> BaseQuery: - # MSSQL exception page/limit must have an order by - if ( - page - and page_size - and not order_column - and self.session.bind.dialect.name == "mssql" - ): - pk_name = self.get_pk_name() - return query.order_by(pk_name) - return query - - def apply_order_by( - self, query: BaseQuery, order_column: str, order_direction: str - ) -> BaseQuery: - return self._apply_query_order(query, order_column, order_direction) - def _get_base_query( self, query=None, filters=None, order_column="", order_direction="" ): if filters: query = filters.apply_all(query) - return self._apply_query_order(query, order_column, order_direction) + return self.apply_order_by(query, order_column, order_direction) def _query_join_relation(self, query: BaseQuery, root_relation: str) -> BaseQuery: """ @@ -160,48 +125,34 @@ def _query_join_dotted_column(self, query: BaseQuery, column: str) -> BaseQuery: return self._query_join_relation(query, get_column_root_relation(column)) return query - def _query_select_options( - self, query: BaseQuery, select_columns: List[str] = None + def apply_engine_specific_hack( + self, query: BaseQuery, page, page_size, order_column ) -> BaseQuery: - """ - Add select load options to query. The goal - is to only SQL select what is requested and join all the necessary - models when dotted notation is used + # MSSQL exception page/limit must have an order by + if ( + page + and page_size + and not order_column + and self.session.bind.dialect.name == "mssql" + ): + pk_name = self.get_pk_name() + return query.order_by(pk_name) + return query - :param query: SQLAlchemy Query obj to apply joins and selects - :param select_columns: (list) of columns - :return: Transformed SQLAlchemy Query - """ - if select_columns: - load_options = list() - joined_models = list() - for column in select_columns: - if is_column_dotted(column): - root_relation = get_column_root_relation(column) - leaf_column = get_column_leaf(column) - if self.is_relation_many_to_many( - root_relation - ) or self.is_relation_one_to_many(root_relation): - load_options.append( - ( - Load(self.obj) - .joinedload(root_relation) - .load_only(leaf_column) - ) - ) - continue - elif root_relation not in joined_models: - query = self._query_join_relation(query, root_relation) - joined_models.append(root_relation) - load_options.append( - (contains_eager(root_relation).load_only(leaf_column)) - ) - else: - if not self.is_relation( - column - ) and not self.is_property_or_function(column): - load_options.append(load_only(column)) - query = query.options(*tuple(load_options)) + def apply_order_by( + self, query: BaseQuery, order_column: str, order_direction: str + ) -> BaseQuery: + if order_column != "": + # if Model has custom decorator **renders('')** + # this decorator will add a property to the method named *_col_name* + if hasattr(self.obj, order_column): + if hasattr(getattr(self.obj, order_column), "_col_name"): + order_column = getattr(self._get_attr(order_column), "_col_name") + _order_column = self._get_attr(order_column) or order_column + if order_direction == "asc": + query = query.order_by(asc(_order_column)) + else: + query = query.order_by(desc(_order_column)) return query def apply_pagination( @@ -219,10 +170,9 @@ def apply_filters(self, query: BaseQuery, filters: Filters) -> BaseQuery: return query def _apply_normal_col_select_option(self, query, column) -> BaseQuery: - if not self.is_relation(column) and not self.is_property_or_function( - column - ): + if not self.is_relation(column) and not self.is_property_or_function(column): return query.options(Load(self.obj).load_only(column)) + return query def apply_inner_select_joins( self, query: BaseQuery, select_columns: List[str] = None @@ -276,9 +226,7 @@ def apply_outer_select_joins( ) else: related_model = self.get_related_model(root_relation) - query = query.options( - Load(related_model).load_only(leaf_column) - ) + query = query.options(Load(related_model).load_only(leaf_column)) else: query = self._apply_normal_col_select_option(query, column) return query @@ -346,9 +294,7 @@ def query( if select_columns and order_column: select_columns = select_columns + [order_column] outer_query = self.apply_outer_select_joins(outer_query, select_columns) - outer_query = self.apply_order_by( - outer_query, order_column, order_direction - ) + outer_query = self.apply_order_by(outer_query, order_column, order_direction) result = list() results = outer_query.all() From 020b8e328357e86a45eef187b213abaeb637ba18 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 23 Jun 2020 10:20:42 +0100 Subject: [PATCH 07/22] fix and cleanup --- flask_appbuilder/tests/test_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flask_appbuilder/tests/test_api.py b/flask_appbuilder/tests/test_api.py index 427b8062a8..6453d9d3ef 100644 --- a/flask_appbuilder/tests/test_api.py +++ b/flask_appbuilder/tests/test_api.py @@ -964,9 +964,9 @@ def test_get_list_dotted_mm_field(self): self.assertIn({"field_integer": 2}, data[API_RESULT_RES_KEY][i]["children"]) self.assertIn({"field_integer": 3}, data[API_RESULT_RES_KEY][i]["children"]) - def test_get_list_dotted_order(self): + def test_get_list_dotted_mo_order(self): """ - REST Api: Test get list and order dotted notation + REST Api: Test get list and order dotted M-O notation """ client = self.app.test_client() token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) From f340c8a2e797325ae0e4e40f92af2dc90869cd06 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 23 Jun 2020 11:51:35 +0100 Subject: [PATCH 08/22] type annotations and code quality --- flask_appbuilder/exceptions.py | 6 ++ flask_appbuilder/models/base.py | 7 +- flask_appbuilder/models/filters.py | 8 +- flask_appbuilder/models/sqla/interface.py | 106 +++++++++++++--------- 4 files changed, 75 insertions(+), 52 deletions(-) diff --git a/flask_appbuilder/exceptions.py b/flask_appbuilder/exceptions.py index 99eebaaeec..7f410ab0fd 100644 --- a/flask_appbuilder/exceptions.py +++ b/flask_appbuilder/exceptions.py @@ -20,3 +20,9 @@ class InvalidOrderByColumnFABException(FABException): """Invalid order by column""" pass + + +class InterfaceQueryWithoutSession(FABException): + """You need to setup a session on the interface to perform queries""" + + pass diff --git a/flask_appbuilder/models/base.py b/flask_appbuilder/models/base.py index 1e4c19ec2b..a4701a3945 100644 --- a/flask_appbuilder/models/base.py +++ b/flask_appbuilder/models/base.py @@ -1,10 +1,11 @@ import datetime from functools import reduce import logging +from typing import Type from flask_babel import lazy_gettext -from .filters import Filters +from .filters import Filters, BaseFilterConverter try: import enum @@ -22,9 +23,7 @@ class BaseInterface(object): Sub class it to implement your own interface for some data engine. """ - obj = None - - filter_converter_class = None + filter_converter_class = Type[BaseFilterConverter] """ when sub classing override with your own custom filter converter """ """ Messages to display on CRUD Events """ diff --git a/flask_appbuilder/models/filters.py b/flask_appbuilder/models/filters.py index 36579add5a..0c11a93a76 100644 --- a/flask_appbuilder/models/filters.py +++ b/flask_appbuilder/models/filters.py @@ -1,6 +1,6 @@ import copy import logging -from typing import Any, Dict, List, Tuple +from typing import Any, Dict, List, Optional, Tuple, Type from .._compat import as_unicode from ..exceptions import ( @@ -128,10 +128,10 @@ class Filters(object): def __init__( self, - filter_converter: BaseFilterConverter, + filter_converter: Type[BaseFilterConverter], datamodel, - search_columns: List[str] = None, - search_filters: Dict[str, List[BaseFilter]] = None, + search_columns: Optional[List[str]] = None, + search_filters: Optional[Dict[str, List[BaseFilter]]] = None, ): """ diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index 10f99d8a79..b55ca507c1 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import logging import sys -from typing import List, Tuple +from typing import List, Optional, Tuple from flask_sqlalchemy import BaseQuery import sqlalchemy as sa @@ -10,8 +10,12 @@ from sqlalchemy.orm import aliased, contains_eager, Load from sqlalchemy.orm.descriptor_props import SynonymProperty from sqlalchemy.sql.elements import BinaryExpression +from sqlalchemy.sql.sqltypes import TypeEngine +from sqlalchemy.orm.session import Session as SessionBase from sqlalchemy_utils.types.uuid import UUIDType + +from flask_appbuilder.exceptions import InterfaceQueryWithoutSession from . import filters, Model from ..base import BaseInterface from ..filters import Filters @@ -32,17 +36,11 @@ log = logging.getLogger(__name__) -def _include_filters(obj): - for key in filters.__all__: - if not hasattr(obj, key): - setattr(obj, key, getattr(filters, key)) - - -def _is_sqla_type(obj, sa_type): +def _is_sqla_type(model: Model, sa_type: TypeEngine) -> bool: return ( - isinstance(obj, sa_type) - or isinstance(obj, sa.types.TypeDecorator) - and isinstance(obj.impl, sa_type) + isinstance(model, sa_type) + or isinstance(model, sa.types.TypeDecorator) + and isinstance(model.impl, sa_type) ) @@ -56,7 +54,7 @@ class SQLAInterface(BaseInterface): filter_converter_class = filters.SQLAFilterConverter - def __init__(self, obj, session=None): + def __init__(self, obj: Model, session: Optional[SessionBase] = None) -> None: _include_filters(self) self.list_columns = dict() self.list_properties = dict() @@ -80,7 +78,7 @@ def model_name(self): return self.obj.__name__ @staticmethod - def is_model_already_joined(query, model): + def is_model_already_joined(query: BaseQuery, model: Model) -> bool: return model in [mapper.class_ for mapper in query._join_entities] def _get_base_query( @@ -109,7 +107,9 @@ def _query_join_relation(self, query: BaseQuery, root_relation: str) -> BaseQuer model_relation = aliased(model_relation) # The binary expression needs to be inverted relation_join = BinaryExpression( - relation_join.left, model_relation.id, relation_join.operator + relation_join.left, + model_relation.__mapper__.primary_key[0], + relation_join.operator, ) query = query.join(model_relation, relation_join, isouter=True) return query @@ -156,7 +156,7 @@ def apply_order_by( return query def apply_pagination( - self, query: BaseQuery, page: int, page_size: int + self, query: BaseQuery, page: Optional[int], page_size: Optional[int] ) -> BaseQuery: if page and page_size: query = query.offset(page * page_size) @@ -231,7 +231,7 @@ def apply_outer_select_joins( query = self._apply_normal_col_select_option(query, column) return query - def get_inner_filters(self, filters: Filters) -> Filters: + def get_inner_filters(self, filters: Optional[Filters]) -> Filters: """ Inner filters are non dotted columns and one to many or one to one relations @@ -239,7 +239,7 @@ def get_inner_filters(self, filters: Filters) -> Filters: :param filters: All filters :return: New filtered filters to apply to an inner query """ - inner_filters = Filters(self.filter_converter_class, self, [], []) + inner_filters = Filters(self.filter_converter_class, self) _filters = [] if filters: for flt, value in zip(filters.filters, filters.values): @@ -254,12 +254,12 @@ def get_inner_filters(self, filters: Filters) -> Filters: def query( self, - filters=None, - order_column="", - order_direction="", - page=None, - page_size=None, - select_columns=None, + filters: Optional[Filters] = None, + order_column: str = "", + order_direction: str = "", + page: Optional[int] = None, + page_size: Optional[int] = None, + select_columns: Optional[List[str]] = None, ): """ Returns the results for a model query, applies filters, sorting and pagination @@ -274,7 +274,11 @@ def query( the current page :param page_size: the current page size + :param select_columns: + A List of columns to be specifically selected on the query """ + if not self.session: + raise InterfaceQueryWithoutSession() query = self.session.query(self.obj) inner_filters = self.get_inner_filters(filters) @@ -337,13 +341,13 @@ def query_year_group(self, group_by="", filters=None): def is_image(self, col_name: str) -> bool: try: return isinstance(self.list_columns[col_name].type, ImageColumn) - except Exception: + except KeyError: return False def is_file(self, col_name: str) -> bool: try: return isinstance(self.list_columns[col_name].type, FileColumn) - except Exception: + except KeyError: return False def is_string(self, col_name: str) -> bool: @@ -352,61 +356,61 @@ def is_string(self, col_name: str) -> bool: _is_sqla_type(self.list_columns[col_name].type, sa.types.String) or self.list_columns[col_name].type.__class__ == UUIDType ) - except Exception: + except KeyError: return False def is_text(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.Text) - except Exception: + except KeyError: return False def is_binary(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.LargeBinary) - except Exception: + except KeyError: return False def is_integer(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.Integer) - except Exception: + except KeyError: return False def is_numeric(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.Numeric) - except Exception: + except KeyError: return False def is_float(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.Float) - except Exception: + except KeyError: return False def is_boolean(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.Boolean) - except Exception: + except KeyError: return False def is_date(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.Date) - except Exception: + except KeyError: return False def is_datetime(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.DateTime) - except Exception: + except KeyError: return False def is_enum(self, col_name: str) -> bool: try: return _is_sqla_type(self.list_columns[col_name].type, sa.types.Enum) - except Exception: + except KeyError: return False def is_relation(self, col_name: str) -> bool: @@ -414,35 +418,39 @@ def is_relation(self, col_name: str) -> bool: return isinstance( self.list_properties[col_name], sa.orm.properties.RelationshipProperty ) - except Exception: + except KeyError: return False def is_relation_many_to_one(self, col_name: str) -> bool: try: if self.is_relation(col_name): return self.list_properties[col_name].direction.name == "MANYTOONE" - except Exception: + return False + except KeyError: return False def is_relation_many_to_many(self, col_name: str) -> bool: try: if self.is_relation(col_name): return self.list_properties[col_name].direction.name == "MANYTOMANY" - except Exception: + return False + except KeyError: return False def is_relation_one_to_one(self, col_name: str) -> bool: try: if self.is_relation(col_name): return self.list_properties[col_name].direction.name == "ONETOONE" - except Exception: + return False + except KeyError: return False def is_relation_one_to_many(self, col_name: str) -> bool: try: if self.is_relation(col_name): return self.list_properties[col_name].direction.name == "ONETOMANY" - except Exception: + return False + except KeyError: return False def is_nullable(self, col_name: str) -> bool: @@ -451,19 +459,19 @@ def is_nullable(self, col_name: str) -> bool: return col.nullable try: return self.list_columns[col_name].nullable - except Exception: + except KeyError: return False def is_unique(self, col_name: str) -> bool: try: return self.list_columns[col_name].unique is True - except Exception: + except KeyError: return False def is_pk(self, col_name: str) -> bool: try: return self.list_columns[col_name].primary_key - except Exception: + except KeyError: return False def is_pk_composite(self) -> bool: @@ -472,7 +480,7 @@ def is_pk_composite(self) -> bool: def is_fk(self, col_name: str) -> bool: try: return self.list_columns[col_name].foreign_keys - except Exception: + except KeyError: return False def is_property(self, col_name: str) -> bool: @@ -781,6 +789,16 @@ def get_pk_name(self): return pk if self.is_pk_composite() else pk[0] +def _include_filters(interface: SQLAInterface) -> None: + """ + Injects all filters on the interface class itself + :param interface: + """ + for key in filters.__all__: + if not hasattr(interface, key): + setattr(interface, key, getattr(filters, key)) + + """ For Retro-Compatibility """ From 8c34130662160e18564ba9457c1b786205a90ea1 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 23 Jun 2020 14:05:01 +0100 Subject: [PATCH 09/22] type annotations and code quality --- flask_appbuilder/models/base.py | 2 +- flask_appbuilder/models/sqla/interface.py | 31 ++++++++++------------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/flask_appbuilder/models/base.py b/flask_appbuilder/models/base.py index a4701a3945..8971da66a8 100644 --- a/flask_appbuilder/models/base.py +++ b/flask_appbuilder/models/base.py @@ -5,7 +5,7 @@ from flask_babel import lazy_gettext -from .filters import Filters, BaseFilterConverter +from .filters import BaseFilterConverter, Filters try: import enum diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index b55ca507c1..913aabb12e 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -9,13 +9,12 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import aliased, contains_eager, Load from sqlalchemy.orm.descriptor_props import SynonymProperty +from sqlalchemy.orm.session import Session as SessionBase from sqlalchemy.sql.elements import BinaryExpression from sqlalchemy.sql.sqltypes import TypeEngine -from sqlalchemy.orm.session import Session as SessionBase from sqlalchemy_utils.types.uuid import UUIDType -from flask_appbuilder.exceptions import InterfaceQueryWithoutSession from . import filters, Model from ..base import BaseInterface from ..filters import Filters @@ -30,6 +29,7 @@ LOGMSG_WAR_DBI_DEL_INTEGRITY, LOGMSG_WAR_DBI_EDIT_INTEGRITY, ) +from ...exceptions import InterfaceQueryWithoutSession from ...filemanager import FileManager, ImageManager from ...utils.base import get_column_leaf, get_column_root_relation, is_column_dotted @@ -106,25 +106,13 @@ def _query_join_relation(self, query: BaseQuery, root_relation: str) -> BaseQuer # Since the join already exists apply a new aliased one model_relation = aliased(model_relation) # The binary expression needs to be inverted + relation_pk = self.get_pk(model_relation) relation_join = BinaryExpression( - relation_join.left, - model_relation.__mapper__.primary_key[0], - relation_join.operator, + relation_join.left, relation_pk, relation_join.operator ) query = query.join(model_relation, relation_join, isouter=True) return query - def _query_join_dotted_column(self, query: BaseQuery, column: str) -> BaseQuery: - """ - - :param query: SQLAlchemy query object - :param column: If the column is dotted will join the root relation - :return: Transformed SQLAlchemy Query - """ - if is_column_dotted(column): - return self._query_join_relation(query, get_column_root_relation(column)) - return query - def apply_engine_specific_hack( self, query: BaseQuery, page, page_size, order_column ) -> BaseQuery: @@ -783,8 +771,15 @@ def get(self, id, filters=None): return query.first() return self.session.query(self.obj).get(id) - def get_pk_name(self): - pk = [pk.name for pk in self.obj.__mapper__.primary_key] + def get_pk_name(self) -> str: + return self._get_pk_name(self.obj) + + def get_pk(self, model: Optional[Model] = None): + model_ = model or self.obj + return getattr(model_, self._get_pk_name(model_)) + + def _get_pk_name(self, model: Model) -> str: + pk = [pk.name for pk in model.__mapper__.primary_key] if pk: return pk if self.is_pk_composite() else pk[0] From 1c5ab9f8c2abf2808540838628519f301da4484a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 23 Jun 2020 14:16:21 +0100 Subject: [PATCH 10/22] type annotations and code quality --- flask_appbuilder/models/sqla/interface.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index 913aabb12e..cfa2164365 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import logging import sys -from typing import List, Optional, Tuple +from typing import List, Optional, Tuple, Union from flask_sqlalchemy import BaseQuery import sqlalchemy as sa @@ -771,18 +771,28 @@ def get(self, id, filters=None): return query.first() return self.session.query(self.obj).get(id) - def get_pk_name(self) -> str: + def get_pk_name(self) -> Optional[Union[List[str], str]]: + """ + Get the model primary key column name. + """ return self._get_pk_name(self.obj) def get_pk(self, model: Optional[Model] = None): + """ + Get the model primary key SQLAlchemy column. + Will not support composite keys + """ model_ = model or self.obj - return getattr(model_, self._get_pk_name(model_)) + pk_name = self._get_pk_name(model_) + if pk_name and isinstance(pk_name, str): + return getattr(model_, pk_name) + return None - def _get_pk_name(self, model: Model) -> str: + def _get_pk_name(self, model: Model) -> Optional[Union[List[str], str]]: pk = [pk.name for pk in model.__mapper__.primary_key] if pk: return pk if self.is_pk_composite() else pk[0] - + return None def _include_filters(interface: SQLAInterface) -> None: """ From 47424d0ec0b37d382b231011a9dbeeb47e2c5c67 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 23 Jun 2020 14:26:06 +0100 Subject: [PATCH 11/22] lint --- flask_appbuilder/models/sqla/interface.py | 1 + 1 file changed, 1 insertion(+) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index cfa2164365..3d192983cb 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -794,6 +794,7 @@ def _get_pk_name(self, model: Model) -> Optional[Union[List[str], str]]: return pk if self.is_pk_composite() else pk[0] return None + def _include_filters(interface: SQLAInterface) -> None: """ Injects all filters on the interface class itself From 108146a6ccd0751460bab171540682a0d72a0939 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 23 Jun 2020 14:51:20 +0100 Subject: [PATCH 12/22] more type annotations --- flask_appbuilder/models/sqla/interface.py | 41 +++++++++++------------ 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index 3d192983cb..efb3ee8c52 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import logging import sys -from typing import List, Optional, Tuple, Union +from typing import Any, List, Optional, Type, Tuple, Union from flask_sqlalchemy import BaseQuery import sqlalchemy as sa @@ -36,7 +36,7 @@ log = logging.getLogger(__name__) -def _is_sqla_type(model: Model, sa_type: TypeEngine) -> bool: +def _is_sqla_type(model: Model, sa_type: Type[TypeEngine]) -> bool: return ( isinstance(model, sa_type) or isinstance(model, sa.types.TypeDecorator) @@ -248,7 +248,7 @@ def query( page: Optional[int] = None, page_size: Optional[int] = None, select_columns: Optional[List[str]] = None, - ): + ) -> Tuple[int, List[Type[Model]]]: """ Returns the results for a model query, applies filters, sorting and pagination @@ -651,25 +651,24 @@ def get_related_model_and_join(self, col_name: str) -> List[Tuple[Model, object] ] return [(relation.mapper.class_, relation.primaryjoin)] - def query_model_relation(self, col_name): - model = self.get_related_model(col_name) - return self.session.query(model).all() - - def get_related_interface(self, col_name): + def get_related_interface(self, col_name: str): return self.__class__(self.get_related_model(col_name), self.session) - def get_related_obj(self, col_name, value): + def get_related_obj(self, col_name: str, value: Any) -> Optional[Type[Model]]: rel_model = self.get_related_model(col_name) - return self.session.query(rel_model).get(value) + if self.session: + return self.session.query(rel_model).get(value) + return None - def get_related_fks(self, related_views): + def get_related_fks(self, related_views) -> List[str]: return [view.datamodel.get_related_fk(self.obj) for view in related_views] - def get_related_fk(self, model): + def get_related_fk(self, model) -> Optional[str]: for col_name in self.list_properties.keys(): if self.is_relation(col_name): if model == self.get_related_model(col_name): return col_name + return None def get_info(self, col_name): if col_name in self.list_properties: @@ -682,15 +681,15 @@ def get_info(self, col_name): ------------- """ - def get_columns_list(self): + def get_columns_list(self) -> List[str]: """ - Returns all model's columns on SQLA properties + Returns all model's columns on SQLA properties """ return list(self.list_properties.keys()) - def get_user_columns_list(self): + def get_user_columns_list(self) -> List[str]: """ - Returns all model's columns except pk or fk + Returns all model's columns except pk or fk """ ret_lst = list() for col_name in self.get_columns_list(): @@ -699,7 +698,7 @@ def get_user_columns_list(self): return ret_lst # TODO get different solution, more integrated with filters - def get_search_columns_list(self): + def get_search_columns_list(self) -> List[str]: ret_lst = list() for col_name in self.get_columns_list(): if not self.is_relation(col_name): @@ -715,12 +714,12 @@ def get_search_columns_list(self): ret_lst.append(col_name) return ret_lst - def get_order_columns_list(self, list_columns=None): + def get_order_columns_list(self, list_columns: List[str] = None) -> List[str]: """ - Returns the columns that can be ordered + Returns the columns that can be ordered - :param list_columns: optional list of columns name, if provided will - use this list only. + :param list_columns: optional list of columns name, if provided will + use this list only. """ ret_lst = list() list_columns = list_columns or self.get_columns_list() From db2127d51f0ac9b560476058848bea1df662bff2 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 23 Jun 2020 15:24:14 +0100 Subject: [PATCH 13/22] only use from_self if necessary --- flask_appbuilder/models/sqla/interface.py | 33 ++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index efb3ee8c52..54fc593c2e 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -240,6 +240,15 @@ def get_inner_filters(self, filters: Optional[Filters]) -> Filters: inner_filters.add_filter_list(_filters) return inner_filters + def exists_col_to_many(self, select_columns: List[str]) -> bool: + for column in select_columns: + if is_column_dotted(column): + root_relation = get_column_root_relation(column) + return self.is_relation_many_to_many( + root_relation + ) or self.is_relation_one_to_many(root_relation) + return False + def query( self, filters: Optional[Filters] = None, @@ -281,15 +290,21 @@ def query( inner_query = self.apply_order_by(inner_query, order_column, order_direction) inner_query = self.apply_pagination(inner_query, page, page_size) - outer_query = inner_query.from_self() - - if select_columns and order_column: - select_columns = select_columns + [order_column] - outer_query = self.apply_outer_select_joins(outer_query, select_columns) - outer_query = self.apply_order_by(outer_query, order_column, order_direction) + # Only use a from_self if we need to select a join one to many or many to many + if select_columns and self.exists_col_to_many(select_columns): + if select_columns and order_column: + select_columns = select_columns + [order_column] + outer_query = inner_query.from_self() + outer_query = self.apply_outer_select_joins(outer_query, select_columns) + final_query = self.apply_order_by( + outer_query, order_column, order_direction + ) + else: + final_query = inner_query result = list() - results = outer_query.all() + results = final_query.all() + for item in results: if hasattr(item, self.obj.__name__): result.append(getattr(item, self.obj.__name__)) @@ -748,11 +763,11 @@ def get_image_column_list(self): if isinstance(i.type, ImageColumn) ] - def get_property_first_col(self, col_name): + def get_property_first_col(self, col_name: str): # support for only one col for pk and fk return self.list_properties[col_name].columns[0] - def get_relation_fk(self, col_name): + def get_relation_fk(self, col_name: str): # support for only one col for pk and fk return list(self.list_properties[col_name].local_columns)[0] From b5bba747495f3bc3596e59202170fea570b5305a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 23 Jun 2020 21:45:39 +0100 Subject: [PATCH 14/22] fix column model query loading for non inner queries --- flask_appbuilder/models/sqla/interface.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index 54fc593c2e..4a6bcb293e 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import logging import sys -from typing import Any, List, Optional, Type, Tuple, Union +from typing import Any, List, Optional, Tuple, Type, Union from flask_sqlalchemy import BaseQuery import sqlalchemy as sa @@ -187,8 +187,10 @@ def apply_inner_select_joins( ) or self.is_relation_one_to_one(root_relation): if root_relation not in joined_models: query = self._query_join_relation(query, root_relation) - related_model = self.get_related_model(root_relation) - query = query.add_entity(related_model) + # only needed if we need to wrap this query, from_self + if select_columns and self.exists_col_to_many(select_columns): + related_model = self.get_related_model(root_relation) + query = query.add_entity(related_model) joined_models.append(root_relation) query = query.options( (contains_eager(root_relation).load_only(leaf_column)) @@ -244,9 +246,10 @@ def exists_col_to_many(self, select_columns: List[str]) -> bool: for column in select_columns: if is_column_dotted(column): root_relation = get_column_root_relation(column) - return self.is_relation_many_to_many( + if self.is_relation_many_to_many( root_relation - ) or self.is_relation_one_to_many(root_relation) + ) or self.is_relation_one_to_many(root_relation): + return True return False def query( From f1041feb1e77b59eef820a73244f54fc93ab2b5c Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Sun, 28 Jun 2020 00:23:13 +0100 Subject: [PATCH 15/22] Drying and fix get with improved performance --- flask_appbuilder/api/__init__.py | 2 +- flask_appbuilder/models/sqla/interface.py | 145 +++++++++++++++++----- flask_appbuilder/tests/test_api.py | 4 +- tox.ini | 10 +- 4 files changed, 119 insertions(+), 42 deletions(-) diff --git a/flask_appbuilder/api/__init__.py b/flask_appbuilder/api/__init__.py index 174b8d0b85..91a63ca6e1 100644 --- a/flask_appbuilder/api/__init__.py +++ b/flask_appbuilder/api/__init__.py @@ -1285,7 +1285,7 @@ def get_headless(self, pk, **kwargs) -> Response: :param kwargs: Query string parameter arguments :return: HTTP Response """ - item = self.datamodel.get(pk, self._base_filters) + item = self.datamodel.get(pk, self._base_filters, self.show_columns) if not item: return self.response_404() diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index 4a6bcb293e..b0e30914aa 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -9,6 +9,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import aliased, contains_eager, Load from sqlalchemy.orm.descriptor_props import SynonymProperty +from sqlalchemy.orm.query import Query from sqlalchemy.orm.session import Session as SessionBase from sqlalchemy.sql.elements import BinaryExpression from sqlalchemy.sql.sqltypes import TypeEngine @@ -152,7 +153,7 @@ def apply_pagination( query = query.limit(page_size) return query - def apply_filters(self, query: BaseQuery, filters: Filters) -> BaseQuery: + def apply_filters(self, query: BaseQuery, filters: Optional[Filters]) -> BaseQuery: if filters: return filters.apply_all(query) return query @@ -252,18 +253,49 @@ def exists_col_to_many(self, select_columns: List[str]) -> bool: return True return False - def query( + def _apply_inner_all( self, + query: Query, filters: Optional[Filters] = None, order_column: str = "", order_direction: str = "", page: Optional[int] = None, page_size: Optional[int] = None, select_columns: Optional[List[str]] = None, - ) -> Tuple[int, List[Type[Model]]]: + ): + inner_filters = self.get_inner_filters(filters) + query = self.apply_inner_select_joins(query, select_columns) + query = self.apply_filters(query, inner_filters) + query = self.apply_engine_specific_hack(query, page, page_size, order_column) + query = self.apply_order_by(query, order_column, order_direction) + query = self.apply_pagination(query, page, page_size) + return query + + def query_count( + self, + query: Query, + filters: Optional[Filters] = None, + select_columns: Optional[List[str]] = None, + ): + return self._apply_inner_all( + query, filters, select_columns=select_columns + ).count() + + def apply_all( + self, + query: Query, + filters: Optional[Filters] = None, + order_column: str = "", + order_direction: str = "", + page: Optional[int] = None, + page_size: Optional[int] = None, + select_columns: Optional[List[str]] = None, + ) -> BaseQuery: """ - Returns the results for a model query, applies filters, sorting and pagination + Accepts a SQLAlchemy Query and applies all filtering logic, order by and + pagination. + :param query: The query to apply all :param filters: dict with filters {: Tuple[int, List[Model]]: + """ + Returns the results for a model query, applies filters, sorting and pagination - for item in results: + :param filters: A Filter class that contains all filters to apply + :param order_column: name of the column to order + :param order_direction: the direction to order <'asc'|'desc'> + :param page: the current page + :param page_size: the current page size + :param select_columns: A List of columns to be specifically selected + on the query. Supports dotted notation. + :return: A tuple with the query count (non paginated) and the results + """ + if not self.session: + raise InterfaceQueryWithoutSession() + query = self.session.query(self.obj) + + count = self.query_count(query, filters, select_columns) + query = self.apply_all( + query, + filters, + order_column, + order_direction, + page, + page_size, + select_columns, + ) + query_results = query.all() + + result = list() + for item in query_results: if hasattr(item, self.obj.__name__): result.append(getattr(item, self.obj.__name__)) else: - return count, results + return count, query_results return count, result def query_simple_group( @@ -766,17 +825,28 @@ def get_image_column_list(self): if isinstance(i.type, ImageColumn) ] - def get_property_first_col(self, col_name: str): + def get_property_first_col(self, col_name: str) -> str: # support for only one col for pk and fk return self.list_properties[col_name].columns[0] - def get_relation_fk(self, col_name: str): + def get_relation_fk(self, col_name: str) -> str: # support for only one col for pk and fk return list(self.list_properties[col_name].local_columns)[0] - def get(self, id, filters=None): + def get( + self, id, filters=Optional[Filters], select_columns: Optional[List[str]] = None + ) -> Optional[Model]: + """ + Returns the result for a model get, applies filters and supports dotted + notation for joins and granular selecting query columns. + + :param id: The model id (pk). + :param filters: A Filter class that contains all filters to apply. + :param select_columns: A List of columns to be specifically selected. + on the query. Supports dotted notation. + :return: + """ if filters: - query = self.session.query(self.obj) _filters = filters.copy() pk = self.get_pk_name() if self.is_pk_composite(): @@ -784,9 +854,16 @@ def get(self, id, filters=None): _filters.add_filter(_pk, self.FilterEqual, _id) else: _filters.add_filter(pk, self.FilterEqual, id) - query = self._get_base_query(query=query, filters=_filters) - return query.first() - return self.session.query(self.obj).get(id) + else: + _filters = filters + query = self.session.query(self.obj) + item = self.apply_all( + query, _filters, select_columns=select_columns + ).one_or_none() + if item: + if hasattr(item, self.obj.__name__): + return getattr(item, self.obj.__name__) + return item def get_pk_name(self) -> Optional[Union[List[str], str]]: """ diff --git a/flask_appbuilder/tests/test_api.py b/flask_appbuilder/tests/test_api.py index 6453d9d3ef..782c306bd4 100644 --- a/flask_appbuilder/tests/test_api.py +++ b/flask_appbuilder/tests/test_api.py @@ -460,7 +460,7 @@ def test_auth_authorization(self): token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) # Test unauthorized DELETE pk = 1 - uri = "api/v1/model1apirestrictedpermissions/{}".format(pk) + uri = f"api/v1/model1apirestrictedpermissions/{pk}" rv = self.auth_client_delete(client, token, uri) self.assertEqual(rv.status_code, 401) # Test unauthorized POST @@ -474,7 +474,7 @@ def test_auth_authorization(self): rv = self.auth_client_post(client, token, uri, item) self.assertEqual(rv.status_code, 401) # Test authorized GET - uri = "api/v1/model1apirestrictedpermissions/1" + uri = f"api/v1/model1apirestrictedpermissions/{pk}" rv = self.auth_client_get(client, token, uri) self.assertEqual(rv.status_code, 200) diff --git a/tox.ini b/tox.ini index 833a897be9..e772cfdc9d 100644 --- a/tox.ini +++ b/tox.ini @@ -16,29 +16,29 @@ deps = setenv = SQLALCHEMY_DATABASE_URI = sqlite:/// commands = - nosetests -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder.tests --ignore-files="test_mongoengine\.py" + nosetests --stop -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder.tests --ignore-files="test_mongoengine\.py" [testenv:mysql] setenv = SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@0.0.0.0/app?charset=utf8 commands = - nosetests -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder.tests --ignore-files="test_mongoengine\.py" + nosetests --stop -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder.tests --ignore-files="test_mongoengine\.py" [testenv:postgres] setenv = SQLALCHEMY_DATABASE_URI = postgresql+psycopg2://pguser:pguserpassword@0.0.0.0/app commands = - nosetests -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder.tests --ignore-files="test_mongoengine\.py" + nosetests --stop -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder.tests --ignore-files="test_mongoengine\.py" [testenv:mssql] setenv = SQLALCHEMY_DATABASE_URI = mssql+pymssql://sa:Password_123@localhost:1433/master commands = - nosetests -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder.tests --ignore-files="test_mongoengine\.py" + nosetests --stop -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder.tests --ignore-files="test_mongoengine\.py" [testenv:mongodb] commands = - nosetests -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder/tests/test_mongoengine.py + nosetests --stop -v --with-coverage --cover-package=flask_appbuilder flask_appbuilder/tests/test_mongoengine.py [testenv:black] commands = From 1085568a23f2bbe6b791e5e61714147395805184 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Sun, 28 Jun 2020 09:47:29 +0100 Subject: [PATCH 16/22] MVC starts using select columns, maybe revert --- flask_appbuilder/baseviews.py | 1 + flask_appbuilder/models/sqla/interface.py | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/flask_appbuilder/baseviews.py b/flask_appbuilder/baseviews.py index 46639bdf1e..5526d785d1 100644 --- a/flask_appbuilder/baseviews.py +++ b/flask_appbuilder/baseviews.py @@ -1024,6 +1024,7 @@ def _get_list_widget( order_direction, page=page, page_size=page_size, + select_columns=self.list_columns, ) pks = self.datamodel.get_keys(lst) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index b0e30914aa..0706f2042d 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -834,7 +834,10 @@ def get_relation_fk(self, col_name: str) -> str: return list(self.list_properties[col_name].local_columns)[0] def get( - self, id, filters=Optional[Filters], select_columns: Optional[List[str]] = None + self, + id, + filters: Optional[Filters] = None, + select_columns: Optional[List[str]] = None, ) -> Optional[Model]: """ Returns the result for a model get, applies filters and supports dotted @@ -846,16 +849,18 @@ def get( on the query. Supports dotted notation. :return: """ + pk = self.get_pk_name() if filters: _filters = filters.copy() - pk = self.get_pk_name() - if self.is_pk_composite(): - for _pk, _id in zip(pk, id): - _filters.add_filter(_pk, self.FilterEqual, _id) - else: - _filters.add_filter(pk, self.FilterEqual, id) else: - _filters = filters + _filters = Filters(self.filter_converter_class, self) + _filters.add_filter(pk, self.FilterEqual, id) + + if self.is_pk_composite(): + for _pk, _id in zip(pk, id): + _filters.add_filter(_pk, self.FilterEqual, _id) + else: + _filters.add_filter(pk, self.FilterEqual, id) query = self.session.query(self.obj) item = self.apply_all( query, _filters, select_columns=select_columns From 00f4e5de04b167824d4b330a40a189c533c55fc8 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Sun, 28 Jun 2020 16:46:12 +0100 Subject: [PATCH 17/22] revert select_columns from mvc --- flask_appbuilder/baseviews.py | 1 - flask_appbuilder/models/sqla/interface.py | 4 ++++ flask_appbuilder/tests/test_mvc.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/flask_appbuilder/baseviews.py b/flask_appbuilder/baseviews.py index 5526d785d1..46639bdf1e 100644 --- a/flask_appbuilder/baseviews.py +++ b/flask_appbuilder/baseviews.py @@ -1024,7 +1024,6 @@ def _get_list_widget( order_direction, page=page, page_size=page_size, - select_columns=self.list_columns, ) pks = self.datamodel.get_keys(lst) diff --git a/flask_appbuilder/models/sqla/interface.py b/flask_appbuilder/models/sqla/interface.py index 0706f2042d..028c0d9f65 100644 --- a/flask_appbuilder/models/sqla/interface.py +++ b/flask_appbuilder/models/sqla/interface.py @@ -138,6 +138,10 @@ def apply_order_by( if hasattr(getattr(self.obj, order_column), "_col_name"): order_column = getattr(self._get_attr(order_column), "_col_name") _order_column = self._get_attr(order_column) or order_column + if is_column_dotted(order_column): + query = self._query_join_relation( + query, get_column_root_relation(order_column) + ) if order_direction == "asc": query = query.order_by(asc(_order_column)) else: diff --git a/flask_appbuilder/tests/test_mvc.py b/flask_appbuilder/tests/test_mvc.py index 18dab80f36..e2f7fe53e5 100644 --- a/flask_appbuilder/tests/test_mvc.py +++ b/flask_appbuilder/tests/test_mvc.py @@ -381,7 +381,7 @@ class Model22View(ModelView): class Model1View(ModelView): datamodel = SQLAInterface(Model1) related_views = [Model2View] - list_columns = ["field_string", "field_file"] + list_columns = ["field_string", "field_integer"] class Model3View(ModelView): datamodel = SQLAInterface(Model3) From 7cd065fb5fb97785524afbec8b91fd2fba78e9e1 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 30 Jun 2020 00:22:36 +0100 Subject: [PATCH 18/22] feat: list_select_columns to enable wider config --- flask_appbuilder/api/__init__.py | 95 ++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 40 deletions(-) diff --git a/flask_appbuilder/api/__init__.py b/flask_appbuilder/api/__init__.py index 91a63ca6e1..ce2a6521f3 100644 --- a/flask_appbuilder/api/__init__.py +++ b/flask_appbuilder/api/__init__.py @@ -3,7 +3,7 @@ import logging import re import traceback -from typing import Dict, Optional +from typing import Callable, Dict, List, Optional, Set import urllib.parse from apispec import APISpec, yaml_utils @@ -11,7 +11,7 @@ from flask import Blueprint, current_app, jsonify, make_response, request, Response from flask_babel import lazy_gettext as _ import jsonschema -from marshmallow import ValidationError +from marshmallow import Schema, ValidationError from marshmallow_sqlalchemy.fields import Related, RelatedList import prison from sqlalchemy.exc import IntegrityError @@ -214,22 +214,22 @@ class BaseApi(object): appbuilder = None blueprint = None - endpoint = None + endpoint: Optional[str] = None - version = "v1" + version: Optional[str] = "v1" """ Define the Api version for this resource/class """ - route_base = None + route_base: Optional[str] = None """ Define the route base where all methods will suffix from """ - resource_name = None + resource_name: Optional[str] = None """ Defines a custom resource name, overrides the inferred from Class name makes no sense to use it with route base """ - base_permissions = None + base_permissions: Optional[List[str]] = None """ A list of allowed base permissions:: @@ -237,16 +237,16 @@ class ExampleApi(BaseApi): base_permissions = ['can_get'] """ - class_permission_name = None + class_permission_name: Optional[str] = None """ Override class permission name default fallback to self.__class__.__name__ """ - previous_class_permission_name = None + previous_class_permission_name: Optional[str] = None """ If set security converge will replace all permissions tuples with this name by the class_permission_name or self.__class__.__name__ """ - method_permission_name = None + method_permission_name: Optional[Dict[str, str]] = None """ Override method permission names, example:: @@ -258,7 +258,7 @@ class ExampleApi(BaseApi): 'delete': 'write' } """ - previous_method_permission_name = None + previous_method_permission_name: Optional[Dict[str, str]] = None """ Use same structure as method_permission_name. If set security converge will replace all method permissions by the new ones @@ -272,7 +272,7 @@ class ExampleApi(BaseApi): """ If using flask-wtf CSRFProtect exempt the API from check """ - apispec_parameter_schemas = None + apispec_parameter_schemas: Optional[Dict[str, Dict]] = None """ Set your custom Rison parameter schemas here so that they get registered on the OpenApi spec:: @@ -377,7 +377,7 @@ class ContactModelView(ModelRestApi): The previous examples will only register the `put`, `post` and `delete` routes """ - include_route_methods = None + include_route_methods: Set[str] = None """ If defined will assume a white list setup, where all endpoints are excluded except those define on this attribute @@ -412,7 +412,7 @@ class GreetingApi(BaseApi): Use this attribute to override the tag name """ - def __init__(self): + def __init__(self) -> None: """ Initialization of base permissions based on exposed methods and actions @@ -855,72 +855,83 @@ class ModelRestApi(BaseModelApi): List Title, if not configured the default is 'List ' with pretty model name """ - show_title = "" + show_title: Optional[str] = "" """ Show Title , if not configured the default is 'Show ' with pretty model name """ - add_title = "" + add_title: Optional[str] = "" """ Add Title , if not configured the default is 'Add ' with pretty model name """ - edit_title = "" + edit_title: Optional[str] = "" """ Edit Title , if not configured the default is 'Edit ' with pretty model name """ - - list_columns = None + list_select_columns: Optional[List[str]] = None + """ + A List of column names that will be included on the SQL select. + This is useful for including all necessary columns that are referenced + by properties listed on `list_columns` without generating N+1 queries. + """ + list_columns: Optional[List[str]] = None """ A list of columns (or model's methods) to be displayed on the list view. Use it to control the order of the display """ - show_columns = None + show_select_columns: Optional[List[str]] = None + """ + A List of column names that will be included on the SQL select. + This is useful for including all necessary columns that are referenced + by properties listed on `show_columns` without generating N+1 queries. + """ + show_columns: Optional[List[str]] = None """ A list of columns (or model's methods) for the get item endpoint. Use it to control the order of the results """ - add_columns = None + add_columns: Optional[List[str]] = None """ A list of columns (or model's methods) to be allowed to post """ - edit_columns = None + edit_columns: Optional[List[str]] = None """ A list of columns (or model's methods) to be allowed to update """ - list_exclude_columns = None + list_exclude_columns: Optional[List[str]] = None """ A list of columns to exclude from the get list endpoint. By default all columns are included. """ - show_exclude_columns = None + show_exclude_columns: Optional[List[str]] = None """ A list of columns to exclude from the get item endpoint. By default all columns are included. """ - add_exclude_columns = None + add_exclude_columns: Optional[List[str]] = None """ A list of columns to exclude from the add endpoint. By default all columns are included. """ - edit_exclude_columns = None + edit_exclude_columns: Optional[List[str]] = None """ A list of columns to exclude from the edit endpoint. By default all columns are included. """ - order_columns = None + order_columns: Optional[List[str]] = None """ Allowed order columns """ page_size = 20 """ Use this property to change default page size """ - max_page_size = None + max_page_size: Optional[int] = None """ class override for the FAB_API_MAX_SIZE, use special -1 to allow for any page size """ - description_columns = None + description_columns: Optional[Dict[str, str]] = None """ Dictionary with column descriptions that will be shown on the forms:: @@ -930,8 +941,8 @@ class MyView(ModelView): description_columns = {'name':'your models name column', 'address':'the address column'} """ - validators_columns = None - """ Dictionary to add your own validators for forms """ + validators_columns: Optional[Dict[str, Callable]] = None + """ Dictionary to add your own marshmallow validators """ add_query_rel_fields = None """ @@ -973,22 +984,22 @@ class ContactModelView(ModelRestApi): 'gender': ('name', 'asc') } """ - list_model_schema = None + list_model_schema: Optional[Schema] = None """ Override to provide your own marshmallow Schema for JSON to SQLA dumps """ - add_model_schema = None + add_model_schema: Optional[Schema] = None """ Override to provide your own marshmallow Schema for JSON to SQLA dumps """ - edit_model_schema = None + edit_model_schema: Optional[Schema] = None """ Override to provide your own marshmallow Schema for JSON to SQLA dumps """ - show_model_schema = None + show_model_schema: Optional[Schema] = None """ Override to provide your own marshmallow Schema for JSON to SQLA dumps @@ -1069,7 +1080,7 @@ def _init_titles(self): self.show_title = "Show " + self._prettify_name(class_name) self.title = self.list_title - def _init_properties(self): + def _init_properties(self) -> None: """ Init Properties """ @@ -1091,6 +1102,7 @@ def _init_properties(self): for x in self.datamodel.get_user_columns_list() if x not in self.list_exclude_columns ] + self.list_select_columns = self.list_select_columns or self.list_columns self.order_columns = ( self.order_columns @@ -1101,6 +1113,8 @@ def _init_properties(self): self.show_columns = [ x for x in list_cols if x not in self.show_exclude_columns ] + self.show_select_columns = self.show_select_columns or self.show_columns + if not self.add_columns: self.add_columns = [ x for x in list_cols if x not in self.add_exclude_columns @@ -1285,7 +1299,7 @@ def get_headless(self, pk, **kwargs) -> Response: :param kwargs: Query string parameter arguments :return: HTTP Response """ - item = self.datamodel.get(pk, self._base_filters, self.show_columns) + item = self.datamodel.get(pk, self._base_filters, self.show_select_columns) if not item: return self.response_404() @@ -1377,13 +1391,15 @@ def get_list_headless(self, **kwargs) -> Response: # handle select columns select_cols = _args.get(API_SELECT_COLUMNS_RIS_KEY, []) _pruned_select_cols = [col for col in select_cols if col in self.list_columns] + # map decorated metadata self.set_response_key_mappings( _response, self.get_list, _args, **{API_SELECT_COLUMNS_RIS_KEY: _pruned_select_cols}, ) - + # Create a response schema with the computed response columns, + # defined or requested if _pruned_select_cols: _list_model_schema = self.model2schemaconverter.convert(_pruned_select_cols) else: @@ -1401,14 +1417,13 @@ def get_list_headless(self, **kwargs) -> Response: # handle pagination page_index, page_size = self._handle_page_args(_args) # Make the query - query_select_columns = _pruned_select_cols or self.list_columns count, lst = self.datamodel.query( joined_filters, order_column, order_direction, page=page_index, page_size=page_size, - select_columns=query_select_columns, + select_columns=self.list_select_columns, ) pks = self.datamodel.get_keys(lst) _response[API_RESULT_RES_KEY] = _list_model_schema.dump(lst, many=True) From a570de6f0527d47cf64977b2779e9634d66900f2 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 30 Jun 2020 21:43:50 +0100 Subject: [PATCH 19/22] one or none instead of first --- flask_appbuilder/security/sqla/manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/flask_appbuilder/security/sqla/manager.py b/flask_appbuilder/security/sqla/manager.py index 93bc01b977..320d01c534 100644 --- a/flask_appbuilder/security/sqla/manager.py +++ b/flask_appbuilder/security/sqla/manager.py @@ -295,7 +295,7 @@ def find_permission(self, name): Finds and returns a Permission by name """ return ( - self.get_session.query(self.permission_model).filter_by(name=name).first() + self.get_session.query(self.permission_model).filter_by(name=name).one_or_none() ) def exist_permission_on_roles( @@ -417,7 +417,7 @@ def find_view_menu(self, name): """ Finds and returns a ViewMenu by name """ - return self.get_session.query(self.viewmenu_model).filter_by(name=name).first() + return self.get_session.query(self.viewmenu_model).filter_by(name=name).one_or_none() def get_all_view_menu(self): return self.get_session.query(self.viewmenu_model).all() @@ -485,7 +485,7 @@ def find_permission_view_menu(self, permission_name, view_menu_name): return ( self.get_session.query(self.permissionview_model) .filter_by(permission=permission, view_menu=view_menu) - .first() + .one_or_none() ) def find_permissions_view_menu(self, view_menu): @@ -537,7 +537,7 @@ def del_permission_view_menu(self, permission_name, view_menu_name, cascade=True roles_pvs = ( self.get_session.query(self.role_model) .filter(self.role_model.permissions.contains(pv)) - .first() + .one_or_none() ) if roles_pvs: log.warning( From deecae8e3afb0ba9d649e3f14e29479637626a0c Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 30 Jun 2020 22:05:06 +0100 Subject: [PATCH 20/22] doc show_select_columns and list_select_columns --- docs/rest_api.rst | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/docs/rest_api.rst b/docs/rest_api.rst index 4fda4b0693..f3a16eaaff 100644 --- a/docs/rest_api.rst +++ b/docs/rest_api.rst @@ -138,7 +138,7 @@ so data can be translated back and forth without loss or guesswork:: if 'name' in kwargs['rison']: return self.response( 200, - message="Hello {}".format(kwargs['rison']['name']) + message=f"Hello {kwargs['rison']['name']}" ) return self.response_400(message="Please send your name") @@ -238,7 +238,7 @@ validate your Rison arguments, this way you can implement a very strict API easi def greeting4(self, **kwargs): return self.response( 200, - message="Hello {}".format(kwargs['rison']['name']) + message=f"Hello {kwargs['rison']['name']}" ) Finally to properly handle all possible exceptions use the ``safe`` decorator, @@ -396,7 +396,7 @@ easily reference them:: """ return self.response( 200, - message="Hello {}".format(kwargs['rison']['name']) + message=f"Hello {kwargs['rison']['name']}" ) @@ -1015,6 +1015,33 @@ the ``show_columns`` property. This takes precedence from the *Rison* arguments: datamodel = SQLAInterface(Contact) show_columns = ['name'] +By default FAB will issue a query containing the exact fields for `show_columns`, but these are also associated with +the response object. Sometimes it's useful to distinguish between the query select columns and the response itself. +Imagine the case you want to use a `@property` to further transform the output, and that transformation implies +two model fields (concat or sum for example):: + + class ContactModelApi(ModelRestApi): + resource_name = 'contact' + datamodel = SQLAInterface(Contact) + show_columns = ['name', 'birthday'] + show_select_columns = ['name', 'age'] + + +The Model:: + + class Contact(Model): + id = Column(Integer, primary_key=True) + name = Column(String(150), unique=True, nullable=False) + ... + birthday = Column(Date, nullable=True) + ... + + @property + def age(self): + return date.today().year - self.birthday.year + +Note: The same principal exists on `list_select_columns` + We can add fields that are python functions also, for this on the SQLAlchemy definition, let's add a new function:: @@ -1034,7 +1061,7 @@ let's add a new function:: return self.name def some_function(self): - return "Hello {}".format(self.name) + return f"Hello {self.name}" And then on the REST API:: From ecd5bfb6a4a4b8a47babb40ba4aff4031eedf2f0 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 1 Jul 2020 12:46:46 +0100 Subject: [PATCH 21/22] fix, one or none instead of first --- flask_appbuilder/security/sqla/manager.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/flask_appbuilder/security/sqla/manager.py b/flask_appbuilder/security/sqla/manager.py index 320d01c534..35f1710e8c 100644 --- a/flask_appbuilder/security/sqla/manager.py +++ b/flask_appbuilder/security/sqla/manager.py @@ -272,7 +272,9 @@ def update_role(self, pk, name: str) -> Optional[Role]: return def find_role(self, name): - return self.get_session.query(self.role_model).filter_by(name=name).first() + return ( + self.get_session.query(self.role_model).filter_by(name=name).one_or_none() + ) def get_all_roles(self): return self.get_session.query(self.role_model).all() @@ -281,7 +283,7 @@ def get_public_role(self): return ( self.get_session.query(self.role_model) .filter_by(name=self.auth_role_public) - .first() + .one_or_none() ) def get_public_permissions(self): @@ -295,7 +297,9 @@ def find_permission(self, name): Finds and returns a Permission by name """ return ( - self.get_session.query(self.permission_model).filter_by(name=name).one_or_none() + self.get_session.query(self.permission_model) + .filter_by(name=name) + .one_or_none() ) def exist_permission_on_roles( @@ -417,7 +421,11 @@ def find_view_menu(self, name): """ Finds and returns a ViewMenu by name """ - return self.get_session.query(self.viewmenu_model).filter_by(name=name).one_or_none() + return ( + self.get_session.query(self.viewmenu_model) + .filter_by(name=name) + .one_or_none() + ) def get_all_view_menu(self): return self.get_session.query(self.viewmenu_model).all() From 9036f6538d77454571faffe60c11f11ccd7b29d1 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 2 Jul 2020 18:35:09 +0100 Subject: [PATCH 22/22] fix, docs --- docs/rest_api.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rest_api.rst b/docs/rest_api.rst index f3a16eaaff..7f0c6cf7f1 100644 --- a/docs/rest_api.rst +++ b/docs/rest_api.rst @@ -1023,8 +1023,8 @@ two model fields (concat or sum for example):: class ContactModelApi(ModelRestApi): resource_name = 'contact' datamodel = SQLAInterface(Contact) - show_columns = ['name', 'birthday'] - show_select_columns = ['name', 'age'] + show_columns = ['name', 'age'] + show_select_columns = ['name', 'birthday'] The Model:: @@ -1040,7 +1040,7 @@ The Model:: def age(self): return date.today().year - self.birthday.year -Note: The same principal exists on `list_select_columns` +Note: The same logic is applied on `list_select_columns` We can add fields that are python functions also, for this on the SQLAlchemy definition, let's add a new function::