avatar: Add checks to make sure system bot avatar exists.

This commit introduces an assertion to verify that the avatar file for
system bots exists and findable.

In development, it'll assert that the avatar file exists in the static
directory. This isn't done in production environment to avoid
unnecessary overhead. It helps verify that the protocol to fetch system
bot avatars still works when making changes during development.

In production it'll check if the avatar file exists in the STATIC_ROOT
and return a default avatar png if it doesn't.
This commit is contained in:
PieterCK 2024-10-23 14:08:44 +07:00 committed by Tim Abbott
parent 516d1ab82b
commit 068ab6e11e
2 changed files with 34 additions and 5 deletions

View File

@ -46,6 +46,20 @@ def get_static_avatar_url(email: str, medium: bool) -> str:
avatar_file_name = get_system_bots_avatar_file_name(email)
avatar_file_name += "-medium.png" if medium else ".png"
if settings.DEBUG:
# This find call may not be cheap, so we only do it in the
# development environment to do an assertion.
from django.contrib.staticfiles.finders import find
if not find(avatar_file_name):
raise AssertionError(f"Unknown avatar file for: {email}")
elif settings.STATIC_ROOT and not staticfiles_storage.exists(avatar_file_name):
# Fallback for the case where no avatar exists; this should
# never happen in practice. This logic cannot be executed
# while STATIC_ROOT is not defined, so the above STATIC_ROOT
# check is important.
return DEFAULT_AVATAR_FILE
return staticfiles_storage.url(avatar_file_name)

View File

@ -26,7 +26,12 @@ from zerver.actions.realm_logo import do_change_logo_source
from zerver.actions.realm_settings import do_change_realm_plan_type, do_set_realm_property
from zerver.actions.user_settings import do_delete_avatar_image
from zerver.lib.attachments import validate_attachment_request
from zerver.lib.avatar import avatar_url, get_avatar_field
from zerver.lib.avatar import (
DEFAULT_AVATAR_FILE,
avatar_url,
get_avatar_field,
get_static_avatar_url,
)
from zerver.lib.cache import cache_delete, cache_get, get_realm_used_upload_space_cache_key
from zerver.lib.create_user import copy_default_settings
from zerver.lib.initial_password import initial_password
@ -1540,20 +1545,30 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase):
settings.EMAIL_GATEWAY_BOT,
]
internal_realm = get_realm(settings.SYSTEM_BOT_REALM)
default_avatar = DEFAULT_AVATAR_FILE
for email in system_bot_emails:
system_bot = get_system_bot(email, internal_realm.id)
response = self.client_get(f"/avatar/{email}")
redirect_url = response["Location"]
self.assertEqual(redirect_url, str(avatar_url(system_bot)))
self.assertTrue(str(redirect_url).endswith(".png"))
self.assertFalse(str(redirect_url).endswith("unknown.png"))
response = self.client_get(f"/avatar/{email}/medium")
redirect_url = response["Location"]
self.assertEqual(redirect_url, str(avatar_url(system_bot, medium=True)))
self.assertTrue(str(redirect_url).endswith("-medium.png"))
self.assertFalse(str(redirect_url).endswith("unknown-medium.png"))
with (
self.settings(STATIC_ROOT="static/"),
patch("zerver.lib.avatar.staticfiles_storage.exists") as mock_exists,
):
mock_exists.return_value = False
static_avatar_url = get_static_avatar_url("false-bot@zulip.com", False)
self.assertIn(default_avatar, static_avatar_url)
with self.settings(DEBUG=True), self.assertRaises(AssertionError) as e:
get_static_avatar_url("false-bot@zulip.com", False)
expected_error_message = "Unknown avatar file for: false-bot@zulip.com"
self.assertEqual(str(e.exception), expected_error_message)
class RealmIconTest(UploadSerializeMixin, ZulipTestCase):