ARIA-125-Filtering-returns-the-wrong-models
Project: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/commit/2d834753 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/tree/2d834753 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/diff/2d834753 Branch: refs/heads/ARIA-48-aria-cli Commit: 2d834753a03564d1cd1f413268b5d769d3144845 Parents: 16ae46a Author: max-orlov <ma...@gigaspaces.com> Authored: Sun Apr 2 14:53:46 2017 +0300 Committer: max-orlov <ma...@gigaspaces.com> Committed: Sun Apr 2 15:35:13 2017 +0300 ---------------------------------------------------------------------- aria/storage/sql_mapi.py | 50 +++-------- tests/mock/models.py | 13 +-- tests/mock/topology.py | 15 ++-- tests/modeling/test_model_storage.py | 102 ---------------------- tests/modeling/test_models.py | 12 +-- tests/storage/test_model_storage.py | 139 ++++++++++++++++++++++++++++++ 6 files changed, 169 insertions(+), 162 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/2d834753/aria/storage/sql_mapi.py ---------------------------------------------------------------------- diff --git a/aria/storage/sql_mapi.py b/aria/storage/sql_mapi.py index 8bbec54..59e1896 100644 --- a/aria/storage/sql_mapi.py +++ b/aria/storage/sql_mapi.py @@ -187,55 +187,31 @@ class SQLAlchemyModelAPI(api.ModelAPI): # If all columns should be returned, query directly from the model query = self._session.query(self.model_cls) - if not self._skip_joining(joins, include): - for join_table in joins: - query = query.join(join_table) - + query = query.join(*joins) return query @staticmethod def _get_joins(model_class, columns): """Get a list of all the tables on which we need to join - :param columns: A set of all columns involved in the query + :param columns: A set of all attributes involved in the query """ - joins = [] # Using a list instead of a set because order is important + + # Using a list instead of a set because order is important + joins = OrderedDict() for column_name in columns: column = getattr(model_class, column_name) while not column.is_attribute: + join_attr = column.local_attr + # This is a hack, to deal with the fact that SQLA doesn't + # fully support doing something like: `if join_attr in joins`, + # because some SQLA elements have their own comparators + join_attr_name = str(join_attr) + if join_attr_name not in joins: + joins[join_attr_name] = join_attr column = column.remote_attr - if column.is_attribute: - join_class = column.class_ - else: - join_class = column.local_attr.class_ - - # Don't add the same class more than once - if join_class not in joins: - joins.append(join_class) - return joins - - @staticmethod - def _skip_joining(joins, include): - """Dealing with an edge case where the only included column comes from - an other table. In this case, we mustn't join on the same table again - :param joins: A list of tables on which we're trying to join - :param include: The list of - :return: True if we need to skip joining - """ - if not joins: - return True - join_table_names = [t.__tablename__ for t in joins] - - if len(include) != 1: - return False - - column = include[0] - if column.is_clause_element: - table_name = column.element.table.name - else: - table_name = column.class_.__tablename__ - return table_name in join_table_names + return joins.values() @staticmethod def _sort_query(query, sort=None): http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/2d834753/tests/mock/models.py ---------------------------------------------------------------------- diff --git a/tests/mock/models.py b/tests/mock/models.py index 457e7cb..1d29e2d 100644 --- a/tests/mock/models.py +++ b/tests/mock/models.py @@ -50,10 +50,10 @@ DEPENDENT_NODE_TEMPLATE_NAME = 'dependent_node_template' DEPENDENT_NODE_NAME = 'dependent_node' -def create_service_template(): +def create_service_template(name=SERVICE_TEMPLATE_NAME): now = datetime.now() return models.ServiceTemplate( - name=SERVICE_TEMPLATE_NAME, + name=name, description=None, created_at=now, updated_at=now, @@ -68,10 +68,10 @@ def create_service_template(): ) -def create_service(service_template): +def create_service(service_template, name=SERVICE_NAME): now = datetime.utcnow() return models.Service( - name=SERVICE_NAME, + name=name, service_template=service_template, description='', created_at=now, @@ -81,7 +81,7 @@ def create_service(service_template): ) -def create_dependency_node_template(name, service_template): +def create_dependency_node_template(service_template, name=DEPENDENCY_NODE_TEMPLATE_NAME): node_type = service_template.node_types.get_descendant('test_node_type') capability_type = service_template.capability_types.get_descendant('test_capability_type') @@ -103,7 +103,8 @@ def create_dependency_node_template(name, service_template): return node_template -def create_dependent_node_template(name, service_template, dependency_node_template): +def create_dependent_node_template( + service_template, dependency_node_template, name=DEPENDENT_NODE_TEMPLATE_NAME): the_type = service_template.node_types.get_descendant('test_node_type') requirement_template = models.RequirementTemplate( http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/2d834753/tests/mock/topology.py ---------------------------------------------------------------------- diff --git a/tests/mock/topology.py b/tests/mock/topology.py index edf6b7d..e5b4e01 100644 --- a/tests/mock/topology.py +++ b/tests/mock/topology.py @@ -22,8 +22,7 @@ def create_simple_topology_single_node(model_storage, create_operation): service_template = models.create_service_template() service = models.create_service(service_template) - node_template = models.create_dependency_node_template( - models.DEPENDENCY_NODE_TEMPLATE_NAME, service_template) + node_template = models.create_dependency_node_template(service_template) interface_template = models.create_interface_template( service_template, 'Standard', 'create', @@ -55,10 +54,9 @@ def create_simple_topology_two_nodes(model_storage): # Creating a simple service with node -> node as a graph - dependency_node_template = models.create_dependency_node_template( - models.DEPENDENCY_NODE_TEMPLATE_NAME, service_template) - dependent_node_template = models.create_dependent_node_template( - models.DEPENDENT_NODE_TEMPLATE_NAME, service_template, dependency_node_template) + dependency_node_template = models.create_dependency_node_template(service_template) + dependent_node_template = models.create_dependent_node_template(service_template, + dependency_node_template) dependency_node = models.create_node( models.DEPENDENCY_NODE_NAME, dependency_node_template, service) @@ -87,9 +85,8 @@ def create_simple_topology_three_nodes(model_storage): service_id = create_simple_topology_two_nodes(model_storage) service = model_storage.service.get(service_id) third_node_template = models.create_dependency_node_template( - 'another_dependency_node_template', service.service_template) - third_node = models.create_node( - 'another_dependency_node', third_node_template, service) + service.service_template, name='another_dependency_node_template') + third_node = models.create_node('another_dependency_node', third_node_template, service) new_relationship = models.create_relationship( source=model_storage.node.get_by_name(models.DEPENDENT_NODE_NAME), target=third_node, http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/2d834753/tests/modeling/test_model_storage.py ---------------------------------------------------------------------- diff --git a/tests/modeling/test_model_storage.py b/tests/modeling/test_model_storage.py deleted file mode 100644 index bb778d4..0000000 --- a/tests/modeling/test_model_storage.py +++ /dev/null @@ -1,102 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with -# this work for additional information regarding copyright ownership. -# The ASF licenses this file to You under the Apache License, Version 2.0 -# (the "License"); you may not use this file except in compliance with -# the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import pytest - -from aria.storage import ( - ModelStorage, - exceptions, - sql_mapi -) -from aria import (application_model_storage, modeling) -from ..storage import (release_sqlite_storage, init_inmemory_model_storage) - -from . import MockModel - - -@pytest.fixture -def storage(): - base_storage = ModelStorage(sql_mapi.SQLAlchemyModelAPI, - initiator=init_inmemory_model_storage) - base_storage.register(MockModel) - yield base_storage - release_sqlite_storage(base_storage) - - -@pytest.fixture(scope='module', autouse=True) -def module_cleanup(): - modeling.models.aria_declarative_base.metadata.remove(MockModel.__table__) #pylint: disable=no-member - - -def test_storage_base(storage): - with pytest.raises(AttributeError): - storage.non_existent_attribute() - - -def test_model_storage(storage): - mock_model = MockModel(value=0, name='model_name') - storage.mock_model.put(mock_model) - - assert storage.mock_model.get_by_name('model_name') == mock_model - - assert [mm_from_storage for mm_from_storage in storage.mock_model.iter()] == [mock_model] - assert [mm_from_storage for mm_from_storage in storage.mock_model] == [mock_model] - - storage.mock_model.delete(mock_model) - with pytest.raises(exceptions.StorageError): - storage.mock_model.get(mock_model.id) - - -def test_application_storage_factory(): - storage = application_model_storage(sql_mapi.SQLAlchemyModelAPI, - initiator=init_inmemory_model_storage) - - assert storage.service_template - assert storage.node_template - assert storage.group_template - assert storage.policy_template - assert storage.substitution_template - assert storage.substitution_template_mapping - assert storage.requirement_template - assert storage.relationship_template - assert storage.capability_template - assert storage.interface_template - assert storage.operation_template - assert storage.artifact_template - - assert storage.service - assert storage.node - assert storage.group - assert storage.policy - assert storage.substitution - assert storage.substitution_mapping - assert storage.relationship - assert storage.capability - assert storage.interface - assert storage.operation - assert storage.artifact - - assert storage.execution - assert storage.service_update - assert storage.service_update_step - assert storage.service_modification - assert storage.plugin - assert storage.task - - assert storage.parameter - assert storage.type - assert storage.metadata - - release_sqlite_storage(storage) http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/2d834753/tests/modeling/test_models.py ---------------------------------------------------------------------- diff --git a/tests/modeling/test_models.py b/tests/modeling/test_models.py index 35ae09f..8cd00f8 100644 --- a/tests/modeling/test_models.py +++ b/tests/modeling/test_models.py @@ -89,10 +89,8 @@ def _service_update_storage(): def _node_template_storage(): storage = _service_storage() service_template = storage.service_template.list()[0] - dependency_node_template = mock.models.create_dependency_node_template( - mock.models.DEPENDENCY_NODE_TEMPLATE_NAME, service_template) - mock.models.create_dependent_node_template( - mock.models.DEPENDENCY_NODE_NAME, service_template, dependency_node_template) + dependency_node_template = mock.models.create_dependency_node_template(service_template) + mock.models.create_dependent_node_template(service_template, dependency_node_template) storage.service_template.update(service_template) return storage @@ -104,10 +102,8 @@ def _nodes_storage(): mock.models.DEPENDENCY_NODE_TEMPLATE_NAME) mock.models.create_node(mock.models.DEPENDENCY_NODE_NAME, dependency_node_template, service) - dependent_node_template = \ - mock.models.create_dependent_node_template(mock.models.DEPENDENT_NODE_TEMPLATE_NAME, - service.service_template, - dependency_node_template) + dependent_node_template = mock.models.create_dependent_node_template(service.service_template, + dependency_node_template) mock.models.create_node(mock.models.DEPENDENT_NODE_NAME, dependent_node_template, service) storage.service.update(service) http://git-wip-us.apache.org/repos/asf/incubator-ariatosca/blob/2d834753/tests/storage/test_model_storage.py ---------------------------------------------------------------------- diff --git a/tests/storage/test_model_storage.py b/tests/storage/test_model_storage.py new file mode 100644 index 0000000..fa57e6f --- /dev/null +++ b/tests/storage/test_model_storage.py @@ -0,0 +1,139 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import pytest + +from aria import ( + application_model_storage, + modeling +) +from aria.storage import ( + ModelStorage, + exceptions, + sql_mapi, +) + +from tests import ( + mock, + storage as tests_storage, + modeling as tests_modeling +) + + +@pytest.fixture +def storage(): + base_storage = ModelStorage(sql_mapi.SQLAlchemyModelAPI, + initiator=tests_storage.init_inmemory_model_storage) + base_storage.register(tests_modeling.MockModel) + yield base_storage + tests_storage.release_sqlite_storage(base_storage) + + +@pytest.fixture(scope='module', autouse=True) +def module_cleanup(): + modeling.models.aria_declarative_base.metadata.remove(tests_modeling.MockModel.__table__) #pylint: disable=no-member + + +def test_storage_base(storage): + with pytest.raises(AttributeError): + storage.non_existent_attribute() + + +def test_model_storage(storage): + mock_model = tests_modeling.MockModel(value=0, name='model_name') + storage.mock_model.put(mock_model) + + assert storage.mock_model.get_by_name('model_name') == mock_model + + assert [mm_from_storage for mm_from_storage in storage.mock_model.iter()] == [mock_model] + assert [mm_from_storage for mm_from_storage in storage.mock_model] == [mock_model] + + storage.mock_model.delete(mock_model) + with pytest.raises(exceptions.StorageError): + storage.mock_model.get(mock_model.id) + + +def test_application_storage_factory(): + storage = application_model_storage(sql_mapi.SQLAlchemyModelAPI, + initiator=tests_storage.init_inmemory_model_storage) + + assert storage.service_template + assert storage.node_template + assert storage.group_template + assert storage.policy_template + assert storage.substitution_template + assert storage.substitution_template_mapping + assert storage.requirement_template + assert storage.relationship_template + assert storage.capability_template + assert storage.interface_template + assert storage.operation_template + assert storage.artifact_template + + assert storage.service + assert storage.node + assert storage.group + assert storage.policy + assert storage.substitution + assert storage.substitution_mapping + assert storage.relationship + assert storage.capability + assert storage.interface + assert storage.operation + assert storage.artifact + + assert storage.execution + assert storage.service_update + assert storage.service_update_step + assert storage.service_modification + assert storage.plugin + assert storage.task + + assert storage.parameter + assert storage.type + assert storage.metadata + + tests_storage.release_sqlite_storage(storage) + + +@pytest.fixture +def context(tmpdir): + result = mock.context.simple(str(tmpdir)) + yield result + tests_storage.release_sqlite_storage(result.model) + + +def test_mapi_include(context): + service1 = context.model.service.list()[0] + service1.name = 'service1' + service1.service_template.name = 'service_template1' + context.model.service.update(service1) + + service_template2 = mock.models.create_service_template('service_template2') + service2 = mock.models.create_service(service_template2, 'service2') + context.model.service.put(service2) + + assert service1 != service2 + assert service1.service_template != service2.service_template + + def assert_include(service): + st_name = context.model.service.get(service.id, include=('service_template_name',)) + st_name_list = context.model.service.list(filters={'id': service.id}, + include=('service_template_name', )) + assert len(st_name) == len(st_name_list) == 1 + assert st_name[0] == st_name_list[0][0] == service.service_template.name + + assert_include(service1) + assert_include(service2)