From 068b471e493f33f3f4018b6854c05e6f86d41ed6 Mon Sep 17 00:00:00 2001
From: Kushal Agrawal <kushal.agrawal@nokia.com>
Date: Wed, 9 May 2018 16:09:55 +0530
Subject: [PATCH] Tag search update

In case Artifact does not have tags associated and query
constain tags/tags-any condition with any other base artifact
field or custom field condition. It used to fail because of
INNER JOIN between glare_artifacts and glare_artifact_tags table.
Replaced "join" with "outerjoin" to rectify this issue.

Change-Id: Ie03520a38d2c726117195ea92c7fbd88c04bff18
---
 glare/db/sqlalchemy/api.py        |  3 +-
 glare/tests/unit/api/test_list.py | 62 +++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/glare/db/sqlalchemy/api.py b/glare/db/sqlalchemy/api.py
index 746fd8e..50ab9cb 100644
--- a/glare/db/sqlalchemy/api.py
+++ b/glare/db/sqlalchemy/api.py
@@ -268,10 +268,11 @@ def _apply_user_filters(query, basic_conds, tag_conds, prop_conds):
         tag_or_queries = []
         for tag_condition in tag_conds['or']:
             artifact_tag_alias = aliased(models.ArtifactTag)
-            query = query.join(artifact_tag_alias)
+            query = query.outerjoin(artifact_tag_alias)
             for tag_cond in tag_condition:
                 tag_cond.left = artifact_tag_alias.value
             tag_or_queries.append(and_(*tag_condition))
+        # If tag_or_queries is blank, there will not be any effect on query
         or_queries.append(and_(*tag_or_queries))
 
     if prop_conds:
diff --git a/glare/tests/unit/api/test_list.py b/glare/tests/unit/api/test_list.py
index 98f41ba..edcfe35 100644
--- a/glare/tests/unit/api/test_list.py
+++ b/glare/tests/unit/api/test_list.py
@@ -496,6 +496,7 @@ class TestArtifactList(base.BaseTestArtifactAPI):
             {'name': 'name4', 'tags': ['tag2']},
             {'name': 'name5', 'tags': ['tag4']},
             {'name': 'name6', 'tags': ['tag4', 'tag5']},
+            {'name': 'name7'},
         ]
         arts = [self.controller.create(self.req, 'sample_artifact', val)
                 for val in values]
@@ -551,6 +552,10 @@ class TestArtifactList(base.BaseTestArtifactAPI):
         for i in (3, 1, 0):
             self.assertIn(arts[i], res['artifacts'])
 
+        filters = [('name', 'or:like:name%'), ('tags-any', 'or:tag_nox_exist')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(7, len(res['artifacts']))
+
         # Filtering by tags with operators leads to BadRequest
         for f in ('tags', 'tags-any'):
             filters = [(f, 'eq:tag1')]
@@ -558,6 +563,63 @@ class TestArtifactList(base.BaseTestArtifactAPI):
                 exc.BadRequest, self.controller.list,
                 self.req, 'sample_artifact', filters)
 
+    def test_list_tags_base_and_additional_properties_with_sort(self):
+        values = [
+            {'name': 'name1', 'int1': 12, 'float1': 12.35,
+             'str1': 'string_value', 'tags': ['tag1', 'tag2']},
+            {'name': 'name2', 'int1': 15, 'float1': 14.38, 'str1': 'new_value',
+             'list_of_str': ['str1', 'str2'], 'tags': ['tag1', 'tag3']},
+            {'name': 'name3', 'int1': 15, 'float1': 14.38, 'str1': 'new_value',
+             'list_of_str': ['str11', 'str2'], 'tags': ['tag1'],
+             'dict_of_str': {'key1': 'value1', 'key2': 'value2'}},
+            {'name': 'name4', 'int1': 15, 'float1': 14.38, 'tags': ['tag2']},
+            {'name': 'name5', 'list_of_str': ['str1', 'str2'],
+             'tags': ['tag4']},
+            {'name': 'name6', 'tags': ['tag4', 'tag5']},
+            {'name': 'name7', 'list_of_str': ['str21', 'str12']},
+            {'name': 'name8', 'int1': 12,
+             'dict_of_str': {'key11': 'value1', 'key12': 'value2'}},
+            {'name': 'name9'}
+        ]
+        arts = [self.controller.create(self.req, 'sample_artifact', val)
+                for val in values]
+
+        filters = [('name', 'or:like:name%'), ('tags-any', 'or:tag_nox_exist')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(9, len(res['artifacts']))
+
+        filters = [('name', 'or:in:name1,name9'), ('tags-any', 'or:tag2,tag4')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(5, len(res['artifacts']))
+        for i in (0, 3, 4, 5, 8):
+            self.assertIn(arts[i], res['artifacts'])
+
+        filters = [('name', 'or:in:name1,name5,name9'), ('int1', 'or:lte:13')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(4, len(res['artifacts']))
+        for i in (0, 4, 7, 8):
+            self.assertIn(arts[i], res['artifacts'])
+
+        filters = [('list_of_str', 'or:in:str11'),
+                   ('tags-any', 'or:tag4,tag5')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(3, len(res['artifacts']))
+        for i in (2, 4, 5):
+            self.assertIn(arts[i], res['artifacts'])
+
+        filters = [('list_of_str', 'or:in:str11'), ('tags', 'or:tag4,tag5')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(2, len(res['artifacts']))
+        for i in (2, 5):
+            self.assertIn(arts[i], res['artifacts'])
+
+        filters = [('name', 'or:name7'), ('dict_of_str', 'or:in:key1'),
+                   ('tags', 'or:tag4,tag5')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(3, len(res['artifacts']))
+        for i in (2, 5, 6):
+            self.assertIn(arts[i], res['artifacts'])
+
     def test_list_and_sort_fields(self):
         amount = 7
         # Create a bunch of artifacts for list sorting tests