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

Add an approximate difference method to StateFilters #10825

Merged
merged 35 commits into from
Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
7dad902
Add an approximate difference method to StateFilters
reivilibre Sep 15, 2021
1fe75e6
Add tests for the approximate difference of StateFilters
reivilibre Sep 15, 2021
a05692c
Newsfile
reivilibre Sep 15, 2021
10a7071
Try to clarify docstring for `approx_difference`
reivilibre Sep 17, 2021
d0e14d5
Process all the keys to return a narrower state filter
reivilibre Sep 17, 2021
a5fdd46
Add more test cases
reivilibre Sep 17, 2021
0e0085c
STASH
reivilibre Sep 17, 2021
9d50f05
Tighten up the postconditions of `approx_difference`
reivilibre Sep 20, 2021
ace3316
Merge remote-tracking branch 'origin/develop' into rei/sf_diff
reivilibre Sep 20, 2021
c72c436
More wordsmithing — thanks David
reivilibre Sep 20, 2021
bacd394
Revert "STASH"
reivilibre Sep 20, 2021
cd1de9b
Remove needless set construction
reivilibre Sep 22, 2021
6bedcba
Simplify logic a bit, since this isn't operating in-place anyway
reivilibre Sep 22, 2021
42617db
Attempt to clean up `approx_difference` with improved comments and names
reivilibre Sep 22, 2021
0c8e930
Split out tests into own TestCase class
reivilibre Sep 22, 2021
b6274d6
Add extensive tests for all 4 combinations of include_others
reivilibre Sep 22, 2021
f6b4dc5
Merge a test
reivilibre Sep 22, 2021
0d1c3d8
Split out some very simple tests
reivilibre Sep 22, 2021
18714d7
Deduplicate the old tests into the systematic style tests
reivilibre Sep 22, 2021
770afea
Add function to decompose a StateFilter into four parts
reivilibre Sep 22, 2021
e119af9
Add `StateFilter.freeze` convenience constructor
reivilibre Sep 22, 2021
70f646a
Add `recompose_from_four_parts` method as inverse
reivilibre Sep 22, 2021
c9bb226
Use a shorter version of `decompose_into_four_parts`.
reivilibre Sep 24, 2021
093f670
Use a shorter version of `recompose_from_four_parts`
reivilibre Sep 24, 2021
a187c24
Use a step-by-step implementation of `approx_difference` rather than …
reivilibre Sep 24, 2021
4bbe3d1
Nest the ifs for clarity; no functional change
reivilibre Sep 24, 2021
20bc299
Prefix decompose/recompose method names with underscores
reivilibre Sep 24, 2021
27c3a7a
Use self_excludes as it's equivalent with this definition of decompose
reivilibre Sep 24, 2021
54d77c9
Rename `subtrahend` to `other` to follow convention
reivilibre Sep 28, 2021
538f99e
Try 'included' rather than 'admitted' to describe state filters
reivilibre Sep 28, 2021
bf202bc
Rename derived variables from sub(trahend) to other.
reivilibre Sep 28, 2021
9169d38
Add a little bit of context as to why this is useful
reivilibre Sep 28, 2021
1f3008b
Use 'returned' instead of 'resultant' as that may be clearer
reivilibre Sep 28, 2021
3552bc1
Remove formal definition to focus on one way of explaining
reivilibre Sep 28, 2021
4eaf980
Use `StateFilter.freeze` in tests to improve readability
reivilibre Sep 29, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10825.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add an 'approximate difference' method to `StateFilter`.
54 changes: 54 additions & 0 deletions synapse/storage/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
reivilibre marked this conversation as resolved.
Show resolved Hide resolved

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).
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
"""

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."""
Expand Down
86 changes: 85 additions & 1 deletion tests/storage/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, "", {})
Expand Down Expand Up @@ -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(),
)
reivilibre marked this conversation as resolved.
Show resolved Hide resolved

# 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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really relevant to this change, but: it wasn't obvious to me what an empty set and None mean here (and if they ought to have different meanings).

I don't know if there's a nice way. If we had sum types I'd suggest

enum Fetch {
    All,
    None,
    WithStateKey(set_of_strings),
}

But that's slightly awkward because None and WithStateKey(empty_set) represent the same thing. /shrug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docstring for StateFilter specifies what they mean, though that's not to say I don't agree with you — and yeah, I think empty set makes sense, but 'None' very much reads as, well, none. It's half tempting to have a constant WILDCARD for None ;p, but I feel like that might just hide things more.

}
),
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,
),
)