From c3017e55d2ae261ebd770162ba15f43f66fa30c1 Mon Sep 17 00:00:00 2001
From: PieterCK <pieterceka123@gmail.com>
Date: Thu, 24 Oct 2024 14:23:34 +0700
Subject: [PATCH] storage: Rework static avatar files hashing logic.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, the hashing logic for static avatar files hashed the default
and medium files separately, which didn’t match how user-uploaded
avatars work—where you just add the "-medium.png" suffix to get the
medium version. Since we don’t have clear documentation for avatars yet,
this caused some issues for the mobile apps.

This commit makes sure the default and its medium variation share the
same hash.
---
 zerver/lib/storage.py | 79 ++++++++++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 23 deletions(-)

diff --git a/zerver/lib/storage.py b/zerver/lib/storage.py
index 0465ae69a8..8ca7f66e1f 100644
--- a/zerver/lib/storage.py
+++ b/zerver/lib/storage.py
@@ -24,26 +24,62 @@ else:
         return os.path.join(settings.STATIC_ROOT, path)
 
 
-def reformat_medium_filename(hashed_name: str) -> str:
-    """
-    Because the protocol for getting medium-size avatar URLs
-    was never fully documented, the mobile apps use a
-    substitution of the form s/.png/-medium.png/ to get the
-    medium-size avatar URLs. Thus, we must ensure the hashed
-    filenames for system bot avatars follow this naming convention.
-    """
-
-    name_parts = hashed_name.rsplit(".", 1)
-    base_name = name_parts[0]
-
-    if len(name_parts) != 2 or "-medium" not in base_name:
-        return hashed_name
-    extension = name_parts[1].replace("png", "medium.png")
-    base_name = base_name.replace("-medium", "")
-    return f"{base_name}-{extension}"
-
-
 class IgnoreBundlesManifestStaticFilesStorage(ManifestStaticFilesStorage):
+    hashed_static_avatar_file_map: dict[str, str] = {}
+
+    def process_static_avatars_name(
+        self,
+        name: str,
+        content: Optional["File[bytes]"] = None,
+        filename: str | None = None,
+    ) -> str:
+        """
+        Because the protocol for getting medium-size avatar URLs
+        was never fully documented, the mobile apps use a
+        substitution of the form s/.png/-medium.png/ to get the
+        medium-size avatar URLs.
+
+        This function hashes system bots' avatar files in a way
+        that follows the pattern used for user-uploaded avatars.
+
+        It ensures the following:
+
+            * Hashed filenames for system bot avatars follow this
+            naming convention:
+            - avatar.png -> avatar-medium.png
+
+            * The system bots' default avatar file and its medium
+            version share the same hash:
+            - bot.36f721bad3d0.png -> bot.36f721bad3d0-medium.png
+        """
+
+        def reformat_medium_filename(hashed_name: str) -> str:
+            name_parts = hashed_name.rsplit(".", 1)
+            base_name = name_parts[0]
+
+            if len(name_parts) != 2 or "-medium" not in base_name:
+                return hashed_name
+            extension = name_parts[1].replace("png", "medium.png")
+            base_name = base_name.replace("-medium", "")
+            return f"{base_name}-{extension}"
+
+        if name.endswith("-medium.png"):
+            # This logic relies on the fact that the medium files will
+            # be hashed first due to the "-medium" string, which places
+            # them earlier in the naming order. So, adhering to the
+            # medium file naming convention is crucial for this logic.
+            hashed_medium_file: str = reformat_medium_filename(
+                super().hashed_name(name, content, filename)
+            )
+
+            default_file = name.replace("-medium.png", ".png")
+            hashed_default_file = hashed_medium_file.replace("-medium.png", ".png")
+
+            self.hashed_static_avatar_file_map[default_file] = hashed_default_file
+            return hashed_medium_file
+        assert name in self.hashed_static_avatar_file_map
+        return self.hashed_static_avatar_file_map[name]
+
     @override
     def hashed_name(
         self, name: str, content: Optional["File[bytes]"] = None, filename: str | None = None
@@ -64,11 +100,8 @@ class IgnoreBundlesManifestStaticFilesStorage(ManifestStaticFilesStorage):
             # so they can hit our Nginx caching block for static files.
             # We don't need to worry about stale caches since these are
             # only used by the system bots.
-            if not name.endswith("-medium.png"):
-                return super().hashed_name(name, content, filename)
+            return self.process_static_avatars_name(name, content, filename)
 
-            hashed_medium_file = super().hashed_name(name, content, filename)
-            return reformat_medium_filename(hashed_medium_file)
         if name == "generated/emoji/emoji_api.json":
             # Unlike most .json files, we do want to hash this file;
             # its hashed URL is returned as part of the API.  See