From d75d2c9974339d269ba0add9220f19c7440929d7 Mon Sep 17 00:00:00 2001 From: Puneeth Chaganti Date: Tue, 12 Mar 2019 08:58:40 +0530 Subject: [PATCH] soft-deactivation: Run catch-up when "auto" deactivate is run. When soft deactivation is run for in "auto" mode (no emails are specified and all users inactive for specified number of days are deactivated), catch-up is also run in the "auto" mode if AUTO_CATCH_UP_SOFT_DEACTIVATED_USERS is True. Automatically catching up soft-deactivated users periodically would ensure a good user experience for returning users, but on some servers we may want to turn off this option to save on some disk space. Fixes #8858, at least for the default configuration, by eliminating the situation where there are a very large number of messages to recover. --- zerver/lib/soft_deactivation.py | 21 +++++- .../commands/soft_deactivate_users.py | 15 ++-- zerver/tests/test_soft_deactivation.py | 70 ++++++++++++++++++- zproject/settings.py | 6 ++ 4 files changed, 100 insertions(+), 12 deletions(-) diff --git a/zerver/lib/soft_deactivation.py b/zerver/lib/soft_deactivation.py index 13aa10bab8..7e0b0e37e0 100644 --- a/zerver/lib/soft_deactivation.py +++ b/zerver/lib/soft_deactivation.py @@ -7,10 +7,10 @@ from django.db import transaction from django.db.models import Max from django.conf import settings from django.utils.timezone import now as timezone_now -from typing import DefaultDict, List, Union, Any +from typing import DefaultDict, Dict, List, Optional, Union, Any from zerver.models import UserProfile, UserMessage, RealmAuditLog, \ - Subscription, Message, Recipient, UserActivity + Subscription, Message, Recipient, UserActivity, Realm logger = logging.getLogger("zulip.soft_deactivation") log_to_file(logger, settings.SOFT_DEACTIVATION_LOG_PATH) @@ -210,6 +210,23 @@ def do_soft_deactivate_users(users: List[UserProfile]) -> List[UserProfile]: return users_soft_deactivated +def do_auto_soft_deactivate_users(inactive_for_days: int, realm: Optional[Realm]) -> List[UserProfile]: + filter_kwargs = {} # type: Dict[str, Realm] + if realm is not None: + filter_kwargs = dict(user_profile__realm=realm) + users_to_deactivate = get_users_for_soft_deactivation(inactive_for_days, filter_kwargs) + users_deactivated = do_soft_deactivate_users(users_to_deactivate) + + if not settings.AUTO_CATCH_UP_SOFT_DEACTIVATED_USERS: + logging.info('Not catching up users since AUTO_CATCH_UP_SOFT_DEACTIVATED_USERS if off') + return users_deactivated + + if realm is not None: + filter_kwargs = dict(realm=realm) + users_to_catch_up = get_soft_deactivated_users_for_catch_up(filter_kwargs) + do_catch_up_soft_deactivated_users(users_to_catch_up) + return users_deactivated + def reactivate_user_if_soft_deactivated(user_profile: UserProfile) -> Union[UserProfile, None]: if user_profile.long_term_idle: add_missing_messages(user_profile) diff --git a/zerver/management/commands/soft_deactivate_users.py b/zerver/management/commands/soft_deactivate_users.py index 97231a790f..fa351576c2 100644 --- a/zerver/management/commands/soft_deactivate_users.py +++ b/zerver/management/commands/soft_deactivate_users.py @@ -7,7 +7,7 @@ from django.core.management.base import CommandError from zerver.lib.management import ZulipBaseCommand from zerver.lib.soft_deactivation import do_soft_activate_users, \ - do_soft_deactivate_users, get_users_for_soft_deactivation, logger + do_soft_deactivate_users, do_auto_soft_deactivate_users, logger from zerver.models import Realm, UserProfile def get_users_from_emails(emails: Any, @@ -73,15 +73,12 @@ class Command(ZulipBaseCommand): if user_emails: users_to_deactivate = get_users_from_emails(user_emails, filter_kwargs) print('Soft deactivating forcefully...') - else: - if realm is not None: - filter_kwargs = dict(user_profile__realm=realm) - users_to_deactivate = get_users_for_soft_deactivation(int(options['inactive_for']), - filter_kwargs) - - if users_to_deactivate: users_deactivated = do_soft_deactivate_users(users_to_deactivate) - logger.info('Soft Deactivated %d user(s)' % (len(users_deactivated))) + else: + users_deactivated = do_auto_soft_deactivate_users(int(options['inactive_for']), + realm) + logger.info('Soft Deactivated %d user(s)' % (len(users_deactivated))) + else: self.print_help("./manage.py", "soft_deactivate_users") sys.exit(1) diff --git a/zerver/tests/test_soft_deactivation.py b/zerver/tests/test_soft_deactivation.py index 0a20d526e5..6b05bab665 100644 --- a/zerver/tests/test_soft_deactivation.py +++ b/zerver/tests/test_soft_deactivation.py @@ -10,7 +10,8 @@ from zerver.lib.soft_deactivation import ( get_users_for_soft_deactivation, do_soft_activate_users, get_soft_deactivated_users_for_catch_up, - do_catch_up_soft_deactivated_users + do_catch_up_soft_deactivated_users, + do_auto_soft_deactivate_users ) from zerver.lib.test_classes import ZulipTestCase from zerver.models import ( @@ -149,3 +150,70 @@ class UserSoftDeactivationTests(ZulipTestCase): user.refresh_from_db() self.assertTrue(user.long_term_idle) self.assertEqual(user.last_active_message_id, message_id) + + def test_do_auto_soft_deactivate_users(self) -> None: + users = [ + self.example_user('iago'), + self.example_user('cordelia'), + self.example_user('ZOE'), + self.example_user('othello'), + self.example_user('prospero'), + self.example_user('aaron'), + self.example_user('polonius'), + ] + sender = self.example_user('hamlet') + realm = get_realm('zulip') + stream_name = 'announce' + for user in users + [sender]: + self.subscribe(user, stream_name) + + client, _ = Client.objects.get_or_create(name='website') + query = '/json/users/me/pointer' + last_visit = timezone_now() + count = 150 + for user_profile in users: + UserActivity.objects.get_or_create( + user_profile=user_profile, + client=client, + query=query, + count=count, + last_visit=last_visit + ) + + with mock.patch('logging.info'): + users_deactivated = do_auto_soft_deactivate_users(-1, realm) + self.assert_length(users_deactivated, len(users)) + for user in users: + self.assertTrue(user in users_deactivated) + + # Verify that deactivated users are caught up automatically + + message_id = self.send_stream_message(sender.email, stream_name) + received_count = UserMessage.objects.filter(user_profile__in=users, + message_id=message_id).count() + self.assertEqual(0, received_count) + + with mock.patch('logging.info'): + users_deactivated = do_auto_soft_deactivate_users(-1, realm) + + self.assert_length(users_deactivated, 0) # all users are already deactivated + received_count = UserMessage.objects.filter(user_profile__in=users, + message_id=message_id).count() + self.assertEqual(len(users), received_count) + + # Verify that deactivated users are NOT caught up if + # AUTO_CATCH_UP_SOFT_DEACTIVATED_USERS is off + + message_id = self.send_stream_message(sender.email, stream_name) + received_count = UserMessage.objects.filter(user_profile__in=users, + message_id=message_id).count() + self.assertEqual(0, received_count) + + with self.settings(AUTO_CATCH_UP_SOFT_DEACTIVATED_USERS=False): + with mock.patch('logging.info'): + users_deactivated = do_auto_soft_deactivate_users(-1, realm) + + self.assert_length(users_deactivated, 0) # all users are already deactivated + received_count = UserMessage.objects.filter(user_profile__in=users, + message_id=message_id).count() + self.assertEqual(0, received_count) diff --git a/zproject/settings.py b/zproject/settings.py index d527ad17fe..c1a61053b3 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -454,6 +454,12 @@ DEFAULT_SETTINGS.update({ # Enables billing pages and plan-based feature gates. If False, all features # are available to all realms. 'BILLING_ENABLED': False, + + # Automatically catch-up soft deactivated users when running the + # `soft-deactivate-users` cron. Turn this off if the server has 10Ks of + # users, and you would like to save some disk space. Soft-deactivated + # returning users would still be caught-up normally. + 'AUTO_CATCH_UP_SOFT_DEACTIVATED_USERS': True, })