From 61b05727fa6ba879f507d13ff40c0197916ef795 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 5 Sep 2018 22:30:36 +0100 Subject: [PATCH 1/3] guest users should not be part of mau total --- synapse/storage/monthly_active_users.py | 7 +++- tests/storage/test_monthly_active_users.py | 44 +++++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index c7899d7fd269..e34e16c07979 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -192,14 +192,19 @@ def user_last_seen_monthly_active(self, user_id): )) @defer.inlineCallbacks - def populate_monthly_active_users(self, user_id): + def populate_monthly_active_users(self, user_id, is_guest=False): """Checks on the state of monthly active user limits and optionally add the user to the monthly active tables Args: user_id(str): the user_id to query """ + if self.hs.config.limit_usage_by_mau: + # Guests should not be included as part of MAU group + if is_guest: + return + is_trial = yield self.is_trial_user(user_id) if is_trial: # we don't track trial users in the MAU table. diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index 2036287288ac..cfe0a7c77d25 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -12,6 +12,9 @@ # 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. +from mock import Mock + +from twisted.internet import defer from tests.unittest import HomeserverTestCase @@ -23,7 +26,8 @@ def make_homeserver(self, reactor, clock): hs = self.setup_test_homeserver() self.store = hs.get_datastore() - + hs.config.limit_usage_by_mau = True + hs.config.max_mau_value = 50 # Advance the clock a bit reactor.advance(FORTY_DAYS) @@ -73,7 +77,7 @@ def test_initialise_reserved_users(self): active_count = self.store.get_monthly_active_count() self.assertEquals(self.get_success(active_count), user_num) - # Test that regalar users are removed from the db + # Test that regular users are removed from the db ru_count = 2 self.store.upsert_monthly_active_user("@ru1:server") self.store.upsert_monthly_active_user("@ru2:server") @@ -139,3 +143,39 @@ def test_reap_monthly_active_users(self): count = self.store.get_monthly_active_count() self.assertEquals(self.get_success(count), 0) + + def test_populate_monthly_users_is_guest(self): + # Test that guest users are not added to mau list + self.store.upsert_monthly_active_user = Mock() + self.store.populate_monthly_active_users('user_id', True) + self.pump() + self.store.upsert_monthly_active_user.assert_not_called() + + def test_populate_monthly_users_should_update(self): + self.store.upsert_monthly_active_user = Mock() + + self.store.is_trial_user = Mock( + return_value=defer.succeed(False) + ) + + self.store.user_last_seen_monthly_active = Mock( + return_value=defer.succeed(None) + ) + self.store.populate_monthly_active_users('user_id') + self.pump() + self.store.upsert_monthly_active_user.assert_called_once() + + def test_populate_monthly_users_should_not_update(self): + self.store.upsert_monthly_active_user = Mock() + + self.store.is_trial_user = Mock( + return_value=defer.succeed(False) + ) + self.store.user_last_seen_monthly_active = Mock( + return_value=defer.succeed( + self.hs.get_clock().time_msec() + ) + ) + self.store.populate_monthly_active_users('user_id') + self.pump() + self.store.upsert_monthly_active_user.assert_not_called() From 92657be7d0e108f11f5390222df9efc9f1e55908 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 5 Sep 2018 22:33:45 +0100 Subject: [PATCH 2/3] towncrier --- changelog.d/3800.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3800.bugfix diff --git a/changelog.d/3800.bugfix b/changelog.d/3800.bugfix new file mode 100644 index 000000000000..6b2e18b4a6e4 --- /dev/null +++ b/changelog.d/3800.bugfix @@ -0,0 +1 @@ +guest users should not be part of mau total From 84a750e0c36f653b71a06c564154f257c1b7dee3 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Thu, 6 Sep 2018 17:22:53 +0100 Subject: [PATCH 3/3] ensure guests never enter mau list --- synapse/storage/monthly_active_users.py | 7 +++---- tests/storage/test_client_ips.py | 4 +--- tests/storage/test_monthly_active_users.py | 6 +++++- tests/utils.py | 1 + 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index e34e16c07979..b890c152dbec 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -192,7 +192,7 @@ def user_last_seen_monthly_active(self, user_id): )) @defer.inlineCallbacks - def populate_monthly_active_users(self, user_id, is_guest=False): + def populate_monthly_active_users(self, user_id): """Checks on the state of monthly active user limits and optionally add the user to the monthly active tables @@ -201,13 +201,12 @@ def populate_monthly_active_users(self, user_id, is_guest=False): """ if self.hs.config.limit_usage_by_mau: - # Guests should not be included as part of MAU group + # Trial users and guests should not be included as part of MAU group + is_guest = yield self.is_guest(user_id) if is_guest: return - is_trial = yield self.is_trial_user(user_id) if is_trial: - # we don't track trial users in the MAU table. return last_seen_timestamp = yield self.user_last_seen_monthly_active(user_id) diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py index c2e88bdbafeb..c9b02a062be5 100644 --- a/tests/storage/test_client_ips.py +++ b/tests/storage/test_client_ips.py @@ -101,13 +101,11 @@ def test_updating_monthly_active_user_when_space(self): self.hs.config.limit_usage_by_mau = True self.hs.config.max_mau_value = 50 user_id = "@user:server" + yield self.store.register(user_id=user_id, token="123", password_hash=None) active = yield self.store.user_last_seen_monthly_active(user_id) self.assertFalse(active) - yield self.store.insert_client_ip( - user_id, "access_token", "ip", "user_agent", "device_id" - ) yield self.store.insert_client_ip( user_id, "access_token", "ip", "user_agent", "device_id" ) diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index cfe0a7c77d25..ccfc21b7fcdf 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -146,8 +146,12 @@ def test_reap_monthly_active_users(self): def test_populate_monthly_users_is_guest(self): # Test that guest users are not added to mau list + user_id = "user_id" + self.store.register( + user_id=user_id, token="123", password_hash=None, make_guest=True + ) self.store.upsert_monthly_active_user = Mock() - self.store.populate_monthly_active_users('user_id', True) + self.store.populate_monthly_active_users(user_id) self.pump() self.store.upsert_monthly_active_user.assert_not_called() diff --git a/tests/utils.py b/tests/utils.py index 114470d64118..f24bd81bf64f 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -147,6 +147,7 @@ def setup_test_homeserver( config.hs_disabled_message = "" config.hs_disabled_limit_type = "" config.max_mau_value = 50 + config.mau_trial_days = 0 config.mau_limits_reserved_threepids = [] config.admin_contact = None config.rc_messages_per_second = 10000