Skip to content

Commit

Permalink
feat: restructured caching. (#114)
Browse files Browse the repository at this point in the history
* feat: restructured caching.

* Created Utils for setting cache and getting cache in our core app.
* Added CachedPaymentSerializer in all our caches to make sure caches are consistent.
* Added `new_payment_number` in our caching on both places(webhook and get payment view.)
* Fixed "Mark Payment Ready for Processing" view. We were not providing `payment_state`. So rename `UpdateTitanPayment` to `MarkTitanPaymentPending` and payment state there.
* There are some other minor fixes.

* feat: Restructured caching from functions to classes.
  • Loading branch information
HammadAhmadWaqas committed Aug 28, 2023
1 parent e166db0 commit f4cee0e
Show file tree
Hide file tree
Showing 14 changed files with 352 additions and 147 deletions.
88 changes: 71 additions & 17 deletions commerce_coordinator/apps/core/cache.py
Original file line number Diff line number Diff line change
@@ -1,32 +1,86 @@
"""
Core Cache utils.
"""
from enum import Enum

from edx_django_utils.cache import get_cache_key
from django.conf import settings
from edx_django_utils.cache import TieredCache, get_cache_key

from commerce_coordinator.apps.titan.serializers import CachedPaymentSerializer

class CachePaymentStates(Enum):
PROCESSING = 'PROCESSING'
PAID = 'PAID'


def get_payment_state_cache_key(payment_number, payment_state):
class CacheBase:
"""
Wrapper method on edx_django_utils to payment_state_cache.
Base class for versioned cache. Extend this class to create custom cache.
"""
return get_cache_key(cache_name='payment', identifier=payment_number, version=payment_state)
serializer_class = None
cache_name = ''
identifier_key = ''
versions = ()

def __init__(self):
assert self.serializer_class, 'serializer_class override missing.'
assert self.cache_name, 'cache_name override missing.'
assert self.identifier_key, 'identifier_key override missing.'
assert self.versions, 'versions override missing.'

def get_paid_payment_state_cache_key(payment_number):
"""
Wrapper method on edx_django_utils to paid_payment_state_cache.
"""
return get_payment_state_cache_key(payment_number=payment_number, payment_state=CachePaymentStates.PAID.value)
def get_cache_key(self, identifier, version):
"""
Wrapper method on edx_django_utils to get cache_key.
"""
assert version in self.versions, f'Invalid cache key version: {version}. Supported versions: {self.versions}.'
return get_cache_key(cache_name=self.cache_name, version=version, identifier=identifier)

def set_cache(self, data, version):
"""
Utility for caching data for given cache version
"""
serializer = self.serializer_class(data=data) # pylint: disable=not-callable
serializer.is_valid(raise_exception=True)
data = serializer.validated_data
cache_key = self.get_cache_key(data[self.identifier_key], version)
TieredCache.set_all_tiers(cache_key, data, settings.DEFAULT_TIMEOUT)

def get_cache(self, identifier, version):
"""
Utility for getting cache data for given cache version
"""
cache_key = self.get_cache_key(identifier, version)
cached_response = TieredCache.get_cached_response(cache_key)
if cached_response.is_found:
serializer = self.serializer_class(data=cached_response.value) # pylint: disable=not-callable
serializer.is_valid(raise_exception=True)
return serializer.validated_data
return None

def get_processing_payment_state_cache_key(payment_number):

class PaymentCache(CacheBase):
"""
Wrapper method on edx_django_utils to processing_payment_state_cache.
Cache class for payment data.
"""
return get_payment_state_cache_key(payment_number=payment_number, payment_state=CachePaymentStates.PROCESSING.value)
PROCESSING = 'PROCESSING'
PAID = 'PAID'
versions = (PROCESSING, PAID)
serializer_class = CachedPaymentSerializer
identifier_key = 'payment_number'
cache_name = 'payment'

def set_paid_cache_payment(self, payment):
self.set_cache(payment, self.PAID)

def set_processing_cache_payment(self, payment):
self.set_cache(payment, self.PROCESSING)

def get_paid_cache_payment(self, payment_number):
return self.get_cache(payment_number, self.PAID)

def get_processing_cache_payment(self, payment_number):
return self.get_cache(payment_number, self.PROCESSING)

def get_cache_payment(self, payment_number):
"""
Utility to get cached payment.
This utility is for getting cached payment. First tries to fetch it from PAID cache and
then from PROCESSING Cache. Returns None otherwise.
"""
return self.get_paid_cache_payment(payment_number) or self.get_processing_cache_payment(payment_number)
119 changes: 119 additions & 0 deletions commerce_coordinator/apps/core/tests/test_cache.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
""" Stripe Utils Tests"""
import unittest
from uuid import UUID

import ddt
from django.conf import settings
from edx_django_utils.cache import TieredCache
from rest_framework.exceptions import ErrorDetail, ValidationError

from commerce_coordinator.apps.core.cache import CacheBase, PaymentCache
from commerce_coordinator.apps.core.tests.utils import name_test


@ddt.ddt
class TestPaymentCache(unittest.TestCase):
"""
Test class for PaymentCache.
"""
def setUp(self) -> None:
TieredCache.dangerous_clear_all_tiers()
self.payment_number = '12345'
self.payment = {
'payment_number': self.payment_number,
'order_uuid': UUID('123e4567-e89b-12d3-a456-426614174000'),
'key_id': 'test-code',
'new_payment_number': '67890',
'state': 'a-paid-state',
}
self.payment_cache = PaymentCache()

def test_error_on_using_base_cache_class(self):
"""
Test error on using CacheBase directly.
"""
with self.assertRaises(AssertionError) as ex:
CacheBase()
self.assertEqual(str(ex.exception), 'serializer_class override missing.')

def test_get_cache_invalid_version(self):
with self.assertRaises(AssertionError) as ex:
self.payment_cache.get_cache_key(self.payment_number, 'invalid-state')
self.assertEqual(
str(ex.exception),
"Invalid cache key version: invalid-state. Supported versions: ('PROCESSING', 'PAID')."
)

@ddt.data(
name_test("test order_uuid in required", (
{}, 'order_uuid',
{'order_uuid': [ErrorDetail(string='This field is required.', code='required')]},
)),
name_test("test order_uuid format", (
{'order_uuid': 'invalid-uuid'}, None,
{'order_uuid': [ErrorDetail(string='Must be a valid UUID.', code='invalid')]},
)),
name_test("test key_id in required", (
{}, 'key_id',
{'key_id': [ErrorDetail(string='This field is required.', code='required')]},
)),
name_test("test payment_number in required", (
{}, 'payment_number',
{'payment_number': [ErrorDetail(string='This field is required.', code='required')]},
)),
name_test("test state in required", (
{}, 'state',
{'state': [ErrorDetail(string='This field is required.', code='required')]},
)),
)
@ddt.unpack
def test_cache_payment_error(self, update_params, skip_param, expected_error):
"""
Test proper error on invalid data.
"""
payment = {**self.payment}
payment.update(update_params)
if skip_param:
del payment[skip_param]

def _assert_error(payment_state):
with self.assertRaises(ValidationError) as ex:
self.payment_cache.set_cache(payment, payment_state)
self.assertEqual(ex.exception.detail, expected_error)

cache_key = self.payment_cache.get_cache_key(self.payment_number, payment_state)
TieredCache.set_all_tiers(cache_key, payment, settings.DEFAULT_TIMEOUT)
with self.assertRaises(ValidationError) as ex:
self.payment_cache.get_cache(self.payment_number, payment_state)
self.assertEqual(ex.exception.detail, expected_error)

_assert_error(self.payment_cache.PAID)
_assert_error(self.payment_cache.PROCESSING)

def test_get_cache_payment(self):
"""
Test PaymentCache setters and getters.
"""
paid_payment = {**self.payment, 'state': 'a-paid-state'}
processing_payment = {**self.payment, 'state': 'a-processing-state'}

response = self.payment_cache.get_processing_cache_payment(self.payment_number)
self.assertIsNone(response)
self.payment_cache.set_processing_cache_payment(processing_payment)
response = self.payment_cache.get_processing_cache_payment(self.payment_number)
self.assertEqual(dict(response), processing_payment)

# get_cache_payment should return PROCESSING payment. as we don't have PAID payment in cache.
response = self.payment_cache.get_cache_payment(self.payment_number)
self.assertEqual(dict(response), processing_payment)

response = self.payment_cache.get_paid_cache_payment(self.payment_number)
self.assertIsNone(response)
self.payment_cache.set_paid_cache_payment(paid_payment)
response = self.payment_cache.get_paid_cache_payment(self.payment_number)
self.assertEqual(dict(response), paid_payment)

# get_cache_payment should return PAID payment instead if PROCESSING payment.
response = self.payment_cache.get_cache_payment(self.payment_number)
self.assertEqual(dict(response), paid_payment)
self.assertNotEqual(dict(response), processing_payment)
9 changes: 9 additions & 0 deletions commerce_coordinator/apps/frontend_app_payment/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
"""Custom exceptions."""

from rest_framework.exceptions import APIException


class UnhandledPaymentStateAPIError(APIException):
status_code = 422
default_detail = 'Get Payment received a Payment State that is not handled.'
default_code = 'unhandled_payment_state'
46 changes: 3 additions & 43 deletions commerce_coordinator/apps/frontend_app_payment/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@
"""
import logging

from django.conf import settings
from edx_django_utils.cache import TieredCache
from openedx_filters.tooling import OpenEdxPublicFilter

from commerce_coordinator.apps.core.cache import CachePaymentStates, get_payment_state_cache_key
from commerce_coordinator.apps.core.constants import PaymentState
from commerce_coordinator.apps.frontend_app_payment.serializers import DraftPaymentCreateViewOutputSerializer

logger = logging.getLogger(__name__)
Expand All @@ -22,39 +18,13 @@ class PaymentRequested(OpenEdxPublicFilter):
filter_type = "org.edx.coordinator.frontend_app_payment.payment.get.requested.v1"

@classmethod
def run_filter(cls, params):
def run_filter(cls, **kwargs):
"""
Call the PipelineStep(s) defined for this filter, to gather orders and return together
Arguments:
params (dict): Arguments passed through from the original get payment url querystring
kwargs (dict): Arguments passed through from the original get payment url querystring
"""
payment_number = params['payment_number']

# check if we have PAID cache stored
payment_state_paid_cache_key = get_payment_state_cache_key(
payment_number, CachePaymentStates.PAID.value
)
cached_response = TieredCache.get_cached_response(payment_state_paid_cache_key)
if cached_response.is_found:
return cached_response.value

# PAID cache is not found, Try PROCESSING cache to see if payment is in processing or failed
payment_state_processing_cache_key = get_payment_state_cache_key(
payment_number, CachePaymentStates.PROCESSING.value
)
cached_response = TieredCache.get_cached_response(payment_state_processing_cache_key)
if cached_response.is_found:
return cached_response.value

# PROCESSING cache not found as well. We have to call Titan to fetch Payment information
payment = super().run_pipeline(**params)['payment_data']
# Set cache for future use
payment_state = payment["state"]
if payment_state == PaymentState.COMPLETED.value:
TieredCache.set_all_tiers(payment_state_paid_cache_key, payment, settings.DEFAULT_TIMEOUT)
elif payment_state in [PaymentState.PENDING.value, PaymentState.FAILED.value]:
TieredCache.set_all_tiers(payment_state_processing_cache_key, payment, settings.DEFAULT_TIMEOUT)

payment = super().run_pipeline(**kwargs)['payment_data']
return payment


Expand Down Expand Up @@ -126,14 +96,4 @@ def run_filter(cls, **kwargs):
pipeline_output = super().run_pipeline(
**kwargs,
)
if 'payment_data' in pipeline_output:
payment_data = pipeline_output['payment_data']
payment_state_processing_cache_key = get_payment_state_cache_key(
payment_data['payment_number'],
CachePaymentStates.PROCESSING.value
)
TieredCache.set_all_tiers(payment_state_processing_cache_key, payment_data, settings.DEFAULT_TIMEOUT)
else:
logger.info('frontend_app_payment.PaymentProcessingRequested pipeline did not return any payment_data. '
'Unable to cache Payment data in Processing state')
return pipeline_output
5 changes: 3 additions & 2 deletions commerce_coordinator/apps/frontend_app_payment/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

class GetPaymentInputSerializer(CoordinatorSerializer):
"""
Serializer for OrderFulfillView input validation.
Serializer for PaymentGetView input validation.
"""
payment_number = serializers.CharField(allow_null=False)
order_uuid = serializers.UUIDField(allow_null=False)
Expand All @@ -16,9 +16,10 @@ class GetPaymentInputSerializer(CoordinatorSerializer):

class GetPaymentOutputSerializer(CoordinatorSerializer):
"""
Serializer for OrderFulfillView input validation.
Serializer for PaymentGetView output validation.
"""
state = serializers.CharField(allow_null=False)
new_payment_number = serializers.CharField(required=False)


class DraftPaymentCreateViewInputSerializer(CoordinatorSerializer):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from django.test import override_settings
from edx_django_utils.cache import TieredCache

from commerce_coordinator.apps.core.cache import CachePaymentStates, get_payment_state_cache_key
from commerce_coordinator.apps.core.constants import OrderPaymentState, PaymentState
from commerce_coordinator.apps.frontend_app_payment.filters import (
DraftPaymentRequested,
Expand Down Expand Up @@ -120,18 +119,18 @@ class TestPaymentProcessingRequestedFilter(TestCase):
'commerce_coordinator.apps.titan.pipeline.ValidatePaymentReadyForProcessing',
'commerce_coordinator.apps.titan.pipeline.UpdateBillingAddress',
'commerce_coordinator.apps.stripe.pipeline.ConfirmPayment',
'commerce_coordinator.apps.titan.pipeline.UpdateTitanPayment',
'commerce_coordinator.apps.titan.pipeline.MarkTitanPaymentPending',
]
},
},
)
@patch('commerce_coordinator.apps.titan.pipeline.GetTitanPayment.run_filter')
@patch('commerce_coordinator.apps.titan.pipeline.UpdateBillingAddress.run_filter')
@patch('commerce_coordinator.apps.stripe.pipeline.ConfirmPayment.run_filter')
@patch('commerce_coordinator.apps.titan.pipeline.UpdateTitanPayment.run_filter')
@patch('commerce_coordinator.apps.titan.pipeline.MarkTitanPaymentPending.run_filter')
def test_filter_when_payment_exist_in_titan(
self,
mock_update_titan_payment_step,
mock_mark_titan_payment_pending_step,
mock_confirm_payment_step,
mock_update_billing_address_step,
mock_get_titan_payment_step,
Expand Down Expand Up @@ -173,7 +172,7 @@ def test_filter_when_payment_exist_in_titan(
mock_get_titan_payment_step.return_value = mock_payment
mock_update_billing_address_step.return_value = mock_billing_details_data
mock_confirm_payment_step.return_value = mock_pending_payment
mock_update_titan_payment_step.return_value = mock_pending_payment
mock_mark_titan_payment_pending_step.return_value = mock_pending_payment

filter_params = {
'order_uuid': ORDER_UUID,
Expand All @@ -184,23 +183,18 @@ def test_filter_when_payment_exist_in_titan(
payment_details = PaymentProcessingRequested.run_filter(**filter_params)
expected_payment = {**mock_pending_payment, **mock_billing_details_data, **filter_params}
self.assertEqual(expected_payment, payment_details)
payment_state_processing_cache_key = get_payment_state_cache_key(
filter_params['payment_number'], CachePaymentStates.PROCESSING.value
)
cached_response = TieredCache.get_cached_response(payment_state_processing_cache_key)
self.assertTrue(cached_response.is_found)

@override_settings(
OPEN_EDX_FILTERS_CONFIG={
"org.edx.coordinator.frontend_app_payment.payment.processing.requested.v1": {
"fail_silently": False,
"pipeline": [
'commerce_coordinator.apps.titan.pipeline.UpdateTitanPayment',
'commerce_coordinator.apps.titan.pipeline.MarkTitanPaymentPending',
]
},
},
)
@patch('commerce_coordinator.apps.titan.pipeline.UpdateTitanPayment.run_filter')
@patch('commerce_coordinator.apps.titan.pipeline.MarkTitanPaymentPending.run_filter')
def test_pipeline_no_result(self, mock_pipeline):
"""
Test pipeline does not return payment
Expand Down Expand Up @@ -261,7 +255,7 @@ def test_filter(self, payment_state, mock_get_payment_step):
filter_params = {
'payment_number': mock_payment_number,
}
output = PaymentRequested.run_filter(filter_params)
output = PaymentRequested.run_filter(**filter_params)

# Check output matches expected:
self.assertEqual(expected_output, output)
Loading

0 comments on commit f4cee0e

Please sign in to comment.