Update logic to transform filters then call _search_table (#402)

* Update logic to transform filters then call _search_table

* Update tests

amundsen_application/api/search/v0.py

@@ -11,7 +11,8 @@ from flask.blueprints import Blueprint
 
 from amundsen_application.log.action_log import action_logging
 from amundsen_application.api.utils.request_utils import get_query_param, request_search
-from amundsen_application.api.utils.search_utils import generate_query_json, has_filters, map_table_result
+from amundsen_application.api.utils.search_utils import generate_query_json, has_filters, \
+    map_table_result, transform_filters
 from amundsen_application.models.user import load_user, dump_user
 
 LOGGER = logging.getLogger(__name__)
@@ -39,9 +40,9 @@ def search_table() -> Response:
 
         search_type = request_json.get('searchType')
 
-        filters = request_json.get('filters', {})
+        transformed_filters = transform_filters(filters=request_json.get('filters', {}))
 
-        results_dict = _search_table(filters=filters,
+        results_dict = _search_table(filters=transformed_filters,
                                      search_term=search_term,
                                      page_index=page_index,
                                      search_type=search_type)
@@ -76,14 +77,7 @@ def _search_table(*, search_term: str, page_index: int, filters: Dict, search_ty
 
     try:
         if has_filters(filters=filters):
-            try:
-                query_json = generate_query_json(filters=filters, page_index=page_index, search_term=search_term)
-            except Exception as e:
-                message = 'Encountered exception generating query json: ' + str(e)
-                results_dict['msg'] = message
-                logging.exception(message)
-                return results_dict
-
+            query_json = generate_query_json(filters=filters, page_index=page_index, search_term=search_term)
             url_base = app.config['SEARCHSERVICE_BASE'] + SEARCH_TABLE_FILTER_ENDPOINT
             response = request_search(url=url_base,
                                       headers={'Content-Type': 'application/json'},

amundsen_application/api/utils/search_utils.py

@@ -23,13 +23,12 @@ def map_table_result(result: Dict) -> Dict:
     }
 
 
-def generate_query_json(*, filters: Dict = {}, page_index: int, search_term: str) -> Dict:
+def transform_filters(*, filters: Dict = {}) -> Dict:
     """
-    Transforms the given paramaters to the query json for the search service according to
-    the api defined at:
+    Transforms the data shape of filters from the application to the data
+    shape required by the search service according to the api defined at:
     https://github.com/lyft/amundsensearchlibrary/blob/master/search_service/api/swagger_doc/table/search_table_filter.yml
     """
-    # Generate the filter payload
     filter_payload = {}
     for category in valid_search_fields:
         values = filters.get(category)
@@ -42,19 +41,32 @@ def generate_query_json(*, filters: Dict = {}, page_index: int, search_term: str
         if len(value_list) > 0:
             filter_payload[category] = value_list
 
-    # Return the full query json
+    return filter_payload
+
+
+def generate_query_json(*, filters: Dict = {}, page_index: int, search_term: str) -> Dict:
+    """
+    Transforms the given paramaters to the query json for the search service according to
+    the api defined at:
+    https://github.com/lyft/amundsensearchlibrary/blob/master/search_service/api/swagger_doc/table/search_table_filter.yml
+    """
     return {
         'page_index': int(page_index),
         'search_request': {
             'type': 'AND',
-            'filters': filter_payload
+            'filters': filters
         },
         'query_term': search_term
     }
 
 
 def has_filters(*, filters: Dict = {}) -> bool:
+    """
+    Returns whether or not the filter dictionary passed to the search service
+    has at least one filter value for a valid filter category
+    """
     for category in valid_search_fields:
-        if filters.get(category) is not None:
+        filter_list = filters.get(category, [])
+        if len(filter_list) > 0:
             return True
     return False

tests/unit/api/search/test_v0.py

@@ -72,17 +72,42 @@ class SearchTable(unittest.TestCase):
             self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR)
 
     @responses.activate
+    @patch('amundsen_application.api.search.v0.transform_filters')
+    def test_calls_transform_filters(self, transform_filter_mock) -> None:
+        """
+        Test transform_filters is called with the filters from the request json
+        from the request_json
+        :return:
+        """
+        test_filters = {'schema': 'test_schema'}
+        responses.add(responses.POST,
+                      self.search_service_filter_url,
+                      json=self.mock_table_results,
+                      status=HTTPStatus.OK)
+
+        with local_app.test_client() as test:
+            test.post(self.fe_flask_endpoint,
+                      json={
+                          'term': 'hello',
+                          'pageIndex': 1,
+                          'filters': test_filters,
+                          'searchType': 'test'})
+            transform_filter_mock.assert_called_with(filters=test_filters)
+
+    @responses.activate
+    @patch('amundsen_application.api.search.v0.transform_filters')
     @patch('amundsen_application.api.search.v0._search_table')
-    def test_calls_search_table_log_helper(self, search_table_mock) -> None:
+    def test_calls_search_table_log_helper(self, search_table_mock, transform_filter_mock) -> None:
         """
         Test _search_table helper method is called with correct arguments for logging
         from the request_json
         :return:
         """
-        test_filters = {'schema': 'test_schema'}
         test_term = 'hello'
         test_index = 1
         test_search_type = 'test'
+        mock_filters = {'schema': ['test_schema']}
+        transform_filter_mock.return_value = mock_filters
         responses.add(responses.POST,
                       self.search_service_filter_url,
                       json=self.mock_table_results,
@@ -93,23 +118,23 @@ class SearchTable(unittest.TestCase):
                       json={
                           'term': test_term,
                           'pageIndex': test_index,
-                          'filters': test_filters,
+                          'filters': {},
                           'searchType': test_search_type})
-            search_table_mock.assert_called_with(filters=test_filters,
+            search_table_mock.assert_called_with(filters=mock_filters,
                                                  page_index=test_index,
                                                  search_term=test_term,
                                                  search_type=test_search_type)
 
     @responses.activate
+    @patch('amundsen_application.api.search.v0.transform_filters')
     @patch('amundsen_application.api.search.v0.has_filters')
     @patch('amundsen_application.api.search.v0.generate_query_json')
-    def test_calls_generate_query_json(self, mock_generate_query_json, has_filters_mock) -> None:
+    def test_calls_generate_query_json(self, mock_generate_query_json, has_filters_mock, transform_filter_mock) -> None:
         """
         Test generate_query_json helper method is called with correct arguments
         from the request_json if filters exist
         :return:
         """
-        test_filters = {'schema': 'test_schema'}
         test_term = 'hello'
         test_index = 1
         responses.add(responses.POST,
@@ -117,11 +142,13 @@ class SearchTable(unittest.TestCase):
                       json=self.mock_table_results,
                       status=HTTPStatus.OK)
         has_filters_mock.return_value = True
+        mock_filters = {'schema': ['test_schema']}
+        transform_filter_mock.return_value = mock_filters
 
         with local_app.test_client() as test:
             test.post(self.fe_flask_endpoint,
-                      json={'term': test_term, 'pageIndex': test_index, 'filters': test_filters})
-            mock_generate_query_json.assert_called_with(filters=test_filters,
+                      json={'term': test_term, 'pageIndex': test_index, 'filters': {}})
+            mock_generate_query_json.assert_called_with(filters=mock_filters,
                                                         page_index=test_index,
                                                         search_term=test_term)
 
@@ -142,25 +169,6 @@ class SearchTable(unittest.TestCase):
             test.post(self.fe_flask_endpoint, json={'term': test_term, 'pageIndex': test_index, 'filters': {}})
             mock_generate_query_json.assert_not_called()
 
-    @patch('amundsen_application.api.search.v0.generate_query_json')
-    def test_catch_exception_generate_query_json(self, mock_generate_query_json) -> None:
-        """
-        Test that any execeptions thrown by generate_query_json are caught
-        from the request_json
-        :return:
-        """
-        test_filters = {'schema': 'test_schema'}
-        test_term = 'hello'
-        test_index = 1
-        mock_generate_query_json.side_effect = Exception('Test exception')
-
-        with local_app.test_client() as test:
-            response = test.post(self.fe_flask_endpoint,
-                                 json={'term': test_term, 'pageIndex': test_index, 'filters': test_filters})
-            data = json.loads(response.data)
-            self.assertEqual(data.get('msg'), 'Encountered exception generating query json: Test exception')
-            self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR)
-
     @responses.activate
     def test_request_success(self) -> None:
         """

tests/unit/utils/test_search_utils.py

 import unittest
 
-from amundsen_application.api.utils.search_utils import generate_query_json, has_filters
+from amundsen_application.api.utils.search_utils import generate_query_json, has_filters, transform_filters
 
 
 class SearchUtilsTest(unittest.TestCase):
@@ -27,12 +27,19 @@ class SearchUtilsTest(unittest.TestCase):
         self.test_page_index = "1"
         self.test_term = 'hello'
 
+    def test_transform_filters(self) -> None:
+        """
+        Verify that the given filters are correctly transformed
+        :return:
+        """
+        self.assertEqual(transform_filters(filters=self.test_filters), self.expected_transformed_filters)
+
     def test_generate_query_json(self) -> None:
         """
         Verify that the returned diction correctly transforms the parameters
         :return:
         """
-        query_json = generate_query_json(filters=self.test_filters,
+        query_json = generate_query_json(filters=self.expected_transformed_filters,
                                          page_index=self.test_page_index,
                                          search_term=self.test_term)
         self.assertEqual(query_json.get('page_index'), int(self.test_page_index))
@@ -47,13 +54,13 @@ class SearchUtilsTest(unittest.TestCase):
         Returns true if called with a dictionary that has values for a valid filter category
         :return:
         """
-        self.assertTrue(has_filters(filters=self.test_filters))
+        self.assertTrue(has_filters(filters=self.expected_transformed_filters))
 
     def test_has_filters_return_false(self) -> None:
         """
         Returns false if called with a dictionary that has no values for a valid filter category
         :return:
         """
-        test_filters = {'fake_category': {'db1': True}}
-        self.assertFalse(has_filters(filters=test_filters))
+        self.assertFalse(has_filters(filters={'fake_category': ['db1']}))
+        self.assertFalse(has_filters(filters={'tag': []}))
         self.assertFalse(has_filters())