Unit tests

only consider users MAU after they've been around N days.
This is an alternative implementation to https://github.com/matrix-org/synapse/pull/3739
as suggested by @neilisfragile, which is much simpler as you just hold off adding
users to the MAU table until they've been active for more than N days.

changelog.d/3744.feature

@@ -0,0 +1 @@
+add mau_trial_days config param, so that users only get counted as MAU after N days.

synapse/api/auth.py

@@ -799,8 +799,10 @@ class Auth(object):
         if self.hs.config.limit_usage_by_mau is True:
             # If the user is already part of the MAU cohort
             if user_id:
-                timestamp = yield self.store.user_last_seen_monthly_active(user_id)
-                if timestamp:
+                activity, is_trial = yield self.store.user_last_seen_monthly_active(
+                    user_id,
+                )
+                if activity or is_trial:
                     return
             # Else if there is no room in the MAU bucket, bail
             current_mau = yield self.store.get_monthly_active_count()

synapse/config/server.py

@@ -80,6 +80,9 @@ class ServerConfig(Config):
         self.mau_limits_reserved_threepids = config.get(
             "mau_limit_reserved_threepids", []
         )
+        self.mau_trial_days = config.get(
+            "mau_trial_days", 0,
+        )
 
         # Options to disable HS
         self.hs_disabled = config.get("hs_disabled", False)
@@ -365,6 +368,7 @@ class ServerConfig(Config):
           # Enables monthly active user checking
           # limit_usage_by_mau: False
           # max_mau_value: 50
+          # mau_trial_days: 2
           #
           # Sometimes the server admin will want to ensure certain accounts are
           # never blocked by mau checking. These accounts are specified here.

synapse/server_notices/resource_limits_server_notices.py

@@ -66,8 +66,8 @@ class ResourceLimitsServerNotices(object):
         if self._config.limit_usage_by_mau is False:
             return
 
-        timestamp = yield self._store.user_last_seen_monthly_active(user_id)
-        if timestamp is None:
+        timestamp, is_trial = yield self._store.user_last_seen_monthly_active(user_id)
+        if timestamp is None or is_trial:
             # This user will be blocked from receiving the notice anyway.
             # In practice, not sure we can ever get here
             return

synapse/storage/monthly_active_users.py

@@ -128,7 +128,7 @@ class MonthlyActiveUsersStore(SQLBaseStore):
         # is racy.
         # Have resolved to invalidate the whole cache for now and do
         # something about it if and when the perf becomes significant
-        self.user_last_seen_monthly_active.invalidate_all()
+        self._user_last_seen_monthly_active.invalidate_all()
         self.get_monthly_active_count.invalidate_all()
 
     @cached(num_args=0)
@@ -168,29 +168,59 @@ class MonthlyActiveUsersStore(SQLBaseStore):
             lock=False,
         )
         if is_insert:
-            self.user_last_seen_monthly_active.invalidate((user_id,))
+            self._user_last_seen_monthly_active.invalidate((user_id,))
             self.get_monthly_active_count.invalidate(())
 
-    @cached(num_args=1)
+    @defer.inlineCallbacks
     def user_last_seen_monthly_active(self, user_id):
-        """
-            Checks if a given user is part of the monthly active user group
-            Arguments:
-                user_id (str): user to add/update
-            Return:
-                Deferred[int] : timestamp since last seen, None if never seen
+        """Checks if a given user is part of the monthly active user group
 
+        Args:
+            user_id (str): user to add/update
+
+        Return:
+            Deferred[(int|None, bool)]: First arg is None if the user is not
+                in the mau group, otherwise approximate timestamp they were last
+                seen in milliseconds.
+                The second is a bool indicating if the user is in the trial
+                period or not.
         """
 
-        return(self._simple_select_one_onecol(
-            table="monthly_active_users",
-            keyvalues={
-                "user_id": user_id,
-            },
-            retcol="timestamp",
-            allow_none=True,
-            desc="user_last_seen_monthly_active",
-        ))
+        ret = yield self._user_last_seen_monthly_active(user_id)
+        last_seen, created_at = ret
+
+        mau_trial_ms = self.hs.config.mau_trial_days * 24 * 60 * 60 * 1000
+        is_trial = (self._clock.time_msec() - created_at) < mau_trial_ms
+
+        defer.returnValue((last_seen, is_trial))
+
+    @cached(num_args=1)
+    def _user_last_seen_monthly_active(self, user_id):
+        """Checks if a given user is part of the monthly active user group
+
+        Args:
+            user_id (str): user to add/update
+
+        Return:
+            Deferred[(int, int)|None]: None if never seen, otherwise time user
+            was last seen and registration time of user (both in milliseconds)
+        """
+
+        def _user_last_seen_monthly_active(txn):
+            sql = """
+                SELECT timestamp, creation_ts
+                FROM users LEFT JOIN monthly_active_users
+                ON monthly_active_users.user_id = users.name
+                WHERE name = ?
+            """
+            txn.execute(sql, (user_id,))
+            row = txn.fetchone()
+            return row[0], row[1] * 1000
+
+        return self.runInteraction(
+            "user_last_seen_monthly_active",
+            _user_last_seen_monthly_active
+        )
 
     @defer.inlineCallbacks
     def populate_monthly_active_users(self, user_id):
@@ -201,7 +231,12 @@ class MonthlyActiveUsersStore(SQLBaseStore):
             user_id(str): the user_id to query
         """
         if self.hs.config.limit_usage_by_mau:
-            last_seen_timestamp = yield self.user_last_seen_monthly_active(user_id)
+            last_seen, is_trial = yield self.user_last_seen_monthly_active(user_id)
+
+            if is_trial:
+                # we don't track trial users in the MAU table.
+                return
+
             now = self.hs.get_clock().time_msec()
 
             # We want to reduce to the total number of db writes, and are happy
@@ -209,9 +244,9 @@ class MonthlyActiveUsersStore(SQLBaseStore):
             # We always insert new users (where MAU threshold has not been reached),
             # but only update if we have not previously seen the user for
             # LAST_SEEN_GRANULARITY ms
-            if last_seen_timestamp is None:
+            if last_seen is None:
                 count = yield self.get_monthly_active_count()
                 if count < self.hs.config.max_mau_value:
                     yield self.upsert_monthly_active_user(user_id)
-            elif now - last_seen_timestamp > LAST_SEEN_GRANULARITY:
+            elif now - last_seen > LAST_SEEN_GRANULARITY:
                 yield self.upsert_monthly_active_user(user_id)

tests/handlers/test_auth.py

@@ -41,6 +41,7 @@ class AuthTestCase(unittest.TestCase):
         self.macaroon_generator = self.hs.get_macaroon_generator()
         # MAU tests
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         self.small_number_of_users = 1
         self.large_number_of_users = 100
 
@@ -161,14 +162,14 @@ class AuthTestCase(unittest.TestCase):
             )
         # If in monthly active cohort
         self.hs.get_datastore().user_last_seen_monthly_active = Mock(
-            return_value=defer.succeed(self.hs.get_clock().time_msec())
+            return_value=defer.succeed((self.hs.get_clock().time_msec(), False))
         )
         self.hs.get_datastore().get_monthly_active_count = Mock(
             return_value=defer.succeed(self.hs.config.max_mau_value)
         )
         yield self.auth_handler.get_access_token_for_user_id('user_a')
         self.hs.get_datastore().user_last_seen_monthly_active = Mock(
-            return_value=defer.succeed(self.hs.get_clock().time_msec())
+            return_value=defer.succeed((self.hs.get_clock().time_msec(), False))
         )
         self.hs.get_datastore().get_monthly_active_count = Mock(
             return_value=defer.succeed(self.hs.config.max_mau_value)

tests/handlers/test_register.py

@@ -54,6 +54,7 @@ class RegistrationTestCase(unittest.TestCase):
         self.handler = self.hs.get_handlers().registration_handler
         self.store = self.hs.get_datastore()
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         self.lots_of_users = 100
         self.small_number_of_users = 1
 

tests/handlers/test_sync.py

@@ -42,6 +42,7 @@ class SyncTestCase(tests.unittest.TestCase):
 
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 1
+        self.hs.config.mau_trial_days = 0
 
         # Check that the happy case does not throw errors
         yield self.store.upsert_monthly_active_user(user_id1)

tests/server.py

@@ -5,7 +5,7 @@ from six import text_type
 
 import attr
 
-from twisted.internet import threads
+from twisted.internet import address, threads
 from twisted.internet.defer import Deferred
 from twisted.python.failure import Failure
 from twisted.test.proto_helpers import MemoryReactorClock
@@ -63,7 +63,9 @@ class FakeChannel(object):
         self.result["done"] = True
 
     def getPeer(self):
-        return None
+        # We give an address so that getClientIP returns a non null entry,
+        # causing us to record the MAU
+        return address.IPv4Address(b"TCP", "127.0.0.1", 3423)
 
     def getHost(self):
         return None
@@ -91,7 +93,7 @@ class FakeSite:
         return FakeLogger()
 
 
-def make_request(method, path, content=b""):
+def make_request(method, path, content=b"", access_token=None):
     """
     Make a web request using the given method and path, feed it the
     content, and return the Request and the Channel underneath.
@@ -116,6 +118,11 @@ def make_request(method, path, content=b""):
     req = SynapseRequest(site, channel)
     req.process = lambda: b""
     req.content = BytesIO(content)
+
+    if access_token:
+        req.requestHeaders.addRawHeader(b"Authorization", b"Bearer " + access_token)
+
+    req.requestHeaders.addRawHeader(b"X-Forwarded-For", b"127.0.0.1")
     req.requestReceived(method, path, b"1.1")
 
     return req, channel

tests/storage/test_client_ips.py

@@ -59,6 +59,7 @@ class ClientIpStoreTestCase(tests.unittest.TestCase):
     def test_disabled_monthly_active_user(self):
         self.hs.config.limit_usage_by_mau = False
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         user_id = "@user:server"
         yield self.store.insert_client_ip(
             user_id, "access_token", "ip", "user_agent", "device_id"
@@ -70,6 +71,7 @@ class ClientIpStoreTestCase(tests.unittest.TestCase):
     def test_adding_monthly_active_user_when_full(self):
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         lots_of_users = 100
         user_id = "@user:server"
 
@@ -86,6 +88,7 @@ class ClientIpStoreTestCase(tests.unittest.TestCase):
     def test_adding_monthly_active_user_when_space(self):
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         user_id = "@user:server"
         active = yield self.store.user_last_seen_monthly_active(user_id)
         self.assertFalse(active)
@@ -100,6 +103,7 @@ class ClientIpStoreTestCase(tests.unittest.TestCase):
     def test_updating_monthly_active_user_when_space(self):
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         user_id = "@user:server"
 
         active = yield self.store.user_last_seen_monthly_active(user_id)

tests/storage/test_monthly_active_users.py

@@ -30,6 +30,7 @@ class MonthlyActiveUsersTestCase(tests.unittest.TestCase):
     def setUp(self):
         self.hs = yield setup_test_homeserver(self.addCleanup)
         self.store = self.hs.get_datastore()
+        self.hs.config.mau_trial_days = 0
 
     @defer.inlineCallbacks
     def test_initialise_reserved_users(self):
@@ -105,13 +106,13 @@ class MonthlyActiveUsersTestCase(tests.unittest.TestCase):
         user_id3 = "@user3:server"
 
         result = yield self.store.user_last_seen_monthly_active(user_id1)
-        self.assertFalse(result == 0)
+        self.assertFalse(result)
         yield self.store.upsert_monthly_active_user(user_id1)
         yield self.store.upsert_monthly_active_user(user_id2)
         result = yield self.store.user_last_seen_monthly_active(user_id1)
-        self.assertTrue(result > 0)
+        self.assertTrue(result[0] > 0)
         result = yield self.store.user_last_seen_monthly_active(user_id3)
-        self.assertFalse(result == 0)
+        self.assertFalse(result)
 
     @defer.inlineCallbacks
     def test_reap_monthly_active_users(self):

tests/test_mau.py

@@ -0,0 +1,217 @@
+# -*- coding: utf-8 -*-
+# Copyright 2018 New Vector Ltd
+#
+# 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.
+
+"""Tests REST events for /rooms paths."""
+
+import json
+
+from mock import Mock, NonCallableMock
+
+from synapse.api.constants import LoginType
+from synapse.api.errors import Codes, HttpResponseException, SynapseError
+from synapse.http.server import JsonResource
+from synapse.rest.client.v2_alpha import register, sync
+from synapse.util import Clock
+
+from tests import unittest
+from tests.server import (
+    ThreadedMemoryReactorClock,
+    make_request,
+    render,
+    setup_test_homeserver,
+)
+
+
+class TestMauLimit(unittest.TestCase):
+    def setUp(self):
+        self.reactor = ThreadedMemoryReactorClock()
+        self.clock = Clock(self.reactor)
+
+        self.hs = setup_test_homeserver(
+            self.addCleanup,
+            "red",
+            http_client=None,
+            clock=self.clock,
+            reactor=self.reactor,
+            federation_client=Mock(),
+            ratelimiter=NonCallableMock(spec_set=["send_message"]),
+        )
+
+        self.store = self.hs.get_datastore()
+
+        self.hs.config.registrations_require_3pid = []
+        self.hs.config.enable_registration_captcha = False
+        self.hs.config.recaptcha_public_key = []
+
+        self.hs.config.limit_usage_by_mau = True
+        self.hs.config.hs_disabled = False
+        self.hs.config.max_mau_value = 2
+        self.hs.config.mau_trial_days = 0
+        self.hs.config.server_notices_mxid = "@server:red"
+        self.hs.config.server_notices_mxid_display_name = None
+        self.hs.config.server_notices_mxid_avatar_url = None
+        self.hs.config.server_notices_room_name = "Test Server Notice Room"
+
+        self.resource = JsonResource(self.hs)
+        register.register_servlets(self.hs, self.resource)
+        sync.register_servlets(self.hs, self.resource)
+
+    def test_simple_deny_mau(self):
+        # Create and sync so that the MAU counts get updated
+        token1 = self.create_user("kermit1")
+        self.do_sync_for_user(token1)
+        token2 = self.create_user("kermit2")
+        self.do_sync_for_user(token2)
+
+        # We've created and activated two users, we shouldn't be able to
+        # register new users
+        with self.assertRaises(SynapseError) as cm:
+            self.create_user("kermit3")
+
+        e = cm.exception
+        self.assertEqual(e.code, 403)
+        self.assertEqual(e.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
+
+    def test_allowed_after_a_month_mau(self):
+        # Create and sync so that the MAU counts get updated
+        token1 = self.create_user("kermit1")
+        self.do_sync_for_user(token1)
+        token2 = self.create_user("kermit2")
+        self.do_sync_for_user(token2)
+
+        # Advance time by 31 days
+        self.reactor.advance(31 * 24 * 60 * 60)
+
+        self.store.reap_monthly_active_users()
+
+        self.reactor.advance(0)
+
+        # We should be able to register more users
+        token3 = self.create_user("kermit3")
+        self.do_sync_for_user(token3)
+
+    def test_trial_delay(self):
+        self.hs.config.mau_trial_days = 1
+
+        # We should be able to register more than the limit initially
+        token1 = self.create_user("kermit1")
+        self.do_sync_for_user(token1)
+        token2 = self.create_user("kermit2")
+        self.do_sync_for_user(token2)
+        token3 = self.create_user("kermit3")
+        self.do_sync_for_user(token3)
+
+        # Advance time by 2 days
+        self.reactor.advance(2 * 24 * 60 * 60)
+
+        # Two users should be able to sync
+        self.do_sync_for_user(token1)
+        self.do_sync_for_user(token2)
+
+        # But the third should fail
+        with self.assertRaises(SynapseError) as cm:
+            self.do_sync_for_user(token3)
+
+        e = cm.exception
+        self.assertEqual(e.code, 403)
+        self.assertEqual(e.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
+
+        # And new registrations are now denied too
+        with self.assertRaises(SynapseError) as cm:
+            self.create_user("kermit4")
+
+        e = cm.exception
+        self.assertEqual(e.code, 403)
+        self.assertEqual(e.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
+
+    def test_trial_users_cant_come_back(self):
+        self.hs.config.mau_trial_days = 1
+
+        # We should be able to register more than the limit initially
+        token1 = self.create_user("kermit1")
+        self.do_sync_for_user(token1)
+        token2 = self.create_user("kermit2")
+        self.do_sync_for_user(token2)
+        token3 = self.create_user("kermit3")
+        self.do_sync_for_user(token3)
+
+        # Advance time by 2 days
+        self.reactor.advance(2 * 24 * 60 * 60)
+
+        # Two users should be able to sync
+        self.do_sync_for_user(token1)
+        self.do_sync_for_user(token2)
+
+        # Advance by 2 months so everyone falls out of MAU
+        self.reactor.advance(60 * 24 * 60 * 60)
+        self.store.reap_monthly_active_users()
+        self.reactor.advance(0)
+
+        # We can create as many new users as we want
+        token4 = self.create_user("kermit4")
+        self.do_sync_for_user(token4)
+        token5 = self.create_user("kermit5")
+        self.do_sync_for_user(token5)
+        token6 = self.create_user("kermit6")
+        self.do_sync_for_user(token6)
+
+        # users 2 and 3 can come back to bring us back up to MAU limit
+        self.do_sync_for_user(token2)
+        self.do_sync_for_user(token3)
+
+        # New trial users can still sync
+        self.do_sync_for_user(token4)
+        self.do_sync_for_user(token5)
+        self.do_sync_for_user(token6)
+
+        # But old user cant
+        with self.assertRaises(SynapseError) as cm:
+            self.do_sync_for_user(token1)
+
+        e = cm.exception
+        self.assertEqual(e.code, 403)
+        self.assertEqual(e.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
+
+    def create_user(self, localpart):
+        request_data = json.dumps({
+            "username": localpart,
+            "password": "monkey",
+            "auth": {"type": LoginType.DUMMY},
+        })
+
+        request, channel = make_request(b"POST", b"/register", request_data)
+        render(request, self.resource, self.reactor)
+
+        if channel.result["code"] != b"200":
+            raise HttpResponseException(
+                int(channel.result["code"]),
+                channel.result["reason"],
+                channel.result["body"],
+            ).to_synapse_error()
+
+        access_token = channel.json_body["access_token"]
+
+        return access_token
+
+    def do_sync_for_user(self, token):
+        request, channel = make_request(b"GET", b"/sync", access_token=token)
+        render(request, self.resource, self.reactor)
+
+        if channel.result["code"] != b"200":
+            raise HttpResponseException(
+                int(channel.result["code"]),
+                channel.result["reason"],
+                channel.result["body"],
+            ).to_synapse_error()