From 15695207362a7ac32b4c328cecdd1fdb7f594b79 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 15 Aug 2022 11:35:56 +0100 Subject: [PATCH 1/4] Make /event_reports' total count align with the results you get --- synapse/storage/databases/main/room.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 0f1f0d11eac6..b7d4baa6bb30 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -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 From 1c7af820fa0b3fc48ed69ee04d6c416591d8863f Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 15 Aug 2022 11:37:29 +0100 Subject: [PATCH 2/4] Newsfile Signed-off-by: Olivier Wilkinson (reivilibre) --- changelog.d/13525.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13525.bugfix diff --git a/changelog.d/13525.bugfix b/changelog.d/13525.bugfix new file mode 100644 index 000000000000..dbd1adbc88c5 --- /dev/null +++ b/changelog.d/13525.bugfix @@ -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. \ No newline at end of file From fd04601aeeb6c271d3cf24ed75f3533ed8092280 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 17 Aug 2022 12:24:02 +0100 Subject: [PATCH 3/4] Add test that the total is correct under room_stats_state deletions --- tests/rest/admin/test_event_reports.py | 29 ++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index fbc490f46d5a..6b6e468b542e 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -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. + """ + + # 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 = [ From 41cc7199126a4eca23ae0e912e245d8de60fd4c5 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Wed, 17 Aug 2022 16:09:23 +0100 Subject: [PATCH 4/4] Update tests/rest/admin/test_event_reports.py Co-authored-by: Brendan Abolivier --- tests/rest/admin/test_event_reports.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index 6b6e468b542e..8a4e5c3f777b 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -412,9 +412,7 @@ def _check_fields(self, content: List[JsonDict]) -> None: 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 + Tests that the count matches the number of rows, even if rows in joined tables are missing. """