From 7dad90291290ffed9a67794e472b796c4f9325de Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 15 Sep 2021 09:48:14 +0100 Subject: [PATCH 01/34] Add an approximate difference method to StateFilters --- synapse/storage/state.py | 54 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index c76529cb5710..7cbd087a0697 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -356,6 +356,60 @@ def get_member_split(self) -> Tuple["StateFilter", "StateFilter"]: return member_filter, non_member_filter + def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": + """ + Returns a state filter which represents self - subtrahend; + if the set of state events given by a state filter F are represented as + E(F), then the resultant state filter bears this property: + + E(difference(self, subtrahend)) ⊇ E(self) ∖ E(subtrahend) + + Ideally, this should be the narrowest such state filter, but this + function returns an approximation (since, for example, the set of + possible state keys is infinite). + """ + + types = dict(self.types) + new_include_others = self.include_others and not subtrahend.include_others + # if this is an include_others state filter, then all unmentioned + # event types are wildcards; otherwise they're empty. + current_default_for_unspecified: Optional[FrozenSet[str]] = ( + None if self.include_others else frozenset() + ) + new_default_for_unspecified: Optional[FrozenSet[str]] = ( + None if new_include_others else frozenset() + ) + + for sub_type, sub_keys in subtrahend.types.items(): + current: Optional[FrozenSet[str]] = types.get( + sub_type, current_default_for_unspecified + ) + + new: Optional[FrozenSet[str]] + if sub_keys is None: + # anything minus all is none + new = frozenset() + elif current is None: + # all minus a few specific keys is something we can only + # approximate as 'all' + new = None + else: + # a few specific keys minus a few specific keys is just the set + # difference of those keys + new = current.difference(sub_keys) + + if new == new_default_for_unspecified: + # if the result is the same as the default assumption, + # don't bother storing it. + if sub_type in types: + types.pop(sub_type) + else: + # this is not the same as the default assumption, so + # we store it + types[sub_type] = new + + return StateFilter(frozendict(types), include_others=new_include_others) + class StateGroupStorage: """High level interface to fetching state for event.""" From 1fe75e6911e7b9b6082d4dda473f729648dcbf13 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 15 Sep 2021 09:48:35 +0100 Subject: [PATCH 02/34] Add tests for the approximate difference of StateFilters --- tests/storage/test_state.py | 86 ++++++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py index 32060f2abd63..b472ab3d065c 100644 --- a/tests/storage/test_state.py +++ b/tests/storage/test_state.py @@ -105,7 +105,6 @@ def test_get_state_groups(self): self.assertEqual({ev.event_id for ev in state_list}, {e1.event_id, e2.event_id}) def test_get_state_for_event(self): - # this defaults to a linear DAG as each new injection defaults to whatever # forward extremities are currently in the DB for this room. e1 = self.inject_state_event(self.room, self.u_alice, EventTypes.Create, "", {}) @@ -483,3 +482,88 @@ def test_get_state_for_event(self): self.assertEqual(is_all, True) self.assertDictEqual({(e5.type, e5.state_key): e5.event_id}, state_dict) + + def test_state_filter_difference(self): + def assert_difference( + minuend: StateFilter, subtrahend: StateFilter, expected: StateFilter + ): + self.assertEqual( + minuend.approx_difference(subtrahend), + expected, + f"StateFilter difference not correct:\n\n\t{minuend!r}\nminus\n\t{subtrahend!r}\nwas\n\t{minuend.approx_difference(subtrahend)}\nexpected\n\t{expected}", + ) + + # it's not possible to subtract individual state keys from + # a wildcard + assert_difference( + StateFilter.all(), + StateFilter.from_types( + ((EventTypes.Member, "@wombat:hs1"), (EventTypes.Member, "@spqr:hs1")) + ), + StateFilter.all(), + ) + self.assertEqual( + StateFilter.all().approx_difference( + StateFilter.from_types( + ( + (EventTypes.Member, "@wombat:hs1"), + (EventTypes.Member, "@spqr:hs1"), + ) + ) + ), + StateFilter.all(), + ) + + # we can subtract wildcards from wildcards + assert_difference(StateFilter.all(), StateFilter.all(), StateFilter.none()) + assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + EventTypes.CanonicalAlias: None, + } + ), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.JoinRules: None}), include_others=False + ), + StateFilter( + types=frozendict( + {EventTypes.Member: frozenset(), EventTypes.JoinRules: frozenset()} + ), + include_others=True, + ), + ) + + # we can subtract individual state keys, except from wildcards + assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:hs1", "@kristina:hs2"}), + EventTypes.CanonicalAlias: None, + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@kristina:hs2"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:hs1"}), + EventTypes.CanonicalAlias: None, + } + ), + include_others=False, + ), + ) From a05692cadf83901b1a4fbdec8bcf7e859d086f58 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 15 Sep 2021 11:49:27 +0100 Subject: [PATCH 03/34] Newsfile Signed-off-by: Olivier Wilkinson (reivilibre) --- changelog.d/10825.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10825.misc diff --git a/changelog.d/10825.misc b/changelog.d/10825.misc new file mode 100644 index 000000000000..f9786164d7ec --- /dev/null +++ b/changelog.d/10825.misc @@ -0,0 +1 @@ +Add an 'approximate difference' method to `StateFilter`. From 10a707124e2a03d90700cc863a520ee910c355cf Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 17 Sep 2021 10:21:59 +0100 Subject: [PATCH 04/34] Try to clarify docstring for `approx_difference` --- synapse/storage/state.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 7cbd087a0697..1ef510fc1de6 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -358,15 +358,33 @@ def get_member_split(self) -> Tuple["StateFilter", "StateFilter"]: def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": """ - Returns a state filter which represents self - subtrahend; - if the set of state events given by a state filter F are represented as - E(F), then the resultant state filter bears this property: + Returns a state filter which represents `self - subtrahend`. + + The resultant state filter MUST admit all state events that are admitted + by only this filter (`self`) and not `subtrahend`. (1) + The resultant filter MAY be an over-approximation: the resultant state + filter MAY additionally admit other state events. + + + Formally, if the set of state events admitted by a state filter F are + written as E(F), then the resultant state filter bears this property: E(difference(self, subtrahend)) ⊇ E(self) ∖ E(subtrahend) - Ideally, this should be the narrowest such state filter, but this - function returns an approximation (since, for example, the set of - possible state keys is infinite). + + This function attempts to return the narrowest such state filter. + In the case that `self` contains wildcards for state types where + `subtrahend` contains specific state keys, an approximation must be made: + the resultant state filter keeps the wildcard, as state filters are not + able to express 'all state keys except some given examples'. + e.g. + StateFilter(m.room.member -> None (wildcard)) + minus + StateFilter(m.room.member -> {'@wombat:example.org'}) + is approximated as + StateFilter(m.room.member -> None (wildcard)) + which satisfies the condition that the resultant state filter + is an over-approximation. """ types = dict(self.types) From d0e14d5e2b43226ad9e619e9a0e5b26635df24b1 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 17 Sep 2021 10:48:19 +0100 Subject: [PATCH 05/34] Process all the keys to return a narrower state filter --- synapse/storage/state.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 1ef510fc1de6..b2f497a7c021 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -391,40 +391,47 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": new_include_others = self.include_others and not subtrahend.include_others # if this is an include_others state filter, then all unmentioned # event types are wildcards; otherwise they're empty. + subtrahend_default_for_unspecified: Optional[FrozenSet[str]] = ( + None if subtrahend.include_others else frozenset() + ) current_default_for_unspecified: Optional[FrozenSet[str]] = ( None if self.include_others else frozenset() ) new_default_for_unspecified: Optional[FrozenSet[str]] = ( None if new_include_others else frozenset() ) - - for sub_type, sub_keys in subtrahend.types.items(): - current: Optional[FrozenSet[str]] = types.get( - sub_type, current_default_for_unspecified + state_types = set(subtrahend.types.keys()) + state_types.update(set(self.types.keys())) + for state_type in state_types: + sub_keys: Optional[FrozenSet[str]] = subtrahend.types.get( + state_type, subtrahend_default_for_unspecified + ) + current_keys: Optional[FrozenSet[str]] = types.get( + state_type, current_default_for_unspecified ) - new: Optional[FrozenSet[str]] + new_keys: Optional[FrozenSet[str]] if sub_keys is None: # anything minus all is none - new = frozenset() - elif current is None: + new_keys = frozenset() + elif current_keys is None: # all minus a few specific keys is something we can only # approximate as 'all' - new = None + new_keys = None else: # a few specific keys minus a few specific keys is just the set # difference of those keys - new = current.difference(sub_keys) + new_keys = current_keys.difference(sub_keys) - if new == new_default_for_unspecified: + if new_keys == new_default_for_unspecified: # if the result is the same as the default assumption, # don't bother storing it. - if sub_type in types: - types.pop(sub_type) + if state_type in types: + types.pop(state_type) else: # this is not the same as the default assumption, so # we store it - types[sub_type] = new + types[state_type] = new_keys return StateFilter(frozendict(types), include_others=new_include_others) From a5fdd46d4c29d5d3c6a0e6a584c5380314a7101f Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 17 Sep 2021 10:48:37 +0100 Subject: [PATCH 06/34] Add more test cases --- tests/storage/test_state.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py index b472ab3d065c..daa3fc0c5895 100644 --- a/tests/storage/test_state.py +++ b/tests/storage/test_state.py @@ -537,6 +537,22 @@ def assert_difference( ), ) + # subtract two strange wildcards + assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.CanonicalAlias: frozenset({""})}), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.CanonicalAlias: None}), + include_others=False, + ), + ) + # we can subtract individual state keys, except from wildcards assert_difference( StateFilter( @@ -567,3 +583,18 @@ def assert_difference( include_others=False, ), ) + + # we can subtract wildcards from non-wildcards + assert_difference( + StateFilter( + types=frozendict( + {EventTypes.Member: frozenset({"@wombat:hs1", "@kristina:hs2"})} + ), + include_others=False, + ), + StateFilter( + types=frozendict({EventTypes.CanonicalAlias: frozenset({""})}), + include_others=True, + ), + StateFilter.none(), + ) From 0e0085c9a7d2509a81a72e9427a29457d397272d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 17 Sep 2021 12:21:53 +0100 Subject: [PATCH 07/34] STASH --- synapse/storage/databases/state/store.py | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/synapse/storage/databases/state/store.py b/synapse/storage/databases/state/store.py index f839c0c24f1c..c3181e4a3952 100644 --- a/synapse/storage/databases/state/store.py +++ b/synapse/storage/databases/state/store.py @@ -24,6 +24,7 @@ from synapse.storage.types import Cursor from synapse.storage.util.sequence import build_sequence_generator from synapse.types import MutableStateMap, StateMap +from synapse.util.async_helpers import ObservableDeferred from synapse.util.caches.descriptors import cached from synapse.util.caches.dictionary_cache import DictionaryCache @@ -91,6 +92,12 @@ def __init__(self, database: DatabasePool, db_conn, hs): 500000, ) + # Current ongoing get_state_for_groups in-flight requests + # {group ID -> {StateFilter -> ObservableDeferred}} + self._state_group_inflight_requests: Dict[ + int, Dict[StateFilter, ObservableDeferred[StateMap[str]]] + ] = {} + def get_max_state_group_txn(txn: Cursor): txn.execute("SELECT COALESCE(max(id), 0) FROM state_groups") return txn.fetchone()[0] @@ -249,6 +256,44 @@ async def _get_state_for_groups( if not incomplete_groups: return state + # try and rely on in-flight requests to complete this request without + # needing to spawn additional queries. + + # (group ID, ObservableDeferred of request result) + reusable_inflight_requests: List[ + Tuple[int, ObservableDeferred[StateMap[str]]] + ] = [] + # group ID -> left over StateFilter to request + requests_to_spawn: Dict[int, StateFilter] = {} + + for group in incomplete_groups: + requests_in_flight_for_group = self._state_group_inflight_requests.get( + group + ) + if requests_in_flight_for_group is None: + continue + + state_filter_left_over = state_filter + for ( + request_state_filter, + request_deferred, + ) in requests_in_flight_for_group.items(): + new_state_filter_left_over = state_filter_left_over.approx_difference( + request_state_filter + ) + if new_state_filter_left_over != state_filter_left_over: + # reusing this request narrows our StateFilter down a bit. + reusable_inflight_requests.append((group, request_deferred)) + state_filter_left_over = new_state_filter_left_over + if state_filter_left_over == StateFilter.none(): + # we have managed to collect enough of the in-flight requests + # to cover our StateFilter and give us the state we need. + break + else: + # we have some of the state filter left over, so need to spawn + # a request + requests_to_spawn[group] = state_filter_left_over + cache_sequence_nm = self._state_group_cache.sequence cache_sequence_m = self._state_group_members_cache.sequence From 9d50f05804d164b8762a332081d16bd3da4c1b3d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 20 Sep 2021 12:04:07 +0100 Subject: [PATCH 08/34] Tighten up the postconditions of `approx_difference` With thanks to David --- synapse/storage/state.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index b2f497a7c021..f0dc465cef7e 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -361,18 +361,20 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": Returns a state filter which represents `self - subtrahend`. The resultant state filter MUST admit all state events that are admitted - by only this filter (`self`) and not `subtrahend`. (1) + by only this filter (`self`) and not `subtrahend`. The resultant filter MAY be an over-approximation: the resultant state - filter MAY additionally admit other state events. + filter MAY additionally admit some state events from `subtrahend`. Formally, if the set of state events admitted by a state filter F are written as E(F), then the resultant state filter bears this property: - E(difference(self, subtrahend)) ⊇ E(self) ∖ E(subtrahend) + E(self) ∖ E(subtrahend) + ⊆ E(approx_difference(self, subtrahend)) + ⊆ E(self) - This function attempts to return the narrowest such state filter. + This implementation attempts to return the narrowest such state filter. In the case that `self` contains wildcards for state types where `subtrahend` contains specific state keys, an approximation must be made: the resultant state filter keeps the wildcard, as state filters are not From c72c436adc0ca757a6d69d5514104607f57c66ee Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 20 Sep 2021 15:09:33 +0100 Subject: [PATCH 09/34] =?UTF-8?q?More=20wordsmithing=20=E2=80=94=20thanks?= =?UTF-8?q?=20David?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- synapse/storage/state.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 2e5ab257200c..7b3649c6a715 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -360,10 +360,12 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": """ Returns a state filter which represents `self - subtrahend`. - The resultant state filter MUST admit all state events that are admitted - by only this filter (`self`) and not `subtrahend`. - The resultant filter MAY be an over-approximation: the resultant state - filter MAY additionally admit some state events from `subtrahend`. + The resultant state filter + - MUST admit all state events that are admitted + by only this filter (`self`) and not `subtrahend`; + - MUST NOT admit state events rejected by this filter (`self`); and + - MAY be an over-approximation: the resultant state + filter MAY additionally admit some state events from `subtrahend`. Formally, if the set of state events admitted by a state filter F are From bacd3940cad9d62132e14ffd210142a7a3b3a3b2 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 20 Sep 2021 15:28:25 +0100 Subject: [PATCH 10/34] Revert "STASH" This reverts commit 0e0085c9a7d2509a81a72e9427a29457d397272d. --- synapse/storage/databases/state/store.py | 45 ------------------------ 1 file changed, 45 deletions(-) diff --git a/synapse/storage/databases/state/store.py b/synapse/storage/databases/state/store.py index 6edc31625db2..c4c8c0021bca 100644 --- a/synapse/storage/databases/state/store.py +++ b/synapse/storage/databases/state/store.py @@ -29,7 +29,6 @@ from synapse.storage.types import Cursor from synapse.storage.util.sequence import build_sequence_generator from synapse.types import MutableStateMap, StateKey, StateMap -from synapse.util.async_helpers import ObservableDeferred from synapse.util.caches.descriptors import cached from synapse.util.caches.dictionary_cache import DictionaryCache @@ -107,12 +106,6 @@ def __init__( 500000, ) - # Current ongoing get_state_for_groups in-flight requests - # {group ID -> {StateFilter -> ObservableDeferred}} - self._state_group_inflight_requests: Dict[ - int, Dict[StateFilter, ObservableDeferred[StateMap[str]]] - ] = {} - def get_max_state_group_txn(txn: Cursor) -> int: txn.execute("SELECT COALESCE(max(id), 0) FROM state_groups") return txn.fetchone()[0] # type: ignore @@ -276,44 +269,6 @@ async def _get_state_for_groups( if not incomplete_groups: return state - # try and rely on in-flight requests to complete this request without - # needing to spawn additional queries. - - # (group ID, ObservableDeferred of request result) - reusable_inflight_requests: List[ - Tuple[int, ObservableDeferred[StateMap[str]]] - ] = [] - # group ID -> left over StateFilter to request - requests_to_spawn: Dict[int, StateFilter] = {} - - for group in incomplete_groups: - requests_in_flight_for_group = self._state_group_inflight_requests.get( - group - ) - if requests_in_flight_for_group is None: - continue - - state_filter_left_over = state_filter - for ( - request_state_filter, - request_deferred, - ) in requests_in_flight_for_group.items(): - new_state_filter_left_over = state_filter_left_over.approx_difference( - request_state_filter - ) - if new_state_filter_left_over != state_filter_left_over: - # reusing this request narrows our StateFilter down a bit. - reusable_inflight_requests.append((group, request_deferred)) - state_filter_left_over = new_state_filter_left_over - if state_filter_left_over == StateFilter.none(): - # we have managed to collect enough of the in-flight requests - # to cover our StateFilter and give us the state we need. - break - else: - # we have some of the state filter left over, so need to spawn - # a request - requests_to_spawn[group] = state_filter_left_over - cache_sequence_nm = self._state_group_cache.sequence cache_sequence_m = self._state_group_members_cache.sequence From cd1de9b0db41c7e411b19f18efcfb3b856856352 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 10:35:55 +0100 Subject: [PATCH 11/34] Remove needless set construction --- synapse/storage/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 7b3649c6a715..deec6772aa8b 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -405,7 +405,7 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": None if new_include_others else frozenset() ) state_types = set(subtrahend.types.keys()) - state_types.update(set(self.types.keys())) + state_types.update(self.types.keys()) for state_type in state_types: sub_keys: Optional[FrozenSet[str]] = subtrahend.types.get( state_type, subtrahend_default_for_unspecified From 6bedcba2b5e89bdc2f086bc4177133359af38cff Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 10:39:46 +0100 Subject: [PATCH 12/34] Simplify logic a bit, since this isn't operating in-place anyway --- synapse/storage/state.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index deec6772aa8b..94775da9552f 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -391,7 +391,7 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": is an over-approximation. """ - types = dict(self.types) + types = dict() new_include_others = self.include_others and not subtrahend.include_others # if this is an include_others state filter, then all unmentioned # event types are wildcards; otherwise they're empty. @@ -410,7 +410,7 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": sub_keys: Optional[FrozenSet[str]] = subtrahend.types.get( state_type, subtrahend_default_for_unspecified ) - current_keys: Optional[FrozenSet[str]] = types.get( + current_keys: Optional[FrozenSet[str]] = self.types.get( state_type, current_default_for_unspecified ) @@ -427,12 +427,7 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": # difference of those keys new_keys = current_keys.difference(sub_keys) - if new_keys == new_default_for_unspecified: - # if the result is the same as the default assumption, - # don't bother storing it. - if state_type in types: - types.pop(state_type) - else: + if new_keys != new_default_for_unspecified: # this is not the same as the default assumption, so # we store it types[state_type] = new_keys From 42617dbdef70a222962e60e03af03dacd70b159b Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 10:47:49 +0100 Subject: [PATCH 13/34] Attempt to clean up `approx_difference` with improved comments and names --- synapse/storage/state.py | 46 +++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 94775da9552f..f4f52da96723 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -391,48 +391,54 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": is an over-approximation. """ - types = dict() + new_types = {} new_include_others = self.include_others and not subtrahend.include_others - # if this is an include_others state filter, then all unmentioned - # event types are wildcards; otherwise they're empty. - subtrahend_default_for_unspecified: Optional[FrozenSet[str]] = ( - None if subtrahend.include_others else frozenset() - ) + + # When include_others is set, that means that all unmentioned state event + # types are equivalent to wildcards (i.e. None). + # When include_others is not set, that means that all unmentioned state event + # types are equivalent to empty sets of state keys. current_default_for_unspecified: Optional[FrozenSet[str]] = ( None if self.include_others else frozenset() ) + subtrahend_default_for_unspecified: Optional[FrozenSet[str]] = ( + None if subtrahend.include_others else frozenset() + ) new_default_for_unspecified: Optional[FrozenSet[str]] = ( None if new_include_others else frozenset() ) + + # Get all the state types we need to consider. state_types = set(subtrahend.types.keys()) state_types.update(self.types.keys()) + for state_type in state_types: - sub_keys: Optional[FrozenSet[str]] = subtrahend.types.get( - state_type, subtrahend_default_for_unspecified - ) current_keys: Optional[FrozenSet[str]] = self.types.get( state_type, current_default_for_unspecified ) + subtrahend_keys: Optional[FrozenSet[str]] = subtrahend.types.get( + state_type, subtrahend_default_for_unspecified + ) new_keys: Optional[FrozenSet[str]] - if sub_keys is None: - # anything minus all is none + if subtrahend_keys is None: + # (anything) removing everything leaves you with nothing new_keys = frozenset() elif current_keys is None: - # all minus a few specific keys is something we can only - # approximate as 'all' + # everything with a few specific keys removed is something we + # can only approximate as 'everything' new_keys = None else: - # a few specific keys minus a few specific keys is just the set - # difference of those keys - new_keys = current_keys.difference(sub_keys) + # a few specific keys with a few specific keys removed is just + # the set difference of those keys + new_keys = current_keys.difference(subtrahend_keys) if new_keys != new_default_for_unspecified: - # this is not the same as the default assumption, so - # we store it - types[state_type] = new_keys + # the keys for this state type are not the same as the default + # implied by `include_others`, so we will store them explicitly. + new_types[state_type] = new_keys - return StateFilter(frozendict(types), include_others=new_include_others) + return StateFilter(frozendict(new_types), include_others=new_include_others) class StateGroupStorage: From 0c8e9301e4366305f20d0103e41ad36ce4c0c57d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 14:22:43 +0100 Subject: [PATCH 14/34] Split out tests into own TestCase class --- tests/storage/test_state.py | 43 +++++++++++++++---------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py index daa3fc0c5895..0e9ef1a39957 100644 --- a/tests/storage/test_state.py +++ b/tests/storage/test_state.py @@ -483,40 +483,31 @@ def test_get_state_for_event(self): self.assertEqual(is_all, True) self.assertDictEqual({(e5.type, e5.state_key): e5.event_id}, state_dict) - def test_state_filter_difference(self): - def assert_difference( - minuend: StateFilter, subtrahend: StateFilter, expected: StateFilter - ): - self.assertEqual( - minuend.approx_difference(subtrahend), - expected, - f"StateFilter difference not correct:\n\n\t{minuend!r}\nminus\n\t{subtrahend!r}\nwas\n\t{minuend.approx_difference(subtrahend)}\nexpected\n\t{expected}", - ) +class StateFilterDifferenceTestCase(HomeserverTestCase): + def assert_difference( + self, minuend: StateFilter, subtrahend: StateFilter, expected: StateFilter + ): + self.assertEqual( + minuend.approx_difference(subtrahend), + expected, + f"StateFilter difference not correct:\n\n\t{minuend!r}\nminus\n\t{subtrahend!r}\nwas\n\t{minuend.approx_difference(subtrahend)}\nexpected\n\t{expected}", + ) + + def test_state_filter_difference(self): # it's not possible to subtract individual state keys from # a wildcard - assert_difference( + self.assert_difference( StateFilter.all(), StateFilter.from_types( ((EventTypes.Member, "@wombat:hs1"), (EventTypes.Member, "@spqr:hs1")) ), StateFilter.all(), ) - self.assertEqual( - StateFilter.all().approx_difference( - StateFilter.from_types( - ( - (EventTypes.Member, "@wombat:hs1"), - (EventTypes.Member, "@spqr:hs1"), - ) - ) - ), - StateFilter.all(), - ) # we can subtract wildcards from wildcards - assert_difference(StateFilter.all(), StateFilter.all(), StateFilter.none()) - assert_difference( + self.assert_difference(StateFilter.all(), StateFilter.all(), StateFilter.none()) + self.assert_difference( StateFilter( types=frozendict( { @@ -538,7 +529,7 @@ def assert_difference( ) # subtract two strange wildcards - assert_difference( + self.assert_difference( StateFilter( types=frozendict({EventTypes.Member: None}), include_others=True, @@ -554,7 +545,7 @@ def assert_difference( ) # we can subtract individual state keys, except from wildcards - assert_difference( + self.assert_difference( StateFilter( types=frozendict( { @@ -585,7 +576,7 @@ def assert_difference( ) # we can subtract wildcards from non-wildcards - assert_difference( + self.assert_difference( StateFilter( types=frozendict( {EventTypes.Member: frozenset({"@wombat:hs1", "@kristina:hs2"})} From b6274d6a1114bea85474127da59dffcfc2616483 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 15:41:02 +0100 Subject: [PATCH 15/34] Add extensive tests for all 4 combinations of include_others --- tests/storage/test_state.py | 569 +++++++++++++++++++++++++++++++++++- 1 file changed, 567 insertions(+), 2 deletions(-) diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py index 0e9ef1a39957..cadbf14dc692 100644 --- a/tests/storage/test_state.py +++ b/tests/storage/test_state.py @@ -21,7 +21,7 @@ from synapse.storage.state import StateFilter from synapse.types import RoomID, UserID -from tests.unittest import HomeserverTestCase +from tests.unittest import HomeserverTestCase, TestCase logger = logging.getLogger(__name__) @@ -484,7 +484,7 @@ def test_get_state_for_event(self): self.assertDictEqual({(e5.type, e5.state_key): e5.event_id}, state_dict) -class StateFilterDifferenceTestCase(HomeserverTestCase): +class StateFilterDifferenceTestCase(TestCase): def assert_difference( self, minuend: StateFilter, subtrahend: StateFilter, expected: StateFilter ): @@ -494,6 +494,571 @@ def assert_difference( f"StateFilter difference not correct:\n\n\t{minuend!r}\nminus\n\t{subtrahend!r}\nwas\n\t{minuend.approx_difference(subtrahend)}\nexpected\n\t{expected}", ) + def test_state_filter_difference_no_include_other_minus_no_include_other(self): + """ + Tests the StateFilter.approx_difference method + where, in a.approx_difference(b), both a and b do not have the + include_others flag set. + """ + # (wildcard on state keys) - (wildcard on state keys): + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None, EventTypes.Create: None}), + include_others=False, + ), + StateFilter( + types=frozendict( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None} + ), + include_others=False, + ), + StateFilter( + types=frozendict({EventTypes.Create: None}), include_others=False + ), + ) + + # (wildcard on state keys) - (specific state keys) + # This one is an over-approximation because we can't represent + # 'all state keys except a few named examples' + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), include_others=False + ), + StateFilter( + types=frozendict({EventTypes.Member: frozenset({"@wombat:spqr"})}), + include_others=False, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), include_others=False + ), + ) + + # (wildcard on state keys) - (no state keys) + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=False, + ), + ) + + # (specific state keys) - (wildcard on state keys): + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=False, + ), + StateFilter( + types=frozendict({EventTypes.CanonicalAlias: frozenset({""})}), + include_others=False, + ), + ) + + # (specific state keys) - (specific state keys) + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr"}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + ) + + # (specific state keys) - (no state keys) + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + ) + + def test_state_filter_difference_include_other_minus_no_include_other(self): + """ + Tests the StateFilter.approx_difference method + where, in a.approx_difference(b), only a has the include_others flag set. + """ + # (wildcard on state keys) - (wildcard on state keys): + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None, EventTypes.Create: None}), + include_others=True, + ), + StateFilter( + types=frozendict( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None} + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Create: None, + EventTypes.Member: frozenset(), + EventTypes.CanonicalAlias: frozenset(), + } + ), + include_others=True, + ), + ) + + # (wildcard on state keys) - (specific state keys) + # This one is an over-approximation because we can't represent + # 'all state keys except a few named examples' + # This also shows that the resultant state filter is normalised. + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), include_others=True + ), + StateFilter( + types=frozendict({EventTypes.Member: frozenset({"@wombat:spqr"})}), + include_others=False, + ), + StateFilter(types=frozendict(), include_others=True), + ) + + # (wildcard on state keys) - (no state keys) + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict(), + include_others=True, + ), + ) + + # (specific state keys) - (wildcard on state keys): + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.CanonicalAlias: frozenset({""}), + EventTypes.Member: frozenset(), + } + ), + include_others=True, + ), + ) + + # (specific state keys) - (specific state keys) + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr"}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=True, + ), + ) + + # (specific state keys) - (no state keys) + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=True, + ), + ) + + def test_state_filter_difference_include_other_minus_include_other(self): + """ + Tests the StateFilter.approx_difference method + where, in a.approx_difference(b), both a and b have the include_others + flag set. + """ + # (wildcard on state keys) - (wildcard on state keys): + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None, EventTypes.Create: None}), + include_others=True, + ), + StateFilter( + types=frozendict( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None} + ), + include_others=True, + ), + StateFilter(types=frozendict(), include_others=False), + ) + + # (wildcard on state keys) - (specific state keys) + # This one is an over-approximation because we can't represent + # 'all state keys except a few named examples' + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), include_others=True + ), + StateFilter( + types=frozendict({EventTypes.Member: frozenset({"@wombat:spqr"})}), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), include_others=False + ), + ) + + # (wildcard on state keys) - (no state keys) + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=False, + ), + ) + + # (specific state keys) - (wildcard on state keys): + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=True, + ), + StateFilter( + types=frozendict(), + include_others=False, + ), + ) + + # (specific state keys) - (specific state keys) + # This one is an over-approximation because we can't represent + # 'all state keys except a few named examples' + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr"}), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@spqr:spqr"}), + } + ), + include_others=False, + ), + ) + + # (specific state keys) - (no state keys) + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + } + ), + include_others=False, + ), + ) + + def test_state_filter_difference_no_include_other_minus_include_other(self): + """ + Tests the StateFilter.approx_difference method + where, in a.approx_difference(b), only b has the include_others flag set. + """ + # (wildcard on state keys) - (wildcard on state keys): + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None, EventTypes.Create: None}), + include_others=False, + ), + StateFilter( + types=frozendict( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None} + ), + include_others=True, + ), + StateFilter(types=frozendict(), include_others=False), + ) + + # (wildcard on state keys) - (specific state keys) + # This one is an over-approximation because we can't represent + # 'all state keys except a few named examples' + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), include_others=False + ), + StateFilter( + types=frozendict({EventTypes.Member: frozenset({"@wombat:spqr"})}), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), include_others=False + ), + ) + + # (wildcard on state keys) - (no state keys) + self.assert_difference( + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=False, + ), + ) + + # (specific state keys) - (wildcard on state keys): + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict({EventTypes.Member: None}), + include_others=True, + ), + StateFilter( + types=frozendict(), + include_others=False, + ), + ) + + # (specific state keys) - (specific state keys) + # This one is an over-approximation because we can't represent + # 'all state keys except a few named examples' + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr"}), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@spqr:spqr"}), + } + ), + include_others=False, + ), + ) + + # (specific state keys) - (no state keys) + self.assert_difference( + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), + include_others=False, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset(), + } + ), + include_others=True, + ), + StateFilter( + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), + } + ), + include_others=False, + ), + ) + def test_state_filter_difference(self): # it's not possible to subtract individual state keys from # a wildcard From f6b4dc5b2594f1a4f7e18f1a2f500fd10a4c6cca Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 15:43:31 +0100 Subject: [PATCH 16/34] Merge a test --- tests/storage/test_state.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py index cadbf14dc692..d0beefa84c22 100644 --- a/tests/storage/test_state.py +++ b/tests/storage/test_state.py @@ -672,7 +672,12 @@ def test_state_filter_difference_include_other_minus_no_include_other(self): types=frozendict({EventTypes.Member: None}), include_others=True ), StateFilter( - types=frozendict({EventTypes.Member: frozenset({"@wombat:spqr"})}), + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr"}), + EventTypes.Create: frozenset({""}), + } + ), include_others=False, ), StateFilter(types=frozendict(), include_others=True), @@ -1060,16 +1065,6 @@ def test_state_filter_difference_no_include_other_minus_include_other(self): ) def test_state_filter_difference(self): - # it's not possible to subtract individual state keys from - # a wildcard - self.assert_difference( - StateFilter.all(), - StateFilter.from_types( - ((EventTypes.Member, "@wombat:hs1"), (EventTypes.Member, "@spqr:hs1")) - ), - StateFilter.all(), - ) - # we can subtract wildcards from wildcards self.assert_difference(StateFilter.all(), StateFilter.all(), StateFilter.none()) self.assert_difference( From 0d1c3d86941cfaee0ae484471ea84c91ac374c1f Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 15:46:41 +0100 Subject: [PATCH 17/34] Split out some very simple tests --- tests/storage/test_state.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py index d0beefa84c22..0cc859d7d14a 100644 --- a/tests/storage/test_state.py +++ b/tests/storage/test_state.py @@ -1064,9 +1064,26 @@ def test_state_filter_difference_no_include_other_minus_include_other(self): ), ) + def test_state_filter_difference_simple_cases(self): + """ + Tests some very simple cases of the StateFilter approx_difference, + that are not explicitly tested by the more in-depth tests. + """ + + self.assert_difference( + StateFilter.all(), + StateFilter.all(), + StateFilter.none() + ) + + self.assert_difference( + StateFilter.all(), + StateFilter.none(), + StateFilter.all(), + ) + def test_state_filter_difference(self): # we can subtract wildcards from wildcards - self.assert_difference(StateFilter.all(), StateFilter.all(), StateFilter.none()) self.assert_difference( StateFilter( types=frozendict( From 18714d70fd1c0f82ae54d515969d10e3cb7fd80a Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 15:52:50 +0100 Subject: [PATCH 18/34] Deduplicate the old tests into the systematic style tests --- tests/storage/test_state.py | 106 +++++------------------------------- 1 file changed, 14 insertions(+), 92 deletions(-) diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py index 0cc859d7d14a..dc8ccdbb14e6 100644 --- a/tests/storage/test_state.py +++ b/tests/storage/test_state.py @@ -818,11 +818,19 @@ def test_state_filter_difference_include_other_minus_include_other(self): types=frozendict({EventTypes.Member: None}), include_others=True ), StateFilter( - types=frozendict({EventTypes.Member: frozenset({"@wombat:spqr"})}), + types=frozendict( + { + EventTypes.Member: frozenset({"@wombat:spqr"}), + EventTypes.CanonicalAlias: frozenset({""}), + } + ), include_others=True, ), StateFilter( - types=frozendict({EventTypes.Member: None}), include_others=False + types=frozendict( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None} + ), + include_others=False, ), ) @@ -876,6 +884,7 @@ def test_state_filter_difference_include_other_minus_include_other(self): { EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), EventTypes.CanonicalAlias: frozenset({""}), + EventTypes.Create: frozenset({""}), } ), include_others=True, @@ -884,6 +893,7 @@ def test_state_filter_difference_include_other_minus_include_other(self): types=frozendict( { EventTypes.Member: frozenset({"@wombat:spqr"}), + EventTypes.Create: frozenset(), } ), include_others=True, @@ -892,6 +902,7 @@ def test_state_filter_difference_include_other_minus_include_other(self): types=frozendict( { EventTypes.Member: frozenset({"@spqr:spqr"}), + EventTypes.Create: frozenset({""}), } ), include_others=False, @@ -1070,99 +1081,10 @@ def test_state_filter_difference_simple_cases(self): that are not explicitly tested by the more in-depth tests. """ - self.assert_difference( - StateFilter.all(), - StateFilter.all(), - StateFilter.none() - ) + self.assert_difference(StateFilter.all(), StateFilter.all(), StateFilter.none()) self.assert_difference( StateFilter.all(), StateFilter.none(), StateFilter.all(), ) - - def test_state_filter_difference(self): - # we can subtract wildcards from wildcards - self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - EventTypes.CanonicalAlias: None, - } - ), - include_others=True, - ), - StateFilter( - types=frozendict({EventTypes.JoinRules: None}), include_others=False - ), - StateFilter( - types=frozendict( - {EventTypes.Member: frozenset(), EventTypes.JoinRules: frozenset()} - ), - include_others=True, - ), - ) - - # subtract two strange wildcards - self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), - include_others=True, - ), - StateFilter( - types=frozendict({EventTypes.CanonicalAlias: frozenset({""})}), - include_others=True, - ), - StateFilter( - types=frozendict({EventTypes.CanonicalAlias: None}), - include_others=False, - ), - ) - - # we can subtract individual state keys, except from wildcards - self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:hs1", "@kristina:hs2"}), - EventTypes.CanonicalAlias: None, - } - ), - include_others=False, - ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@kristina:hs2"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), - include_others=False, - ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:hs1"}), - EventTypes.CanonicalAlias: None, - } - ), - include_others=False, - ), - ) - - # we can subtract wildcards from non-wildcards - self.assert_difference( - StateFilter( - types=frozendict( - {EventTypes.Member: frozenset({"@wombat:hs1", "@kristina:hs2"})} - ), - include_others=False, - ), - StateFilter( - types=frozendict({EventTypes.CanonicalAlias: frozenset({""})}), - include_others=True, - ), - StateFilter.none(), - ) From 770afeaed3b31964e45a8f8f16a3025bc54aeea7 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 16:02:31 +0100 Subject: [PATCH 19/34] Add function to decompose a StateFilter into four parts --- synapse/storage/state.py | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index f4f52da96723..39f22a452bb9 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -29,7 +29,7 @@ from synapse.api.constants import EventTypes from synapse.events import EventBase -from synapse.types import MutableStateMap, StateMap +from synapse.types import MutableStateMap, StateKey, StateMap if TYPE_CHECKING: from typing import FrozenSet # noqa: used within quoted type hint; flake8 sad @@ -356,6 +356,40 @@ def get_member_split(self) -> Tuple["StateFilter", "StateFilter"]: return member_filter, non_member_filter + def decompose_into_four_parts( + self, + ) -> Tuple[bool, Set[str], Set[str], Set[StateKey]]: + """ + Decomposes this state filter into 4 constituent parts, which can be + thought of as this: + all? - minus_wildcards + plus_wildcards + plus_state_keys + + where + * all represents ALL state + * minus_wildcards represents entire state types to remove + * plus_wildcards represents entire state types to add + * plus_state_keys represents individual state keys to add + """ + all_part: bool = self.include_others + minus_wildcards: Set[str] = set() + plus_wildcards: Set[str] = set() + plus_state_keys: Set[StateKey] = set() + + for state_type, state_keys in self.types.items(): + if state_keys is None: + # this is a wildcard + if not all_part: + plus_wildcards.add(state_type) + else: + if all_part: + # we remove the wildcard on this type, because we will + # instead specify individual keys + minus_wildcards.add(state_type) + for state_key in state_keys: + plus_state_keys.add((state_type, state_key)) + + return all_part, minus_wildcards, plus_wildcards, plus_state_keys + def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": """ Returns a state filter which represents `self - subtrahend`. From e119af9e7da243ee64319f6bed508e7dfd3502df Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 16:13:16 +0100 Subject: [PATCH 20/34] Add `StateFilter.freeze` convenience constructor --- synapse/storage/state.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 39f22a452bb9..3f29aab58242 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -15,9 +15,11 @@ from typing import ( TYPE_CHECKING, Awaitable, + Collection, Dict, Iterable, List, + Mapping, Optional, Set, Tuple, @@ -134,6 +136,23 @@ def from_lazy_load_member_list(members: Iterable[str]) -> "StateFilter": include_others=True, ) + @staticmethod + def freeze(types: Mapping[str, Optional[Collection[str]]], include_others: bool): + """ + Returns a (frozen) StateFilter with the same contents as the parameters + specified here, which can be made of mutable types. + """ + types_with_frozen_values: Dict[str, Optional[FrozenSet[str]]] = {} + for state_types, state_keys in types.items(): + if state_keys is not None: + types_with_frozen_values[state_types] = frozenset(state_keys) + else: + types_with_frozen_values[state_types] = None + + return StateFilter( + frozendict(types_with_frozen_values), include_others=include_others + ) + def return_expanded(self) -> "StateFilter": """Creates a new StateFilter where type wild cards have been removed (except for memberships). The returned filter is a superset of the From 70f646accb1a0334d5224dbb64bd4b922c7e2519 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 22 Sep 2021 17:30:22 +0100 Subject: [PATCH 21/34] Add `recompose_from_four_parts` method as inverse --- synapse/storage/state.py | 58 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 3f29aab58242..2d09cd5c9f3d 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -388,6 +388,9 @@ def decompose_into_four_parts( * minus_wildcards represents entire state types to remove * plus_wildcards represents entire state types to add * plus_state_keys represents individual state keys to add + + See `recompose_from_four_parts` for the other direction of this + correspondence. """ all_part: bool = self.include_others minus_wildcards: Set[str] = set() @@ -409,6 +412,61 @@ def decompose_into_four_parts( return all_part, minus_wildcards, plus_wildcards, plus_state_keys + @staticmethod + def recompose_from_four_parts( + all_part: bool, + minus_wildcards: Set[str], + plus_wildcards: Set[str], + plus_state_keys: Set[StateKey], + ) -> "StateFilter": + """ + Recomposes a state filter from 4 parts. + + See `decompose_into_four_parts` (the other direction of this + correspondence) for descriptions on each of the parts. + """ + + # {state type -> set of state keys OR None for wildcard} + # (The same structure as that of a StateFilter.) + new_types: Dict[str, Optional[Set[str]]] = {} + + if all_part: + for minus_wildcard in minus_wildcards: + if minus_wildcard not in plus_wildcards: + # as long as the wildcard is not added back in, we + # reset that state type to allow no state keys. + new_types[minus_wildcard] = set() + + for state_type, state_key in plus_state_keys: + # We don't bother adding the state key if there's no set for + # its state type, because that means it's already included + # by virtue of the 'all' part. + if state_type in new_types: + state_set = new_types[state_type] + # We only just added this set so it can't be None. + assert state_set is not None + # Add the state key to the set. + state_set.add(state_key) + else: + # As we don't start out with any types to begin with, we don't + # care about 'minus' wildcards. + + # Add all 'plus' wildcards. + for plus_wildcard in plus_wildcards: + new_types[plus_wildcard] = None + + # Add all the 'plus' state keys, taking care not to overwrite any + # wildcards that have already been added. + for state_type, state_key in plus_state_keys: + if state_type in new_types: + state_set = new_types[state_type] + if state_set is not None: + state_set.add(state_key) + else: + new_types[state_type] = {state_key} + + return StateFilter.freeze(new_types, include_others=all_part) + def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": """ Returns a state filter which represents `self - subtrahend`. From c9bb22665fdaebaf8f3061fd205d83fd59bf9123 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 24 Sep 2021 12:31:17 +0100 Subject: [PATCH 22/34] Use a shorter version of `decompose_into_four_parts`. --- synapse/storage/state.py | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 2d09cd5c9f3d..aeec0d6241b7 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -377,7 +377,7 @@ def get_member_split(self) -> Tuple["StateFilter", "StateFilter"]: def decompose_into_four_parts( self, - ) -> Tuple[bool, Set[str], Set[str], Set[StateKey]]: + ) -> Tuple[Tuple[bool, Set[str]], Tuple[Set[str], Set[StateKey]]]: """ Decomposes this state filter into 4 constituent parts, which can be thought of as this: @@ -392,25 +392,12 @@ def decompose_into_four_parts( See `recompose_from_four_parts` for the other direction of this correspondence. """ - all_part: bool = self.include_others - minus_wildcards: Set[str] = set() - plus_wildcards: Set[str] = set() - plus_state_keys: Set[StateKey] = set() + is_all = self.include_others + excluded_types: Set[str] = {t for t in self.types if is_all} + wildcard_types: Set[str] = {t for t, s in self.types.items() if s is None} + concrete_keys: Set[StateKey] = set(self.concrete_types()) - for state_type, state_keys in self.types.items(): - if state_keys is None: - # this is a wildcard - if not all_part: - plus_wildcards.add(state_type) - else: - if all_part: - # we remove the wildcard on this type, because we will - # instead specify individual keys - minus_wildcards.add(state_type) - for state_key in state_keys: - plus_state_keys.add((state_type, state_key)) - - return all_part, minus_wildcards, plus_wildcards, plus_state_keys + return (is_all, excluded_types), (wildcard_types, concrete_keys) @staticmethod def recompose_from_four_parts( From 093f67034dc4a3f0f927621c7a825c9705580951 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 24 Sep 2021 12:32:34 +0100 Subject: [PATCH 23/34] Use a shorter version of `recompose_from_four_parts` --- synapse/storage/state.py | 51 ++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 33 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index aeec0d6241b7..d941945afc9c 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -417,40 +417,25 @@ def recompose_from_four_parts( # (The same structure as that of a StateFilter.) new_types: Dict[str, Optional[Set[str]]] = {} + # if we start with all, insert the excluded statetypes as empty sets + # to prevent them from being included if all_part: - for minus_wildcard in minus_wildcards: - if minus_wildcard not in plus_wildcards: - # as long as the wildcard is not added back in, we - # reset that state type to allow no state keys. - new_types[minus_wildcard] = set() - - for state_type, state_key in plus_state_keys: - # We don't bother adding the state key if there's no set for - # its state type, because that means it's already included - # by virtue of the 'all' part. - if state_type in new_types: - state_set = new_types[state_type] - # We only just added this set so it can't be None. - assert state_set is not None - # Add the state key to the set. - state_set.add(state_key) - else: - # As we don't start out with any types to begin with, we don't - # care about 'minus' wildcards. - - # Add all 'plus' wildcards. - for plus_wildcard in plus_wildcards: - new_types[plus_wildcard] = None - - # Add all the 'plus' state keys, taking care not to overwrite any - # wildcards that have already been added. - for state_type, state_key in plus_state_keys: - if state_type in new_types: - state_set = new_types[state_type] - if state_set is not None: - state_set.add(state_key) - else: - new_types[state_type] = {state_key} + new_types.update({state_type: set() for state_type in minus_wildcards}) + + # insert the plus wildcards + new_types.update({state_type: None for state_type in plus_wildcards}) + + # insert the specific state keys + for state_type, state_key in plus_state_keys: + if state_type in new_types: + entry = new_types[state_type] + if entry is not None: + entry.add(state_key) + elif not all_part: + # don't insert if the entire type is already included by + # include_others as this would actually shrink the state allowed + # by this filter. + new_types[state_type] = {state_key} return StateFilter.freeze(new_types, include_others=all_part) From a187c240786bb67d97038aa52ad32135073929cb Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 24 Sep 2021 12:35:40 +0100 Subject: [PATCH 24/34] Use a step-by-step implementation of `approx_difference` rather than splitting along types --- synapse/storage/state.py | 104 ++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 46 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index d941945afc9c..a612dd16227b 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -474,54 +474,66 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": is an over-approximation. """ - new_types = {} - new_include_others = self.include_others and not subtrahend.include_others - - # When include_others is set, that means that all unmentioned state event - # types are equivalent to wildcards (i.e. None). - # When include_others is not set, that means that all unmentioned state event - # types are equivalent to empty sets of state keys. - current_default_for_unspecified: Optional[FrozenSet[str]] = ( - None if self.include_others else frozenset() - ) - subtrahend_default_for_unspecified: Optional[FrozenSet[str]] = ( - None if subtrahend.include_others else frozenset() - ) - new_default_for_unspecified: Optional[FrozenSet[str]] = ( - None if new_include_others else frozenset() - ) - - # Get all the state types we need to consider. - state_types = set(subtrahend.types.keys()) - state_types.update(self.types.keys()) - - for state_type in state_types: - current_keys: Optional[FrozenSet[str]] = self.types.get( - state_type, current_default_for_unspecified - ) - subtrahend_keys: Optional[FrozenSet[str]] = subtrahend.types.get( - state_type, subtrahend_default_for_unspecified + # We first transform self and subtrahend into an alternative representation: + # - whether or not they include all events to begin with ('all') + # - if so, which event types are excluded? ('excludes') + # - which entire event types to include ('wildcards') + # - which concrete state keys to include ('concrete state keys') + (self_all, self_excludes), ( + self_wildcards, + self_concrete_keys, + ) = self.decompose_into_four_parts() + (sub_all, sub_excludes), ( + sub_wildcards, + sub_concrete_keys, + ) = subtrahend.decompose_into_four_parts() + + # Start with an estimate of the difference based on self + new_all = self_all + # Wildcards from the subtrahend can be added to the exclusion filter + new_excludes = self_excludes | sub_wildcards + # We remove wildcards that appeared as wildcards in the subtrahend + new_wildcards = self_wildcards - sub_wildcards + # We filter out the concrete state keys that appear in the subtrahend + # as wildcards or concrete state keys. + new_concrete_keys = { + (state_type, state_key) + for (state_type, state_key) in self_concrete_keys + if state_type not in sub_wildcards + } - sub_concrete_keys + + if self_all: + self_concretes_or_wild_types = self_wildcards.union( + typ for typ, _key in self_concrete_keys ) + # If self starts with all, then we add as wildcards any + # types which appear in the subtrahend's exclusion filter but do + # not already appear in self's concrete or wild types. + # This is needed to handle the fact that `include_others` includes + # all unspecified state types. + new_wildcards |= sub_excludes.difference(self_concretes_or_wild_types) + + if sub_all: + # If subtrahend is an `include_others` then the difference isn't. + new_all = False + # (We have no need for excludes when we don't start with all, as there + # is nothing to exclude.) + new_excludes = set() + + # We also filter out all state types that aren't in the exclusion + # list of the subtrahend. + new_wildcards &= sub_excludes + new_concrete_keys = { + (state_type, state_key) + for (state_type, state_key) in new_concrete_keys + if state_type in sub_excludes + } - new_keys: Optional[FrozenSet[str]] - if subtrahend_keys is None: - # (anything) removing everything leaves you with nothing - new_keys = frozenset() - elif current_keys is None: - # everything with a few specific keys removed is something we - # can only approximate as 'everything' - new_keys = None - else: - # a few specific keys with a few specific keys removed is just - # the set difference of those keys - new_keys = current_keys.difference(subtrahend_keys) - - if new_keys != new_default_for_unspecified: - # the keys for this state type are not the same as the default - # implied by `include_others`, so we will store them explicitly. - new_types[state_type] = new_keys - - return StateFilter(frozendict(new_types), include_others=new_include_others) + # Transform our newly-constructed state filter from the alternative + # representation back into the normal StateFilter representation. + return StateFilter.recompose_from_four_parts( + new_all, new_excludes, new_wildcards, new_concrete_keys + ) class StateGroupStorage: From 4bbe3d164cee76dda430d5035438efc5e9a0185d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 24 Sep 2021 14:23:19 +0100 Subject: [PATCH 25/34] Nest the ifs for clarity; no functional change --- synapse/storage/state.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index a612dd16227b..584d11e646d4 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -502,18 +502,18 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": if state_type not in sub_wildcards } - sub_concrete_keys - if self_all: - self_concretes_or_wild_types = self_wildcards.union( - typ for typ, _key in self_concrete_keys - ) - # If self starts with all, then we add as wildcards any - # types which appear in the subtrahend's exclusion filter but do - # not already appear in self's concrete or wild types. - # This is needed to handle the fact that `include_others` includes - # all unspecified state types. - new_wildcards |= sub_excludes.difference(self_concretes_or_wild_types) - if sub_all: + if self_all: + self_concretes_or_wild_types = self_wildcards.union( + typ for typ, _key in self_concrete_keys + ) + # If self starts with all, then we add as wildcards any + # types which appear in the subtrahend's exclusion filter but do + # not already appear in self's concrete or wild types. + # This is needed to handle the fact that `include_others` includes + # all unspecified state types. + new_wildcards |= sub_excludes.difference(self_concretes_or_wild_types) + # If subtrahend is an `include_others` then the difference isn't. new_all = False # (We have no need for excludes when we don't start with all, as there From 20bc2990713d64aa1ac936c42316dab8123b94af Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 24 Sep 2021 14:23:56 +0100 Subject: [PATCH 26/34] Prefix decompose/recompose method names with underscores --- synapse/storage/state.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 584d11e646d4..35553d744a51 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -375,7 +375,7 @@ def get_member_split(self) -> Tuple["StateFilter", "StateFilter"]: return member_filter, non_member_filter - def decompose_into_four_parts( + def _decompose_into_four_parts( self, ) -> Tuple[Tuple[bool, Set[str]], Tuple[Set[str], Set[StateKey]]]: """ @@ -400,7 +400,7 @@ def decompose_into_four_parts( return (is_all, excluded_types), (wildcard_types, concrete_keys) @staticmethod - def recompose_from_four_parts( + def _recompose_from_four_parts( all_part: bool, minus_wildcards: Set[str], plus_wildcards: Set[str], @@ -482,11 +482,11 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": (self_all, self_excludes), ( self_wildcards, self_concrete_keys, - ) = self.decompose_into_four_parts() + ) = self._decompose_into_four_parts() (sub_all, sub_excludes), ( sub_wildcards, sub_concrete_keys, - ) = subtrahend.decompose_into_four_parts() + ) = subtrahend._decompose_into_four_parts() # Start with an estimate of the difference based on self new_all = self_all @@ -531,7 +531,7 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": # Transform our newly-constructed state filter from the alternative # representation back into the normal StateFilter representation. - return StateFilter.recompose_from_four_parts( + return StateFilter._recompose_from_four_parts( new_all, new_excludes, new_wildcards, new_concrete_keys ) From 27c3a7a38bd749308ee1e1a73e37b04e9836e35e Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 24 Sep 2021 14:36:52 +0100 Subject: [PATCH 27/34] Use self_excludes as it's equivalent with this definition of decompose --- synapse/storage/state.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 35553d744a51..d5b78f4f53ea 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -504,15 +504,13 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": if sub_all: if self_all: - self_concretes_or_wild_types = self_wildcards.union( - typ for typ, _key in self_concrete_keys - ) # If self starts with all, then we add as wildcards any - # types which appear in the subtrahend's exclusion filter but do - # not already appear in self's concrete or wild types. - # This is needed to handle the fact that `include_others` includes - # all unspecified state types. - new_wildcards |= sub_excludes.difference(self_concretes_or_wild_types) + # types which appear in the subtrahend's exclusion filter (but + # aren't in the self exclusion filter). This is as the subtrahend + # filter will return everything BUT the types in its exclusion, so + # we need to add those excluded types that also match the self + # filter as wildcard types in the new filter. + new_wildcards |= sub_excludes.difference(self_excludes) # If subtrahend is an `include_others` then the difference isn't. new_all = False From 54d77c909dc45622a87c81cb6cc7e6e92991747e Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 28 Sep 2021 14:11:53 +0100 Subject: [PATCH 28/34] Rename `subtrahend` to `other` to follow convention --- synapse/storage/state.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index d5b78f4f53ea..9295d39394a9 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -439,29 +439,29 @@ def _recompose_from_four_parts( return StateFilter.freeze(new_types, include_others=all_part) - def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": + def approx_difference(self, other: "StateFilter") -> "StateFilter": """ - Returns a state filter which represents `self - subtrahend`. + Returns a state filter which represents `self - other`. The resultant state filter - MUST admit all state events that are admitted - by only this filter (`self`) and not `subtrahend`; + by only this filter (`self`) and not `other`; - MUST NOT admit state events rejected by this filter (`self`); and - MAY be an over-approximation: the resultant state - filter MAY additionally admit some state events from `subtrahend`. + filter MAY additionally admit some state events from `other`. Formally, if the set of state events admitted by a state filter F are written as E(F), then the resultant state filter bears this property: - E(self) ∖ E(subtrahend) - ⊆ E(approx_difference(self, subtrahend)) + E(self) ∖ E(other) + ⊆ E(approx_difference(self, other)) ⊆ E(self) This implementation attempts to return the narrowest such state filter. In the case that `self` contains wildcards for state types where - `subtrahend` contains specific state keys, an approximation must be made: + `other` contains specific state keys, an approximation must be made: the resultant state filter keeps the wildcard, as state filters are not able to express 'all state keys except some given examples'. e.g. @@ -474,7 +474,7 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": is an over-approximation. """ - # We first transform self and subtrahend into an alternative representation: + # We first transform self and other into an alternative representation: # - whether or not they include all events to begin with ('all') # - if so, which event types are excluded? ('excludes') # - which entire event types to include ('wildcards') @@ -486,15 +486,15 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": (sub_all, sub_excludes), ( sub_wildcards, sub_concrete_keys, - ) = subtrahend._decompose_into_four_parts() + ) = other._decompose_into_four_parts() # Start with an estimate of the difference based on self new_all = self_all - # Wildcards from the subtrahend can be added to the exclusion filter + # Wildcards from the other can be added to the exclusion filter new_excludes = self_excludes | sub_wildcards - # We remove wildcards that appeared as wildcards in the subtrahend + # We remove wildcards that appeared as wildcards in the other new_wildcards = self_wildcards - sub_wildcards - # We filter out the concrete state keys that appear in the subtrahend + # We filter out the concrete state keys that appear in the other # as wildcards or concrete state keys. new_concrete_keys = { (state_type, state_key) @@ -505,21 +505,21 @@ def approx_difference(self, subtrahend: "StateFilter") -> "StateFilter": if sub_all: if self_all: # If self starts with all, then we add as wildcards any - # types which appear in the subtrahend's exclusion filter (but - # aren't in the self exclusion filter). This is as the subtrahend + # types which appear in the other's exclusion filter (but + # aren't in the self exclusion filter). This is as the other # filter will return everything BUT the types in its exclusion, so # we need to add those excluded types that also match the self # filter as wildcard types in the new filter. new_wildcards |= sub_excludes.difference(self_excludes) - # If subtrahend is an `include_others` then the difference isn't. + # If other is an `include_others` then the difference isn't. new_all = False # (We have no need for excludes when we don't start with all, as there # is nothing to exclude.) new_excludes = set() # We also filter out all state types that aren't in the exclusion - # list of the subtrahend. + # list of the other. new_wildcards &= sub_excludes new_concrete_keys = { (state_type, state_key) From 538f99e4f76714de011c72f36a92811fbeabcd10 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 28 Sep 2021 14:38:05 +0100 Subject: [PATCH 29/34] Try 'included' rather than 'admitted' to describe state filters I don't think it's quite as good a fit but it might be more approachable. --- synapse/storage/state.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 9295d39394a9..06d13fbd806c 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -444,14 +444,14 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": Returns a state filter which represents `self - other`. The resultant state filter - - MUST admit all state events that are admitted - by only this filter (`self`) and not `other`; - - MUST NOT admit state events rejected by this filter (`self`); and + - MUST include all state events that are included by this filter (`self`) + unless they are included by `other`; + - MUST NOT include state events not included by this filter (`self`); and - MAY be an over-approximation: the resultant state - filter MAY additionally admit some state events from `other`. + filter MAY additionally include some state events from `other`. - Formally, if the set of state events admitted by a state filter F are + Formally, if the set of state events included by a state filter F are written as E(F), then the resultant state filter bears this property: E(self) ∖ E(other) @@ -470,8 +470,6 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": StateFilter(m.room.member -> {'@wombat:example.org'}) is approximated as StateFilter(m.room.member -> None (wildcard)) - which satisfies the condition that the resultant state filter - is an over-approximation. """ # We first transform self and other into an alternative representation: From bf202bc5c14ffb7ede72c9a8687727f176a85113 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 28 Sep 2021 14:38:22 +0100 Subject: [PATCH 30/34] Rename derived variables from sub(trahend) to other. --- synapse/storage/state.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 06d13fbd806c..5f82d96c51a8 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -481,26 +481,26 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": self_wildcards, self_concrete_keys, ) = self._decompose_into_four_parts() - (sub_all, sub_excludes), ( - sub_wildcards, - sub_concrete_keys, + (other_all, other_excludes), ( + other_wildcards, + other_concrete_keys, ) = other._decompose_into_four_parts() # Start with an estimate of the difference based on self new_all = self_all # Wildcards from the other can be added to the exclusion filter - new_excludes = self_excludes | sub_wildcards + new_excludes = self_excludes | other_wildcards # We remove wildcards that appeared as wildcards in the other - new_wildcards = self_wildcards - sub_wildcards + new_wildcards = self_wildcards - other_wildcards # We filter out the concrete state keys that appear in the other # as wildcards or concrete state keys. new_concrete_keys = { (state_type, state_key) for (state_type, state_key) in self_concrete_keys - if state_type not in sub_wildcards - } - sub_concrete_keys + if state_type not in other_wildcards + } - other_concrete_keys - if sub_all: + if other_all: if self_all: # If self starts with all, then we add as wildcards any # types which appear in the other's exclusion filter (but @@ -508,7 +508,7 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": # filter will return everything BUT the types in its exclusion, so # we need to add those excluded types that also match the self # filter as wildcard types in the new filter. - new_wildcards |= sub_excludes.difference(self_excludes) + new_wildcards |= other_excludes.difference(self_excludes) # If other is an `include_others` then the difference isn't. new_all = False @@ -518,11 +518,11 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": # We also filter out all state types that aren't in the exclusion # list of the other. - new_wildcards &= sub_excludes + new_wildcards &= other_excludes new_concrete_keys = { (state_type, state_key) for (state_type, state_key) in new_concrete_keys - if state_type in sub_excludes + if state_type in other_excludes } # Transform our newly-constructed state filter from the alternative From 9169d388ced2ba8300af3d1dbfd202eb07c7de9e Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 28 Sep 2021 14:43:28 +0100 Subject: [PATCH 31/34] Add a little bit of context as to why this is useful --- synapse/storage/state.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 5f82d96c51a8..9c36791037c3 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -443,6 +443,10 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": """ Returns a state filter which represents `self - other`. + This is useful for determining what state remains to be pulled out of the + database if we want the state included by `self` but already have the state + included by `other`. + The resultant state filter - MUST include all state events that are included by this filter (`self`) unless they are included by `other`; From 1f3008b0ef3e9964a91f34d73e8ff2ae852eebe3 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 28 Sep 2021 14:46:03 +0100 Subject: [PATCH 32/34] Use 'returned' instead of 'resultant' as that may be clearer --- synapse/storage/state.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 9c36791037c3..88b7f634d721 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -447,12 +447,12 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": database if we want the state included by `self` but already have the state included by `other`. - The resultant state filter + The returned state filter - MUST include all state events that are included by this filter (`self`) unless they are included by `other`; - MUST NOT include state events not included by this filter (`self`); and - - MAY be an over-approximation: the resultant state - filter MAY additionally include some state events from `other`. + - MAY be an over-approximation: the returned state filter + MAY additionally include some state events from `other`. Formally, if the set of state events included by a state filter F are @@ -466,7 +466,7 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": This implementation attempts to return the narrowest such state filter. In the case that `self` contains wildcards for state types where `other` contains specific state keys, an approximation must be made: - the resultant state filter keeps the wildcard, as state filters are not + the returned state filter keeps the wildcard, as state filters are not able to express 'all state keys except some given examples'. e.g. StateFilter(m.room.member -> None (wildcard)) From 3552bc1e88d3a9ec1827593bd5690cff03f91e7d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 28 Sep 2021 14:51:11 +0100 Subject: [PATCH 33/34] Remove formal definition to focus on one way of explaining --- synapse/storage/state.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 88b7f634d721..b5ba1560d139 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -454,15 +454,6 @@ def approx_difference(self, other: "StateFilter") -> "StateFilter": - MAY be an over-approximation: the returned state filter MAY additionally include some state events from `other`. - - Formally, if the set of state events included by a state filter F are - written as E(F), then the resultant state filter bears this property: - - E(self) ∖ E(other) - ⊆ E(approx_difference(self, other)) - ⊆ E(self) - - This implementation attempts to return the narrowest such state filter. In the case that `self` contains wildcards for state types where `other` contains specific state keys, an approximation must be made: From 4eaf98006f1bfdc85f83070b1e5d6c2c19cbabd1 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 29 Sep 2021 09:57:39 +0100 Subject: [PATCH 34/34] Use `StateFilter.freeze` in tests to improve readability --- tests/storage/test_state.py | 538 +++++++++++++++--------------------- 1 file changed, 221 insertions(+), 317 deletions(-) diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py index dc8ccdbb14e6..70d52b088c81 100644 --- a/tests/storage/test_state.py +++ b/tests/storage/test_state.py @@ -502,134 +502,110 @@ def test_state_filter_difference_no_include_other_minus_no_include_other(self): """ # (wildcard on state keys) - (wildcard on state keys): self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None, EventTypes.Create: None}), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.Create: None}, include_others=False, ), - StateFilter( - types=frozendict( - {EventTypes.Member: None, EventTypes.CanonicalAlias: None} - ), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None}, include_others=False, ), - StateFilter( - types=frozendict({EventTypes.Create: None}), include_others=False - ), + StateFilter.freeze({EventTypes.Create: None}, include_others=False), ) # (wildcard on state keys) - (specific state keys) # This one is an over-approximation because we can't represent # 'all state keys except a few named examples' self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), include_others=False - ), - StateFilter( - types=frozendict({EventTypes.Member: frozenset({"@wombat:spqr"})}), + StateFilter.freeze({EventTypes.Member: None}, include_others=False), + StateFilter.freeze( + {EventTypes.Member: {"@wombat:spqr"}}, include_others=False, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), include_others=False - ), + StateFilter.freeze({EventTypes.Member: None}, include_others=False), ) # (wildcard on state keys) - (no state keys) self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: set(), + }, include_others=False, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=False, ), ) # (specific state keys) - (wildcard on state keys): self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=False, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=False, ), - StateFilter( - types=frozendict({EventTypes.CanonicalAlias: frozenset({""})}), + StateFilter.freeze( + {EventTypes.CanonicalAlias: {""}}, include_others=False, ), ) # (specific state keys) - (specific state keys) self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr"}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr"}, + }, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=False, ), ) # (specific state keys) - (no state keys) self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: set(), + }, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=False, ), ) @@ -641,24 +617,20 @@ def test_state_filter_difference_include_other_minus_no_include_other(self): """ # (wildcard on state keys) - (wildcard on state keys): self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None, EventTypes.Create: None}), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.Create: None}, include_others=True, ), - StateFilter( - types=frozendict( - {EventTypes.Member: None, EventTypes.CanonicalAlias: None} - ), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None}, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Create: None, - EventTypes.Member: frozenset(), - EventTypes.CanonicalAlias: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Create: None, + EventTypes.Member: set(), + EventTypes.CanonicalAlias: set(), + }, include_others=True, ), ) @@ -668,16 +640,12 @@ def test_state_filter_difference_include_other_minus_no_include_other(self): # 'all state keys except a few named examples' # This also shows that the resultant state filter is normalised. self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), include_others=True - ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr"}), - EventTypes.Create: frozenset({""}), - } - ), + StateFilter.freeze({EventTypes.Member: None}, include_others=True), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr"}, + EventTypes.Create: {""}, + }, include_others=False, ), StateFilter(types=frozendict(), include_others=True), @@ -685,16 +653,14 @@ def test_state_filter_difference_include_other_minus_no_include_other(self): # (wildcard on state keys) - (no state keys) self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: set(), + }, include_others=False, ), StateFilter( @@ -705,86 +671,70 @@ def test_state_filter_difference_include_other_minus_no_include_other(self): # (specific state keys) - (wildcard on state keys): self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=True, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.CanonicalAlias: frozenset({""}), - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.CanonicalAlias: {""}, + EventTypes.Member: set(), + }, include_others=True, ), ) # (specific state keys) - (specific state keys) self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr"}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr"}, + }, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=True, ), ) # (specific state keys) - (no state keys) self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: set(), + }, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=True, ), ) @@ -797,14 +747,12 @@ def test_state_filter_difference_include_other_minus_include_other(self): """ # (wildcard on state keys) - (wildcard on state keys): self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None, EventTypes.Create: None}), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.Create: None}, include_others=True, ), - StateFilter( - types=frozendict( - {EventTypes.Member: None, EventTypes.CanonicalAlias: None} - ), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None}, include_others=True, ), StateFilter(types=frozendict(), include_others=False), @@ -814,59 +762,49 @@ def test_state_filter_difference_include_other_minus_include_other(self): # This one is an over-approximation because we can't represent # 'all state keys except a few named examples' self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), include_others=True - ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze({EventTypes.Member: None}, include_others=True), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=True, ), - StateFilter( - types=frozendict( - {EventTypes.Member: None, EventTypes.CanonicalAlias: None} - ), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None}, include_others=False, ), ) # (wildcard on state keys) - (no state keys) self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: set(), + }, include_others=True, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=False, ), ) # (specific state keys) - (wildcard on state keys): self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=True, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=True, ), StateFilter( @@ -879,61 +817,49 @@ def test_state_filter_difference_include_other_minus_include_other(self): # This one is an over-approximation because we can't represent # 'all state keys except a few named examples' self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - EventTypes.Create: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + EventTypes.Create: {""}, + }, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr"}), - EventTypes.Create: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr"}, + EventTypes.Create: set(), + }, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@spqr:spqr"}), - EventTypes.Create: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@spqr:spqr"}, + EventTypes.Create: {""}, + }, include_others=False, ), ) # (specific state keys) - (no state keys) self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: set(), + }, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + }, include_others=False, ), ) @@ -945,14 +871,12 @@ def test_state_filter_difference_no_include_other_minus_include_other(self): """ # (wildcard on state keys) - (wildcard on state keys): self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None, EventTypes.Create: None}), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.Create: None}, include_others=False, ), - StateFilter( - types=frozendict( - {EventTypes.Member: None, EventTypes.CanonicalAlias: None} - ), + StateFilter.freeze( + {EventTypes.Member: None, EventTypes.CanonicalAlias: None}, include_others=True, ), StateFilter(types=frozendict(), include_others=False), @@ -962,51 +886,43 @@ def test_state_filter_difference_no_include_other_minus_include_other(self): # This one is an over-approximation because we can't represent # 'all state keys except a few named examples' self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), include_others=False - ), - StateFilter( - types=frozendict({EventTypes.Member: frozenset({"@wombat:spqr"})}), + StateFilter.freeze({EventTypes.Member: None}, include_others=False), + StateFilter.freeze( + {EventTypes.Member: {"@wombat:spqr"}}, include_others=True, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), include_others=False - ), + StateFilter.freeze({EventTypes.Member: None}, include_others=False), ) # (wildcard on state keys) - (no state keys) self.assert_difference( - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: set(), + }, include_others=True, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=False, ), ) # (specific state keys) - (wildcard on state keys): self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=False, ), - StateFilter( - types=frozendict({EventTypes.Member: None}), + StateFilter.freeze( + {EventTypes.Member: None}, include_others=True, ), StateFilter( @@ -1019,58 +935,46 @@ def test_state_filter_difference_no_include_other_minus_include_other(self): # This one is an over-approximation because we can't represent # 'all state keys except a few named examples' self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr"}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr"}, + }, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@spqr:spqr"}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@spqr:spqr"}, + }, include_others=False, ), ) # (specific state keys) - (no state keys) self.assert_difference( - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - EventTypes.CanonicalAlias: frozenset({""}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + EventTypes.CanonicalAlias: {""}, + }, include_others=False, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset(), - } - ), + StateFilter.freeze( + { + EventTypes.Member: set(), + }, include_others=True, ), - StateFilter( - types=frozendict( - { - EventTypes.Member: frozenset({"@wombat:spqr", "@spqr:spqr"}), - } - ), + StateFilter.freeze( + { + EventTypes.Member: {"@wombat:spqr", "@spqr:spqr"}, + }, include_others=False, ), )