diff --git a/common/djangoapps/terrain/stubs/catalog.py b/common/djangoapps/terrain/stubs/catalog.py index 6716fbd1d06a781d1ffe4a7e931d5e0d7e1810c2..58c51b6e38d23badd0ea14510a690e89786bf9e1 100644 --- a/common/djangoapps/terrain/stubs/catalog.py +++ b/common/djangoapps/terrain/stubs/catalog.py @@ -15,6 +15,7 @@ class StubCatalogServiceHandler(StubHttpRequestHandler): r'/api/v1/programs/$': self.program_list, r'/api/v1/programs/([0-9a-f-]+)/$': self.program_detail, r'/api/v1/program_types/$': self.program_types, + r'/api/v1/credit_pathways/$': self.credit_pathways } if self.match_pattern(pattern_handlers): @@ -47,6 +48,10 @@ class StubCatalogServiceHandler(StubHttpRequestHandler): program_types = self.server.config.get('catalog.programs_types', []) self.send_json_response(program_types) + def credit_pathways(self): + credit_pathways = self.server.config.get('catalog.credit_pathways', []) + self.send_json_response(credit_pathways) + class StubCatalogService(StubHttpService): HANDLER_CLASS = StubCatalogServiceHandler diff --git a/common/test/acceptance/fixtures/catalog.py b/common/test/acceptance/fixtures/catalog.py index 20262cb9b72de8804d29e853b1ba7564ce016c57..8ee94c00b58a3c06acceebc9cd1f5810e771f671 100644 --- a/common/test/acceptance/fixtures/catalog.py +++ b/common/test/acceptance/fixtures/catalog.py @@ -40,6 +40,18 @@ class CatalogFixture(object): data={key: json.dumps(uuids)}, ) + def install_credit_pathways(self, credit_pathways): + """ + Stub the discovery service's credit pathways API endpoint + + Arguments: + credit_pathways (list): A list of credit pathways. List endpoint will be stubbed using data from this list. + """ + requests.put( + '{}/set_config'.format(CATALOG_STUB_URL), + data={'catalog.credit_pathways': json.dumps({'results': credit_pathways})} + ) + def install_program_types(self, program_types): """ Stub the discovery service's program type list API endpoints. diff --git a/common/test/acceptance/tests/lms/test_programs.py b/common/test/acceptance/tests/lms/test_programs.py index d1cf2737669873729499d24b32d414d33473ad95..2ba18994d0ec397e3b99fbc6210951de9d9826e9 100644 --- a/common/test/acceptance/tests/lms/test_programs.py +++ b/common/test/acceptance/tests/lms/test_programs.py @@ -11,6 +11,7 @@ from common.test.acceptance.tests.helpers import UniqueCourseTest from openedx.core.djangoapps.catalog.tests.factories import ( CourseFactory, CourseRunFactory, + CreditPathwayFactory, ProgramFactory, ProgramTypeFactory ) @@ -24,6 +25,14 @@ class ProgramPageBase(ProgramsConfigMixin, CatalogIntegrationMixin, UniqueCourse self.set_programs_api_configuration(is_enabled=True) self.programs = ProgramFactory.create_batch(3) + self.credit_pathways = CreditPathwayFactory.create_batch(3) + for pathway in self.credit_pathways: + self.programs += pathway['programs'] + + # add some of the previously created programs to some pathways + self.credit_pathways[0]['programs'].extend([self.programs[0], self.programs[1]]) + self.credit_pathways[1]['programs'].append(self.programs[0]) + self.username = None def auth(self, enroll=True): @@ -44,9 +53,10 @@ class ProgramPageBase(ProgramsConfigMixin, CatalogIntegrationMixin, UniqueCourse program_type = ProgramTypeFactory() return ProgramFactory(courses=[course], type=program_type['name']) - def stub_catalog_api(self, programs): + def stub_catalog_api(self, programs, credit_pathways): """ - Stub the discovery service's program list and detail API endpoints. + Stub the discovery service's program list and detail API endpoints, as well as + the credit pathway list endpoint. """ self.set_catalog_integration(is_enabled=True, service_username=self.username) CatalogFixture().install_programs(programs) @@ -54,6 +64,8 @@ class ProgramPageBase(ProgramsConfigMixin, CatalogIntegrationMixin, UniqueCourse program_types = [program['type'] for program in programs] CatalogFixture().install_program_types(program_types) + CatalogFixture().install_credit_pathways(credit_pathways) + def cache_programs(self): """ Populate the LMS' cache of program data. @@ -73,7 +85,7 @@ class ProgramListingPageTest(ProgramPageBase): def test_no_enrollments(self): """Verify that no cards appear when the user has no enrollments.""" self.auth(enroll=False) - self.stub_catalog_api(self.programs) + self.stub_catalog_api(self.programs, self.credit_pathways) self.cache_programs() self.listing_page.visit() @@ -87,7 +99,7 @@ class ProgramListingPageTest(ProgramPageBase): but none are included in an active program. """ self.auth() - self.stub_catalog_api(self.programs) + self.stub_catalog_api(self.programs, self.credit_pathways) self.cache_programs() self.listing_page.visit() @@ -109,7 +121,7 @@ class ProgramListingPageA11yTest(ProgramPageBase): def test_empty_a11y(self): """Test a11y of the page's empty state.""" self.auth(enroll=False) - self.stub_catalog_api(programs=[self.program]) + self.stub_catalog_api(programs=[self.program], credit_pathways=[]) self.cache_programs() self.listing_page.visit() @@ -121,7 +133,7 @@ class ProgramListingPageA11yTest(ProgramPageBase): def test_cards_a11y(self): """Test a11y when program cards are present.""" self.auth() - self.stub_catalog_api(programs=[self.program]) + self.stub_catalog_api(programs=[self.program], credit_pathways=[]) self.cache_programs() self.listing_page.visit() @@ -145,7 +157,7 @@ class ProgramDetailsPageA11yTest(ProgramPageBase): def test_a11y(self): """Test the page's a11y compliance.""" self.auth() - self.stub_catalog_api(programs=[self.program]) + self.stub_catalog_api(programs=[self.program], credit_pathways=[]) self.cache_programs() self.details_page.visit() diff --git a/openedx/core/djangoapps/catalog/cache.py b/openedx/core/djangoapps/catalog/cache.py index ec73627c8714a50bf06b3a466915d887e0a36901..38460bcde40ca1c52050127491101aabe48d5fe4 100644 --- a/openedx/core/djangoapps/catalog/cache.py +++ b/openedx/core/djangoapps/catalog/cache.py @@ -3,3 +3,9 @@ PROGRAM_CACHE_KEY_TPL = 'program-{uuid}' # Cache key used to locate an item containing a list of all program UUIDs for a site. SITE_PROGRAM_UUIDS_CACHE_KEY_TPL = 'program-uuids-{domain}' + +# Template used to create cache keys for individual pathways +CREDIT_PATHWAY_CACHE_KEY_TPL = 'credit-pathway-{id}' + +# Cache key used to locate an item containing a list of all credit pathway ids for a site. +SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL = 'credit-pathway-ids-{domain}' diff --git a/openedx/core/djangoapps/catalog/management/commands/cache_programs.py b/openedx/core/djangoapps/catalog/management/commands/cache_programs.py index aa6583ea929a890cc53bb3ec71fabd19b1ac01c0..586206c9919e37f8fdb910833610f035e81049d8 100644 --- a/openedx/core/djangoapps/catalog/management/commands/cache_programs.py +++ b/openedx/core/djangoapps/catalog/management/commands/cache_programs.py @@ -7,7 +7,9 @@ from django.core.cache import cache from django.core.management import BaseCommand from openedx.core.djangoapps.catalog.cache import ( + CREDIT_PATHWAY_CACHE_KEY_TPL, PROGRAM_CACHE_KEY_TPL, + SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL, SITE_PROGRAM_UUIDS_CACHE_KEY_TPL ) from openedx.core.djangoapps.catalog.models import CatalogIntegration @@ -43,21 +45,27 @@ class Command(BaseCommand): raise programs = {} + pathways = {} 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'): logger.info('Skipping site {domain}. No configuration.'.format(domain=site.domain)) cache.set(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), [], None) + cache.set(SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL.format(domain=site.domain), [], None) continue client = create_catalog_api_client(user, site=site) uuids, program_uuids_failed = self.get_site_program_uuids(client, site) new_programs, program_details_failed = self.fetch_program_details(client, uuids) + new_pathways, pathways_failed = self.get_pathways(client, site) + new_pathways, new_programs, pathway_processing_failed = self.process_pathways(site, new_pathways, + new_programs) - if program_uuids_failed or program_details_failed: + if program_uuids_failed or program_details_failed or pathways_failed or pathway_processing_failed: failure = True programs.update(new_programs) + pathways.update(new_pathways) logger.info('Caching UUIDs for {total} programs for site {site_name}.'.format( total=len(uuids), @@ -65,10 +73,23 @@ class Command(BaseCommand): )) cache.set(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), uuids, None) - successful = len(programs) - logger.info('Caching details for {successful} programs.'.format(successful=successful)) + pathway_ids = new_pathways.keys() + logger.info('Caching ids for {total} credit pathways for site {site_name}.'.format( + total=len(pathway_ids), + site_name=site.domain, + )) + cache.set(SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL.format(domain=site.domain), pathway_ids, None) + + successful_programs = len(programs) + logger.info('Caching details for {successful_programs} programs.'.format( + successful_programs=successful_programs)) cache.set_many(programs, None) + successful_pathways = len(pathways) + logger.info('Caching details for {successful_pathways} credit pathways.'.format( + successful_pathways=successful_pathways)) + cache.set_many(pathways, None) + if failure: # This will fail a Jenkins job running this command, letting site # operators know that there was a problem. @@ -104,9 +125,59 @@ class Command(BaseCommand): cache_key = PROGRAM_CACHE_KEY_TPL.format(uuid=uuid) logger.info('Requesting details for program {uuid}.'.format(uuid=uuid)) program = client.programs(uuid).get(exclude_utm=1) + # pathways get added in process_pathways + program['pathway_ids'] = [] programs[cache_key] = program except: # pylint: disable=bare-except logger.exception('Failed to retrieve details for program {uuid}.'.format(uuid=uuid)) failure = True continue return programs, failure + + def get_pathways(self, client, site): + """ + Get all pathways for the current client + """ + pathways = {} + failure = False + logger.info('Requesting credit pathways for {domain}.'.format(domain=site.domain)) + try: + pathways = client.credit_pathways.get(exclude_utm=1)['results'] + except: # pylint: disable=bare-except + logger.error('Failed to retrieve credit pathways for site: {domain}.'.format(domain=site.domain)) + failure = True + + logger.info('Received {total} credit pathways for site {domain}'.format( + total=len(pathways), + domain=site.domain + )) + + return pathways, failure + + def process_pathways(self, site, pathways, programs): + """ + For each program, add references to each pathway it is a part of. + For each pathway, replace the "programs" dict with "program_uuids", + which only contains uuids (since program data is already cached) + """ + processed_pathways = {} + failure = False + for pathway in pathways: + try: + pathway_id = pathway['id'] + pathway_cache_key = CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway_id) + processed_pathways[pathway_cache_key] = pathway + uuids = [] + + for program in pathway['programs']: + program_uuid = program['uuid'] + program_cache_key = PROGRAM_CACHE_KEY_TPL.format(uuid=program_uuid) + programs[program_cache_key]['pathway_ids'].append(pathway_id) + uuids.append(program_uuid) + + del pathway['programs'] + pathway['program_uuids'] = uuids + except: # pylint: disable=bare-except + logger.error('Failed to process pathways for {domain}'.format(domain=site.domain)) + failure = True + return processed_pathways, programs, failure diff --git a/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py b/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py index 0b382cd4f91e1ebebb1af25e0e01ea4eb3db205c..7f46504a5bfbefd82203728896afea77e1148f2b 100644 --- a/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py +++ b/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py @@ -5,10 +5,12 @@ from django.core.cache import cache from django.core.management import call_command from openedx.core.djangoapps.catalog.cache import ( + CREDIT_PATHWAY_CACHE_KEY_TPL, PROGRAM_CACHE_KEY_TPL, + SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL, SITE_PROGRAM_UUIDS_CACHE_KEY_TPL ) -from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory +from openedx.core.djangoapps.catalog.tests.factories import CreditPathwayFactory, ProgramFactory from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms @@ -36,10 +38,20 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix self.list_url = self.catalog_integration.get_internal_api_url().rstrip('/') + '/programs/' self.detail_tpl = self.list_url.rstrip('/') + '/{uuid}/' + self.pathway_url = self.catalog_integration.get_internal_api_url().rstrip('/') + '/credit_pathways/' self.programs = ProgramFactory.create_batch(3) + self.pathways = CreditPathwayFactory.create_batch(3) + + for pathway in self.pathways: + self.programs += pathway['programs'] + self.uuids = [program['uuid'] for program in self.programs] + # add some of the previously created programs to some pathways + self.pathways[0]['programs'].extend([self.programs[0], self.programs[1]]) + self.pathways[1]['programs'].append(self.programs[0]) + def mock_list(self): def list_callback(request, uri, headers): expected = { @@ -74,7 +86,29 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix content_type='application/json' ) - def test_handle(self): + def mock_pathways(self): + """ + Mock the data for discovery's credit pathways endpoint + """ + def pathways_callback(request, uri, headers): # pylint: disable=unused-argument + """ + Mocks response + """ + expected = { + 'exclude_utm': ['1'], + } + self.assertEqual(request.querystring, expected) + + return (200, headers, json.dumps({'results': self.pathways})) + + httpretty.register_uri( + httpretty.GET, + self.pathway_url, + body=pathways_callback, + content_type='application/json' + ) + + def test_handle_programs(self): """ Verify that the command requests and caches program UUIDs and details. """ @@ -89,6 +123,7 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix } self.mock_list() + self.mock_pathways() for uuid in self.uuids: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] @@ -115,8 +150,58 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix # of the cache above, so all we need to do here is verify the accuracy of # the data itself. for key, program in cached_programs.items(): + # cached programs have a pathways field added to them, remove before comparing + del program['pathway_ids'] self.assertEqual(program, programs[key]) + def test_handle_pathways(self): + """ + Verify that the command requests and caches credit pathways + """ + + UserFactory(username=self.catalog_integration.service_username) + + programs = { + PROGRAM_CACHE_KEY_TPL.format(uuid=program['uuid']): program for program in self.programs + } + + pathways = { + CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']): pathway for pathway in self.pathways + } + + self.mock_list() + self.mock_pathways() + + for uuid in self.uuids: + program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] + self.mock_detail(uuid, program) + + call_command('cache_programs') + + cached_pathway_keys = cache.get(SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL.format(domain=self.site_domain)) + pathway_keys = pathways.keys() + self.assertEqual( + set(cached_pathway_keys), + set(pathway_keys) + ) + + cached_pathways = cache.get_many(pathway_keys) + self.assertEqual( + set(cached_pathways), + set(pathways) + ) + + # We can't use a set comparison here because these values are dictionaries + # and aren't hashable. We've already verified that all pathways came out + # of the cache above, so all we need to do here is verify the accuracy of + # the data itself. + for key, pathway in cached_pathways.items(): + # cached pathways store just program uuids instead of the full programs, transform before comparing + pathways[key]['program_uuids'] = [program['uuid'] for program in pathways[key]['programs']] + del pathways[key]['programs'] + + self.assertEqual(pathway, pathways[key]) + def test_handle_missing_service_user(self): """ Verify that the command raises an exception when run without a service @@ -142,6 +227,29 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain)) self.assertEqual(cached_uuids, []) + def test_handle_missing_pathways(self): + """ + Verify that the command raises an exception when it fails to retrieve pathways. + """ + UserFactory(username=self.catalog_integration.service_username) + + programs = { + PROGRAM_CACHE_KEY_TPL.format(uuid=program['uuid']): program for program in self.programs + } + + self.mock_list() + + for uuid in self.uuids: + program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] + self.mock_detail(uuid, program) + + with self.assertRaises(SystemExit) as context: + call_command('cache_programs') + self.assertEqual(context.exception.code, 1) + + cached_pathways = cache.get(SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL.format(domain=self.site_domain)) + self.assertEqual(cached_pathways, []) + def test_handle_missing_programs(self): """ Verify that a problem retrieving a program doesn't prevent the command @@ -183,4 +291,6 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix ) for key, program in cached_programs.items(): + # cached programs have a pathways field added to them, remove before comparing + del program['pathway_ids'] self.assertEqual(program, partial_programs[key]) diff --git a/openedx/core/djangoapps/catalog/tests/factories.py b/openedx/core/djangoapps/catalog/tests/factories.py index 0ff54fc12610aaccc181f2f47d79f9ae2cb42f6d..357dfd3dce9b53a5968936fe1837c13c59ad4611 100644 --- a/openedx/core/djangoapps/catalog/tests/factories.py +++ b/openedx/core/djangoapps/catalog/tests/factories.py @@ -211,3 +211,13 @@ class ProgramFactory(DictFactoryBase): class ProgramTypeFactory(DictFactoryBase): name = factory.Faker('word') logo_image = factory.LazyFunction(generate_sized_stdimage) + + +class CreditPathwayFactory(DictFactoryBase): + id = factory.Sequence(lambda x: x) + description = factory.Faker('sentence') + destination_url = factory.Faker('url') + email = factory.Faker('email') + name = factory.Faker('sentence') + org_name = factory.Faker('company') + programs = factory.LazyFunction(partial(generate_instances, ProgramFactory)) diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index 7f47a855ca598d39b09be2edec3ed91f8a1e7b95..d2609c83f74dca926136b3a73eb70b10e0439b46 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -16,11 +16,17 @@ from course_modes.helpers import CourseMode from course_modes.tests.factories import CourseModeFactory from entitlements.tests.factories import CourseEntitlementFactory from openedx.core.constants import COURSE_UNPUBLISHED -from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL, SITE_PROGRAM_UUIDS_CACHE_KEY_TPL +from openedx.core.djangoapps.catalog.cache import ( + CREDIT_PATHWAY_CACHE_KEY_TPL, + PROGRAM_CACHE_KEY_TPL, + SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL, + SITE_PROGRAM_UUIDS_CACHE_KEY_TPL +) from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.catalog.tests.factories import ( CourseFactory, CourseRunFactory, + CreditPathwayFactory, ProgramFactory, ProgramTypeFactory ) @@ -29,6 +35,7 @@ from openedx.core.djangoapps.catalog.utils import ( get_course_runs, get_course_runs_for_course, get_course_run_details, + get_credit_pathways, get_currency_data, get_localized_price_text, get_program_types, @@ -177,6 +184,137 @@ class TestGetPrograms(CacheIsolationTestCase): self.assertFalse(mock_warning.called) +@skip_unless_lms +@mock.patch(UTILS_MODULE + '.logger.info') +@mock.patch(UTILS_MODULE + '.logger.warning') +class TestGetCreditPathways(CacheIsolationTestCase): + ENABLED_CACHES = ['default'] + + def setUp(self): + super(TestGetCreditPathways, self).setUp() + self.site = SiteFactory() + + def test_get_many(self, mock_warning, mock_info): + pathways = CreditPathwayFactory.create_batch(3) + + # Cache details for 2 of 3 programs. + partial_pathways = { + CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']): pathway for pathway in pathways[:2] + } + cache.set_many(partial_pathways, None) + + # When called before pathways are cached, the function should return an + # empty list and log a warning. + self.assertEqual(get_credit_pathways(self.site), []) + mock_warning.assert_called_once_with('Failed to get credit pathway ids from the cache.') + mock_warning.reset_mock() + + # Cache all 3 pathways + cache.set( + SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL.format(domain=self.site.domain), + [pathway['id'] for pathway in pathways], + None + ) + + actual_pathways = get_credit_pathways(self.site) + + # The 2 cached pathways should be returned while info and warning + # messages should be logged for the missing one. + self.assertEqual( + set(pathway['id'] for pathway in actual_pathways), + set(pathway['id'] for pathway in partial_pathways.values()) + ) + mock_info.assert_called_with('Failed to get details for 1 pathways. Retrying.') + mock_warning.assert_called_with( + 'Failed to get details for credit pathway {id} from the cache.'.format(id=pathways[2]['id']) + ) + mock_warning.reset_mock() + + # We can't use a set comparison here because these values are dictionaries + # and aren't hashable. We've already verified that all pathways came out + # of the cache above, so all we need to do here is verify the accuracy of + # the data itself. + for pathway in actual_pathways: + key = CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']) + self.assertEqual(pathway, partial_pathways[key]) + + # Cache details for all 3 pathways. + all_pathways = { + CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']): pathway for pathway in pathways + } + cache.set_many(all_pathways, None) + + actual_pathways = get_credit_pathways(self.site) + + # All 3 pathways should be returned. + self.assertEqual( + set(pathway['id'] for pathway in actual_pathways), + set(pathway['id'] for pathway in all_pathways.values()) + ) + self.assertFalse(mock_warning.called) + + for pathway in actual_pathways: + key = CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']) + self.assertEqual(pathway, all_pathways[key]) + + @mock.patch(UTILS_MODULE + '.cache') + def test_get_many_with_missing(self, mock_cache, mock_warning, mock_info): + pathways = CreditPathwayFactory.create_batch(3) + + all_pathways = { + CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']): pathway for pathway in pathways + } + + partial_pathways = { + CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']): pathway for pathway in pathways[:2] + } + + def fake_get_many(keys): + if len(keys) == 1: + return {CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathways[-1]['id']): pathways[-1]} + else: + return partial_pathways + + mock_cache.get.return_value = [pathway['id'] for pathway in pathways] + mock_cache.get_many.side_effect = fake_get_many + + actual_pathways = get_credit_pathways(self.site) + + # All 3 cached pathways should be returned. An info message should be + # logged about the one that was initially missing, but the code should + # be able to stitch together all the details. + self.assertEqual( + set(pathway['id'] for pathway in actual_pathways), + set(pathway['id'] for pathway in all_pathways.values()) + ) + self.assertFalse(mock_warning.called) + mock_info.assert_called_with('Failed to get details for 1 pathways. Retrying.') + + for pathway in actual_pathways: + key = CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']) + self.assertEqual(pathway, all_pathways[key]) + + def test_get_one(self, mock_warning, _mock_info): + expected_pathway = CreditPathwayFactory() + expected_id = expected_pathway['id'] + + self.assertEqual(get_credit_pathways(self.site, pathway_id=expected_id), None) + mock_warning.assert_called_once_with( + 'Failed to get details for credit pathway {id} from the cache.'.format(id=expected_id) + ) + mock_warning.reset_mock() + + cache.set( + CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=expected_id), + expected_pathway, + None + ) + + actual_pathway = get_credit_pathways(self.site, pathway_id=expected_id) + self.assertEqual(actual_pathway, expected_pathway) + self.assertFalse(mock_warning.called) + + @mock.patch(UTILS_MODULE + '.get_edx_api_data') class TestGetProgramTypes(CatalogIntegrationMixin, TestCase): """Tests covering retrieval of program types from the catalog service.""" diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index 86d4d12cfffbe69a3e3d22aeba1591db517d7fea..b371f05355f95ea195e526276d98c1c8930cef6b 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -14,7 +14,8 @@ 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 (PROGRAM_CACHE_KEY_TPL, +from openedx.core.djangoapps.catalog.cache import (CREDIT_PATHWAY_CACHE_KEY_TPL, PROGRAM_CACHE_KEY_TPL, + SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL, SITE_PROGRAM_UUIDS_CACHE_KEY_TPL) from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.lib.edx_api_utils import get_edx_api_data @@ -124,6 +125,62 @@ def get_program_types(name=None): return [] +def get_credit_pathways(site, pathway_id=None): + """ + Read pathways from the cache. + The cache is populated by a management command, cache_programs. + + Arguments: + site (Site): django.contrib.sites.models object + + Keyword Arguments: + pathway_id (string): id identifying a specific pathway to read from the cache. + + Returns: + list of dict, representing pathways. + dict, if a specific pathway is requested. + """ + missing_details_msg_tpl = 'Failed to get details for credit pathway {id} from the cache.' + + if pathway_id: + pathway = cache.get(CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway_id)) + if not pathway: + logger.warning(missing_details_msg_tpl.format(id=pathway_id)) + + return pathway + pathway_ids = cache.get(SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL.format(domain=site.domain), []) + if not pathway_ids: + logger.warning('Failed to get credit pathway ids from the cache.') + + pathways = cache.get_many([CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway_id) for pathway_id in pathway_ids]) + pathways = pathways.values() + + # The get_many above sometimes fails to bring back details cached on one or + # more Memcached nodes. It doesn't look like these keys are being evicted. + # 99% of the time all keys come back, but 1% of the time all the keys stored + # on one or more nodes are missing from the result of the get_many. One + # get_many may fail to bring these keys back, but a get_many occurring + # immediately afterwards will succeed in bringing back all the keys. This + # behavior can be mitigated by trying again for the missing keys, which is + # what we do here. Splitting the get_many into smaller chunks may also help. + missing_ids = set(pathway_ids) - set(pathway['id'] for pathway in pathways) + if missing_ids: + logger.info( + 'Failed to get details for {count} pathways. Retrying.'.format(count=len(missing_ids)) + ) + + retried_pathways = cache.get_many( + [CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway_id) for pathway_id in missing_ids] + ) + pathways += retried_pathways.values() + + still_missing_ids = set(pathway_ids) - set(pathway['id'] for pathway in pathways) + for missing_id in still_missing_ids: + logger.warning(missing_details_msg_tpl.format(id=missing_id)) + + return pathways + + def get_currency_data(): """Retrieve currency data from the catalog service.