mirror of
https://git.anonymousland.org/anonymousland/synapse.git
synced 2024-09-09 14:32:06 +00:00
Make pagination of rooms in admin api stable (#11737)
Always add state.room_id after the configurable ORDER BY. Otherwise, for any sort, certain pages can contain results from other pages. (Especially when sorting by creator, since there may be many rooms by the same creator) * Document different order direction of numerical fields "joined_members", "joined_local_members", "version" and "state_events" are ordered in descending direction by default (dir=f). Added a note in tests to explain the differences in ordering. Signed-off-by: Daniël Sonck <daniel@sonck.nl>
This commit is contained in:
parent
e7da1ced24
commit
6b241f5286
1
changelog.d/11737.bugfix
Normal file
1
changelog.d/11737.bugfix
Normal file
@ -0,0 +1 @@
|
|||||||
|
Make the list rooms admin api sort stable. Contributed by Daniël Sonck.
|
@ -551,24 +551,24 @@ class RoomWorkerStore(CacheInvalidationWorkerStore):
|
|||||||
FROM room_stats_state state
|
FROM room_stats_state state
|
||||||
INNER JOIN room_stats_current curr USING (room_id)
|
INNER JOIN room_stats_current curr USING (room_id)
|
||||||
INNER JOIN rooms USING (room_id)
|
INNER JOIN rooms USING (room_id)
|
||||||
%s
|
{where}
|
||||||
ORDER BY %s %s
|
ORDER BY {order_by} {direction}, state.room_id {direction}
|
||||||
LIMIT ?
|
LIMIT ?
|
||||||
OFFSET ?
|
OFFSET ?
|
||||||
""" % (
|
""".format(
|
||||||
where_statement,
|
where=where_statement,
|
||||||
order_by_column,
|
order_by=order_by_column,
|
||||||
"ASC" if order_by_asc else "DESC",
|
direction="ASC" if order_by_asc else "DESC",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Use a nested SELECT statement as SQL can't count(*) with an OFFSET
|
# Use a nested SELECT statement as SQL can't count(*) with an OFFSET
|
||||||
count_sql = """
|
count_sql = """
|
||||||
SELECT count(*) FROM (
|
SELECT count(*) FROM (
|
||||||
SELECT room_id FROM room_stats_state state
|
SELECT room_id FROM room_stats_state state
|
||||||
%s
|
{where}
|
||||||
) AS get_room_ids
|
) AS get_room_ids
|
||||||
""" % (
|
""".format(
|
||||||
where_statement,
|
where=where_statement,
|
||||||
)
|
)
|
||||||
|
|
||||||
def _get_rooms_paginate_txn(
|
def _get_rooms_paginate_txn(
|
||||||
|
@ -1089,6 +1089,8 @@ class RoomTestCase(unittest.HomeserverTestCase):
|
|||||||
)
|
)
|
||||||
room_ids.append(room_id)
|
room_ids.append(room_id)
|
||||||
|
|
||||||
|
room_ids.sort()
|
||||||
|
|
||||||
# Request the list of rooms
|
# Request the list of rooms
|
||||||
url = "/_synapse/admin/v1/rooms"
|
url = "/_synapse/admin/v1/rooms"
|
||||||
channel = self.make_request(
|
channel = self.make_request(
|
||||||
@ -1360,6 +1362,12 @@ class RoomTestCase(unittest.HomeserverTestCase):
|
|||||||
room_id_2 = self.helper.create_room_as(self.admin_user, tok=self.admin_user_tok)
|
room_id_2 = self.helper.create_room_as(self.admin_user, tok=self.admin_user_tok)
|
||||||
room_id_3 = self.helper.create_room_as(self.admin_user, tok=self.admin_user_tok)
|
room_id_3 = self.helper.create_room_as(self.admin_user, tok=self.admin_user_tok)
|
||||||
|
|
||||||
|
# Also create a list sorted by IDs for properties that are equal (and thus sorted by room_id)
|
||||||
|
sorted_by_room_id_asc = [room_id_1, room_id_2, room_id_3]
|
||||||
|
sorted_by_room_id_asc.sort()
|
||||||
|
sorted_by_room_id_desc = sorted_by_room_id_asc.copy()
|
||||||
|
sorted_by_room_id_desc.reverse()
|
||||||
|
|
||||||
# Set room names in alphabetical order. room 1 -> A, 2 -> B, 3 -> C
|
# Set room names in alphabetical order. room 1 -> A, 2 -> B, 3 -> C
|
||||||
self.helper.send_state(
|
self.helper.send_state(
|
||||||
room_id_1,
|
room_id_1,
|
||||||
@ -1405,41 +1413,42 @@ class RoomTestCase(unittest.HomeserverTestCase):
|
|||||||
_order_test("canonical_alias", [room_id_1, room_id_2, room_id_3])
|
_order_test("canonical_alias", [room_id_1, room_id_2, room_id_3])
|
||||||
_order_test("canonical_alias", [room_id_3, room_id_2, room_id_1], reverse=True)
|
_order_test("canonical_alias", [room_id_3, room_id_2, room_id_1], reverse=True)
|
||||||
|
|
||||||
|
# Note: joined_member counts are sorted in descending order when dir=f
|
||||||
_order_test("joined_members", [room_id_3, room_id_2, room_id_1])
|
_order_test("joined_members", [room_id_3, room_id_2, room_id_1])
|
||||||
_order_test("joined_members", [room_id_1, room_id_2, room_id_3], reverse=True)
|
_order_test("joined_members", [room_id_1, room_id_2, room_id_3], reverse=True)
|
||||||
|
|
||||||
|
# Note: joined_local_member counts are sorted in descending order when dir=f
|
||||||
_order_test("joined_local_members", [room_id_3, room_id_2, room_id_1])
|
_order_test("joined_local_members", [room_id_3, room_id_2, room_id_1])
|
||||||
_order_test(
|
_order_test(
|
||||||
"joined_local_members", [room_id_1, room_id_2, room_id_3], reverse=True
|
"joined_local_members", [room_id_1, room_id_2, room_id_3], reverse=True
|
||||||
)
|
)
|
||||||
|
|
||||||
_order_test("version", [room_id_1, room_id_2, room_id_3])
|
# Note: versions are sorted in descending order when dir=f
|
||||||
_order_test("version", [room_id_1, room_id_2, room_id_3], reverse=True)
|
_order_test("version", sorted_by_room_id_asc, reverse=True)
|
||||||
|
_order_test("version", sorted_by_room_id_desc)
|
||||||
|
|
||||||
_order_test("creator", [room_id_1, room_id_2, room_id_3])
|
_order_test("creator", sorted_by_room_id_asc)
|
||||||
_order_test("creator", [room_id_1, room_id_2, room_id_3], reverse=True)
|
_order_test("creator", sorted_by_room_id_desc, reverse=True)
|
||||||
|
|
||||||
_order_test("encryption", [room_id_1, room_id_2, room_id_3])
|
_order_test("encryption", sorted_by_room_id_asc)
|
||||||
_order_test("encryption", [room_id_1, room_id_2, room_id_3], reverse=True)
|
_order_test("encryption", sorted_by_room_id_desc, reverse=True)
|
||||||
|
|
||||||
_order_test("federatable", [room_id_1, room_id_2, room_id_3])
|
_order_test("federatable", sorted_by_room_id_asc)
|
||||||
_order_test("federatable", [room_id_1, room_id_2, room_id_3], reverse=True)
|
_order_test("federatable", sorted_by_room_id_desc, reverse=True)
|
||||||
|
|
||||||
_order_test("public", [room_id_1, room_id_2, room_id_3])
|
_order_test("public", sorted_by_room_id_asc)
|
||||||
# Different sort order of SQlite and PostreSQL
|
_order_test("public", sorted_by_room_id_desc, reverse=True)
|
||||||
# _order_test("public", [room_id_3, room_id_2, room_id_1], reverse=True)
|
|
||||||
|
|
||||||
_order_test("join_rules", [room_id_1, room_id_2, room_id_3])
|
_order_test("join_rules", sorted_by_room_id_asc)
|
||||||
_order_test("join_rules", [room_id_1, room_id_2, room_id_3], reverse=True)
|
_order_test("join_rules", sorted_by_room_id_desc, reverse=True)
|
||||||
|
|
||||||
_order_test("guest_access", [room_id_1, room_id_2, room_id_3])
|
_order_test("guest_access", sorted_by_room_id_asc)
|
||||||
_order_test("guest_access", [room_id_1, room_id_2, room_id_3], reverse=True)
|
_order_test("guest_access", sorted_by_room_id_desc, reverse=True)
|
||||||
|
|
||||||
_order_test("history_visibility", [room_id_1, room_id_2, room_id_3])
|
_order_test("history_visibility", sorted_by_room_id_asc)
|
||||||
_order_test(
|
_order_test("history_visibility", sorted_by_room_id_desc, reverse=True)
|
||||||
"history_visibility", [room_id_1, room_id_2, room_id_3], reverse=True
|
|
||||||
)
|
|
||||||
|
|
||||||
|
# Note: state_event counts are sorted in descending order when dir=f
|
||||||
_order_test("state_events", [room_id_3, room_id_2, room_id_1])
|
_order_test("state_events", [room_id_3, room_id_2, room_id_1])
|
||||||
_order_test("state_events", [room_id_1, room_id_2, room_id_3], reverse=True)
|
_order_test("state_events", [room_id_1, room_id_2, room_id_3], reverse=True)
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user