Merge lp:~benji/charmworld/doctype-search into lp:charmworld

[Benji York]

Add "charms:" and "bundles:" prefix to search (both via the web interface and the API).

Add "charms:" and "bundles:" prefix to search (both via the web interface and the API).

[lp:~benji/charmworld/doctype-search]

488. By Benji York on 2014-02-21

fix indentation

[Richard Harding (rharding)]

Code looks great. Will QA.

[Richard Harding (rharding)]

Thanks for the change. LGTM and qa-ok.

review: Approve

Unmerged revisions

=== modified file 'charmworld/search.py'
--- charmworld/search.py	2013-11-19 17:26:23 +0000
+++ charmworld/search.py	2014-02-21 20:41:09 +0000
@@ -104,6 +104,20 @@
     unichr(c) if unichr(c) not in _BAD_CHARS else u' ' for c in range(128)]
 _BAD_CHAR_TABLE = u''.join(_BAD_CHAR_TABLE)
 _BAD_TRAILING_CHARS = r'\!:'
+KIND_RE = re.compile('^\s*(bundle|charm)s?(?::(.+))?')
+
+
+def extract_doctype_from_search_terms(terms, doctype=None):
+    """A search string with a specified doctype overrides the given doctype.
+
+    The doctype syntax is of the form "charm:apache2".
+    """
+    match = KIND_RE.match(terms)
+    if match:
+        doctype = match.group(1)
+        terms = match.group(2) or ''
+
+    return terms, doctype
 
 
 class ElasticSearchClient:
@@ -393,7 +407,9 @@
                          for field, boost in bundle_free_text_fields.items()]
         bundle_fields.extend(bundle_exact_fields)
 
-        if text != '':
+        if text == '':
+            dsl = {'match_all': {}}
+        else:
             def make_query(data_fields, other_fields=[]):
                 fields = ['data.' + field for field in data_fields]
                 fields.extend(other_fields)
@@ -414,8 +430,6 @@
             }}
             # Union the bundle_dsl and charm_dsl results
             dsl = {'bool': {'should': [bundle_dsl, charm_dsl]}}
-        else:
-            dsl = {'match_all': {}}
         return dsl
 
     @classmethod
@@ -555,6 +569,7 @@
             'charm', 'bundle', or None.  If None, do not filter.
         return: A list of {doctype=<charm|bundle>, data=entity}.
         """
+        text, doctype = extract_doctype_from_search_terms(text, doctype)
         if filters is None:
             filters = {}
         dsl = self._get_text_query(text, autocomplete)
@@ -591,13 +606,13 @@
             Bundle,
             Charm,
         )
-        size = None
-        if terms == '':
-            size = 0
+        doctype = None
+        if isinstance(terms, basestring):
+            terms, doctype = extract_doctype_from_search_terms(terms)
 
         dsl = self._get_text_query(terms)
         dsl = self._get_official_boost(dsl)
-        dsl = self._get_filtered(dsl, {}, None, valid_only, doctype=None)
+        dsl = self._get_filtered(dsl, {}, None, valid_only, doctype=doctype)
 
         with Timer() as timer:
             status = self.get_status()
@@ -614,10 +629,11 @@
                 log.info(message + '  Raising IndexNotReady.')
                 raise IndexNotReady(message)
             result_total = docs['num_docs']
-            if size is None:
-                size = result_total
-            hits = self._client.search(dsl, index=self.index_name, size=size)
+            hits = self._client.search(
+                dsl, index=self.index_name, size=result_total)
         hitlist = hits['hits']['hits']
+        charms = []
+        bundles = []
         charms = [{'data': Charm(hit['_source']['data']),
                    'weight': hit['_score']}
                   for hit in hitlist if hit['_type'] == CHARM]

=== modified file 'charmworld/tests/test_search.py'
--- charmworld/tests/test_search.py	2013-11-15 20:00:35 +0000
+++ charmworld/tests/test_search.py	2014-02-21 20:41:09 +0000
@@ -28,6 +28,7 @@
     BUNDLE,
     CHARM,
     ElasticSearchClient,
+    extract_doctype_from_search_terms,
     charm_exact_fields,
     charm_free_text_fields,
     IncompatibleMapping,
@@ -244,6 +245,67 @@
         self.assertEqual(charm, results['charm'][0]['data'])
         self.assertEqual(bundle, results['bundle'][0]['data'])
 
+    def test_searching_for_bundles_only(self):
+        Charm(self.makeCharm(description='a mozilla charm'))
+        bundle = Bundle(self.makeBundle(description='a mozilla bundle'))
+        client = self.index_client
+        for prefix in ('bundle', 'bundles'):
+            term = '{}:mozilla'.format(prefix)
+            search_results = client.search(term)
+            results = search_results['results']
+            # Results still has entries for charms and bundles.
+            self.assertEqual(2, len(results))
+            self.assertEqual(2, search_results['result_total'])
+            self.assertEqual(1, search_results['matches'])
+            self.assertEqual(0, len(results['charm']))
+            self.assertEqual(1, len(results['bundle']))
+            self.assertEqual(bundle, results['bundle'][0]['data'])
+
+    def test_searching_for_all_bundles(self):
+        Bundle(self.makeBundle(description='A'))
+        Bundle(self.makeBundle(description='B'))
+        Charm(self.makeCharm(description='C'))
+        client = self.index_client
+        # Both the singular and plural forms of the keyword work.
+        for search_term in ('bundle', 'bundles'):
+            search_results = client.search(search_term)
+            results = search_results['results']
+            # There were two matches, both of which were bundles.
+            self.assertEqual(2, search_results['matches'])
+            self.assertEqual(2, len(results['bundle']))
+            self.assertEqual(0, len(results['charm']))
+
+    def test_searching_for_charms_only(self):
+        charm = Charm(self.makeCharm(description='a mozilla charm'))
+        Bundle(self.makeBundle(description='a mozilla bundle'))
+        client = self.index_client
+        for prefix in ('charm', 'charms'):
+            term = '{}:mozilla'.format(prefix)
+            search_results = client.search(term)
+            search_results = client.search('charm: mozilla')
+            results = search_results['results']
+            # Results still has entries for charms and bundles.
+            self.assertEqual(2, len(results))
+            self.assertEqual(2, search_results['result_total'])
+            self.assertEqual(1, search_results['matches'])
+            self.assertEqual(1, len(results['charm']))
+            self.assertEqual(0, len(results['bundle']))
+            self.assertEqual(charm, results['charm'][0]['data'])
+
+    def test_searching_for_all_charms(self):
+        Charm(self.makeCharm(description='A'))
+        Charm(self.makeCharm(description='B'))
+        Bundle(self.makeBundle(description='C'))
+        client = self.index_client
+        # Both the singular and plural forms of the keyword work.
+        for search_term in ('charm', 'charms'):
+            search_results = client.search(search_term)
+            results = search_results['results']
+            # There were two matches, both of which were charms.
+            self.assertEqual(2, search_results['matches'])
+            self.assertEqual(2, len(results['charm']))
+            self.assertEqual(0, len(results['bundle']))
+
     def test_search_not_ready(self):
 
         class FakeElasticSearch:
@@ -555,6 +617,22 @@
         self.assertEqual(1, len(charms))
         self.assertEqual(abc_charm['_id'], charms[0]['_id'])
 
+    def test_charm_type_search(self):
+        # We can ask for all the charms and get them.
+        self.makeCharm()
+        self.makeCharm()
+        self.makeBundle()
+        results = self.get_charms_and_bundles(text='charms', doctype=None)
+        self.assertEqual(2, len(results))
+
+    def test_bundle_type_search(self):
+        # We can ask for all the charms and get them.
+        self.makeCharm()
+        self.makeBundle()
+        self.makeBundle()
+        results = self.get_charms_and_bundles(text='bundles', doctype=None)
+        self.assertEqual(2, len(results))
+
     def test_charms_text_search_respects_filters(self):
         official = self.makeCharm(summary='abc def', promulgated=True)
         self.makeCharm(promulgated=True)
@@ -1121,3 +1199,48 @@
         # exist.
         new_client = reindex(client, charms=[])
         new_client.delete_index()
+
+
+class TestDoctypeExtraction(TestCase):
+    """Specially-formatted search strings can filter by doctype.
+
+    If the search terms start with "bundles:" or "charms:" (with or without
+    the "s"), then the results are limited to items of that type.  The search
+    text "charms" or "bundles" (again, singular or plural) return all items of
+    that type.
+    """
+
+    def test_selecting_all_charms(self):
+        # If the query string only asks for charms, then...
+        terms, doctype = extract_doctype_from_search_terms('charms')
+        # ...the doctype reflects the request...
+        self.assertEqual(doctype, 'charm')
+        # ...and there are no "real" search terms.
+        self.assertEqual(terms, '')
+
+    def test_selecting_all_charms_singular(self):
+        # The query string still works without an "s".
+        self.assertEqual(
+            extract_doctype_from_search_terms('charm'),
+            ('', 'charm'))
+
+    def test_selecting_all_bundles(self):
+        # If the query string only asks for bundles, then...
+        terms, doctype = extract_doctype_from_search_terms('bundles')
+        # ...the doctype reflects the request...
+        self.assertEqual(doctype, 'bundle')
+        # ...and there are no "real" search terms.
+        self.assertEqual(terms, '')
+
+    def test_selecting_all_bundles_singular(self):
+        # The query string still works without an "s".
+        self.assertEqual(
+            extract_doctype_from_search_terms('bundle'),
+            ('', 'bundle'))
+
+    def test_including_additional_search_terms(self):
+        # If there are non-doctype search terms, they are unmolested.
+        terms, doctype = extract_doctype_from_search_terms('charms:foo bar baz')
+        self.assertEqual(terms, 'foo bar baz')
+        # The doctype was still extracted correctly though.
+        self.assertEqual(doctype, 'charm')