From 68b663d60f59a4ae4d2906f1a648ca2af13d11ab Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 21 Dec 2021 11:39:25 +0000 Subject: [PATCH 01/20] Remove account data upon user deactivation --- synapse/handlers/deactivate_account.py | 3 ++ .../storage/databases/main/account_data.py | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index bee62cf36088..7a13d76a687f 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -157,6 +157,9 @@ async def deactivate_account( # Mark the user as deactivated. await self.store.set_user_deactivated_status(user_id, True) + # Remove account data (including ignored users and push rules). + await self.store.purge_account_data_for_user(user_id) + return identity_server_supports_unbinding async def _reject_pending_invites_for_user(self, user_id: str) -> None: diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 32a553fdd7bd..83066a9c3baf 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -546,6 +546,45 @@ def _add_account_data_for_user( for ignored_user_id in previously_ignored_users ^ currently_ignored_users: self._invalidate_cache_and_stream(txn, self.ignored_by, (ignored_user_id,)) + async def purge_account_data_for_user(self, user_id: str) -> None: + """ + Removes ALL the account data for a user. + Intended to be used upon user deactivation. + + Also purges the user from the ignored_users cache table + and the push_rules cache tables. + """ + + def purge_account_data_for_user_txn(txn: LoggingTransaction) -> None: + # Purge from the primary account_data table. + self.db_pool.simple_delete_txn( + txn, table="account_data", keyvalues={"user_id": user_id} + ) + + # Purge from ignored_users where this user is the ignorer. + # N.B. We don't purge where this user is the ignoree, because that + # interferes with other users' account data. + # It's also not this user's data to delete! + self.db_pool.simple_delete_txn( + txn, table="ignored_users", keyvalues={"ignorer_user_id": user_id} + ) + + # Remove the push rules + self.db_pool.simple_delete_txn( + txn, table="push_rules", keyvalues={"user_name": user_id} + ) + self.db_pool.simple_delete_txn( + txn, table="push_rules_enable", keyvalues={"user_name": user_id} + ) + self.db_pool.simple_delete_txn( + txn, table="push_rules_stream", keyvalues={"user_name": user_id} + ) + + await self.db_pool.runInteraction( + "purge_account_data_for_user_txn", + purge_account_data_for_user_txn, + ) + class AccountDataStore(AccountDataWorkerStore): pass From 13fec0d8d86324ab1c74f60bc4085b95bbdb0163 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 21 Dec 2021 11:39:49 +0000 Subject: [PATCH 02/20] Document that account data is removed upon user deactivation --- docs/admin_api/user_admin_api.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index ba574d795fcc..25048a95e05e 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -352,6 +352,7 @@ The following actions are performed when deactivating an user: - Remove the user from the user directory - Reject all pending invites - Remove all account validity information related to the user +- Remove the arbitrary data store known as *account data* (list of ignored users, push rules, etc.) The following additional actions are performed during deactivation if `erase` is set to `true`: From c1fed495ce23fee1aee924262e2723ed2dd0f320 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 21 Dec 2021 11:41:07 +0000 Subject: [PATCH 03/20] Newsfile Signed-off-by: Olivier Wilkinson (reivilibre) --- changelog.d/11621.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11621.feature diff --git a/changelog.d/11621.feature b/changelog.d/11621.feature new file mode 100644 index 000000000000..dc426fb658ac --- /dev/null +++ b/changelog.d/11621.feature @@ -0,0 +1 @@ +Remove account data (including client config, push rules and ignored users) upon user deactivation. \ No newline at end of file From 4da869ea1af50f92d5ae29d63882571e55954811 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 21 Dec 2021 11:39:25 +0000 Subject: [PATCH 04/20] Remove account data upon user deactivation --- synapse/storage/databases/main/account_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 83066a9c3baf..4ebc44308b78 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -577,7 +577,7 @@ def purge_account_data_for_user_txn(txn: LoggingTransaction) -> None: txn, table="push_rules_enable", keyvalues={"user_name": user_id} ) self.db_pool.simple_delete_txn( - txn, table="push_rules_stream", keyvalues={"user_name": user_id} + txn, table="push_rules_stream", keyvalues={"user_id": user_id} ) await self.db_pool.runInteraction( From b132bbab3a8e67f5b17018db852df1d6cc0d29e8 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 29 Dec 2021 13:19:00 +0000 Subject: [PATCH 05/20] Test the removal of account data upon deactivation --- tests/handlers/test_deactivate_account.py | 78 +++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/handlers/test_deactivate_account.py diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py new file mode 100644 index 000000000000..34b98f079a7e --- /dev/null +++ b/tests/handlers/test_deactivate_account.py @@ -0,0 +1,78 @@ +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from twisted.test.proto_helpers import MemoryReactor + +from synapse.api.constants import AccountDataTypes +from synapse.rest import admin +from synapse.rest.client import account, login +from synapse.server import HomeServer +from synapse.util import Clock + +from tests.unittest import HomeserverTestCase + + +class DeactivateAccountTestCase(HomeserverTestCase): + servlets = [ + login.register_servlets, + admin.register_servlets, + account.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + super(DeactivateAccountTestCase, self).prepare(reactor, clock, hs) + self._store = hs.get_datastore() + + self.user = self.register_user("user", "pass") + self.token = self.login("user", "pass") + + def test_account_data_deleted_upon_deactivation(self) -> None: + """ + Tests that account data is removed upon deactivation. + """ + # Add some account data + self.get_success( + self._store.add_account_data_for_user( + self.user, + AccountDataTypes.DIRECT, + {"@someone:remote": ["!somewhere:remote"]}, + ) + ) + + # Request the deactivation of our account + req = self.get_success( + self.make_request( + "POST", + "account/deactivate", + { + "auth": { + "type": "m.login.password", + "user": self.user, + "password": "pass", + }, + "erase": True, + }, + access_token=self.token, + ) + ) + self.assertEqual(req.code, 200, req) + + # Check that the account data does not persist. + self.assertIsNone( + self.get_success( + self._store.get_global_account_data_by_type_for_user( + AccountDataTypes.DIRECT, + self.user, + ) + ), + ) From 1589d6a14754eb515090ae787325f5c8ca5f779b Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 29 Dec 2021 12:14:43 +0000 Subject: [PATCH 06/20] Pull out the account data transaction so it can be reused in other transactions --- .../storage/databases/main/account_data.py | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 4ebc44308b78..46e81dd6e048 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -555,34 +555,40 @@ async def purge_account_data_for_user(self, user_id: str) -> None: and the push_rules cache tables. """ - def purge_account_data_for_user_txn(txn: LoggingTransaction) -> None: - # Purge from the primary account_data table. - self.db_pool.simple_delete_txn( - txn, table="account_data", keyvalues={"user_id": user_id} - ) + await self.db_pool.runInteraction( + "purge_account_data_for_user_txn", + self._purge_account_data_for_user_txn, + user_id, + ) - # Purge from ignored_users where this user is the ignorer. - # N.B. We don't purge where this user is the ignoree, because that - # interferes with other users' account data. - # It's also not this user's data to delete! - self.db_pool.simple_delete_txn( - txn, table="ignored_users", keyvalues={"ignorer_user_id": user_id} - ) + def _purge_account_data_for_user_txn( + self, txn: LoggingTransaction, user_id: str + ) -> None: + """ + See `purge_account_data_for_user`. + """ + # Purge from the primary account_data table. + self.db_pool.simple_delete_txn( + txn, table="account_data", keyvalues={"user_id": user_id} + ) - # Remove the push rules - self.db_pool.simple_delete_txn( - txn, table="push_rules", keyvalues={"user_name": user_id} - ) - self.db_pool.simple_delete_txn( - txn, table="push_rules_enable", keyvalues={"user_name": user_id} - ) - self.db_pool.simple_delete_txn( - txn, table="push_rules_stream", keyvalues={"user_id": user_id} - ) + # Purge from ignored_users where this user is the ignorer. + # N.B. We don't purge where this user is the ignoree, because that + # interferes with other users' account data. + # It's also not this user's data to delete! + self.db_pool.simple_delete_txn( + txn, table="ignored_users", keyvalues={"ignorer_user_id": user_id} + ) - await self.db_pool.runInteraction( - "purge_account_data_for_user_txn", - purge_account_data_for_user_txn, + # Remove the push rules + self.db_pool.simple_delete_txn( + txn, table="push_rules", keyvalues={"user_name": user_id} + ) + self.db_pool.simple_delete_txn( + txn, table="push_rules_enable", keyvalues={"user_name": user_id} + ) + self.db_pool.simple_delete_txn( + txn, table="push_rules_stream", keyvalues={"user_id": user_id} ) From 534444624a5711fa286f092bd8f4eab31cae6047 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 29 Dec 2021 12:15:57 +0000 Subject: [PATCH 07/20] Add a background job to retroactively purge account data for deactivated users --- .../storage/databases/main/account_data.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 46e81dd6e048..012fb6f3721a 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -105,6 +105,11 @@ def __init__( "AccountDataAndTagsChangeCache", account_max ) + self.db_pool.updates.register_background_update_handler( + "delete_account_data_for_deactivated_users", + self._delete_account_data_for_deactivated_users, + ) + def get_max_account_data_stream_id(self) -> int: """Get the current max stream ID for account data stream @@ -591,6 +596,53 @@ def _purge_account_data_for_user_txn( txn, table="push_rules_stream", keyvalues={"user_id": user_id} ) + async def _delete_account_data_for_deactivated_users( + self, progress: dict, batch_size: int + ) -> int: + """ + Retroactively purges account data for users that have already been deactivated. + Gets run as a background update caused by a schema delta. + """ + + last_user: str = progress.get("last_user", "") + + def _delete_account_data_for_deactivated_users_txn( + txn: LoggingTransaction, + ) -> int: + sql = """ + SELECT name FROM users + WHERE deactivated = ? and name > ? + ORDER BY name ASC + LIMIT ? + """ + + txn.execute(sql, (1, last_user, batch_size)) + users = [row[0] for row in txn] + + for user in users: + self._purge_account_data_for_user_txn(txn, user_id=user) + + if users: + self.db_pool.updates._background_update_progress_txn( + txn, + "delete_account_data_for_deactivated_users", + {"last_user": users[-1]}, + ) + + return len(users) + + number_deleted = await self.db_pool.runInteraction( + "_delete_account_data_for_deactivated_users", + _delete_account_data_for_deactivated_users_txn, + ) + + if number_deleted < batch_size: + await self.db_pool.updates._end_background_update( + "delete_account_data_for_deactivated_users" + ) + + return number_deleted + class AccountDataStore(AccountDataWorkerStore): pass From da884c0c30923d73720fdc9ef0a5463493dad2c4 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 29 Dec 2021 12:16:12 +0000 Subject: [PATCH 08/20] Add a schema delta to trigger the clean-up job --- ..._account_data_for_deactivated_accounts.sql | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 synapse/storage/schema/main/delta/65/20_delete_account_data_for_deactivated_accounts.sql diff --git a/synapse/storage/schema/main/delta/65/20_delete_account_data_for_deactivated_accounts.sql b/synapse/storage/schema/main/delta/65/20_delete_account_data_for_deactivated_accounts.sql new file mode 100644 index 000000000000..69176660348e --- /dev/null +++ b/synapse/storage/schema/main/delta/65/20_delete_account_data_for_deactivated_accounts.sql @@ -0,0 +1,20 @@ +/* Copyright 2021 The Matrix.org Foundation C.I.C + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +-- We want to retroactively delete account data for users that were already +-- deactivated. +INSERT INTO background_updates (ordering, update_name, progress_json) VALUES + (6520, 'delete_account_data_for_deactivated_users', '{}'); From 288b0e8d28524b757a721c12ed65ca8652e7f0ff Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 29 Dec 2021 12:21:16 +0000 Subject: [PATCH 09/20] Newsfile Signed-off-by: Olivier Wilkinson (reivilibre) --- changelog.d/11655.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11655.feature diff --git a/changelog.d/11655.feature b/changelog.d/11655.feature new file mode 100644 index 000000000000..dc426fb658ac --- /dev/null +++ b/changelog.d/11655.feature @@ -0,0 +1 @@ +Remove account data (including client config, push rules and ignored users) upon user deactivation. \ No newline at end of file From 2690bd684f89e58bcbd97854be46887ecc201cb0 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 17 Jan 2022 13:27:54 +0000 Subject: [PATCH 10/20] Add tests for the background update job --- tests/handlers/test_deactivate_account.py | 121 ++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 34b98f079a7e..ba37c4562c24 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -76,3 +76,124 @@ def test_account_data_deleted_upon_deactivation(self) -> None: ) ), ) + + def _rerun_retroactive_account_data_deletion_job(self) -> None: + # Reset the 'all done' flag + self._store.db_pool.updates._all_done = False + + self.get_success( + self._store.db_pool.simple_insert( + "background_updates", + { + "update_name": "delete_account_data_for_deactivated_users", + "progress_json": "{}", + }, + ) + ) + + self.wait_for_background_updates() + + def test_account_data_deleted_retroactively_by_background_job_if_deactivated( + self, + ) -> None: + """ + Tests that a user, who deactivated their account before account data was + deleted automatically upon deactivation, has their account data retroactively + scrubbed by the background job. + """ + + # Request the deactivation of our account + req = self.get_success( + self.make_request( + "POST", + "account/deactivate", + { + "auth": { + "type": "m.login.password", + "user": self.user, + "password": "pass", + }, + "erase": True, + }, + access_token=self.token, + ) + ) + self.assertEqual(req.code, 200, req) + + # Add some account data + # (we do this after the deactivation so that the act of deactivating doesn't + # clear it out. This emulates a user that was deactivated before this was cleared + # upon deactivation.) + self.get_success( + self._store.add_account_data_for_user( + self.user, + AccountDataTypes.DIRECT, + {"@someone:remote": ["!somewhere:remote"]}, + ) + ) + + # Check that the account data is there. + self.assertIsNotNone( + self.get_success( + self._store.get_global_account_data_by_type_for_user( + AccountDataTypes.DIRECT, + self.user, + ) + ), + ) + + # Re-run the retroactive deletion job + self._rerun_retroactive_account_data_deletion_job() + + # Check that the account data was cleared. + self._store.get_global_account_data_by_type_for_user.invalidate_all() + self.assertIsNone( + self.get_success( + self._store.get_global_account_data_by_type_for_user( + AccountDataTypes.DIRECT, + self.user, + ) + ), + ) + + def test_account_data_preserved_by_background_job_if_not_deactivated(self) -> None: + """ + Tests that the background job does not scrub account data for users that have + not been deactivated. + """ + + # Add some account data + # (we do this after the deactivation so that the act of deactivating doesn't + # clear it out. This emulates a user that was deactivated before this was cleared + # upon deactivation.) + self.get_success( + self._store.add_account_data_for_user( + self.user, + AccountDataTypes.DIRECT, + {"@someone:remote": ["!somewhere:remote"]}, + ) + ) + + # Check that the account data is there. + self.assertIsNotNone( + self.get_success( + self._store.get_global_account_data_by_type_for_user( + AccountDataTypes.DIRECT, + self.user, + ) + ), + ) + + # Re-run the retroactive deletion job + self._rerun_retroactive_account_data_deletion_job() + + # Check that the account data was NOT cleared. + self._store.get_global_account_data_by_type_for_user.invalidate_all() + self.assertIsNotNone( + self.get_success( + self._store.get_global_account_data_by_type_for_user( + AccountDataTypes.DIRECT, + self.user, + ) + ), + ) From 9413a46089efb9ada643f5a2511dfb08bc0cc716 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 17 Jan 2022 14:16:15 +0000 Subject: [PATCH 11/20] Fix port_db script not being able to run the background job --- scripts/synapse_port_db | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/synapse_port_db b/scripts/synapse_port_db index 640ff15277db..84085f68c0ed 100755 --- a/scripts/synapse_port_db +++ b/scripts/synapse_port_db @@ -36,6 +36,7 @@ from synapse.logging.context import ( run_in_background, ) from synapse.storage.database import DatabasePool, make_conn +from synapse.storage.databases.main.account_data import AccountDataWorkerStore from synapse.storage.databases.main.client_ips import ClientIpBackgroundUpdateStore from synapse.storage.databases.main.deviceinbox import DeviceInboxBackgroundUpdateStore from synapse.storage.databases.main.devices import DeviceBackgroundUpdateStore @@ -183,6 +184,7 @@ class Store( PusherWorkerStore, PresenceBackgroundUpdateStore, GroupServerWorkerStore, + AccountDataWorkerStore, ): def execute(self, f, *args, **kwargs): return self.db_pool.runInteraction(f.__name__, f, *args, **kwargs) From 2337b28510f3c2eac0b710ef9222f7f6c241f9e0 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 24 Jan 2022 13:45:02 +0000 Subject: [PATCH 12/20] Reformat after merge --- synapse/storage/databases/main/account_data.py | 4 +--- tests/handlers/test_deactivate_account.py | 3 --- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index ec179ab1df92..52146aacc8c0 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -614,9 +614,7 @@ def _purge_account_data_for_user_txn( self._invalidate_cache_and_stream( txn, self.get_account_data_for_room, (user_id,) ) - self._invalidate_cache_and_stream( - txn, self.get_push_rules_for_user, (user_id,) - ) + self._invalidate_cache_and_stream(txn, self.get_push_rules_for_user, (user_id,)) self._invalidate_cache_and_stream( txn, self.get_push_rules_enabled_for_user, (user_id,) ) diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 864009dec949..1519b8331909 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -218,7 +218,6 @@ def test_ignored_users_deleted_upon_deactivation(self) -> None: self.get_success(self._store.ignored_by("@sheltie:test")), set() ) - def _rerun_retroactive_account_data_deletion_job(self) -> None: # Reset the 'all done' flag self._store.db_pool.updates._all_done = False @@ -235,7 +234,6 @@ def _rerun_retroactive_account_data_deletion_job(self) -> None: self.wait_for_background_updates() - def test_account_data_deleted_retroactively_by_background_job_if_deactivated( self, ) -> None: @@ -299,7 +297,6 @@ def test_account_data_deleted_retroactively_by_background_job_if_deactivated( ), ) - def test_account_data_preserved_by_background_job_if_not_deactivated(self) -> None: """ Tests that the background job does not scrub account data for users that have From 2ee3d8fe33178638a39072d7bb827c1f1e74b412 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 24 Jan 2022 13:45:53 +0000 Subject: [PATCH 13/20] Simplify test after merge --- tests/handlers/test_deactivate_account.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 1519b8331909..9d33f483f4e8 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -244,22 +244,7 @@ def test_account_data_deleted_retroactively_by_background_job_if_deactivated( """ # Request the deactivation of our account - req = self.get_success( - self.make_request( - "POST", - "account/deactivate", - { - "auth": { - "type": "m.login.password", - "user": self.user, - "password": "pass", - }, - "erase": True, - }, - access_token=self.token, - ) - ) - self.assertEqual(req.code, 200, req) + self._deactivate_my_account() # Add some account data # (we do this after the deactivation so that the act of deactivating doesn't From d362e31d19c5e968ea9e69b1428319d1af424cca Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 24 Jan 2022 13:48:37 +0000 Subject: [PATCH 14/20] Fix order of args after merge (they were re-ordered to make the function cacheable effectively by a tree cache) --- tests/handlers/test_deactivate_account.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 9d33f483f4e8..1c21799b1fd9 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -262,8 +262,8 @@ def test_account_data_deleted_retroactively_by_background_job_if_deactivated( self.assertIsNotNone( self.get_success( self._store.get_global_account_data_by_type_for_user( - AccountDataTypes.DIRECT, self.user, + AccountDataTypes.DIRECT, ) ), ) @@ -276,8 +276,8 @@ def test_account_data_deleted_retroactively_by_background_job_if_deactivated( self.assertIsNone( self.get_success( self._store.get_global_account_data_by_type_for_user( - AccountDataTypes.DIRECT, self.user, + AccountDataTypes.DIRECT, ) ), ) @@ -304,8 +304,8 @@ def test_account_data_preserved_by_background_job_if_not_deactivated(self) -> No self.assertIsNotNone( self.get_success( self._store.get_global_account_data_by_type_for_user( - AccountDataTypes.DIRECT, self.user, + AccountDataTypes.DIRECT, ) ), ) @@ -318,8 +318,8 @@ def test_account_data_preserved_by_background_job_if_not_deactivated(self) -> No self.assertIsNotNone( self.get_success( self._store.get_global_account_data_by_type_for_user( - AccountDataTypes.DIRECT, self.user, + AccountDataTypes.DIRECT, ) ), ) From 3ecc6bc27060a3ae1bc0ff10758db48d187ce099 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 24 Jan 2022 14:00:03 +0000 Subject: [PATCH 15/20] Move schema delta to correct version --- .../02_delete_account_data_for_deactivated_accounts.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename synapse/storage/schema/main/delta/{65/20_delete_account_data_for_deactivated_accounts.sql => 68/02_delete_account_data_for_deactivated_accounts.sql} (100%) diff --git a/synapse/storage/schema/main/delta/65/20_delete_account_data_for_deactivated_accounts.sql b/synapse/storage/schema/main/delta/68/02_delete_account_data_for_deactivated_accounts.sql similarity index 100% rename from synapse/storage/schema/main/delta/65/20_delete_account_data_for_deactivated_accounts.sql rename to synapse/storage/schema/main/delta/68/02_delete_account_data_for_deactivated_accounts.sql From 9a4ec652e51bf89413fa7a55856783f4e5440629 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 24 Jan 2022 14:02:30 +0000 Subject: [PATCH 16/20] Fix database port script's Method Resolution Order --- scripts/synapse_port_db | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/synapse_port_db b/scripts/synapse_port_db index 84085f68c0ed..6c6caf8a0942 100755 --- a/scripts/synapse_port_db +++ b/scripts/synapse_port_db @@ -181,10 +181,10 @@ class Store( UserDirectoryBackgroundUpdateStore, EndToEndKeyBackgroundStore, StatsStore, + AccountDataWorkerStore, PusherWorkerStore, PresenceBackgroundUpdateStore, GroupServerWorkerStore, - AccountDataWorkerStore, ): def execute(self, f, *args, **kwargs): return self.db_pool.runInteraction(f.__name__, f, *args, **kwargs) From 05439467e97091575eedd13099e92c2458215d43 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 28 Jan 2022 11:38:43 +0000 Subject: [PATCH 17/20] Fix abstract method error in synapse_port_db --- scripts/synapse_port_db | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/synapse_port_db b/scripts/synapse_port_db index 6c6caf8a0942..70ee4e5c7f8e 100755 --- a/scripts/synapse_port_db +++ b/scripts/synapse_port_db @@ -36,6 +36,7 @@ from synapse.logging.context import ( run_in_background, ) from synapse.storage.database import DatabasePool, make_conn +from synapse.storage.databases.main import PushRuleStore from synapse.storage.databases.main.account_data import AccountDataWorkerStore from synapse.storage.databases.main.client_ips import ClientIpBackgroundUpdateStore from synapse.storage.databases.main.deviceinbox import DeviceInboxBackgroundUpdateStore @@ -182,6 +183,7 @@ class Store( EndToEndKeyBackgroundStore, StatsStore, AccountDataWorkerStore, + PushRuleStore, PusherWorkerStore, PresenceBackgroundUpdateStore, GroupServerWorkerStore, From d18e2dd128d8a82b41855ea69dd1fa1e3b5e319e Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 28 Jan 2022 14:41:55 +0000 Subject: [PATCH 18/20] =?UTF-8?q?job=20=E2=86=92=20update?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/handlers/test_deactivate_account.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 1c21799b1fd9..9d6a3276cdf7 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -218,7 +218,7 @@ def test_ignored_users_deleted_upon_deactivation(self) -> None: self.get_success(self._store.ignored_by("@sheltie:test")), set() ) - def _rerun_retroactive_account_data_deletion_job(self) -> None: + def _rerun_retroactive_account_data_deletion_update(self) -> None: # Reset the 'all done' flag self._store.db_pool.updates._all_done = False @@ -234,13 +234,13 @@ def _rerun_retroactive_account_data_deletion_job(self) -> None: self.wait_for_background_updates() - def test_account_data_deleted_retroactively_by_background_job_if_deactivated( + def test_account_data_deleted_retroactively_by_background_update_if_deactivated( self, ) -> None: """ Tests that a user, who deactivated their account before account data was deleted automatically upon deactivation, has their account data retroactively - scrubbed by the background job. + scrubbed by the background update. """ # Request the deactivation of our account @@ -268,8 +268,8 @@ def test_account_data_deleted_retroactively_by_background_job_if_deactivated( ), ) - # Re-run the retroactive deletion job - self._rerun_retroactive_account_data_deletion_job() + # Re-run the retroactive deletion update + self._rerun_retroactive_account_data_deletion_update() # Check that the account data was cleared. self._store.get_global_account_data_by_type_for_user.invalidate_all() @@ -282,9 +282,11 @@ def test_account_data_deleted_retroactively_by_background_job_if_deactivated( ), ) - def test_account_data_preserved_by_background_job_if_not_deactivated(self) -> None: + def test_account_data_preserved_by_background_update_if_not_deactivated( + self, + ) -> None: """ - Tests that the background job does not scrub account data for users that have + Tests that the background update does not scrub account data for users that have not been deactivated. """ @@ -310,8 +312,8 @@ def test_account_data_preserved_by_background_job_if_not_deactivated(self) -> No ), ) - # Re-run the retroactive deletion job - self._rerun_retroactive_account_data_deletion_job() + # Re-run the retroactive deletion update + self._rerun_retroactive_account_data_deletion_update() # Check that the account data was NOT cleared. self._store.get_global_account_data_by_type_for_user.invalidate_all() From 99353f35cb27e309e222b721a24dd2029de2a2f7 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 2 Feb 2022 10:41:25 +0000 Subject: [PATCH 19/20] Remove obsolete manual cache invalidations in tests --- tests/handlers/test_deactivate_account.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 9d6a3276cdf7..01096a1581e3 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -272,7 +272,6 @@ def test_account_data_deleted_retroactively_by_background_update_if_deactivated( self._rerun_retroactive_account_data_deletion_update() # Check that the account data was cleared. - self._store.get_global_account_data_by_type_for_user.invalidate_all() self.assertIsNone( self.get_success( self._store.get_global_account_data_by_type_for_user( @@ -316,7 +315,6 @@ def test_account_data_preserved_by_background_update_if_not_deactivated( self._rerun_retroactive_account_data_deletion_update() # Check that the account data was NOT cleared. - self._store.get_global_account_data_by_type_for_user.invalidate_all() self.assertIsNotNone( self.get_success( self._store.get_global_account_data_by_type_for_user( From 2f7955946ceb30ade5929ca4d3e1f3777a62fa90 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 2 Feb 2022 10:44:34 +0000 Subject: [PATCH 20/20] Fix up background update delta number --- ....sql => 03_delete_account_data_for_deactivated_accounts.sql} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename synapse/storage/schema/main/delta/68/{02_delete_account_data_for_deactivated_accounts.sql => 03_delete_account_data_for_deactivated_accounts.sql} (92%) diff --git a/synapse/storage/schema/main/delta/68/02_delete_account_data_for_deactivated_accounts.sql b/synapse/storage/schema/main/delta/68/03_delete_account_data_for_deactivated_accounts.sql similarity index 92% rename from synapse/storage/schema/main/delta/68/02_delete_account_data_for_deactivated_accounts.sql rename to synapse/storage/schema/main/delta/68/03_delete_account_data_for_deactivated_accounts.sql index 69176660348e..e1249338438e 100644 --- a/synapse/storage/schema/main/delta/68/02_delete_account_data_for_deactivated_accounts.sql +++ b/synapse/storage/schema/main/delta/68/03_delete_account_data_for_deactivated_accounts.sql @@ -17,4 +17,4 @@ -- We want to retroactively delete account data for users that were already -- deactivated. INSERT INTO background_updates (ordering, update_name, progress_json) VALUES - (6520, 'delete_account_data_for_deactivated_users', '{}'); + (6803, 'delete_account_data_for_deactivated_users', '{}');