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

MSC3856: Threads List API #3856

Merged
merged 14 commits into from
Sep 25, 2022
Merged

MSC3856: Threads List API #3856

merged 14 commits into from
Sep 25, 2022

Conversation

clokep
Copy link
Contributor

@clokep clokep commented Jul 26, 2022

@clokep clokep changed the title MSC3855: Threads List API MSC3856: Threads List API Jul 26, 2022
@turt2live turt2live added proposal A matrix spec change proposal kind:core MSC which is critical to the protocol's success needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Jul 26, 2022
@clokep clokep marked this pull request as ready for review July 28, 2022 12:08
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

A few comments, but generally looks sensible to me. If you could clarify a couple of things we can probably put this up for FCP.

proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
proposals/3856-threads-list-api.md Show resolved Hide resolved
proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
clokep and others added 5 commits August 9, 2022 15:37
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
@@ -0,0 +1,182 @@
# MSC3856: Threads List API
Copy link
Member

Choose a reason for hiding this comment

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

How does this work over federation as the local server may not have all the thread roots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not change any behavior there -- a server could backfill to fetch additional thread roots if it wanted to, but that seems rather implementation specific.

[MSC3440](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#event-format)
for the format of bundled aggregations of threads).

The returned events are ordered by the latest event of each thread.
Copy link
Member

Choose a reason for hiding this comment

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

Latest by what metric? origin_server_ts? Arrival time on the server? Topological ordering?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is meant to be rather vague and it is further defined below under chunk, although that doesn't fully answer the question either:

The order is chronological by the latest event in that thread.

Note that this is mostly copied from the chunk definition of /messages, which includes the following text:

(The exact definition of chronological is dependent on the server implementation.)

proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
@turt2live turt2live added this to Needs idea feedback / initial review in Spec Core Team Backlog via automation Sep 5, 2022
@turt2live turt2live moved this from Needs idea feedback / initial review to Proposed for FCP readiness in Spec Core Team Backlog Sep 5, 2022
@germain-gg
Copy link
Contributor

Proof of client side implementation for this proposal, matrix-org/matrix-js-sdk#2602

@turt2live
Copy link
Member

The people involved here believe this is ready for FCP. Implementations have not been checked yet though: if suitable, the label will be removed and review continue. If not suitable, comments will be left.

@turt2live turt2live added client-server Client-Server API and removed needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Sep 6, 2022
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

Generally seems fine to me, though I'm not looking forward to specifying another pagination API 🙃

That's a future problem, though.

proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
proposals/3856-threads-list-api.md Outdated Show resolved Hide resolved
@mscbot mscbot added the final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. label Sep 20, 2022
@KitsuneRal
Copy link
Member

The concern is formally resolved in order to let the FCP start and the threads get into the spec within this cycle, with a caveat that we will either fix the logic for ignored events or document the discrepancy I described in the comment as a known issue (that will be addressed in a separate MSC).

@mscbot
Copy link
Collaborator

mscbot commented Sep 25, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Sep 25, 2022
@turt2live turt2live merged commit aa51353 into main Sep 25, 2022
@turt2live turt2live moved this from Ready for FCP ticks to Done to some definition in Spec Core Team Backlog Sep 25, 2022
@turt2live turt2live self-assigned this Sep 26, 2022
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Sep 26, 2022
@turt2live
Copy link
Member

Spec PR: matrix-org/matrix-spec#1254

@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels Sep 27, 2022
@turt2live
Copy link
Member

Merged 🎉

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Sep 27, 2022
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 29, 2022
Upstream changes:

Synapse 1.70.1 (2022-10-28)
===========================

(bugfixes)


Synapse 1.70.0 (2022-10-26)
===========================

Features
--------

- Support for
  [MSC3856](matrix-org/matrix-spec-proposals#3856):
  threads list
  API. ([\#13394](matrix-org/synapse#13394),
  [\#14171](matrix-org/synapse#14171),
  [\#14175](matrix-org/synapse#14175))

- Support for thread-specific notifications & receipts
  ([MSC3771](matrix-org/matrix-spec-proposals#3771)
  and
  [MSC3773](matrix-org/matrix-spec-proposals#3773)). ([\#13776](matrix-org/synapse#13776),
  [\#13824](matrix-org/synapse#13824),
  [\#13877](matrix-org/synapse#13877),
  [\#13878](matrix-org/synapse#13878),
  [\#14050](matrix-org/synapse#14050),
  [\#14140](matrix-org/synapse#14140),
  [\#14159](matrix-org/synapse#14159),
  [\#14163](matrix-org/synapse#14163),
  [\#14174](matrix-org/synapse#14174),
  [\#14222](matrix-org/synapse#14222))

- Stop fetching missing `prev_events` after we already know their
  signature is
  invalid. ([\#13816](matrix-org/synapse#13816))

- Send application service access tokens as a header (and query
  parameter). Implements
  [MSC2832](matrix-org/matrix-spec-proposals#2832). ([\#13996](matrix-org/synapse#13996))

- Ignore server ACL changes when generating pushes. Implements
  [MSC3786](matrix-org/matrix-spec-proposals#3786). ([\#13997](matrix-org/synapse#13997))

- Experimental support for redirecting to an implementation of a
  [MSC3886](matrix-org/matrix-spec-proposals#3886)
  HTTP rendezvous
  service. ([\#14018](matrix-org/synapse#14018))

- The `/relations` endpoint can now be used on
  workers. ([\#14028](matrix-org/synapse#14028))

- Advertise support for Matrix 1.3 and 1.4 on
  `/_matrix/client/versions`. ([\#14032](matrix-org/synapse#14032),
  [\#14184](matrix-org/synapse#14184))

- Improve validation of request bodies for the [Device
  Management](https://spec.matrix.org/v1.4/client-server-api/#device-management)
  and [MSC2697 Device
  Dehyrdation](matrix-org/matrix-spec-proposals#2697)
  client-server API
  endpoints. ([\#14054](matrix-org/synapse#14054))

- Experimental support for
  [MSC3874](matrix-org/matrix-spec-proposals#3874):
  Filtering threads from the `/messages`
  endpoint. ([\#14148](matrix-org/synapse#14148))

- Improve the validation of the following PUT endpoints:
  [`/directory/room/{roomAlias}`](https://spec.matrix.org/v1.4/client-server-api/#put_matrixclientv3directoryroomroomalias),
  [`/directory/list/room/{roomId}`](https://spec.matrix.org/v1.4/client-server-api/#put_matrixclientv3directorylistroomroomid)
  and
  [`/directory/list/appservice/{networkId}/{roomId}`](https://spec.matrix.org/v1.4/application-service-api/#put_matrixclientv3directorylistappservicenetworkidroomid). ([\#14179](matrix-org/synapse#14179))


Deprecations and Removals
-------------------------

- Remove the experimental implementation of
  [MSC3772](matrix-org/matrix-spec-proposals#3772). ([\#14094](matrix-org/synapse#14094))

- Remove the unstable identifier for
  [MSC3715](matrix-org/matrix-spec-proposals#3715). ([\#14106](matrix-org/synapse#14106),
  [\#14146](matrix-org/synapse#14146))
@clokep clokep deleted the clokep/threads-list-api branch May 17, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:core MSC which is critical to the protocol's success merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal
Projects
Archived in project
Spec Core Team Backlog
  
Done to some definition
Development

Successfully merging this pull request may close these issues.

None yet

10 participants