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

Rename various ApplicationServices interested methods #11915

Merged
merged 10 commits into from
Mar 3, 2022
Merged

Rename various ApplicationServices interested methods #11915

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
aaa402e
Rename various ApplicationServices interested methods
anoadragon453 Mar 2, 2022
fe17863
Add 'is_interested_in_user' and 'is_interested_in_room'
anoadragon453 Mar 3, 2022
a493a16
Update tests to use the new methods
anoadragon453 Mar 2, 2022
4866227
Remove _matches_user
anoadragon453 Mar 2, 2022
bcae63e
Remove inline'd _matches_room_id and _matches_aliases methods
anoadragon453 Mar 3, 2022
381b560
Add caching to ApplicationService.is_interested_in_event
anoadragon453 Nov 29, 2021
bf00539
Make matches_user_in_member_list method private; update refs
anoadragon453 Mar 3, 2022
27ffd6d
Changelog
anoadragon453 Mar 3, 2022
eb779e0
Type hints and docstring for _mkservice, _mkservice_alias
anoadragon453 Mar 3, 2022
453fdac
Add an event ID arg to is_interested_in_event to cache with
anoadragon453 Mar 3, 2022
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
17 changes: 11 additions & 6 deletions synapse/appservice/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ async def is_interested_in_room(
Returns:
True if the application service is interested in the room, False if not.
"""
# Check if this room ID matches the appservice's room ID namespace
# Check if we have interest in this room ID
if self.is_room_id_in_namespace(room_id):
return True

Expand All @@ -263,8 +263,9 @@ async def is_interested_in_room(
room_id, store, on_invalidate=cache_context.invalidate
)

@cached(num_args=1, cache_context=True)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this will work exactly as we're hoping -- EventBase equality is checked by it being the exact same object (i.e. if you built two EventBase instances with identical properties they aren't equal).

This might be OK, but I'm not 100% sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good catch. Ideally we'd cache on the event's ID instead of EventBase. I don't suppose there's an easy way to do that with @cached, though I can just create an LruCache much as we do for _get_event_cache:

synapse/synapse/storage/databases/main/events_worker.py

Lines 240 to 243 in e2e1d90

self._get_event_cache: LruCache[Tuple[str], EventCacheEntry] = LruCache(
cache_name="*getEvent*",
max_size=hs.config.caches.event_cache_size,
)

Copy link
Member

Choose a reason for hiding this comment

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

That might be better, in the past I've wonder if we should define __eq__ on EventBase which compares IDs, but I could see that not...being great in some situations.

Maybe using it is fine though? Might want to check if we do it anywhere else.

Copy link
Member Author

Choose a reason for hiding this comment

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

From the conversation in #synapse-dev, I believe we concluded on adding an event_id argument to is_interested_in_event. Doing so was simple, though updating the tests was slightly tedious.

Done in 453fdac!

async def is_interested_in_event(
self, event: EventBase, store: "DataStore"
self, event: EventBase, store: "DataStore", cache_context: _CacheContext
) -> bool:
"""Check if this service is interested in this event.

Expand All @@ -288,14 +289,16 @@ async def is_interested_in_event(
return True

# This will check the datastore, so should be run last
if await self.is_interested_in_room(event.room_id, store):
if await self.is_interested_in_room(
event.room_id, store, on_invalidate=cache_context.invalidate
):
return True

return False

@cached(num_args=1)
@cached(num_args=1, cache_context=True)
async def is_interested_in_presence(
self, user_id: UserID, store: "DataStore"
self, user_id: UserID, store: "DataStore", cache_context: _CacheContext
) -> bool:
"""Check if this service is interested a user's presence

Expand All @@ -313,7 +316,9 @@ async def is_interested_in_presence(

# Then find out if the appservice is interested in any of those rooms
for room_id in room_ids:
if await self.matches_user_in_member_list(room_id, store):
if await self.matches_user_in_member_list(
room_id, store, on_invalidate=cache_context.invalidate
):
return True
return False

Expand Down