From e6372c3cc6fef67c43768c212f1fab675c53f34c Mon Sep 17 00:00:00 2001 From: Joren Van Onder Date: Mon, 26 Aug 2019 12:03:05 -0700 Subject: [PATCH] [IMP] pos_adyen: avoid concurrent updates Every time we check the status of a request (every 3 seconds) we also launch another request to check the status of the terminal. This way we can notify the user if the terminal is no longer reachable. Even though we call Adyen's async endpoint for this it can still take a while to complete (>2 seconds). Before this change the pos_payment_table would be locked for this entire duration. This means it's now likely that during this time Adyen calls us back to say the transaction is finished. It needs to write on the payment method which causes the concurrent update error. Odoo's automatic retry mechanism would usually handle this fine but it's not pretty solution. Instead let's convert the methods that do the async call to @api.model functions and pass in all the data they need. This solves the problem because now we won't need to lock pos_payment_method for the duration of the request. --- addons/pos_adyen/models/pos_payment_method.py | 44 ++++++++++++------- addons/pos_adyen/static/src/js/models.js | 2 +- .../pos_adyen/static/src/js/payment_adyen.js | 8 +++- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/addons/pos_adyen/models/pos_payment_method.py b/addons/pos_adyen/models/pos_payment_method.py index 3461efbc91a..c3489797173 100644 --- a/addons/pos_adyen/models/pos_payment_method.py +++ b/addons/pos_adyen/models/pos_payment_method.py @@ -36,7 +36,7 @@ class PosPaymentMethod(models.Model): whitelisted_fields = set(('adyen_latest_response', 'adyen_latest_diagnosis')) return super(PosPaymentMethod, self)._is_write_forbidden(fields - whitelisted_fields) - def _adyen_diagnosis_request_data(self, pos_config_name): + def _adyen_diagnosis_request_data(self, pos_config_name, terminal_identifier): service_id = ''.join(random.choices(string.ascii_letters + string.digits, k=10)) return { "SaleToPOIRequest": { @@ -47,7 +47,7 @@ class PosPaymentMethod(models.Model): "MessageType": "Request", "ServiceID": service_id, "SaleID": pos_config_name, - "POIID": self.adyen_terminal_identifier + "POIID": terminal_identifier, }, "DiagnosisRequest": { "HostDiagnosisFlag": False @@ -55,35 +55,49 @@ class PosPaymentMethod(models.Model): } } - def get_latest_adyen_status(self, pos_config_name): - self.ensure_one() - latest_response = self.sudo().adyen_latest_response - latest_response = json.loads(latest_response) if latest_response else False - self.sudo().adyen_latest_response = '' # avoid handling old responses multiple times + @api.model + def get_latest_adyen_status(self, payment_method_id, pos_config_name, terminal_identifier, test_mode, api_key): + '''See the description of proxy_adyen_request as to why this is an + @api.model function. + ''' # Poll the status of the terminal if there's no new # notification we received. This is done so we can quickly # notify the user if the terminal is no longer reachable due # to connectivity issues. - if not latest_response: - self.proxy_adyen_request(self._adyen_diagnosis_request_data(pos_config_name)) + self.proxy_adyen_request(self._adyen_diagnosis_request_data(pos_config_name, terminal_identifier), + test_mode, + api_key) + + payment_method = self.sudo().browse(payment_method_id) + latest_response = payment_method.adyen_latest_response + latest_response = json.loads(latest_response) if latest_response else False + payment_method.adyen_latest_response = '' # avoid handling old responses multiple times return { 'latest_response': latest_response, - 'last_received_diagnosis_id': self.adyen_latest_diagnosis, + 'last_received_diagnosis_id': payment_method.adyen_latest_diagnosis, } - def proxy_adyen_request(self, data): - ''' Necessary because Adyen's endpoints don't have CORS enabled ''' - self.ensure_one() + @api.model + def proxy_adyen_request(self, data, test_mode, api_key): + '''Necessary because Adyen's endpoints don't have CORS enabled. This is an + @api.model function to avoid concurrent update errors. Adyen's + async endpoint can still take well over a second to complete a + request. By using @api.model and passing in all data we need from + the POS we avoid locking the pos_payment_method table. This way we + avoid concurrent update errors when Adyen calls us back on + /pos_adyen/notification which will need to write on + pos.payment.method. + ''' TIMEOUT = 10 endpoint = 'https://terminal-api-live.adyen.com/async' - if self.adyen_test_mode: + if test_mode: endpoint = 'https://terminal-api-test.adyen.com/async' _logger.info('request to adyen\n%s', pprint.pformat(data)) headers = { - 'x-api-key': self.adyen_api_key, + 'x-api-key': api_key, 'Content-Type': 'application/json' } req = requests.post(endpoint, data=json.dumps(data), headers=headers, timeout=TIMEOUT) diff --git a/addons/pos_adyen/static/src/js/models.js b/addons/pos_adyen/static/src/js/models.js index f2734e00a94..6865f795733 100644 --- a/addons/pos_adyen/static/src/js/models.js +++ b/addons/pos_adyen/static/src/js/models.js @@ -3,5 +3,5 @@ var models = require('point_of_sale.models'); var PaymentAdyen = require('pos_adyen.payment'); models.register_payment_method('adyen', PaymentAdyen); -models.load_fields('pos.payment.method', 'adyen_terminal_identifier'); +models.load_fields('pos.payment.method', ['adyen_terminal_identifier', 'adyen_test_mode', 'adyen_api_key']); }); diff --git a/addons/pos_adyen/static/src/js/payment_adyen.js b/addons/pos_adyen/static/src/js/payment_adyen.js index fbf4549a5f3..b16f0b70666 100644 --- a/addons/pos_adyen/static/src/js/payment_adyen.js +++ b/addons/pos_adyen/static/src/js/payment_adyen.js @@ -47,7 +47,7 @@ var PaymentAdyen = PaymentInterface.extend({ return rpc.query({ model: 'pos.payment.method', method: 'proxy_adyen_request', - args: [[this.payment_method.id], data], + args: [data, this.payment_method.adyen_test_mode, this.payment_method.adyen_api_key], }, { // When a payment terminal is disconnected it takes Adyen // a while to return an error (~6s). So wait 10 seconds @@ -173,7 +173,11 @@ var PaymentAdyen = PaymentInterface.extend({ return rpc.query({ model: 'pos.payment.method', method: 'get_latest_adyen_status', - args: [[this.payment_method.id], this._adyen_get_sale_id()], + args: [this.payment_method.id, + this._adyen_get_sale_id(), + this.payment_method.adyen_terminal_identifier, + this.payment_method.adyen_test_mode, + this.payment_method.adyen_api_key], }, { timeout: 5000, shadow: true,