Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Additional test for cachedList #11246

Merged
merged 2 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11246.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add an additional test for the `cachedList` method decorator.
43 changes: 43 additions & 0 deletions tests/util/caches/test_descriptors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Comment on lines +714 to +716
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is used---or is the "fn" on 718 referring to this somehow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the "fn" on 718 referring to this somehow?

yes, it is. an @cachedList function is alwaysan extension of an @cached function - it doesn't actually have a cache of its own.

So if you don't really care about having a non-list version, you still have to have a stub, just to hold the cache.

There are a few instances of this in the codebase, though there they throw NotImplementedError.


@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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for the reset? Just tidying up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly that.


# ... 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."""
Expand Down