Skip to content

Commit 1111135

Browse files
jensensclaude
andcommitted
Use request["key"] instead of request._attr for cache storage
As suggested by @davisagli in PR review, store the cache in request.other (via item access) rather than on request.__dict__. request.other is explicitly cleared at end-of-request processing, avoiding potential reference cycles and stale caches in tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0f2139a commit 1111135

File tree

2 files changed

+27
-11
lines changed

2 files changed

+27
-11
lines changed

src/plone/registry/registry.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,10 @@ def _get_request_cache(context):
3131
request = getRequest()
3232
if request is None:
3333
return None
34-
try:
35-
all_caches = request._plone_registry_cache
36-
except AttributeError:
34+
all_caches = request.get("_plone_registry_cache")
35+
if all_caches is None:
3736
all_caches = {}
38-
request._plone_registry_cache = all_caches
37+
request["_plone_registry_cache"] = all_caches
3938
registry_id = id(aq_base(context))
4039
try:
4140
return all_caches[registry_id]

src/plone/registry/tests.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,27 @@ def test_auto_migration(self):
142142

143143

144144
class FakeRequest:
145-
"""Minimal request-like object for testing request-level caching."""
145+
"""Minimal request-like object for testing request-level caching.
146+
147+
Uses a dict (``other``) for item access, mirroring Zope's
148+
``HTTPRequest.other`` which is cleared at end-of-request.
149+
"""
146150

147151
def __init__(self):
148152
self.environ = {}
153+
self.other = {}
154+
155+
def get(self, key, default=None):
156+
return self.other.get(key, default)
157+
158+
def __getitem__(self, key):
159+
return self.other[key]
160+
161+
def __setitem__(self, key, value):
162+
self.other[key] = value
163+
164+
def __contains__(self, key):
165+
return key in self.other
149166

150167

151168
class TestRequestValueCache(unittest.TestCase):
@@ -184,19 +201,19 @@ def _getCache(self, request=None):
184201
"""Return the per-registry cache dict from the given request."""
185202
if request is None:
186203
request = self.request
187-
all_caches = getattr(request, "_plone_registry_cache", {})
204+
all_caches = request.get("_plone_registry_cache", {})
188205
return all_caches.get(id(self.registry), {})
189206

190207
def _setCache(self, data):
191208
"""Inject values into the per-registry cache on the request."""
192-
self.request._plone_registry_cache = {id(self.registry): data}
209+
self.request["_plone_registry_cache"] = {id(self.registry): data}
193210

194211
# --- __getitem__ tests ---
195212

196213
def test_getitem_no_request(self):
197214
"""Without a request, values are fetched directly from OOBTree."""
198215
self.assertEqual(self.registry["test.key1"], "value1")
199-
self.assertFalse(hasattr(self.request, "_plone_registry_cache"))
216+
self.assertNotIn("_plone_registry_cache", self.request)
200217

201218
def test_getitem_populates_cache(self):
202219
"""First read with a request populates the cache."""
@@ -295,7 +312,7 @@ def test_cache_isolation_between_requests(self):
295312

296313
new_request = FakeRequest()
297314
setRequest(new_request)
298-
self.assertFalse(hasattr(new_request, "_plone_registry_cache"))
315+
self.assertNotIn("_plone_registry_cache", new_request)
299316

300317
self.assertEqual(self.registry["test.key1"], "value1")
301318
self.assertIn("test.key1", self._getCache(new_request))
@@ -304,9 +321,9 @@ def test_cache_isolation_between_requests(self):
304321
def test_cache_lazily_created(self):
305322
"""Cache dict is only created on first registry access."""
306323
self._setRequest()
307-
self.assertFalse(hasattr(self.request, "_plone_registry_cache"))
324+
self.assertNotIn("_plone_registry_cache", self.request)
308325
self.registry["test.key1"]
309-
self.assertTrue(hasattr(self.request, "_plone_registry_cache"))
326+
self.assertIn("_plone_registry_cache", self.request)
310327

311328
# --- __contains__ tests ---
312329

0 commit comments

Comments
 (0)