✨(backend) wait for analysis before converting a file

Conversion reads the source bytes, so it must not run before malware
analysis confirms the file is safe. convert_file now waits while the
source is analyzing and drops the placeholder if it never clears.

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to
 
 ## [Unreleased]
 
+### Added
+
+- ✨(backend) allow converting a file while it is being analyzed
+
 ### Fixed
 
 - 🐛(frontend) keep uploaded items usable while malware analysis runs

src/backend/wopi/tasks/conversion.py

@@ -12,6 +12,12 @@ from drive.celery_app import app
 
 logger = logging.getLogger(__name__)
 
+# Seconds to wait before re-checking a source still under malware analysis,
+# and how many times to wait before giving up (~10 minutes total, matching the
+# frontend polling timeout).
+ANALYSIS_RETRY_COUNTDOWN = 30
+ANALYSIS_MAX_RETRIES = 20
+
 
 class ConvertFileTask(app.Task):
     """Celery task base deleting the placeholder when conversion ultimately fails."""
@@ -32,6 +38,7 @@ class ConvertFileTask(app.Task):
 
 
 @app.task(
+    bind=True,
     base=ConvertFileTask,
     autoretry_for=(ConversionProviderError,),
     retry_backoff=True,
@@ -39,7 +46,7 @@ class ConvertFileTask(app.Task):
     retry_jitter=True,
     max_retries=3,
 )
-def convert_file(source_item_id, converted_item_id, user_id):
+def convert_file(self, source_item_id, converted_item_id, user_id):
     """Convert the source item and attach the result to the placeholder."""
     User = get_user_model()  # pylint: disable=invalid-name
 
@@ -71,6 +78,13 @@ def convert_file(source_item_id, converted_item_id, user_id):
         logger.error("convert_file: user %s does not exist, aborting", user_id)
         return
 
+    if source.upload_state == models.ItemUploadStateChoices.ANALYZING:
+        logger.info(
+            "convert_file: source %s still analyzing, retrying conversion later",
+            source_item_id,
+        )
+        raise self.retry(countdown=ANALYSIS_RETRY_COUNTDOWN, max_retries=ANALYSIS_MAX_RETRIES)
+
     try:
         perform_conversion(source, placeholder, user)
     except Exception:

src/backend/wopi/tests/tasks/test_conversion.py

@@ -3,6 +3,7 @@
 from unittest import mock
 
 import pytest
+from celery.exceptions import Retry
 
 from core import factories, models
 from wopi.conversion import exceptions
@@ -10,6 +11,9 @@ from wopi.tasks import conversion
 
 pytestmark = pytest.mark.django_db
 
+# As the task is bound, we need to ignore this error.
+# pylint: disable=no-value-for-parameter
+
 
 def _source_and_placeholder():
     """Build a user with a ready source file and a converting placeholder."""
@@ -100,6 +104,27 @@ def test_convert_file_aborts_when_placeholder_state_changed():
     perform_mock.assert_not_called()
 
 
+def test_convert_file_retries_while_source_analyzing():
+    """Wait for malware analysis to finish before converting the source bytes."""
+    user, source, placeholder = _source_and_placeholder()
+    source.upload_state = models.ItemUploadStateChoices.ANALYZING
+    source.save(update_fields=["upload_state", "updated_at"])
+
+    with (
+        mock.patch.object(conversion.convert_file, "retry", side_effect=Retry()) as retry_mock,
+        mock.patch.object(conversion, "perform_conversion") as perform_mock,
+    ):
+        with pytest.raises(Retry):
+            conversion.convert_file(
+                source_item_id=str(source.id),
+                converted_item_id=str(placeholder.id),
+                user_id=str(user.id),
+            )
+
+    retry_mock.assert_called_once()
+    perform_mock.assert_not_called()
+
+
 def test_convert_file_ignores_missing_source():
     """Ignore a missing source item without crashing."""
     conversion.convert_file(