You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ariatosca.apache.org by em...@apache.org on 2017/04/05 15:58:22 UTC

[2/6] incubator-ariatosca git commit: ARIA-125-Filtering-returns-the-wrong-models

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-134-populate-workflows
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)