Various fixups

6 files changed, 68 insertions, 55 deletions

diff --git a/synapse/federation/transport/server/__init__.py b/synapse/federation/transport/server/__init__.py
index 55d2cd0a9a..16663e97ce 100644
--- a/synapse/federation/transport/server/__init__.py
+++ b/synapse/federation/transport/server/__init__.py
@@ -149,7 +149,10 @@ class PublicRoomList(BaseFederationServlet):
             limit = None
 
         data = await self.handler.get_local_public_room_list(
-            limit, since_token, network_tuple=network_tuple, from_federation=True
+            limit,
+            since_token,
+            network_tuple=network_tuple,
+            from_remote_server_name=origin,
         )
         return 200, data
 
@@ -190,7 +193,7 @@ class PublicRoomList(BaseFederationServlet):
             since_token=since_token,
             search_filter=search_filter,
             network_tuple=network_tuple,
-            from_federation=True,
+            from_remote_server_name=origin,
         )
 
         return 200, data
diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py
index 9e2b46699d..1df739aa1f 100644
--- a/synapse/handlers/room_list.py
+++ b/synapse/handlers/room_list.py
@@ -34,7 +34,7 @@ from synapse.api.errors import (
     RequestSendFailed,
     SynapseError,
 )
-from synapse.types import JsonDict, ThirdPartyInstanceID
+from synapse.types import JsonDict, PublicRoom, ThirdPartyInstanceID
 from synapse.util.caches.descriptors import _CacheContext, cached
 from synapse.util.caches.response_cache import ResponseCache
 
@@ -166,30 +166,47 @@ class RoomListHandler:
         # we request one more than wanted to see if there are more pages to come
         probing_limit = limit + 1 if limit is not None else None
 
-        public_rooms = await self.store.get_largest_public_rooms(
+        results = await self.store.get_largest_public_rooms(
             network_tuple,
             search_filter,
             probing_limit,
-            bounds=(
-                [batch_token.last_joined_members, batch_token.last_room_id]
-                if batch_token else None
-            ),
+            bounds=(batch_token.last_joined_members, batch_token.last_room_id)
+            if batch_token
+            else None,
             forwards=forwards,
             ignore_non_federatable=bool(from_remote_server_name),
         )
 
-        for fetch_public_rooms in self._module_api_callbacks.fetch_public_rooms_callbacks:
+        for (
+            fetch_public_rooms
+        ) in self._module_api_callbacks.fetch_public_rooms_callbacks:
             # Ask each module for a list of public rooms given the last_joined_members
             # value from the since token and the probing limit.
             module_public_rooms = await fetch_public_rooms(
-                limit=probing_limit,
-                max_member_count=(
-                    batch_token.last_joined_members
-                    if batch_token else None
-                ),
+                forwards,
+                probing_limit,
+                batch_token.last_joined_members if batch_token else None,
             )
 
             # Insert the module's reported public rooms into the list
+            for new_room in module_public_rooms:
+                inserted = False
+                for i in range(len(results)):
+                    r = results[i]
+                    if (
+                        forwards
+                        and new_room["num_joined_members"] >= r["num_joined_members"]
+                    ) or (
+                        not forwards
+                        and new_room["num_joined_members"] <= r["num_joined_members"]
+                    ):
+                        results.insert(i, new_room)
+                        break
+                if not inserted:
+                    if forwards:
+                        results.append(new_room)
+                    else:
+                        results.insert(0, new_room)
 
         response: JsonDict = {}
         num_results = len(results)
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index ac5be34c6d..16bd966efe 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -79,7 +79,6 @@ from synapse.module_api.callbacks.account_validity_callbacks import (
 )
 from synapse.module_api.callbacks.public_rooms_callbacks import (
     FETCH_PUBLIC_ROOMS_CALLBACK,
-    PublicRoomChunk,
 )
 from synapse.module_api.callbacks.spamchecker_callbacks import (
     CHECK_EVENT_FOR_SPAM_CALLBACK,
@@ -123,6 +122,7 @@ from synapse.types import (
     DomainSpecificString,
     JsonDict,
     JsonMapping,
+    PublicRoom,
     Requester,
     RoomAlias,
     StateMap,
diff --git a/synapse/module_api/callbacks/public_rooms_callbacks.py b/synapse/module_api/callbacks/public_rooms_callbacks.py
index b2a02e3ab8..969b1b1faa 100644
--- a/synapse/module_api/callbacks/public_rooms_callbacks.py
+++ b/synapse/module_api/callbacks/public_rooms_callbacks.py
@@ -14,30 +14,17 @@
 
 import logging
 from typing import Awaitable, Callable, Iterable, List, Optional, Tuple
+from synapse.types import PublicRoom
 
 import attr
 
 logger = logging.getLogger(__name__)
 
 
-@attr.s(auto_attribs=True)
-class PublicRoom:
-    room_id: str
-    name: str
-    topic: str
-    num_joined_members: int
-    canonical_alias: str
-    avatar_url: str
-    world_readable: bool
-    guest_can_join: bool
-    join_rule: str
-    room_type: str
-
-
 # Types for callbacks to be registered via the module api
 FETCH_PUBLIC_ROOMS_CALLBACK = Callable[
-    [int, Optional[Tuple[int, bool]], Optional[dict], Optional[str], Optional[str]],
-    Awaitable[Tuple[Iterable[PublicRoom], bool]],
+    [bool, Optional[int], Optional[int]],
+    Awaitable[Iterable[PublicRoom]],
 ]
 
 
diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py
index 33cb109d17..d81068fe69 100644
--- a/synapse/storage/databases/main/room.py
+++ b/synapse/storage/databases/main/room.py
@@ -62,7 +62,13 @@ from synapse.storage.util.id_generators import (
     MultiWriterIdGenerator,
     StreamIdGenerator,
 )
-from synapse.types import JsonDict, RetentionPolicy, StrCollection, ThirdPartyInstanceID
+from synapse.types import (
+    JsonDict,
+    PublicRoom,
+    RetentionPolicy,
+    StrCollection,
+    ThirdPartyInstanceID,
+)
 from synapse.util import json_encoder
 from synapse.util.caches.descriptors import cached, cachedList
 from synapse.util.stringutils import MXC_REGEX
@@ -369,7 +375,7 @@ class RoomWorkerStore(CacheInvalidationWorkerStore):
         bounds: Optional[Tuple[int, str]],
         forwards: bool,
         ignore_non_federatable: bool = False,
-    ) -> List[Dict[str, Any]]:
+    ) -> List[PublicRoom]:
         """Gets the largest public rooms (where largest is in terms of joined
         members, as tracked in the statistics table).
 
@@ -520,23 +526,23 @@ class RoomWorkerStore(CacheInvalidationWorkerStore):
             "get_largest_public_rooms", _get_largest_public_rooms_txn
         )
 
-        def build_room_entry(room: JsonDict) -> JsonDict:
-            entry = {
-                "room_id": room["room_id"],
-                "name": room["name"],
-                "topic": room["topic"],
-                "canonical_alias": room["canonical_alias"],
-                "num_joined_members": room["joined_members"],
-                "avatar_url": room["avatar"],
-                "world_readable": room["history_visibility"]
-                                  == HistoryVisibility.WORLD_READABLE,
-                "guest_can_join": room["guest_access"] == "can_join",
-                "join_rule": room["join_rules"],
-                "room_type": room["room_type"],
-            }
+        def build_room_entry(room: JsonDict) -> PublicRoom:
+            return PublicRoom(
+                room_id=room["room_id"],
+                name=room["name"],
+                topic=room["topic"],
+                canonical_alias=room["canonical_alias"],
+                num_joined_members=room["joined_members"],
+                avatar_url=room["avatar"],
+                world_readable=room["history_visibility"]
+                == HistoryVisibility.WORLD_READABLE,
+                guest_can_join=room["guest_access"] == "can_join",
+                join_rule=room["join_rules"],
+                room_type=room["room_type"],
+            )
 
             # Filter out Nones – rather omit the field altogether
-            return {k: v for k, v in entry.items() if v is not None}
+            # return {k: v for k, v in entry.items() if v is not None}
 
         return [build_room_entry(r) for r in ret_val]
 
diff --git a/synapse/types/__init__.py b/synapse/types/__init__.py
index 5dce579ec1..c593299a14 100644
--- a/synapse/types/__init__.py
+++ b/synapse/types/__init__.py
@@ -953,17 +953,17 @@ class UserInfo:
     is_shadow_banned: bool
 
 
-class PublicRoomsChunk:
+class PublicRoom(TypedDict):
     room_id: str
-    name: str
-    topic: str
+    name: Optional[str]
+    topic: Optional[str]
     num_joined_members: int
-    canonical_alias: str
-    avatar_url: str
+    canonical_alias: Optional[str]
+    avatar_url: Optional[str]
     world_readable: bool
     guest_can_join: bool
-    join_rule: str
-    room_type: str
+    join_rule: Optional[str]
+    room_type: Optional[str]
 
 
 class UserProfile(TypedDict):