uploads: Add LOCAL_AVATARS_DIR / LOCAL_FILES_DIR computed settings.

This avoids strewing "avatars" and "files" constants throughout.

zerver/lib/export.py

@@ -1409,23 +1409,25 @@ def export_uploads_and_avatars(
         realm_emojis = list(RealmEmoji.objects.filter(author_id=user.id))
 
     if settings.LOCAL_UPLOADS_DIR:
+        assert settings.LOCAL_FILES_DIR
+        assert settings.LOCAL_AVATARS_DIR
         # Small installations and developers will usually just store files locally.
         export_uploads_from_local(
             realm,
-            local_dir=os.path.join(settings.LOCAL_UPLOADS_DIR, "files"),
+            local_dir=settings.LOCAL_FILES_DIR,
             output_dir=uploads_output_dir,
             attachments=attachments,
         )
         export_avatars_from_local(
             realm,
-            local_dir=os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars"),
+            local_dir=settings.LOCAL_AVATARS_DIR,
             output_dir=avatars_output_dir,
             users=users,
             handle_system_bots=handle_system_bots,
         )
         export_emoji_from_local(
             realm,
-            local_dir=os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars"),
+            local_dir=settings.LOCAL_AVATARS_DIR,
             output_dir=emoji_output_dir,
             realm_emojis=realm_emojis,
         )
@@ -1433,7 +1435,7 @@ def export_uploads_and_avatars(
         if user is None:
             export_realm_icons(
                 realm,
-                local_dir=os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars"),
+                local_dir=settings.LOCAL_AVATARS_DIR,
                 output_dir=realm_icons_output_dir,
             )
     else:

zerver/lib/import_realm.py

@@ -725,11 +725,9 @@ def process_avatars(record: Dict[str, Any]) -> None:
 
     if record["s3_path"].endswith(".original"):
         user_profile = get_user_profile_by_id(record["user_profile_id"])
-        if settings.LOCAL_UPLOADS_DIR is not None:
+        if settings.LOCAL_AVATARS_DIR is not None:
             avatar_path = user_avatar_path_from_ids(user_profile.id, record["realm_id"])
-            medium_file_path = (
+            medium_file_path = os.path.join(settings.LOCAL_AVATARS_DIR, avatar_path) + "-medium.png"
-                os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", avatar_path) + "-medium.png"
-            )
             if os.path.exists(medium_file_path):
                 # We remove the image here primarily to deal with
                 # issues when running the import script multiple
@@ -875,10 +873,12 @@ def import_uploads(
             )
         else:
             assert settings.LOCAL_UPLOADS_DIR is not None
+            assert settings.LOCAL_AVATARS_DIR is not None
+            assert settings.LOCAL_FILES_DIR is not None
             if processing_avatars or processing_emojis or processing_realm_icons:
-                file_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", relative_path)
+                file_path = os.path.join(settings.LOCAL_AVATARS_DIR, relative_path)
             else:
-                file_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "files", relative_path)
+                file_path = os.path.join(settings.LOCAL_FILES_DIR, relative_path)
             orig_file_path = os.path.join(import_dir, record["path"])
             os.makedirs(os.path.dirname(file_path), exist_ok=True)
             shutil.copy(orig_file_path, file_path)

zerver/lib/test_helpers.py

@@ -225,9 +225,9 @@ def avatar_disk_path(
     avatar_url_path = avatar_url(user_profile, medium)
     assert avatar_url_path is not None
     assert settings.LOCAL_UPLOADS_DIR is not None
+    assert settings.LOCAL_AVATARS_DIR is not None
     avatar_disk_path = os.path.join(
-        settings.LOCAL_UPLOADS_DIR,
+        settings.LOCAL_AVATARS_DIR,
-        "avatars",
         avatar_url_path.split("/")[-2],
         avatar_url_path.split("/")[-1].split("?")[0],
     )
@@ -549,6 +549,8 @@ FuncT = TypeVar("FuncT", bound=Callable[..., None])
 def use_s3_backend(method: FuncT) -> FuncT:
     @mock_s3
     @override_settings(LOCAL_UPLOADS_DIR=None)
+    @override_settings(LOCAL_AVATARS_DIR=None)
+    @override_settings(LOCAL_FILES_DIR=None)
     def new_method(*args: Any, **kwargs: Any) -> Any:
         zerver.lib.upload.upload_backend = S3UploadBackend()
         try:

zerver/lib/test_runner.py

@@ -212,9 +212,9 @@ def init_worker(
     from zproject.dev_urls import avatars_url
 
     assert settings.LOCAL_UPLOADS_DIR is not None
+    assert settings.LOCAL_AVATARS_DIR is not None
     assert avatars_url.default_args is not None
-    new_root = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars")
+    avatars_url.default_args["document_root"] = settings.LOCAL_AVATARS_DIR
-    avatars_url.default_args["document_root"] = new_root
 
 
 class ParallelTestSuite(django_runner.ParallelTestSuite):
@@ -248,6 +248,8 @@ def initialize_worker_path(worker_id: int) -> None:
             "test_uploads",
         )
     )
+    settings.LOCAL_AVATARS_DIR = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars")
+    settings.LOCAL_FILES_DIR = os.path.join(settings.LOCAL_UPLOADS_DIR, "files")
 
 
 class Runner(DiscoverRunner):

zerver/lib/transfer.py

@@ -25,7 +25,8 @@ def transfer_uploads_to_s3(processes: int) -> None:
 def _transfer_avatar_to_s3(user: UserProfile) -> None:
     avatar_path = user_avatar_path(user)
     assert settings.LOCAL_UPLOADS_DIR is not None
-    file_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", avatar_path) + ".original"
+    assert settings.LOCAL_AVATARS_DIR is not None
+    file_path = os.path.join(settings.LOCAL_AVATARS_DIR, avatar_path) + ".original"
     try:
         with open(file_path, "rb") as f:
             s3backend.upload_avatar_image(f, user, user)
@@ -53,7 +54,8 @@ def transfer_avatars_to_s3(processes: int) -> None:
 
 def _transfer_message_files_to_s3(attachment: Attachment) -> None:
     assert settings.LOCAL_UPLOADS_DIR is not None
-    file_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "files", attachment.path_id)
+    assert settings.LOCAL_FILES_DIR is not None
+    file_path = os.path.join(settings.LOCAL_FILES_DIR, attachment.path_id)
     try:
         with open(file_path, "rb") as f:
             guessed_type = guess_type(attachment.file_name)[0]
@@ -95,7 +97,8 @@ def _transfer_emoji_to_s3(realm_emoji: RealmEmoji) -> None:
         emoji_file_name=realm_emoji.file_name,
     )
     assert settings.LOCAL_UPLOADS_DIR is not None
-    emoji_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", emoji_path) + ".original"
+    assert settings.LOCAL_AVATARS_DIR is not None
+    emoji_path = os.path.join(settings.LOCAL_AVATARS_DIR, emoji_path) + ".original"
     try:
         with open(emoji_path, "rb") as f:
             s3backend.upload_emoji_image(f, realm_emoji.file_name, realm_emoji.author)

zerver/lib/upload/local.py

@@ -69,7 +69,7 @@ def delete_local_file(type: Literal["avatars", "files"], path: str) -> bool:
 
 
 def get_local_file_path(path_id: str) -> Optional[str]:
-    local_path = os.path.join(assert_is_not_none(settings.LOCAL_UPLOADS_DIR), "files", path_id)
+    local_path = os.path.join(assert_is_not_none(settings.LOCAL_FILES_DIR), path_id)
     assert_is_local_storage_path("files", local_path)
 
     if os.path.isfile(local_path):
@@ -214,15 +214,15 @@ class LocalUploadBackend(ZulipUploadBackend):
         file_path = user_avatar_path(user_profile)
 
         output_path = os.path.join(
-            assert_is_not_none(settings.LOCAL_UPLOADS_DIR),
+            assert_is_not_none(settings.LOCAL_AVATARS_DIR),
-            "avatars",
             file_path + file_extension,
         )
         if os.path.isfile(output_path):
             return
 
         image_path = os.path.join(
-            assert_is_not_none(settings.LOCAL_UPLOADS_DIR), "avatars", file_path + ".original"
+            assert_is_not_none(settings.LOCAL_AVATARS_DIR),
+            file_path + ".original",
         )
         with open(image_path, "rb") as f:
             image_data = f.read()
@@ -282,7 +282,7 @@ class LocalUploadBackend(ZulipUploadBackend):
             secrets.token_urlsafe(18),
             os.path.basename(tarball_path),
         )
-        abs_path = os.path.join(assert_is_not_none(settings.LOCAL_UPLOADS_DIR), "avatars", path)
+        abs_path = os.path.join(assert_is_not_none(settings.LOCAL_AVATARS_DIR), path)
         os.makedirs(os.path.dirname(abs_path), exist_ok=True)
         shutil.copy(tarball_path, abs_path)
         public_url = realm.uri + "/user_avatars/" + path

zerver/migrations/0149_realm_emoji_drop_unique_constraint.py

@@ -51,9 +51,11 @@ class LocalUploader(Uploader):
 
     def copy_files(self, src_path: str, dst_path: str) -> None:
         assert settings.LOCAL_UPLOADS_DIR is not None
-        src_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", src_path)
+        assert settings.LOCAL_AVATARS_DIR is not None
+        assert settings.LOCAL_FILES_DIR is not None
+        src_path = os.path.join(settings.LOCAL_AVATARS_DIR, src_path)
         self.mkdirs(src_path)
-        dst_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", dst_path)
+        dst_path = os.path.join(settings.LOCAL_AVATARS_DIR, dst_path)
         self.mkdirs(dst_path)
         shutil.copyfile(src_path, dst_path)
 

zerver/tests/test_import_export.py

@@ -1329,9 +1329,10 @@ class RealmImportExportTest(ExportFile):
         self.assert_length(b"zulip!", uploaded_file.size)
 
         assert settings.LOCAL_UPLOADS_DIR is not None
-        attachment_file_path = os.path.join(
+        assert settings.LOCAL_FILES_DIR is not None
-            settings.LOCAL_UPLOADS_DIR, "files", uploaded_file.path_id
+        assert settings.LOCAL_AVATARS_DIR is not None
-        )
+
+        attachment_file_path = os.path.join(settings.LOCAL_FILES_DIR, uploaded_file.path_id)
         self.assertTrue(os.path.isfile(attachment_file_path))
 
         # Test emojis
@@ -1340,18 +1341,18 @@ class RealmImportExportTest(ExportFile):
             realm_id=imported_realm.id,
             emoji_file_name=realm_emoji.file_name,
         )
-        emoji_file_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", emoji_path)
+        emoji_file_path = os.path.join(settings.LOCAL_AVATARS_DIR, emoji_path)
         self.assertTrue(os.path.isfile(emoji_file_path))
 
         # Test avatars
         user_profile = UserProfile.objects.get(full_name=user.full_name, realm=imported_realm)
         avatar_path_id = user_avatar_path(user_profile) + ".original"
-        avatar_file_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", avatar_path_id)
+        avatar_file_path = os.path.join(settings.LOCAL_AVATARS_DIR, avatar_path_id)
         self.assertTrue(os.path.isfile(avatar_file_path))
 
         # Test realm icon and logo
         upload_path = upload.upload_backend.realm_avatar_and_logo_path(imported_realm)
-        full_upload_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", upload_path)
+        full_upload_path = os.path.join(settings.LOCAL_AVATARS_DIR, upload_path)
 
         test_image_data = read_test_image_file("img.png")
         self.assertIsNotNone(test_image_data)

zerver/tests/test_upload.py

@@ -1819,7 +1819,8 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase):
         self.assertEqual(base, uri[: len(base)])
         path_id = re.sub("/user_uploads/", "", uri)
         assert settings.LOCAL_UPLOADS_DIR is not None
-        file_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "files", path_id)
+        assert settings.LOCAL_FILES_DIR is not None
+        file_path = os.path.join(settings.LOCAL_FILES_DIR, path_id)
         self.assertTrue(os.path.isfile(file_path))
 
         uploaded_file = Attachment.objects.get(owner=user_profile, path_id=path_id)
@@ -1859,19 +1860,20 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase):
         write_local_file("avatars", file_path + ".original", read_test_image_file("img.png"))
 
         assert settings.LOCAL_UPLOADS_DIR is not None
-        image_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", file_path + ".original")
+        assert settings.LOCAL_AVATARS_DIR is not None
+        image_path = os.path.join(settings.LOCAL_AVATARS_DIR, file_path + ".original")
         with open(image_path, "rb") as f:
             image_data = f.read()
 
         resized_avatar = resize_avatar(image_data)
         zerver.lib.upload.upload_backend.ensure_avatar_image(user_profile)
-        output_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", file_path + ".png")
+        output_path = os.path.join(settings.LOCAL_AVATARS_DIR, file_path + ".png")
         with open(output_path, "rb") as original_file:
             self.assertEqual(resized_avatar, original_file.read())
 
         resized_avatar = resize_avatar(image_data, MEDIUM_AVATAR_SIZE)
         zerver.lib.upload.upload_backend.ensure_avatar_image(user_profile, is_medium=True)
-        output_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", file_path + "-medium.png")
+        output_path = os.path.join(settings.LOCAL_AVATARS_DIR, file_path + "-medium.png")
         with open(output_path, "rb") as original_file:
             self.assertEqual(resized_avatar, original_file.read())
 
@@ -1887,8 +1889,8 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase):
             emoji_file_name=file_name,
         )
 
-        assert settings.LOCAL_UPLOADS_DIR is not None
+        assert settings.LOCAL_AVATARS_DIR is not None
-        file_path = os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", emoji_path)
+        file_path = os.path.join(settings.LOCAL_AVATARS_DIR, emoji_path)
         with open(file_path + ".original", "rb") as original_file:
             self.assertEqual(read_test_image_file("img.png"), original_file.read())
 
@@ -1942,11 +1944,9 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase):
         with open(tarball_path, "w") as f:
             f.write("dummy")
 
-        assert settings.LOCAL_UPLOADS_DIR is not None
+        assert settings.LOCAL_AVATARS_DIR is not None
         uri = upload_export_tarball(user_profile.realm, tarball_path)
-        self.assertTrue(
+        self.assertTrue(os.path.isfile(os.path.join(settings.LOCAL_AVATARS_DIR, tarball_path)))
-            os.path.isfile(os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars", tarball_path))
-        )
 
         result = re.search(re.compile(r"([A-Za-z0-9\-_]{24})"), uri)
         if result is not None:

zproject/computed_settings.py

@@ -40,6 +40,7 @@ from .configured_settings import (
     EXTRA_INSTALLED_APPS,
     GOOGLE_OAUTH2_CLIENT_ID,
     IS_DEV_DROPLET,
+    LOCAL_UPLOADS_DIR,
     MEMCACHED_LOCATION,
     MEMCACHED_USERNAME,
     RATE_LIMITING_RULES,
@@ -539,6 +540,9 @@ if PRODUCTION or IS_DEV_DROPLET or os.getenv("EXTERNAL_HOST") is not None:
 else:
     STATIC_URL = "http://localhost:9991/static/"
 
+LOCAL_AVATARS_DIR = os.path.join(LOCAL_UPLOADS_DIR, "avatars") if LOCAL_UPLOADS_DIR else None
+LOCAL_FILES_DIR = os.path.join(LOCAL_UPLOADS_DIR, "files") if LOCAL_UPLOADS_DIR else None
+
 # ZulipStorage is a modified version of ManifestStaticFilesStorage,
 # and, like that class, it inserts a file hash into filenames
 # to prevent the browser from using stale files from cache.

zproject/default_settings.py

@@ -141,6 +141,8 @@ S3_REGION: Optional[str] = None
 S3_ENDPOINT_URL: Optional[str] = None
 S3_SKIP_PROXY = True
 LOCAL_UPLOADS_DIR: Optional[str] = None
+LOCAL_AVATARS_DIR: Optional[str] = None
+LOCAL_FILES_DIR: Optional[str] = None
 MAX_FILE_UPLOAD_SIZE = 25
 
 # Jitsi Meet video call integration; set to None to disable integration.

zproject/dev_urls.py

@@ -125,10 +125,10 @@ i18n_urls = [
 urls += i18n_urls
 
 # On a production instance, these files would be served by nginx.
-if settings.LOCAL_UPLOADS_DIR is not None:
+if settings.LOCAL_AVATARS_DIR is not None:
     avatars_url = path(
         "user_avatars/<path:path>",
         serve,
-        {"document_root": os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars")},
+        {"document_root": os.path.join(settings.LOCAL_AVATARS_DIR)},
     )
     urls += [avatars_url]