WIP: Output type redacted event

[rcarback]

This patch enables appservices to receive Redaction events.

Pull Request Checklist

• I have added Go unit tests or Complement integration tests for this PR or I have justified why this PR doesn't need tests
• Pull request includes a sign off below using a legally identifiable name or I have already signed off privately

Signed-off-by: Your Name <[email protected]>

[CLAassistant]

All committers have signed the CLA.

[S7evinK]

Looks good to me.
There is just one thing we should probably take care of as per this comment: The appservice consumer should ignore m.room.redaction events if they come in through OutputTypeNewRoomEvent. See this. (Though I don't know if that really matters for bridges/appservices, not aware of how they are handled by them.)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.

Project coverage is 49.39%. Comparing base (7cc7ebb) to head (253d7a9).

Files with missing lines	Patch %	Lines
appservice/consumers/roomserver.go	0.00%	4 Missing and 1 partial ⚠️
syncapi/consumers/roomserver.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3445      +/-   ##
==========================================
+ Coverage   49.36%   49.39%   +0.03%     
==========================================
  Files         522      522              
  Lines       59465    59473       +8     
==========================================
+ Hits        29353    29375      +22     
+ Misses      26647    26639       -8     
+ Partials     3465     3459       -6     

Flag	Coverage Δ
unittests	49.39% <0.00%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rcarback]

Looks good to me. There is just one thing we should probably take care of as per this comment: The appservice consumer should ignore m.room.redaction events if they come in through OutputTypeNewRoomEvent. See this. (Though I don't know if that really matters for bridges/appservices, not aware of how they are handled by them.)

OK great, I will look at the failing tests then and add one for this.

I am not sure I understand your comment (or rather, understand the spec...) because I thought OutputTypeRedactedEvent was a validated event that could not be part of a OutputTypeNewRoomEvent?

If we add a check for receivedType == api.OutputTypeNewRoomEvent || on the if statement on this line:

https://github.com/element-hq/dendrite/pull/3445/files#diff-594a94eeec27d0c54996b75cd3002ce09168b17432b108d759fb41c6a35e2a4fR151

does that resolve it?

[S7evinK]

I meant that we need this right before this line for this:

	// In order to honour redactions correctly, downstream components must ignore m.room.redaction events emitted via OutputTypeNewRoomEvent.
	// When downstream components receive an OutputTypeRedactedEvent they must:
	// - Pull out the event to redact from the database. They should have this because the redaction is validated.
	// - Redact the event and set the corresponding `unsigned` fields to indicate it as redacted.
	// - Replace the event in the database.

While we don't do anything with the DB in the appservice consumer, we simply forward the redacted event to the appservice/bridge.