Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Return the previous stream token if a non-member event is a duplicate. #8093

Merged
merged 1 commit into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8093.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Return the previous stream token if a non-member event is a duplicate.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an awful changelog entry for a bugfix, but I don't know what an admin / user might see in this situation. It seems to only be an issue in replication, but I'm not 100% sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is an actual issue, as if this is only a problem in create room then we won't ever hit the "duplicate state" path. So maybe its not a bug fix and just a misc fix to type hinting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I missed this sorry. I think you're correct that it is probably just a misc, not a "real" bug.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this in 408aef8.

25 changes: 15 additions & 10 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -667,42 +667,47 @@ async def send_nonmember_event(
assert self.hs.is_mine(user), "User must be our own: %s" % (user,)

if event.is_state():
prev_state = await self.deduplicate_state_event(event, context)
if prev_state is not None:
prev_event = await self.deduplicate_state_event(event, context)
if prev_event is not None:
logger.info(
"Not bothering to persist state event %s duplicated by %s",
event.event_id,
prev_state.event_id,
prev_event.event_id,
)
return prev_state
return await self.store.get_stream_token_for_event(prev_event.event_id)

return await self.handle_new_client_event(
requester=requester, event=event, context=context, ratelimit=ratelimit
)

async def deduplicate_state_event(
self, event: EventBase, context: EventContext
) -> None:
) -> Optional[EventBase]:
"""
Checks whether event is in the latest resolved state in context.

If so, returns the version of the event in context.
Otherwise, returns None.
Args:
event: The event to check for duplication.
context: The event context.

Returns:
The previous verion of the event is returned, if it is found in the
event context. Otherwise, None is returned.
"""
prev_state_ids = await context.get_prev_state_ids()
prev_event_id = prev_state_ids.get((event.type, event.state_key))
if not prev_event_id:
return
return None
prev_event = await self.store.get_event(prev_event_id, allow_none=True)
if not prev_event:
return
return None

if prev_event and event.user_id == prev_event.user_id:
prev_content = encode_canonical_json(prev_event.content)
next_content = encode_canonical_json(event.content)
if prev_content == next_content:
return prev_event
return
return None

async def create_and_send_nonmember_event(
self,
Expand Down