FC-0001: Remove old EdxRestAPIClient usage across the platform (#30301)

* refactor: remove EdxRestAPIClient

* test: update tests according to EdxRestAPIClient removal

* fix: remove unused import

openedx/features/enterprise_support/api.py

@@ -2,11 +2,12 @@
 APIs providing support for enterprise functionality.
 """
 
-
 import logging
 import traceback
 from functools import wraps
+from urllib.parse import urljoin
 
+import requests
 from crum import get_current_request
 from django.conf import settings
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
@@ -18,8 +19,8 @@ from django.urls import reverse
 from django.utils.http import urlencode
 from django.utils.translation import gettext as _
 from edx_django_utils.cache import TieredCache, get_cache_key
-from edx_rest_api_client.client import EdxRestApiClient
-from slumber.exceptions import HttpClientError, HttpNotFoundError, HttpServerError
+from edx_rest_api_client.auth import SuppliedJwtAuth
+from requests.exceptions import HTTPError
 
 from common.djangoapps.third_party_auth.pipeline import get as get_partial_pipeline
 from common.djangoapps.third_party_auth.provider import Registry
@@ -70,13 +71,12 @@ class ConsentApiClient:
         provided user.
         """
         jwt = create_jwt_for_user(user)
-        url = configuration_helpers.get_value('ENTERPRISE_CONSENT_API_URL', settings.ENTERPRISE_CONSENT_API_URL)
-        self.client = EdxRestApiClient(
-            url,
-            jwt=jwt,
-            append_slash=False,
+        base_api_url = configuration_helpers.get_value(
+            'ENTERPRISE_CONSENT_API_URL', settings.ENTERPRISE_CONSENT_API_URL
         )
-        self.consent_endpoint = self.client.data_sharing_consent
+        self.client = requests.Session()
+        self.client.auth = SuppliedJwtAuth(jwt)
+        self.consent_endpoint = urljoin(f"{base_api_url}/", "data_sharing_consent")
 
     def revoke_consent(self, **kwargs):
         """
@@ -85,7 +85,9 @@ class ConsentApiClient:
         This endpoint takes any given kwargs, which are understood as filtering the
         conceptual scope of the consent involved in the request.
         """
-        return self.consent_endpoint.delete(**kwargs)
+        response = self.client.delete(self.consent_endpoint, json=kwargs)
+        response.raise_for_status()
+        return response.json()
 
     def provide_consent(self, **kwargs):
         """
@@ -94,7 +96,9 @@ class ConsentApiClient:
         This endpoint takes any given kwargs, which are understood as filtering the
         conceptual scope of the consent involved in the request.
         """
-        return self.consent_endpoint.post(kwargs)
+        response = self.client.post(self.consent_endpoint, json=kwargs)
+        response.raise_for_status()
+        return response.json()
 
     def consent_required(self, enrollment_exists=False, **kwargs):
         """
@@ -105,7 +109,9 @@ class ConsentApiClient:
         """
 
         # Call the endpoint with the given kwargs, and check the value that it provides.
-        response = self.consent_endpoint.get(**kwargs)
+        response = self.client.get(self.consent_endpoint, params=kwargs)
+        response.raise_for_status()
+        response = response.json()
 
         LOGGER.info(
             '[ENTERPRISE DSC] Consent Requirement Info. APIParams: [%s], APIResponse: [%s], EnrollmentExists: [%s]',
@@ -149,19 +155,21 @@ class EnterpriseApiClient:
 
     def __init__(self, user):
         """
-        Initialize an authenticated Enterprise service API client by using the
-        provided user.
+        Initialize an authenticated Enterprise service API client.
+
+        Authentificate by jwt token using the provided user.
         """
         self.user = user
         jwt = create_jwt_for_user(user)
-        self.client = EdxRestApiClient(
-            configuration_helpers.get_value('ENTERPRISE_API_URL', settings.ENTERPRISE_API_URL),
-            jwt=jwt
-        )
+        self.base_api_url = configuration_helpers.get_value('ENTERPRISE_API_URL', settings.ENTERPRISE_API_URL)
+        self.client = requests.Session()
+        self.client.auth = SuppliedJwtAuth(jwt)
 
     def get_enterprise_customer(self, uuid):
-        endpoint = getattr(self.client, 'enterprise-customer')
-        return endpoint(uuid).get()
+        api_url = urljoin(f"{self.base_api_url}/", f"enterprise-customer/{uuid}/")
+        response = self.client.get(api_url)
+        response.raise_for_status()
+        return response.json()
 
     def post_enterprise_course_enrollment(self, username, course_id):
         """
@@ -171,10 +179,11 @@ class EnterpriseApiClient:
             'username': username,
             'course_id': course_id,
         }
-        endpoint = getattr(self.client, 'enterprise-course-enrollment')
+        api_url = urljoin(f"{self.base_api_url}/", "enterprise-course-enrollment/")
         try:
-            endpoint.post(data=data)
-        except (HttpClientError, HttpServerError):
+            response = self.client.post(api_url, data=data)
+            response.raise_for_status()
+        except HTTPError:
             message = (
                 "An error occured while posting EnterpriseCourseEnrollment for user {username} and "
                 "course run {course_id}."
@@ -251,26 +260,17 @@ class EnterpriseApiClient:
                     }
                 ],
             }
-
-        Raises:
-            ConnectionError: requests exception "ConnectionError", raised if if ecommerce is unable to connect
-                to enterprise api server.
-            SlumberBaseException: base slumber exception "SlumberBaseException", raised if API response contains
-                http error status like 4xx, 5xx etc.
-            Timeout: requests exception "Timeout", raised if enterprise API is taking too long for returning
-                a response. This exception is raised for both connection timeout and read timeout.
-
         """
         if not user.is_authenticated:
             return None
 
-        api_resource_name = 'enterprise-learner'
+        api_url = urljoin(f"{self.base_api_url}/", "enterprise-learner/")
 
         try:
-            endpoint = getattr(self.client, api_resource_name)
             querystring = {'username': user.username}
-            response = endpoint().get(**querystring)
-        except (HttpClientError, HttpServerError):
+            response = self.client.get(api_url, params=querystring)
+            response.raise_for_status()
+        except HTTPError:
             LOGGER.exception(
                 'Failed to get enterprise-learner for user [%s] with client user [%s]. Caller: %s, Request PATH: %s',
                 user.username,
@@ -280,7 +280,7 @@ class EnterpriseApiClient:
             )
             return None
 
-        return response
+        return response.json()
 
 
 class EnterpriseApiServiceClient(EnterpriseServiceClientMixin, EnterpriseApiClient):
@@ -295,8 +295,10 @@ class EnterpriseApiServiceClient(EnterpriseServiceClientMixin, EnterpriseApiClie
         """
         enterprise_customer = enterprise_customer_from_cache(uuid=uuid)
         if enterprise_customer is _CACHE_MISS:
-            endpoint = getattr(self.client, 'enterprise-customer')
-            enterprise_customer = endpoint(uuid).get()
+            api_url = urljoin(f"{self.base_api_url}/", f"enterprise-customer/{uuid}/")
+            response = self.client.get(api_url)
+            response.raise_for_status()
+            enterprise_customer = response.json() if response.content else None
             if enterprise_customer:
                 cache_enterprise(enterprise_customer)
 
@@ -476,8 +478,11 @@ def enterprise_customer_from_api(request):
 
         try:
             enterprise_customer = enterprise_api_client.get_enterprise_customer(enterprise_customer_uuid)
-        except HttpNotFoundError:
-            enterprise_customer = None
+        except HTTPError as err:
+            if err.response.status_code == 404:
+                enterprise_customer = None
+            else:
+                raise
     return enterprise_customer
 
 

openedx/features/enterprise_support/signals.py

@@ -4,23 +4,24 @@ This module contains signals related to enterprise.
 
 
 import logging
+from urllib.parse import urljoin
 
 from django.conf import settings
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
 from django.db.models.signals import post_save, pre_save
 from django.dispatch import receiver
-from enterprise.models import EnterpriseCourseEnrollment, EnterpriseCustomer, EnterpriseCustomerUser  # lint-amnesty, pylint: disable=unused-import
+from enterprise.models import EnterpriseCourseEnrollment, EnterpriseCustomer
 from integrated_channels.integrated_channel.tasks import (
     transmit_single_learner_data,
     transmit_single_subsection_learner_data
 )
-from slumber.exceptions import HttpClientError
+from requests.exceptions import HTTPError
 
-from openedx.core.djangoapps.commerce.utils import ecommerce_api_client
-from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED, COURSE_ASSESSMENT_GRADE_CHANGED
+from common.djangoapps.student.signals import UNENROLL_DONE
+from openedx.core.djangoapps.commerce.utils import get_ecommerce_api_base_url, get_ecommerce_api_client
+from openedx.core.djangoapps.signals.signals import COURSE_ASSESSMENT_GRADE_CHANGED, COURSE_GRADE_NOW_PASSED
 from openedx.features.enterprise_support.tasks import clear_enterprise_customer_data_consent_share_cache
 from openedx.features.enterprise_support.utils import clear_data_consent_share_cache, is_enterprise_learner
-from common.djangoapps.student.signals import UNENROLL_DONE
 
 log = logging.getLogger(__name__)
 
@@ -105,13 +106,17 @@ def refund_order_voucher(sender, course_enrollment, skip_refund=False, **kwargs)
         return
 
     service_user = User.objects.get(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME)
-    client = ecommerce_api_client(service_user)
+    client = get_ecommerce_api_client(service_user)
+    api_url = urljoin(
+        f"{get_ecommerce_api_base_url()}/", "coupons/create_refunded_voucher/"
+    )
     order_number = course_enrollment.get_order_attribute_value('order_number')
     if order_number:
         error_message = "Encountered {} from ecommerce while creating refund voucher. Order={}, enrollment={}, user={}"
         try:
-            client.enterprise.coupons.create_refunded_voucher.post({"order": order_number})
-        except HttpClientError as ex:
+            response = client.post(api_url, data={"order": order_number})
+            response.raise_for_status()
+        except HTTPError as ex:
             log.info(
                 error_message.format(type(ex).__name__, order_number, course_enrollment, course_enrollment.user)
             )

openedx/features/enterprise_support/tests/test_api.py

@@ -15,8 +15,8 @@ from django.test.utils import override_settings
 from django.urls import reverse
 from edx_django_utils.cache import get_cache_key
 from enterprise.models import EnterpriseCustomerUser  # lint-amnesty, pylint: disable=wrong-import-order
+from requests.exceptions import HTTPError
 from six.moves.urllib.parse import parse_qs
-from slumber.exceptions import HttpClientError
 
 from common.djangoapps.student.tests.factories import UserFactory
 from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
@@ -48,13 +48,13 @@ from openedx.features.enterprise_support.api import (
     get_enterprise_learner_data_from_db,
     get_enterprise_learner_portal_enabled_message,
     insert_enterprise_pipeline_elements,
-    unlink_enterprise_user_from_idp
+    unlink_enterprise_user_from_idp,
 )
 from openedx.features.enterprise_support.tests import FEATURES_WITH_ENTERPRISE_ENABLED
 from openedx.features.enterprise_support.tests.factories import (
     EnterpriseCourseEnrollmentFactory,
     EnterpriseCustomerIdentityProviderFactory,
-    EnterpriseCustomerUserFactory
+    EnterpriseCustomerUserFactory,
 )
 from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseServiceMockMixin
 from openedx.features.enterprise_support.utils import clear_data_consent_share_cache
@@ -98,7 +98,7 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
 
         mocked_jwt_builder.assert_called_once_with(enterprise_service_user)
         # pylint: disable=protected-access
-        assert enterprise_api_service_client.client._store['session'].auth.token == 'test-token'
+        assert enterprise_api_service_client.client.auth.token == 'test-token'
 
     def _assert_api_client_with_user(self, api_client, mocked_jwt_builder):
         """
@@ -115,7 +115,7 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
 
         mocked_jwt_builder.assert_called_once_with(dummy_enterprise_user)
         # pylint: disable=protected-access
-        assert enterprise_api_service_client.client._store['session'].auth.token == 'test-token'
+        assert enterprise_api_service_client.client.auth.token == 'test-token'
         return enterprise_api_service_client
 
     def _assert_get_enterprise_customer(self, api_client, enterprise_api_data_for_mock):
@@ -181,10 +181,10 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
         authenticate and access enterprise API.
         """
         api_client = self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder)
-        setattr(api_client.client, 'enterprise-course-enrollment', mock.Mock())
-        mock_endpoint = getattr(api_client.client, 'enterprise-course-enrollment')
+        mock_client = mock.Mock()
+        api_client.client = mock_client
         if should_raise_http_error:
-            mock_endpoint.post.side_effect = HttpClientError
+            mock_client.post.side_effect = HTTPError
 
         username = 'spongebob'
         course_id = 'burger-flipping-101'
@@ -195,10 +195,13 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
         else:
             api_client.post_enterprise_course_enrollment(username, course_id)
 
-        mock_endpoint.post.assert_called_once_with(data={
-            'username': username,
-            'course_id': course_id,
-        })
+        mock_client.post.assert_called_once_with(
+            f"{api_client.base_api_url}enterprise-course-enrollment/",
+            data={
+                'username': username,
+                'course_id': course_id,
+            }
+        )
 
     @mock.patch('openedx.features.enterprise_support.api.enterprise_customer_uuid_for_request')
     @mock.patch('openedx.features.enterprise_support.api.EnterpriseApiClient')
@@ -227,7 +230,8 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
         user to authenticate and access enterprise API.
         """
         consent_client = self._assert_api_client_with_user(ConsentApiClient, mock_jwt_builder)
-        consent_client.consent_endpoint = mock.Mock()
+        mock_client = mock.Mock()
+        consent_client.client = mock_client
 
         kwargs = {
             'foo': 'a',
@@ -236,8 +240,8 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
         consent_client.provide_consent(**kwargs)
         consent_client.revoke_consent(**kwargs)
 
-        consent_client.consent_endpoint.post.assert_called_once_with(kwargs)
-        consent_client.consent_endpoint.delete.assert_called_once_with(**kwargs)
+        mock_client.post.assert_called_once_with(consent_client.consent_endpoint, json=kwargs)
+        mock_client.delete.assert_called_once_with(consent_client.consent_endpoint, json=kwargs)
 
     @httpretty.activate
     @mock.patch('openedx.features.enterprise_support.api.get_enterprise_learner_data_from_db')
@@ -343,23 +347,33 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
 
     @mock.patch('openedx.features.enterprise_support.api.create_jwt_for_user')
     def test_fetch_enterprise_learner_data(self, mock_jwt_builder):
+        """
+        Test EnterpriseApiClient's fetch_enterprise_learner_data method.
+        """
         api_client = self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder)
-        setattr(api_client.client, 'enterprise-learner', mock.Mock())
-        mock_endpoint = getattr(api_client.client, 'enterprise-learner')
+        mock_client = mock.Mock()
+        api_client.client = mock_client
 
         user = mock.Mock(is_authenticated=True, username='spongebob')
         response = api_client.fetch_enterprise_learner_data(user)
 
-        assert mock_endpoint.return_value.get.return_value == response
-        mock_endpoint.return_value.get.assert_called_once_with(username=user.username)
+        assert mock_client.get.return_value.json.return_value == response
+        mock_client.get.assert_called_once_with(
+            f"{api_client.base_api_url}enterprise-learner/",
+            params={'username': user.username},
+        )
 
     @mock.patch('openedx.features.enterprise_support.api.get_current_request')
     @mock.patch('openedx.features.enterprise_support.api.create_jwt_for_user')
     def test_fetch_enterprise_learner_data_http_error(self, mock_jwt_builder, mock_get_current_request):
+        """
+        Test error handling for the EnterpriseApiClient's fetch_enterprise_learner_data method.
+        """
         api_client = self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder)
-        setattr(api_client.client, 'enterprise-learner', mock.Mock())
-        mock_endpoint = getattr(api_client.client, 'enterprise-learner')
-        mock_endpoint.return_value.get.side_effect = HttpClientError
+
+        mock_client = mock.Mock()
+        mock_client.get.side_effect = HTTPError
+        api_client.client = mock_client
         mock_get_current_request.return_value.META = {
             'PATH_INFO': 'whatever',
         }
@@ -367,8 +381,8 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
         user = mock.Mock(is_authenticated=True, username='spongebob')
 
         assert api_client.fetch_enterprise_learner_data(user) is None
-
-        mock_endpoint.return_value.get.assert_called_once_with(username=user.username)
+        url = f"{api_client.base_api_url}enterprise-learner/"
+        mock_client.get.assert_called_once_with(url, params={'username': user.username})
 
     @mock.patch('openedx.features.enterprise_support.api.EnterpriseApiClient')
     def test_get_enterprise_learner_data_from_api(self, mock_api_client_class):

openedx/features/enterprise_support/tests/test_signals.py

@@ -10,7 +10,8 @@ from django.test.utils import override_settings
 from django.utils.timezone import now
 from edx_django_utils.cache import TieredCache
 from opaque_keys.edx.keys import CourseKey
-from slumber.exceptions import HttpClientError, HttpServerError
+# from slumber.exceptions import HttpClientError, HttpServerError
+from requests.exceptions import HTTPError
 from testfixtures import LogCapture
 
 from common.djangoapps.course_modes.tests.factories import CourseModeFactory
@@ -152,27 +153,30 @@ class EnterpriseSupportSignals(SharedModuleStoreTestCase):
         api_called,
         mock_is_order_voucher_refundable
     ):
-        """Test refund_order_voucher signal"""
+        """
+        Test refund_order_voucher signal
+        """
         mock_is_order_voucher_refundable.return_value = order_voucher_refundable
         enrollment = self._create_enrollment_to_refund(no_of_days_placed, enterprise_enrollment_exists)
-        with patch('openedx.features.enterprise_support.signals.ecommerce_api_client') as mock_ecommerce_api_client:
+        with patch('openedx.features.enterprise_support.signals.get_ecommerce_api_client') as mock_ecommerce_api_client:
             enrollment.update_enrollment(is_active=False, skip_refund=skip_refund)
             assert mock_ecommerce_api_client.called == api_called
 
     @patch('common.djangoapps.student.models.CourseEnrollment.is_order_voucher_refundable')
     @ddt.data(
-        (HttpClientError, 'INFO'),
-        (HttpServerError, 'ERROR'),
+        (HTTPError, 'INFO'),
         (Exception, 'ERROR'),
     )
     @ddt.unpack
     def test_refund_order_voucher_with_client_errors(self, mock_error, log_level, mock_is_order_voucher_refundable):
-        """Test refund_order_voucher signal client_error"""
+        """
+        Test refund_order_voucher signal client_error.
+        """
         mock_is_order_voucher_refundable.return_value = True
         enrollment = self._create_enrollment_to_refund()
-        with patch('openedx.features.enterprise_support.signals.ecommerce_api_client') as mock_ecommerce_api_client:
+        with patch('openedx.features.enterprise_support.signals.get_ecommerce_api_client') as mock_ecommerce_api_client:
             client_instance = mock_ecommerce_api_client.return_value
-            client_instance.enterprise.coupons.create_refunded_voucher.post.side_effect = mock_error()
+            client_instance.post.side_effect = mock_error()
             with LogCapture(LOGGER_NAME) as logger:
                 enrollment.update_enrollment(is_active=False)
                 assert mock_ecommerce_api_client.called is True