Skip to content
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

Follow Matrix spec for MatrixClient.getRoomMembers arguments #254

Closed
wants to merge 1 commit into from

Conversation

B4dM4n
Copy link
Contributor

@B4dM4n B4dM4n commented Aug 8, 2022

The MatrixClient.getRoomMembers arguments allow to specify multiple membership kind filters, wich are passed to the Matrix server as repeated query arguments to the /rooms/{roomId}/members API.
The spec defines the membership and not_membership arguments as an enum, which can not be repeated.

This is also how Synapse implements this API, by only using the first occurrence of each parameter and ignoring the rest:

$ # Lists all members
$ curl -sS 'http://localhost:8008/_matrix/client/r0/rooms/!FbdTKTZKWpCPuOTyyt:local/members' -H 'Authorization: Bearer <token>' | jq
{
  "chunk": [
    {
      "type": "m.room.member",
      "room_id": "!FbdTKTZKWpCPuOTyyt:local",
      "sender": "@admin:local",
      "content": {
        "membership": "join",
        "displayname": "admin"
      },
      "state_key": "@admin:local",
      "origin_server_ts": 1659696716579,
      "unsigned": {
        "age": 101233
      },
      "event_id": "$ivix5q0enY6NbdWh1k1eQ6misoEJ8rzgqUgLpKOIVbg",
      "user_id": "@admin:local",
      "age": 101233
    },
    {
      "type": "m.room.member",
      "room_id": "!FbdTKTZKWpCPuOTyyt:local",
      "sender": "@admin:local",
      "content": {
        "membership": "invite",
        "displayname": "test"
      },
      "state_key": "@test:local",
      "origin_server_ts": 1659696724300,
      "unsigned": {
        "age": 93512
      },
      "event_id": "$qvpBNoB609YtOKPVqV3n8LY_Oubh8LM-lufxwhXS8YU",
      "user_id": "@admin:local",
      "age": 93512
    }
  ]
}
$ # Only lists joined and ignores invited members
$ curl -sS 'http://localhost:8008/_matrix/client/r0/rooms/!FbdTKTZKWpCPuOTyyt:local/members?membership=join&membership=invite' -H 'Authorization: Bearer <token>' | jq
{
  "chunk": [
    {
      "type": "m.room.member",
      "room_id": "!FbdTKTZKWpCPuOTyyt:local",
      "sender": "@admin:local",
      "content": {
        "membership": "join",
        "displayname": "admin"
      },
      "state_key": "@admin:local",
      "origin_server_ts": 1659696716579,
      "unsigned": {
        "age": 106183
      },
      "event_id": "$ivix5q0enY6NbdWh1k1eQ6misoEJ8rzgqUgLpKOIVbg",
      "user_id": "@admin:local",
      "age": 106183
    }
  ]
}
$ # Lists joined members as expected
$ curl -sS 'http://localhost:8008/_matrix/client/r0/rooms/!FbdTKTZKWpCPuOTyyt:local/members?membership=join' -H 'Authorization: Bearer <token>' | jq
{
  "chunk": [
    {
      "type": "m.room.member",
      "room_id": "!FbdTKTZKWpCPuOTyyt:local",
      "sender": "@admin:local",
      "content": {
        "membership": "join",
        "displayname": "admin"
      },
      "state_key": "@admin:local",
      "origin_server_ts": 1659696716579,
      "unsigned": {
        "age": 113113
      },
      "event_id": "$ivix5q0enY6NbdWh1k1eQ6misoEJ8rzgqUgLpKOIVbg",
      "user_id": "@admin:local",
      "age": 113113
    }
  ]
}
$ # Lists invited members as expected
$ curl -sS 'http://localhost:8008/_matrix/client/r0/rooms/!FbdTKTZKWpCPuOTyyt:local/members?membership=invite' -H 'Authorization: Bearer <token>' | jq
{
  "chunk": [
    {
      "type": "m.room.member",
      "room_id": "!FbdTKTZKWpCPuOTyyt:local",
      "sender": "@admin:local",
      "content": {
        "membership": "invite",
        "displayname": "test"
      },
      "state_key": "@test:local",
      "origin_server_ts": 1659696724300,
      "unsigned": {
        "age": 110940
      },
      "event_id": "$qvpBNoB609YtOKPVqV3n8LY_Oubh8LM-lufxwhXS8YU",
      "user_id": "@admin:local",
      "age": 110940
    }
  ]
}

This PR uses function overloads to deprecate the original function signature and adds a new signature with single membership kind arguments.

Checklist

  • Tests written for all new code
  • Linter has been satisfied
  • Sign-off given on the changes (see CONTRIBUTING.md)

The /rooms/{roomId}/members API only allows to filter for at most one membership
kind and the same goes for not_membership.

This deprecates the function signature with arrays and adds a new
signature with single membership arguments.

Signed-off-by: Fabian Möller <fabianm88@gmail.com>
@turt2live
Copy link
Owner

The spec allows the fields to be specified multiple times. If it's not working, it's a server bug.

@B4dM4n
Copy link
Contributor Author

B4dM4n commented Aug 8, 2022

This is not how I would read the spec.

Methods like POST /_matrix/client/v3/join/{roomIdOrAlias} and POST /_matrix/client/v3/knock/{roomIdOrAlias} explicitly specify the server_name query parameter as type [string], but the GET /_matrix/client/v3/rooms/{roomId}/members query parameters are only defined as enum.

So either the spec is to loose in it's definition or the parameters are not repeatable.

How do you know that the spec allows it?

@turt2live
Copy link
Owner

The spec was written here, which is suffering from the structure not supporting how to reference the same argument multiple times in a sane way. This was specifying matrix-org/matrix-spec-proposals#1227 which was meant to specify it as multiple.

Though, having re-read all this history here I am struggling to see this actually happening. Will investigate further.

@turt2live
Copy link
Owner

I've ended up fixing this a different way which I think will be easier for consumers to use, though is functionally the same idea: #260

Thank you for the PR in any case :)

@turt2live turt2live closed this Sep 21, 2022
@B4dM4n B4dM4n deleted the room-get-members-api branch September 23, 2022 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants