Use json serialization in mock index instead of pickle

search/result_processor.py

@@ -1,16 +1,22 @@
 """ overridable result processor object to allow additional properties to be exposed """
 import inspect
 from itertools import chain
+import json
+import logging
 import re
 import textwrap
 
 from django.conf import settings
+from django.core.serializers.json import DjangoJSONEncoder
 
 from .utils import _load_class
 
 DESIRED_EXCERPT_LENGTH = 100
 ELLIPSIS = "…"
 
+# log appears to be standard name used for logger
+log = logging.getLogger(__name__)  # pylint: disable=invalid-name
+
 
 class SearchResultProcessor(object):
 
@@ -105,7 +111,13 @@ def process_result(cls, dictionary, match_phrase, user):
         srp = result_processor(dictionary, match_phrase)
         if srp.should_remove(user):
             return None
-        srp.add_properties()
+        try:
+            srp.add_properties()
+        # protect around any problems introduced by subclasses within their properties
+        except Exception as ex:  # pylint: disable=broad-except
+            log.exception("error processing properties for %s - %s: will remove from results",
+                          json.dumps(dictionary, cls=DjangoJSONEncoder), ex.message)
+            return None
         return dictionary
 
     @property

search/tests/mock_search_engine.py

@@ -1,15 +1,35 @@
 """ Implementation of search interface to be used for tests where ElasticSearch is unavailable """
 import copy
-import pickle
+from datetime import datetime
+import json
 import os
 
 from django.conf import settings
-from django.utils import timezone
+from django.core.serializers.json import DjangoJSONEncoder
 
 from search.search_engine_base import SearchEngine
 from search.utils import ValueRange, DateRange
 
 
+def json_date_to_datetime(json_date_string_value):
+    ''' converts json date string to date object '''
+    if "T" in json_date_string_value:
+        if "." in json_date_string_value:
+            format_string = "%Y-%m-%dT%H:%M:%S.%f"
+        else:
+            format_string = "%Y-%m-%dT%H:%M:%S"
+        if json_date_string_value.endswith("Z"):
+            format_string += "Z"
+
+    else:
+        format_string = "%Y-%m-%d"
+
+    return datetime.strptime(
+        json_date_string_value,
+        format_string
+    )
+
+
 def _find_field(doc, field_name):
     """ find the dictionary field corresponding to the . limited name """
     if not isinstance(doc, dict):
@@ -43,9 +63,10 @@ def value_matches(doc, field_name, field_value):
         if compare_value is None:
             return include_blanks
 
-        if isinstance(field_value, DateRange):
-            if timezone.is_aware(compare_value):
-                compare_value = timezone.make_naive(compare_value, timezone.utc)
+        # if we have a string that we are trying to process as a date object
+        if (isinstance(compare_value, basestring) and
+                (isinstance(field_value, DateRange) or isinstance(field_value, datetime))):
+            compare_value = json_date_to_datetime(compare_value)
 
         if isinstance(field_value, ValueRange):
             return (
@@ -138,15 +159,15 @@ def _write_to_file(cls, create_if_missing=False):
         file_name = cls._backing_file(create_if_missing)
         if file_name:
             with open(file_name, "w+") as dict_file:
-                pickle.dump(cls._mock_elastic, dict_file)
+                json.dump(cls._mock_elastic, dict_file, cls=DjangoJSONEncoder)
 
     @classmethod
     def _load_from_file(cls):
         """ load the index dict from the contents of the backing file """
         file_name = cls._backing_file()
         if file_name and os.path.exists(file_name):
             with open(file_name, "r") as dict_file:
-                cls._mock_elastic = pickle.load(dict_file)
+                cls._mock_elastic = json.load(dict_file)
 
     @staticmethod
     def _paginate_results(size, from_, raw_results):

search/tests/tests.py

@@ -3,10 +3,10 @@
 # Some of the subclasses that get used as settings-overrides will yield this pylint
 # error, but they do get used when included as part of the override_settings
 # pylint: disable=abstract-class-not-used
+# pylint: disable=too-few-public-methods
 """ Tests for search functionalty """
 from datetime import datetime
 import json
-import pickle
 import os
 
 from django.conf import settings
@@ -21,7 +21,7 @@
 from search.utils import ValueRange, DateRange
 from search.api import perform_search, NoSearchEngine
 
-from .mock_search_engine import MockSearchEngine, _find_field, _filter_intersection
+from .mock_search_engine import MockSearchEngine, _find_field, _filter_intersection, json_date_to_datetime
 
 TEST_INDEX_NAME = "test_index"
 
@@ -609,9 +609,26 @@ def test_filter_optimization(self):
         test_docs = [{"A": {"X": 1, "Y": 2, "Z": 3}}, {"B": {"X": 9, "Y": 8, "Z": 7}}]
         self.assertTrue(_filter_intersection(test_docs, None), test_docs)
 
+    def test_datetime_conversion(self):
+        """ tests json_date_to_datetime with different formats """
+        json_date = "2015-01-31"
+        self.assertTrue(json_date_to_datetime(json_date), datetime(2015, 1, 31))
+
+        json_datetime = "2015-01-31T07:30:28"
+        self.assertTrue(json_date_to_datetime(json_datetime), datetime(2015, 1, 31, 7, 30, 28))
+
+        json_datetime = "2015-01-31T07:30:28.65785"
+        self.assertTrue(json_date_to_datetime(json_datetime), datetime(2015, 1, 31, 7, 30, 28, 65785))
+
+        json_datetime = "2015-01-31T07:30:28Z"
+        self.assertTrue(json_date_to_datetime(json_datetime), datetime(2015, 1, 31, 7, 30, 28))
+
+        json_datetime = "2015-01-31T07:30:28.65785Z"
+        self.assertTrue(json_date_to_datetime(json_datetime), datetime(2015, 1, 31, 7, 30, 28, 65785))
 
 # Uncomment below in order to test against installed Elastic Search installation
-# pylint: disable=too-few-public-methods
+
+
 @override_settings(SEARCH_ENGINE="search.tests.tests.ForceRefreshElasticSearchEngine")
 class ElasticSearchTests(MockSearchTests):
     """ Override that runs the same tests for ElasticSearchEngine instead of MockSearchEngine """
@@ -659,7 +676,9 @@ def test_file_reopen(self):
 
     def test_file_value_formats(self):
         """ test the format of values that write/read from the file """
-        this_moment = datetime.utcnow()
+        # json serialization removes microseconds part of the datetime object, so
+        # we strip it at the beginning to allow equality comparison to be correct
+        this_moment = datetime.utcnow().replace(microsecond=0)
         test_object = {
             "content": {
                 "name": "How did 11 of 12 balls get deflated during the game"
@@ -678,7 +697,7 @@ def test_file_value_formats(self):
 
         # and values should be what we desire
         returned_result = response["results"][0]["data"]
-        self.assertEqual(returned_result["my_date_value"], this_moment)
+        self.assertEqual(json_date_to_datetime(returned_result["my_date_value"]), this_moment)
         self.assertEqual(returned_result["my_integer_value"], 172)
         self.assertEqual(returned_result["my_float_value"], 57.654)
         self.assertEqual(
@@ -710,7 +729,7 @@ def test_disabled_index(self):
         # copy content, and then erase file so that backed file is not present and work is disabled
         initial_file_content = None
         with open("testfile.pkl", "r") as dict_file:
-            initial_file_content = pickle.load(dict_file)
+            initial_file_content = json.load(dict_file)
         os.remove("testfile.pkl")
 
         response = self.searcher.search(query_string="ABC")
@@ -1022,7 +1041,7 @@ def url(self):
         Property to display the url for the given location, useful for allowing navigation
         """
         if "course" not in self._results_fields or "id" not in self._results_fields:
-            return None
+            raise ValueError("expect this error when not providing a course and/or id")
 
         return u"/courses/{course_id}/jump_to/{location}".format(
             course_id=self._results_fields["course"],
@@ -1031,7 +1050,7 @@ def url(self):
 
     def should_remove(self, user):
         """ remove items when url is None """
-        return self.url is None
+        return "remove_me" in self._results_fields
 
 
 @override_settings(SEARCH_RESULT_PROCESSOR="search.tests.tests.OverrideSearchResultProcessor")
@@ -1053,8 +1072,18 @@ def test_additional_property(self):
     def test_removal(self):
         """ make sure that the override of should remove let's the application prevent access to a result """
         test_result = {
-            "not_course": "testmetestme",
-            "id": "herestheid"
+            "course": "remove_course",
+            "id": "remove_id",
+            "remove_me": True
+        }
+        new_result = SearchResultProcessor.process_result(test_result, "fake search pattern", None)
+        self.assertIsNone(new_result)
+
+    def test_property_error(self):
+        """ result should be removed from list if there is an error in the handler properties """
+        test_result = {
+            "not_course": "asdasda",
+            "not_id": "rthrthretht"
         }
         new_result = SearchResultProcessor.process_result(test_result, "fake search pattern", None)
         self.assertIsNone(new_result)

search/views.py

@@ -3,9 +3,9 @@
 # pylint: disable=too-few-public-methods
 import logging
 import json
-import datetime
 
 from django.conf import settings
+from django.core.serializers.json import DjangoJSONEncoder
 from django.http import HttpResponse
 from django.utils.translation import ugettext as _
 from django.views.decorators.http import require_POST
@@ -16,18 +16,6 @@
 log = logging.getLogger(__name__)  # pylint: disable=invalid-name
 
 
-class DateTimeEncoder(json.JSONEncoder):
-    """ encode datetimes into json appropriately """
-
-    def default(self, obj):  # pylint: disable=method-hidden
-        """ override default encoding """
-        if isinstance(obj, datetime.datetime):
-            encoded_object = obj.isoformat()
-        else:
-            encoded_object = super(DateTimeEncoder, self).default(self, obj)
-        return encoded_object
-
-
 class InvalidPageSize(ValueError):
     """ Exception for invalid page size value passed in """
     pass
@@ -106,7 +94,7 @@ def do_search(request, course_id=None):
         )
 
     return HttpResponse(
-        json.dumps(results, cls=DateTimeEncoder),
+        json.dumps(results, cls=DjangoJSONEncoder),
         content_type='application/json',
         status=status_code
     )