The last member of a room cannot /forget it after leaving

[dklimpel]

Description

If you call POST /_matrix/client/v3/rooms/{roomId}/forget the user does not forget the room.
This only seems to occur when the user was the last member.

Steps to reproduce

• leave a room
• try to forget the room via /forget or webclient
• get http 200 and nothing happens
• there are no changes in db table room_memberships

If you run Synapse in SQL debug mode you can see the db runs synapse.storage.txn - 691 - DEBUG - POST-148 - [TXN START] {get_filtered_current_state_ids-3c6} but no forget_membership txn.

synapse/synapse/handlers/room_member.py

Lines 1912 to 1927 in e825f73

	async def forget(self, user: UserID, room_id: str) -> None:
	user_id = user.to_string()
	member = await self._storage_controllers.state.get_current_state_event(
	room_id=room_id, event_type=EventTypes.Member, state_key=user_id
	)
	membership = member.membership if member else None
	if membership is not None and membership not in [
	Membership.LEAVE,
	Membership.BAN,
	]:
	raise SynapseError(400, "User %s in room %s" % (user_id, room_id))
	if membership:
	await self.store.forget(user_id, room_id)

synapse/synapse/storage/databases/main/roommember.py

Lines 1478 to 1499 in e825f73

	async def forget(self, user_id: str, room_id: str) -> None:
	"""Indicate that user_id wishes to discard history for room_id."""
	def f(txn: LoggingTransaction) -> None:
	sql = (
	"UPDATE"
	" room_memberships"
	" SET"
	" forgotten = 1"
	" WHERE"
	" user_id = ?"
	" AND"
	" room_id = ?"
	)
	txn.execute(sql, (user_id, room_id))
	self._invalidate_cache_and_stream(txn, self.did_forget, (user_id, room_id))
	self._invalidate_cache_and_stream(
	txn, self.get_forgotten_rooms_for_user, (user_id,)
	)
	await self.db_pool.runInteraction("forget_membership", f)

I have added two lines for debugging in room_member.py

        logger.info("add info: %s", (membership,))
        logger.info("add info: %s", (member,))

• If you are member of the room, member is a FrozenEventV3 and membership join. Synapse raises http 400. 😄
• If you have left the room, member and membership is None. Nothing happens, only http 200 😢
  • If the user was the last member of the room
• If the user was not the last member, member is a FrozenEventV3 and membership leave

Homeserver

another homeserver

Synapse Version

Synapse/1.65.0rc1

Installation Method

Debian packages from packages.matrix.org

Platform

vm: Ubuntu 20.04.4 LTS

Relevant log output

see above

Anything else that would be useful to know?

Discovered while working on #13503 (comment)

[H-Shay]

This is reproducible-I wrote a unit test to reproduce this, and did some debugging. 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. Thus when the /forget request executes this line:

member = await self._storage_controllers.state.get_current_state_event(
            room_id=room_id, event_type=EventTypes.Member, state_key=user_id
        )

the return value for get_current_state_events is None, as the table current_state_events now has no rows related to the room. I did verify that after the last member leaves the room, get_current_state_events no longer retains any rows from this room.

[dklimpel]

Is there an idea how to fix it?
One possible solution could be to run forget also if get_current_state_events is None. If the server is not member of the room ther should not be a problem. In worst case only a not needed update query is running, which updates 0 lines.

[anoadragon453]

@dklimpel I think that would be fine, even if the case of you later rejoining the room. It would be good to have a test for rejoining a room included with a fix though, to weed out any other potential gotchas. But in theory it should work.

My only worry is whether it's a bit flaky to rely on the function returning None... thus it'd be good to have a test.

[dklimpel]

@anoadragon453 I have created a test in #13503 in DB layer.

synapse/tests/storage/test_roommember.py

Lines 189 to 207 in 8fa7eb8

	def test_join_locally_forgotten_room(self):
	"""Tests if a user joins a forgotten room the room is not forgotten anymore."""
	self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice)
	self.assertFalse(
	self.get_success(self.store.is_locally_forgotten_room(self.room))
	)
	# after leaving and forget the room, it is forgotten
	self.inject_room_member(self.room, self.u_alice, Membership.LEAVE)
	self.get_success(self.store.forget(self.u_alice, self.room))
	self.assertTrue(
	self.get_success(self.store.is_locally_forgotten_room(self.room))
	)
	# after rejoin the room is not forgotten anymore
	self.inject_room_member(self.room, self.u_alice, Membership.JOIN)
	self.assertFalse(
	self.get_success(self.store.is_locally_forgotten_room(self.room))
	)

This issue needs additonal tests in handlers layer.

@H-Shay

I wrote a unit test to reproduce this

Do you create a PR for this test or can post your test here?

[H-Shay]

I didn't create a PR, it was just some hacky code to try and reproduce the situation. If it's helpful, here it is:

    def test_leave_and_forget(self):
        self.helper.join(self.room_id, user=self.second_user_id, tok=self.second_tok)
        self.helper.leave(self.room_id, user=self.creator, tok=self.creator_tok)
        self.helper.leave(self.room_id, user=self.second_user_id, tok=self.second_tok)

        channel = self.make_request(
            "POST",
            f"/_matrix/client/v3/rooms/{self.room_id}/forget",
            access_token=self.second_tok,
        )

        self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
        self.assertEqual(self.get_success(
            self.hs.get_datastores().main.did_forget(self.second_user_id, self.room_id)), True)