migration: Clear old data for unused usermessage flags.

In c37871ac3a, we renamed the
two unused and historical bits of the 'flags' bitfield of
the 'UserMessage' table:

* 'summarize_in_home' to 'topic_wildcard_mentioned'
* 'summarize_in_stream' to 'group_mentioned'

This commit clears out the old data for those bits.

Additionally, we are clearing 'force_expand' and 'force_collapse'
unused flags to save future work.

zerver/migrations/0486_clear_old_data_for_unused_usermessage_flags.py

@@ -0,0 +1,72 @@
+from django.contrib.postgres.operations import AddIndexConcurrently, RemoveIndexConcurrently
+from django.db import connection, migrations, models
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+from psycopg2.sql import SQL
+
+
+def clear_old_data_for_unused_usermessage_flags(
+    apps: StateApps, schema_editor: BaseDatabaseSchemaEditor
+) -> None:
+    """Because 'topic_wildcard_mentioned' and 'group_mentioned' flags are
+    reused flag slots (ref: c37871a) in the 'flags' bitfield, we're not
+    confident that their value is in 0 state on very old servers, and this
+    migration is to ensure that's the case.
+    Additionally, we are clearing 'force_expand' and 'force_collapse' unused
+    flags to save future work.
+    """
+    with connection.cursor() as cursor:
+        cursor.execute(SQL("SELECT MAX(id) FROM zerver_usermessage WHERE flags & 480 <> 0;"))
+        (max_id,) = cursor.fetchone()
+
+    # nothing to update
+    if not max_id:
+        return
+
+    BATCH_SIZE = 5000
+    lower_id_bound = 0
+    while lower_id_bound < max_id:
+        upper_id_bound = min(lower_id_bound + BATCH_SIZE, max_id)
+        with connection.cursor() as cursor:
+            query = SQL(
+                """
+                    UPDATE zerver_usermessage
+                    SET flags = (flags & ~(1 << 5) & ~(1 << 6) & ~(1 << 7) & ~(1 << 8))
+                    WHERE flags & 480 <> 0
+                    AND id > %(lower_id_bound)s AND id <= %(upper_id_bound)s;
+            """
+            )
+            cursor.execute(
+                query,
+                {"lower_id_bound": lower_id_bound, "upper_id_bound": upper_id_bound},
+            )
+
+        print(f"Processed {upper_id_bound} / {max_id}")
+        lower_id_bound = lower_id_bound + BATCH_SIZE
+
+
+class Migration(migrations.Migration):
+    atomic = False
+    dependencies = [
+        ("zerver", "0485_alter_usermessage_flags_and_add_index"),
+    ]
+
+    operations = [
+        AddIndexConcurrently(
+            model_name="usermessage",
+            index=models.Index(
+                "id",
+                condition=models.Q(("flags__andnz", 480)),
+                name="zerver_usermessage_temp_clear_flags",
+            ),
+        ),
+        migrations.RunPython(
+            clear_old_data_for_unused_usermessage_flags,
+            reverse_code=migrations.RunPython.noop,
+            elidable=True,
+        ),
+        RemoveIndexConcurrently(
+            model_name="usermessage",
+            name="zerver_usermessage_temp_clear_flags",
+        ),
+    ]

zerver/models.py

@@ -3489,6 +3489,7 @@ class AbstractUserMessage(models.Model):
         "topic_wildcard_mentioned",
         "group_mentioned",
         # These next 2 flags are from features that have since been removed.
+        # We've cleared these 2 flags in migration 0486.
         "force_expand",
         "force_collapse",
         # Whether the message contains any of the user's alert words.

zerver/tests/test_migrations.py

@@ -4,10 +4,9 @@
 # You can also read
 #   https://www.caktusgroup.com/blog/2016/02/02/writing-unit-tests-django-migrations/
 # to get a tutorial on the framework that inspired this feature.
-from typing import Optional
+from unittest.mock import patch
 
 from django.db.migrations.state import StateApps
-from django.utils.timezone import now
 from typing_extensions import override
 
 from zerver.lib.test_classes import MigrationsTestCase
@@ -25,51 +24,53 @@ from zerver.lib.test_classes import MigrationsTestCase
 #   "zerver_subscription" because it has pending trigger events
 
 
-class PushBouncerBackfillIosAppId(MigrationsTestCase):
+class UserMessageIndex(MigrationsTestCase):
-    @property
+    migrate_from = "0485_alter_usermessage_flags_and_add_index"
-    @override
+    migrate_to = "0486_clear_old_data_for_unused_usermessage_flags"
-    def app(self) -> str:
-        return "zilencer"
 
-    migrate_from = "0031_alter_remoteinstallationcount_remote_id_and_more"
+    @override
-    migrate_to = "0032_remotepushdevicetoken_backfill_ios_app_id"
+    def setUp(self) -> None:
+        with patch("builtins.print") as _:
+            super().setUp()
 
     @override
     def setUpBeforeMigration(self, apps: StateApps) -> None:
-        user = self.example_user("hamlet")
+        UserMessage = apps.get_model("zerver", "usermessage")
 
-        RemoteZulipServer = apps.get_model("zilencer", "RemoteZulipServer")
+        um_1 = UserMessage.objects.get(id=1)
-        server = RemoteZulipServer.objects.create(
+        um_1.flags.topic_wildcard_mentioned = True
-            uuid="6cde5f7a-1f7e-4978-9716-49f69ebfc9fe",
+        um_1.flags.wildcard_mentioned = True
-            api_key="secret",
+        um_1.flags.force_expand = True
-            hostname="chat.example",
+        um_1.save()
-            last_updated=now(),
-        )
 
-        RemotePushDeviceToken = apps.get_model("zilencer", "RemotePushDeviceToken")
+        um_2 = UserMessage.objects.get(id=2)
+        um_2.flags.group_mentioned = True
+        um_2.flags.topic_wildcard_mentioned = True
+        um_2.flags.mentioned = True
+        um_2.flags.force_collapse = True
+        um_2.save()
 
-        def create(kind: int, token: str, ios_app_id: Optional[str]) -> None:
+        um_1 = UserMessage.objects.get(id=1)
-            RemotePushDeviceToken.objects.create(
+        um_2 = UserMessage.objects.get(id=2)
-                server=server,
-                user_uuid=user.uuid,
-                kind=kind,
-                token=token,
-                ios_app_id=ios_app_id,
-            )
 
-        kinds = {choice[1]: choice[0] for choice in RemotePushDeviceToken.kind.field.choices}
+        self.assertTrue(um_1.flags.topic_wildcard_mentioned)
-        create(kinds["apns"], "1234", None)
+        self.assertTrue(um_1.flags.wildcard_mentioned)
-        create(kinds["apns"], "2345", "example.app")
+        self.assertTrue(um_1.flags.force_expand)
-        create(kinds["gcm"], "3456", None)
+        self.assertTrue(um_2.flags.group_mentioned)
+        self.assertTrue(um_2.flags.topic_wildcard_mentioned)
+        self.assertTrue(um_2.flags.mentioned)
+        self.assertTrue(um_2.flags.force_collapse)
 
-    @override
+    def test_clear_topic_wildcard_and_group_mentioned_flags(self) -> None:
-    def tearDown(self) -> None:
+        UserMessage = self.apps.get_model("zerver", "usermessage")
-        RemotePushDeviceToken = self.apps.get_model("zilencer", "RemotePushDeviceToken")
-        RemotePushDeviceToken.objects.all().delete()
 
-    def test_worked(self) -> None:
+        um_1 = UserMessage.objects.get(id=1)
-        RemotePushDeviceToken = self.apps.get_model("zilencer", "RemotePushDeviceToken")
+        um_2 = UserMessage.objects.get(id=2)
-        self.assertEqual(
+
-            dict(RemotePushDeviceToken.objects.values_list("token", "ios_app_id")),
+        self.assertFalse(um_1.flags.topic_wildcard_mentioned)
-            {"1234": "org.zulip.Zulip", "2345": "example.app", "3456": None},
+        self.assertTrue(um_1.flags.wildcard_mentioned)
-        )
+        self.assertFalse(um_1.flags.force_expand)
+        self.assertFalse(um_2.flags.group_mentioned)
+        self.assertFalse(um_2.flags.topic_wildcard_mentioned)
+        self.assertTrue(um_2.flags.mentioned)
+        self.assertFalse(um_2.flags.force_collapse)