Remove use of read_cached_programs switch

The read_cached_programs switch was used to control the release of changes for reading programs exclusively from the cache. With those changes stable, the switch is no longer necessary.

LEARNER-382

common/test/acceptance/fixtures/catalog.py

@@ -13,23 +13,34 @@ class CatalogFixture(object):
     """
     Interface to set up mock responses from the Catalog stub server.
     """
-    def install_programs(self, data):
-        """Set response data for the catalog's course run API."""
+    def install_programs(self, programs):
+        """
+        Stub the discovery service's program list and detail API endpoints.
+
+        Arguments:
+            programs (list): A list of programs. Both list and detail endpoints
+                will be stubbed using data from this list.
+        """
         key = 'catalog.programs'
 
-        if isinstance(data, dict):
-            key += '.' + data['uuid']
+        uuids = []
+        for program in programs:
+            uuid = program['uuid']
+            uuids.append(uuid)
+
+            program_key = '{base}.{uuid}'.format(base=key, uuid=uuid)
 
             requests.put(
                 '{}/set_config'.format(CATALOG_STUB_URL),
-                data={key: json.dumps(data)},
-            )
-        else:
-            requests.put(
-                '{}/set_config'.format(CATALOG_STUB_URL),
-                data={key: json.dumps({'results': data})},
+                data={program_key: json.dumps(program)},
             )
 
+        # Stub list endpoint as if the uuids_only query param had been passed.
+        requests.put(
+            '{}/set_config'.format(CATALOG_STUB_URL),
+            data={key: json.dumps(uuids)},
+        )
+
 
 class CatalogIntegrationMixin(object):
     """Mixin providing a method used to configure the catalog integration."""

common/test/acceptance/pages/lms/catalog.py

+import re
+
+from bok_choy.page_object import PageObject
+
+from common.test.acceptance.pages.lms import BASE_URL
+
+
+class CacheProgramsPage(PageObject):
+    """
+    Visit this page to call the cache_programs management command.
+
+    This page makes a GET request to a view which is only meant to be enabled in
+    testing contexts where the LMS can only be reached over HTTP. Stub the
+    discovery service before visiting this page.
+    """
+    url = BASE_URL + '/catalog/management/cache_programs/'
+
+    def is_browser_on_page(self):
+        body = self.q(css='body').text[0]
+        match = re.search(r'programs cached', body, flags=re.IGNORECASE)
+
+        return True if match else False

common/test/acceptance/tests/lms/test_programs.py

@@ -6,6 +6,7 @@ from common.test.acceptance.fixtures.programs import ProgramsConfigMixin
 from common.test.acceptance.fixtures.course import CourseFixture
 from common.test.acceptance.tests.helpers import UniqueCourseTest
 from common.test.acceptance.pages.lms.auto_auth import AutoAuthPage
+from common.test.acceptance.pages.lms.catalog import CacheProgramsPage
 from common.test.acceptance.pages.lms.programs import ProgramListingPage, ProgramDetailsPage
 from openedx.core.djangoapps.catalog.tests.factories import (
     ProgramFactory, CourseFactory, CourseRunFactory
@@ -39,10 +40,19 @@ class ProgramPageBase(ProgramsConfigMixin, CatalogIntegrationMixin, UniqueCourse
 
         return ProgramFactory(courses=[course])
 
-    def stub_catalog_api(self, data=None):
-        """Stub out the catalog API's program and course run endpoints."""
+    def stub_catalog_api(self, programs):
+        """
+        Stub the discovery service's program list and detail API endpoints.
+        """
         self.set_catalog_integration(is_enabled=True, service_username=self.username)
-        CatalogFixture().install_programs(data or self.programs)
+        CatalogFixture().install_programs(programs)
+
+    def cache_programs(self):
+        """
+        Populate the LMS' cache of program data.
+        """
+        cache_programs_page = CacheProgramsPage(self.browser)
+        cache_programs_page.visit()
 
 
 class ProgramListingPageTest(ProgramPageBase):
@@ -55,7 +65,8 @@ 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.stub_catalog_api(self.programs)
+        self.cache_programs()
 
         self.listing_page.visit()
 
@@ -68,7 +79,8 @@ class ProgramListingPageTest(ProgramPageBase):
         but none are included in an active program.
         """
         self.auth()
-        self.stub_catalog_api()
+        self.stub_catalog_api(self.programs)
+        self.cache_programs()
 
         self.listing_page.visit()
 
@@ -83,7 +95,8 @@ class ProgramListingPageTest(ProgramPageBase):
         self.auth()
 
         program = self.create_program()
-        self.stub_catalog_api(data=[program])
+        self.stub_catalog_api(programs=[program])
+        self.cache_programs()
 
         self.listing_page.visit()
 
@@ -104,7 +117,8 @@ class ProgramListingPageA11yTest(ProgramPageBase):
     def test_empty_a11y(self):
         """Test a11y of the page's empty state."""
         self.auth(enroll=False)
-        self.stub_catalog_api(data=[self.program])
+        self.stub_catalog_api(programs=[self.program])
+        self.cache_programs()
 
         self.listing_page.visit()
 
@@ -115,7 +129,8 @@ class ProgramListingPageA11yTest(ProgramPageBase):
     def test_cards_a11y(self):
         """Test a11y when program cards are present."""
         self.auth()
-        self.stub_catalog_api(data=[self.program])
+        self.stub_catalog_api(programs=[self.program])
+        self.cache_programs()
 
         self.listing_page.visit()
 
@@ -138,7 +153,8 @@ class ProgramDetailsPageA11yTest(ProgramPageBase):
     def test_a11y(self):
         """Test the page's a11y compliance."""
         self.auth()
-        self.stub_catalog_api(data=self.program)
+        self.stub_catalog_api(programs=[self.program])
+        self.cache_programs()
 
         self.details_page.visit()
 

lms/djangoapps/courseware/tests/test_views.py

@@ -50,12 +50,10 @@ from lms.djangoapps.grades.config.waffle import waffle as grades_waffle, ASSUME_
 from milestones.tests.utils import MilestonesTestCaseMixin
 from openedx.core.djangoapps.catalog.tests.factories import CourseFactory as CatalogCourseFactory
 from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory, CourseRunFactory
-from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.crawlers.models import CrawlersConfig
 from openedx.core.djangoapps.credit.api import set_credit_requirements
 from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider
-from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin
 from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
 from openedx.core.lib.gating import api as gating_api
 from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired
@@ -966,8 +964,10 @@ class ViewsTestCase(ModuleStoreTestCase):
 
 
 @attr(shard=2)
-@patch('openedx.core.djangoapps.catalog.utils.get_edx_api_data')
-class TestProgramMarketingView(ProgramsApiConfigMixin, CatalogIntegrationMixin, SharedModuleStoreTestCase):
+# Patching 'lms.djangoapps.courseware.views.views.get_programs' would be ideal,
+# but for some unknown reason that patch doesn't seem to be applied.
+@patch('openedx.core.djangoapps.catalog.utils.cache')
+class TestProgramMarketingView(SharedModuleStoreTestCase):
     """Unit tests for the program marketing page."""
     program_uuid = str(uuid4())
     url = reverse('program_marketing_view', kwargs={'program_uuid': program_uuid})
@@ -982,25 +982,20 @@ class TestProgramMarketingView(ProgramsApiConfigMixin, CatalogIntegrationMixin,
 
         cls.data = ProgramFactory(uuid=cls.program_uuid, courses=[course])
 
-    def test_404_if_no_data(self, _mock_get_edx_api_data):
+    def test_404_if_no_data(self, mock_cache):
         """
         Verify that the page 404s if no program data is found.
         """
-        self.create_programs_config()
+        mock_cache.get.return_value = None
 
         response = self.client.get(self.url)
         self.assertEqual(response.status_code, 404)
 
-    def test_200(self, mock_get_edx_api_data):
+    def test_200(self, mock_cache):
         """
         Verify the view returns a 200.
         """
-        self.create_programs_config()
-
-        catalog_integration = self.create_catalog_integration()
-        UserFactory(username=catalog_integration.service_username)
-
-        mock_get_edx_api_data.return_value = self.data
+        mock_cache.get.return_value = self.data
 
         response = self.client.get(self.url)
         self.assertEqual(response.status_code, 200)

lms/djangoapps/learner_dashboard/tests/test_programs.py

@@ -228,7 +228,7 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar
 
 
 @skip_unless_lms
-@mock.patch(CATALOG_UTILS_MODULE + '.get_edx_api_data')
+@mock.patch(PROGRAMS_UTILS_MODULE + '.get_programs')
 class TestProgramDetails(ProgramsApiConfigMixin, CatalogIntegrationMixin, SharedModuleStoreTestCase):
     """Unit tests for the program details page."""
     program_uuid = str(uuid4())
@@ -266,7 +266,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, CatalogIntegrationMixin, Shared
             any(soup.find_all('a', class_='tab-nav-link', href=reverse('program_listing_view')))
         )
 
-    def test_login_required(self, mock_get_edx_api_data):
+    def test_login_required(self, mock_get_programs):
         """
         Verify that login is required to access the page.
         """
@@ -275,7 +275,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, CatalogIntegrationMixin, Shared
         catalog_integration = self.create_catalog_integration()
         UserFactory(username=catalog_integration.service_username)
 
-        mock_get_edx_api_data.return_value = self.data
+        mock_get_programs.return_value = self.data
 
         self.client.logout()
 
@@ -290,7 +290,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, CatalogIntegrationMixin, Shared
         response = self.client.get(self.url)
         self.assert_program_data_present(response)
 
-    def test_404_if_disabled(self, _mock_get_edx_api_data):
+    def test_404_if_disabled(self, _mock_get_programs):
         """
         Verify that the page 404s if disabled.
         """
@@ -299,9 +299,11 @@ class TestProgramDetails(ProgramsApiConfigMixin, CatalogIntegrationMixin, Shared
         response = self.client.get(self.url)
         self.assertEqual(response.status_code, 404)
 
-    def test_404_if_no_data(self, _mock_get_edx_api_data):
+    def test_404_if_no_data(self, mock_get_programs):
         """Verify that the page 404s if no program data is found."""
         self.create_programs_config()
 
+        mock_get_programs.return_value = None
+
         response = self.client.get(self.url)
         self.assertEqual(response.status_code, 404)

lms/envs/bok_choy.env.json

@@ -84,6 +84,7 @@
         "ALLOW_AUTOMATED_SIGNUPS": true,
         "AUTOMATIC_AUTH_FOR_TESTING": true,
         "MODE_CREATION_FOR_TESTING": true,
+        "EXPOSE_CACHE_PROGRAMS_ENDPOINT": true,
         "AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING": true,
         "ENABLE_COURSE_DISCOVERY": true,
         "ENABLE_SPECIAL_EXAMS": true,

lms/envs/common.py

@@ -290,6 +290,10 @@ FEATURES = {
     # For easily adding modes to courses during acceptance testing
     'MODE_CREATION_FOR_TESTING': False,
 
+    # For caching programs in contexts where the LMS can only
+    # be reached over HTTP.
+    'EXPOSE_CACHE_PROGRAMS_ENDPOINT': False,
+
     # Courseware search feature
     'ENABLE_COURSEWARE_SEARCH': False,
 

lms/urls.py

@@ -86,6 +86,8 @@ urlpatterns = (
     url(r'^rss_proxy/', include('rss_proxy.urls', namespace='rss_proxy')),
     url(r'^api/organizations/', include('organizations.urls', namespace='organizations')),
 
+    url(r'^catalog/', include('openedx.core.djangoapps.catalog.urls', namespace='catalog')),
+
     # Update session view
     url(
         r'^lang_pref/session_language',

openedx/core/djangoapps/catalog/tests/test_utils.py

@@ -7,7 +7,6 @@ import mock
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
 from django.test import TestCase
-from waffle.models import Switch
 
 from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL, PROGRAM_UUIDS_CACHE_KEY
 from openedx.core.djangoapps.catalog.models import CatalogIntegration
@@ -29,14 +28,9 @@ User = get_user_model()  # pylint: disable=invalid-name
 
 @skip_unless_lms
 @mock.patch(UTILS_MODULE + '.logger.warning')
-class TestGetCachedPrograms(CacheIsolationTestCase):
+class TestGetPrograms(CacheIsolationTestCase):
     ENABLED_CACHES = ['default']
 
-    def setUp(self):
-        super(TestGetCachedPrograms, self).setUp()
-
-        Switch.objects.create(name='read_cached_programs', active=True)
-
     def test_get_many(self, mock_warning):
         programs = ProgramFactory.create_batch(3)
 
@@ -122,130 +116,6 @@ class TestGetCachedPrograms(CacheIsolationTestCase):
 
 @skip_unless_lms
 @mock.patch(UTILS_MODULE + '.get_edx_api_data')
-class TestGetPrograms(CatalogIntegrationMixin, TestCase):
-    """Tests covering retrieval of programs from the catalog service."""
-    def setUp(self):
-        super(TestGetPrograms, self).setUp()
-
-        self.uuid = str(uuid.uuid4())
-        self.types = ['Foo', 'Bar', 'FooBar']
-        self.catalog_integration = self.create_catalog_integration(cache_ttl=1)
-
-        UserFactory(username=self.catalog_integration.service_username)
-
-    def assert_contract(self, call_args, program_uuid=None, types=None):
-        """Verify that API data retrieval utility is used correctly."""
-        args, kwargs = call_args
-
-        for arg in (self.catalog_integration, 'programs'):
-            self.assertIn(arg, args)
-
-        self.assertEqual(kwargs['resource_id'], program_uuid)
-
-        types_param = ','.join(types) if types and isinstance(types, list) else None
-        cache_key = '{base}.programs{types}'.format(
-            base=self.catalog_integration.CACHE_KEY,
-            types='.' + types_param if types_param else ''
-        )
-        self.assertEqual(
-            kwargs['cache_key'],
-            cache_key if self.catalog_integration.is_cache_enabled else None
-        )
-
-        self.assertEqual(kwargs['api']._store['base_url'], self.catalog_integration.internal_api_url)  # pylint: disable=protected-access
-
-        querystring = {
-            'exclude_utm': 1,
-            'status': ('active', 'retired',),
-        }
-        if program_uuid:
-            querystring['use_full_course_serializer'] = 1
-        if types:
-            querystring['types'] = types_param
-
-        self.assertEqual(kwargs['querystring'], querystring)
-
-        return args, kwargs
-
-    def test_get_programs(self, mock_get_edx_api_data):
-        programs = ProgramFactory.create_batch(3)
-        mock_get_edx_api_data.return_value = programs
-
-        data = get_programs()
-
-        self.assert_contract(mock_get_edx_api_data.call_args)
-        self.assertEqual(data, programs)
-
-    def test_get_one_program(self, mock_get_edx_api_data):
-        program = ProgramFactory()
-        mock_get_edx_api_data.return_value = program
-
-        data = get_programs(uuid=self.uuid)
-
-        self.assert_contract(mock_get_edx_api_data.call_args, program_uuid=self.uuid)
-        self.assertEqual(data, program)
-
-    def test_programs_unavailable(self, mock_get_edx_api_data):
-        mock_get_edx_api_data.return_value = []
-
-        data = get_programs()
-
-        self.assert_contract(mock_get_edx_api_data.call_args)
-        self.assertEqual(data, [])
-
-    def test_cache_disabled(self, mock_get_edx_api_data):
-        self.catalog_integration = self.create_catalog_integration(cache_ttl=0)
-        get_programs()
-        self.assert_contract(mock_get_edx_api_data.call_args)
-
-    def test_config_missing(self, _mock_get_edx_api_data):
-        """
-        Verify that no errors occur if this method is called when catalog config
-        is missing.
-        """
-        CatalogIntegration.objects.all().delete()
-
-        data = get_programs()
-        self.assertEqual(data, [])
-
-    def test_service_user_missing(self, _mock_get_edx_api_data):
-        """
-        Verify that no errors occur if this method is called when the catalog
-        service user is missing.
-        """
-        # Note: Deleting the service user would be ideal, but causes mysterious
-        # errors on Jenkins.
-        self.create_catalog_integration(service_username='nonexistent-user')
-
-        data = get_programs()
-        self.assertEqual(data, [])
-
-    @mock.patch(UTILS_MODULE + '.get_programs')
-    @mock.patch(UTILS_MODULE + '.get_program_types')
-    def test_get_programs_with_type(self, mock_get_program_types, mock_get_programs, _mock_get_edx_api_data):
-        """Verify get_programs_with_type returns the expected list of programs."""
-        programs_with_program_type = []
-        programs = ProgramFactory.create_batch(2)
-        program_types = []
-
-        for program in programs:
-            program_type = ProgramTypeFactory(name=program['type'])
-            program_types.append(program_type)
-
-            program_with_type = copy.deepcopy(program)
-            program_with_type['type'] = program_type
-            programs_with_program_type.append(program_with_type)
-
-        mock_get_programs.return_value = programs
-        mock_get_program_types.return_value = program_types
-
-        actual = get_programs_with_type()
-
-        self.assertEqual(actual, programs_with_program_type)
-
-
-@skip_unless_lms
-@mock.patch(UTILS_MODULE + '.get_edx_api_data')
 class TestGetProgramTypes(CatalogIntegrationMixin, TestCase):
     """Tests covering retrieval of program types from the catalog service."""
     def test_get_program_types(self, mock_get_edx_api_data):

openedx/core/djangoapps/catalog/urls.py

+from django.conf.urls import url
+
+from . import views
+
+
+urlpatterns = [
+    url(r'^management/cache_programs/$', views.cache_programs, name='cache_programs'),
+]

openedx/core/djangoapps/catalog/utils.py

@@ -2,7 +2,6 @@
 import copy
 import logging
 
-import waffle
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
@@ -39,62 +38,27 @@ def get_programs(uuid=None):
         list of dict, representing programs.
         dict, if a specific program is requested.
     """
-    if waffle.switch_is_active('read_cached_programs'):
-        missing_details_msg_tpl = 'Details for program {uuid} are not cached.'
+    missing_details_msg_tpl = 'Details for program {uuid} are not cached.'
 
-        if uuid:
-            program = cache.get(PROGRAM_CACHE_KEY_TPL.format(uuid=uuid))
-            if not program:
-                logger.warning(missing_details_msg_tpl.format(uuid=uuid))
+    if uuid:
+        program = cache.get(PROGRAM_CACHE_KEY_TPL.format(uuid=uuid))
+        if not program:
+            logger.warning(missing_details_msg_tpl.format(uuid=uuid))
 
-            return program
+        return program
 
-        uuids = cache.get(PROGRAM_UUIDS_CACHE_KEY, [])
-        if not uuids:
-            logger.warning('Program UUIDs are not cached.')
+    uuids = cache.get(PROGRAM_UUIDS_CACHE_KEY, [])
+    if not uuids:
+        logger.warning('Program UUIDs are not cached.')
 
-        programs = cache.get_many([PROGRAM_CACHE_KEY_TPL.format(uuid=uuid) for uuid in uuids])
-        programs = list(programs.values())
+    programs = cache.get_many([PROGRAM_CACHE_KEY_TPL.format(uuid=uuid) for uuid in uuids])
+    programs = list(programs.values())
 
-        missing_uuids = set(uuids) - set(program['uuid'] for program in programs)
-        for uuid in missing_uuids:
-            logger.warning(missing_details_msg_tpl.format(uuid=uuid))
+    missing_uuids = set(uuids) - set(program['uuid'] for program in programs)
+    for uuid in missing_uuids:
+        logger.warning(missing_details_msg_tpl.format(uuid=uuid))
 
-        return programs
-    else:
-        # Old implementation which may request programs in-process. To be removed
-        # as part of LEARNER-382.
-        catalog_integration = CatalogIntegration.current()
-        if catalog_integration.enabled:
-            try:
-                user = User.objects.get(username=catalog_integration.service_username)
-            except User.DoesNotExist:
-                return []
-
-            api = create_catalog_api_client(user, catalog_integration)
-
-            cache_key = '{base}.programs'.format(
-                base=catalog_integration.CACHE_KEY
-            )
-
-            querystring = {
-                'exclude_utm': 1,
-                'status': ('active', 'retired',),
-            }
-
-            if uuid:
-                querystring['use_full_course_serializer'] = 1
-
-            return get_edx_api_data(
-                catalog_integration,
-                'programs',
-                api=api,
-                resource_id=uuid,
-                cache_key=cache_key if catalog_integration.is_cache_enabled else None,
-                querystring=querystring,
-            )
-        else:
-            return []
+    return programs
 
 
 def get_program_types(name=None):

openedx/core/djangoapps/catalog/views.py

+from django.conf import settings
+from django.core.management import call_command
+from django.http import Http404, HttpResponse
+from django.views.decorators.http import require_GET
+
+
+@require_GET
+def cache_programs(request):
+    """
+    Call the cache_programs management command.
+
+    This view is intended for use in testing contexts where the LMS can only be
+    reached over HTTP (e.g., Selenium-based browser tests). The discovery service
+    API the management command attempts to call should be stubbed out first.
+    """
+    if settings.FEATURES.get('EXPOSE_CACHE_PROGRAMS_ENDPOINT'):
+        call_command('cache_programs')
+
+        return HttpResponse('Programs cached.')
+
+    raise Http404