diff options
| -rw-r--r-- | changelog.d/10730.bugfix | 1 | ||||
| -rw-r--r-- | synapse/handlers/room_summary.py | 15 | ||||
| -rw-r--r-- | tests/handlers/test_room_summary.py | 18 |
3 files changed, 22 insertions, 12 deletions
diff --git a/changelog.d/10730.bugfix b/changelog.d/10730.bugfix new file mode 100644 index 0000000000..f1612d3c08 --- /dev/null +++ b/changelog.d/10730.bugfix @@ -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. diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 906985c754..d1b6f3253e 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -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) diff --git a/tests/handlers/test_room_summary.py b/tests/handlers/test_room_summary.py index ac800afa7d..449ba89e5a 100644 --- a/tests/handlers/test_room_summary.py +++ b/tests/handlers/test_room_summary.py @@ -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)) |