fix: don't recreate course overview images every request

This reduces database churn and helps avoid locks.
This commit is contained in:
Michael Terry
2021-07-30 15:27:27 -04:00
parent 61b9a523e7
commit 557d4f1d34
2 changed files with 19 additions and 20 deletions

View File

@@ -387,14 +387,8 @@ class CourseOverview(TimeStampedModel):
# Regenerate the thumbnail images if they're missing (either because
# they were never generated, or because they were flushed out after
# a change to CourseOverviewImageConfig.
if course_overview:
if hasattr(course_overview, 'image_set'):
image_set = course_overview.image_set
if not image_set.small_url or not image_set.large_url:
CourseOverviewImageSet.objects.filter(course_overview=course_overview).delete()
CourseOverviewImageSet.create(course_overview)
else:
CourseOverviewImageSet.create(course_overview)
if course_overview and not hasattr(course_overview, 'image_set'):
CourseOverviewImageSet.create(course_overview)
return course_overview or cls.load_from_module_store(course_id)

View File

@@ -601,6 +601,11 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
course_image_content = StaticContent(course_image_asset_key, image_name, 'image/png', image_buff)
contentstore().save(course_image_content)
def get_from_id(self, course_id):
"""Get course overview, but makes sure that we are actually calling the method by wiping cache"""
self.clear_caches() # wipe out the request cache so that get_from_id is actually run each time
return CourseOverview.get_from_id(course_id)
def set_config(self, enabled):
"""
Enable or disable thumbnail generation config.
@@ -682,7 +687,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
course = CourseFactory.create(
default_store=modulestore_type, course_image=course_image
)
course_overview_before = CourseOverview.get_from_id(course.id)
course_overview_before = self.get_from_id(course.id)
# This initial seeding should create an entry for the image_set.
assert hasattr(course_overview_before, 'image_set')
@@ -697,7 +702,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
self.set_config(False)
# Fetch a new CourseOverview
course_overview_after = CourseOverview.get_from_id(course.id)
course_overview_after = self.get_from_id(course.id)
# Assert that the data still exists for debugging purposes
assert hasattr(course_overview_after, 'image_set')
@@ -717,7 +722,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
"""
with self.store.default_store(modulestore_type):
course = CourseFactory.create(default_store=modulestore_type)
overview = CourseOverview.get_from_id(course.id)
overview = self.get_from_id(course.id)
# First the behavior when there's no CDN enabled...
AssetBaseUrlConfig.objects.all().delete()
@@ -743,7 +748,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
"""
with self.store.default_store(modulestore_type):
course = CourseFactory.create(default_store=modulestore_type)
overview = CourseOverview.get_from_id(course.id)
overview = self.get_from_id(course.id)
# Now enable the CDN...
AssetBaseUrlConfig.objects.create(enabled=True, base_url='fakecdn.edx.org')
@@ -771,7 +776,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
"""
with self.store.default_store(modulestore_type):
course = CourseFactory.create(default_store=modulestore_type)
overview = CourseOverview.get_from_id(course.id)
overview = self.get_from_id(course.id)
# Now enable the CDN...
AssetBaseUrlConfig.objects.create(enabled=True, base_url='fakecdn.edx.org')
@@ -818,7 +823,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
# The next time we create a CourseOverview, the images are explicitly
# *not* regenerated.
with mock.patch('openedx.core.lib.courses.create_course_image_thumbnail') as patched_create_thumbnail:
course_overview = CourseOverview.get_from_id(course_overview.id)
self.get_from_id(course_overview.id)
patched_create_thumbnail.assert_not_called()
@ddt.data(
@@ -862,7 +867,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
self.set_config(enabled=False)
# Now generate the CourseOverview...
course_overview = CourseOverview.get_from_id(course.id)
course_overview = self.get_from_id(course.id)
# If create_after_overview is True, no image_set exists yet. Verify
# that, then switch config back over to True and it should lazily
@@ -870,7 +875,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
if create_after_overview:
assert not hasattr(course_overview, 'image_set')
self.set_config(enabled=True)
course_overview = CourseOverview.get_from_id(course.id)
course_overview = self.get_from_id(course.id)
assert hasattr(course_overview, 'image_set')
image_urls = course_overview.image_urls
@@ -931,7 +936,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
# Now generate the CourseOverview...
config = CourseOverviewImageConfig.current()
course_overview = CourseOverview.get_from_id(course.id)
course_overview = self.get_from_id(course.id)
image_urls = course_overview.image_urls
for image_url, target in [(image_urls['small'], config.small), (image_urls['large'], config.large)]:
@@ -972,7 +977,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
course = CourseFactory.create()
# First create our CourseOverview
overview = CourseOverview.get_from_id(course.id)
overview = self.get_from_id(course.id)
assert not hasattr(overview, 'image_set')
# Now create an ImageSet by hand...
@@ -1012,7 +1017,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
self._create_course_image(course, course_image)
# Create course overview with image set.
overview = CourseOverview.get_from_id(course.id)
overview = self.get_from_id(course.id)
assert hasattr(overview, 'image_set')
# Make sure the thumbnail names come out as expected...
@@ -1053,7 +1058,7 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
if expected_url is None:
expected_url = course_image_url(course)
course_overview = CourseOverview.get_from_id(course.id)
course_overview = self.get_from_id(course.id)
# All the URLs that come back should be for the expected_url
assert course_overview.image_urls == {'raw': expected_url, 'small': expected_url, 'large': expected_url}