zilencer: Handle deleted realms nicely at server/analytics.

zerver/models.py

@@ -4861,6 +4861,7 @@ class AbstractRealmAuditLog(models.Model):
     # via send_server_data_to_push_bouncer.
     REMOTE_REALM_VALUE_UPDATED = 20001
     REMOTE_PLAN_TRANSFERRED_SERVER_TO_REALM = 20002
+    REMOTE_REALM_LOCALLY_DELETED = 20003
 
     event_type = models.PositiveSmallIntegerField()
 

zerver/tests/test_push_notifications.py

@@ -1,5 +1,6 @@
 import asyncio
 import base64
+import logging
 import uuid
 from contextlib import contextmanager
 from datetime import datetime, timedelta, timezone
@@ -1998,19 +1999,25 @@ class AnalyticsBouncerTest(BouncerTestCase):
 
     @override_settings(PUSH_NOTIFICATION_BOUNCER_URL="https://push.zulip.org.example.com")
     @responses.activate
-    def test_non_existent_realm_uuid(self) -> None:
+    def test_deleted_realm(self) -> None:
         self.add_mock_response()
+        logger = logging.getLogger("zulip.analytics")
+
         realm_info = get_realms_info_for_push_bouncer()
 
         # Hard-delete a realm to test the non existent realm uuid case.
-        realm = Realm.objects.order_by("id").first()
+        realm = get_realm("zephyr")
         assert realm is not None
         deleted_realm_uuid = realm.uuid
         realm.delete()
 
+        # This mock causes us to still send data to the bouncer as if the realm existed,
+        # causing the bouncer to include its corresponding info in the response. Through
+        # that, we're testing our graceful handling of seeing a non-existent realm uuid
+        # in that response.
         with mock.patch(
             "zerver.lib.remote_server.get_realms_info_for_push_bouncer", return_value=realm_info
-        ) as m, self.assertLogs("zulip.analytics", level="WARNING") as analytics_logger:
+        ) as m, self.assertLogs(logger, level="WARNING") as analytics_logger:
             send_server_data_to_push_bouncer(consider_usage_statistics=False)
             m.assert_called()
             realms = Realm.objects.all()
@@ -2026,6 +2033,26 @@ class AnalyticsBouncerTest(BouncerTestCase):
             ],
         )
 
+        # Now we want to test the other side of this - bouncer's handling
+        # of a deleted realm.
+        with self.assertLogs(logger, level="WARNING") as analytics_logger:
+            # This time the logger shouldn't get triggered - because the bouncer doesn't
+            # include .realm_locally_deleted realms in its response.
+            # Note: This is hacky, because until Python 3.10 we don't have access to
+            # assertNoLogs - and regular assertLogs demands that the logger gets triggered.
+            # So we do a dummy warning ourselves here, to satisfy it.
+            # TODO: Replace this with assertNoLogs once we fully upgrade to Python 3.10.
+            logger.warning("Dummy warning")
+            send_server_data_to_push_bouncer(consider_usage_statistics=False)
+        remote_realm_for_deleted_realm = RemoteRealm.objects.get(uuid=deleted_realm_uuid)
+        self.assertEqual(remote_realm_for_deleted_realm.registration_deactivated, True)
+        self.assertEqual(remote_realm_for_deleted_realm.realm_locally_deleted, True)
+        self.assertEqual(analytics_logger.output, ["WARNING:zulip.analytics:Dummy warning"])
+
+        audit_log = RemoteRealmAuditLog.objects.latest("id")
+        self.assertEqual(audit_log.event_type, RemoteRealmAuditLog.REMOTE_REALM_LOCALLY_DELETED)
+        self.assertEqual(audit_log.remote_realm, remote_realm_for_deleted_realm)
+
 
 class PushNotificationTest(BouncerTestCase):
     @override

zilencer/migrations/0056_remoterealm_realm_locally_deleted.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.8 on 2023-12-15 13:52
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("zilencer", "0055_remoteserverbillinguser_tos_version"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="remoterealm",
+            name="realm_locally_deleted",
+            field=models.BooleanField(default=False),
+        ),
+    ]

zilencer/models.py

@@ -147,6 +147,9 @@ class RemoteRealm(models.Model):
     registration_deactivated = models.BooleanField(default=False)
     # Whether the realm has been deactivated on the remote server.
     realm_deactivated = models.BooleanField(default=False)
+    # Whether we believe the remote server deleted this realm
+    # from the database.
+    realm_locally_deleted = models.BooleanField(default=False)
 
     # When the realm was created on the remote server.
     realm_date_created = models.DateTimeField()

zilencer/views.py

@@ -654,7 +654,21 @@ def update_remote_realm_data_for_server(
     server: RemoteZulipServer, server_realms_info: List[RealmDataForAnalytics]
 ) -> None:
     uuids = [realm.uuid for realm in server_realms_info]
-    already_registered_remote_realms = RemoteRealm.objects.filter(uuid__in=uuids, server=server)
+    all_registered_remote_realms_for_server = list(RemoteRealm.objects.filter(server=server))
+    already_registered_remote_realms = [
+        remote_realm
+        for remote_realm in all_registered_remote_realms_for_server
+        if remote_realm.uuid in uuids
+    ]
+    # RemoteRealm registrations that we have for this server, but aren't
+    # present in the data sent to us. We assume this to mean the server
+    # must have deleted those realms from the database.
+    remote_realms_missing_from_server_data = [
+        remote_realm
+        for remote_realm in all_registered_remote_realms_for_server
+        if remote_realm.uuid not in uuids
+    ]
+
     already_registered_uuids = {
         remote_realm.uuid for remote_realm in already_registered_remote_realms
     }
@@ -689,6 +703,10 @@ def update_remote_realm_data_for_server(
     # Update RemoteRealm entries, for which the corresponding realm's info has changed
     # (for the attributes that make sense to sync like this).
     for remote_realm in already_registered_remote_realms:
+        # TODO: We'll also want to check if .realm_locally_deleted is True, and if so,
+        # toggle it off (and potentially restore registration_deactivated=True too),
+        # since the server is now sending us data for this realm again.
+
         modified = False
         realm = uuid_to_realm_dict[str(remote_realm.uuid)]
         for remote_realm_attr, realm_dict_key in [
@@ -739,6 +757,32 @@ def update_remote_realm_data_for_server(
     )
     RemoteRealmAuditLog.objects.bulk_create(remote_realm_audit_logs)
 
+    remote_realms_to_update = []
+    remote_realm_audit_logs = []
+    for remote_realm in remote_realms_missing_from_server_data:
+        if not remote_realm.realm_locally_deleted:
+            # Otherwise we already knew about this, so nothing to do.
+            remote_realm.realm_locally_deleted = True
+            remote_realm.registration_deactivated = True
+
+            remote_realm_audit_logs.append(
+                RemoteRealmAuditLog(
+                    server=server,
+                    remote_id=None,
+                    remote_realm=remote_realm,
+                    realm_id=None,
+                    event_type=RemoteRealmAuditLog.REMOTE_REALM_LOCALLY_DELETED,
+                    event_time=now,
+                )
+            )
+            remote_realms_to_update.append(remote_realm)
+
+    RemoteRealm.objects.bulk_update(
+        remote_realms_to_update,
+        ["realm_locally_deleted", "registration_deactivated"],
+    )
+    RemoteRealmAuditLog.objects.bulk_create(remote_realm_audit_logs)
+
 
 def get_human_user_realm_uuids(realms: List[RealmDataForAnalytics]) -> List[UUID]:  # nocoverage
     billable_realm_uuids = []
@@ -998,7 +1042,7 @@ def remote_server_post_analytics(
             remote_server_billing_session.sync_license_ledger_if_needed()
 
     remote_realm_dict: Dict[str, RemoteRealmDictValue] = {}
-    remote_realms = RemoteRealm.objects.filter(server=server)
+    remote_realms = RemoteRealm.objects.filter(server=server, realm_locally_deleted=False)
     for remote_realm in remote_realms:
         uuid = str(remote_realm.uuid)
         billing_session = RemoteRealmBillingSession(remote_realm)