Pass request into config class

The idea is that we want to change certain configuration values based on
the request. To get this working, we need to change the interface for
the config class to pass the request to most of it's methods.

To get the request object, we need to also alter the facade's and
scaffold's interface to more often include the request.

For this commit, I had a look at how data gets passed from e.g. Oscaro
to the scaffold. Unfortunately I discovered an anti-pattern, where a lot
of required data is passed in via an undocumented order_data dictionary.

That needs to be cleaned up separately.

adyen/config.py

@@ -19,16 +19,16 @@ class AbstractAdyenConfig:
     The base implementation for a config class.
     """
 
-    def get_identifier(self):
+    def get_identifier(self, request):
         raise NotImplementedError
 
-    def get_action_url(self):
+    def get_action_url(self, request):
         raise NotImplementedError
 
-    def get_skin_code(self):
+    def get_skin_code(self, request):
         raise NotImplementedError
 
-    def get_skin_secret(self):
+    def get_skin_secret(self, request):
         raise NotImplementedError
 
     def get_ip_address_header(self):

adyen/facade.py

@@ -11,24 +11,22 @@
 
 logger = logging.getLogger('adyen')
 
+def get_gateway(request):
+    return Gateway({
+        Constants.IDENTIFIER: get_config().get_identifier(request),
+        Constants.SECRET_KEY: get_config().get_skin_secret(request),
+        Constants.ACTION_URL: get_config().get_action_url(request),
+    })
 
-class Facade():
 
-    def __init__(self, **kwargs):
-        init_params = {
-            Constants.IDENTIFIER: get_config().get_identifier(),
-            Constants.SECRET_KEY: get_config().get_skin_secret(),
-            Constants.ACTION_URL: get_config().get_action_url(),
-        }
-        # Initialize the gateway.
-        self.gateway = Gateway(init_params)
+class Facade:
 
-    def build_payment_form_fields(self, params):
+    def build_payment_form_fields(self, request, params):
         """
         Return a dict containing the name and value of all the hidden fields
         necessary to build the form that will be POSTed to Adyen.
         """
-        return self.gateway.build_payment_form_fields(params)
+        return get_gateway(request).build_payment_form_fields(params)
 
     @classmethod
     def _is_valid_ip_address(cls, s):
@@ -143,11 +141,7 @@ def handle_payment_feedback(self, request, record_audit_trail):
         Validate, process, optionally record audit trail and provide feedback
         about the current payment response.
         """
-        success, output_data = False, {}
-
         # We must first find out whether this is a redirection or a notification.
-        client = self.gateway
-        params = response_class = None
 
         if request.method == 'GET':
             params = request.GET
@@ -159,7 +153,8 @@ def handle_payment_feedback(self, request, record_audit_trail):
             raise RuntimeError("Only GET and POST requests are supported.")
 
         # Then we can instantiate the appropriate class from the gateway.
-        response = response_class(client, params)
+        gateway = get_gateway(request)
+        response = response_class(gateway, params)
 
         # Note that this may raise an exception if the response is invalid.
         # For example: MissingFieldException, UnexpectedFieldException, ...
@@ -206,7 +201,7 @@ def assess_notification_relevance(self, request):
         # - On the other hand we have the `live` POST parameter, which lets
         # us know which Adyen platform fired this request.
         current_platform = (Constants.LIVE
-                            if Constants.LIVE in get_config().get_action_url()
+                            if Constants.LIVE in get_config().get_action_url(request)
                             else Constants.TEST)
 
         origin_platform = (Constants.LIVE

adyen/scaffold.py

@@ -9,7 +9,7 @@
 from .config import get_config
 
 
-class Scaffold():
+class Scaffold:
 
     # These are the constants that all scaffolds are expected to return
     # to a multi-psp application. They might look like those actually returned
@@ -26,28 +26,31 @@ class Scaffold():
     }
 
     def __init__(self, order_data=None):
-        self.facade = Facade()
+        # This is an anti-pattern. We require passing in additional data
+        # when the Scaffold is initialized (instead of when calling the methods that
+        # might require that data). get_form_fields_list especially relies on this.
+        # This makes the code very Oscaro-specific.
         try:
             for name, value in order_data.items():
                 setattr(self, name, value)
         except AttributeError:
             pass
 
-    def get_form_action(self):
+    def get_form_action(self, request):
         """ Return the URL where the payment form should be submitted. """
-        return get_config().get_action_url()
+        return get_config().get_action_url(request)
 
-    def get_form_fields(self):
+    def get_form_fields(self, request):
         """ Return the payment form fields, rendered into HTML. """
 
-        fields_list = self.get_form_fields_list()
+        fields_list = self.get_form_fields_list(request)
         return ''.join([
             '<input type="%s" name="%s" value="%s">\n' % (
                 f.get('type'), f.get('name'), bleach.clean(f.get('value'))
             ) for f in fields_list
         ])
 
-    def get_form_fields_list(self):
+    def get_form_fields_list(self, request):
         """
         Return the payment form fields as a list of dicts.
         """
@@ -60,21 +63,21 @@ def get_form_fields_list(self):
         # Build common field specs
         try:
             field_specs = {
-                Constants.MERCHANT_ACCOUNT: get_config().get_identifier(),
+                Constants.MERCHANT_ACCOUNT: get_config().get_identifier(request),
+                Constants.SKIN_CODE: get_config().get_skin_code(request),
+                Constants.SESSION_VALIDITY: session_validity.strftime(session_validity_format),
+                Constants.SHIP_BEFORE_DATE: ship_before_date.strftime(ship_before_date_format),
+
+                # The following values must be passed in via order_data in __init__().
                 Constants.MERCHANT_REFERENCE: str(self.order_number),
                 Constants.SHOPPER_REFERENCE: self.client_id,
                 Constants.SHOPPER_EMAIL: self.client_email,
                 Constants.CURRENCY_CODE: self.currency_code,
                 Constants.PAYMENT_AMOUNT: self.amount,
-                Constants.SKIN_CODE: get_config().get_skin_code(),
-                Constants.SESSION_VALIDITY: session_validity.strftime(session_validity_format),
-                Constants.SHIP_BEFORE_DATE: ship_before_date.strftime(ship_before_date_format),
                 Constants.SHOPPER_LOCALE: self.shopper_locale,
                 Constants.COUNTRY_CODE: self.country_code,
-
-                # Adyen does not provide the payment amount in the
-                # return URL, so we store it in this field to
-                # avoid a database query to get it back then.
+                # Adyen does not provide the payment amount in the return URL, so we store it in
+                # this field to avoid a database query to get it back then.
                 Constants.MERCHANT_RETURN_DATA: self.amount,
 
             }
@@ -88,7 +91,7 @@ def get_form_fields_list(self):
             return_url = return_url.replace('PAYMENT_PROVIDER_CODE', Constants.ADYEN)
             field_specs[Constants.MERCHANT_RETURN_URL] = return_url
 
-        return self.facade.build_payment_form_fields(field_specs)
+        return Facade().build_payment_form_fields(request, field_specs)
 
     def _normalize_feedback(self, feedback):
         """
@@ -101,16 +104,14 @@ def _normalize_feedback(self, feedback):
 
     def handle_payment_feedback(self, request):
         return self._normalize_feedback(
-            self.facade.handle_payment_feedback(
-                request, record_audit_trail=True))
+            Facade().handle_payment_feedback(request, record_audit_trail=True))
 
     def check_payment_outcome(self, request):
         return self._normalize_feedback(
-            self.facade.handle_payment_feedback(
-                request, record_audit_trail=False))
+            Facade().handle_payment_feedback(request, record_audit_trail=False))
 
     def assess_notification_relevance(self, request):
-        return self.facade.assess_notification_relevance(request)
+        return Facade().assess_notification_relevance(request)
 
     def build_notification_response(self, request):
-        return self.facade.build_notification_response(request)
+        return Facade().build_notification_response(request)

adyen/settings_config.py

@@ -24,16 +24,16 @@ def __init__(self):
                 "You are using the FromSettingsConfig config class, but haven't set the "
                 "the following required settings: %s" % missing_settings)
 
-    def get_identifier(self):
+    def get_identifier(self, request):
         return settings.ADYEN_IDENTIFIER
 
-    def get_action_url(self):
+    def get_action_url(self, request):
         return settings.ADYEN_ACTION_URL
 
-    def get_skin_code(self):
+    def get_skin_code(self, request):
         return settings.ADYEN_SKIN_CODE
 
-    def get_skin_secret(self):
+    def get_skin_secret(self, request):
         return settings.ADYEN_SECRET_KEY
 
     def get_ip_address_header(self):

tests/test_adyen.py

@@ -100,6 +100,8 @@
     'skinCode': '4d72uQqA',
 }
 
+DUMMY_REQUEST = None
+
 
 @override_settings(
     ADYEN_IDENTIFIER=TEST_IDENTIFIER,
@@ -135,15 +137,15 @@ def test_form_action(self):
         """
         Test that the form action is properly fetched from the settings.
         """
-        action_url = self.scaffold.get_form_action()
+        action_url = self.scaffold.get_form_action(DUMMY_REQUEST)
         self.assertEqual(action_url, TEST_ACTION_URL)
 
     def test_form_fields_ok(self):
         """
         Test that the payment form fields are properly built.
         """
         with freeze_time(TEST_FROZEN_TIME):
-            form_fields = self.scaffold.get_form_fields()
+            form_fields = self.scaffold.get_form_fields(DUMMY_REQUEST)
             for field_spec in EXPECTED_FIELDS_LIST:
                 field = '<input type="%s" name="%s" value="%s">' % (
                     field_spec.get('type'),
@@ -157,7 +159,7 @@ def test_form_fields_list_ok(self):
         Test that the payment form fields list is properly built.
         """
         with freeze_time(TEST_FROZEN_TIME):
-            fields_list = self.scaffold.get_form_fields_list()
+            fields_list = self.scaffold.get_form_fields_list(DUMMY_REQUEST)
             self.assertEqual(len(fields_list), len(EXPECTED_FIELDS_LIST))
             for field in fields_list:
                 self.assertIn(field, EXPECTED_FIELDS_LIST)
@@ -170,7 +172,7 @@ def test_form_fields_list_with_missing_mandatory_field(self):
         del self.order_data['amount']
         scaffold = Scaffold(self.order_data)
         with self.assertRaises(MissingFieldException):
-            scaffold.get_form_fields_list()
+            scaffold.get_form_fields_list(DUMMY_REQUEST)
 
 
 class TestAdyenPaymentResponse(AdyenTestCase):

tests/test_config.py

@@ -4,7 +4,7 @@
 from django.test import TestCase
 from django.test.utils import override_settings
 
-from adyen.config import get_config
+from adyen.config import get_config, AbstractAdyenConfig
 
 
 @override_settings(
@@ -23,7 +23,7 @@ def test_is_default(self):
         assert 'FromSettingsConfig' in get_config().__class__.__name__
 
     def test_value_passing_works(self):
-        assert get_config().get_action_url() == 'foo'
+        assert get_config().get_action_url(None) == 'foo'
 
     # https://docs.djangoproject.com/en/1.8/topics/testing/tools/#django.test.modify_settings
     # Override settings is needed to let us delete settings on a per-test basis.
@@ -35,9 +35,9 @@ def test_complains_when_not_fully_configured(self):
             get_config()
 
 
-class DummyConfigClass:
+class DummyConfigClass(AbstractAdyenConfig):
 
-    def get_action_url(self):
+    def get_action_url(self, request):
         return 'foo'
 
 
@@ -56,6 +56,6 @@ def test_settings_ignored(self):
         """
         Check that we indeed ignore Django settings (apart from the config class).
         """
-        assert get_config().get_action_url() == 'foo'
+        assert get_config().get_action_url(None) == 'foo'