retention: Replace Realm.message_retention_days None value with -1.

To be more consistent with the meaning in the Stream model, and to make
it easier to have a reasonable settings API, we get rid of the None
value for Realm.message_retention_days in favor of the value -1 to
represent the "don't delete messages" default policy.

zerver/lib/retention.py

@@ -173,6 +173,8 @@ def run_archiving_in_chunks(
 def move_expired_messages_to_archive_by_recipient(recipient: Recipient,
                                                   message_retention_days: int, realm: Realm,
                                                   chunk_size: int=MESSAGE_BATCH_SIZE) -> int:
+    assert message_retention_days != -1
+
     # This function will archive appropriate messages and their related objects.
     query = SQL("""
     INSERT INTO zerver_archivedmessage ({dst_fields}, archive_transaction_id)
@@ -198,6 +200,10 @@ def move_expired_messages_to_archive_by_recipient(recipient: Recipient,
 def move_expired_personal_and_huddle_messages_to_archive(realm: Realm,
                                                          chunk_size: int=MESSAGE_BATCH_SIZE,
                                                          ) -> int:
+    message_retention_days = realm.message_retention_days
+    assert message_retention_days != -1
+    check_date = timezone_now() - timedelta(days=message_retention_days)
+
     # This function will archive appropriate messages and their related objects.
     cross_realm_bot_ids = [
         get_user_including_cross_realm(email).id
@@ -221,8 +227,6 @@ def move_expired_personal_and_huddle_messages_to_archive(realm: Realm,
     ON CONFLICT (id) DO UPDATE SET archive_transaction_id = {archive_transaction_id}
     RETURNING id
     """)
-    assert realm.message_retention_days is not None
-    check_date = timezone_now() - timedelta(days=realm.message_retention_days)
 
     message_count = run_archiving_in_chunks(
         query,
@@ -364,7 +368,7 @@ def archive_stream_messages(realm: Realm, streams: List[Stream], chunk_size: int
         if stream.message_retention_days:
             retention_policy_dict[stream.id] = stream.message_retention_days
         else:
-            assert realm.message_retention_days is not None
+            assert realm.message_retention_days != -1
             retention_policy_dict[stream.id] = realm.message_retention_days
 
     recipients = [stream.recipient for stream in streams]
@@ -381,7 +385,7 @@ def archive_messages(chunk_size: int=MESSAGE_BATCH_SIZE) -> None:
 
     for realm, streams in get_realms_and_streams_for_archiving():
         archive_stream_messages(realm, streams, chunk_size)
-        if realm.message_retention_days:
+        if realm.message_retention_days != -1:
             archive_personal_and_huddle_messages(realm, chunk_size)
 
         # Messages have been archived for the realm, now we can clean up attachments:
@@ -401,7 +405,7 @@ def get_realms_and_streams_for_archiving() -> List[Tuple[Realm, List[Stream]]]:
     realm_id_to_streams_list: Dict[int, List[Stream]] = {}
 
     # All realms with a retention policy set qualify for archiving:
-    for realm in Realm.objects.filter(message_retention_days__isnull=False):
+    for realm in Realm.objects.exclude(message_retention_days=-1):
         realm_id_to_realm[realm.id] = realm
         realm_id_to_streams_list[realm.id] = []
 
@@ -409,11 +413,11 @@ def get_realms_and_streams_for_archiving() -> List[Tuple[Realm, List[Stream]]]:
     # First category are streams in retention-enabled realms,
     # that don't have retention explicitly disabled (through the value -1).
     query_one = Stream.objects.exclude(message_retention_days=-1) \
-        .filter(realm__message_retention_days__isnull=False) \
+        .exclude(realm__message_retention_days=-1) \
         .select_related('realm', 'recipient')
     # Second category are streams that are in realms without a realm-wide retention policy,
     # but have their own stream-specific policy enabled.
-    query_two = Stream.objects.filter(realm__message_retention_days__isnull=True) \
+    query_two = Stream.objects.filter(realm__message_retention_days=-1) \
         .exclude(message_retention_days__isnull=True) \
         .exclude(message_retention_days=-1) \
         .select_related('realm', 'recipient')

zerver/migrations/0291_realm_retention_days_not_null.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.13 on 2020-06-24 09:24
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('zerver', '0290_remove_night_mode_add_color_scheme'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='realm',
+            name='message_retention_days',
+            field=models.IntegerField(default=-1),
+        ),
+    ]

zerver/models.py

@@ -309,7 +309,7 @@ class Realm(models.Model):
 
     RETAIN_MESSAGE_FOREVER = -1
     # For old messages being automatically deleted
-    message_retention_days: Optional[int] = models.IntegerField(null=True)
+    message_retention_days: int = models.IntegerField(null=False, default=-1)
 
     # When non-null, all but the latest this many messages in the organization
     # are inaccessible to users (but not deleted).

zerver/tests/test_retention.py

@@ -98,7 +98,7 @@ class ArchiveMessagesTestingBase(RetentionTestingBase):
         # control over what's expired and what isn't.
         Message.objects.all().update(date_sent=timezone_now())
 
-    def _set_realm_message_retention_value(self, realm: Realm, retention_period: Optional[int]) -> None:
+    def _set_realm_message_retention_value(self, realm: Realm, retention_period: int) -> None:
         realm.message_retention_days = retention_period
         realm.save()
 
@@ -208,7 +208,7 @@ class TestArchiveMessagesGeneral(ArchiveMessagesTestingBase):
 
     def test_expired_messages_in_one_realm(self) -> None:
         """Test with a retention policy set for only the MIT realm"""
-        self._set_realm_message_retention_value(self.zulip_realm, None)
+        self._set_realm_message_retention_value(self.zulip_realm, -1)
 
         # Make some expired messages in MIT:
         expired_mit_msg_ids = self._make_mit_messages(
@@ -254,13 +254,13 @@ class TestArchiveMessagesGeneral(ArchiveMessagesTestingBase):
         self._verify_archive_data([], [])
 
         # Don't archive if stream and realm have no retention policy:
-        self._set_realm_message_retention_value(self.zulip_realm, None)
+        self._set_realm_message_retention_value(self.zulip_realm, -1)
         self._set_stream_message_retention_value(verona, None)
         archive_messages()
         self._verify_archive_data([], [])
 
         # Archive if stream has a retention policy set:
-        self._set_realm_message_retention_value(self.zulip_realm, None)
+        self._set_realm_message_retention_value(self.zulip_realm, -1)
         self._set_stream_message_retention_value(verona, 1)
         archive_messages()
         self._verify_archive_data([msg_id], usermsg_ids)
@@ -821,7 +821,7 @@ class TestGetRealmAndStreamsForArchiving(ZulipTestCase):
 
         result = []
         for realm in Realm.objects.all():
-            if realm.message_retention_days is not None:
+            if realm.message_retention_days != -1:
                 streams = Stream.objects.filter(realm=realm).exclude(message_retention_days=-1)
                 result.append((realm, list(streams)))
             else:
@@ -845,7 +845,7 @@ class TestGetRealmAndStreamsForArchiving(ZulipTestCase):
         denmark.save()
 
         zephyr_realm = get_realm("zephyr")
-        zephyr_realm.message_retention_days = None
+        zephyr_realm.message_retention_days = -1
         zephyr_realm.save()
         self.make_stream("normal stream", realm=zephyr_realm)
 
@@ -857,7 +857,7 @@ class TestGetRealmAndStreamsForArchiving(ZulipTestCase):
         archiving_enabled_zephyr_stream.message_retention_days = 1
         archiving_enabled_zephyr_stream.save()
 
-        Realm.objects.create(string_id="no_archiving", invite_required=False, message_retention_days=None)
+        Realm.objects.create(string_id="no_archiving", invite_required=False, message_retention_days=-1)
         empty_realm_with_archiving = Realm.objects.create(string_id="with_archiving", invite_required=False,
                                                           message_retention_days=1)