mirror of
https://github.com/element-hq/synapse.git
synced 2026-09-17 21:05:30 +00:00
Fix event_search background reindex skipping m.room.topic events (#20119)
Room topics are dropped from the search index whenever the `event_search` background reindex runs (e.g. after a search index rebuild, or when the background update is re-run on an upgraded homeserver), making topics unsearchable even though the live write path indexes them correctly. The cause is a trailing comma in `_background_reindex_search`, which turns the topic `value` into a 1-tuple instead of a string: https://github.com/element-hq/synapse/blob/14c96c0f5444cbe28b6ac0361cf94216b8d352db/synapse/storage/databases/main/search.py#L211-L213 The downstream `if not isinstance(value, str): continue` guard then silently skips *every* `m.room.topic` event, so no topic ever reaches `event_search` during a reindex. The regression was introduced in #18195, which added rich-text topic support (MSC3765) to the reindex path. ### Problem Example 1. A room has topic "project roadmap". 2. An admin rebuilds the search index (or the `event_search` background update re-runs). 3. `m.room.message` and `m.room.name` events are reindexed fine, but every `m.room.topic` event is skipped. 4. Searching for "project roadmap" with key `content.topic` returns 0 results — the topic is permanently unsearchable until the event is sent again. --- ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
This commit is contained in:
@@ -0,0 +1 @@
|
||||
Fix a bug where the `event_search` background reindex skipped all `m.room.topic` events, making room topics unsearchable after a search index rebuild.
|
||||
@@ -208,9 +208,7 @@ class SearchBackgroundUpdateStore(SearchWorkerStore):
|
||||
value = content["body"]
|
||||
elif etype == "m.room.topic":
|
||||
key = "content.topic"
|
||||
value = (
|
||||
get_plain_text_topic_from_event_content(content) or "",
|
||||
)
|
||||
value = get_plain_text_topic_from_event_content(content) or ""
|
||||
elif etype == "m.room.name":
|
||||
key = "content.name"
|
||||
value = content["name"]
|
||||
|
||||
@@ -19,6 +19,7 @@
|
||||
#
|
||||
#
|
||||
|
||||
import json
|
||||
from unittest.case import SkipTest
|
||||
|
||||
from twisted.internet.testing import MemoryReactor
|
||||
@@ -202,6 +203,86 @@ class EventSearchInsertionTest(HomeserverTestCase):
|
||||
)
|
||||
self.assertCountEqual(values, ["hi", "2"])
|
||||
|
||||
def _rebuild_search_index(self) -> None:
|
||||
"""Simulate a full search-index rebuild: wipe `event_search` so nothing
|
||||
is indexed, then schedule and run the `event_search` background reindex,
|
||||
which exercises `_background_reindex_search`.
|
||||
"""
|
||||
store = self.hs.get_datastores().main
|
||||
|
||||
self.get_success(
|
||||
store.db_pool.runInteraction(
|
||||
"clear_event_search",
|
||||
lambda txn: txn.execute("DELETE FROM event_search"),
|
||||
)
|
||||
)
|
||||
|
||||
max_stream_id = store.get_room_max_stream_ordering()
|
||||
store.db_pool.updates._all_done = False
|
||||
self.get_success(
|
||||
store.db_pool.simple_insert(
|
||||
"background_updates",
|
||||
{
|
||||
"update_name": store.EVENT_SEARCH_UPDATE_NAME,
|
||||
"progress_json": json.dumps(
|
||||
{
|
||||
"target_min_stream_id_inclusive": 0,
|
||||
"max_stream_id_exclusive": max_stream_id + 1,
|
||||
"rows_inserted": 0,
|
||||
}
|
||||
),
|
||||
},
|
||||
)
|
||||
)
|
||||
self.wait_for_background_updates()
|
||||
|
||||
def test_reindex_search(self) -> None:
|
||||
"""The `event_search` background reindex must index all searchable event
|
||||
types: `m.room.message`, `m.room.name` and `m.room.topic`.
|
||||
"""
|
||||
store = self.hs.get_datastores().main
|
||||
|
||||
# Create a room and set a searchable topic, name and message through the
|
||||
# normal client API so the events land in `events`/`event_json`.
|
||||
self.register_user("alice", "password")
|
||||
access_token = self.login("alice", "password")
|
||||
room_id = self.helper.create_room_as("alice", tok=access_token)
|
||||
|
||||
self.helper.send_state(
|
||||
room_id,
|
||||
"m.room.topic",
|
||||
{"topic": "searchable topic keyword"},
|
||||
tok=access_token,
|
||||
)
|
||||
self.helper.send_state(
|
||||
room_id,
|
||||
"m.room.name",
|
||||
{"name": "searchable name keyword"},
|
||||
tok=access_token,
|
||||
)
|
||||
self.helper.send(room_id, "searchable message keyword", tok=access_token)
|
||||
|
||||
self._rebuild_search_index()
|
||||
|
||||
message_results = self.get_success(
|
||||
store.search_msgs([room_id], "searchable message keyword", ["content.body"])
|
||||
)
|
||||
self.assertEqual(message_results["count"], 1, "message was not reindexed")
|
||||
|
||||
name_results = self.get_success(
|
||||
store.search_msgs([room_id], "searchable name keyword", ["content.name"])
|
||||
)
|
||||
self.assertEqual(name_results["count"], 1, "name was not reindexed")
|
||||
|
||||
topic_results = self.get_success(
|
||||
store.search_msgs([room_id], "searchable topic keyword", ["content.topic"])
|
||||
)
|
||||
self.assertEqual(
|
||||
topic_results["count"],
|
||||
1,
|
||||
"room topic was not indexed by the search reindex",
|
||||
)
|
||||
|
||||
|
||||
class MessageSearchTest(HomeserverTestCase):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user