Build: rename PDF/ePUB filename to valid one automatically

readthedocs/projects/tasks/builds.py

@@ -5,10 +5,12 @@
 rebuilding documentation.
 """
 import os
+import shutil
 import signal
 import socket
 import subprocess
 from dataclasses import dataclass, field
+from pathlib import Path
 
 import structlog
 from celery import Task
@@ -40,6 +42,7 @@
 from readthedocs.builds.utils import memcache_lock
 from readthedocs.config.config import BuildConfigV2
 from readthedocs.config.exceptions import ConfigError
+from readthedocs.core.utils.filesystem import assert_path_is_inside_docroot
 from readthedocs.doc_builder.director import BuildDirector
 from readthedocs.doc_builder.environments import (
     DockerBuildEnvironment,
@@ -605,7 +608,8 @@ def get_valid_artifact_types(self):
             # These output format does not support multiple files yet.
             # In case multiple files are found, the upload for this format is not performed.
             if artifact_type in ARTIFACT_TYPES_WITHOUT_MULTIPLE_FILES_SUPPORT:
-                artifact_format_files = len(os.listdir(artifact_directory))
+                list_dir = os.listdir(artifact_directory)
+                artifact_format_files = len(list_dir)
                 if artifact_format_files > 1:
                     log.error(
                         "Multiple files are not supported for this format. "
@@ -626,6 +630,17 @@ def get_valid_artifact_types(self):
                         },
                     )
 
+                # Rename file as "<project_slug>-<version_slug>.<artifact_type>",
+                # which is the filename that Proxito serves for offline formats.
+                filename = list_dir[0]
+                _, extension = filename.rsplit(".")
+                path = Path(artifact_directory) / filename
+                destination = (
+                    Path(artifact_directory) / f"{self.data.project.slug}.{extension}"
+                )
+                assert_path_is_inside_docroot(path)
+                shutil.move(path, destination)
+
             # If all the conditions were met, the artifact is valid
             valid_artifacts.append(artifact_type)
 

readthedocs/projects/tests/test_build_tasks.py

@@ -1,6 +1,7 @@
 import os
 import pathlib
 import textwrap
+import uuid
 from unittest import mock
 
 import django_dynamic_fixture as fixture
@@ -403,6 +404,7 @@ def test_get_env_vars(self, load_yaml_config, build_environment, config, externa
             expected_build_env_vars["PRIVATE_TOKEN"] = "a1b2c3"
         assert build_env_vars == expected_build_env_vars
 
+    @mock.patch("readthedocs.projects.tasks.builds.shutil")
     @mock.patch("readthedocs.projects.tasks.builds.index_build")
     @mock.patch("readthedocs.projects.tasks.builds.build_complete")
     @mock.patch("readthedocs.projects.tasks.builds.send_external_build_status")
@@ -417,6 +419,7 @@ def test_successful_build(
         send_external_build_status,
         build_complete,
         index_build,
+        shutilmock,
     ):
         load_yaml_config.return_value = get_build_config(
             {
@@ -433,12 +436,16 @@ def test_successful_build(
         # Create the artifact paths, so it's detected by the builder
         os.makedirs(self.project.artifact_path(version=self.version.slug, type_="html"))
         os.makedirs(self.project.artifact_path(version=self.version.slug, type_="json"))
+        filename = str(uuid.uuid4())
         for f in ("htmlzip", "epub", "pdf"):
+            extension = "zip" if f == "htmlzip" else f
             os.makedirs(self.project.artifact_path(version=self.version.slug, type_=f))
             pathlib.Path(
                 os.path.join(
                     self.project.artifact_path(version=self.version.slug, type_=f),
-                    f"{self.project.slug}.{f}",
+                    # Use a random name for the offline format.
+                    # We will automatically rename this file to filename El Proxito expects.
+                    f"{filename}.{extension}",
                 )
             ).touch()
 
@@ -452,6 +459,24 @@ def test_successful_build(
 
         self._trigger_update_docs_task()
 
+        # Offline formats were renamed to the correct filename.
+        shutilmock.move.assert_has_calls(
+            [
+                mock.call(
+                    f"/tmp/readthedocs-tests/git-repository/_readthedocs/epub/{filename}.epub",
+                    f"/tmp/readthedocs-tests/git-repository/_readthedocs/epub/{self.project.slug}.epub",
+                ),
+                mock.call(
+                    f"/tmp/readthedocs-tests/git-repository/_readthedocs/htmlzip/{filename}.zip",
+                    f"/tmp/readthedocs-tests/git-repository/_readthedocs/htmlzip/{self.project.slug}.zip",
+                ),
+                mock.call(
+                    f"/tmp/readthedocs-tests/git-repository/_readthedocs/pdf/{filename}.pdf",
+                    f"/tmp/readthedocs-tests/git-repository/_readthedocs/pdf/{self.project.slug}.pdf",
+                ),
+            ]
+        )
+
         # It has to be called twice, ``before_start`` and ``after_return``
         clean_build.assert_has_calls(
             [mock.call(mock.ANY), mock.call(mock.ANY)]  # the argument is an APIVersion