Make user and code valid_types not interfere when one is language and the other is table.

pokedex/lookup.py

@@ -294,42 +294,44 @@ class PokedexLookup(object):
         # Merge the valid types together.  Only types that appear in BOTH lists
         # may be used.
         # As a special case, if the user asked for types that are explicitly
-        # forbidden, completely ignore what the user requested
-        combined_valid_types = []
-        if user_valid_types and valid_types:
-            combined_valid_types = list(
-                set(user_valid_types) & set(combined_valid_types)
-            )
+        # forbidden, completely ignore what the user requested.
+        # And, just to complicate matters: "type" and language need to be
+        # considered separately.
+        def merge_requirements(func):
+            user = filter(func, user_valid_types)
+            system = filter(func, valid_types)
 
-            if not combined_valid_types:
-                # No overlap!  Just use the enforced ones
-                combined_valid_types = valid_types
-        else:
-            # One list or the other was blank, so just use the one that isn't
-            combined_valid_types = valid_types + user_valid_types
+            if user and system:
+                merged = list(set(user) & set(system))
+                if merged:
+                    return merged
+                else:
+                    # No overlap; use the system restrictions
+                    return system
+            else:
+                # One or the other is blank; use the one that's not
+                return user or system
 
-        if not combined_valid_types:
-            # No restrictions
-            return name, [], None
+        # @foo means language must be foo; otherwise it's a table name
+        lang_requirements = merge_requirements(lambda req: req[0] == u'@')
+        type_requirements = merge_requirements(lambda req: req[0] != u'@')
+        all_requirements = lang_requirements + type_requirements
 
         # Construct the term
-        type_terms = []
         lang_terms = []
-        final_valid_types = []
-        for valid_type in combined_valid_types:
-            if valid_type.startswith(u'@'):
-                # @foo means: language must be foo.
-                # Allow for either country or language codes
-                lang_code = valid_type[1:]
-                lang_terms.append(whoosh.query.Term(u'iso639', lang_code))
-                lang_terms.append(whoosh.query.Term(u'iso3166', lang_code))
-            else:
-                # otherwise, this is a type/table name
-                table_name = self._parse_table_name(valid_type)
+        for lang in lang_requirements:
+            # Allow for either country or language codes
+            lang_code = lang[1:]
+            lang_terms.append(whoosh.query.Term(u'iso639', lang_code))
+            lang_terms.append(whoosh.query.Term(u'iso3166', lang_code))
 
-                # Quietly ignore bogus valid_types; more likely to DTRT
-                if table_name:
-                    type_terms.append(whoosh.query.Term(u'table', table_name))
+        type_terms = []
+        for type in type_requirements:
+            table_name = self._parse_table_name(type)
+
+            # Quietly ignore bogus valid_types; more likely to DTRT
+            if table_name:
+                type_terms.append(whoosh.query.Term(u'table', table_name))
 
         # Combine both kinds of restriction
         all_terms = []
@@ -338,7 +340,7 @@ class PokedexLookup(object):
         if lang_terms:
             all_terms.append(whoosh.query.Or(lang_terms))
 
-        return name, combined_valid_types, whoosh.query.And(all_terms)
+        return name, all_requirements, whoosh.query.And(all_terms)
 
 
     def _parse_table_name(self, name):

pokedex/tests/test_lookup.py

@@ -66,10 +66,12 @@ def test_type_lookup():
     assert_equal(results[0].object.__tablename__, 'pokemon',
                                                 u'Type restriction works correctly')
     assert_equal(len(results), 1,               u'Only one id result when type is specified')
-    assert_equal(results[0].name, u'Bulbasaur', u'Type + id returns the right result')
+    assert_equal(results[0].object.name, u'Bulbasaur',
+                                                u'Type + id returns the right result')
 
     results = lookup.lookup(u'1', valid_types=['pokemon'])
-    assert_equal(results[0].name, u'Bulbasaur', u'valid_types works as well as type: prefix')
+    assert_equal(results[0].object.name, u'Bulbasaur',
+                                                u'valid_types works as well as type: prefix')
 
 def test_language_lookup():
     # There are two objects named "charge": the move Charge, and the move
@@ -91,6 +93,10 @@ def test_language_lookup():
     assert_equal(results[0].object.name, u'Tackle',
                                                 u'Languages and types both work together')
 
+    results = lookup.lookup(u'@fr:charge', valid_types=['move'])
+    assert_equal(results[0].object.name, u'Tackle',
+                                                u'valid_types and language prefixes get along')
+
 def test_fuzzy_lookup():
     tests = [
         # Regular English names