Replace /admin/v1/users_paginate endpoint with /admin/v2/users #5925

awesome-manuel · 2019-08-28T14:58:52Z

Fixes #5494 and #2522.
Needs to be tested for #3666.

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file
Pull request includes a sign off

awesome-manuel · 2019-08-30T12:33:10Z

@richvdh The pagination API is not 100% compatible to the normal users API. Pagination returns a list of users and the total amount as a separate field whereas the users API just returns a list.
We probably don't want to change the users API since this one is actually used by current clients.

richvdh · 2019-08-30T13:26:56Z

might be a case for /admin/v2/users which solves the various problems with the current endpoints.

awesome-manuel · 2019-09-16T10:58:30Z

@richvdh any idea why buildkite does not complete?

richvdh

Sorry for the very slow review on this! I've got some suggestions, mostly around the shape of the API.

The current API is a disaster, so I'm treating this basically as a fresh start, hence some of my comments. But also: because some of the changes I'm asking for will be incompatible, I suggest it goes onto /admin/v2/users, rather than changing the existing endpoint.

docs/admin_api/user_admin_api.rst

synapse/rest/admin/users.py

changelog.d/5925.removal

docs/admin_api/user_admin_api.rst

richvdh · 2019-10-03T12:15:59Z

any idea why buildkite does not complete?

buildkite doesn't run on contributed PRs until someone pushes the button. It's something we're working on fixing, but haven't yet.

awesome-manuel · 2019-10-03T16:00:02Z

Sorry for the very slow review on this! I've got some suggestions, mostly around the shape of the API.

The current API is a disaster, so I'm treating this basically as a fresh start, hence some of my comments. But also: because some of the changes I'm asking for will be incompatible, I suggest it goes onto /admin/v2/users, rather than changing the existing endpoint.

I totally agree, but keeping the old API implies keeping some ugly methods in DataStore. My suggestion is to at least drop the pagination API from v1, since it never worked as intended anyway.

synapse/rest/admin/v1/__init__.py

docs/admin_api/user_admin_api.rst

richvdh

My suggestion is to at least drop the pagination API from v1, since it never worked as intended anyway.

I'm happy with that.

I'm afraid I can't really review this as it stands, due to the size of the patch. Sorry - please can you undo the move into a v1 package?

richvdh · 2019-10-10T11:47:49Z

btw @awesome-manuel: it might be easier for you to drop into #synapse-dev:matrix.org if you'd like to discuss these changes?

awesome-manuel · 2019-10-18T07:52:24Z

My suggestion is to at least drop the pagination API from v1, since it never worked as intended anyway.

I'm happy with that.

I'm afraid I can't really review this as it stands, due to the size of the patch. Sorry - please can you undo the move into a v1 package?

Yeah, that's a limitation of GitHub. You can have a look at each individual commit here: https://github.com/Awesome-Technologies/synapse/commits/user_admin
Each commit is self-contained (e.g. does not break anything). Of course you can't comment on each individual commit. Internally we use Gerrit, it's a huge improvement for the reviewer, though requires a bit more learning from the reviewee.

erikjohnston · 2019-10-18T09:56:23Z

@awesome-manuel I'm afraid I just merged #6196 which ports the synapse.rest.admin package to use async/await, which is the cause for all the conflicts. The change is fairly mechanical, @defer.inlineCallbacks -> async def and yield -> await, but I'm happy to resolve the merge conflict and push here if it turns out to be more difficult to resolve than that

awesome-manuel · 2019-10-23T08:00:59Z

@awesome-manuel I'm afraid I just merged #6196 which ports the synapse.rest.admin package to use async/await, which is the cause for all the conflicts. The change is fairly mechanical, @defer.inlineCallbacks -> async def and yield -> await, but I'm happy to resolve the merge conflict and push here if it turns out to be more difficult to resolve than that

I think I fixed all of these. Can someone please trigger Buildkite?

old

richvdh

looks like there is a lint failure from CI as well.

synapse/rest/admin/users.py

synapse/rest/admin/__init__.py

awesome-manuel · 2019-11-20T15:18:34Z

What do you think about the @deprecation decorator to mark "old" functions?
https://pypi.org/project/deprecation/

richvdh

this is definitely shaping up. I'm afraid I've still got a few requests though.

docs/admin_api/user_admin_api.rst

synapse/rest/admin/users.py

synapse/storage/_base.py

synapse/storage/data_stores/main/__init__.py

richvdh · 2019-12-04T14:40:08Z

Agreed. Would you prefer a fixup commit upon the existing ones, or changing the relevant commits but without rebase?

I'd prefer additional fixup commits. I'll squash them all down when I merge.

docs/admin_api/user_admin_api.rst

synapse/storage/_base.py

docs/admin_api/user_admin_api.rst

synapse/storage/data_stores/main/__init__.py

richvdh

hurrah! 🍾 🚢 🎉

richvdh · 2019-12-05T18:12:39Z

Thanks for your perseverence @awesome-manuel !

awesome-manuel · 2019-12-05T18:48:29Z

I definitely appreciate your input, @richvdh

Synapse 1.7.0rc1 (2019-12-09) ============================= Features -------- - Implement per-room message retention policies. ([\#5815](#5815), [\#6436](#6436)) - Add etag and count fields to key backup endpoints to help clients guess if there are new keys. ([\#5858](#5858)) - Add `/admin/v2/users` endpoint with pagination. Contributed by Awesome Technologies Innovationslabor GmbH. ([\#5925](#5925)) - Require User-Interactive Authentication for `/account/3pid/add`, meaning the user's password will be required to add a third-party ID to their account. ([\#6119](#6119)) - Implement the `/_matrix/federation/unstable/net.atleastfornow/state/<context>` API as drafted in MSC2314. ([\#6176](#6176)) - Configure privacy-preserving settings by default for the room directory. ([\#6354](#6354)) - Add ephemeral messages support by partially implementing [MSC2228](matrix-org/matrix-spec-proposals#2228). ([\#6409](#6409)) - Add support for [MSC 2367](matrix-org/matrix-spec-proposals#2367), which allows specifying a reason on all membership events. ([\#6434](#6434)) Bugfixes -------- - Transfer non-standard power levels on room upgrade. ([\#6237](#6237)) - Fix error from the Pillow library when uploading RGBA images. ([\#6241](#6241)) - Correctly apply the event filter to the `state`, `events_before` and `events_after` fields in the response to `/context` requests. ([\#6329](#6329)) - Fix caching devices for remote users when using workers, so that we don't attempt to refetch (and potentially fail) each time a user requests devices. ([\#6332](#6332)) - Prevent account data syncs getting lost across TCP replication. ([\#6333](#6333)) - Fix bug: TypeError in `register_user()` while using LDAP auth module. ([\#6406](#6406)) - Fix an intermittent exception when handling read-receipts. ([\#6408](#6408)) - Fix broken guest registration when there are existing blocks of numeric user IDs. ([\#6420](#6420)) - Fix startup error when http proxy is defined. ([\#6421](#6421)) - Fix error when using synapse_port_db on a vanilla synapse db. ([\#6449](#6449)) - Fix uploading multiple cross signing signatures for the same user. ([\#6451](#6451)) - Fix bug which lead to exceptions being thrown in a loop when a cross-signed device is deleted. ([\#6462](#6462)) - Fix `synapse_port_db` not exiting with a 0 code if something went wrong during the port process. ([\#6470](#6470)) - Improve sanity-checking when receiving events over federation. ([\#6472](#6472)) - Fix inaccurate per-block Prometheus metrics. ([\#6491](#6491)) - Fix small performance regression for sending invites. ([\#6493](#6493)) - Back out cross-signing code added in Synapse 1.5.0, which caused a performance regression. ([\#6494](#6494)) Improved Documentation ---------------------- - Update documentation and variables in user contributed systemd reference file. ([\#6369](#6369), [\#6490](#6490)) - Fix link in the user directory documentation. ([\#6388](#6388)) - Add build instructions to the docker readme. ([\#6390](#6390)) - Switch Ubuntu package install recommendation to use python3 packages in INSTALL.md. ([\#6443](#6443)) - Write some docs for the quarantine_media api. ([\#6458](#6458)) - Convert CONTRIBUTING.rst to markdown (among other small fixes). ([\#6461](#6461)) Deprecations and Removals ------------------------- - Remove admin/v1/users_paginate endpoint. Contributed by Awesome Technologies Innovationslabor GmbH. ([\#5925](#5925)) - Remove fallback for federation with old servers which lack the /federation/v1/state_ids API. ([\#6488](#6488)) Internal Changes ---------------- - Add benchmarks for structured logging and improve output performance. ([\#6266](#6266)) - Improve the performance of outputting structured logging. ([\#6322](#6322)) - Refactor some code in the event authentication path for clarity. ([\#6343](#6343), [\#6468](#6468), [\#6480](#6480)) - Clean up some unnecessary quotation marks around the codebase. ([\#6362](#6362)) - Complain on startup instead of 500'ing during runtime when `public_baseurl` isn't set when necessary. ([\#6379](#6379)) - Add a test scenario to make sure room history purges don't break `/messages` in the future. ([\#6392](#6392)) - Clarifications for the email configuration settings. ([\#6423](#6423)) - Add more tests to the blacklist when running in worker mode. ([\#6429](#6429)) - Refactor data store layer to support multiple databases in the future. ([\#6454](#6454), [\#6464](#6464), [\#6469](#6469), [\#6487](#6487)) - Port synapse.rest.client.v1 to async/await. ([\#6482](#6482)) - Port synapse.rest.client.v2_alpha to async/await. ([\#6483](#6483)) - Port SyncHandler to async/await. ([\#6484](#6484))

Synapse 1.7.0 (2019-12-13) ========================== This release changes the default settings so that only local authenticated users can query the server's room directory. See the [upgrade notes](UPGRADE.rst#upgrading-to-v170) for details. Support for SQLite versions before 3.11 is now deprecated. A future release will refuse to start if used with an SQLite version before 3.11. Administrators are reminded that SQLite should not be used for production instances. Instructions for migrating to Postgres are available [here](docs/postgres.md). A future release of synapse will, by default, disable federation for servers using SQLite. No significant changes since 1.7.0rc2. Synapse 1.7.0rc2 (2019-12-11) ============================= Bugfixes -------- - Fix incorrect error message for invalid requests when setting user's avatar URL. ([\#6497](#6497)) - Fix support for SQLite 3.7. ([\#6499](#6499)) - Fix regression where sending email push would not work when using a pusher worker. ([\#6507](#6507), [\#6509](#6509)) Synapse 1.7.0rc1 (2019-12-09) ============================= Features -------- - Implement per-room message retention policies. ([\#5815](#5815), [\#6436](#6436)) - Add etag and count fields to key backup endpoints to help clients guess if there are new keys. ([\#5858](#5858)) - Add `/admin/v2/users` endpoint with pagination. Contributed by Awesome Technologies Innovationslabor GmbH. ([\#5925](#5925)) - Require User-Interactive Authentication for `/account/3pid/add`, meaning the user's password will be required to add a third-party ID to their account. ([\#6119](#6119)) - Implement the `/_matrix/federation/unstable/net.atleastfornow/state/<context>` API as drafted in MSC2314. ([\#6176](#6176)) - Configure privacy-preserving settings by default for the room directory. ([\#6355](#6355)) - Add ephemeral messages support by partially implementing [MSC2228](matrix-org/matrix-spec-proposals#2228). ([\#6409](#6409)) - Add support for [MSC 2367](matrix-org/matrix-spec-proposals#2367), which allows specifying a reason on all membership events. ([\#6434](#6434)) Bugfixes -------- - Transfer non-standard power levels on room upgrade. ([\#6237](#6237)) - Fix error from the Pillow library when uploading RGBA images. ([\#6241](#6241)) - Correctly apply the event filter to the `state`, `events_before` and `events_after` fields in the response to `/context` requests. ([\#6329](#6329)) - Fix caching devices for remote users when using workers, so that we don't attempt to refetch (and potentially fail) each time a user requests devices. ([\#6332](#6332)) - Prevent account data syncs getting lost across TCP replication. ([\#6333](#6333)) - Fix bug: TypeError in `register_user()` while using LDAP auth module. ([\#6406](#6406)) - Fix an intermittent exception when handling read-receipts. ([\#6408](#6408)) - Fix broken guest registration when there are existing blocks of numeric user IDs. ([\#6420](#6420)) - Fix startup error when http proxy is defined. ([\#6421](#6421)) - Fix error when using synapse_port_db on a vanilla synapse db. ([\#6449](#6449)) - Fix uploading multiple cross signing signatures for the same user. ([\#6451](#6451)) - Fix bug which lead to exceptions being thrown in a loop when a cross-signed device is deleted. ([\#6462](#6462)) - Fix `synapse_port_db` not exiting with a 0 code if something went wrong during the port process. ([\#6470](#6470)) - Improve sanity-checking when receiving events over federation. ([\#6472](#6472)) - Fix inaccurate per-block Prometheus metrics. ([\#6491](#6491)) - Fix small performance regression for sending invites. ([\#6493](#6493)) - Back out cross-signing code added in Synapse 1.5.0, which caused a performance regression. ([\#6494](#6494)) Improved Documentation ---------------------- - Update documentation and variables in user contributed systemd reference file. ([\#6369](#6369), [\#6490](#6490)) - Fix link in the user directory documentation. ([\#6388](#6388)) - Add build instructions to the docker readme. ([\#6390](#6390)) - Switch Ubuntu package install recommendation to use python3 packages in INSTALL.md. ([\#6443](#6443)) - Write some docs for the quarantine_media api. ([\#6458](#6458)) - Convert CONTRIBUTING.rst to markdown (among other small fixes). ([\#6461](#6461)) Deprecations and Removals ------------------------- - Remove admin/v1/users_paginate endpoint. Contributed by Awesome Technologies Innovationslabor GmbH. ([\#5925](#5925)) - Remove fallback for federation with old servers which lack the /federation/v1/state_ids API. ([\#6488](#6488)) Internal Changes ---------------- - Add benchmarks for structured logging and improve output performance. ([\#6266](#6266)) - Improve the performance of outputting structured logging. ([\#6322](#6322)) - Refactor some code in the event authentication path for clarity. ([\#6343](#6343), [\#6468](#6468), [\#6480](#6480)) - Clean up some unnecessary quotation marks around the codebase. ([\#6362](#6362)) - Complain on startup instead of 500'ing during runtime when `public_baseurl` isn't set when necessary. ([\#6379](#6379)) - Add a test scenario to make sure room history purges don't break `/messages` in the future. ([\#6392](#6392)) - Clarifications for the email configuration settings. ([\#6423](#6423)) - Add more tests to the blacklist when running in worker mode. ([\#6429](#6429)) - Refactor data store layer to support multiple databases in the future. ([\#6454](#6454), [\#6464](#6464), [\#6469](#6469), [\#6487](#6487)) - Port synapse.rest.client.v1 to async/await. ([\#6482](#6482)) - Port synapse.rest.client.v2_alpha to async/await. ([\#6483](#6483)) - Port SyncHandler to async/await. ([\#6484](#6484))

richvdh · 2019-12-16T12:04:35Z

this seems to be broken on (at least) postgres: #6552. I guess we should have written some tests :/

Related: #8334 Deprecated in: #9429 - Synapse 1.28.0 (2021-02-25) `GET /_synapse/admin/v1/users/<user_id>` has no - unit tests - documentation API in v2 is available (#5925 - 12/2019, v1.7.0). API is misleading. It expects `user_id` and returns a list of all users. Signed-off-by: Dirk Klimpel dirk@klimpel.org

* commit '649b6bc08': Replace /admin/v1/users_paginate endpoint with /admin/v2/users (#5925)

awesome-manuel changed the base branch from master to develop August 28, 2019 14:59

awesome-manuel force-pushed the user_admin branch from 10c882f to 9d49edd Compare August 28, 2019 15:11

awesome-manuel force-pushed the user_admin branch from 9d49edd to 2108570 Compare August 30, 2019 13:39

richvdh requested a review from a team September 17, 2019 09:57

awesome-manuel force-pushed the user_admin branch from 2108570 to a9dbcea Compare October 1, 2019 12:29

richvdh suggested changes Oct 3, 2019

View reviewed changes

awesome-manuel force-pushed the user_admin branch from a9dbcea to c118a1c Compare October 3, 2019 17:55

richvdh self-assigned this Oct 4, 2019

richvdh reviewed Oct 10, 2019

View reviewed changes

synapse/rest/admin/v1/__init__.py Outdated Show resolved Hide resolved

docs/admin_api/user_admin_api.rst Outdated Show resolved Hide resolved

richvdh previously requested changes Oct 10, 2019

View reviewed changes

awesome-manuel force-pushed the user_admin branch from c118a1c to e2663c8 Compare October 23, 2019 07:43

richvdh suggested changes Oct 30, 2019

View reviewed changes

synapse/rest/admin/users.py Outdated Show resolved Hide resolved

synapse/rest/admin/__init__.py Outdated Show resolved Hide resolved

awesome-manuel mentioned this pull request Oct 31, 2019

Move admin endpoints into separate files #6308

Merged

3 tasks

awesome-manuel force-pushed the user_admin branch from e2663c8 to 70b9ace Compare November 20, 2019 13:33

awesome-manuel force-pushed the user_admin branch 2 times, most recently from 23d7618 to d63cb72 Compare November 21, 2019 08:33

awesome-manuel requested a review from richvdh November 21, 2019 08:33

awesome-manuel force-pushed the user_admin branch from d63cb72 to b3b6d30 Compare November 26, 2019 09:13

richvdh suggested changes Nov 28, 2019

View reviewed changes

docs/admin_api/user_admin_api.rst Show resolved Hide resolved

synapse/rest/admin/users.py Outdated Show resolved Hide resolved

synapse/storage/_base.py Show resolved Hide resolved

synapse/storage/_base.py Outdated Show resolved Hide resolved

awesome-manuel commented Dec 4, 2019

View reviewed changes

synapse/storage/data_stores/main/__init__.py Show resolved Hide resolved

awesome-manuel added 2 commits December 4, 2019 15:58

Fix review comments

ac8757d

Fix runtime error

d0f4dfd

richvdh mentioned this pull request Dec 4, 2019

Move things out of SQLBaseStore #6454

Merged

richvdh suggested changes Dec 4, 2019

View reviewed changes

docs/admin_api/user_admin_api.rst Outdated Show resolved Hide resolved

synapse/storage/_base.py Outdated Show resolved Hide resolved

docs/admin_api/user_admin_api.rst Outdated Show resolved Hide resolved

Fix comments

b9f6992

richvdh suggested changes Dec 4, 2019

View reviewed changes

docs/admin_api/user_admin_api.rst Show resolved Hide resolved

Fix search expression for users

0df165d

richvdh reviewed Dec 4, 2019

View reviewed changes

synapse/storage/data_stores/main/__init__.py Outdated Show resolved Hide resolved

Use single quotes instead of double quotes

147a9a9

awesome-manuel force-pushed the user_admin branch from 0c324bb to 147a9a9 Compare December 4, 2019 18:22

richvdh self-requested a review December 4, 2019 21:54

richvdh approved these changes Dec 5, 2019

View reviewed changes

richvdh added 2 commits December 5, 2019 17:04

Merge branch 'develop' into user_admin

bdd62cc

fix lint

89925ac

richvdh changed the title ~~Integrate admin endpoint users_paginate into users~~ Replace /admin/v1/users_paginate endpoint with /admin/v2/users Dec 5, 2019

richvdh merged commit 649b6bc into matrix-org:develop Dec 5, 2019

This was referenced Dec 5, 2019

/admin/users_paginate is pointless and should be merged into /admin/users #5494

Closed

<user id> should be removed from /admin/users/<user id> and /admin/users_paginate/<user id> #2522

Closed

awesome-manuel deleted the user_admin branch December 18, 2019 12:48

awesome-manuel mentioned this pull request Apr 22, 2020

Add room details admin endpoint #7317

Merged

4 tasks

dklimpel mentioned this pull request Feb 12, 2021

Remove old admin API GET /_synapse/admin/v1/users/<user_id> #9401

Merged

4 tasks

babolivier pushed a commit that referenced this pull request Sep 1, 2021

Replace /admin/v1/users_paginate endpoint with /admin/v2/users (#5925)

968c74c

* commit '649b6bc08': Replace /admin/v1/users_paginate endpoint with /admin/v2/users (#5925)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace /admin/v1/users_paginate endpoint with /admin/v2/users #5925

Replace /admin/v1/users_paginate endpoint with /admin/v2/users #5925

awesome-manuel commented Aug 28, 2019

awesome-manuel commented Aug 30, 2019

richvdh commented Aug 30, 2019

awesome-manuel commented Sep 16, 2019

richvdh left a comment

richvdh commented Oct 3, 2019

awesome-manuel commented Oct 3, 2019 •

edited

Loading

richvdh left a comment

richvdh commented Oct 10, 2019

awesome-manuel commented Oct 18, 2019

erikjohnston commented Oct 18, 2019

awesome-manuel commented Oct 23, 2019

richvdh left a comment

awesome-manuel commented Nov 20, 2019

richvdh left a comment

richvdh commented Dec 4, 2019

richvdh left a comment

richvdh commented Dec 5, 2019

awesome-manuel commented Dec 5, 2019

richvdh commented Dec 16, 2019

Replace /admin/v1/users_paginate endpoint with /admin/v2/users #5925

Replace /admin/v1/users_paginate endpoint with /admin/v2/users #5925

Conversation

awesome-manuel commented Aug 28, 2019

Pull Request Checklist

awesome-manuel commented Aug 30, 2019

richvdh commented Aug 30, 2019

awesome-manuel commented Sep 16, 2019

richvdh left a comment

Choose a reason for hiding this comment

richvdh commented Oct 3, 2019

awesome-manuel commented Oct 3, 2019 • edited Loading

richvdh left a comment

Choose a reason for hiding this comment

richvdh commented Oct 10, 2019

awesome-manuel commented Oct 18, 2019

erikjohnston commented Oct 18, 2019

awesome-manuel commented Oct 23, 2019

richvdh left a comment

Choose a reason for hiding this comment

awesome-manuel commented Nov 20, 2019

richvdh left a comment

Choose a reason for hiding this comment

richvdh commented Dec 4, 2019

richvdh left a comment

Choose a reason for hiding this comment

richvdh commented Dec 5, 2019

awesome-manuel commented Dec 5, 2019

richvdh commented Dec 16, 2019

awesome-manuel commented Oct 3, 2019 •

edited

Loading