From df5cde7df299291121892db1d8173db7494141a5 Mon Sep 17 00:00:00 2001 From: Simone Orsi Date: Tue, 22 Sep 2026 14:59:49 +0200 Subject: [PATCH] [FIX] webservice: always return the full response, drop content_only `_request()` defaulted `content_only` to `True`, silently returning raw bytes instead of the `requests.Response` object callers expect. The OAuth2 backend adapter ignored the flag entirely, always returning bytes (fixes #142). OCA#144 fixes the OAuth2 inconsistency but keeps `content_only` as a kwarg (default `None`, warns, still defaults to `True`) - perpetuating the actual problem: the return type shouldn't be a per-call switch, especially when raw content is just `.response.content` away. Here, `_request()` always returns the full response; the kwarg is dropped (a stray one is popped and warned about, not left to crash inside `requests.request()`). Backward compat is handled once, at the database level: the `18.0.2.0.1` upgrade script sets a `webservice.request_content_only` system parameter on existing databases only, preserving old behavior for calling code that relied on the implicit default. Delete the parameter to switch to the new behavior. The compat switch lives in `webservice`, not `webservice_core`: the latter is new and unreleased, so its `_request()` stays simple with no compat baggage. --- webservice/__manifest__.py | 2 +- webservice/components/request_adapter.py | 9 ++- webservice/models/webservice_backend.py | 52 +++++++++++++++ webservice/readme/CONFIGURE.md | 10 +++ webservice/readme/DESCRIPTION.md | 2 +- webservice/tests/__init__.py | 1 + webservice/tests/test_content_only_compat.py | 64 +++++++++++++++++++ webservice/tests/test_oauth2.py | 4 +- webservice/upgrades/18.0.2.0.1/post-update.py | 24 +++++++ .../models/webservice_request_mixin.py | 3 - webservice_core/readme/USAGE.md | 12 +--- webservice_core/tests/test_webservice.py | 23 +++---- 12 files changed, 176 insertions(+), 30 deletions(-) create mode 100644 webservice/readme/CONFIGURE.md create mode 100644 webservice/tests/test_content_only_compat.py create mode 100644 webservice/upgrades/18.0.2.0.1/post-update.py diff --git a/webservice/__manifest__.py b/webservice/__manifest__.py index 2fdc0610..a5212927 100644 --- a/webservice/__manifest__.py +++ b/webservice/__manifest__.py @@ -6,7 +6,7 @@ { "name": "WebService", "summary": """Defines webservice abstract definition to be used generally""", - "version": "18.0.2.0.0", + "version": "18.0.2.0.1", "license": "AGPL-3", "development_status": "Production/Stable", "maintainers": ["etobella", "simahawk"], diff --git a/webservice/components/request_adapter.py b/webservice/components/request_adapter.py index 4b5519b8..837fcb94 100644 --- a/webservice/components/request_adapter.py +++ b/webservice/components/request_adapter.py @@ -25,7 +25,7 @@ class BaseRestRequestsAdapter(Component): # TODO: url and url_params could come from work_ctx def _request(self, method, url=None, url_params=None, **kwargs): url = self._get_url(url=url, url_params=url_params) - content_only = kwargs.pop("content_only", True) + self.collection._pop_deprecated_content_only_kwarg(kwargs) # TODO: turn on/off debug from webservice setting? url_to_log = self._sanitize_url_for_log(url) _logger.info("%s call to %s", method, url_to_log) @@ -40,7 +40,7 @@ def _request(self, method, url=None, url_params=None, **kwargs): # pylint: disable=E8106 request = requests.request(method, url, **new_kwargs) request.raise_for_status() - if content_only: + if self.collection._get_request_content_only(): return request.content return request @@ -178,6 +178,7 @@ def _fetch_new_token(self, old_token): def _request(self, method, url=None, url_params=None, **kwargs): url = self._get_url(url=url, url_params=url_params) + self.collection._pop_deprecated_content_only_kwarg(kwargs) new_kwargs = kwargs.copy() new_kwargs.update( { @@ -190,7 +191,9 @@ def _request(self, method, url=None, url_params=None, **kwargs): # pylint: disable=E8106 request = session.request(method, url, **new_kwargs) request.raise_for_status() - return request.content + if self.collection._get_request_content_only(): + return request.content + return request class WebApplicationOAuth2RestRequestsAdapter(Component): diff --git a/webservice/models/webservice_backend.py b/webservice/models/webservice_backend.py index 22b9c8d3..83c71ee8 100644 --- a/webservice/models/webservice_backend.py +++ b/webservice/models/webservice_backend.py @@ -10,6 +10,14 @@ _logger = logging.getLogger(__name__) +# Backward-compat switch for the `content_only` default removal on `_request`. +# Existing databases get it set (see `webservice`'s `18.0.2.0.1` upgrade +# script) to keep returning only the response content; new installs get the +# full `requests.Response` object with no param set. Safe to delete once +# calling code has been adapted; the code checking it can then be dropped +# too. +CONTENT_ONLY_COMPAT_PARAM = "webservice.request_content_only" + class WebserviceBackend(models.Model): _name = "webservice.backend" @@ -89,6 +97,50 @@ def _get_adapter(self): webservice_protocol=self._get_adapter_protocol(), ) + def _request(self, method, url=None, url_params=None, **kwargs): + self._pop_deprecated_content_only_kwarg(kwargs) + response = super()._request(method, url=url, url_params=url_params, **kwargs) + if self._get_request_content_only(): + return response.content + return response + + def _pop_deprecated_content_only_kwarg(self, kwargs): + """Drop the removed ``content_only`` call argument, warning if used. + + It used to switch between returning the raw response content or the + full ``requests.Response`` object per call. It's gone: the full + response is always returned now, controlled only (and temporarily) + by the ``CONTENT_ONLY_COMPAT_PARAM`` system parameter for the whole + database - not something to keep sprinkling through call sites. + """ + if "content_only" in kwargs: + kwargs.pop("content_only") + _logger.warning( + "%s: the 'content_only' argument is no longer supported " + "and was ignored; the full response object is always " + "returned now. Remove it from the calling code.", + self.display_name, + ) + + def _get_request_content_only(self): + """Whether to return only the response content (legacy behavior). + + See ``CONTENT_ONLY_COMPAT_PARAM``. + """ + content_only = bool( + self.env["ir.config_parameter"].sudo().get_param(CONTENT_ONLY_COMPAT_PARAM) + ) + if content_only: + _logger.warning( + "%s: returning only the response content because the " + "'%s' system parameter is set (kept for backward " + "compatibility after upgrade). Delete it once the calling " + "code is adapted to use the full response object.", + self.display_name, + CONTENT_ONLY_COMPAT_PARAM, + ) + return content_only + def _get_adapter_protocol(self): protocol = self.protocol if self.auth_type.startswith("oauth2"): diff --git a/webservice/readme/CONFIGURE.md b/webservice/readme/CONFIGURE.md new file mode 100644 index 00000000..491de0ca --- /dev/null +++ b/webservice/readme/CONFIGURE.md @@ -0,0 +1,10 @@ +On upgrade from a version prior to `18.0.2.0.1`, a +`webservice.request_content_only` system parameter is created automatically +(see that version's migration script) to preserve the previous default +behavior of HTTP calls, which returned only the response content instead of +the full `requests.Response` object. + +If your code relies on that implicit default, keep the parameter for now. +Once it's adapted to use the full response object (see *Usage*), delete the +parameter under *Settings > Technical > System Parameters* - new installs +never get it set. diff --git a/webservice/readme/DESCRIPTION.md b/webservice/readme/DESCRIPTION.md index 1acbda1f..74b608df 100644 --- a/webservice/readme/DESCRIPTION.md +++ b/webservice/readme/DESCRIPTION.md @@ -1,6 +1,6 @@ This module creates WebService frameworks to be used globally. -The module introduces support for HTTP Request protocol. The webservice HTTP call returns by default the content of the response. A context 'content_only' can be passed to get the full response object. +The module introduces support for HTTP Request protocol. The webservice HTTP call returns the full `requests.Response` object. It builds on top of ``webservice_core`` (which provides the ``webservice.backend`` model with public/username-password/API key authentication) to add OAuth2 diff --git a/webservice/tests/__init__.py b/webservice/tests/__init__.py index 38fb5cf0..37c85ace 100644 --- a/webservice/tests/__init__.py +++ b/webservice/tests/__init__.py @@ -1 +1,2 @@ +from . import test_content_only_compat from . import test_oauth2 diff --git a/webservice/tests/test_content_only_compat.py b/webservice/tests/test_content_only_compat.py new file mode 100644 index 00000000..3552ebed --- /dev/null +++ b/webservice/tests/test_content_only_compat.py @@ -0,0 +1,64 @@ +# Copyright 2026 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +import responses + +from .common import CommonWebService + + +class TestContentOnlyCompat(CommonWebService): + """`content_only` removal, and its backward-compat system parameter. + + This lives in `webservice`, not `webservice_core`: the compat switch + only matters for databases that already ran with the old + `content_only=True` default, and that's `webservice`'s history, not + `webservice_core`'s (a new, not-yet-released module nobody depends on + with that old default). + """ + + @classmethod + def _setup_records(cls): + res = super()._setup_records() + cls.url = "https://localhost.demo.odoo/" + cls.webservice = cls.env["webservice.backend"].create( + { + "name": "WebService", + "protocol": "http", + "url": cls.url, + "tech_name": "demo_ws_content_only", + "auth_type": "none", + } + ) + return res + + @responses.activate + def test_returns_full_response_by_default(self): + responses.add(responses.GET, self.url, body="{}") + result = self.webservice.call("get") + self.assertEqual(result.content, b"{}") + self.assertEqual(result.status_code, 200) + + @responses.activate + def test_compat_param_restores_content_only(self): + self.env["ir.config_parameter"].sudo().set_param( + "webservice.request_content_only", "1" + ) + responses.add(responses.GET, self.url, body="{}") + with self.assertLogs( + "odoo.addons.webservice.models.webservice_backend", level="WARNING" + ) as log_catcher: + result = self.webservice.call("get") + self.assertEqual(result, b"{}") + self.assertTrue( + any("webservice.request_content_only" in m for m in log_catcher.output) + ) + + @responses.activate + def test_content_only_kwarg_is_ignored(self): + """The removed `content_only` kwarg no longer has any effect.""" + responses.add(responses.GET, self.url, body="{}") + with self.assertLogs( + "odoo.addons.webservice.models.webservice_backend", level="WARNING" + ) as log_catcher: + result = self.webservice.call("get", content_only=True) + self.assertEqual(result.content, b"{}") + self.assertTrue(any("content_only" in m for m in log_catcher.output)) diff --git a/webservice/tests/test_oauth2.py b/webservice/tests/test_oauth2.py index b0ef2c25..483e1a6e 100644 --- a/webservice/tests/test_oauth2.py +++ b/webservice/tests/test_oauth2.py @@ -69,7 +69,7 @@ def test_fetch_token(self): delta=1, # Accept a diff of 1s ) self.assertEqual(responses.calls[1].response.content.decode(), "OK") - self.assertEqual(result.decode(), "OK") + self.assertEqual(result.content.decode(), "OK") @responses.activate def test_update_token(self): @@ -111,7 +111,7 @@ def test_update_token(self): delta=1, # Accept a diff of 1s ) self.assertEqual(responses.calls[1].response.content.decode(), "OK") - self.assertEqual(result.decode(), "OK") + self.assertEqual(result.content.decode(), "OK") @responses.activate def test_update_token_with_error(self): diff --git a/webservice/upgrades/18.0.2.0.1/post-update.py b/webservice/upgrades/18.0.2.0.1/post-update.py new file mode 100644 index 00000000..5c1f3f42 --- /dev/null +++ b/webservice/upgrades/18.0.2.0.1/post-update.py @@ -0,0 +1,24 @@ +# Copyright 2026 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). + +from openupgradelib import openupgrade + +CONTENT_ONLY_COMPAT_PARAM = "webservice.request_content_only" + + +@openupgrade.migrate() +def migrate(env, version): + """Preserve the pre-upgrade behavior of HTTP webservice calls. + + Before this version, ``webservice.backend.call()`` (and friends) + returned only the response content (raw bytes) by default. That default + is gone: calls now always return the full ``requests.Response`` object. + + Existing databases get ``CONTENT_ONLY_COMPAT_PARAM`` set so their + calling code keeps working unchanged until it's adapted; new installs + never get it, so they see the new behavior right away. Leave the + parameter unset (or delete it) once the calling code is updated. + """ + icp = env["ir.config_parameter"].sudo() + if not icp.search_count([("key", "=", CONTENT_ONLY_COMPAT_PARAM)]): + icp.set_param(CONTENT_ONLY_COMPAT_PARAM, "1") diff --git a/webservice_core/models/webservice_request_mixin.py b/webservice_core/models/webservice_request_mixin.py index 8c86af3f..60f867f1 100644 --- a/webservice_core/models/webservice_request_mixin.py +++ b/webservice_core/models/webservice_request_mixin.py @@ -163,7 +163,6 @@ def call_delete(self, **kwargs): def _request(self, method, url=None, url_params=None, **kwargs): url = self._get_url(url=url, url_params=url_params) - content_only = kwargs.pop("content_only", True) url_to_log = self._sanitize_url_for_log(url) _logger.info("%s call to %s", method, url_to_log) new_kwargs = kwargs.copy() @@ -179,8 +178,6 @@ def _request(self, method, url=None, url_params=None, **kwargs): # pylint: disable=E8106 request = requests.request(method, url, **new_kwargs) request.raise_for_status() - if content_only: - return request.content return request def _sanitize_url_for_log(self, url): diff --git a/webservice_core/readme/USAGE.md b/webservice_core/readme/USAGE.md index 297c0e62..b767124e 100644 --- a/webservice_core/readme/USAGE.md +++ b/webservice_core/readme/USAGE.md @@ -2,7 +2,9 @@ Look up the backend (e.g. by its technical name) and call it: ```python backend = env["webservice.backend"].search([("tech_name", "=", "my_api")]) -result = backend.call("get") # -> bytes: the response content +result = backend.call("get") # -> requests.Response +result.content +result.status_code ``` `call(method, *args, **kwargs)` accepts any of the standard HTTP verbs @@ -48,11 +50,3 @@ for a single call (same format `requests` itself accepts, e.g. a backend.call("get", auth=("other_user", "other_password")) ``` -**Full response**: `call()` returns `response.content` by default. Pass -`content_only=False` to get the full `requests.Response` object instead -(status code, headers, etc.): - -```python -response = backend.call("get", content_only=False) -response.status_code -``` diff --git a/webservice_core/tests/test_webservice.py b/webservice_core/tests/test_webservice.py index 97a0513f..0720a90f 100644 --- a/webservice_core/tests/test_webservice.py +++ b/webservice_core/tests/test_webservice.py @@ -86,7 +86,8 @@ def test_auth_validation(self): def test_web_service_get(self): responses.add(responses.GET, self.url, body="{}") result = self.webservice.call("get") - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") + self.assertEqual(result.status_code, 200) self.assertEqual(len(responses.calls), 1) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" @@ -97,7 +98,7 @@ def test_web_service_get_url_combine(self): endpoint = "api/test" responses.add(responses.GET, self.url + endpoint, body="{}") result = self.webservice.call("get", url="api/test") - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual(len(responses.calls), 1) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" @@ -108,7 +109,7 @@ def test_web_service_get_url_combine_full_url(self): endpoint = "api/test" responses.add(responses.GET, self.url + endpoint, body="{}") result = self.webservice.call("get", url="https://localhost.demo.odoo/api/test") - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual(len(responses.calls), 1) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" @@ -118,7 +119,7 @@ def test_web_service_get_url_combine_full_url(self): def test_web_service_post(self): responses.add(responses.POST, self.url, body="{}") result = self.webservice.call("post", data="demo_response") - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" ) @@ -128,7 +129,7 @@ def test_web_service_post(self): def test_web_service_put(self): responses.add(responses.PUT, self.url, body="{}") result = self.webservice.call("put", data="demo_response") - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" ) @@ -141,7 +142,7 @@ def test_web_service_backend_username(self): ) responses.add(responses.GET, self.url, body="{}") result = self.webservice.call("get") - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual(len(responses.calls), 1) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" @@ -156,7 +157,7 @@ def test_web_service_username(self): ) responses.add(responses.GET, self.url, body="{}") result = self.webservice.call("get", auth=("user2", "pass2")) - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual(len(responses.calls), 1) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" @@ -171,7 +172,7 @@ def test_web_service_backend_api_key(self): ) responses.add(responses.POST, self.url, body="{}") result = self.webservice.call("post") - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual(len(responses.calls), 1) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" @@ -182,7 +183,7 @@ def test_web_service_backend_api_key(self): def test_web_service_headers(self): responses.add(responses.GET, self.url, body="{}") result = self.webservice.call("get", headers={"demo_header": "HEADER"}) - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual(len(responses.calls), 1) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" @@ -196,7 +197,7 @@ def test_web_service_call_args(self): result = self.webservice.call( "post", url=url, headers={"demo_header": "HEADER"} ) - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual(len(responses.calls), 1) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml" @@ -211,7 +212,7 @@ def test_web_service_call_args(self): url_params={"endpoint": "custom/path"}, headers={"demo_header": "HEADER"}, ) - self.assertEqual(result, b"{}") + self.assertEqual(result.content, b"{}") self.assertEqual(len(responses.calls), 2) self.assertEqual( responses.calls[0].request.headers["Content-Type"], "application/xml"