From eb8c841e14e28a21a99599aa0a2190dbf5b29334 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 4 Nov 2021 12:31:40 +0000 Subject: [PATCH 1/2] Additional test for `cachedList` I was trying to understand how `cachedList` works, and ended up writing this extra test. I figure we may as well keep it. --- changelog.d/11246.misc | 1 + tests/util/caches/test_descriptors.py | 43 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 changelog.d/11246.misc diff --git a/changelog.d/11246.misc b/changelog.d/11246.misc new file mode 100644 index 000000000000..e5e912c1b0d6 --- /dev/null +++ b/changelog.d/11246.misc @@ -0,0 +1 @@ +Add an additional test for the `cachedList` method decorator. diff --git a/tests/util/caches/test_descriptors.py b/tests/util/caches/test_descriptors.py index 39947a166b9b..dc747e0220c9 100644 --- a/tests/util/caches/test_descriptors.py +++ b/tests/util/caches/test_descriptors.py @@ -17,6 +17,7 @@ from unittest import mock from twisted.internet import defer, reactor +from twisted.internet.defer import Deferred from synapse.api.errors import SynapseError from synapse.logging.context import ( @@ -703,6 +704,48 @@ async def list_fn(self, args1, arg2): obj.mock.assert_called_once_with((40,), 2) self.assertEqual(r, {10: "fish", 40: "gravy"}) + def test_concurrent_lookups(self): + """All concurrent lookups should get the same result""" + + class Cls: + def __init__(self): + self.mock = mock.Mock() + + @descriptors.cached() + def fn(self, arg1): + pass + + @descriptors.cachedList("fn", "args1") + def list_fn(self, args1) -> Deferred[dict]: + return self.mock(args1) + + obj = Cls() + deferred_result = Deferred() + obj.mock.return_value = deferred_result + + # start off several concurrent lookups of the same key + d1 = obj.list_fn([10]) + d2 = obj.list_fn([10]) + d3 = obj.list_fn([10]) + + # the mock should have been called exactly once + obj.mock.assert_called_once_with((10,)) + obj.mock.reset_mock() + + # ... and none of the calls should yet be complete + self.assertFalse(d1.called) + self.assertFalse(d2.called) + self.assertFalse(d3.called) + + # complete the lookup. @cachedList functions need to complete with a map + # of input->result + deferred_result.callback({10: "peas"}) + + # ... which should give the right result to all the callers + self.assertEqual(self.successResultOf(d1), {10: "peas"}) + self.assertEqual(self.successResultOf(d2), {10: "peas"}) + self.assertEqual(self.successResultOf(d3), {10: "peas"}) + @defer.inlineCallbacks def test_invalidate(self): """Make sure that invalidation callbacks are called.""" From 6ba5af5768466fe07f41bfe7b1b08b5a4d140e7a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 4 Nov 2021 13:12:57 +0000 Subject: [PATCH 2/2] fix olddeps --- tests/util/caches/test_descriptors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/util/caches/test_descriptors.py b/tests/util/caches/test_descriptors.py index dc747e0220c9..ced3efd93f8b 100644 --- a/tests/util/caches/test_descriptors.py +++ b/tests/util/caches/test_descriptors.py @@ -716,7 +716,7 @@ def fn(self, arg1): pass @descriptors.cachedList("fn", "args1") - def list_fn(self, args1) -> Deferred[dict]: + def list_fn(self, args1) -> "Deferred[dict]": return self.mock(args1) obj = Cls()