From a14667c6f23aca9dfc202c347a6d12080f4796ca Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Mon, 27 Jul 2015 14:57:25 -0400 Subject: [PATCH] MA-1061 Fix IntegrityError caused by race condition in CourseOverview.get_from_id --- .../content/course_overviews/models.py | 14 +++++++- .../content/course_overviews/tests.py | 34 ++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index d98b77f1cb..fd98aeb79c 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -5,6 +5,7 @@ Declaration of CourseOverview model import json from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField, FloatField, IntegerField +from django.db.utils import IntegrityError from django.utils.translation import ugettext from model_utils.models import TimeStampedModel @@ -172,7 +173,18 @@ class CourseOverview(TimeStampedModel): course = store.get_course(course_id) if isinstance(course, CourseDescriptor): course_overview = cls._create_from_course(course) - course_overview.save() + try: + course_overview.save() + except IntegrityError: + # There is a rare race condition that will occur if + # CourseOverview.get_from_id is called while a + # another identical overview is already in the process + # of being created. + # One of the overviews will be saved normally, while the + # other one will cause an IntegrityError because it tries + # to save a duplicate. + # (see: https://openedx.atlassian.net/browse/TNL-2854). + pass return course_overview elif course is not None: raise IOError( diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py index 9b0500fa24..9b9b30cbfb 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests.py +++ b/openedx/core/djangoapps/content/course_overviews/tests.py @@ -4,9 +4,9 @@ Tests for course_overviews app. import datetime import ddt import itertools -import pytz import math import mock +import pytz from django.utils import timezone @@ -349,3 +349,35 @@ class CourseOverviewTestCase(ModuleStoreTestCase): # 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) + + def test_course_overview_saving_race_condition(self): + """ + Tests that the following scenario will not cause an unhandled exception: + - Multiple concurrent requests are made for the same non-existent CourseOverview. + - A race condition in the django ORM's save method that checks for the presence + of the primary key performs an Insert instead of an Update operation. + - An IntegrityError is raised when attempting to create duplicate entries. + - This should be handled gracefully in CourseOverview.get_from_id. + + Created in response to https://openedx.atlassian.net/browse/MA-1061. + """ + course = CourseFactory.create() + + # mock the CourseOverview ORM to raise a DoesNotExist exception to force re-creation of the object + with mock.patch( + 'openedx.core.djangoapps.content.course_overviews.models.CourseOverview.objects.get' + ) as mock_getter: + + mock_getter.side_effect = CourseOverview.DoesNotExist + + # mock the CourseOverview ORM to not find the primary-key to force an Insert of the object + with mock.patch( + 'openedx.core.djangoapps.content.course_overviews.models.CourseOverview._get_pk_val' + ) as mock_get_pk_val: + + mock_get_pk_val.return_value = None + + # verify the CourseOverview is loaded successfully both times, + # including after an IntegrityError exception the 2nd time + for _ in range(2): + self.assertIsInstance(CourseOverview.get_from_id(course.id), CourseOverview)