Consider the origin_server_ts of the m.space.child event when ordering rooms. (#10730)

This updates the ordering of the returned events from the spaces
summary API to that defined in MSC2946 (which updates MSC1772).

Previously a step was skipped causing ordering to be inconsistent with
clients.
This commit is contained in:
Patrick Cloke 2021-09-01 12:59:52 -04:00 committed by GitHub
parent d1f1b46c2c
commit 6258730ebe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 22 additions and 12 deletions

1
changelog.d/10730.bugfix Normal file
View file

@ -0,0 +1 @@
Fix a bug where the ordering algorithm was skipping the `origin_server_ts` step in the spaces summary resulting in unstable room orderings.

View file

@ -1139,25 +1139,26 @@ def _is_suggested_child_event(edge_event: EventBase) -> bool:
_INVALID_ORDER_CHARS_RE = re.compile(r"[^\x20-\x7E]")
def _child_events_comparison_key(child: EventBase) -> Tuple[bool, Optional[str], str]:
def _child_events_comparison_key(
child: EventBase,
) -> Tuple[bool, Optional[str], int, str]:
"""
Generate a value for comparing two child events for ordering.
The rules for ordering are supposed to be:
The rules for ordering are:
1. The 'order' key, if it is valid.
2. The 'origin_server_ts' of the 'm.room.create' event.
2. The 'origin_server_ts' of the 'm.space.child' event.
3. The 'room_id'.
But we skip step 2 since we may not have any state from the room.
Args:
child: The event for generating a comparison key.
Returns:
The comparison key as a tuple of:
False if the ordering is valid.
The ordering field.
The 'order' field or None if it is not given or invalid.
The 'origin_server_ts' field.
The room ID.
"""
order = child.content.get("order")
@ -1168,4 +1169,4 @@ def _child_events_comparison_key(child: EventBase) -> Tuple[bool, Optional[str],
order = None
# Items without an order come last.
return (order is None, order, child.room_id)
return (order is None, order, child.origin_server_ts, child.room_id)

View file

@ -35,10 +35,11 @@ from synapse.types import JsonDict, UserID
from tests import unittest
def _create_event(room_id: str, order: Optional[Any] = None):
result = mock.Mock()
def _create_event(room_id: str, order: Optional[Any] = None, origin_server_ts: int = 0):
result = mock.Mock(name=room_id)
result.room_id = room_id
result.content = {}
result.origin_server_ts = origin_server_ts
if order is not None:
result.content["order"] = order
return result
@ -63,10 +64,17 @@ class TestSpaceSummarySort(unittest.TestCase):
self.assertEqual([ev2, ev1], _order(ev1, ev2))
def test_order_origin_server_ts(self):
"""Origin server is a tie-breaker for ordering."""
ev1 = _create_event("!abc:test", origin_server_ts=10)
ev2 = _create_event("!xyz:test", origin_server_ts=30)
self.assertEqual([ev1, ev2], _order(ev1, ev2))
def test_order_room_id(self):
"""Room ID is a tie-breaker for ordering."""
ev1 = _create_event("!abc:test", "abc")
ev2 = _create_event("!xyz:test", "abc")
"""Room ID is a final tie-breaker for ordering."""
ev1 = _create_event("!abc:test")
ev2 = _create_event("!xyz:test")
self.assertEqual([ev1, ev2], _order(ev1, ev2))