Invalidate API cache on course_metadata model changes

Invalidate API cache when any model in the course_metadata app is saved or deleted. Given how interconnected our data is and how infrequently our models change (data loading aside), this is a clean and simple way to ensure correctness of the API while providing closer-to-optimal cache TTLs.

LEARNER-401

course_discovery/apps/api/cache.py

+import logging
+import time
+
+from django.core.cache import cache
+from rest_framework_extensions.key_constructor.bits import KeyBitBase
+from rest_framework_extensions.key_constructor.constructors import (
+    DefaultListKeyConstructor, DefaultObjectKeyConstructor
+)
+
+logger = logging.getLogger(__name__)
+API_TIMESTAMP_KEY = 'api_timestamp'
+
+
+class ApiTimestampKeyBit(KeyBitBase):
+    def get_data(self, **kwargs):
+        return cache.get_or_set(API_TIMESTAMP_KEY, time.time, None)
+
+
+class TimestampedListKeyConstructor(DefaultListKeyConstructor):
+    timestamp = ApiTimestampKeyBit()
+
+
+class TimestampedObjectKeyConstructor(DefaultObjectKeyConstructor):
+    timestamp = ApiTimestampKeyBit()
+
+
+def timestamped_list_key_constructor(*args, **kwargs):  # pylint: disable=unused-argument
+    return TimestampedListKeyConstructor()(**kwargs)
+
+
+def timestamped_object_key_constructor(*args, **kwargs):  # pylint: disable=unused-argument
+    return TimestampedObjectKeyConstructor()(**kwargs)
+
+
+def set_api_timestamp(timestamp):
+    cache.set(API_TIMESTAMP_KEY, timestamp, None)
+
+
+def api_change_receiver(sender, **kwargs):  # pylint: disable=unused-argument
+    """
+    Receiver function for handling post_save and post_delete signals emitted by
+    course_metadata models.
+    """
+    timestamp = time.time()
+
+    logger.info(
+        '{model_name} model changed. Updating API timestamp to {timestamp}.'.format(
+            model_name=sender.__name__,
+            timestamp=timestamp
+        )
+    )
+
+    set_api_timestamp(timestamp)

course_discovery/apps/course_metadata/management/commands/refresh_course_metadata.py

 import concurrent.futures
 import itertools
 import logging
+import time
 
 import jwt
 import waffle
+from django.apps import apps
 from django.core.management import BaseCommand, CommandError
 from django.db import connection
+from django.db.models.signals import post_delete, post_save
 from edx_rest_api_client.client import EdxRestApiClient
 
+from course_discovery.apps.api.cache import api_change_receiver, set_api_timestamp
 from course_discovery.apps.core.models import Partner
 from course_discovery.apps.course_metadata.data_loaders.api import (
     CoursesApiDataLoader, EcommerceApiDataLoader, OrganizationsApiDataLoader, ProgramsApiDataLoader
@@ -60,6 +64,14 @@ class Command(BaseCommand):
         )
 
     def handle(self, *args, **options):
+        # We only want to invalidate the API response cache once data loading
+        # completes. Disconnecting the api_change_receiver function from post_save
+        # and post_delete signals prevents model changes during data loading from
+        # repeatedly invalidating the cache.
+        for model in apps.get_app_config('course_metadata').get_models():
+            for signal in (post_save, post_delete):
+                signal.disconnect(receiver=api_change_receiver, sender=model)
+
         # For each partner defined...
         partners = Partner.objects.all()
 
@@ -179,3 +191,10 @@ class Command(BaseCommand):
                         )
 
             # TODO Cleanup CourseRun overrides equivalent to the Course values.
+
+        timestamp = time.time()
+        logger.info(
+            'Data loading complete. Updating API timestamp to {timestamp}.'.format(timestamp=timestamp)
+        )
+
+        set_api_timestamp(timestamp)

course_discovery/apps/course_metadata/management/commands/tests/test_refresh_course_metadata.py

@@ -124,7 +124,9 @@ class RefreshCourseMetadataCommandTests(TransactionTestCase):
         )
         return bodies
 
-    def test_refresh_course_metadata_serial(self):
+    @mock.patch('course_discovery.apps.api.cache.set_api_timestamp')
+    @mock.patch('course_discovery.apps.course_metadata.management.commands.refresh_course_metadata.set_api_timestamp')
+    def test_refresh_course_metadata_serial(self, mock_set_api_timestamp, mock_receiver):
         with responses.RequestsMock() as rsps:
             self.mock_access_token_api(rsps)
             self.mock_apis()
@@ -139,7 +141,14 @@ class RefreshCourseMetadataCommandTests(TransactionTestCase):
                                   for loader_class, api_url, max_workers in self.pipeline]
                 mock_executor.assert_has_calls(expected_calls)
 
-    def test_refresh_course_metadata_parallel(self):
+        # Verify that the API cache is invalidated once, and that it isn't
+        # being done by the signal receiver.
+        assert mock_set_api_timestamp.call_count == 1
+        assert not mock_receiver.called
+
+    @mock.patch('course_discovery.apps.api.cache.set_api_timestamp')
+    @mock.patch('course_discovery.apps.course_metadata.management.commands.refresh_course_metadata.set_api_timestamp')
+    def test_refresh_course_metadata_parallel(self, mock_set_api_timestamp, mock_receiver):
         for name in ['threaded_metadata_write', 'parallel_refresh_pipeline']:
             toggle_switch(name)
 
@@ -161,6 +170,11 @@ class RefreshCourseMetadataCommandTests(TransactionTestCase):
                                   for loader_class, api_url, max_workers in self.pipeline]
                 mock_executor.assert_has_calls(expected_calls, any_order=True)
 
+        # Verify that the API cache is invalidated once, and that it isn't
+        # being done by the signal receiver.
+        assert mock_set_api_timestamp.call_count == 1
+        assert not mock_receiver.called
+
     def test_refresh_course_metadata_with_invalid_partner_code(self):
         """ Verify an error is raised if an invalid partner code is passed on the command line. """
         with self.assertRaises(CommandError):

course_discovery/apps/course_metadata/signals.py

 import waffle
-from django.db.models.signals import pre_delete
+from django.apps import apps
+from django.db.models.signals import post_delete, post_save, pre_delete
 from django.dispatch import receiver
 
+from course_discovery.apps.api.cache import api_change_receiver
 from course_discovery.apps.course_metadata.models import Program
 from course_discovery.apps.course_metadata.publishers import ProgramMarketingSitePublisher
 
@@ -16,3 +18,12 @@ def delete_program(sender, instance, **kwargs):  # pylint: disable=unused-argume
     if is_publishable:
         publisher = ProgramMarketingSitePublisher(instance.partner)
         publisher.delete_obj(instance)
+
+
+# Invalidate API cache when any model in the course_metadata app is saved or
+# deleted. Given how interconnected our data is and how infrequently our models
+# change (data loading aside), this is a clean and simple way to ensure correctness
+# of the API while providing closer-to-optimal cache TTLs.
+for model in apps.get_app_config('course_metadata').get_models():
+    for signal in (post_save, post_delete):
+        signal.connect(api_change_receiver, sender=model)

course_discovery/apps/course_metadata/tests/factories.py

@@ -64,17 +64,6 @@ class PrerequisiteFactory(AbstractNamedModelFactory):
         model = Prerequisite
 
 
-class SeatFactory(factory.DjangoModelFactory):
-    type = FuzzyChoice([name for name, __ in Seat.SEAT_TYPE_CHOICES])
-    price = FuzzyDecimal(0.0, 650.0)
-    currency = factory.Iterator(Currency.objects.all())
-    upgrade_deadline = FuzzyDateTime(datetime.datetime(2014, 1, 1, tzinfo=UTC))
-    sku = FuzzyText(length=8)
-
-    class Meta:
-        model = Seat
-
-
 class CourseFactory(factory.DjangoModelFactory):
     uuid = factory.LazyFunction(uuid4)
     key = FuzzyText(prefix='course-id/')
@@ -149,6 +138,18 @@ class CourseRunFactory(factory.DjangoModelFactory):
             add_m2m_data(self.authoring_organizations, extracted)
 
 
+class SeatFactory(factory.DjangoModelFactory):
+    type = FuzzyChoice([name for name, __ in Seat.SEAT_TYPE_CHOICES])
+    price = FuzzyDecimal(0.0, 650.0)
+    currency = factory.Iterator(Currency.objects.all())
+    upgrade_deadline = FuzzyDateTime(datetime.datetime(2014, 1, 1, tzinfo=UTC))
+    sku = FuzzyText(length=8)
+    course_run = factory.SubFactory(CourseRunFactory)
+
+    class Meta:
+        model = Seat
+
+
 class OrganizationFactory(factory.DjangoModelFactory):
     uuid = factory.LazyFunction(uuid4)
     key = FuzzyText(prefix='Org.fake/')
@@ -334,3 +335,15 @@ class SeatTypeFactory(factory.django.DjangoModelFactory):
         model = SeatType
 
     name = FuzzyText()
+
+
+class SyllabusItemFactory(factory.django.DjangoModelFactory):
+    class Meta:
+        model = SyllabusItem
+
+
+class PersonWorkFactory(factory.django.DjangoModelFactory):
+    class Meta:
+        model = PersonWork
+
+    person = factory.SubFactory(PersonFactory)

course_discovery/apps/course_metadata/tests/test_signals.py

+import mock
+import pytest
+from django.apps import apps
+from factory import DjangoModelFactory
+
+from course_discovery.apps.course_metadata.models import DataLoaderConfig
+from course_discovery.apps.course_metadata.tests import factories
+
+
+@pytest.mark.django_db
+@mock.patch('course_discovery.apps.api.cache.set_api_timestamp')
+class TestCacheInvalidation:
+    def test_model_change(self, mock_set_api_timestamp):
+        """
+        Verify that the API cache is invalidated after course_metadata models
+        are saved or deleted.
+        """
+        factory_map = {}
+        for factorylike in factories.__dict__.values():
+            if isinstance(factorylike, type) and issubclass(factorylike, DjangoModelFactory):
+                if getattr(factorylike, '_meta', None) and factorylike._meta.model:
+                    factory_map[factorylike._meta.model] = factorylike
+
+        # These are the models whose post_save and post_delete signals we're
+        # connecting to. We want to test each of them.
+        for model in apps.get_app_config('course_metadata').get_models():
+            # Ignore models that aren't exposed by the API or are only used for testing.
+            if model == DataLoaderConfig or 'abstract' in model.__name__.lower():
+                continue
+
+            factory = factory_map.get(model)
+            if not factory:
+                pytest.fail('The {} model is missing a factory.'.format(model))
+
+            # Verify that model creation and deletion invalidates the API cache.
+            instance = factory()
+
+            assert mock_set_api_timestamp.called
+            mock_set_api_timestamp.reset_mock()
+
+            instance.delete()
+
+            assert mock_set_api_timestamp.called
+            mock_set_api_timestamp.reset_mock()

course_discovery/settings/base.py

@@ -337,7 +337,9 @@ REST_FRAMEWORK = {
 # http://chibisov.github.io/drf-extensions/docs/
 REST_FRAMEWORK_EXTENSIONS = {
     'DEFAULT_CACHE_ERRORS': False,
-    'DEFAULT_CACHE_RESPONSE_TIMEOUT': 60,
+    'DEFAULT_CACHE_RESPONSE_TIMEOUT': None,
+    'DEFAULT_LIST_CACHE_KEY_FUNC': 'course_discovery.apps.api.cache.timestamped_list_key_constructor',
+    'DEFAULT_OBJECT_CACHE_KEY_FUNC': 'course_discovery.apps.api.cache.timestamped_object_key_constructor',
 }
 
 # NOTE (CCB): JWT_SECRET_KEY is intentionally not set here to avoid production releases with a public value.