PUT /_matrix/client/r0/rooms/{roomId}/redact/{eventId}/{txnId} makes no sense

[kegsay]

Sytest actually tests for POST /_matrix/client/r0/rooms/{roomId}/redact/{eventId} which makes more sense as there's no point having transaction IDs as the action is idempotent as it is. This means Synapse implements the POST form, and so will Dendrite. Why does this PUT form exist?

[turt2live]

if it's not useful, and not in the spec, it shouldn't be implemented.

[richvdh]

the point is that it is in the spec (https://matrix.org/docs/spec/client_server/r0.6.1#put-matrix-client-r0-rooms-roomid-redact-eventid-txnid), unlike the POST equivalent.

@Kegsay : it's not quite idempotent. synapse at least implements the endpoint as strictly equivalent to PUT /_matrix/client/r0/rooms/{roomId}/send/m.room.redaction/{txnId}: it doesn't check if the redacted event has already been redacted. So by excluding the txnId, you could end up with multiple redaction events redacting the same event id.

You might say it should check if the event has been redacted, but it can get gnarly with multiple concurrent requests.

[richvdh]

(it's certainly annoying if sytest is testing stuff that's not in the spec, though)

[kegsay]

but it can get gnarly with multiple concurrent requests

AFAIK I don't think it's any more gnarly than any other event creating endpoint? 2 requests to update state requests will race, and likewise 2 requests to redact the same event will race, but I don't think that's particularly a problem? Am I missing a certain gotcha somewhere?

[richvdh]

AFAIK I don't think it's any more gnarly than any other event creating endpoint?

I think you mean "state-changing endpoint" ? The regular event creating endpoint, /room/{roomId}/send/, uses transaction IDs for deduping.

Otherwise, you're right. state-change events are also liable to duplication due to the lack of a txnId, and it's not obviously less of a problem than duplication of redactions. I'm still not sure the correct solution to that is to make redaction more racey though.

[timokoesters]

Element iOS currently uses /redact without a txnid, which fails on Conduit.

[reivilibre]

I would have to agree that this endpoint doesn't seem like it makes sense — unless there's a practical purpose to redacting an event multiple times (not as far as I know).

I don't think I agree that removing the txn ID makes it any harder to deduplicate; couldn't you just pretend that the txn_id = concat(room_id, event_id)?

That said, maybe this ship has sailed.

• SyTest now properly uses PUT: https://github.com/matrix-org/sytest/pull/1235

But Synapse still supports the unspecced thing and it sounds like Ele iOS is relying on it.

Unless there's a sudden change of opinion here, it's probably going to be a case of fixing Element iOS and Synapse.

[turt2live]

I'd be quite in favour of keeping the PUT endpoint as-is in the spec for symmetry with /send, despite transaction IDs making no sense. An MSC to change the transaction ID requirement would also be appreciated however, given transaction IDs make no sense 😇

[tezlm]

i think DELETE would make more sense

[turt2live]

DELETE implies the resource (the event) is able to be removed/actually deleted, whereas in Matrix that's not currently possible.