Merge pull request #18 from Jakeway/bug_fix_result_sets

Bug fix result sets

autocompleter/__init__.py

@@ -1,4 +1,4 @@
-VERSION = (0, 7, 1)
+VERSION = (0, 7, 2)
 
 from autocompleter.registry import registry, signal_registry
 from autocompleter.base import AutocompleterBase, AutocompleterModelProvider, AutocompleterDictProvider, Autocompleter

autocompleter/base.py

@@ -538,10 +538,11 @@ def suggest(self, term, facets=[]):
 
         # Generate a unique identifier to be used for storing intermediate results. This is to
         # prevent redis key collisions between competing suggest / exact_suggest calls.
-        base_term_result_key = RESULT_SET_BASE_NAME % str(uuid.uuid4())
-        # If we don't end up using facets for this suggest call, we can just set the base_result_key
-        # to be equal to base_term_result_key since there was no extra manipulation to this set.
-        base_result_key = base_term_result_key
+        base_result_key = RESULT_SET_BASE_NAME % str(uuid.uuid4())
+        base_exact_match_key = RESULT_SET_BASE_NAME % str(uuid.uuid4())
+        # Same idea as the base_result_key, but for when we are using facets in the suggest call.
+        facet_base_result_key = RESULT_SET_BASE_NAME % str(uuid.uuid4())
+        facet_exact_match_key = RESULT_SET_BASE_NAME % str(uuid.uuid4())
 
         facet_keys_set = set()
         if len(facets) > 0:
@@ -550,12 +551,11 @@ def suggest(self, term, facets=[]):
             facet_keys_set = set([sub_facet['key'] for sub_facet in sub_facets])
 
         MOVE_EXACT_MATCHES_TO_TOP = registry.get_autocompleter_setting(self.name, 'MOVE_EXACT_MATCHES_TO_TOP')
-
-        pipe = REDIS.pipeline()
-
         # Get the max results autocompleter setting
         MAX_RESULTS = registry.get_autocompleter_setting(self.name, 'MAX_RESULTS')
 
+        pipe = REDIS.pipeline()
+
         for provider in providers:
             provider_name = provider.provider_name
 
@@ -568,11 +568,11 @@ def suggest(self, term, facets=[]):
             term_result_keys = []
             for norm_term in norm_terms:
                 norm_words = norm_term.split()
-                term_result_key = base_term_result_key + '.' + norm_term
+                term_result_key = base_result_key + '.' + norm_term
                 term_result_keys.append(term_result_key)
                 keys = [PREFIX_BASE_NAME % (provider_name, norm_word,) for norm_word in norm_words]
                 pipe.zinterstore(term_result_key, keys, aggregate='MIN')
-            pipe.zunionstore(base_term_result_key, term_result_keys, aggregate='MIN')
+            pipe.zunionstore(base_result_key, term_result_keys, aggregate='MIN')
             for term_result_key in term_result_keys:
                 pipe.delete(term_result_key)
 
@@ -604,17 +604,17 @@ def suggest(self, term, facets=[]):
                     except KeyError:
                         continue
                 # We want to calculate the intersection of all the intermediate facet sets created so far
-                # along with the base term result set. So we need to use a new unique name for the
-                # intermediate result set and append the base_term_result_key to the list of
-                # facet_result_keys.
-                base_result_key = RESULT_SET_BASE_NAME % str(uuid.uuid4())
-                facet_result_keys.append(base_term_result_key)
-                pipe.zinterstore(base_result_key, facet_result_keys, aggregate='MIN')
+                # along with the base result set. So we append the base_result_key to the list of
+                # facet_result_keys and store the intersection in the facet-dedicated set.
+                facet_result_keys.append(base_result_key)
+                pipe.zinterstore(facet_base_result_key, facet_result_keys, aggregate='MIN')
                 for facet_result_key in facet_result_keys:
                     pipe.delete(facet_result_key)
-                pipe.delete(base_term_result_key)
 
-            pipe.zrange(base_result_key, 0, MAX_RESULTS - 1)
+            if use_facets:
+                pipe.zrange(facet_base_result_key, 0, MAX_RESULTS - 1)
+            else:
+                pipe.zrange(base_result_key, 0, MAX_RESULTS - 1)
 
             # Get exact matches
             if MOVE_EXACT_MATCHES_TO_TOP:
@@ -630,15 +630,20 @@ def suggest(self, term, facets=[]):
                     # exact term matches don't bypass the requirement of having matching facet values.
                     # To achieve this, we append the previous intermediate result key (which at this point will
                     # contain all the facet matches) to the list of exact match keys and perform an intersection.
-                    keys.append(base_result_key)
-                    old_base_result_key = base_result_key
-                    base_result_key = RESULT_SET_BASE_NAME % str(uuid.uuid4())
-                    pipe.zinterstore(base_result_key, keys, aggregate='MIN')
-                    pipe.delete(old_base_result_key)
+                    keys.append(facet_base_result_key)
+                    pipe.zinterstore(facet_exact_match_key, keys, aggregate='MIN')
                 else:
-                    pipe.zunionstore(base_result_key, keys, aggregate='MIN')
-                pipe.zrange(base_result_key, 0, MAX_RESULTS - 1)
-            pipe.delete(base_result_key)
+                    pipe.zunionstore(base_exact_match_key, keys, aggregate='MIN')
+
+                if use_facets:
+                    pipe.zrange(facet_exact_match_key, 0, MAX_RESULTS - 1)
+                else:
+                    pipe.zrange(base_exact_match_key, 0, MAX_RESULTS - 1)
+
+        pipe.delete(base_result_key)
+        pipe.delete(base_exact_match_key)
+        pipe.delete(facet_base_result_key)
+        pipe.delete(facet_exact_match_key)
 
         results = [i for i in pipe.execute() if type(i) == list]
 

setup.py

@@ -2,7 +2,7 @@
 
 setup(
     name='django-autocompleter',
-    version="0.7.1",
+    version="0.7.2",
     description='A redis-backed autocompletor for Django projects',
     author='Ara Anjargolian',
     author_email='[email protected]',

test_project/test_app/autocompleters.py

@@ -224,3 +224,6 @@ def get_iterator(cls):
 registry.register("indicator_selective", IndicatorSelectiveAutocompleteProvider)
 registry.register("metric", CalcAutocompleteProvider)
 registry.register("metric_aliased", CalcAliasedAutocompleteProvider)
+
+registry.register("facet_stock_no_facet_ind", FacetedStockAutocompleteProvider)
+registry.register("facet_stock_no_facet_ind", IndicatorAutocompleteProvider)

test_project/test_app/tests/test_matching.py

@@ -635,3 +635,39 @@ def test_multiple_facet_dicts_match(self):
         ]
         matches = self.autocomp.suggest('ch', facets=facets)
         self.assertEqual(len(matches), 2)
+
+
+class MixedFacetProvidersMatchingTestCase(AutocompleterTestCase):
+    fixtures = ['stock_test_data_small.json', 'indicator_test_data_small.json']
+
+    def setUp(self):
+        super(MixedFacetProvidersMatchingTestCase, self).setUp()
+        self.autocomp = Autocompleter('facet_stock_no_facet_ind')
+        self.autocomp.store_all()
+
+    def test_autocompleter_with_facet_and_non_facet_providers(self):
+        """
+        Autocompleter with facet and non-facet providers works correctly
+        """
+        registry.set_autocompleter_setting('facet_stock_no_facet_ind', 'MAX_RESULTS', 100)
+        facets = [
+            {
+                'type': 'and',
+                'facets': [{'key': 'sector', 'value': 'Financial Services'}]
+            }
+        ]
+        matches = self.autocomp.suggest('a')
+        facet_matches = self.autocomp.suggest('a', facets=facets)
+
+        # because we are using the faceted stock provider in the 'facet_stock_no_facet_ind' AC,
+        # we expect using facets will decrease the amount of results when searching.
+        self.assertEqual(len(matches['faceted_stock']), 25)
+        self.assertEqual(len(facet_matches['faceted_stock']), 2)
+
+        # since the indicator provider does not support facets,
+        # we expect the search results from both a facet and non-facet search to be the same.
+        self.assertEqual(len(matches['ind']), 16)
+        self.assertEqual(len(matches['ind']), len(facet_matches['ind']))
+
+        registry.del_autocompleter_setting('facet_stock_no_facet_ind', 'MAX_RESULTS')
+