-
-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarify the meaning of "public rooms" in the room directory #2104
base: main
Are you sure you want to change the base?
Changes from all commits
5e9f6dd
5be34d8
03c96e5
2933ab5
264f8fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Rooms published in `/publicRooms` don't necessarily have `public` join rules or `world_readable` history visibility. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Rooms published in `/publicRooms` don't necessarily have `public` join rules or `world_readable` history visibility. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,16 +13,20 @@ | |
# limitations under the License. | ||
openapi: 3.1.0 | ||
info: | ||
title: Matrix Federation Public Rooms API | ||
title: Matrix Federation Room Directory API | ||
version: 1.0.0 | ||
paths: | ||
/publicRooms: | ||
get: | ||
summary: Get all the public rooms for a homeserver | ||
summary: Lists the server's published room directory. | ||
description: |- | ||
Gets all the public rooms for the homeserver. This should not return | ||
rooms that are listed on another homeserver's directory, just those | ||
listed on the receiving homeserver's directory. | ||
Lists the server's published room directory. | ||
|
||
This API returns paginated responses. The rooms are ordered by the number | ||
of joined members, with the largest rooms first. | ||
|
||
This should not return rooms that are listed on another homeserver's directory, | ||
just those listed on the receiving homeserver's directory. | ||
operationId: getPublicRooms | ||
security: | ||
- signedRequest: [] | ||
|
@@ -62,21 +66,18 @@ paths: | |
type: string | ||
responses: | ||
"200": | ||
description: The public room list for the homeserver. | ||
description: A list of the published rooms on the server. | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ../client-server/definitions/public_rooms_response.yaml | ||
post: | ||
summary: Gets the public rooms on the server with optional filter. | ||
summary: Lists the server's published room directory with an optional filter | ||
description: |- | ||
Lists the public rooms on the server, with optional filter. | ||
Lists the server's published room directory with an optional filter. | ||
|
||
This API returns paginated responses. The rooms are ordered by the number | ||
of joined members, with the largest rooms first. | ||
|
||
Note that this endpoint receives and returns the same format that is seen | ||
in the Client-Server API's `POST /publicRooms` endpoint. | ||
Comment on lines
-77
to
-79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This felt slightly confusing because the CS endpoint has a |
||
operationId: queryPublicRooms | ||
security: | ||
- signedRequest: [] | ||
|
@@ -147,69 +148,11 @@ paths: | |
required: true | ||
responses: | ||
"200": | ||
description: A list of the rooms on the server. | ||
description: A filtered list of the published rooms on the server. | ||
content: | ||
application/json: | ||
schema: | ||
type: object | ||
description: A list of the rooms on the server. | ||
required: | ||
- chunk | ||
Comment on lines
-156
to
-157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAICT this is the only difference to |
||
properties: | ||
chunk: | ||
title: PublicRoomsChunks | ||
type: array | ||
description: A paginated chunk of public rooms. | ||
items: | ||
allOf: | ||
- $ref: ../client-server/definitions/public_rooms_chunk.yaml | ||
- type: object | ||
properties: | ||
# Override description of join_rule | ||
join_rule: | ||
type: string | ||
description: |- | ||
The room's join rule. When not present, the room is assumed to | ||
be `public`. Note that rooms with `invite` join rules are not | ||
expected here, but rooms with `knock` rules are given their | ||
near-public nature. | ||
next_batch: | ||
type: string | ||
description: |- | ||
A pagination token for the response. The absence of this token | ||
means there are no more results to fetch and the client should | ||
stop paginating. | ||
prev_batch: | ||
type: string | ||
description: |- | ||
A pagination token that allows fetching previous results. The | ||
absence of this token means there are no results before this | ||
batch, i.e. this is the first batch. | ||
total_room_count_estimate: | ||
type: integer | ||
description: |- | ||
An estimate on the total number of public rooms, if the | ||
server has an estimate. | ||
examples: | ||
response: | ||
value: { | ||
"chunk": [ | ||
{ | ||
"avatar_url": "mxc://bleecker.street/CHEDDARandBRIE", | ||
"guest_can_join": false, | ||
"name": "CHEESE", | ||
"num_joined_members": 37, | ||
"room_id": "!ol19s:bleecker.street", | ||
"topic": "Tasty tasty cheese", | ||
"world_readable": true, | ||
"join_rule": "public", | ||
"room_type": "m.space" | ||
} | ||
], | ||
"next_batch": "p190q", | ||
"prev_batch": "p1902", | ||
"total_room_count_estimate": 115 | ||
} | ||
$ref: ../client-server/definitions/public_rooms_response.yaml | ||
servers: | ||
- url: "{protocol}://{hostname}{basePath}" | ||
variables: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This came in through fa6cc8a but I cannot find any references to it in matrix-org/matrix-spec-proposals#2403. I think it is trying to say that knockable rooms can be made visible in the directory? If so, I think this should rather be moved to where the meaning of visibility is described because here it makes it seem as if servers should do this automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that it's more a clarification about which join rules clients can expect to find here.
I agree that this should be next to the meaning of visibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have added a phrase about knockable rooms in the warning box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anoadragon453 says:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the context @anoadragon453! 🙏
So when responding to
/publicRooms
, Synapse first retrieves all rooms withpublic
visibility but then applies an additional filter to only include these cases:I'm struggling a bit with understanding how the room directory is meant to work. Other than the slightly unclear statement "rooms with
knock
rules are given their near-public nature", the spec doesn't mention any restrictions on what rooms can be included in/publicRooms
. Therefore, my assumption was that rooms with apublic
visibility would show up regardless of their join rules or history visibility.In fact, MSC2403 seems to only define
join_rule
as a new field in the response of/publicRooms
. It doesn't appear to specify any filtering and instead says:That matches my assumption but not what Synapse actually does.
😵💫
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, the room is only considered if
visibility
ispublic
when modifying the public rooms list withPUT /_matrix/client/v3/directory/list/room/{roomId}
.To speak to Synapse's implementation, the
join_rule
restriction within the query was only added during a performance improvement change. I could see this requirement being useful, in that if a room is set toprivate
, it would immediately no longer show in the public rooms list (as it is not publicly-joinable). While the spec may not require homeservers to manage their lists in that way, Synapse can choose to do so.Regardless, it's clear that Synapse's filtering of its public rooms list here should not influence the spec's definition of a public room. So I think we can ignore this and consider a
join_rule
ofpublic
the only variant that constitutes a room as public.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, interesting. This sounds sensible. Maybe we could add this additional filtering as a MAY hint into the spec?
You mean
visibility
ofpublic
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable to me.
Hah, yes sorry.
join_rules
aren't mandated affect the filtering. Terminology!