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

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

Merged
merged 9 commits into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/10947.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes a long-standing bug wherin deactivated users still count towards the mau limit.
4 changes: 4 additions & 0 deletions synapse/handlers/deactivate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ async def deactivate_account(
# delete from user directory
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
if erase_data:
user = UserID.from_string(user_id)
Expand Down
24 changes: 24 additions & 0 deletions synapse/storage/databases/main/monthly_active_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,3 +354,27 @@ async def populate_monthly_active_users(self, user_id):
await self.upsert_monthly_active_user(user_id)
elif now - last_seen_timestamp > LAST_SEEN_GRANULARITY:
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", ()
)
37 changes: 34 additions & 3 deletions tests/test_mau.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
# limitations under the License.

"""Tests REST events for /rooms paths."""

import synapse.rest.admin
from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType
from synapse.api.errors import Codes, HttpResponseException, SynapseError
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.unittest import override_config
Expand All @@ -26,7 +26,13 @@

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):
config = default_config("test")
Expand Down Expand Up @@ -229,6 +235,31 @@ def test_tracked_but_not_limited(self):
self.reactor.advance(100)
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):
request_data = {
"username": localpart,
Expand Down