Merge pull request #28 from mhrivnak/search-defaults

Defaults are no longer hard-coded for solr-based search results

README.rst

@@ -113,7 +113,8 @@ Example:
 .. warning:: crane does not currently verify the SSL certificate of the Solr service
 
 The JSON returned by the request must contain the following minimum data
-structure. Any additional keys and values will be ignored.
+structure. ``ir_automated``, ``ir_official``, and ``ir_stars`` are optional and
+will default to ``False``, ``False``, and ``0`` respectively.
 
 ::
 
@@ -122,7 +123,10 @@ structure. Any additional keys and values will be ignored.
       "docs": [
         {
           "allTitle": "pulp/worker",
-          "ir_description": "A short description to display in the terminal"
+          "ir_description": "A short description to display in the terminal",
+          "ir_automated": true,
+          "ir_official": true,
+          "ir_stars": 7
         }
       ]
     }

crane/search/base.py

@@ -44,14 +44,7 @@ def _format_result(result):
                     docker expects to receive.
         :rtype:     dict
         """
-        return {
-            'name': result.name,
-            'description': result.description,
-            'is_trusted': True,
-            'is_official': True,
-            # because we can
-            'star_count': 5,
-        }
+        return dict(result._asdict())
 
     def _filter_result(self, result):
         """
@@ -115,4 +108,6 @@ def _get_data(url):
 
 # this data structure should be used to return search results in a uniform
 # and well-defined way.
-SearchResult = namedtuple('SearchResult', ['name', 'description'])
+class SearchResult(namedtuple('SearchResult', ['name', 'description', 'is_trusted',
+                                               'is_official', 'star_count'])):
+    result_defaults = {'is_trusted': False, 'is_official': False, 'star_count': 0}

crane/search/gsa.py

@@ -105,7 +105,7 @@ def _parse_xml(data):
                         description = mt.attrib['V']
 
                 if name is not None:
-                    yield SearchResult(name, description)
+                    yield SearchResult(name, description, **SearchResult.result_defaults)
 
         except Exception:
             _logger.exception('could not parse xml')

crane/search/solr.py

@@ -58,7 +58,11 @@ def _parse(self, body):
         try:
             data = json.loads(body)
             for item in data['response']['docs']:
-                yield SearchResult(item['allTitle'], item['ir_description'])
+                trusted = item.get('ir_automated', SearchResult.result_defaults['is_trusted'])
+                automated = item.get('ir_official', SearchResult.result_defaults['is_official'])
+                stars = item.get('ir_stars', SearchResult.result_defaults['star_count'])
+                yield SearchResult(item['allTitle'], item['ir_description'],
+                                   trusted, automated, stars)
         except Exception, e:
             _logger.error('could not parse response body: %s' % e)
             _logger.exception('could not parse response')

tests/search/test_base.py

@@ -23,7 +23,22 @@ def test_search(self):
         self.assertEqual(assertion.exception.status_code, httplib.NOT_FOUND)
 
     def test_format_result(self):
-        result = base.SearchResult('rhel', 'Red Hat Enterprise Linux')
+        result = base.SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                   **base.SearchResult.result_defaults)
+
+        ret = self.backend._format_result(result)
+
+        self.assertDictEqual(ret, {
+            'name': 'rhel',
+            'description': 'Red Hat Enterprise Linux',
+            'is_trusted': False,
+            'is_official': False,
+            'star_count': 0,
+        })
+
+    def test_non_defaults(self):
+        result = base.SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                   True, True, 8)
 
         ret = self.backend._format_result(result)
 
@@ -32,12 +47,13 @@ def test_format_result(self):
             'description': 'Red Hat Enterprise Linux',
             'is_trusted': True,
             'is_official': True,
-            'star_count': 5,
+            'star_count': 8,
         })
 
     @mock.patch('crane.app_util.repo_is_authorized', spec_set=True)
     def test_filter_authorized_result(self, mock_is_authorized):
-        result = base.SearchResult('rhel', 'Red Hat Enterprise Linux')
+        result = base.SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                   **base.SearchResult.result_defaults)
 
         ret = self.backend._filter_result(result)
 
@@ -46,7 +62,8 @@ def test_filter_authorized_result(self, mock_is_authorized):
 
     @mock.patch('crane.app_util.repo_is_authorized', spec_set=True)
     def test_filter_nonauthorized_result(self, mock_is_authorized):
-        result = base.SearchResult('rhel', 'Red Hat Enterprise Linux')
+        result = base.SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                   **base.SearchResult.result_defaults)
         mock_is_authorized.side_effect = exceptions.HTTPError(httplib.NOT_FOUND)
 
         ret = self.backend._filter_result(result)

tests/search/test_gsa.py

@@ -41,24 +41,26 @@ class TestSearch(BaseGSATest):
     @mock.patch('crane.search.gsa.GSA._get_data', spec_set=True)
     @mock.patch('crane.search.gsa.GSA._parse_xml')
     def test_workflow_filter_true(self, mock_parse_xml, mock_get_data, mock_filter):
-        mock_parse_xml.return_value = [SearchResult('rhel', 'Red Hat Enterprise Linux')]
+        mock_parse_xml.return_value = [SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                                    **SearchResult.result_defaults)]
 
         ret = self.gsa.search('foo')
 
         mock_get_data.assert_called_once_with('http://pulpproject.org/search?q=foo')
         self.assertDictEqual(list(ret)[0], {
             'name': 'rhel',
             'description': 'Red Hat Enterprise Linux',
-            'star_count': 5,
-            'is_trusted': True,
-            'is_official': True,
+            'star_count': 0,
+            'is_trusted': False,
+            'is_official': False,
         })
 
     @mock.patch('crane.search.gsa.GSA._filter_result', spec_set=True, return_value=False)
     @mock.patch('crane.search.gsa.GSA._get_data', spec_set=True)
     @mock.patch('crane.search.gsa.GSA._parse_xml')
     def test_workflow_filter_false(self, mock_parse_xml, mock_get_data, mock_filter):
-        mock_parse_xml.return_value = [SearchResult('rhel', 'Red Hat Enterprise Linux')]
+        mock_parse_xml.return_value = [SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                                    **SearchResult.result_defaults)]
 
         ret = self.gsa.search('foo')
 
@@ -97,8 +99,10 @@ def test_success(self):
         items = list(generator)
 
         self.assertListEqual(items, [
-            SearchResult('rhel7.0', 'Red Hat Enterprise Linux 7 base image'),
-            SearchResult('redhat/rhel7.0', 'Red Hat Enterprise Linux 7 base image'),
+            SearchResult('rhel7.0', 'Red Hat Enterprise Linux 7 base image',
+                         **SearchResult.result_defaults),
+            SearchResult('redhat/rhel7.0', 'Red Hat Enterprise Linux 7 base image',
+                         **SearchResult.result_defaults),
         ])
 
     def test_no_description(self):
@@ -107,7 +111,7 @@ def test_no_description(self):
         items = list(generator)
 
         self.assertListEqual(items, [
-            SearchResult('rhel7.0', ''),
+            SearchResult('rhel7.0', '', **SearchResult.result_defaults),
         ])
 
     def test_handle_exception(self):

tests/search/test_solr.py

@@ -49,7 +49,8 @@ def test_workflow_filter_true(self, mock_parse, mock_get_data, mock_filter):
     @mock.patch('crane.search.solr.Solr._get_data', spec_set=True)
     @mock.patch('crane.search.solr.Solr._parse')
     def test_workflow_filter_true(self, mock_parse, mock_get_data, mock_filter):
-        mock_parse.return_value = [SearchResult('rhel', 'Red Hat Enterprise Linux')]
+        mock_parse.return_value = [SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                                **SearchResult.result_defaults)]
 
         ret = self.solr.search('foo')
 
@@ -66,6 +67,21 @@ def test_normal(self):
         self.assertTrue(isinstance(result[0], SearchResult))
         self.assertEqual(result[0].name, 'foo/bar')
         self.assertEqual(result[0].description, 'marketing speak yada yada')
+        self.assertTrue(result[0].is_official is True)
+        self.assertTrue(result[0].is_trusted is True)
+        self.assertEqual(result[0].star_count, 7)
+
+    def test_with_defaults(self):
+        result = list(self.solr._parse(json.dumps(fake_body_with_defaults)))
+
+        self.assertEqual(len(result), 1)
+
+        self.assertTrue(isinstance(result[0], SearchResult))
+        self.assertEqual(result[0].name, 'foo/bar')
+        self.assertEqual(result[0].description, 'marketing speak yada yada')
+        self.assertTrue(result[0].is_official is SearchResult.result_defaults['is_official'])
+        self.assertTrue(result[0].is_trusted is SearchResult.result_defaults['is_trusted'])
+        self.assertEqual(result[0].star_count, SearchResult.result_defaults['star_count'])
 
     def test_json_exception(self):
         """
@@ -87,6 +103,21 @@ def test_attribute_exception(self):
 
 
 fake_body = {
+    'response': {
+        'docs': [
+            {
+                'allTitle': 'foo/bar',
+                'ir_description': 'marketing speak yada yada',
+                'ir_automated': True,
+                'ir_official': True,
+                'ir_stars': 7,
+            }
+        ]
+    }
+}
+
+
+fake_body_with_defaults = {
     'response': {
         'docs': [
             {

tests/views/test_search.py

@@ -21,7 +21,8 @@ def test_empty_query(self):
     @mock.patch('crane.search.backend.search', spec_set=True)
     def test_with_results(self, mock_search):
         mock_search.return_value = [
-            SearchBackend._format_result(SearchResult('rhel', 'Red Hat Enterprise Linux')),
+            SearchBackend._format_result(SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                                      **SearchResult.result_defaults)),
         ]
 
         response = self.test_client.get('/v1/search?q=rhel')
@@ -37,9 +38,12 @@ def test_with_results(self, mock_search):
     @mock.patch('crane.search.backend.search', spec_set=True)
     def test_num_results(self, mock_search):
         mock_search.return_value = [
-            SearchBackend._format_result(SearchResult('rhel', 'Red Hat Enterprise Linux')),
-            SearchBackend._format_result(SearchResult('foo', 'Foo')),
-            SearchBackend._format_result(SearchResult('bar', 'Bar')),
+            SearchBackend._format_result(SearchResult('rhel', 'Red Hat Enterprise Linux',
+                                                      **SearchResult.result_defaults)),
+            SearchBackend._format_result(SearchResult('foo', 'Foo',
+                                                      **SearchResult.result_defaults)),
+            SearchBackend._format_result(SearchResult('bar', 'Bar',
+                                                      **SearchResult.result_defaults)),
         ]
 
         response = self.test_client.get('/v1/search?q=rhel')