Add functionality to remove deactivated users from the monthly_active_users table

changelog.d/10947.bugfix

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

synapse/handlers/deactivate_account.py

@@ -133,6 +133,12 @@ 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
+        monthly_user = await self.store.user_last_seen_monthly_active(user_id)
+        if monthly_user:
+            await self.store.remove_deactivated_user(user_id)
+
         # Mark the user as erased, if they asked for that
         if erase_data:
             user = UserID.from_string(user_id)

synapse/storage/databases/main/monthly_active_users.py

@@ -354,3 +354,17 @@ 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(self, user_id: str) -> None:
+        """
+        Removes a deactivated user from the monthly active user
+        table and resets the "get_monthly_active_count" cache.
+
+        Args:
+            user_id(str): the user_id to remove
+        """
+
+        await self.db_pool.simple_delete_one(
+            table="monthly_active_users", keyvalues={"user_id": user_id}
+        )
+        await self.invalidate_cache_and_stream("get_monthly_active_count", ())

tests/test_mau.py

@@ -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
@@ -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")
@@ -229,6 +235,32 @@ 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)
+        print(channel.json_body)
+        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,