From 57b7150b090d4ef5434acd86aeb9bd2f4b304a79 Mon Sep 17 00:00:00 2001 From: Emma Green Date: Thu, 21 Feb 2019 15:17:09 -0500 Subject: [PATCH] add course to program caching --- common/djangoapps/student/views/dashboard.py | 2 +- lms/djangoapps/courseware/views/views.py | 2 +- openedx/core/djangoapps/catalog/cache.py | 3 ++ .../management/commands/cache_programs.py | 41 +++++++++++++++++-- .../djangoapps/catalog/tests/test_utils.py | 12 +++--- openedx/core/djangoapps/catalog/utils.py | 31 +++++++++----- openedx/core/djangoapps/programs/utils.py | 2 +- 7 files changed, 71 insertions(+), 22 deletions(-) diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 9a49dec2d5..be52bf8360 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -695,7 +695,7 @@ def student_dashboard(request): for program in inverted_programs.values(): try: program_uuid = program[0]['uuid'] - program_data = get_programs(request.site, uuid=program_uuid) + program_data = get_programs(uuid=program_uuid) program_data = ProgramDataExtender(program_data, request.user).extend() skus = program_data.get('skus') checkout_page_url = ecommerce_service.get_checkout_page_url(*skus) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 8b660bc061..e5c9d97343 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -900,7 +900,7 @@ def program_marketing(request, program_uuid): """ Display the program marketing page. """ - program_data = get_programs(request.site, uuid=program_uuid) + program_data = get_programs(uuid=program_uuid) if not program_data: raise Http404 diff --git a/openedx/core/djangoapps/catalog/cache.py b/openedx/core/djangoapps/catalog/cache.py index c0111b1363..fdb77122c6 100644 --- a/openedx/core/djangoapps/catalog/cache.py +++ b/openedx/core/djangoapps/catalog/cache.py @@ -9,3 +9,6 @@ PATHWAY_CACHE_KEY_TPL = 'pathway-{id}' # Cache key used to locate an item containing a list of all credit pathway ids for a site. SITE_PATHWAY_IDS_CACHE_KEY_TPL = 'pathway-ids-{domain}' + +# Template used to create cache keys for individual courses to program uuids. +COURSE_PROGRAMS_CACHE_KEY_TPL = 'course-programs-{course_run_id}' diff --git a/openedx/core/djangoapps/catalog/management/commands/cache_programs.py b/openedx/core/djangoapps/catalog/management/commands/cache_programs.py index 9e83c03a35..35b2416bd9 100644 --- a/openedx/core/djangoapps/catalog/management/commands/cache_programs.py +++ b/openedx/core/djangoapps/catalog/management/commands/cache_programs.py @@ -7,10 +7,11 @@ from django.core.cache import cache from django.core.management import BaseCommand from openedx.core.djangoapps.catalog.cache import ( + COURSE_PROGRAMS_CACHE_KEY_TPL, PATHWAY_CACHE_KEY_TPL, PROGRAM_CACHE_KEY_TPL, SITE_PATHWAY_IDS_CACHE_KEY_TPL, - SITE_PROGRAM_UUIDS_CACHE_KEY_TPL + SITE_PROGRAM_UUIDS_CACHE_KEY_TPL, ) from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.catalog.utils import create_catalog_api_client @@ -46,6 +47,7 @@ class Command(BaseCommand): programs = {} pathways = {} + courses = {} for site in Site.objects.all(): site_config = getattr(site, 'configuration', None) if site_config is None or not site_config.get_value('COURSE_CATALOG_API_URL'): @@ -60,12 +62,19 @@ class Command(BaseCommand): new_pathways, pathways_failed = self.get_pathways(client, site) new_pathways, new_programs, pathway_processing_failed = self.process_pathways(site, new_pathways, new_programs) + new_courses, courses_failed = self.get_courses(new_programs) - if program_uuids_failed or program_details_failed or pathways_failed or pathway_processing_failed: - failure = True + failure = any([ + program_uuids_failed, + program_details_failed, + pathways_failed, + pathway_processing_failed, + courses_failed, + ]) programs.update(new_programs) pathways.update(new_pathways) + courses.update(new_courses) logger.info(u'Caching UUIDs for {total} programs for site {site_name}.'.format( total=len(uuids), @@ -90,6 +99,11 @@ class Command(BaseCommand): successful_pathways=successful_pathways)) cache.set_many(pathways, None) + successful_courses = len(courses) + logger.info(u'Caching programs uuids for {successful_courses} courses.'.format( + successful_courses=successful_courses)) + cache.set_many(courses, None) + if failure: # This will fail a Jenkins job running this command, letting site # operators know that there was a problem. @@ -188,3 +202,24 @@ class Command(BaseCommand): logger.exception(u'Failed to process pathways for {domain}'.format(domain=site.domain)) failure = True return processed_pathways, programs, failure + + def get_courses(self, programs): + """ + Get all courses for the programs. + + TODO: when course discovery can handle it, use that instead. That will allow us to put all course runs + in the cache not just the course runs in a program. Therefore, a cache miss would be different from a + course not in a program. + """ + course_runs = {} + failure = False + + for program_uuid, program in programs.items(): + for course in program['courses']: + for course_run in course['course_runs']: + course_run_key = course_run['key'] + if course_run_key in course_runs: + course_runs[course_run_key] += program_uuid + else: + course_runs[course_run_key] = [program_uuid] + return course_runs, failure diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index 6122cef7be..bc5657d4a4 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -72,7 +72,7 @@ class TestGetPrograms(CacheIsolationTestCase): # When called before UUIDs are cached, the function should return an # empty list and log a warning. - self.assertEqual(get_programs(self.site), []) + self.assertEqual(get_programs(site=self.site), []) mock_warning.assert_called_once_with( u'Failed to get program UUIDs from the cache for site {}.'.format(self.site.domain) ) @@ -85,7 +85,7 @@ class TestGetPrograms(CacheIsolationTestCase): None ) - actual_programs = get_programs(self.site) + actual_programs = get_programs(site=self.site) # The 2 cached programs should be returned while info and warning # messages should be logged for the missing one. @@ -113,7 +113,7 @@ class TestGetPrograms(CacheIsolationTestCase): } cache.set_many(all_programs, None) - actual_programs = get_programs(self.site) + actual_programs = get_programs(site=self.site) # All 3 programs should be returned. self.assertEqual( @@ -147,7 +147,7 @@ class TestGetPrograms(CacheIsolationTestCase): mock_cache.get.return_value = [program['uuid'] for program in programs] mock_cache.get_many.side_effect = fake_get_many - actual_programs = get_programs(self.site) + actual_programs = get_programs(site=self.site) # All 3 cached programs should be returned. An info message should be # logged about the one that was initially missing, but the code should @@ -167,7 +167,7 @@ class TestGetPrograms(CacheIsolationTestCase): expected_program = ProgramFactory() expected_uuid = expected_program['uuid'] - self.assertEqual(get_programs(self.site, uuid=expected_uuid), None) + self.assertEqual(get_programs(uuid=expected_uuid), None) mock_warning.assert_called_once_with( u'Failed to get details for program {uuid} from the cache.'.format(uuid=expected_uuid) ) @@ -179,7 +179,7 @@ class TestGetPrograms(CacheIsolationTestCase): None ) - actual_program = get_programs(self.site, uuid=expected_uuid) + actual_program = get_programs(uuid=expected_uuid) self.assertEqual(actual_program, expected_program) self.assertFalse(mock_warning.called) diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index 0f5d0e63ab..6e07f41eb9 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -13,9 +13,13 @@ from pytz import UTC from entitlements.utils import is_course_run_entitlement_fulfillable from openedx.core.constants import COURSE_PUBLISHED -from openedx.core.djangoapps.catalog.cache import (PATHWAY_CACHE_KEY_TPL, PROGRAM_CACHE_KEY_TPL, - SITE_PATHWAY_IDS_CACHE_KEY_TPL, - SITE_PROGRAM_UUIDS_CACHE_KEY_TPL) +from openedx.core.djangoapps.catalog.cache import ( + COURSE_PROGRAMS_CACHE_KEY_TPL, + PATHWAY_CACHE_KEY_TPL, + PROGRAM_CACHE_KEY_TPL, + SITE_PATHWAY_IDS_CACHE_KEY_TPL, + SITE_PROGRAM_UUIDS_CACHE_KEY_TPL +) from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.lib.edx_api_utils import get_edx_api_data @@ -75,21 +79,23 @@ def check_catalog_integration_and_get_user(error_message_field): return None, catalog_integration -def get_programs(site, uuid=None): +def get_programs(site=None, uuid=None, course=None): # pylint: disable=redefined-outer-name """Read programs from the cache. The cache is populated by a management command, cache_programs. - Arguments: - site (Site): django.contrib.sites.models object - Keyword Arguments: + site (Site): django.contrib.sites.models object uuid (string): UUID identifying a specific program to read from the cache. + course (string): course id identifying a specific course run to read from the cache. Returns: list of dict, representing programs. dict, if a specific program is requested. """ + if len([arg for arg in (site, uuid, course) if arg is not None]) != 1: + raise TypeError('get_programs takes exactly one argument') + missing_details_msg_tpl = u'Failed to get details for program {uuid} from the cache.' if uuid: @@ -98,9 +104,14 @@ def get_programs(site, uuid=None): logger.warning(missing_details_msg_tpl.format(uuid=uuid)) return program - uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), []) - if not uuids: - logger.warning(u'Failed to get program UUIDs from the cache for site {}.'.format(site.domain)) + elif course: + uuids = cache.get(COURSE_PROGRAMS_CACHE_KEY_TPL.format(course_run_id=course)) + if not uuids: + logger.warning(missing_details_msg_tpl.format(course=course)) + else: + uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), []) + if not uuids: + logger.warning(u'Failed to get program UUIDs from the cache for site {}.'.format(site.domain)) programs = cache.get_many([PROGRAM_CACHE_KEY_TPL.format(uuid=uuid) for uuid in uuids]) programs = list(programs.values()) diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index 3aa7eb1e57..8ed354dd79 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -109,7 +109,7 @@ class ProgramProgressMeter(object): self.course_grade_factory = CourseGradeFactory() if uuid: - self.programs = [get_programs(self.site, uuid=uuid)] + self.programs = [get_programs(uuid=uuid)] else: self.programs = attach_program_detail_url(get_programs(self.site), self.mobile_only)