Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add module API method to resolve a room alias to a room ID #13428

Conversation

buffless-matt
Copy link
Contributor

Description

Addresses #13359.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

@buffless-matt buffless-matt requested a review from a team as a code owner August 1, 2022 05:16
Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test for this method please? There are several tests already in this class which you can use for inspiration.

@dklimpel
Copy link
Contributor

dklimpel commented Aug 1, 2022

Could you add a test for this method please? There are several tests already in this class which you can use for inspiration.

And some docs please (https://github.com/matrix-org/synapse/tree/develop/docs/modules).

@buffless-matt
Copy link
Contributor Author

buffless-matt commented Aug 2, 2022

Could you add a test for this method please? There are several tests already in this class which you can use for inspiration.

@babolivier, added in 83de48b.

@buffless-matt
Copy link
Contributor Author

Could you add a test for this method please? There are several tests already in this class which you can use for inspiration.

And some docs please (https://github.com/matrix-org/synapse/tree/develop/docs/modules).

@dklimpel, I can see quite a few pre-existing module API methods (e.g. this one) that aren't documented in here. Perhaps those docs are for mainly for callbacks? If you insist on the docs, could you please provide a suggestion for where they should go?

@dklimpel
Copy link
Contributor

dklimpel commented Aug 2, 2022

I don't have any special ideas. I only see that the documentation is often forgotten and that currently approx. 60 issues are already tagged with documentation.
It doesn't make for acceptance of the system and ecosystem if you have to scroll through the code to get information.
The alternative is to create another issue that deals with the missing documentation.

Probably a new page with all helper functions (e.g. check_push_rule_actions, lookup_room_alias, get_monthly_active_users_by_service) would be useful.

Related to:

@buffless-matt
Copy link
Contributor Author

@dklimpel I agree that documentation is important. The biggest challenge with documentation (in my opinion) is ensuring that it remains up-to-date (i.e. doesn't go stale). I think docs are less likely to become stale if their canonical source lives closest to the source code that is being documented. Of course it's not always practical to have all documentation in docstrings, but in this case I think the docstring documentation is sufficient.

It doesn't make for acceptance of the system and ecosystem if you have to scroll through the code to get information.

I'd argue:

  • Ideally the docstrings should be used to automatically generate documentation and I think this endeavour should be pursued, rather than making copies of documentation.
  • While not all use-cases warrant scrolling through code, I think it's good general practice to read (at least once) the code that you haven't written and will depend on (where feasible obviously).

Perhaps @babolivier can be the arbiter of this discussion?

@babolivier
Copy link
Contributor

Empirically documentation for the ModuleApi class has always lived in docstrings in the code (and that's what we're linking to in the docs when we do), so having it there in this case is fine. We (the Synapse team) agree that this documentation should be available on the documentation website but we haven't figured out how to automate this yet. There is an ongoing discussion about this here: #10687.

Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly lgtm, just a couple of points to clear

synapse/module_api/__init__.py Show resolved Hide resolved
synapse/module_api/__init__.py Outdated Show resolved Hide resolved
@buffless-matt buffless-matt force-pushed the add-module-api-method-to-resolve-room-alias-to-room-id branch from 0d5ff6c to 353ee92 Compare August 3, 2022 03:53
Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks! :)

synapse/module_api/__init__.py Outdated Show resolved Hide resolved
changelog.d/13428.feature Outdated Show resolved Hide resolved
@babolivier babolivier enabled auto-merge (squash) August 3, 2022 08:46
@babolivier babolivier merged commit 570bf32 into matrix-org:develop Aug 3, 2022
@buffless-matt buffless-matt deleted the add-module-api-method-to-resolve-room-alias-to-room-id branch August 3, 2022 23:50
@DMRobertson DMRobertson linked an issue Aug 8, 2022 that may be closed by this pull request
azmeuk pushed a commit to azmeuk/synapse that referenced this pull request Aug 8, 2022
…g#13428)

Co-authored-by: MattC <buffless-matt@users.noreply.github.com>
Co-authored-by: Brendan Abolivier <babolivier@matrix.org>
Fizzadar added a commit to beeper/synapse-legacy-fork that referenced this pull request Aug 25, 2022
Synapse 1.65.0 (2022-08-16)
===========================

No significant changes since 1.65.0rc2.

Synapse 1.65.0rc2 (2022-08-11)
==============================

Internal Changes
----------------

- Revert 'Remove the unspecced `room_id` field in the `/hierarchy` response. ([\matrix-org#13365](matrix-org#13365))' to give more time for clients to update. ([\matrix-org#13501](matrix-org#13501))

Synapse 1.65.0rc1 (2022-08-09)
==============================

Features
--------

- Add support for stable prefixes for [MSC2285 (private read receipts)](matrix-org/matrix-spec-proposals#2285). ([\matrix-org#13273](matrix-org#13273))
- Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED`, `ORG.MATRIX.MSC3848.NOT_JOINED`, and `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in [MSC3848](matrix-org/matrix-spec-proposals#3848). ([\matrix-org#13343](matrix-org#13343))
- Use stable prefixes for [MSC3827](matrix-org/matrix-spec-proposals#3827). ([\matrix-org#13370](matrix-org#13370))
- Add a new module API method to translate a room alias into a room ID. ([\matrix-org#13428](matrix-org#13428))
- Add a new module API method to create a room. ([\matrix-org#13429](matrix-org#13429))
- Add remote join capability to the module API's `update_room_membership` method (in a backwards compatible manner). ([\matrix-org#13441](matrix-org#13441))

Bugfixes
--------

- Update the version of the LDAP3 auth provider module included in the `matrixdotorg/synapse` DockerHub images and the Debian packages hosted on packages.matrix.org to 0.2.2. This version fixes a regression in the module. ([\matrix-org#13470](matrix-org#13470))
- Fix a bug introduced in Synapse v1.41.0 where the `/hierarchy` API returned non-standard information (a `room_id` field under each entry in `children_state`). ([\matrix-org#13365](matrix-org#13365))
- Fix a bug introduced in Synapse 0.24.0 that would respond with the wrong error status code to `/joined_members` requests when the requester is not a current member of the room. Contributed by @andrewdoh. ([\matrix-org#13374](matrix-org#13374))
- Fix bug in handling of typing events for appservices. Contributed by Nick @ Beeper (@Fizzadar). ([\matrix-org#13392](matrix-org#13392))
- Fix a bug introduced in Synapse 1.57.0 where rooms listed in `exclude_rooms_from_sync` in the configuration file would not be properly excluded from incremental syncs. ([\matrix-org#13408](matrix-org#13408))
- Fix a bug in the experimental faster-room-joins support which could cause it to get stuck in an infinite loop. ([\matrix-org#13353](matrix-org#13353))
- Faster room joins: fix a bug which caused rejected events to become un-rejected during state syncing. ([\matrix-org#13413](matrix-org#13413))
- Faster room joins: fix error when running out of servers to sync partial state with, so that Synapse raises the intended error instead. ([\matrix-org#13432](matrix-org#13432))

Updates to the Docker image
---------------------------

- Make Docker images build on armv7 by installing cryptography dependencies in the 'requirements' stage. Contributed by Jasper Spaans. ([\matrix-org#13372](matrix-org#13372))

Improved Documentation
----------------------

- Update the 'registration tokens' page to acknowledge that the relevant MSC was merged into version 1.2 of the Matrix specification. Contributed by @moan0s. ([\matrix-org#11897](matrix-org#11897))
- Document which HTTP resources support gzip compression. ([\matrix-org#13221](matrix-org#13221))
- Add steps describing how to elevate an existing user to administrator by manipulating the database. ([\matrix-org#13230](matrix-org#13230))
- Fix wrong headline for `url_preview_accept_language` in documentation. ([\matrix-org#13437](matrix-org#13437))
- Remove redundant 'Contents' section from the Configuration Manual. Contributed by @dklimpel. ([\matrix-org#13438](matrix-org#13438))
- Update documentation for config setting `macaroon_secret_key`. ([\matrix-org#13443](matrix-org#13443))
- Update outdated information on `sso_mapping_providers` documentation. ([\matrix-org#13449](matrix-org#13449))
- Fix example code in module documentation of `password_auth_provider_callbacks`. ([\matrix-org#13450](matrix-org#13450))
- Make the configuration for the cache clearer. ([\matrix-org#13481](matrix-org#13481))

Internal Changes
----------------

- Extend the release script to automatically push a new SyTest branch, rather than having that be a manual process. ([\matrix-org#12978](matrix-org#12978))
- Make minor clarifications to the error messages given when we fail to join a room via any server. ([\matrix-org#13160](matrix-org#13160))
- Enable Complement CI tests in the 'latest deps' test run. ([\matrix-org#13213](matrix-org#13213))
- Fix long-standing bugged logic which was never hit in `get_pdu` asking every remote destination even after it finds an event. ([\matrix-org#13346](matrix-org#13346))
- Faster room joins: avoid blocking when pulling events with partially missing prev events. ([\matrix-org#13355](matrix-org#13355))
- Instrument `/messages` for understandable traces in Jaeger. ([\matrix-org#13368](matrix-org#13368))
- Remove an unused argument to `get_relations_for_event`. ([\matrix-org#13383](matrix-org#13383))
- Add a `merge-back` command to the release script, which automates merging the correct branches after a release. ([\matrix-org#13393](matrix-org#13393))
- Adding missing type hints to tests. ([\matrix-org#13397](matrix-org#13397))
- Faster Room Joins: don't leave a stuck room partial state flag if the join fails. ([\matrix-org#13403](matrix-org#13403))
- Refactor `_resolve_state_at_missing_prevs` to compute an `EventContext` instead. ([\matrix-org#13404](matrix-org#13404), [\matrix-org#13431](matrix-org#13431))
- Faster Room Joins: prevent Synapse from answering federated join requests for a room which it has not fully joined yet. ([\matrix-org#13416](matrix-org#13416))
- Re-enable running Complement tests against Synapse with workers. ([\matrix-org#13420](matrix-org#13420))
- Prevent unnecessary lookups to any external `get_event` cache. Contributed by Nick @ Beeper (@Fizzadar). ([\matrix-org#13435](matrix-org#13435))
- Add some tracing to give more insight into local room joins. ([\matrix-org#13439](matrix-org#13439))
- Rename class `RateLimitConfig` to `RatelimitSettings` and `FederationRateLimitConfig` to `FederationRatelimitSettings`. ([\matrix-org#13442](matrix-org#13442))
- Add some comments about how event push actions are stored. ([\matrix-org#13445](matrix-org#13445), [\matrix-org#13455](matrix-org#13455))
- Improve rebuild speed for the "synapse-workers" docker image. ([\matrix-org#13447](matrix-org#13447))
- Fix `@tag_args` being off-by-one with the arguments when tagging a span (tracing). ([\matrix-org#13452](matrix-org#13452))
- Update type of `EventContext.rejected`. ([\matrix-org#13460](matrix-org#13460))
- Use literals in place of `HTTPStatus` constants in tests. ([\matrix-org#13463](matrix-org#13463), [\matrix-org#13469](matrix-org#13469))
- Correct a misnamed argument in state res v2 internals. ([\matrix-org#13467](matrix-org#13467))
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 4, 2022
packaging changes:

summary of upstream changes:

Synapse 1.65.0 (2022-08-16)
===========================

Features
--------

- Add support for stable prefixes for [MSC2285 (private read receipts)](matrix-org/matrix-spec-proposals#2285). ([\#13273](matrix-org/synapse#13273))

- Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED`,
  `ORG.MATRIX.MSC3848.NOT_JOINED`, and
  `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in
  [MSC3848](matrix-org/matrix-spec-proposals#3848).
  ([\#13343](matrix-org/synapse#13343))

- Use stable prefixes for
  [MSC3827](matrix-org/matrix-spec-proposals#3827).
  ([\#13370](matrix-org/synapse#13370))

- Add a new module API method to translate a room alias into a room
  ID. ([\#13428](matrix-org/synapse#13428))

- Add a new module API method to create a
  room. ([\#13429](matrix-org/synapse#13429))

- Add remote join capability to the module API's
  `update_room_membership` method (in a backwards compatible
  manner). ([\#13441](matrix-org/synapse#13441))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ModuleAPI: provide method to resolve room_alias to room_id
3 participants