Use a JSONField for BasePayment.extra_data

Author: Hugo Osvaldo Barrera

CHANGELOG.rst

@@ -7,6 +7,11 @@ releases, in reverse chronological order.
 v3.0.0
 ------
 - Dropped support for Django 2.2.
+- ``BasePayment.extra_data`` is now a JSONField and django will handle the
+  serialisation. Due to this, usage of the ``BasePayment.attrs`` proxy has been
+  deprecated. A migration needs to be generated to update this column in place.
+  Application code needs to be updated from ``payment.extra_data.field`` to
+  ``payment.extra_data["field"]``.
 
 v2.0.1
 ------

docs/backends.rst

@@ -108,7 +108,7 @@ about the payment or the order, such as an order number, additional customer
 information, or a special comment or request from the customer. This can be
 accomplished by passing your data to the :class:`Payment` instance::
 
-      >>> payment.attrs.merchant_defined_data = {'01': 'foo', '02': 'bar'}
+      >>> payment.extra_data["merchant_defined_data"] = {'01': 'foo', '02': 'bar'}
 
 Fingerprinting::
 

payments/core.py

@@ -1,4 +1,5 @@
 import re
+from importlib import import_module
 from typing import TYPE_CHECKING
 from typing import Dict
 from typing import Optional
@@ -155,7 +156,7 @@ def _default_provider_factory(variant: str, payment: Optional["BasePayment"] = N
         raise ValueError("Payment variant does not exist: {}".format(variant))
     if variant not in PROVIDER_CACHE:
         module_path, class_name = handler.rsplit(".", 1)
-        module = __import__(str(module_path), globals(), locals(), [str(class_name)])
+        module = import_module(module_path)
         class_ = getattr(module, class_name)
         PROVIDER_CACHE[variant] = class_(**config)
     return PROVIDER_CACHE[variant]

payments/cybersource/__init__.py

@@ -166,11 +166,11 @@ def charge(self, payment, data):
         else:
             params = self._prepare_preauth(payment, data)
         response = self._make_request(payment, params)
-        payment.attrs.capture = self._capture
+        payment.extra_data["capture"] = self._capture
         payment.transaction_id = response.requestID
         if response.reasonCode == AUTHENTICATE_REQUIRED:
             xid = response.payerAuthEnrollReply.xid
-            payment.attrs.xid = xid
+            payment.extra_data["xid"] = xid
             payment.change_status(
                 PaymentStatus.WAITING, message=_("3-D Secure verification in progress")
             )
@@ -279,8 +279,8 @@ def _get_params_for_new_payment(self, payment):
             "merchantReferenceCode": payment.id,
         }
         try:
-            fingerprint_id = payment.attrs.fingerprint_session_id
-        except AttributeError:
+            fingerprint_id = payment.extra_data["fingerprint_session_id"]
+        except KeyError:
             pass
         else:
             params["deviceFingerprintID"] = fingerprint_id
@@ -291,7 +291,7 @@ def _get_params_for_new_payment(self, payment):
 
     def _make_request(self, payment, params):
         response = self.client.service.runTransaction(**params)
-        payment.attrs.last_response = self._serialize_response(response)
+        payment.extra_data["last_response"] = self._serialize_response(response)
         return response
 
     def _prepare_payer_auth_validation_check(self, payment, card_data, pa_response):
@@ -300,7 +300,7 @@ def _prepare_payer_auth_validation_check(self, payment, card_data, pa_response):
         check_service.signedPARes = pa_response
         params = self._get_params_for_new_payment(payment)
         params["payerAuthValidateService"] = check_service
-        if payment.attrs.capture:
+        if payment.extra_data["capture"]:
             service = self.client.factory.create("data:CCCreditService")
             service._run = "true"
             params["ccCreditService"] = service
@@ -445,8 +445,8 @@ def _prepare_items(self, payment):
 
     def _prepare_merchant_defined_data(self, payment):
         try:
-            merchant_defined_data = payment.attrs.merchant_defined_data
-        except AttributeError:
+            merchant_defined_data = payment.extra_data["merchant_defined_data"]
+        except KeyError:
             return
         else:
             data = self.client.factory.create("data:MerchantDefinedData")
@@ -476,7 +476,7 @@ def _serialize_response(self, response):
 
     def process_data(self, payment, request):
         xid = request.POST.get("MD")
-        if xid != payment.attrs.xid:
+        if xid != payment.extra_data["xid"]:
             return redirect(payment.get_failure_url())
         if payment.status in [PaymentStatus.CONFIRMED, PaymentStatus.PREAUTH]:
             return redirect(payment.get_success_url())

payments/cybersource/forms.py

@@ -39,7 +39,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         if self.provider.org_id:
             try:
-                fingerprint_id = self.payment.attrs.fingerprint_session_id
+                fingerprint_id = self.payment.extra_data["fingerprint_session_id"]
             except KeyError:
                 fingerprint_id = str(uuid4())
             self.fields["fingerprint"] = FingerprintInput(
@@ -55,7 +55,7 @@ def clean(self):
         if not self.errors:
             if self.provider.org_id:
                 fingerprint = cleaned_data["fingerprint"]
-                self.payment.attrs.fingerprint_session_id = fingerprint
+                self.payment.extra_data["fingerprint_session_id"] = fingerprint
             if not self.payment.transaction_id:
                 try:
                     self.provider.charge(self.payment, cleaned_data)

payments/cybersource/test_cybersource.py

@@ -1,5 +1,4 @@
 from decimal import Decimal
-from typing import Dict
 from unittest import TestCase
 from unittest.mock import MagicMock
 from unittest.mock import Mock
@@ -38,10 +37,10 @@ class Payment(Mock):
     transaction_id = None
     captured_amount = 0
     message = ""
-
-    class attrs:
-        fingerprint_session_id = "fake"
-        merchant_defined_data: Dict[str, str] = {}
+    extra_data = {
+        "fingerprint_session_id": "fake",
+        "merchant_defined_data": {},
+    }
 
     def get_process_url(self):
         return "http://example.com"
@@ -152,7 +151,7 @@ def test_provider_redirects_on_success_captured_payment(
     ):
         transaction_id = 1234
         xid = "abc"
-        self.payment.attrs.xid = xid
+        self.payment.extra_data["xid"] = xid
 
         response = MagicMock()
         response.requestID = transaction_id
@@ -187,7 +186,7 @@ def test_provider_redirects_on_success_preauth_payment(
         )
         transaction_id = 1234
         xid = "abc"
-        self.payment.attrs.xid = xid
+        self.payment.extra_data["xid"] = xid
 
         response = MagicMock()
         response.requestID = transaction_id
@@ -217,7 +216,7 @@ def test_provider_redirects_on_success_preauth_payment(
     def test_provider_redirects_on_failure(self, mocked_request, mocked_redirect):
         transaction_id = 1234
         xid = "abc"
-        self.payment.attrs.xid = xid
+        self.payment.extra_data["xid"] = xid
 
         response = MagicMock()
         response.requestID = transaction_id

payments/mercadopago/__init__.py

@@ -71,7 +71,7 @@ def create_preference(self, payment: BasePayment):
         if payment.transaction_id:
             raise ValueError("This payment already has a preference.")
 
-        payment.attrs.external_reference = uuid4().hex
+        payment.extra_data["external_reference"] = uuid4().hex
 
         payload = {
             "auto_return": "all",
@@ -86,7 +86,7 @@ def create_preference(self, payment: BasePayment):
                 }
                 for item in payment.get_purchased_items()
             ],
-            "external_reference": payment.attrs.external_reference,
+            "external_reference": payment.extra_data["external_reference"],
             "back_urls": {
                 "success": self.get_return_url(payment),
                 "pending": self.get_return_url(payment),
@@ -214,7 +214,7 @@ def poll_for_updates(self, payment: BasePayment):
         """
         data = self.client.payment().search(
             {
-                "external_reference": payment.attrs.external_reference,
+                "external_reference": payment.extra_data["external_reference"],
             }
         )
 

payments/mercadopago/test_mercadopago.py

@@ -25,6 +25,7 @@ class Payment(Mock):
     captured_amount = 0
     transaction_id: Optional[str] = None
     billing_email = "[email protected]"
+    extra_data: dict = {}
 
     def change_status(self, status, message=""):
         self.status = status

payments/models.py

@@ -1,4 +1,4 @@
-import json
+import warnings
 from typing import Iterable
 from typing import Union
 from uuid import uuid4
@@ -14,29 +14,6 @@
 from .core import provider_factory
 
 
-class PaymentAttributeProxy:
-    def __init__(self, payment):
-        self._payment = payment
-        super().__init__()
-
-    def __getattr__(self, item):
-        data = json.loads(self._payment.extra_data or "{}")
-        try:
-            return data[item]
-        except KeyError as e:
-            raise AttributeError(*e.args)
-
-    def __setattr__(self, key, value):
-        if key == "_payment":
-            return super().__setattr__(key, value)
-        try:
-            data = json.loads(self._payment.extra_data)
-        except ValueError:
-            data = {}
-        data[key] = value
-        self._payment.extra_data = json.dumps(data)
-
-
 class BasePayment(models.Model):
     """
     Represents a single transaction. Each instance has one or more PaymentItem.
@@ -78,7 +55,7 @@ class BasePayment(models.Model):
     billing_email = models.EmailField(blank=True)
     billing_phone = PhoneNumberField(blank=True)
     customer_ip_address = models.GenericIPAddressField(blank=True, null=True)
-    extra_data = models.TextField(blank=True, default="")
+    extra_data = models.JSONField(blank=True, default=dict)
     message = models.TextField(blank=True, default="")
     token = models.CharField(max_length=36, blank=True, default="")
     captured_amount = models.DecimalField(max_digits=9, decimal_places=2, default="0.0")
@@ -225,14 +202,18 @@ def refund(self, amount=None):
 
     @property
     def attrs(self):
-        """A JSON-serialised wrapper around `extra_data`.
-
-        This property exposes a a dict or list which is serialised into the `extra_data`
-        text field. Usage of this wrapper is preferred over accessing the underlying
-        field directly.
-
-        You may think of this as a `JSONField` which is saved to the `extra_data`
-        column.
-        """
-        # TODO: Deprecate in favour of JSONField when we drop support for django 2.2.
-        return PaymentAttributeProxy(self)
+        warnings.warn(
+            "Using BasePayment.attrs is deprecated. Use BasePayment.extra_data instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.extra_data
+
+    @attrs.setter
+    def attrs(self, value):
+        warnings.warn(
+            "Using BasePayment.attrs is deprecated. Use BasePayment.extra_data instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        self.extra_data = value

payments/paypal/__init__.py

@@ -77,34 +77,34 @@ def __init__(
         super().__init__(capture=capture)
 
     def set_response_data(self, payment, response, is_auth=False):
-        extra_data = json.loads(payment.extra_data or "{}")
+        extra_data = payment.extra_data or {}
         if is_auth:
             extra_data["auth_response"] = response
         else:
             extra_data["response"] = response
             if "links" in response:
                 extra_data["links"] = {link["rel"]: link for link in response["links"]}
-        payment.extra_data = json.dumps(extra_data)
+        payment.extra_data = extra_data
         payment.save()
 
     def set_response_links(self, payment, response):
         transaction = response["transactions"][0]
         related_resources = transaction["related_resources"][0]
         resource_key = "sale" if self._capture else "authorization"
         links = related_resources[resource_key]["links"]
-        extra_data = json.loads(payment.extra_data or "{}")
+        extra_data = payment.extra_data or {}
         extra_data["links"] = {link["rel"]: link for link in links}
-        payment.extra_data = json.dumps(extra_data)
+        payment.extra_data = extra_data
         payment.save()
 
     def set_error_data(self, payment, error):
-        extra_data = json.loads(payment.extra_data or "{}")
+        extra_data = payment.extra_data or {}
         extra_data["error"] = error
-        payment.extra_data = json.dumps(extra_data)
+        payment.extra_data = extra_data
         payment.save()
 
     def _get_links(self, payment):
-        extra_data = json.loads(payment.extra_data or "{}")
+        extra_data = payment.extra_data or {}
         links = extra_data.get("links", {})
         return links
 
@@ -141,7 +141,7 @@ def post(self, payment, *args, **kwargs):
         return data
 
     def get_last_response(self, payment, is_auth=False):
-        extra_data = json.loads(payment.extra_data or "{}")
+        extra_data = payment.extra_data or {}
         if is_auth:
             return extra_data.get("auth_response", {})
         return extra_data.get("response", {})
@@ -249,7 +249,7 @@ def process_data(self, payment, request):
         except PaymentError:
             return redirect(failure_url)
         self.set_response_links(payment, executed_payment)
-        payment.attrs.payer_info = executed_payment["payer"]["payer_info"]
+        payment.extra_data["payer_info"] = executed_payment["payer"]["payer_info"]
         if self._capture:
             payment.captured_amount = payment.total
             payment.change_status(PaymentStatus.CONFIRMED)