Add functionality to remove deactivated users from the monthly_active_users table (#10947)

* add test

* add function to remove user from monthly active table in deactivate code

* add function to remove user from monthly active table

* add changelog entry

* update changelog number

* requested changes

* update docstring on new function

* fix lint error

* Update synapse/storage/databases/main/monthly_active_users.py

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
This commit is contained in:
Hillery Shay 2021-10-04 08:34:42 -07:00 committed by GitHub
parent 30f0240401
commit eda8c88b84
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 63 additions and 3 deletions

1
changelog.d/10947.bugfix Normal file
View File

@ -0,0 +1 @@
Fixes a long-standing bug wherin deactivated users still count towards the mau limit.

View File

@ -133,6 +133,10 @@ class DeactivateAccountHandler(BaseHandler):
# delete from user directory # delete from user directory
await self.user_directory_handler.handle_local_user_deactivated(user_id) await self.user_directory_handler.handle_local_user_deactivated(user_id)
# If the user is present in the monthly active users table
# remove them
await self.store.remove_deactivated_user_from_mau_table(user_id)
# Mark the user as erased, if they asked for that # Mark the user as erased, if they asked for that
if erase_data: if erase_data:
user = UserID.from_string(user_id) user = UserID.from_string(user_id)

View File

@ -354,3 +354,27 @@ class MonthlyActiveUsersStore(MonthlyActiveUsersWorkerStore):
await self.upsert_monthly_active_user(user_id) await self.upsert_monthly_active_user(user_id)
elif now - last_seen_timestamp > LAST_SEEN_GRANULARITY: elif now - last_seen_timestamp > LAST_SEEN_GRANULARITY:
await self.upsert_monthly_active_user(user_id) await self.upsert_monthly_active_user(user_id)
async def remove_deactivated_user_from_mau_table(self, user_id: str) -> None:
"""
Removes a deactivated user from the monthly active user
table and resets affected caches.
Args:
user_id(str): the user_id to remove
"""
rows_deleted = await self.db_pool.simple_delete(
table="monthly_active_users",
keyvalues={"user_id": user_id},
desc="simple_delete",
)
if rows_deleted != 0:
await self.invalidate_cache_and_stream(
"user_last_seen_monthly_active", (user_id,)
)
await self.invalidate_cache_and_stream("get_monthly_active_count", ())
await self.invalidate_cache_and_stream(
"get_monthly_active_count_by_service", ()
)

View File

@ -13,11 +13,11 @@
# limitations under the License. # limitations under the License.
"""Tests REST events for /rooms paths.""" """Tests REST events for /rooms paths."""
import synapse.rest.admin
from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType
from synapse.api.errors import Codes, HttpResponseException, SynapseError from synapse.api.errors import Codes, HttpResponseException, SynapseError
from synapse.appservice import ApplicationService from synapse.appservice import ApplicationService
from synapse.rest.client import register, sync from synapse.rest.client import login, profile, register, sync
from tests import unittest from tests import unittest
from tests.unittest import override_config from tests.unittest import override_config
@ -26,7 +26,13 @@ from tests.utils import default_config
class TestMauLimit(unittest.HomeserverTestCase): class TestMauLimit(unittest.HomeserverTestCase):
servlets = [register.register_servlets, sync.register_servlets] servlets = [
register.register_servlets,
sync.register_servlets,
synapse.rest.admin.register_servlets_for_client_rest_resource,
profile.register_servlets,
login.register_servlets,
]
def default_config(self): def default_config(self):
config = default_config("test") config = default_config("test")
@ -229,6 +235,31 @@ class TestMauLimit(unittest.HomeserverTestCase):
self.reactor.advance(100) self.reactor.advance(100)
self.assertEqual(2, self.successResultOf(count)) self.assertEqual(2, self.successResultOf(count))
def test_deactivated_users_dont_count_towards_mau(self):
user1 = self.register_user("madonna", "password")
self.register_user("prince", "password2")
self.register_user("frodo", "onering", True)
token1 = self.login("madonna", "password")
token2 = self.login("prince", "password2")
admin_token = self.login("frodo", "onering")
self.do_sync_for_user(token1)
self.do_sync_for_user(token2)
# Check that mau count is what we expect
count = self.get_success(self.store.get_monthly_active_count())
self.assertEqual(count, 2)
# Deactivate user1
url = "/_synapse/admin/v1/deactivate/%s" % user1
channel = self.make_request("POST", url, access_token=admin_token)
self.assertIn("success", channel.json_body["id_server_unbind_result"])
# Check that deactivated user is no longer counted
count = self.get_success(self.store.get_monthly_active_count())
self.assertEqual(count, 1)
def create_user(self, localpart, token=None, appservice=False): def create_user(self, localpart, token=None, appservice=False):
request_data = { request_data = {
"username": localpart, "username": localpart,