From 10aae0d4806b544faca6bf758f6c072835bcd369 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Thu, 30 Jul 2015 11:13:00 -0400 Subject: [PATCH] MA-1063 Add versioning and timestamping to CourseOverview --- .../0006_add_version_and_timestamp.py | 75 ++++++++++++++++ .../content/course_overviews/models.py | 85 +++++++++++++------ .../content/course_overviews/tests.py | 18 ++++ 3 files changed, 151 insertions(+), 27 deletions(-) create mode 100644 openedx/core/djangoapps/content/course_overviews/migrations/0006_add_version_and_timestamp.py diff --git 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 new file mode 100644 index 0000000000..0cf8b31716 --- /dev/null +++ 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'] \ No newline at end of file diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 517eda28a5..a6db91fbcc 100644 --- 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='='): """ diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py index 25035b2606..c55a3e0afc 100644 --- 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)