Merge pull request #9126 from edx/kyle/course-overviews-versioning

WIP: MA-1063 Add versioning and timestamping to CourseOverview
2015-07-31 11:22:58 -04:00
parent 337024bd98 10aae0d480
commit 2e53b9e82c
3 changed files with 151 additions and 27 deletions
--- a/openedx/core/djangoapps/content/course_overviews/migrations/0006_add_version_and_timestamp.py
+++ b/openedx/core/djangoapps/content/course_overviews/migrations/0006_add_version_and_timestamp.py
@@ -0,0 +1,75 @@
+# -*- coding: utf-8 -*-
+from south.utils import datetime_utils as datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding field 'CourseOverview.created'
+        db.add_column('course_overviews_courseoverview', 'created',
+                      self.gf('model_utils.fields.AutoCreatedField')(default=datetime.datetime.now),
+                      keep_default=False)
+
+        # Adding field 'CourseOverview.modified'
+        db.add_column('course_overviews_courseoverview', 'modified',
+                      self.gf('model_utils.fields.AutoLastModifiedField')(default=datetime.datetime.now),
+                      keep_default=False)
+
+        # Adding field 'CourseOverview.version'
+        db.add_column('course_overviews_courseoverview', 'version',
+                      self.gf('django.db.models.fields.IntegerField')(default=0),
+                      keep_default=False)
+
+
+    def backwards(self, orm):
+        # Deleting field 'CourseOverview.created'
+        db.delete_column('course_overviews_courseoverview', 'created')
+
+        # Deleting field 'CourseOverview.modified'
+        db.delete_column('course_overviews_courseoverview', 'modified')
+
+        # Deleting field 'CourseOverview.version'
+        db.delete_column('course_overviews_courseoverview', 'version')
+
+
+    models = {
+        'course_overviews.courseoverview': {
+            'Meta': {'object_name': 'CourseOverview'},
+            '_location': ('xmodule_django.models.UsageKeyField', [], {'max_length': '255'}),
+            '_pre_requisite_courses_json': ('django.db.models.fields.TextField', [], {}),
+            'advertised_start': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'cert_html_view_enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'cert_name_long': ('django.db.models.fields.TextField', [], {}),
+            'cert_name_short': ('django.db.models.fields.TextField', [], {}),
+            'certificates_display_behavior': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'certificates_show_before_end': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'course_image_url': ('django.db.models.fields.TextField', [], {}),
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'days_early_for_beta': ('django.db.models.fields.FloatField', [], {'null': 'True'}),
+            'display_name': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'display_number_with_default': ('django.db.models.fields.TextField', [], {}),
+            'display_org_with_default': ('django.db.models.fields.TextField', [], {}),
+            'end': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'end_of_course_survey_url': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'enrollment_domain': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'enrollment_end': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'enrollment_start': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'facebook_url': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'has_any_active_web_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'primary_key': 'True', 'db_index': 'True'}),
+            'invitation_only': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'lowest_passing_grade': ('django.db.models.fields.DecimalField', [], {'null': 'True', 'max_digits': '5', 'decimal_places': '2'}),
+            'max_student_enrollments_allowed': ('django.db.models.fields.IntegerField', [], {'null': 'True'}),
+            'mobile_available': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'social_sharing_url': ('django.db.models.fields.TextField', [], {'null': 'True'}),
+            'start': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'version': ('django.db.models.fields.IntegerField', [], {}),
+            'visible_to_staff_only': ('django.db.models.fields.BooleanField', [], {'default': 'False'})
+        }
+    }
+
+    complete_apps = ['course_overviews']
--- a/openedx/core/djangoapps/content/course_overviews/models.py
+++ b/openedx/core/djangoapps/content/course_overviews/models.py
@@ -4,9 +4,9 @@ Declaration of CourseOverview model

 import json

-import django.db.models
 from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField, FloatField, IntegerField
 from django.utils.translation import ugettext
+from model_utils.models import TimeStampedModel

 from util.date_utils import strftime_localized
 from xmodule import course_metadata_utils
@@ -16,7 +16,7 @@ from xmodule.modulestore.django import modulestore
 from xmodule_django.models import CourseKeyField, UsageKeyField


-class CourseOverview(django.db.models.Model):
+class CourseOverview(TimeStampedModel):
    """
    Model for storing and caching basic information about a course.

@@ -25,6 +25,12 @@ class CourseOverview(django.db.models.Model):
    a course as part of a user dashboard or enrollment API.
    """

+    # IMPORTANT: Bump this whenever you modify this model and/or add a migration.
+    VERSION = 1
+
+    # Cache entry versioning.
+    version = IntegerField()
+
    # Course identification
    id = CourseKeyField(db_index=True, primary_key=True, max_length=255)  # pylint: disable=invalid-name
    _location = UsageKeyField(max_length=255)
@@ -67,8 +73,8 @@ class CourseOverview(django.db.models.Model):
    invitation_only = BooleanField(default=False)
    max_student_enrollments_allowed = IntegerField(null=True)

-    @staticmethod
-    def _create_from_course(course):
+    @classmethod
+    def _create_from_course(cls, course):
        """
        Creates a CourseOverview object from a CourseDescriptor.

@@ -94,7 +100,9 @@ class CourseOverview(django.db.models.Model):
        except ValueError:
            lowest_passing_grade = None

-        return CourseOverview(
+        return cls(
+            version=cls.VERSION,
+
            id=course.id,
            _location=course.location,
            display_name=course.display_name,
@@ -130,8 +138,42 @@ class CourseOverview(django.db.models.Model):
            max_student_enrollments_allowed=course.max_student_enrollments_allowed,
        )

-    @staticmethod
-    def get_from_id(course_id):
+    @classmethod
+    def _load_from_module_store(cls, course_id):
+        """
+        Load a CourseDescriptor, create a new CourseOverview from it, cache the
+        overview, and return it.
+
+        Arguments:
+            course_id (CourseKey): the ID of the course overview to be loaded.
+
+        Returns:
+            CourseOverview: overview of the requested course.
+
+        Raises:
+            - CourseOverview.DoesNotExist if the course specified by course_id
+                was not found.
+            - IOError if some other error occurs while trying to load the
+                course from the module store.
+        """
+        store = modulestore()
+        with store.bulk_operations(course_id):
+            course = store.get_course(course_id)
+            if isinstance(course, CourseDescriptor):
+                course_overview = cls._create_from_course(course)
+                course_overview.save()
+                return course_overview
+            elif course is not None:
+                raise IOError(
+                    "Error while loading course {} from the module store: {}",
+                    unicode(course_id),
+                    course.error_msg if isinstance(course, ErrorDescriptor) else unicode(course)
+                )
+            else:
+                raise cls.DoesNotExist()
+
+    @classmethod
+    def get_from_id(cls, course_id):
        """
        Load a CourseOverview object for a given course ID.

@@ -144,8 +186,7 @@ class CourseOverview(django.db.models.Model):
            course_id (CourseKey): the ID of the course overview to be loaded.

        Returns:
-            CourseOverview: overview of the requested course. If loading course
-            from the module store failed, returns None.
+            CourseOverview: overview of the requested course.

        Raises:
            - CourseOverview.DoesNotExist if the course specified by course_id
@@ -153,25 +194,15 @@ class CourseOverview(django.db.models.Model):
            - IOError if some other error occurs while trying to load the
                course from the module store.
        """
-        course_overview = None
        try:
-            course_overview = CourseOverview.objects.get(id=course_id)
-        except CourseOverview.DoesNotExist:
-            store = modulestore()
-            with store.bulk_operations(course_id):
-                course = store.get_course(course_id)
-                if isinstance(course, CourseDescriptor):
-                    course_overview = CourseOverview._create_from_course(course)
-                    course_overview.save()
-                elif course is not None:
-                    raise IOError(
-                        "Error while loading course {} from the module store: {}",
-                        unicode(course_id),
-                        course.error_msg if isinstance(course, ErrorDescriptor) else unicode(course)
-                    )
-                else:
-                    raise CourseOverview.DoesNotExist()
-        return course_overview
+            course_overview = cls.objects.get(id=course_id)
+            if course_overview.version != cls.VERSION:
+                # Throw away old versions of CourseOverview, as they might contain stale data.
+                course_overview.delete()
+                course_overview = None
+        except cls.DoesNotExist:
+            course_overview = None
+        return course_overview or cls._load_from_module_store(course_id)

    def clean_id(self, padding_char='='):
        """
--- a/openedx/core/djangoapps/content/course_overviews/tests.py
+++ b/openedx/core/djangoapps/content/course_overviews/tests.py
@@ -331,3 +331,21 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
            __ = course.lowest_passing_grade
        course_overview = CourseOverview._create_from_course(course)  # pylint: disable=protected-access
        self.assertEqual(course_overview.lowest_passing_grade, None)
+
+    @ddt.data((ModuleStoreEnum.Type.mongo, 1, 1), (ModuleStoreEnum.Type.split, 3, 4))
+    @ddt.unpack
+    def test_versioning(self, modulestore_type, min_mongo_calls, max_mongo_calls):
+        """
+        Test that CourseOverviews with old version numbers are thrown out.
+        """
+        with self.store.default_store(modulestore_type):
+            course = CourseFactory.create()
+            course_overview = CourseOverview.get_from_id(course.id)
+            course_overview.version = CourseOverview.VERSION - 1
+            course_overview.save()
+
+            # Because the course overview now has an old version number, it should
+            # be thrown out after being loaded from the cache, which results in
+            # a call to get_course.
+            with check_mongo_calls_range(max_finds=max_mongo_calls, min_finds=min_mongo_calls):
+                _course_overview_2 = CourseOverview.get_from_id(course.id)