Add some rest-level unit tests of new 'published' room creation config key

synapse/handlers/federation.py

@@ -189,7 +189,7 @@ class FederationHandler(BaseHandler):
                 yield self.store.store_room(
                     room_id=event.room_id,
                     room_creator_user_id="",
-                    is_public=False,
+                    published=False,
                 )
             except StoreError:
                 logger.exception("Failed to store room.")
@@ -594,7 +594,7 @@ class FederationHandler(BaseHandler):
                 yield self.store.store_room(
                     room_id=room_id,
                     room_creator_user_id="",
-                    is_public=False
+                    published=False,
                 )
             except:
                 # FIXME

synapse/handlers/message.py

@@ -278,7 +278,7 @@ class MessageHandler(BaseHandler):
             user, pagination_config.get_source_config("presence"), None
         )
 
-        public_room_ids = yield self.store.get_public_room_ids()
+        published_room_ids = yield self.store.get_published_room_ids()
 
         limit = pagin_config.limit
         if limit is None:
@@ -290,7 +290,8 @@ class MessageHandler(BaseHandler):
                 "room_id": event.room_id,
                 "membership": event.membership,
                 "visibility": (
-                    "public" if event.room_id in public_room_ids
+                    # TODO(paul): This should be specified as "published"
+                    "public" if event.room_id in published_room_ids
                     else "private"
                 ),
             }

synapse/handlers/room.py

@@ -77,6 +77,14 @@ class RoomCreationHandler(BaseHandler):
 
         is_public = config.get("visibility", None) == "public"
 
+        # By default, all public-joinable rooms are published. Allow overriding
+        # that decision.
+        # TODO(paul): Specify 'published' key
+        if "published" in config:
+            published = config["published"]
+        else:
+            published = is_public
+
         if room_id:
             # Ensure room_id is the correct type
             room_id_obj = RoomID.from_string(room_id)
@@ -86,7 +94,7 @@ class RoomCreationHandler(BaseHandler):
             yield self.store.store_room(
                 room_id=room_id,
                 room_creator_user_id=user_id,
-                is_public=is_public
+                published=published,
             )
         else:
             # autogen room IDs and try to create it. We may clash, so just
@@ -103,7 +111,7 @@ class RoomCreationHandler(BaseHandler):
                     yield self.store.store_room(
                         room_id=gen_room_id.to_string(),
                         room_creator_user_id=user_id,
-                        is_public=is_public
+                        published=published,
                     )
                     room_id = gen_room_id.to_string()
                     break
@@ -532,8 +540,8 @@ class RoomMemberHandler(BaseHandler):
 class RoomListHandler(BaseHandler):
 
     @defer.inlineCallbacks
-    def get_public_room_list(self):
+    def get_published_rooms(self):
-        chunk = yield self.store.get_rooms(is_public=True)
+        chunk = yield self.store.get_published_rooms()
         results = yield defer.gatherResults(
             [
                 self.store.get_users_in_room(room["room_id"])

synapse/handlers/sync.py

@@ -147,8 +147,7 @@ class SyncHandler(BaseHandler):
             membership_list=[Membership.INVITE, Membership.JOIN]
         )
 
-        # TODO (mjark): Does public mean "published"?
+        published_rooms = yield self.store.get_published_rooms()
-        published_rooms = yield self.store.get_rooms(is_public=True)
         published_room_ids = set(r["room_id"] for r in published_rooms)
 
         rooms = []
@@ -231,8 +230,7 @@ class SyncHandler(BaseHandler):
         rm_handler = self.hs.get_handlers().room_member_handler
         room_ids = yield rm_handler.get_joined_rooms_for_user(sync_config.user)
 
-        # TODO (mjark): Does public mean "published"?
+        published_rooms = yield self.store.get_published_rooms()
-        published_rooms = yield self.store.get_rooms(is_public=True)
         published_room_ids = set(r["room_id"] for r in published_rooms)
 
         room_events, _ = yield self.store.get_room_events_stream(

synapse/rest/client/v1/room.py

@@ -273,12 +273,13 @@ class JoinRoomAliasServlet(ClientV1RestServlet):
 
 # TODO: Needs unit testing
 class PublicRoomListRestServlet(ClientV1RestServlet):
+    # TODO(paul): Can't rename this now but 'v2 ought to call this publishedRooms
     PATTERN = client_path_pattern("/publicRooms$")
 
     @defer.inlineCallbacks
     def on_GET(self, request):
         handler = self.handlers.room_list_handler
-        data = yield handler.get_public_room_list()
+        data = yield handler.get_published_rooms()
         defer.returnValue((200, data))
 
 

synapse/storage/room.py

@@ -34,14 +34,14 @@ OpsLevel = collections.namedtuple(
 class RoomStore(SQLBaseStore):
 
     @defer.inlineCallbacks
-    def store_room(self, room_id, room_creator_user_id, is_public):
+    def store_room(self, room_id, room_creator_user_id, published):
         """Stores a room.
 
         Args:
             room_id (str): The desired room ID, can be None.
             room_creator_user_id (str): The user ID of the room creator.
-            is_public (bool): True to indicate that this room should appear in
+            published (bool): True to indicate that this room should appear in
-            public room lists.
+                published room lists.
         Raises:
             StoreError if the room could not be stored.
         """
@@ -51,7 +51,8 @@ class RoomStore(SQLBaseStore):
                 {
                     "room_id": room_id,
                     "creator": room_creator_user_id,
-                    "is_public": is_public,
+                    # TODO(paul): rename this table in the SQL schema
+                    "is_public": published,
                 },
                 desc="store_room",
             )
@@ -75,22 +76,20 @@ class RoomStore(SQLBaseStore):
             allow_none=True,
         )
 
-    def get_public_room_ids(self):
+    def get_published_room_ids(self):
         return self._simple_select_onecol(
             table="rooms",
             keyvalues={
                 "is_public": True,
             },
             retcol="room_id",
-            desc="get_public_room_ids",
+            desc="get_published_room_ids",
         )
 
     @defer.inlineCallbacks
-    def get_rooms(self, is_public):
+    def get_published_rooms(self):
-        """Retrieve a list of all public rooms.
+        """Retrieve a list of all published rooms.
 
-        Args:
-            is_public (bool): True if the rooms returned should be public.
         Returns:
             A list of room dicts containing at least a "room_id" key, a
             "topic" key if one is set, and a "name" key if one is set
@@ -119,14 +118,14 @@ class RoomStore(SQLBaseStore):
                 " FROM rooms AS r"
                 " LEFT JOIN (%(topic)s) AS t ON t.room_id = r.room_id"
                 " LEFT JOIN (%(name)s) AS n ON n.room_id = r.room_id"
-                " WHERE r.is_public = ?"
+                " WHERE r.is_public"
                 " GROUP BY r.room_id"
             ) % {
                 "topic": topic_subquery,
                 "name": name_subquery,
             }
 
-            txn.execute(sql, (is_public,))
+            txn.execute(sql)
 
             rows = txn.fetchall()
 
@@ -146,7 +145,7 @@ class RoomStore(SQLBaseStore):
             return rows
 
         rows = yield self.runInteraction(
-            "get_rooms", f
+            "get_published_rooms", f
         )
 
         ret = [

tests/rest/client/v1/test_rooms.py

@@ -558,6 +558,70 @@ class RoomsCreateTestCase(RestTestCase):
         self.assertEquals(200, code)
         self.assertTrue("room_id" in response)
 
+        (code, response) = yield self.mock_resource.trigger(
+            "GET",
+            "/publicRooms",
+            "")
+        self.assertEquals(200, code)
+        self.assertEquals({
+            "start": "START",
+            "end": "END",
+            "chunk": [],
+        }, response)
+
+    @defer.inlineCallbacks
+    def test_post_room_visibility_public_key(self):
+        (code, response) = yield self.mock_resource.trigger(
+            "POST",
+            "/createRoom",
+            '{"visibility":"public", '
+            '"room_alias_name": "my-alias-test"}')
+        self.assertEquals(200, code)
+        self.assertTrue("room_id" in response)
+
+        room_id = response["room_id"]
+
+        (code, response) = yield self.mock_resource.trigger(
+            "GET",
+            "/publicRooms",
+            "")
+        self.assertEquals(200, code)
+        self.assertEquals({
+            "chunk": [
+                {
+                    "room_id": room_id,
+                    "name": None,
+                    "topic": None,
+                    "num_joined_members": 1,
+                    "aliases": ["#my-alias-test:red"],
+                },
+            ],
+            "start": "START",
+            "end": "END",
+        }, response)
+
+    @defer.inlineCallbacks
+    def test_post_room_visibility_public_unpublished_key(self):
+        (code, response) = yield self.mock_resource.trigger(
+            "POST",
+            "/createRoom",
+            '{"visibility":"public", '
+            '"room_alias_name": "my-alias-test", '
+            '"published": false}')
+        self.assertEquals(200, code)
+        self.assertTrue("room_id" in response)
+
+        (code, response) = yield self.mock_resource.trigger(
+            "GET",
+            "/publicRooms",
+            "")
+        self.assertEquals(200, code)
+        self.assertEquals({
+            "chunk": [],
+            "start": "START",
+            "end": "END",
+        }, response)
+
     @defer.inlineCallbacks
     def test_post_room_custom_key(self):
         # POST with custom config keys, expect new room id

tests/storage/test_room.py

@@ -39,7 +39,7 @@ class RoomStoreTestCase(unittest.TestCase):
 
         yield self.store.store_room(self.room.to_string(),
             room_creator_user_id=self.u_creator.to_string(),
-            is_public=True
+            published=True,
         )
 
     @defer.inlineCallbacks
@@ -52,10 +52,10 @@ class RoomStoreTestCase(unittest.TestCase):
         )
 
     @defer.inlineCallbacks
-    def test_get_rooms(self):
+    def test_get_published_rooms(self):
-        # get_rooms does an INNER JOIN on the room_aliases table :(
+        # get_published_rooms does an INNER JOIN on the room_aliases table :(
 
-        rooms = yield self.store.get_rooms(is_public=True)
+        rooms = yield self.store.get_published_rooms()
         # Should be empty before we add the alias
         self.assertEquals([], rooms)
 
@@ -65,7 +65,7 @@ class RoomStoreTestCase(unittest.TestCase):
             servers=["test"]
         )
 
-        rooms = yield self.store.get_rooms(is_public=True)
+        rooms = yield self.store.get_published_rooms()
 
         self.assertEquals(1, len(rooms))
         self.assertEquals({
@@ -91,7 +91,7 @@ class RoomEventsStoreTestCase(unittest.TestCase):
 
         yield self.store.store_room(self.room.to_string(),
             room_creator_user_id="@creator:text",
-            is_public=True
+            published=False,
         )
 
     @defer.inlineCallbacks