Don't assume that any pk castable to an integer should be an integer.

Author: carljm

AUTHORS

@@ -45,3 +45,4 @@ Thanks to
     * ghostrocket for a patch regarding the simple backend.
     * Rob Hudson (robhudson) for improvements to the admin search.
     * apollo13 for simplifying ``SearchForm.__init__``.
+    * Carl Meyer (carljm) for a patch regarding character primary keys.

haystack/query.py

@@ -160,15 +160,16 @@ def _fill_cache(self, start, end):
 
         for result in results:
             if self._load_all:
-                # We have to deal with integer keys being cast from strings; if this
-                # fails we've got a character pk.
-                try:
-                    result.pk = int(result.pk)
-                except ValueError:
-                    pass
+                # We have to deal with integer keys being cast from strings
+                model_objects = loaded_objects.get(result.model, {})
+                if not result.pk in model_objects:
+                    try:
+                        result.pk = int(result.pk)
+                    except ValueError:
+                        pass
                 try:
-                    result._object = loaded_objects[result.model][result.pk]
-                except (KeyError, IndexError):
+                    result._object = model_objects[result.pk]
+                except KeyError:
                     # The object was either deleted since we indexed or should
                     # be ignored; fail silently.
                     self._ignored_result_count += 1

tests/core/fixtures/initial_data.json

@@ -42,5 +42,17 @@
       "pub_date": "2009-03-17 08:00:00",
       "tag": 2
     }
+  },
+  {
+    "pk": "sometext",
+    "model": "core.charpkmockmodel",
+    "fields": {
+    }
+  },
+  {
+    "pk": "1234",
+    "model": "core.charpkmockmodel",
+    "fields": {
+    }
   }
 ]

tests/core/models.py

@@ -31,3 +31,7 @@ def __unicode__(self):
 class AThirdMockModel(AnotherMockModel):
     average_delay = models.FloatField(default=0.0)
     view_count = models.PositiveIntegerField(default=0)
+
+
+class CharPKMockModel(models.Model):
+    key = models.CharField(primary_key=True, max_length=10)

tests/core/tests/mocks.py

@@ -9,13 +9,15 @@
 class MockSearchResult(SearchResult):
     def __init__(self, app_label, model_name, pk, score, **kwargs):
         super(MockSearchResult, self).__init__(app_label, model_name, pk, score, **kwargs)
-        self._model = MockModel
-
+        self._model = get_model('core', model_name)
 
 MOCK_SEARCH_RESULTS = [MockSearchResult('core', 'MockModel', i, 1 - (i / 100.0)) for i in xrange(100)]
 
 
 class MockSearchBackend(BaseSearchBackend):
+    model_name = 'mockmodel'
+    mock_search_results = MOCK_SEARCH_RESULTS
+    
     def __init__(self, site=None):
         self.docs = {}
         self.site = site
@@ -38,13 +40,13 @@ def search(self, query_string, sort_by=None, start_offset=0, end_offset=None,
                limit_to_registered_models=None, **kwargs):
         from haystack import site
         results = []
-        hits = len(MOCK_SEARCH_RESULTS)
+        hits = len(self.mock_search_results)
         indexed_models = site.get_indexed_models()
 
-        sliced = MOCK_SEARCH_RESULTS[start_offset:end_offset]
+        sliced = self.mock_search_results[start_offset:end_offset]
 
         for result in sliced:
-            model = get_model('core', 'mockmodel')
+            model = get_model('core', self.model_name)
 
             if model:
                 if model in indexed_models:
@@ -61,11 +63,16 @@ def search(self, query_string, sort_by=None, start_offset=0, end_offset=None,
 
     def more_like_this(self, model_instance, additional_query_string=None):
         return {
-            'results': MOCK_SEARCH_RESULTS,
-            'hits': len(MOCK_SEARCH_RESULTS),
+            'results': self.mock_search_results,
+            'hits': len(self.mock_search_results),
         }
 
 
+class CharPKMockSearchBackend(MockSearchBackend):
+    model_name = 'charpkmockmodel'
+    mock_search_results = [MockSearchResult('core', 'CharPKMockModel', 'sometext', 0.5),
+                           MockSearchResult('core', 'CharPKMockModel', '1234', 0.3)]
+
 class MixedMockSearchBackend(MockSearchBackend):
     @log_query
     def search(self, query_string, **kwargs):

tests/core/tests/query.py

@@ -11,8 +11,8 @@
 from haystack.models import SearchResult
 from haystack.query import SearchQuerySet, EmptySearchQuerySet
 from haystack.sites import SearchSite
-from core.models import MockModel, AnotherMockModel
-from core.tests.mocks import MockSearchQuery, MockSearchBackend, MixedMockSearchBackend, MOCK_SEARCH_RESULTS
+from core.models import MockModel, AnotherMockModel, CharPKMockModel
+from core.tests.mocks import MockSearchQuery, MockSearchBackend, CharPKMockSearchBackend, MixedMockSearchBackend, MOCK_SEARCH_RESULTS
 try:
     set
 except NameError:
@@ -284,6 +284,7 @@ def setUp(self):
         self.old_site = haystack.site
         test_site = SearchSite()
         test_site.register(MockModel)
+        test_site.register(CharPKMockModel)
         haystack.site = test_site
 
         backends.reset_search_queries()
@@ -468,6 +469,13 @@ def test_raw_search(self):
         self.assertEqual(len(self.bsqs.raw_search('(content__exact hello AND content__exact world)')), 1)
 
     def test_load_all(self):
+        # Models with character primary keys
+        sqs = SearchQuerySet(query=MockSearchQuery(backend=CharPKMockSearchBackend()))
+        results = sqs.load_all().all()
+        self.assertEqual(len(results._result_cache), 0)
+        results._fill_cache(0, 2)
+        self.assertEqual(len([result for result in results._result_cache if result is not None]), 2)
+        
         # If nothing is registered, you get nothing.
         haystack.site.unregister(MockModel)
         sqs = self.msqs.load_all()