mirror of
https://github.com/element-hq/synapse.git
synced 2026-07-28 07:50:01 +00:00
Ensure we bust the lazy loading cache for profile updates, when field values change
This commit is contained in:
+29
-16
@@ -18,6 +18,7 @@
|
||||
# [This file includes modifications made by New Vector Limited]
|
||||
#
|
||||
#
|
||||
import hashlib
|
||||
import itertools
|
||||
import logging
|
||||
from typing import (
|
||||
@@ -345,7 +346,10 @@ class SyncHandler:
|
||||
max_len=0,
|
||||
expiry_ms=LAZY_LOADED_MEMBERS_CACHE_MAX_AGE,
|
||||
)
|
||||
# ExpiringCache((User, Device)) -> LruCache(Other User ID + Field Name -> bool)
|
||||
# ExpiringCache((User, Device))
|
||||
# -> LruCache(
|
||||
# md5(Other User ID + Field Name + Field value) -> bool
|
||||
# )
|
||||
self.lazy_loaded_profile_fields_cache: ExpiringCache[
|
||||
tuple[str, str | None], LruCache[str, bool]
|
||||
] = ExpiringCache(
|
||||
@@ -356,12 +360,12 @@ class SyncHandler:
|
||||
max_len=0,
|
||||
expiry_ms=LAZY_LOADED_PROFILE_FIELDS_CACHE_MAX_AGE,
|
||||
)
|
||||
"""This cache contains fields we have sent to clients as profile updates,
|
||||
for a particular user + device combo. The cache entry is a combination of the
|
||||
user + field name, with the value existing indicating the field has recently
|
||||
been sent. The boolean value does not hold other significance. A missing
|
||||
cache entry means "we have not sent this user + field name combo to the
|
||||
syncing user".
|
||||
"""This cache contains fields and values we have sent to clients as profile
|
||||
updates, for a particular user + device combo. The cache entry is a combination
|
||||
of the user + field name + value, all hashed into md5, an existing value
|
||||
indicating the field has recently been sent. The boolean value does not hold
|
||||
other significance. A missing cache entry means "we have not sent this user +
|
||||
field name + value combo to the syncing user".
|
||||
|
||||
We don't manually remove entries from this cache, though it may be ignored
|
||||
in cases where the sync must send the field down to the client.
|
||||
@@ -1076,12 +1080,12 @@ class SyncHandler:
|
||||
def get_lazy_loaded_profile_fields_cache(
|
||||
self, cache_key: tuple[str, str | None]
|
||||
) -> LruCache[str, bool]:
|
||||
"""This cache contains fields we have sent to clients as profile updates,
|
||||
for a particular user + device combo. The cache entry is a combination of the
|
||||
user + field name, with the value existing indicating the field has recently
|
||||
been sent. The boolean value does not hold other significance. A missing
|
||||
cache entry means "we have not sent this user + field name combo to the
|
||||
syncing user".
|
||||
"""This cache contains fields and values we have sent to clients as profile
|
||||
updates, for a particular user + device combo. The cache entry is a combination
|
||||
of the user + field name + value, all hashed into md5, an existing value
|
||||
indicating the field has recently been sent. The boolean value does not hold
|
||||
other significance. A missing cache entry means "we have not sent this user +
|
||||
field name + value combo to the syncing user".
|
||||
|
||||
We don't manually remove entries from this cache, though it may be ignored
|
||||
in cases where the sync must send the field down to the client.
|
||||
@@ -2395,13 +2399,22 @@ class SyncHandler:
|
||||
sync_config.device_id,
|
||||
)
|
||||
cache = self.get_lazy_loaded_profile_fields_cache(cache_key)
|
||||
# Only send this users field if we haven't recently sent it
|
||||
if cache.get(f"{other_user_id}-{field_name}") is None:
|
||||
# Only send this users field if we haven't recently sent it.
|
||||
# Our cache value to check against is an md5 of a string of
|
||||
# user ID + field name + value, which ensures if the value
|
||||
# changes, we'll miss the cache, thus sending the field update
|
||||
# to the syncing user.
|
||||
cache_value = hashlib.md5(
|
||||
f"{other_user_id}-{field_name}-{profile_data.get(field_name)}".encode(
|
||||
"utf8"
|
||||
)
|
||||
).hexdigest()
|
||||
if cache.get(cache_value) is None:
|
||||
per_user_updates[field_name] = profile_data.get(field_name)
|
||||
# Update our cache to indicate this user/field combo
|
||||
# has been recently sent.
|
||||
cache.set(
|
||||
f"{other_user_id}-{field_name}",
|
||||
cache_value,
|
||||
True,
|
||||
)
|
||||
else:
|
||||
|
||||
@@ -2332,6 +2332,132 @@ class SyncProfileUpdatesTestCase(tests.unittest.HomeserverTestCase):
|
||||
incremental_result.profile_updates["@third_user:test"],
|
||||
)
|
||||
|
||||
@parameterized.expand(
|
||||
[
|
||||
["string value", "new string value"],
|
||||
[True, False],
|
||||
[None, "not None"],
|
||||
[[], ["with item"]],
|
||||
[{}, {"key": "value"}],
|
||||
[{"foo": "bar"}, {"bar": "foo"}],
|
||||
[42, 42.24],
|
||||
]
|
||||
)
|
||||
@override_config({"include_profile_updates_in_sync": True})
|
||||
def test_profile_updates_dont_get_silenced_by_cache(
|
||||
self,
|
||||
value: str | bool | list | dict | int | float | None,
|
||||
new_value: str | bool | list | dict | int | float | None,
|
||||
) -> None:
|
||||
"""Test that with MSC4429 enabled the incremental lazy sync response
|
||||
includes all the profile update changes for the user, even if the profile
|
||||
field has been recently sent and is in our lazy loading cache.
|
||||
|
||||
Parameterize across different types of potential value types that profile
|
||||
field updates could have to ensure robustness.
|
||||
"""
|
||||
requester = create_requester(self.user)
|
||||
initial_result = self.get_success(
|
||||
self.sync_handler.wait_for_sync_for_user(
|
||||
requester,
|
||||
sync_config=generate_sync_config(
|
||||
user_id=self.user,
|
||||
filter_collection=FilterCollection(
|
||||
hs=self.hs,
|
||||
filter_json={
|
||||
"org.matrix.msc4429.profile_fields": {"ids": ["field"]},
|
||||
"room": {
|
||||
"state": {
|
||||
"lazy_load_members": True,
|
||||
},
|
||||
},
|
||||
},
|
||||
),
|
||||
),
|
||||
request_key=generate_request_key(),
|
||||
)
|
||||
)
|
||||
self.assertFalse(
|
||||
"@other_user:test" in initial_result.profile_updates,
|
||||
)
|
||||
|
||||
# Update the field
|
||||
self.get_success(
|
||||
self.profile_handler.set_field(
|
||||
target_user=UserID.from_string(self.other_user),
|
||||
requester=create_requester(self.other_user),
|
||||
field_name="field",
|
||||
new_value=cast(JsonValue | dict[str, JsonValue], value),
|
||||
)
|
||||
)
|
||||
incremental_result = self.get_success(
|
||||
self.sync_handler.wait_for_sync_for_user(
|
||||
requester,
|
||||
since_token=initial_result.next_batch,
|
||||
sync_config=generate_sync_config(
|
||||
user_id=self.user,
|
||||
filter_collection=FilterCollection(
|
||||
hs=self.hs,
|
||||
filter_json={
|
||||
"org.matrix.msc4429.profile_fields": {"ids": ["field"]},
|
||||
"room": {
|
||||
"state": {
|
||||
"lazy_load_members": True,
|
||||
},
|
||||
},
|
||||
},
|
||||
),
|
||||
),
|
||||
request_key=generate_request_key(),
|
||||
)
|
||||
)
|
||||
# We should have the field change in our sync response.
|
||||
# It will also be added to the lazy loading cache, so the same field value
|
||||
# isn't sent again immediately.
|
||||
assert incremental_result.profile_updates["@other_user:test"] is not None
|
||||
self.assertEqual(
|
||||
incremental_result.profile_updates["@other_user:test"]["field"],
|
||||
value,
|
||||
)
|
||||
|
||||
# Update the field again, busting our cache
|
||||
self.get_success(
|
||||
self.profile_handler.set_field(
|
||||
target_user=UserID.from_string(self.other_user),
|
||||
requester=create_requester(self.other_user),
|
||||
field_name="field",
|
||||
new_value=cast(JsonValue | dict[str, JsonValue], new_value),
|
||||
)
|
||||
)
|
||||
incremental_result = self.get_success(
|
||||
self.sync_handler.wait_for_sync_for_user(
|
||||
requester,
|
||||
since_token=incremental_result.next_batch,
|
||||
sync_config=generate_sync_config(
|
||||
user_id=self.user,
|
||||
filter_collection=FilterCollection(
|
||||
hs=self.hs,
|
||||
filter_json={
|
||||
"org.matrix.msc4429.profile_fields": {"ids": ["field"]},
|
||||
"room": {
|
||||
"state": {
|
||||
"lazy_load_members": True,
|
||||
},
|
||||
},
|
||||
},
|
||||
),
|
||||
),
|
||||
request_key=generate_request_key(),
|
||||
)
|
||||
)
|
||||
# Even though the field was added to the lazy loading members cache,
|
||||
# it should come through as an update, as the field value changed.
|
||||
assert incremental_result.profile_updates["@other_user:test"] is not None
|
||||
self.assertEqual(
|
||||
incremental_result.profile_updates["@other_user:test"]["field"],
|
||||
new_value,
|
||||
)
|
||||
|
||||
|
||||
class SyncStateAfterTestCase(tests.unittest.HomeserverTestCase):
|
||||
"""Tests Sync Handler state behavior when using `use_state_after."""
|
||||
|
||||
Reference in New Issue
Block a user