Skip to content

Commit 7319791

Browse files
JordiBForgeFlowMiquelRForgeFlow
authored andcommitted
[IMP] endpoint_route_handler: memoize route version per request
_endpoint_route_last_version() is part of the routing_map ormcache key, so it runs on every routing map access -- e.g. once per url_for while rendering a website page, which adds up to dozens of identical 'SELECT last_value FROM endpoint_route_version' round-trips per page on content-heavy sites. The version cannot change within a request: memoize it on the request object.
1 parent 9e656c6 commit 7319791

3 files changed

Lines changed: 65 additions & 2 deletions

File tree

endpoint_route_handler/models/endpoint_route_sync_mixin.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ def _register_controllers(self, init=False, options=None, clear_cache=True):
9595
self._endpoint_registry.update_rules(rules, init=init)
9696
if clear_cache:
9797
self.env.registry.clear_cache("routing")
98+
self.env["ir.http"]._endpoint_route_reset_last_version()
9899
_logger.debug(
99100
"%s registered controllers: %s",
100101
self._name,
@@ -107,6 +108,7 @@ def _unregister_controllers(self, clear_cache=True):
107108
self._endpoint_registry.drop_rules(self._registered_endpoint_rule_keys())
108109
if clear_cache:
109110
self.env.registry.clear_cache("routing")
111+
self.env["ir.http"]._endpoint_route_reset_last_version()
110112
_logger.debug(
111113
"%s unregistered controllers: %s",
112114
self._name,

endpoint_route_handler/models/ir_http.py

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,44 @@ def routing_map(self, key=None):
4343
res = super().routing_map(key=key)
4444
return res
4545

46+
# Sentinel stored in the request `__dict__` to memoize the version.
47+
_endpoint_route_version_attr = "_endpoint_route_last_version"
48+
4649
@classmethod
4750
def _endpoint_route_last_version(cls):
48-
res = cls._get_routing_map_last_version(http.request.env)
49-
return res
51+
# This is part of the `routing_map` ormcache key, so it runs on
52+
# every routing map access (e.g. once per `url_for` while rendering
53+
# a website page). The version only changes when routes are
54+
# (un)registered, which resets the memo via
55+
# `_endpoint_route_reset_last_version`. Memoize it on the request
56+
# object to avoid one SQL round-trip per call.
57+
# NB: use `__dict__` rather than `getattr`: on a mocked request
58+
# `getattr` would auto-create a child mock instead of falling back
59+
# to the default, defeating the memoization.
60+
# NB: `http.request` is a werkzeug `LocalProxy`; when no request is
61+
# bound it is falsy (but not `None`), hence the truthiness check.
62+
request = http.request
63+
if not request:
64+
return cls._get_routing_map_last_version(http.request.env)
65+
version = request.__dict__.get(cls._endpoint_route_version_attr)
66+
if version is None:
67+
version = cls._get_routing_map_last_version(request.env)
68+
request.__dict__[cls._endpoint_route_version_attr] = version
69+
return version
70+
71+
@classmethod
72+
def _endpoint_route_reset_last_version(cls):
73+
"""Drop the memoized version so the next access reads it afresh.
74+
75+
Must be called whenever routes are (un)registered within a request,
76+
as the version changes and any value memoized earlier is now stale.
77+
"""
78+
# `http.request` is a werkzeug `LocalProxy`: falsy (but not `None`)
79+
# when no request is bound, e.g. when (un)registering routes outside
80+
# of an HTTP request (tests, crons, module install).
81+
request = http.request
82+
if request:
83+
request.__dict__.pop(cls._endpoint_route_version_attr, None)
5084

5185
@classmethod
5286
def _get_routing_map_last_version(cls, env):

endpoint_route_handler/tests/test_endpoint.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# @author: Simone Orsi <simone.orsi@camptocamp.com>
33
# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
44
from contextlib import contextmanager
5+
from unittest import mock
56

67
from odoo import api, modules
78
from odoo.tests import common
@@ -115,6 +116,32 @@ def test_as_tool_register_controllers_dynamic_route(self):
115116
rmap = self.env["ir.http"].routing_map()
116117
self.assertIn(route, [x.rule for x in rmap._rules])
117118

119+
def test_route_version_memoized_per_request(self):
120+
"""The route version is memoized on the request and reset on demand."""
121+
ir_http = self.env["ir.http"]
122+
attr = ir_http._endpoint_route_version_attr
123+
with self._get_mocked_request() as req:
124+
# Nothing memoized yet.
125+
self.assertNotIn(attr, req.__dict__)
126+
127+
# First access reads the version and memoizes it on the request.
128+
version = ir_http._endpoint_route_last_version()
129+
self.assertEqual(req.__dict__[attr], version)
130+
131+
# While memoized, further accesses don't hit the source again,
132+
# even if the underlying version changes.
133+
with mock.patch.object(
134+
type(ir_http),
135+
"_get_routing_map_last_version",
136+
return_value=version + 999,
137+
):
138+
self.assertEqual(ir_http._endpoint_route_last_version(), version)
139+
140+
# Resetting drops the memo so the next access recomputes.
141+
ir_http._endpoint_route_reset_last_version()
142+
self.assertNotIn(attr, req.__dict__)
143+
self.assertEqual(ir_http._endpoint_route_last_version(), version + 999)
144+
118145

119146
class TestEndpointCrossEnv(CommonEndpoint):
120147
def setUp(self):

0 commit comments

Comments
 (0)