diff --git a/lms/djangoapps/certificates/apis/v0/views.py b/lms/djangoapps/certificates/apis/v0/views.py
index 4479086ac3787fc7bacbcde068f32347efab5aaf..b8b0f47aa3d399c3c89a4322f746df0f6743d5af 100644
--- a/lms/djangoapps/certificates/apis/v0/views.py
+++ b/lms/djangoapps/certificates/apis/v0/views.py
@@ -260,7 +260,7 @@ class CertificatesListView(GenericAPIView):
passing_certificates[course_key] = course_certificate
viewable_certificates = []
- for course_key, course_overview in CourseOverview.get_from_ids_if_exists(
+ for course_key, course_overview in CourseOverview.get_from_ids(
list(passing_certificates.keys())
).items():
if certificates_viewable_for_course(course_overview):
diff --git a/lms/djangoapps/program_enrollments/rest_api/v1/views.py b/lms/djangoapps/program_enrollments/rest_api/v1/views.py
index 59d48f5a647e44cd08f5046c5915ea70b5deb969..277cac8d4cb540c64369a0e3bf8d80c70315fd2d 100644
--- a/lms/djangoapps/program_enrollments/rest_api/v1/views.py
+++ b/lms/djangoapps/program_enrollments/rest_api/v1/views.py
@@ -900,7 +900,7 @@ class ProgramCourseEnrollmentOverviewView(
is_active=True,
)
- overviews = CourseOverview.get_from_ids_if_exists(course_run_keys)
+ overviews = CourseOverview.get_from_ids(course_run_keys)
course_run_resume_urls = get_resume_urls_for_enrollments(user, course_enrollments)
diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py
index e32f8068e1605939fc291be1a3aa05d884202a37..fd417dbfb28b61186f06d21c091e58eb61abc111 100644
--- a/openedx/core/djangoapps/content/course_overviews/models.py
+++ b/openedx/core/djangoapps/content/course_overviews/models.py
@@ -335,48 +335,34 @@ class CourseOverview(TimeStampedModel):
return course_overview or cls.load_from_module_store(course_id)
@classmethod
- def get_from_ids_if_exists(cls, course_ids):
+ def get_from_ids(cls, course_ids):
"""
- Return a dict mapping course_ids to CourseOverviews, if they exist.
+ Return a dict mapping course_ids to CourseOverviews.
- This method will *not* generate new CourseOverviews or delete outdated
- ones. It exists only as a small optimization used when CourseOverviews
- are known to exist, for common situations like the student dashboard.
+ Tries to select all CourseOverviews in one query,
+ then fetches remaining (uncached) overviews from the modulestore.
- Callers should assume that this list is incomplete and fall back to
- get_from_id if they need to guarantee CourseOverview generation.
+ Course IDs for non-existant courses will map to None.
+
+ Arguments:
+ course_ids (iterable[CourseKey])
+
+ Returns: dict[CourseKey, CourseOverview|None]
"""
- return {
+ overviews = {
overview.id: overview
- for overview
- in cls.objects.select_related('image_set').filter(
+ for overview in cls.objects.select_related('image_set').filter(
id__in=course_ids,
version__gte=cls.VERSION
)
}
-
- @classmethod
- def get_from_id_if_exists(cls, course_id):
- """
- Return a CourseOverview for the provided course_id if it exists.
- Returns None if no CourseOverview exists with the provided course_id
-
- This method will *not* generate new CourseOverviews or delete outdated
- ones. It exists only as a small optimization used when CourseOverviews
- are known to exist, for common situations like the student dashboard.
-
- Callers should assume that this list is incomplete and fall back to
- get_from_id if they need to guarantee CourseOverview generation.
- """
- try:
- course_overview = cls.objects.select_related('image_set').get(
- id=course_id,
- version__gte=cls.VERSION
- )
- except cls.DoesNotExist:
- course_overview = None
-
- return course_overview
+ for course_id in course_ids:
+ if course_id not in overviews:
+ try:
+ overviews[course_id] = cls.load_from_module_store(course_id)
+ except CourseOverview.DoesNotExist:
+ overviews[course_id] = None
+ return overviews
def clean_id(self, padding_char='='):
"""
diff --git a/openedx/core/djangoapps/content/course_overviews/signals.py b/openedx/core/djangoapps/content/course_overviews/signals.py
index a683bf7ca3d79d2789da6777f658026c2606e742..1ce66e3b0d2b2dcbac2892ec0c1fda8f5845b153 100644
--- a/openedx/core/djangoapps/content/course_overviews/signals.py
+++ b/openedx/core/djangoapps/content/course_overviews/signals.py
@@ -25,7 +25,10 @@ def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable
Catches the signal that a course has been published in Studio and
updates the corresponding CourseOverview cache entry.
"""
- previous_course_overview = CourseOverview.get_from_ids_if_exists([course_key]).get(course_key)
+ try:
+ previous_course_overview = CourseOverview.objects.get(id=course_key)
+ except CourseOverview.DoesNotExist:
+ previous_course_overview = None
updated_course_overview = CourseOverview.load_from_module_store(course_key)
_check_for_course_changes(previous_course_overview, updated_course_overview)
diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py
index a7cd1fa65cd38e35ea68df234d4faf124db763f2..3b11a36be7f905d3332a5405648940a7ae93ff34 100644
--- a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py
+++ b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py
@@ -16,6 +16,7 @@ from django.conf import settings
from django.db.utils import IntegrityError
from django.test.utils import override_settings
from django.utils import timezone
+from opaque_keys.edx.keys import CourseKey
from PIL import Image
from six.moves import range # pylint: disable=ungrouped-imports
@@ -550,46 +551,48 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache
.format(filter_),
)
- def test_get_from_ids_if_exists(self):
+ def test_get_from_ids(self):
+ """
+ Assert that CourseOverviews.get_from_ids works as expected.
+
+ We expect that if we have four courses, of which:
+ * two have cached course overviews,
+ * one does *not* have a cache course overview, and
+ * one has an *out-of-date* course overview, that
+ all four course overviews will appear in teh resulting dictionary,
+ with the former two coming from the CourseOverviews SQL cache
+ and the latter two coming from the modulestore.
+ """
course_with_overview_1 = CourseFactory.create(emit_signals=True)
course_with_overview_2 = CourseFactory.create(emit_signals=True)
course_without_overview = CourseFactory.create(emit_signals=False)
-
- courses = [course_with_overview_1, course_with_overview_2, course_without_overview]
- course_ids_to_overviews = CourseOverview.get_from_ids_if_exists(
- course.id for course in courses
- )
-
- # We should see the ones that have published CourseOverviews
- # (when the signals were emitted), but not the one that didn't issue
- # a publish signal.
- self.assertEqual(len(course_ids_to_overviews), 2)
- self.assertIn(course_with_overview_1.id, course_ids_to_overviews)
- self.assertIn(course_with_overview_2.id, course_ids_to_overviews)
- self.assertNotIn(course_without_overview.id, course_ids_to_overviews)
-
- # But if we set a CourseOverview to be an old version, it shouldn't be
- # returned by get_from_ids_if_exists()
- overview_2 = course_ids_to_overviews[course_with_overview_2.id]
- overview_2.version = CourseOverview.VERSION - 1
- overview_2.save()
- course_ids_to_overviews = CourseOverview.get_from_ids_if_exists(
- course.id for course in courses
- )
- self.assertEqual(len(course_ids_to_overviews), 1)
- self.assertIn(course_with_overview_1.id, course_ids_to_overviews)
-
- def test_get_from_id_if_exists(self):
- course_with_overview = CourseFactory.create(emit_signals=True)
- course_id_to_overview = CourseOverview.get_from_id_if_exists(course_with_overview.id)
- self.assertEqual(course_with_overview.id, course_id_to_overview.id)
-
- overview_prev_version = CourseOverview.get_from_id_if_exists(course_with_overview.id)
- overview_prev_version.version = CourseOverview.VERSION - 1
- overview_prev_version.save()
-
- course_id_to_overview = CourseOverview.get_from_id_if_exists(course_with_overview.id)
- self.assertEqual(course_id_to_overview, None)
+ course_with_old_overview = CourseFactory.create(emit_signals=True)
+ old_overview = CourseOverview.objects.get(id=course_with_old_overview.id)
+ old_overview.version = CourseOverview.VERSION - 1
+ old_overview.save()
+
+ courses = [
+ course_with_overview_1,
+ course_with_overview_2,
+ course_without_overview,
+ course_with_old_overview,
+ ]
+ non_existent_course_key = CourseKey.from_string('course-v1:This+Course+IsFake')
+ course_ids = [course.id for course in courses] + [non_existent_course_key]
+
+ with mock.patch.object(
+ CourseOverview,
+ 'load_from_module_store',
+ wraps=CourseOverview.load_from_module_store
+ ) as mock_load_from_modulestore:
+ overviews_by_id = CourseOverview.get_from_ids(course_ids)
+ assert len(overviews_by_id) == 5
+ assert overviews_by_id[course_with_overview_1.id].id == course_with_overview_1.id
+ assert overviews_by_id[course_with_overview_2.id].id == course_with_overview_2.id
+ assert overviews_by_id[course_with_old_overview.id].id == course_with_old_overview.id
+ assert overviews_by_id[old_overview.id].id == old_overview.id
+ assert overviews_by_id[non_existent_course_key] is None
+ assert mock_load_from_modulestore.call_count == 3
@ddt.ddt
diff --git a/openedx/core/lib/api/tests/test_view_utils.py b/openedx/core/lib/api/tests/test_view_utils.py
new file mode 100644
index 0000000000000000000000000000000000000000..ac3c5779375b7a7f9056c4cbec608f0d4f282fcf
--- /dev/null
+++ b/openedx/core/lib/api/tests/test_view_utils.py
@@ -0,0 +1,66 @@
+"""
+Tests for (some of) the view utilities.
+"""
+from django.conf.urls import url
+from django.test.utils import override_settings
+from rest_framework.response import Response
+from rest_framework.test import APITestCase
+from rest_framework.views import APIView
+
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+from ..view_utils import DeveloperErrorViewMixin, verify_course_exists
+
+
+class MockAPIView(DeveloperErrorViewMixin, APIView):
+ """
+ Mock API view for testing.
+ """
+
+ @verify_course_exists
+ def get(self, request, course_id):
+ """
+ Mock GET handler for testing.
+ """
+ return Response("Success {}".format(course_id))
+
+urlpatterns = [
+ url(r'^mock/(?P<course_id>.*)/$', MockAPIView.as_view()), # Only works with new-style course keys
+]
+
+
+@override_settings(ROOT_URLCONF=__name__)
+class VerifyCourseExistsTestCase(SharedModuleStoreTestCase, APITestCase):
+ """
+ Tests for behavior of @verify_course_exists.
+ """
+ def test_bad_key_404(self):
+ response = self.client.get('/mock/this-is-a-bad-key/')
+ assert response.status_code == 404
+
+ def test_nonexistent_course_404(self):
+ CourseFactory()
+ response = self.client.get('/mock/course-v1:Non+Existent+Course/')
+ assert response.status_code == 404
+
+ def test_course_exists_200(self):
+ course = CourseFactory(
+ org="This", course="IsA", run="Course",
+ default_store=ModuleStoreEnum.Type.split,
+ )
+ response = self.client.get('/mock/{}/'.format(course.id))
+ assert response.status_code == 200
+
+ def test_course_with_outdated_overview_200(self):
+ course = CourseFactory(
+ org="This", course="IsAnother", run="Course",
+ default_store=ModuleStoreEnum.Type.split,
+ )
+ course_overview = CourseOverview.get_from_id(course.id)
+ course_overview.version = CourseOverview.VERSION - 1
+ course_overview.save()
+ response = self.client.get('/mock/{}/'.format(course.id))
+ assert response.status_code == 200
diff --git a/openedx/core/lib/api/view_utils.py b/openedx/core/lib/api/view_utils.py
index 47cc8e3a7a5f7337f41444827fe54c100991ea66..b25c072fc186edbb6d1b067792583fa9ac76cdc9 100644
--- a/openedx/core/lib/api/view_utils.py
+++ b/openedx/core/lib/api/view_utils.py
@@ -417,7 +417,7 @@ def verify_course_exists(view_func):
error_code='invalid_course_key'
)
- if not CourseOverview.get_from_id_if_exists(course_key):
+ if not CourseOverview.course_exists(course_key):
raise self.api_error(
status_code=status.HTTP_404_NOT_FOUND,
developer_message=u"Requested grade for unknown course {course}".format(course=text_type(course_key)),