Merge PR #520 into 16.0

Signed-off-by lmignon

Author: OCA-git-bot

base_rest_demo/services/exception_services.py

@@ -1,6 +1,8 @@
 # Copyright 2018 ACSONE SA/NV
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
 
+from psycopg2 import errorcodes
+from psycopg2.errors import OperationalError
 from werkzeug.exceptions import MethodNotAllowed
 
 from odoo import _
@@ -12,9 +14,13 @@
     ValidationError,
 )
 from odoo.http import SessionExpiredException
+from odoo.service.model import MAX_TRIES_ON_CONCURRENCY_FAILURE
 
+from odoo.addons.base_rest.components.service import to_int
 from odoo.addons.component.core import Component
 
+_CPT_RETRY = 0
+
 
 class ExceptionService(Component):
     _inherit = "base.rest.service"
@@ -90,6 +96,23 @@ def bare_exception(self):
         """
         raise IOError("My IO error")
 
+    def retryable_error(self, nbr_retries):
+        """This method is used in the test suite to check that the retrying
+        functionality in case of concurrency error on the database is working
+        correctly for retryable exceptions.
+
+        The output will be the number of retries that have been done.
+
+        This method is mainly used to test the retrying functionality
+        """
+        global _CPT_RETRY
+        if _CPT_RETRY < nbr_retries:
+            _CPT_RETRY += 1
+            raise FakeConcurrentUpdateError("fake error")
+        tryno = _CPT_RETRY
+        _CPT_RETRY = 0
+        return {"retries": tryno}
+
     # Validator
     def _validator_user_error(self):
         return {}
@@ -138,3 +161,22 @@ def _validator_bare_exception(self):
 
     def _validator_return_bare_exception(self):
         return {}
+
+    def _validator_retryable_error(self):
+        return {
+            "nbr_retries": {
+                "type": "integer",
+                "required": True,
+                "default": MAX_TRIES_ON_CONCURRENCY_FAILURE,
+                "coerce": to_int,
+            }
+        }
+
+    def _validator_return_retryable_error(self):
+        return {"retries": {"type": "integer"}}
+
+
+class FakeConcurrentUpdateError(OperationalError):
+    @property
+    def pgcode(self):
+        return errorcodes.SERIALIZATION_FAILURE

base_rest_demo/tests/test_exception.py

@@ -104,3 +104,17 @@ def test_bare_exception(self):
         self.assertEqual(response.headers["content-type"], "application/json")
         body = json.loads(response.content.decode("utf-8"))
         self.assertDictEqual(body, {"code": 500, "name": "Internal Server Error"})
+
+    @odoo.tools.mute_logger("odoo.addons.base_rest.http", "odoo.http")
+    def test_retrying(self):
+        """Test that the retrying mechanism is working as expected with the
+        FastAPI endpoints in case of POST request with a file.
+        """
+        nbr_retries = 3
+        response = self.url_open(
+            "%s/retryable_error" % self.url,
+            '{"nbr_retries": %d}' % nbr_retries,
+            timeout=20000,
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        self.assertDictEqual(response.json(), {"retries": nbr_retries})

rest_log/components/service.py

@@ -7,10 +7,12 @@
 import logging
 import traceback
 
+from psycopg2.errors import OperationalError
 from werkzeug.urls import url_encode, url_join
 
 from odoo import exceptions, registry
 from odoo.http import Response, request
+from odoo.service.model import PG_CONCURRENCY_ERRORS_TO_RETRY
 
 from odoo.addons.base_rest.http import JSONEncoder
 from odoo.addons.component.core import AbstractComponent
@@ -111,6 +113,15 @@ def _dispatch_exception(
                 log_entry_url = self._get_log_entry_url(log_entry)
         except Exception as e:
             _logger.exception("Rest Log Error Creation: %s", e)
+        # let the OperationalError bubble up to the retrying mechanism
+        # We can't wrap the OperationalError because we want to let it
+        # bubble up to the retrying mechanism, it will be handled by
+        # the default handler at the end of the chain.
+        if (
+            isinstance(orig_exception, OperationalError)
+            and orig_exception.pgcode in PG_CONCURRENCY_ERRORS_TO_RETRY
+        ):
+            raise orig_exception
         raise exception_klass(exc_msg, log_entry_url) from orig_exception
 
     def _get_exception_message(self, exception):

rest_log/tests/common.py

@@ -4,6 +4,9 @@
 
 import contextlib
 
+from psycopg2 import errorcodes
+from psycopg2.errors import OperationalError
+
 from odoo import exceptions
 
 from odoo.addons.base_rest import restapi
@@ -42,6 +45,7 @@ def fail(self, how):
                     "value": ValueError,
                     "validation": exceptions.ValidationError,
                     "user": exceptions.UserError,
+                    "retryable": FakeConcurrentUpdateError,
                 }
                 raise exc[how]("Failed as you wanted!")
 
@@ -61,3 +65,9 @@ def _get_mocked_request(self, env=None, httprequest=None, extra_headers=None):
             headers.update(extra_headers or {})
             mocked_request.httprequest.headers = headers
             yield mocked_request
+
+
+class FakeConcurrentUpdateError(OperationalError):
+    @property
+    def pgcode(self):
+        return errorcodes.SERIALIZATION_FAILURE

rest_log/tests/test_db_logging.py

@@ -13,7 +13,7 @@
 from odoo.addons.component.tests.common import new_rollbacked_env
 from odoo.addons.rest_log import exceptions as log_exceptions  # pylint: disable=W7950
 
-from .common import TestDBLoggingMixin
+from .common import FakeConcurrentUpdateError, TestDBLoggingMixin
 
 
 class TestDBLogging(TransactionRestServiceRegistryCase, TestDBLoggingMixin):
@@ -374,3 +374,66 @@ def test_log_exception_value(self):
         self._test_exception(
             "value", log_exceptions.RESTServiceDispatchException, "ValueError", "severe"
         )
+
+
+class TestDBLoggingRetryableError(
+    TransactionRestServiceRegistryCase, TestDBLoggingMixin
+):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls._setup_registry(cls)
+
+    @classmethod
+    def tearDownClass(cls):
+        # pylint: disable=W8110
+        cls._teardown_registry(cls)
+        super().tearDownClass()
+
+    def _test_exception(self, test_type, wrapping_exc, exc_name, severity):
+        log_model = self.env["rest.log"].sudo()
+        initial_entries = log_model.search([])
+        # Context: we are running in a transaction case which uses savepoints.
+        # The log machinery is going to rollback the transation when catching errors.
+        # Hence we need a completely separated env for the service.
+        with new_rollbacked_env() as new_env:
+            # Init fake collection w/ new env
+            collection = _PseudoCollection(self._collection_name, new_env)
+            service = self._get_service(self, collection=collection)
+            with self._get_mocked_request(env=new_env):
+                try:
+                    service.dispatch("fail", test_type)
+                except Exception as err:
+                    # Not using `assertRaises` to inspect the exception directly
+                    self.assertTrue(isinstance(err, wrapping_exc))
+                    self.assertEqual(
+                        service._get_exception_message(err), "Failed as you wanted!"
+                    )
+
+        with new_rollbacked_env() as new_env:
+            log_model = new_env["rest.log"].sudo()
+            entry = log_model.search([]) - initial_entries
+            expected = {
+                "collection": service._collection,
+                "state": "failed",
+                "result": "null",
+                "exception_name": exc_name,
+                "exception_message": "Failed as you wanted!",
+                "severity": severity,
+            }
+            self.assertRecordValues(entry, [expected])
+
+    @staticmethod
+    def _get_test_controller(class_or_instance, root_path=None):
+        return super()._get_test_controller(
+            class_or_instance, root_path="/test_log_exception_retryable/"
+        )
+
+    def test_log_exception_retryable(self):
+        # retryable error must bubble up to the retrying mechanism
+        self._test_exception(
+            "retryable",
+            FakeConcurrentUpdateError,
+            "odoo.addons.rest_log.tests.common.FakeConcurrentUpdateError",
+            "warning",
+        )