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

Support AS timestamp massaging for state events #10634

Closed
Half-Shot opened this issue Aug 18, 2021 · 12 comments · Fixed by #11866
Closed

Support AS timestamp massaging for state events #10634

Half-Shot opened this issue Aug 18, 2021 · 12 comments · Fixed by #11866
Labels
A-Application-Service Related to AS support good first issue Good for newcomers T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@Half-Shot
Copy link
Collaborator

MSC3316 recommends that PUT /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey} should support a ts key for appservices to override the origin_server_ts value of an event. This is already supported in Synapse for normal events, as the feature has been in Synapse for a long time (albeit excluded from the spec; MSC3316 aims to revive it).

@anoadragon453 anoadragon453 added good first issue Good for newcomers T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. labels Aug 18, 2021
@lukasdenk
Copy link
Contributor

@Half-Shot: Does it make sense to include a ts key even though MSC3316 is not merged yet?

@Half-Shot
Copy link
Collaborator Author

Well...it's a weird one. Synapse already supports MSC3316 for the ts parameter by virtue of the parameter being undocumented for ages. I don't think it's necessarily a bad thing to:

  • Treat the existing implementation in Synapse as an unstable feature for MSC3316
  • Extend the existing implementation to support state events.

It's perfectly normal to implement unmerged specs, in order to have a proven implementation.

@lukasdenk
Copy link
Contributor

Ok, I'll work on it

@lukasdenk
Copy link
Contributor

@Half-Shot Can you tell me, where PUT /_matrix/client/r0/rooms/{roomId}/send/{eventType}/{txnId} and /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey} are implemented?

@lukasdenk
Copy link
Contributor

@Half-Shot Ok, I think I found it.

@anoadragon453
Copy link
Member

I would hope that we wouldn't introduce more stable identifiers for unmerged MSCs where we can if possible. The existing ts parameter falls into the category of changes made before we started the concept of unstable prefixes.

I think the lack of an unstable prefix mentioned in an MSC assumes that Synapse supports ts across all relevant endpoints - when it reality that's not the case.

I would prefer if we came up with an unstable prefix, added support for it to PUT /_matrix/client/r0/rooms/{roomId}/send/{eventType}/{txnId} and then only allow the unstable prefix for PUT /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey} until MSC3316 is merged.

@Half-Shot
Copy link
Collaborator Author

I think that's quite reasonable. I would probably prefer it if we kept ts for the endpoint that already works, to avoid a breaking change (even though it's technically not supported -- a lot of things now use it).

@anoadragon453
Copy link
Member

Since it was implemented before unstable prefixes, it makes sense to keep it around.

@lukasdenk
Copy link
Contributor

@anoadragon453 So does this mean that PR #11866 will only be merged after MSC3316 is merged?

@anoadragon453
Copy link
Member

@lukasdenk no, the MSC will need an implementation anyhow, which #11866 could serve as. #11866 is just blocked on MSC3316 gaining an unstable prefix.

But you can probably just use org.matrix.msc3316.ts as a placeholder for now and we'll switch it out for whatever the MSC eventually chooses.

@lukasdenk
Copy link
Contributor

@anoadragon453 In what context should the org.matrix.msc3316.ts placeholder be used?

@anoadragon453
Copy link
Member

Hi @lukasdenk. Please see an explanation of unstable prefixes here: https://spec.matrix.org/unstable/proposals/#early-release-of-an-mscidea

You'll want to use it in any user-facing query parameters. In this case specifically, you'll want to add org.matrix.msc3316.ts as a supported parameter to both PUT /_matrix/client/r0/rooms/{roomId}/send/{eventType}/{txnId} and PUT /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}. In the case of PUT /_matrix/client/r0/rooms/{roomId}/send/{eventType}/{txnId}, both org.matrix.msc3316.ts and ts would be accepted.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Application-Service Related to AS support good first issue Good for newcomers T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants