diff --git a/changelog.d/18676.misc b/changelog.d/18676.misc new file mode 100644 index 00000000000..81306954b6d --- /dev/null +++ b/changelog.d/18676.misc @@ -0,0 +1 @@ +Remove unused `allow_no_prev_events` option when creating an event. diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 5d6ee6996f7..6f68b8f6038 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -568,7 +568,6 @@ async def create_event( requester: Requester, event_dict: dict, txn_id: Optional[str] = None, - allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, @@ -594,10 +593,6 @@ async def create_event( requester event_dict: An entire event txn_id - allow_no_prev_events: Whether to allow this event to be created an empty - list of prev_events. Normally this is prohibited just because most - events should have a prev_event and we should only use this in special - cases (previously useful for MSC2716). prev_event_ids: the forward extremities to use as the prev_events for the new event. @@ -717,7 +712,6 @@ async def create_event( event, unpersisted_context = await self.create_new_client_event( builder=builder, requester=requester, - allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, state_event_ids=state_event_ids, @@ -945,7 +939,6 @@ async def create_and_send_nonmember_event( self, requester: Requester, event_dict: dict, - allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, ratelimit: bool = True, @@ -962,10 +955,6 @@ async def create_and_send_nonmember_event( Args: requester: The requester sending the event. event_dict: An entire event. - allow_no_prev_events: Whether to allow this event to be created an empty - list of prev_events. Normally this is prohibited just because most - events should have a prev_event and we should only use this in special - cases (previously useful for MSC2716). prev_event_ids: The event IDs to use as the prev events. Should normally be left as None to automatically request them @@ -1051,7 +1040,6 @@ async def create_and_send_nonmember_event( return await self._create_and_send_nonmember_event_locked( requester=requester, event_dict=event_dict, - allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, state_event_ids=state_event_ids, ratelimit=ratelimit, @@ -1065,7 +1053,6 @@ async def _create_and_send_nonmember_event_locked( self, requester: Requester, event_dict: dict, - allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, ratelimit: bool = True, @@ -1097,7 +1084,6 @@ async def _create_and_send_nonmember_event_locked( requester, event_dict, txn_id=txn_id, - allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, state_event_ids=state_event_ids, outlier=outlier, @@ -1180,7 +1166,6 @@ async def create_new_client_event( self, builder: EventBuilder, requester: Optional[Requester] = None, - allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, @@ -1200,10 +1185,6 @@ async def create_new_client_event( Args: builder: requester: - allow_no_prev_events: Whether to allow this event to be created an empty - list of prev_events. Normally this is prohibited just because most - events should have a prev_event and we should only use this in special - cases (previously useful for MSC2716). prev_event_ids: the forward extremities to use as the prev_events for the new event. @@ -1241,7 +1222,6 @@ async def create_new_client_event( if state_event_ids is not None: # Do a quick check to make sure that prev_event_ids is present to # make the type-checking around `builder.build` happy. - # prev_event_ids could be an empty array though. assert prev_event_ids is not None temp_event = await builder.build( @@ -1269,24 +1249,14 @@ async def create_new_client_event( else: prev_event_ids = await self.store.get_prev_events_for_room(builder.room_id) + # We now ought to have some `prev_events` (unless it's a create event). + # # Do a quick sanity check here, rather than waiting until we've created the # event and then try to auth it (which fails with a somewhat confusing "No # create event in auth events") - if allow_no_prev_events: - # We allow events with no `prev_events` but it better have some `auth_events` - assert ( - builder.type == EventTypes.Create - # Allow an event to have empty list of prev_event_ids - # only if it has auth_event_ids. - or auth_event_ids - ), ( - "Attempting to create a non-m.room.create event with no prev_events or auth_event_ids" - ) - else: - # we now ought to have some prev_events (unless it's a create event). - assert builder.type == EventTypes.Create or prev_event_ids, ( - "Attempting to create a non-m.room.create event with no prev_events" - ) + assert builder.type == EventTypes.Create or len(prev_event_ids) > 0, ( + "Attempting to create an event with no prev_events" + ) if for_batch: assert prev_event_ids is not None diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index cf9db7b0189..b6800a9f638 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -388,11 +388,11 @@ async def ratelimit_invite( async def _local_membership_update( self, + *, requester: Requester, target: UserID, room_id: str, membership: str, - allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, depth: Optional[int] = None, @@ -414,11 +414,6 @@ async def _local_membership_update( desired membership event. room_id: membership: - - allow_no_prev_events: Whether to allow this event to be created an empty - list of prev_events. Normally this is prohibited just because most - events should have a prev_event and we should only use this in special - cases (previously useful for MSC2716). prev_event_ids: The event IDs to use as the prev events state_event_ids: The full state at a given event. This was previously used particularly @@ -486,7 +481,6 @@ async def _local_membership_update( "origin_server_ts": origin_server_ts, }, txn_id=txn_id, - allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, state_event_ids=state_event_ids, depth=depth, @@ -583,7 +577,6 @@ async def update_membership( new_room: bool = False, require_consent: bool = True, outlier: bool = False, - allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, depth: Optional[int] = None, @@ -607,10 +600,6 @@ async def update_membership( outlier: Indicates whether the event is an `outlier`, i.e. if it's from an arbitrary point and floating in the DAG as opposed to being inline with the current DAG. - allow_no_prev_events: Whether to allow this event to be created an empty - list of prev_events. Normally this is prohibited just because most - events should have a prev_event and we should only use this in special - cases (previously useful for MSC2716). prev_event_ids: The event IDs to use as the prev events state_event_ids: The full state at a given event. This was previously used particularly @@ -680,7 +669,6 @@ async def update_membership( new_room=new_room, require_consent=require_consent, outlier=outlier, - allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, state_event_ids=state_event_ids, depth=depth, @@ -703,7 +691,6 @@ async def update_membership_locked( new_room: bool = False, require_consent: bool = True, outlier: bool = False, - allow_no_prev_events: bool = False, prev_event_ids: Optional[List[str]] = None, state_event_ids: Optional[List[str]] = None, depth: Optional[int] = None, @@ -729,10 +716,6 @@ async def update_membership_locked( outlier: Indicates whether the event is an `outlier`, i.e. if it's from an arbitrary point and floating in the DAG as opposed to being inline with the current DAG. - allow_no_prev_events: Whether to allow this event to be created an empty - list of prev_events. Normally this is prohibited just because most - events should have a prev_event and we should only use this in special - cases (previously useful for MSC2716). prev_event_ids: The event IDs to use as the prev events state_event_ids: The full state at a given event. This was previously used particularly @@ -933,7 +916,6 @@ async def update_membership_locked( ) # InviteRule.IGNORE is handled at the sync layer. - # An empty prev_events list is allowed as long as the auth_event_ids are present if prev_event_ids is not None: return await self._local_membership_update( requester=requester, @@ -942,7 +924,6 @@ async def update_membership_locked( membership=effective_membership_state, txn_id=txn_id, ratelimit=ratelimit, - allow_no_prev_events=allow_no_prev_events, prev_event_ids=prev_event_ids, state_event_ids=state_event_ids, depth=depth, diff --git a/tests/handlers/test_message.py b/tests/handlers/test_message.py index 76ab83d1f77..990c906d2c6 100644 --- a/tests/handlers/test_message.py +++ b/tests/handlers/test_message.py @@ -204,46 +204,19 @@ def test_duplicated_txn_id_one_call(self) -> None: self.assertEqual(len(events), 2) self.assertEqual(events[0].event_id, events[1].event_id) - def test_when_empty_prev_events_allowed_create_event_with_empty_prev_events( + def test_reject_event_with_empty_prev_events( self, ) -> None: - """When we set allow_no_prev_events=True, should be able to create a - event without any prev_events (only auth_events). """ - # Create a member event we can use as an auth_event - memberEvent, _ = self._create_and_persist_member_event() - - # Try to create the event with empty prev_events bit with some auth_events - event, _ = self.get_success( - self.handler.create_event( - self.requester, - { - "type": EventTypes.Message, - "room_id": self.room_id, - "sender": self.requester.user.to_string(), - "content": {"msgtype": "m.text", "body": random_string(5)}, - }, - # Empty prev_events is the key thing we're testing here - prev_event_ids=[], - # But with some auth_events - auth_event_ids=[memberEvent.event_id], - # Allow no prev_events! - allow_no_prev_events=True, - ) - ) - self.assertIsNotNone(event) - - def test_when_empty_prev_events_not_allowed_reject_event_with_empty_prev_events( - self, - ) -> None: - """When we set allow_no_prev_events=False, shouldn't be able to create a - event without any prev_events even if it has auth_events. Expect an - exception to be raised. + Shouldn't be able to create an event without any `prev_events` even if it has + `auth_events`. Expect an exception to be raised. """ # Create a member event we can use as an auth_event memberEvent, _ = self._create_and_persist_member_event() # Try to create the event with empty prev_events but with some auth_events + # + # We expect the test to fail because empty prev_events are not allowed self.get_failure( self.handler.create_event( self.requester, @@ -257,35 +230,6 @@ def test_when_empty_prev_events_not_allowed_reject_event_with_empty_prev_events( prev_event_ids=[], # But with some auth_events auth_event_ids=[memberEvent.event_id], - # We expect the test to fail because empty prev_events are not - # allowed here! - allow_no_prev_events=False, - ), - AssertionError, - ) - - def test_when_empty_prev_events_allowed_reject_event_with_empty_prev_events_and_auth_events( - self, - ) -> None: - """When we set allow_no_prev_events=True, should be able to create a - event without any prev_events or auth_events. Expect an exception to be - raised. - """ - # Try to create the event with empty prev_events and empty auth_events - self.get_failure( - self.handler.create_event( - self.requester, - { - "type": EventTypes.Message, - "room_id": self.room_id, - "sender": self.requester.user.to_string(), - "content": {"msgtype": "m.text", "body": random_string(5)}, - }, - prev_event_ids=[], - # The event should be rejected when there are no auth_events - auth_event_ids=[], - # Allow no prev_events! - allow_no_prev_events=True, ), AssertionError, )