Fleshed out the search client so it supports more filters required by…

… the API.

apps/amo/__init__.py

@@ -179,3 +179,19 @@ class MOBILE:
 PLATFORM_BSD = 4
 PLATFORM_WIN = 5
 PLATFORM_SUN = 6
+
+PLATFORMS = {
+    'all': PLATFORM_ALL,
+    'linux': PLATFORM_LINUX,
+    'mac': PLATFORM_MAC,
+    'macosx': PLATFORM_MAC,
+    'darwin': PLATFORM_MAC,
+    'bsd': PLATFORM_BSD,
+    'bsd_os': PLATFORM_BSD,
+    'win': PLATFORM_WIN,
+    'winnt': PLATFORM_WIN,
+    'windows': PLATFORM_WIN,
+    'sun': PLATFORM_SUN,
+    'sunos': PLATFORM_SUN,
+    'solaris': PLATFORM_SUN,
+}

apps/search/client.py

@@ -3,14 +3,47 @@
 from django.conf import settings
 from django.utils import translation
 
+import amo
 import amo.models
+from addons.models import Addon, Category
 from .sphinxapi import SphinxClient
 import sphinxapi as sphinx
 from .utils import convert_version, crc32
 
 m_dot_n_re = re.compile(r'^\d+\.\d+$')
 SEARCH_ENGINE_APP = 99
 
+
+def get_category_id(category, application):
+    """
+    Given a string, get the category id associated with it.
+    """
+    category = Category.objects.filter(
+            slug__istartswith=category,
+            application=application)[:1]
+
+    if len(category):
+        return category[0].id
+
+
+def extract_from_query(term, filter, regexp, options={}):
+    """
+    This pulls out a keyword filter from a search term and returns the value
+    for the filter and a new term with the filter removed.
+
+    E.g. "yslow version:3" will result in (yslow, 3).  Failing this, we'll look
+    in the search options dictionary to see if there is a value.
+    """
+    match = re.search(r'\b%s:\s*(%s)\b' % (filter, regexp), term)
+
+    if match:
+        term = term.replace(match.group(0), '')
+        value = match.group(1)
+    else:
+        value = options.get(filter, None)
+    return (term, value)
+
+
 class SearchError(Exception):
     pass
 
@@ -39,7 +72,6 @@ def restrict_version(self, version):
         high_int = convert_version(version)
         low_int = high_int
 
-
         if m_dot_n_re.match(version):
             low_int = convert_version(version + "apre")
 
@@ -51,7 +83,7 @@ def restrict_version(self, version):
             sc.SetFilterRange('max_ver', low_int, 10 * high_int)
             sc.SetFilterRange('min_ver', 0, high_int)
 
-    def query(self, term, **kwargs):
+    def query(self, term, limit=10, offset=0, **kwargs):
         """
         Queries sphinx for a term, and parses specific options.
 
@@ -77,11 +109,10 @@ def query(self, term, **kwargs):
         # Setup some default parameters for the search.
         fields = "addon_id, app, category"
 
-        limit = kwargs.get('limit', 2000)
-
         sc.SetSelect(fields)
         sc.SetFieldWeights({'name': 4})
-        sc.SetLimits(0, limit)
+        # limiting happens later, since Sphinx returns more than we need.
+        sc.SetLimits(0, 2000)
         sc.SetFilter('inactive', (0,))
 
         # STATUS_DISABLED and 0 (which likely means null) are filtered from
@@ -123,13 +154,36 @@ def query(self, term, **kwargs):
             sc.SetFilter('app', (kwargs['app'], SEARCH_ENGINE_APP))
 
         # Version filtering.
-        match = re.match('\bversion:([0-9\.]+)/', term)
+        (term, version) = extract_from_query(term, 'version', '[0-9.]+',
+                                             kwargs)
+
+        if version:
+            self.restrict_version(version)
+
+        # Category filtering.
+        (term, category) = extract_from_query(term, 'category', '\w+')
+
+        if category and 'app' in kwargs:
+            category = get_category_id(category, kwargs['app'])
+            if category:
+                sc.SetFilter('category', [int(category)])
 
-        if match:
-            term = term.replace(match.group(0), '')
-            self.restrict_version(match.group(1))
-        elif 'version' in kwargs:
-            self.restrict_version(kwargs['version'])
+        (term, platform) = extract_from_query(term, 'platform', '\w+', kwargs)
+
+        if platform:
+            platform = amo.PLATFORMS.get(platform)
+            if platform:
+                sc.SetFilter('platform', (int(platform), amo.PLATFORM_ALL,))
+
+        (term, addon_type) = extract_from_query(term, 'type', '\w+', kwargs)
+
+        if addon_type:
+            if not isinstance(addon_type, int):
+                types = dict((name.lower(), id) for id, name
+                             in amo.ADDON_TYPE.items())
+                addon_type = types[addon_type.lower()]
+            if addon_type:
+                sc.SetFilter('type', (addon_type,))
 
         # Xenophobia - restrict to just my language.
         if 'xenophobia' in kwargs and 'admin' not in kwargs:
@@ -142,11 +196,8 @@ def query(self, term, **kwargs):
         # XXX - Todo:
         # In the interest of having working code sooner than later, we're
         # skipping the following... for now:
-        #   * Type filter
-        #   * Platform filter
         #   * Date filter
         #   * GUID filter
-        #   * Category filter
         #   * Tag filter
         #   * Num apps filter
         #   * Logging
@@ -155,6 +206,22 @@ def query(self, term, **kwargs):
 
         if sc.GetLastError():
             raise SearchError(sc.GetLastError())
-
-        if result:
-            return result['matches']
+        if result and result['total']:
+            # Return results as a list of addons.
+            results = [m['attrs']['addon_id'] for m in result['matches']]
+
+            # Uniquify
+            ids = []
+            for the_id in results:
+                if the_id not in ids:
+                    ids.append(the_id)
+
+            ids = ids[offset:limit]
+            addons = Addon.objects.filter(id__in=ids).extra(
+                    select={'manual': 'FIELD(id,%s)'
+                            % ','.join(map(str, ids))},
+                    order_by=['manual'])
+
+            return addons
+        else:
+            return []

apps/search/management/commands/sphinxreindex.py

@@ -4,7 +4,7 @@
 from django.core.management.base import BaseCommand, CommandError
 
 
-class Command(BaseCommand):
+class Command(BaseCommand): #pragma: no cover
     help = ("Runs the indexer script for sphinx as defined in "
     " settings.SPHINX_INDEXER")
 

apps/search/tests.py

@@ -5,12 +5,16 @@
 import shutil
 import time
 
-from django.test import TransactionTestCase
+from django.test import TestCase, TransactionTestCase
+from django.core.management import call_command
+from django.utils import translation
 
 from nose.tools import eq_
 
+import amo
+import settings
 from .utils import start_sphinx, stop_sphinx, reindex, convert_version
-from .client import Client as SearchClient
+from .client import Client as SearchClient, SearchError, get_category_id, extract_from_query
 
 
 def test_convert_version():
@@ -37,27 +41,67 @@ def c(x, y):
     eq_(c(v[5], v[6]), 1)
 
 
-class SphinxTest(TransactionTestCase):
+def test_extract_from_query():
+    """Test that the correct terms are extracted from query strings."""
+
+    eq_(("yslow ", "3.4",),
+        extract_from_query("yslow voo:3.4", "voo", "[0-9.]+"))
+
+
+class SphinxTestCase(TransactionTestCase):
+    """
+    This test case type can setUp and tearDown the sphinx daemon.  Use this
+    when testing any feature that requires sphinx.
+    """
 
     fixtures = ["base/addons.json"]
     sphinx = True
+    sphinx_is_running = False
 
     def setUp(self):
-        os.environ['DJANGO_ENVIRONMENT'] = 'test'
+        if not SphinxTestCase.sphinx_is_running:
+            os.environ['DJANGO_ENVIRONMENT'] = 'test'
+
+            if os.path.exists('/tmp/data/sphinx'):
+                shutil.rmtree('/tmp/data/sphinx')
+            if os.path.exists('/tmp/log/searchd'):
+                shutil.rmtree('/tmp/log/searchd')
+
+            os.makedirs('/tmp/data/sphinx')
+            os.makedirs('/tmp/log/searchd')
+            reindex()
+            start_sphinx()
+            time.sleep(1)
+            SphinxTestCase.sphinx_is_running = True
+
+    @classmethod
+    def tearDownClass(cls):
+        call_command('flush', verbosity=0, interactive=False)
+        stop_sphinx()
+        SphinxTestCase.sphinx_is_running = False
 
-        if os.path.exists('/tmp/data/sphinx'):
-            shutil.rmtree('/tmp/data/sphinx')
-        if os.path.exists('/tmp/log/searchd'):
-            shutil.rmtree('/tmp/log/searchd')
 
-        os.makedirs('/tmp/data/sphinx')
-        os.makedirs('/tmp/log/searchd')
-        reindex()
-        start_sphinx()
-        time.sleep(1)
+class GetCategoryIdTest(TestCase):
+    fixtures = ["base/category"]
+
+    def test_get_category_id(self):
+        """Tests that we get the expected category ids"""
+        eq_(get_category_id('feeds', amo.FIREFOX.id), 1)
 
-    def tearDown(self):
-        stop_sphinx()
+
+query = lambda *args, **kwargs: SearchClient().query(*args, **kwargs)
+
+
+class SearchDownTest(TestCase):
+
+    def test_search_down(self):
+        """
+        Test that we raise a SearchError if search is not running.
+        """
+        self.assertRaises(SearchError, query, "")
+
+
+class SearchTest(SphinxTestCase):
 
     def test_sphinx_indexer(self):
         """
@@ -67,5 +111,69 @@ def test_sphinx_indexer(self):
         # we have to specify to sphinx to look at test_ dbs
         c = SearchClient()
         results = c.query('Delicious')
-        assert results[0]['attrs']['addon_id'] == 3615, \
+        assert results[0].id == 3615, \
             "Didn't get the addon ID I wanted."
+
+    def test_version_restriction(self):
+        """
+        This tests that sphinx will properly restrict by version.
+        """
+        eq_(query("Firebug version:3.6")[0].id, 1843)
+        eq_(len(query("Firebug version:4.0")), 0)
+
+    def test_sorts(self):
+        """
+        This tests the various sorting.
+        """
+        eq_(query("", limit=1, sort='newest')[0].id, 10869)
+        eq_(query("", limit=1, sort='updated')[0].id, 4664)
+        eq_(query("", limit=1, sort='name')[0].id, 3615)
+        eq_(query("", limit=1, sort='averagerating')[0].id, 7172)
+        eq_(query("", limit=1, sort='weeklydownloads')[0].id, 55)
+
+    def test_app_filter(self):
+        """
+        This tests filtering by application id.
+        """
+
+        eq_(query("", limit=1, app=amo.MOBILE.id)[0].id, 4664)
+        # Poor sunbird, nobody likes them.
+        eq_(len(query("", limit=1, app=amo.SUNBIRD.id)), 0)
+
+
+    def test_category_filter(self):
+        """
+        This tests filtering by category.
+        """
+
+        eq_(len(query("Firebug category:alerts", app=amo.FIREFOX.id)), 0)
+        eq_(len(query("category:alerts", app=amo.MOBILE.id)), 1)
+
+    def test_type_filter(self):
+        """
+        This tests filtering by addon type.
+        """
+
+        eq_(query("type:theme", limit=1)[0].id, 7172)
+
+    def test_platform_filter(self):
+        """
+        This tests filtering by platform.
+        """
+        eq_(len(query("grapple", platform='sunos')), 0)
+        eq_(len(query("grapple", platform='macos')), 1)
+
+    def test_xenophobia_filter(self):
+        """
+        Setting the language to 'fr' and turning xenophobia should give us no
+        results... since our fixture is fr incapable.
+        """
+        translation.activate('fr')
+        eq_(len(query("grapple", xenophobia=True)), 0)
+        translation.activate(settings.LANGUAGE_CODE)
+
+    def test_locale_filter(self):
+        """
+        Similar to test_xenophobia_filter.
+        """
+        eq_(len(query("grapple", locale='fr')), 0)

apps/search/utils.py

@@ -45,19 +45,6 @@ def stop_sphinx():
 pattern_plus = re.compile(r'((\d+)\+)')
 
 
-def convert_type(type):
-    if type == 'extension' or type == 'extensions':
-        return amo.ADDON_EXTENSIONS
-    elif type == 'theme' or type == 'themes':
-        return amo.ADDON_THEME
-    elif type == 'dict' or type == 'dicts':
-        return amo.ADDON_DICT
-    elif type == 'language' or type == 'languages':
-        return amo.ADDON_LPAPP
-    elif type == 'plugin' or type == 'plugins':
-        return amo.ADDON_PLUGIN
-
-
 def convert_version(version_string):
     """
     This will enumerate a version so that it can be used for comparisons and