Sort child events according to MSC1772 for the spaces summary API. (#9954)

This should help ensure that equivalent results are achieved between
homeservers querying for the summary of a space.

This implements modified MSC1772 rules, according to MSC2946.
The different is that the origin_server_ts of the m.room.create event
is not used as a tie-breaker since this might not be known if the
homeserver is not part of the room.
This commit is contained in:
Patrick Cloke 2021-05-11 12:57:39 -04:00 committed by GitHub
parent f4833e0c06
commit 27c375f812
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 151 additions and 2 deletions

1
changelog.d/9954.feature Normal file
View File

@ -0,0 +1 @@
Update support for [MSC2946](https://github.com/matrix-org/matrix-doc/pull/2946): Spaces Summary.

View File

@ -14,6 +14,7 @@
import itertools import itertools
import logging import logging
import re
from collections import deque from collections import deque
from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Set, Tuple, cast from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Set, Tuple, cast
@ -226,6 +227,23 @@ class SpaceSummaryHandler:
suggested_only: bool, suggested_only: bool,
max_children: Optional[int], max_children: Optional[int],
) -> Tuple[Sequence[JsonDict], Sequence[JsonDict]]: ) -> Tuple[Sequence[JsonDict], Sequence[JsonDict]]:
"""
Generate a room entry and a list of event entries for a given room.
Args:
requester: The requesting user, or None if this is over federation.
room_id: The room ID to summarize.
suggested_only: True if only suggested children should be returned.
Otherwise, all children are returned.
max_children: The maximum number of children to return for this node.
Returns:
A tuple of:
An iterable of a single value of the room.
An iterable of the sorted children events. This may be limited
to a maximum size or may include all children.
"""
if not await self._is_room_accessible(room_id, requester): if not await self._is_room_accessible(room_id, requester):
return (), () return (), ()
@ -357,6 +375,18 @@ class SpaceSummaryHandler:
return room_entry return room_entry
async def _get_child_events(self, room_id: str) -> Iterable[EventBase]: async def _get_child_events(self, room_id: str) -> Iterable[EventBase]:
"""
Get the child events for a given room.
The returned results are sorted for stability.
Args:
room_id: The room id to get the children of.
Returns:
An iterable of sorted child events.
"""
# look for child rooms/spaces. # look for child rooms/spaces.
current_state_ids = await self._store.get_current_state_ids(room_id) current_state_ids = await self._store.get_current_state_ids(room_id)
@ -370,8 +400,9 @@ class SpaceSummaryHandler:
] ]
) )
# filter out any events without a "via" (which implies it has been redacted) # filter out any events without a "via" (which implies it has been redacted),
return (e for e in events if _has_valid_via(e)) # and order to ensure we return stable results.
return sorted(filter(_has_valid_via, events), key=_child_events_comparison_key)
@attr.s(frozen=True, slots=True) @attr.s(frozen=True, slots=True)
@ -397,3 +428,39 @@ def _is_suggested_child_event(edge_event: EventBase) -> bool:
return True return True
logger.debug("Ignorning not-suggested child %s", edge_event.state_key) logger.debug("Ignorning not-suggested child %s", edge_event.state_key)
return False return False
# Order may only contain characters in the range of \x20 (space) to \x7F (~).
_INVALID_ORDER_CHARS_RE = re.compile(r"[^\x20-\x7F]")
def _child_events_comparison_key(child: EventBase) -> Tuple[bool, Optional[str], str]:
"""
Generate a value for comparing two child events for ordering.
The rules for ordering are supposed to be:
1. The 'order' key, if it is valid.
2. The 'origin_server_ts' of the 'm.room.create' 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 room ID.
"""
order = child.content.get("order")
# If order is not a string or doesn't meet the requirements, ignore it.
if not isinstance(order, str):
order = None
elif len(order) > 50 or _INVALID_ORDER_CHARS_RE.search(order):
order = None
# Items without an order come last.
return (order is None, order, child.room_id)

View File

@ -0,0 +1,81 @@
# Copyright 2021 The Matrix.org Foundation C.I.C.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import Any, Optional
from unittest import mock
from synapse.handlers.space_summary import _child_events_comparison_key
from tests import unittest
def _create_event(room_id: str, order: Optional[Any] = None):
result = mock.Mock()
result.room_id = room_id
result.content = {}
if order is not None:
result.content["order"] = order
return result
def _order(*events):
return sorted(events, key=_child_events_comparison_key)
class TestSpaceSummarySort(unittest.TestCase):
def test_no_order_last(self):
"""An event with no ordering is placed behind those with an ordering."""
ev1 = _create_event("!abc:test")
ev2 = _create_event("!xyz:test", "xyz")
self.assertEqual([ev2, ev1], _order(ev1, ev2))
def test_order(self):
"""The ordering should be used."""
ev1 = _create_event("!abc:test", "xyz")
ev2 = _create_event("!xyz:test", "abc")
self.assertEqual([ev2, ev1], _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")
self.assertEqual([ev1, ev2], _order(ev1, ev2))
def test_invalid_ordering_type(self):
"""Invalid orderings are considered the same as missing."""
ev1 = _create_event("!abc:test", 1)
ev2 = _create_event("!xyz:test", "xyz")
self.assertEqual([ev2, ev1], _order(ev1, ev2))
ev1 = _create_event("!abc:test", {})
self.assertEqual([ev2, ev1], _order(ev1, ev2))
ev1 = _create_event("!abc:test", [])
self.assertEqual([ev2, ev1], _order(ev1, ev2))
ev1 = _create_event("!abc:test", True)
self.assertEqual([ev2, ev1], _order(ev1, ev2))
def test_invalid_ordering_value(self):
"""Invalid orderings are considered the same as missing."""
ev1 = _create_event("!abc:test", "foo\n")
ev2 = _create_event("!xyz:test", "xyz")
self.assertEqual([ev2, ev1], _order(ev1, ev2))
ev1 = _create_event("!abc:test", "a" * 51)
self.assertEqual([ev2, ev1], _order(ev1, ev2))