Fix that user cannot /forget rooms after the last member has left

[dklimpel]

#13517 (comment):

I suspect that after the last member leaves the room, the background update _background_remove_left_rooms is deleting rows related to this room from the table current_state_events.
the return value for get_current_state_events is None

Fixes: #13517

Blocked by: #13503
for using is_locally_forgotten_room in tests

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file.
• Pull request includes a sign off
• Code style is correct
  (run the linters)

Signed-off-by: Dirk Klimpel [email protected]

[dklimpel]

This test is failing

Error: 
Traceback (most recent call last):
  File "/home/runner/work/synapse/synapse/tests/handlers/test_room_member.py", line 376, in test_rejoin_forgotten_by_server
    self.helper.join(self.room_id, user=self.alice, tok=self.alice_token)
  File "/home/runner/work/synapse/synapse/tests/rest/client/utils.py", line 178, in join
    self.change_membership(
  File "/home/runner/work/synapse/synapse/tests/rest/client/utils.py", line 313, in change_membership
    assert channel.code == expect_code, "Expected: %d, got: %d, resp: %r" % (
builtins.AssertionError: Expected: 200, got: 404, resp: b'{"errcode":"M_UNKNOWN","error":"Can\'t join remote room because no servers that are in the room have been provided."}'

tests.handlers.test_room_member.RoomMemberMasterHandlerTestCase.test_rejoin_forgotten_by_server
-------------------------------------------------------------------------------

I am not sure if this is a bug in Synapse or a wrong test.
The error occures there:

synapse/synapse/handlers/room_member.py

Lines 1668 to 1689 in 0a4efbc

	async def _remote_join(
	self,
	requester: Requester,
	remote_room_hosts: List[str],
	room_id: str,
	user: UserID,
	content: dict,
	) -> Tuple[str, int]:
	"""Implements RoomMemberHandler._remote_join"""
	# filter ourselves out of remote_room_hosts: do_invite_join ignores it
	# and if it is the only entry we'd like to return a 404 rather than a
	# 500.
	remote_room_hosts = [
	host for host in remote_room_hosts if host != self.hs.hostname
	]
	if len(remote_room_hosts) == 0:
	raise SynapseError(
	404,
	"Can't join remote room because no servers "
	"that are in the room have been provided.",
	)

Actually, this is not a fault of forgetting rooms, rather of leaving and joining rooms.
Perhaps related to:

• User is unable to accept an invite to rejoin a room #8796
• completely broken room after purge_room/rejoin #11521

How to deal with it?

[DMRobertson]

Synapse doesn't know how to rejoin the user to self.room_id because

• the server has no record of self.room_id after it's been forgotten
• the server doesn't know any other servers in that room.

This one might be better handled with a complement test, so that you genuinely do have a remote homeserver; otherwise I think you'd have to do a fair chunk of mocking to do it as a Synapse-only test.

[dklimpel]

• the server doesn't know any other servers in that room.

Interesting. The room id is actually one of his own, since his user originally created the room.
A little bit off topic: The server only finds the room again when a local user is invited or with a foreign alias.

The conclusion: The test should be removed.

[reivilibre]

Interesting. The room id is actually one of his own, since his user originally created the room.

The server name in any room ID is not meant to be parsed; it's only there to ensure that two servers don't create a room with the same room ID.
That's why the room ID having been created by this server is actually irrelevant here, I think.

(This surprises people: they assume that !blahblah:example.com should be enough to join a room via example.com and are confused when Synapse will refuse to if you don't tell it any server names. There are also proposals to remove the server name from room IDs, if I remember, instead using the hash of the create event or something equivalent.)

A little bit off topic: The server only finds the room again when a local user is invited or with a foreign alias.

This is more or less what I'd expect: in both those cases, the server gets a list of other servers that are in the room.
The server then knows which server it can use to do the /make_join, /send_join handshake with — see https://spec.matrix.org/v1.3/server-server-api/#joining-rooms if you haven't already to get a picture of how this works.

Joining a remote room requires you to know at least one server that's in the room already, otherwise you have nowhere to join it via.

I agree with @DMRobertson here that a Complement test for this seems like it would be ideal, since it would let you prove this works externally with 2 real servers and you don't have to do a lot of mocking.

[reivilibre]

I agree with your conclusion that the test_rejoin_forgotten_by_server test should be removed.
I think you'd have to do a lot of mocking to get it to work. It may be more fruitful to write a Complement test that ensures the room still works after rejoining a forgotten room, plus you'd get to avoid a lot of the pain.