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

Fix the return type of send_nonmember_events. #8112

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/8112.misc
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.
2 changes: 1 addition & 1 deletion synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ async def send_nonmember_event(
event.event_id,
prev_event.event_id,
)
return await self.store.get_stream_token_for_event(prev_event.event_id)
return await self.store.get_stream_id_for_event(prev_event.event_id)

return await self.handle_new_client_event(
requester=requester, event=event, context=context, ratelimit=ratelimit
Expand Down
19 changes: 15 additions & 4 deletions synapse/storage/databases/main/stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,19 @@ async def get_room_events_max_id(self, room_id: Optional[str] = None) -> str:
)
return "t%d-%d" % (topo, token)

async def get_stream_id_for_event(self, event_id: str) -> int:
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 seems poorly named and bound to be confusing with get_stream_token_for_event? A better name would be appreciated!

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fair. Stream tokens and stream ids are different things.

tbh I feel like get_stream_token_for_event could just be inlined in the two (?) places it is still used. but 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

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

That's fair. I thought keeping it separate was nice to ensure that they both have the same format? Can go either way though.

"""The stream ID for an event
Args:
event_id: The id of the event to look up a stream token for.
Raises:
StoreError if the event wasn't in the database.
Returns:
A stream ID.
"""
return await self.db_pool.simple_select_one_onecol(
table="events", keyvalues={"event_id": event_id}, retcol="stream_ordering"
)

async def get_stream_token_for_event(self, event_id: str) -> str:
"""The stream token for an event
Args:
Expand All @@ -591,10 +604,8 @@ async def get_stream_token_for_event(self, event_id: str) -> str:
Returns:
A "s%d" stream token.
"""
row = await self.db_pool.simple_select_one_onecol(
table="events", keyvalues={"event_id": event_id}, retcol="stream_ordering"
)
return "s%d" % (row,)
stream_id = await self.get_stream_id_for_event(event_id)
return "s%d" % (stream_id,)

async def get_topological_token_for_event(self, event_id: str) -> str:
"""The stream token for an event
Expand Down