Protects shared room history from absent knocking member

[zamanzamzz]

Fix #13968

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)

[DMRobertson]

Putting this in the review queue to force us to decide what to do with this change.

[erikjohnston]

Sorry for not getting back to you about this, it's a little bit tricky to figure out the best way of doing this. You're right that we need to think a bit more carefully about the knock -> leave transition.

The tricky thing is that I don't think we have an efficient way of answering the question "has the user ever been in the room". I think the proposed approach likely works, but is going to be very inefficient on large databases.

Maybe we'll need to track "which local users have been in which rooms" separately?

[reivilibre]

The tricky thing is that I don't think we have an efficient way of answering the question "has the user ever been in the room". I think the proposed approach likely works, but is going to be very inefficient on large databases.

Can we fix this going forward by adding some internal_metadata to m.room.member events along the lines of 'has user even been in room / does user satisfy shared readability requirements'?
We could also populate this retrospectively on demand if needed.

Then when processing a new m.room.member event, you can update it based on the internal metadata / membership of the previous state event. Plus, whenever you ask for the current state at a given point in time, it would dig out the events with their internal metadata at the same time, so updating the existing filter code would be fairly simple(???).

[DMRobertson]

To summarise:

• there are performance concerns here. It sounds like tackling this in a performant way would require more effort.
• I note the PR lacks tests---it would be good to have something which reproduces the problem before this change.

[clokep]

Removing from the review queue since @DMRobertson responded above.