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

Pass room_id to get_auth_chain_difference #8879

Merged
merged 2 commits into from
Dec 4, 2020
Merged

Pass room_id to get_auth_chain_difference #8879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
137bae0
Pass room_id to get_auth_chain_difference
erikjohnston Dec 4, 2020
fb549da
Newsfile
erikjohnston Dec 4, 2020
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/8879.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Pass `room_id` to `get_auth_chain_difference`.
Copy link
Member

Choose a reason for hiding this comment

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

Should this be what the eventual change will be instead (i.e. should it match #8868)? We seem to end up with really large changelogs, much of which are refactoring to support a feature / fix a 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.

Yeah, though I think that is best done by changing the changelog when that PR lands, rather than risking this landing and going in a release before the other PR lands, iyswim

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense!

4 changes: 2 additions & 2 deletions synapse/state/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ def get_events(
)

def get_auth_chain_difference(
self, state_sets: List[Set[str]]
self, room_id: str, state_sets: List[Set[str]]
) -> Awaitable[Set[str]]:
"""Given sets of state events figure out the auth chain difference (as
per state res v2 algorithm).
Expand All @@ -796,4 +796,4 @@ def get_auth_chain_difference(
An awaitable that resolves to a set of event IDs.
"""

return self.store.get_auth_chain_difference(state_sets)
return self.store.get_auth_chain_difference(room_id, state_sets)
9 changes: 7 additions & 2 deletions synapse/state/v2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ async def resolve_events_with_store(

# Also fetch all auth events that appear in only some of the state sets'
# auth chains.
auth_diff = await _get_auth_chain_difference(state_sets, event_map, state_res_store)
auth_diff = await _get_auth_chain_difference(
room_id, state_sets, event_map, state_res_store
)

full_conflicted_set = set(
itertools.chain(
Expand Down Expand Up @@ -236,6 +238,7 @@ async def _get_power_level_for_sender(


async def _get_auth_chain_difference(
room_id: str,
state_sets: Sequence[StateMap[str]],
event_map: Dict[str, EventBase],
state_res_store: "synapse.state.StateResolutionStore",
Expand Down Expand Up @@ -332,7 +335,9 @@ async def _get_auth_chain_difference(
difference_from_event_map = ()
state_sets_ids = [set(state_set.values()) for state_set in state_sets]

difference = await state_res_store.get_auth_chain_difference(state_sets_ids)
difference = await state_res_store.get_auth_chain_difference(
room_id, state_sets_ids
)
difference.update(difference_from_event_map)

return difference
Expand Down
4 changes: 3 additions & 1 deletion synapse/storage/databases/main/event_federation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ def _get_auth_chain_ids_txn(

return list(results)

async def get_auth_chain_difference(self, state_sets: List[Set[str]]) -> Set[str]:
async def get_auth_chain_difference(
self, room_id: str, state_sets: List[Set[str]]
) -> Set[str]:
"""Given sets of state events figure out the auth chain difference (as
per state res v2 algorithm).

Expand Down
14 changes: 10 additions & 4 deletions tests/state/test_v2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,9 @@ def test_simple(self):

store = TestStateResolutionStore(persisted_events)

diff_d = _get_auth_chain_difference(state_sets, unpersited_events, store)
diff_d = _get_auth_chain_difference(
ROOM_ID, state_sets, unpersited_events, store
)
difference = self.successResultOf(defer.ensureDeferred(diff_d))

self.assertEqual(difference, {c.event_id})
Expand Down Expand Up @@ -662,7 +664,9 @@ def test_multiple_unpersisted_chain(self):

store = TestStateResolutionStore(persisted_events)

diff_d = _get_auth_chain_difference(state_sets, unpersited_events, store)
diff_d = _get_auth_chain_difference(
ROOM_ID, state_sets, unpersited_events, store
)
difference = self.successResultOf(defer.ensureDeferred(diff_d))

self.assertEqual(difference, {d.event_id, c.event_id})
Expand Down Expand Up @@ -707,7 +711,9 @@ def test_unpersisted_events_different_sets(self):

store = TestStateResolutionStore(persisted_events)

diff_d = _get_auth_chain_difference(state_sets, unpersited_events, store)
diff_d = _get_auth_chain_difference(
ROOM_ID, state_sets, unpersited_events, store
)
difference = self.successResultOf(defer.ensureDeferred(diff_d))

self.assertEqual(difference, {d.event_id, e.event_id})
Expand Down Expand Up @@ -773,7 +779,7 @@ def _get_auth_chain(self, event_ids: List[str]) -> List[str]:

return list(result)

def get_auth_chain_difference(self, auth_sets):
def get_auth_chain_difference(self, room_id, auth_sets):
chains = [frozenset(self._get_auth_chain(a)) for a in auth_sets]

common = set(chains[0]).intersection(*chains[1:])
Expand Down
18 changes: 10 additions & 8 deletions tests/storage/test_event_federation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,39 +202,41 @@ def insert_event(txn, event_id, stream_ordering):
# Now actually test that various combinations give the right result:

difference = self.get_success(
self.store.get_auth_chain_difference([{"a"}, {"b"}])
self.store.get_auth_chain_difference(room_id, [{"a"}, {"b"}])
)
self.assertSetEqual(difference, {"a", "b"})

difference = self.get_success(
self.store.get_auth_chain_difference([{"a"}, {"b"}, {"c"}])
self.store.get_auth_chain_difference(room_id, [{"a"}, {"b"}, {"c"}])
)
self.assertSetEqual(difference, {"a", "b", "c", "e", "f"})

difference = self.get_success(
self.store.get_auth_chain_difference([{"a", "c"}, {"b"}])
self.store.get_auth_chain_difference(room_id, [{"a", "c"}, {"b"}])
)
self.assertSetEqual(difference, {"a", "b", "c"})

difference = self.get_success(
self.store.get_auth_chain_difference([{"a", "c"}, {"b", "c"}])
self.store.get_auth_chain_difference(room_id, [{"a", "c"}, {"b", "c"}])
)
self.assertSetEqual(difference, {"a", "b"})

difference = self.get_success(
self.store.get_auth_chain_difference([{"a"}, {"b"}, {"d"}])
self.store.get_auth_chain_difference(room_id, [{"a"}, {"b"}, {"d"}])
)
self.assertSetEqual(difference, {"a", "b", "d", "e"})

difference = self.get_success(
self.store.get_auth_chain_difference([{"a"}, {"b"}, {"c"}, {"d"}])
self.store.get_auth_chain_difference(room_id, [{"a"}, {"b"}, {"c"}, {"d"}])
)
self.assertSetEqual(difference, {"a", "b", "c", "d", "e", "f"})

difference = self.get_success(
self.store.get_auth_chain_difference([{"a"}, {"b"}, {"e"}])
self.store.get_auth_chain_difference(room_id, [{"a"}, {"b"}, {"e"}])
)
self.assertSetEqual(difference, {"a", "b"})

difference = self.get_success(self.store.get_auth_chain_difference([{"a"}]))
difference = self.get_success(
self.store.get_auth_chain_difference(room_id, [{"a"}])
)
self.assertSetEqual(difference, set())