stream: Add entropy to deactivated streams.

Adding an additional `!` to the stream name each time a stream is
deactivated, to a maximum of 21 times, effectively limits number of
times a stream with a given name can be deactivated.  This is unlikely
to come up in common usage, but may be confusing when testing.

Change what we prepend to deactivated stream names to something with
more entropy than just `!`, by instead prepending a substring of hash
of the stream's ID.  `!`s.  Using 128 bits of the hash means that it
will require more than 10^18th renames to have a 1% chance of collision.

Because too-long stream names are also truncated at 60 characters,
having this entropy in the beginning of the name also helps address
potential issues from stream names that differed only in, e.g. the
60th character.

Fixes #17016.

zerver/lib/actions.py

@@ -1,4 +1,5 @@
 import datetime
+import hashlib
 import itertools
 import logging
 import os
@@ -232,7 +233,6 @@ from zerver.models import (
     is_cross_realm_bot_email,
     query_for_ids,
     realm_filters_for_realm,
-    stream_name_in_use,
     validate_attachment_request,
 )
 from zerver.tornado.django_api import send_event
@@ -1237,17 +1237,13 @@ def do_deactivate_stream(
     # special prefix that both indicates that the stream is deactivated and
     # frees up the original name for reuse.
     old_name = stream.name
-    new_name = ("!DEACTIVATED:" + old_name)[: Stream.MAX_NAME_LENGTH]
-    for i in range(20):
-        if stream_name_in_use(new_name, stream.realm_id):
-            # This stream has already been deactivated, keep prepending !s until
-            # we have a unique stream name or you've hit a rename limit.
-            new_name = ("!" + new_name)[: Stream.MAX_NAME_LENGTH]
-        else:
-            break
 
-    # If you don't have a unique name at this point, this will fail later in the
+    # Prepend a substring of the hashed stream ID to the new stream name
-    # code path.
+    streamID = str(stream.id)
+    stream_id_hash_object = hashlib.sha512(streamID.encode("utf-8"))
+    hashed_stream_id = stream_id_hash_object.hexdigest()[0:7]
+
+    new_name = (hashed_stream_id + "!DEACTIVATED:" + old_name)[: Stream.MAX_NAME_LENGTH]
 
     stream.name = new_name[: Stream.MAX_NAME_LENGTH]
     stream.save(update_fields=["name", "deactivated", "invite_only"])

zerver/models.py

@@ -1863,13 +1863,6 @@ def get_realm_stream(stream_name: str, realm_id: int) -> Stream:
     return Stream.objects.select_related().get(name__iexact=stream_name.strip(), realm_id=realm_id)
 
 
-def stream_name_in_use(stream_name: str, realm_id: int) -> bool:
-    return Stream.objects.filter(
-        name__iexact=stream_name.strip(),
-        realm_id=realm_id,
-    ).exists()
-
-
 def get_active_streams(realm: Optional[Realm]) -> QuerySet:
     # TODO: Change return type to QuerySet[Stream]
     # NOTE: Return value is used as a QuerySet, so cannot currently be Sequence[QuerySet]

zerver/tests/test_subs.py

@@ -1,3 +1,4 @@
+import hashlib
 import random
 from datetime import timedelta
 from typing import Any, Dict, List, Mapping, Optional, Sequence, Set, Union
@@ -1330,7 +1331,8 @@ class StreamAdminTest(ZulipTestCase):
 
         # Simulate that a stream by the same name has already been
         # deactivated, just to exercise our renaming logic:
-        ensure_stream(realm, "!DEACTIVATED:" + active_name)
+        # Since we do not know the id of these simulated stream we prepend the name with a random hashed_stream_id
+        ensure_stream(realm, "DB32B77" + "!DEACTIVATED:" + active_name)
 
         events: List[Mapping[str, Any]] = []
         with tornado_redirected_to_list(events):
@@ -1352,7 +1354,8 @@ class StreamAdminTest(ZulipTestCase):
 
         # A deleted stream's name is changed, is deactivated, is invite-only,
         # and has no subscribers.
-        deactivated_stream_name = "!!DEACTIVATED:" + active_name
+        hashed_stream_id = hashlib.sha512(str(stream_id).encode("utf-8")).hexdigest()[0:7]
+        deactivated_stream_name = hashed_stream_id + "!DEACTIVATED:" + active_name
         deactivated_stream = get_stream(deactivated_stream_name, realm)
         self.assertTrue(deactivated_stream.deactivated)
         self.assertTrue(deactivated_stream.invite_only)