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

Fix a bug in the /event_reports Admin API which meant that the total count could be larger than the number of results you can actually query for. #13525

Merged
merged 5 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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/13525.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug in the `/event_reports` Admin API which meant that the total count could be larger than the number of results you can actually query for.
6 changes: 6 additions & 0 deletions synapse/storage/databases/main/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -2001,9 +2001,15 @@ def _get_event_reports_paginate_txn(

where_clause = "WHERE " + " AND ".join(filters) if len(filters) > 0 else ""

# We join on room_stats_state despite not using any columns from it
# because the join can influence the number of rows returned;
# e.g. a room that doesn't have state, maybe because it was deleted.
# The query returning the total count should be consistent with
# the query returning the results.
sql = """
SELECT COUNT(*) as total_event_reports
FROM event_reports AS er
JOIN room_stats_state ON room_stats_state.room_id = er.room_id
{}
""".format(
where_clause
Expand Down
29 changes: 29 additions & 0 deletions tests/rest/admin/test_event_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,35 @@ def _check_fields(self, content: List[JsonDict]) -> None:
self.assertIn("score", c)
self.assertIn("reason", c)

def test_count_correct_despite_table_deletions(self) -> None:
"""
Tests that the issue fixed by https://github.com/matrix-org/synapse/pull/13525
does not recur:
the count should match the number of rows, even if rows in joined tables
are missing.
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
"""

# Delete rows from room_stats_state for one of our rooms.
self.get_success(
self.hs.get_datastores().main.db_pool.simple_delete(
"room_stats_state", {"room_id": self.room_id1}, desc="_"
)
)

channel = self.make_request(
"GET",
self.url,
access_token=self.admin_user_tok,
)

self.assertEqual(200, channel.code, msg=channel.json_body)
# The 'total' field is 10 because only 10 reports will actually
# be retrievable since we deleted the rows in the room_stats_state
# table.
self.assertEqual(channel.json_body["total"], 10)
# This is consistent with the number of rows actually returned.
self.assertEqual(len(channel.json_body["event_reports"]), 10)


class EventReportDetailTestCase(unittest.HomeserverTestCase):
servlets = [
Expand Down