Ability to search entire room history after upgrading room #4415
Conversation
Signed-off-by: Andrew Morgan <[email protected]>
anoadragon453
force-pushed
the
anoa/full_search_upgraded_rooms
branch
from
6221cd7
to
cb80db8
Compare
Codecov Report
@@ Coverage Diff @@
## develop #4415 +/- ##
===========================================
- Coverage 73.68% 73.62% -0.07%
===========================================
Files 300 300
Lines 29703 29731 +28
Branches 4882 4887 +5
===========================================
+ Hits 21887 21888 +1
- Misses 6386 6412 +26
- Partials 1430 1431 +1
Continue to review full report at Codecov.
|
synapse/api/filtering.py
Outdated
| @@ -444,6 +444,9 @@ def lazy_load_members(self): | |||
| def include_redundant_members(self): | |||
| return self.filter_json.get("include_redundant_members", False) | |||
|
|
|||
| def add_room_ids(self, room_ids): | |||
There was a problem hiding this comment.
Filters are currently immutable, and I think that's a good thing for being able to reason about them. One way to do this would be to have a with_room_ids method which returns a new Filter, having replaced the room list.
Needs a docstring too, please.
synapse/handlers/search.py
Outdated
| @@ -139,6 +187,27 @@ def search(self, user, content, batch=None): | |||
|
|
|||
| room_ids = search_filter.filter_rooms(room_ids) | |||
There was a problem hiding this comment.
should this not happen after we add the historical rooms to the filter?
synapse/handlers/search.py
Outdated
| @@ -139,6 +187,27 @@ def search(self, user, content, batch=None): | |||
|
|
|||
| room_ids = search_filter.filter_rooms(room_ids) | |||
|
|
|||
| # If doing a subset of all rooms seearch, check if any of the rooms | |||
| # are from an upgraded room, and search their contents as well | |||
| # XXX: There is the possibility that we don't have a create event for | |||
There was a problem hiding this comment.
we ought to have the create event for any room that the user has ever been a member of, so I don't think this is relevant
synapse/handlers/search.py
Outdated
| historical_room_ids = [] | ||
|
|
||
| while True: | ||
| state_ids = yield self.store.get_current_state_ids(room_id) |
There was a problem hiding this comment.
I'd suggest adding a get_room_predecessor to StateGroupWorkerStore alongside get_room_version.
synapse/handlers/search.py
Outdated
| room_id (str): The ID of the room to search through. | ||
|
|
||
| Returns: | ||
| dict of past room IDs as strings |
There was a problem hiding this comment.
I don't think it's a dict...
Itym Deferred[list[str]]: predecessor room ids, or better yet, Deferred[iterable[str]]: predecessor room ids
synapse/storage/state.py
Outdated
| @@ -448,6 +448,7 @@ def get_current_state_ids(self, room_id): | |||
| Returns: | |||
| deferred: dict of (type, state_key) -> event_id | |||
| """ | |||
|
|
|||
There was a problem hiding this comment.
please try to avoid unrelated reformatting in your PRs; it makes git history harder to read.
anoadragon453
force-pushed
the
anoa/full_search_upgraded_rooms
branch
from
6ca6d12
to
c2bbe85
Compare
* Create a new method for getting predecessor rooms * Remove formatting change
anoadragon453
force-pushed
the
anoa/full_search_upgraded_rooms
branch
from
c2bbe85
to
c9bfb05
Compare
synapse/storage/state.py
Outdated
| return None | ||
|
|
||
| # Return predecessor if present | ||
| return create_event.content.get("predecessor", None) |
There was a problem hiding this comment.
@anoadragon453 you can't return in an inlinecallbacks
There was a problem hiding this comment.
Ah, you're right, thank you. Still getting used to this execution model.
synapse/storage/state.py
Outdated
| # Retrieve the room's create event | ||
| create_event = yield self.get_event(create_id) | ||
|
|
||
| if not create_event: |
There was a problem hiding this comment.
this is redundant fwiw: get_event cannot return None.
Co-Authored-By: anoadragon453 <[email protected]>
Co-Authored-By: anoadragon453 <[email protected]>
…rg/synapse into anoa/full_search_upgraded_rooms
Pull Request Checklist
When searching a subset of rooms, we check each one for any predecessor rooms, and those for any predecessor rooms and so on, and return all results to the client. No change when searching 'all rooms' as predecessor rooms are already included in this (though it shows for "My Room", "My Room" and "My Room". Maybe it should say "My Room - v1", "My Room - v2" and "My Room - v3" instead).
Closes #4410