From 06e46b778ff9865eac8a26cbed7b880c8ec828e7 Mon Sep 17 00:00:00 2001
From: Kushal Agrawal <kushal.agrawal@nokia.com>
Date: Tue, 17 Apr 2018 20:55:29 +0530
Subject: [PATCH] Condition based INNER JOIN for artifact_properties table

In case that "artifact data" does not contain properties other
than base artifact properties, query will return no records.
That because of INNER JOIN usage between glare_artifacts and
glare_artifact_properties tables (as there are no rows in
glare_artifact_properties corresponding to that artifact_id).

Therefore INNER JOIN needed to be added only if there is
any glare_artifact_properties based condition.

Change-Id: Icfcbeb41703771145db5405ffc2609dbb6ca1741
---
 glare/db/sqlalchemy/api.py                     |  6 ++++--
 glare/tests/functional/test_sample_artifact.py | 16 ++++++++++------
 glare/tests/unit/api/test_list.py              | 10 ++++++++++
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/glare/db/sqlalchemy/api.py b/glare/db/sqlalchemy/api.py
index e357ea7..6ed2b54 100644
--- a/glare/db/sqlalchemy/api.py
+++ b/glare/db/sqlalchemy/api.py
@@ -276,8 +276,10 @@ def _apply_user_filters(query, basic_conds, tag_conds, prop_conds):
             or_queries.append(and_(*prop_condition))
 
     if len(or_queries) != 0:
-        query = query.join(models.ArtifactProperty, aliased=True).filter(
-            or_(*or_queries))
+        if len(prop_conds['or']) > 0:
+            query = query.join(models.ArtifactProperty, aliased=True)
+
+        query = query.filter(or_(*or_queries))
 
     return query
 
diff --git a/glare/tests/functional/test_sample_artifact.py b/glare/tests/functional/test_sample_artifact.py
index d51372d..7526b48 100644
--- a/glare/tests/functional/test_sample_artifact.py
+++ b/glare/tests/functional/test_sample_artifact.py
@@ -2193,17 +2193,21 @@ class TestUpdate(base.TestArtifact):
                  'value': 'value1'}]
         url = '/sample_artifact/%s' % art1['id']
         result = self.patch(url=url, data=data)
-        self.assertEqual(['tag1', 'tag2'], result['tags'])
+        self.assertEqual(['tag1', 'tag2'], sorted(result['tags']))
         self.assertEqual({'meta1': 'value1'}, result['metadata'])
 
+        # Tags is set data structure so sequence of data is not fixed
+        first_tag = result['tags'][0]
+        second_tag = result['tags'][1]
+
         # move tag to metadata
         data = [{"op": "move",
                  "from": "/tags/0",
                  "path": "/metadata/meta2"}]
         url = '/sample_artifact/%s' % art1['id']
         result = self.patch(url=url, data=data)
-        self.assertEqual(['tag2'], result['tags'])
-        self.assertEqual({'meta1': 'value1', 'meta2': 'tag1'},
+        self.assertEqual([second_tag], result['tags'])
+        self.assertEqual({'meta1': 'value1', 'meta2': first_tag},
                          result['metadata'])
 
         # move data from one dict to another one
@@ -2214,7 +2218,7 @@ class TestUpdate(base.TestArtifact):
         result = self.patch(url=url, data=data)
         self.assertEqual({}, result['dict_of_str'])
         self.assertEqual({'meta1': 'value1',
-                          'meta2': 'tag1',
+                          'meta2': first_tag,
                           'wrong_type': '1'}, result['metadata'])
 
         # move data from one data to another one having same key
@@ -2229,7 +2233,7 @@ class TestUpdate(base.TestArtifact):
         result = self.patch(url=url, data=data)
         self.assertEqual({"new_key": "new_value"}, result['dict_of_str'])
         self.assertEqual({'meta1': 'value1',
-                          'meta2': 'tag1',
+                          'meta2': first_tag,
                           'wrong_type': '1',
                           "new_key": "new_value"}, result['metadata'])
 
@@ -2239,7 +2243,7 @@ class TestUpdate(base.TestArtifact):
         result = self.patch(url=url, data=data)
         self.assertEqual({}, result['dict_of_str'])
         self.assertEqual({'meta1': 'value1',
-                          'meta2': 'tag1',
+                          'meta2': first_tag,
                           'wrong_type': '1',
                           "new_key": "new_value"}, result['metadata'])
 
diff --git a/glare/tests/unit/api/test_list.py b/glare/tests/unit/api/test_list.py
index 3b31d05..5b72286 100644
--- a/glare/tests/unit/api/test_list.py
+++ b/glare/tests/unit/api/test_list.py
@@ -657,3 +657,13 @@ class TestArtifactList(base.BaseTestArtifactAPI):
         filters = [('name', 'or:tt:ttt'), ('str1', "or:blabla")]
         self.assertRaises(exc.BadRequest, self.controller.list,
                           self.req, 'sample_artifact', filters)
+
+        res = self.controller.create(self.req, 'heat_templates',
+                                     {'name': "artifact_without_properties"})
+
+        filters = [('name', 'or:eq:non_existant_name'),
+                   ('id', 'or:eq:' + res['id'])]
+        res = self.controller.list(self.req, 'heat_templates',
+                                   filters)['artifacts']
+        self.assertEqual(1, len(res))
+        self.assertEqual('artifact_without_properties', res[0]['name'])