Merge PR #354 into 16.0

Signed-off-by lmignon

.pre-commit-config.yaml

@@ -4,7 +4,6 @@ exclude: |
   ^base_rest_auth_jwt/|
   ^base_rest_auth_user_service/|
   ^model_serializer/|
-  ^rest_log/|
   # END NOT INSTALLABLE ADDONS
   # Files and folders generated by bots, to avoid loops
   ^setup/|/static/description/index\.html$|

rest_log/README.rst

@@ -2,10 +2,13 @@
 REST Log
 ========
 
-.. !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+.. 
+   !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    !! This file is generated by oca-gen-addon-readme !!
    !! changes will be overwritten.                   !!
    !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+   !! source digest: sha256:c4d8f6d63c3748741d5306c2b4327785eaec0a0c9770397ef4610266c515d0f2
+   !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
 .. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png
     :target: https://odoo-community.org/page/development-status
@@ -14,16 +17,16 @@ REST Log
     :target: http://www.gnu.org/licenses/lgpl-3.0-standalone.html
     :alt: License: LGPL-3
 .. |badge3| image:: https://img.shields.io/badge/github-OCA%2Frest--framework-lightgray.png?logo=github
-    :target: https://github.com/OCA/rest-framework/tree/15.0/rest_log
+    :target: https://github.com/OCA/rest-framework/tree/16.0/rest_log
     :alt: OCA/rest-framework
 .. |badge4| image:: https://img.shields.io/badge/weblate-Translate%20me-F47D42.png
-    :target: https://translation.odoo-community.org/projects/rest-framework-15-0/rest-framework-15-0-rest_log
+    :target: https://translation.odoo-community.org/projects/rest-framework-16-0/rest-framework-16-0-rest_log
     :alt: Translate me on Weblate
-.. |badge5| image:: https://img.shields.io/badge/runbot-Try%20me-875A7B.png
-    :target: https://runbot.odoo-community.org/runbot/271/15.0
-    :alt: Try me on Runbot
+.. |badge5| image:: https://img.shields.io/badge/runboat-Try%20me-875A7B.png
+    :target: https://runboat.odoo-community.org/builds?repo=OCA/rest-framework&target_branch=16.0
+    :alt: Try me on Runboat
 
-|badge1| |badge2| |badge3| |badge4| |badge5| 
+|badge1| |badge2| |badge3| |badge4| |badge5|
 
 When exposing REST services is often useful to see what's happening
 especially in case of errors.
@@ -90,8 +93,8 @@ Bug Tracker
 
 Bugs are tracked on `GitHub Issues <https://github.com/OCA/rest-framework/issues>`_.
 In case of trouble, please check there if your issue has already been reported.
-If you spotted it first, help us smashing it by providing a detailed and welcomed
-`feedback <https://github.com/OCA/rest-framework/issues/new?body=module:%20rest_log%0Aversion:%2015.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
+If you spotted it first, help us to smash it by providing a detailed and welcomed
+`feedback <https://github.com/OCA/rest-framework/issues/new?body=module:%20rest_log%0Aversion:%2016.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
 
 Do not contact contributors directly about support or help with technical issues.
 
@@ -140,6 +143,6 @@ Current `maintainer <https://odoo-community.org/page/maintainer-role>`__:
 
 |maintainer-simahawk| 
 
-This module is part of the `OCA/rest-framework <https://github.com/OCA/rest-framework/tree/15.0/rest_log>`_ project on GitHub.
+This module is part of the `OCA/rest-framework <https://github.com/OCA/rest-framework/tree/16.0/rest_log>`_ project on GitHub.
 
 You are welcome to contribute. To learn how please visit https://odoo-community.org/page/Contribute.

rest_log/__manifest__.py

@@ -5,7 +5,7 @@
 {
     "name": "REST Log",
     "summary": "Track REST API calls into DB",
-    "version": "15.0.1.0.0",
+    "version": "16.0.1.0.0",
     "development_status": "Beta",
     "website": "https://github.com/OCA/rest-framework",
     "author": "Camptocamp, ACSONE, Odoo Community Association (OCA)",
@@ -20,5 +20,4 @@
         "views/rest_log_views.xml",
         "views/menu.xml",
     ],
-    "installable": False,
 }

rest_log/components/service.py

@@ -4,12 +4,13 @@
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html).
 
 import json
+import logging
 import traceback
 
 from werkzeug.urls import url_encode, url_join
 
 from odoo import exceptions, registry
-from odoo.http import request
+from odoo.http import Response, request
 
 from odoo.addons.base_rest.http import JSONEncoder
 from odoo.addons.component.core import AbstractComponent
@@ -20,10 +21,12 @@
     RESTServiceValidationErrorException,
 )
 
+_logger = logging.getLogger(__name__)
+
 
 def json_dump(data):
     """Encode data to JSON as we like."""
-    return json.dumps(data, cls=JSONEncoder, indent=4, sort_keys=True)
+    return json.dumps(data, cls=JSONEncoder, indent=4, sort_keys=True, default=str)
 
 
 class BaseRESTService(AbstractComponent):
@@ -37,10 +40,9 @@ def dispatch(self, method_name, *args, params=None):
         return self._dispatch_with_db_logging(method_name, *args, params=params)
 
     def _dispatch_with_db_logging(self, method_name, *args, params=None):
-        # TODO: consider refactoring thi using a savepoint as described here
-        # https://github.com/OCA/rest-framework/pull/106#pullrequestreview-582099258
         try:
-            result = super().dispatch(method_name, *args, params=params)
+            with self.env.cr.savepoint():
+                result = super().dispatch(method_name, *args, params=params)
         except exceptions.ValidationError as orig_exception:
             self._dispatch_exception(
                 method_name,
@@ -65,34 +67,41 @@ def _dispatch_with_db_logging(self, method_name, *args, params=None):
                 *args,
                 params=params,
             )
-        log_entry = self._log_call_in_db(
-            self.env, request, method_name, *args, params=params, result=result
-        )
-        if log_entry and isinstance(result, dict):
-            log_entry_url = self._get_log_entry_url(log_entry)
-            result["log_entry_url"] = log_entry_url
+        self._log_dispatch_success(method_name, result, *args, params)
         return result
 
+    def _log_dispatch_success(self, method_name, result, *args, params=None):
+        try:
+            with self.env.cr.savepoint():
+                log_entry = self._log_call_in_db(
+                    self.env, request, method_name, *args, params, result=result
+                )
+                if log_entry and not isinstance(result, Response):
+                    log_entry_url = self._get_log_entry_url(log_entry)
+                    result["log_entry_url"] = log_entry_url
+        except Exception as e:
+            _logger.exception("Rest Log Error Creation: %s", e)
+
     def _dispatch_exception(
         self, method_name, exception_klass, orig_exception, *args, params=None
     ):
-        tb = traceback.format_exc()
-        # TODO: how to test this? Cannot rollback nor use another cursor
-        self.env.cr.rollback()
-        with registry(self.env.cr.dbname).cursor() as cr:
-            env = self.env(cr=cr)
-            log_entry = self._log_call_in_db(
-                env,
-                request,
-                method_name,
-                *args,
-                params=params,
-                traceback=tb,
-                orig_exception=orig_exception,
-            )
-            log_entry_url = self._get_log_entry_url(log_entry)
-        # UserError and alike have `name` attribute to store the msg
-        exc_msg = self._get_exception_message(orig_exception)
+        exc_msg, log_entry_url = None, None  # in case it fails below
+        try:
+            exc_msg = self._get_exception_message(orig_exception)
+            tb = traceback.format_exc()
+            with registry(self.env.cr.dbname).cursor() as cr:
+                log_entry = self._log_call_in_db(
+                    self.env(cr=cr),
+                    request,
+                    method_name,
+                    *args,
+                    params=params,
+                    traceback=tb,
+                    orig_exception=orig_exception,
+                )
+                log_entry_url = self._get_log_entry_url(log_entry)
+        except Exception as e:
+            _logger.exception("Rest Log Error Creation: %s", e)
         raise exception_klass(exc_msg, log_entry_url) from orig_exception
 
     def _get_exception_message(self, exception):
@@ -115,45 +124,75 @@ def _log_call_header_strip(self):
 
     def _log_call_in_db_values(self, _request, *args, params=None, **kw):
         httprequest = _request.httprequest
-        headers = dict(httprequest.headers)
-        for header_key in self._log_call_header_strip:
-            if header_key in headers:
-                headers[header_key] = "<redacted>"
+        headers = self._log_call_sanitize_headers(dict(httprequest.headers or []))
+        params = dict(params or {})
         if args:
-            params = dict(params or {}, args=args)
-
-        result = kw.get("result")
-        error = kw.get("traceback")
-        orig_exception = kw.get("orig_exception")
-        exception_name = None
-        exception_message = None
-        if orig_exception:
-            exception_name = orig_exception.__class__.__name__
-            if hasattr(orig_exception, "__module__"):
-                exception_name = orig_exception.__module__ + "." + exception_name
-            exception_message = self._get_exception_message(orig_exception)
+            params.update(args=args)
+        params = self._log_call_sanitize_params(params)
+        error, exception_name, exception_message = self._log_call_prepare_error(**kw)
+        result, state = self._log_call_prepare_result(kw.get("result"))
         collection = self.work.collection
         return {
             "collection": collection._name,
             "collection_id": collection.id,
             "request_url": httprequest.url,
             "request_method": httprequest.method,
-            "params": json_dump(params),
-            "headers": json_dump(headers),
-            "result": json_dump(result),
+            "params": params,
+            "headers": headers,
+            "result": result,
             "error": error,
             "exception_name": exception_name,
             "exception_message": exception_message,
-            "state": "success" if result else "failed",
+            "state": state,
         }
 
+    def _log_call_prepare_result(self, result):
+        # NB: ``result`` might be an object of class ``odoo.http.Response``,
+        # for example when you try to download a file. In this case, we need to
+        # handle it properly, without the assumption that ``result`` is a dict.
+        if isinstance(result, Response):
+            status_code = result.status_code
+            result = {
+                "status": status_code,
+                "headers": self._log_call_sanitize_headers(dict(result.headers or [])),
+            }
+            state = "success" if status_code in range(200, 300) else "failed"
+        else:
+            state = "success" if result else "failed"
+        return result, state
+
+    def _log_call_prepare_error(self, traceback=None, orig_exception=None, **kw):
+        exception_name = None
+        exception_message = None
+        if orig_exception:
+            exception_name = orig_exception.__class__.__name__
+            if hasattr(orig_exception, "__module__"):
+                exception_name = orig_exception.__module__ + "." + exception_name
+            exception_message = self._get_exception_message(orig_exception)
+        return traceback, exception_name, exception_message
+
+    _log_call_in_db_keys_to_serialize = ("params", "headers", "result")
+
     def _log_call_in_db(self, env, _request, method_name, *args, params=None, **kw):
         values = self._log_call_in_db_values(_request, *args, params=params, **kw)
+        for k in self._log_call_in_db_keys_to_serialize:
+            values[k] = json_dump(values[k])
         enabled_states = self._get_matching_active_conf(method_name)
         if not values or enabled_states and values["state"] not in enabled_states:
             return
         return env["rest.log"].sudo().create(values)
 
+    def _log_call_sanitize_params(self, params: dict) -> dict:
+        if "password" in params:
+            params["password"] = "<redacted>"
+        return params
+
+    def _log_call_sanitize_headers(self, headers: dict) -> dict:
+        for header_key in self._log_call_header_strip:
+            if header_key in headers:
+                headers[header_key] = "<redacted>"
+        return headers
+
     def _db_logging_active(self, method_name):
         enabled = self._log_calls_in_db
         if not enabled: