add mau_trial_days config param.

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.

8 files changed, 55 insertions, 21 deletions

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 6502a6be7b..3d8e7ae5de 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -793,8 +793,8 @@ 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 = yield self.store.user_last_seen_monthly_active(user_id)
+                if activity:
                     return
             # Else if there is no room in the MAU bucket, bail
             current_mau = yield self.store.get_monthly_active_count()
diff --git a/synapse/config/server.py b/synapse/config/server.py
index 68a612e594..40bbe9133c 100644
--- a/synapse/config/server.py
+++ b/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.
diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py
index 06f9a75a97..84297f4fe1 100644
--- a/synapse/storage/monthly_active_users.py
+++ b/synapse/storage/monthly_active_users.py
@@ -177,19 +177,36 @@ class MonthlyActiveUsersStore(SQLBaseStore):
             Arguments:
                 user_id (str): user to add/update
             Return:
-                Deferred[int] : timestamp since last seen, None if never seen
-
+                Deferred[(int,bool)|None] : (time since last seen, bool if trial user)
+                    None if never seen
         """
 
-        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",
-        ))
+        mau_trial_ms = self.hs.config.mau_trial_days * 24 * 60 * 60 * 1000
+
+        def _user_last_seen_monthly_active(txn):
+
+            sql = """
+                SELECT timestamp, creation_ts
+                FROM monthly_active_users LEFT JOIN users
+                ON monthly_active_users.user_id = users.name
+                WHERE user_id = ?
+            """
+
+            txn.execute(sql, (user_id,))
+            rows = txn.fetchall()
+            if not rows or not rows[0][0]:
+                return None
+            else:
+                (timestamp, created) = (rows[0][0], rows[0][1])
+                if not created:
+                    created = 0
+                trial = (timestamp - created * 1000) < mau_trial_ms
+                return (timestamp, trial)
+
+        return self.runInteraction(
+            "user_last_seen_monthly_active",
+            _user_last_seen_monthly_active
+        )
 
     @defer.inlineCallbacks
     def populate_monthly_active_users(self, user_id):
@@ -200,7 +217,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)
+            activity = yield self.user_last_seen_monthly_active(user_id)
+
+            if activity and activity[1]:
+                # 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
@@ -208,9 +230,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 activity 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 - activity[0] > LAST_SEEN_GRANULARITY:
                 yield self.upsert_monthly_active_user(user_id)
diff --git a/tests/handlers/test_auth.py b/tests/handlers/test_auth.py
index 1e39fe0ec2..2abca653aa 100644
--- a/tests/handlers/test_auth.py
+++ b/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)
diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py
index 7b4ade3dfb..81947820a1 100644
--- a/tests/handlers/test_register.py
+++ b/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
 
diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py
index a01ab471f5..9825bb65c0 100644
--- a/tests/handlers/test_sync.py
+++ b/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)
diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py
index c2e88bdbaf..5df78b3d0a 100644
--- a/tests/storage/test_client_ips.py
+++ b/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)
diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py
index f2ed866ae7..8e7b93a93d 100644
--- a/tests/storage/test_monthly_active_users.py
+++ b/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):