diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a09ccb22..843e1f9d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to - ✨(backend) manage reconciliation requests for user accounts - ✨(backend) add recursive folder export as ZIP archive - ✨(frontend) add folder export action +- ✨(backend) background conversion of legacy Office files ### Changed diff --git a/compose.yaml b/compose.yaml index c0d490d87..f84ba2db6 100644 --- a/compose.yaml +++ b/compose.yaml @@ -270,6 +270,7 @@ services: environment: TZ: "Europe/Berlin" USE_UNAUTHORIZED_STORAGE: "true" + JWT_ENABLED: "false" ports: - "9981:80" volumes: diff --git a/docker/onlyoffice/local-development.json b/docker/onlyoffice/local-development.json index cedbb6eae..b54475fe9 100644 --- a/docker/onlyoffice/local-development.json +++ b/docker/onlyoffice/local-development.json @@ -18,5 +18,12 @@ "converter": { "maxDownloadBytes": 209715200 } + }, + "services": { + "CoAuthoring": { + "request-filtering-agent": { + "allowPrivateIPAddress": true + } + } } } diff --git a/env.d/development/common b/env.d/development/common index d6f8e5cfa..06d146b7d 100644 --- a/env.d/development/common +++ b/env.d/development/common @@ -84,6 +84,9 @@ WOPI_SRC_BASE_URL=http://app-dev:8000 # Disable rename for Collabora by default because it does not post messages when renaming a file. # So the frontend cannot reflect renaming operations. WOPI_COLLABORA_OPTIONS={"SupportsRename": False} +# Background conversion of legacy Office files (.doc, .xls, .ppt) is forced +# server-side via the OnlyOffice /converter endpoint. +WOPI_ONLYOFFICE_OPTIONS={"ForceConvertExtensions": ["doc", "xls", "ppt"], "ConvertServiceUrl": "http://onlyoffice/converter"} # Indexer # SEARCH_INDEXER_CLASS="core.services.search_indexers.SearchIndexer" diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 2a1d98974..bf42b18e3 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -56,7 +56,10 @@ from core.storage import get_storage_compute_backend from core.tasks.item import duplicate_file, process_item_purge, rename_file from core.utils.analytics import posthog_capture +from wopi.conversion import exceptions as conversion_exceptions +from wopi.conversion.services import prepare_conversion from wopi.services import access as access_service +from wopi.tasks.conversion import convert_file as convert_file_task from wopi.utils import compute_wopi_launch_url, get_wopi_client_config from . import permissions, serializers, utils @@ -644,6 +647,38 @@ def hard_delete(self, request, *args, **kwargs): process_item_purge.delay(instance.id) return drf.response.Response(status=status.HTTP_204_NO_CONTENT) + @drf.decorators.action(detail=True, methods=["post"], url_path="convert") + def convert(self, request, *args, **kwargs): + """Queue a legacy Office file conversion. + + Creates a placeholder Item in CONVERTING state in the destination folder + and dispatches the celery task that will attach the converted bytes. + """ + source = self.get_object() + try: + placeholder = prepare_conversion(source, request.user) + except conversion_exceptions.ConversionPermissionDenied as exc: + raise drf.exceptions.PermissionDenied() from exc + except ( + conversion_exceptions.ConversionRejected, + conversion_exceptions.ConversionMisconfigured, + ) as exc: + raise drf.exceptions.ValidationError({"detail": str(exc)}) from exc + + try: + convert_file_task.delay( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(request.user.id), + ) + except Exception: + placeholder.soft_delete() + placeholder.delete() + raise + + serializer = self.get_serializer(placeholder) + return drf.response.Response(serializer.data, status=status.HTTP_201_CREATED) + def list(self, request, *args, **kwargs): """List top level items with pagination and filtering.""" # Not calling filter_queryset. We do our own cooking. diff --git a/src/backend/core/migrations/0024_alter_item_upload_state.py b/src/backend/core/migrations/0024_alter_item_upload_state.py new file mode 100644 index 000000000..01ceaa9b4 --- /dev/null +++ b/src/backend/core/migrations/0024_alter_item_upload_state.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.14 on 2026-05-28 07:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0023_userreconciliationcsvimport_userreconciliation'), + ] + + operations = [ + migrations.AlterField( + model_name='item', + name='upload_state', + field=models.CharField(blank=True, choices=[('pending', 'Pending'), ('duplicating', 'Duplicating'), ('converting', 'Converting'), ('analyzing', 'Analyzing'), ('suspicious', 'Suspicious'), ('file_too_large_to_analyze', 'File too large to analyze'), ('ready', 'Ready')], max_length=25, null=True), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index d3f1575f4..501c40e86 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -43,6 +43,7 @@ from timezone_field import TimeZoneField from core.utils.item_title import manage_unique_title as manage_unique_title_utils +from wopi.conversion.policy import target_extension_for logger = getLogger(__name__) @@ -73,6 +74,7 @@ class ItemUploadStateChoices(models.TextChoices): PENDING = "pending", _("Pending") DUPLICATING = "duplicating", ("Duplicating") + CONVERTING = "converting", _("Converting") ANALYZING = "analyzing", _("Analyzing") SUSPICIOUS = "suspicious", _("Suspicious") FILE_TOO_LARGE_TO_ANALYZE = ( @@ -1033,7 +1035,11 @@ def save(self, *args, **kwargs): if ( self.created_at is None and self.type == ItemTypeChoices.FILE - and self.upload_state != ItemUploadStateChoices.DUPLICATING + and self.upload_state + not in ( + ItemUploadStateChoices.DUPLICATING, + ItemUploadStateChoices.CONVERTING, + ) ): self.upload_state = ItemUploadStateChoices.PENDING @@ -1288,6 +1294,12 @@ def get_abilities(self, user): and self.upload_state == ItemUploadStateChoices.READY ) can_export = can_get and self.type == ItemTypeChoices.FOLDER + can_convert = ( + can_update + and self.type == ItemTypeChoices.FILE + and self.upload_state == ItemUploadStateChoices.READY + and bool(target_extension_for(self.extension)) + ) return { "accesses_manage": can_manage, @@ -1313,6 +1325,7 @@ def get_abilities(self, user): "update": can_update, "upload_ended": can_update and user.is_authenticated, "wopi": can_get, + "convert": can_convert, } def send_email(self, subject, emails, context=None, language=None): diff --git a/src/backend/core/tests/items/test_api_items_convert.py b/src/backend/core/tests/items/test_api_items_convert.py new file mode 100644 index 000000000..9bb13c78c --- /dev/null +++ b/src/backend/core/tests/items/test_api_items_convert.py @@ -0,0 +1,116 @@ +"""Tests for the /items//convert/ endpoint.""" + +from unittest import mock + +import pytest +from kombu.exceptions import KombuError +from rest_framework.test import APIClient + +from core import factories, models + +pytestmark = pytest.mark.django_db + + +def _build_user_and_item(): + """Build an EDITOR user with a legacy .doc item ready for conversion.""" + user = factories.UserFactory() + parent = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + item = factories.ItemFactory( + parent=parent, + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FILE, + filename="document.doc", + mimetype="application/msword", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + return user, item + + +@pytest.fixture(autouse=True) +def _wopi_settings(settings): + """Configure the OnlyOffice WOPI client for the conversion tests.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example" + settings.WOPI_CLIENTS_CONFIGURATION = { + "onlyoffice": { + "options": { + "ForceConvertExtensions": ["doc"], + "ConvertServiceUrl": "http://onlyoffice/converter", + }, + } + } + + +def test_convert_endpoint_creates_placeholder_and_queues_task(): + """Return the placeholder and enqueue the conversion task on POST /convert.""" + user, item = _build_user_and_item() + client = APIClient() + client.force_login(user) + + with mock.patch("core.api.viewsets.convert_file_task.delay") as delay_mock: + response = client.post(f"/api/v1.0/items/{item.id}/convert/") + + assert response.status_code == 201 + body = response.json() + placeholder = models.Item.objects.get(id=body["id"]) + assert placeholder.upload_state == models.ItemUploadStateChoices.CONVERTING + assert placeholder.filename == "document.docx" + assert placeholder.parent().id == item.parent().id + delay_mock.assert_called_once_with( + source_item_id=str(item.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + +def test_convert_endpoint_cleans_up_placeholder_when_queueing_fails(): + """Remove the placeholder and let the broker error bubble up.""" + user, item = _build_user_and_item() + client = APIClient(raise_request_exception=False) + client.force_login(user) + + with mock.patch("core.api.viewsets.convert_file_task.delay", side_effect=KombuError): + response = client.post(f"/api/v1.0/items/{item.id}/convert/") + + assert response.status_code == 500 + assert not models.Item.objects.filter( + upload_state=models.ItemUploadStateChoices.CONVERTING + ).exists() + + +def test_convert_endpoint_returns_403_when_user_cannot_update_item(): + """Return 403 and skip placeholder creation when the user cannot update.""" + _, item = _build_user_and_item() + other_user = factories.UserFactory() + client = APIClient() + client.force_login(other_user) + + with mock.patch("core.api.viewsets.convert_file_task.delay") as delay_mock: + response = client.post(f"/api/v1.0/items/{item.id}/convert/") + + assert response.status_code == 403 + assert not models.Item.objects.filter( + upload_state=models.ItemUploadStateChoices.CONVERTING + ).exists() + delay_mock.assert_not_called() + + +def test_convert_endpoint_returns_403_for_unsupported_extension(): + """Reject conversion with 403 for non-legacy file extensions.""" + user = factories.UserFactory() + item = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FILE, + filename="image.png", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + client = APIClient() + client.force_login(user) + + with mock.patch("core.api.viewsets.convert_file_task.delay") as delay_mock: + response = client.post(f"/api/v1.0/items/{item.id}/convert/") + + assert response.status_code == 403 + delay_mock.assert_not_called() diff --git a/src/backend/core/tests/items/test_api_items_retrieve.py b/src/backend/core/tests/items/test_api_items_retrieve.py index c8415f8bc..30b373185 100644 --- a/src/backend/core/tests/items/test_api_items_retrieve.py +++ b/src/backend/core/tests/items/test_api_items_retrieve.py @@ -1516,7 +1516,10 @@ def test_api_items_retrieve_wopi_supported(): WOPI_CONFIGURATION_CACHE_KEY, { "mimetypes": { - "application/vnd.oasis.opendocument.text": "https://vendorA.com/launch_url", + "application/vnd.oasis.opendocument.text": { + "url": "https://vendorA.com/launch_url", + "client": "vendorA", + }, }, "extensions": {}, }, diff --git a/src/backend/core/tests/test_models_items.py b/src/backend/core/tests/test_models_items.py index c3757e3c0..cd9ba4362 100644 --- a/src/backend/core/tests/test_models_items.py +++ b/src/backend/core/tests/test_models_items.py @@ -212,6 +212,31 @@ def test_models_items_hard_delete(): # get_abilities +@pytest.mark.parametrize( + "extension,expected", + [ + ("doc", True), + ("DOC", True), + ("xls", True), + ("ppt", True), + ("docx", False), + ("pdf", False), + (None, False), + ], +) +def test_models_items_get_abilities_convert(extension, expected): + """Flag convert only for legacy extensions on READY files with update access.""" + user = factories.UserFactory() + filename = f"file.{extension}" if extension else None + item = factories.ItemFactory( + users=[(user, "editor")], + type=models.ItemTypeChoices.FILE, + filename=filename or "file", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + assert item.get_abilities(user)["convert"] is expected + + @pytest.mark.parametrize( "is_authenticated,reach,role", [ @@ -256,6 +281,7 @@ def test_models_items_get_abilities_forbidden( "update": False, "upload_ended": False, "wopi": False, + "convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -305,6 +331,7 @@ def test_models_items_get_abilities_reader(is_authenticated, reach, django_asser "update": False, "upload_ended": False, "wopi": True, + "convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -380,6 +407,7 @@ def test_models_items_get_abilities_editor( # noqa: PLR0913 link_role="editor", type=item_type, update_upload_state=upload_state, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, ) user = factories.UserFactory() if is_authenticated else AnonymousUser() @@ -408,6 +436,7 @@ def test_models_items_get_abilities_editor( # noqa: PLR0913 "update": True, "upload_ended": is_authenticated, "wopi": True, + "convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -443,7 +472,10 @@ def test_models_items_not_root_get_abilities_owner( """Check abilities returned for the owner of an item.""" user = factories.UserFactory() item = factories.ItemFactory( - users=[(user, "owner")], type=item_type, update_upload_state=upload_state + users=[(user, "owner")], + type=item_type, + update_upload_state=upload_state, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, ) can_export = item_type == models.ItemTypeChoices.FOLDER expected_abilities = { @@ -474,6 +506,7 @@ def test_models_items_not_root_get_abilities_owner( "update": True, "upload_ended": True, "wopi": True, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -503,6 +536,7 @@ def test_models_items_not_root_get_abilities_owner( "update": False, "upload_ended": False, "wopi": False, + "convert": False, } @@ -531,6 +565,7 @@ def test_models_items_not_root_get_abilities_administrator( users=[(user, "administrator")], type=item_type, update_upload_state=upload_state, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, ) can_export = item_type == models.ItemTypeChoices.FOLDER expected_abilities = { @@ -561,6 +596,7 @@ def test_models_items_not_root_get_abilities_administrator( "update": True, "upload_ended": True, "wopi": True, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -601,6 +637,7 @@ def test_models_items_not_root_get_abilities_editor_user( item = factories.ItemFactory( parent=parent, type=item_type, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, update_upload_state=upload_state, ) link_select_options = LinkReachChoices.get_select_options(**item.ancestors_link_definition) @@ -629,6 +666,7 @@ def test_models_items_not_root_get_abilities_editor_user( "update": True, "upload_ended": True, "wopi": True, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -681,6 +719,7 @@ def test_models_items_not_root_get_abilities_reader_user(django_assert_num_queri "update": access_from_link, "upload_ended": access_from_link, "wopi": True, + "convert": False, } with django_assert_num_queries(2): assert item.get_abilities(user) == expected_abilities @@ -738,6 +777,7 @@ def test_models_items_get_abilities_hard_delete_non_root_by_non_creator( "update": True, "upload_ended": True, "wopi": True, + "convert": False, } with django_assert_num_queries(1): assert child.get_abilities(owner) == expected_abilities @@ -768,6 +808,7 @@ def test_models_items_get_abilities_hard_delete_non_root_by_non_creator( "update": False, "upload_ended": False, "wopi": False, + "convert": False, } diff --git a/src/backend/core/tests/test_models_items_root.py b/src/backend/core/tests/test_models_items_root.py index d79bb596c..90a103e3d 100644 --- a/src/backend/core/tests/test_models_items_root.py +++ b/src/backend/core/tests/test_models_items_root.py @@ -71,6 +71,7 @@ def test_models_sub_item_abilities_downgraded(): "update": True, "upload_ended": True, "wopi": True, + "convert": False, } # Downgrade the role on the root item @@ -106,6 +107,7 @@ def test_models_sub_item_abilities_downgraded(): "update": False, "upload_ended": False, "wopi": True, + "convert": False, } @@ -159,6 +161,7 @@ def test_models_items_root_get_abilities_owner( "update": True, "upload_ended": True, "wopi": True, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -188,6 +191,7 @@ def test_models_items_root_get_abilities_owner( "update": False, "upload_ended": False, "wopi": False, + "convert": False, } @@ -215,6 +219,7 @@ def test_models_items_root_get_abilities_administrator( item = factories.ItemFactory( users=[(user, "administrator")], type=item_type, + filename=("document.pdf" if item_type == models.ItemTypeChoices.FILE else None), update_upload_state=upload_state, ) link_select_options = LinkReachChoices.get_select_options(**item.ancestors_link_definition) @@ -243,6 +248,7 @@ def test_models_items_root_get_abilities_administrator( "update": True, "upload_ended": True, "wopi": True, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -277,7 +283,10 @@ def test_models_items_root_get_abilities_editor_user( """Check abilities returned for the editor of a root item.""" user = factories.UserFactory() item = factories.ItemFactory( - users=[(user, "editor")], type=item_type, update_upload_state=upload_state + users=[(user, "editor")], + type=item_type, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, + update_upload_state=upload_state, ) link_select_options = LinkReachChoices.get_select_options(**item.ancestors_link_definition) can_export = item_type == models.ItemTypeChoices.FOLDER @@ -305,6 +314,7 @@ def test_models_items_root_get_abilities_editor_user( "update": True, "upload_ended": True, "wopi": True, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -357,6 +367,7 @@ def test_models_items_root_get_abilities_reader_user( "update": access_from_link, "upload_ended": access_from_link, "wopi": True, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities diff --git a/src/backend/drive/settings.py b/src/backend/drive/settings.py index 2672ce1c2..ba4c1d68f 100755 --- a/src/backend/drive/settings.py +++ b/src/backend/drive/settings.py @@ -1395,9 +1395,29 @@ class Base(Configuration): WOPI_ACCESS_TOKEN_TIMEOUT = values.IntegerValue( 60 * 60 * 10, environ_name="WOPI_ACCESS_TOKEN_TIMEOUT", environ_prefix=None ) + WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT = values.IntegerValue( + 120, environ_name="WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT", environ_prefix=None + ) WOPI_LOCK_TIMEOUT = values.IntegerValue( 30 * 60, environ_name="WOPI_LOCK_TIMEOUT", environ_prefix=None ) + WOPI_LEGACY_CONVERSION_TARGETS = { + "doc": "docx", + "xls": "xlsx", + "ppt": "pptx", + } + WOPI_ONLYOFFICE_CONVERT_HTTP_CONNECT_TIMEOUT = values.IntegerValue( + 5, environ_name="WOPI_ONLYOFFICE_CONVERT_HTTP_CONNECT_TIMEOUT", environ_prefix=None + ) + WOPI_ONLYOFFICE_CONVERT_HTTP_READ_TIMEOUT = values.IntegerValue( + 60, environ_name="WOPI_ONLYOFFICE_CONVERT_HTTP_READ_TIMEOUT", environ_prefix=None + ) + WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_CONNECT_TIMEOUT = values.IntegerValue( + 5, environ_name="WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_CONNECT_TIMEOUT", environ_prefix=None + ) + WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_READ_TIMEOUT = values.IntegerValue( + 30, environ_name="WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_READ_TIMEOUT", environ_prefix=None + ) WOPI_DISABLE_CHAT = values.IntegerValue( 0, environ_name="WOPI_DISABLE_CHAT", environ_prefix=None ) diff --git a/src/backend/wopi/apps.py b/src/backend/wopi/apps.py index 43a31009f..f6dc99de5 100644 --- a/src/backend/wopi/apps.py +++ b/src/backend/wopi/apps.py @@ -7,3 +7,8 @@ class WopiConfig(AppConfig): """Configuration class for the wopi app.""" name = "wopi" + + def ready(self): + """Import celery tasks.""" + # pylint: disable=import-outside-toplevel, unused-import + from .tasks import conversion # noqa: PLC0415,F401 diff --git a/src/backend/wopi/conversion/__init__.py b/src/backend/wopi/conversion/__init__.py new file mode 100644 index 000000000..701c0f9e7 --- /dev/null +++ b/src/backend/wopi/conversion/__init__.py @@ -0,0 +1 @@ +"""Server-to-server conversion of legacy Office files via WOPI providers.""" diff --git a/src/backend/wopi/conversion/backends/__init__.py b/src/backend/wopi/conversion/backends/__init__.py new file mode 100644 index 000000000..d3b6e6731 --- /dev/null +++ b/src/backend/wopi/conversion/backends/__init__.py @@ -0,0 +1 @@ +"""Conversion backends called by the WOPI conversion service.""" diff --git a/src/backend/wopi/conversion/backends/onlyoffice.py b/src/backend/wopi/conversion/backends/onlyoffice.py new file mode 100644 index 000000000..756e1f6a9 --- /dev/null +++ b/src/backend/wopi/conversion/backends/onlyoffice.py @@ -0,0 +1,108 @@ +"""OnlyOffice server-to-server conversion backend.""" + +from uuid import uuid4 + +from django.conf import settings +from django.core.files.base import ContentFile + +import jwt +import requests + +from wopi.conversion.exceptions import ( + ConversionMisconfigured, + ConversionProviderError, +) + + +class OnlyOfficeConversionBackend: + """Run a synchronous OnlyOffice conversion through the /converter endpoint.""" + + # pylint: disable-next=too-many-arguments,too-many-positional-arguments + def __init__( + self, + convert_service_url, + jwt_secret=None, + jwt_required=False, + http_timeout=None, + download_timeout=None, + ): + if jwt_required and not jwt_secret: + raise ConversionMisconfigured( + "OnlyOffice JWT is required but no ConvertJwtSecret is configured." + ) + self.convert_service_url = convert_service_url + self.jwt_secret = jwt_secret + self.http_timeout = http_timeout or ( + settings.WOPI_ONLYOFFICE_CONVERT_HTTP_CONNECT_TIMEOUT, + settings.WOPI_ONLYOFFICE_CONVERT_HTTP_READ_TIMEOUT, + ) + self.download_timeout = download_timeout or ( + settings.WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_CONNECT_TIMEOUT, + settings.WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_READ_TIMEOUT, + ) + + def _request(self, method, url, label, **kwargs): + """Issue an HTTP request and translate transport/HTTP errors.""" + try: + response = getattr(requests, method)(url, **kwargs) + except requests.exceptions.RequestException as exc: + raise ConversionProviderError(str(exc)) from exc + + if not response.ok: + raise ConversionProviderError( + f"OnlyOffice {label} returned status {response.status_code}" + ) + return response + + def _post_convert(self, payload, headers): + """Call /converter and return the parsed completion payload.""" + response = self._request( + "post", + self.convert_service_url, + label="/converter", + params={"shardkey": payload["key"]}, + json=payload, + headers=headers, + timeout=self.http_timeout, + ) + + try: + data = response.json() + except ValueError as exc: + raise ConversionProviderError("OnlyOffice returned a non-JSON body") from exc + + if data.get("error"): + raise ConversionProviderError(f"OnlyOffice error code {data['error']}") + if not data.get("endConvert") or not data.get("fileUrl"): + raise ConversionProviderError("OnlyOffice did not report a completed conversion") + return data + + def _download(self, file_url, target_extension): + """Fetch the converted file and wrap it as a ContentFile.""" + response = self._request( + "get", + file_url, + label="file download", + timeout=self.download_timeout, + ) + return ContentFile(response.content, name=f"converted.{target_extension}") + + def convert(self, item, source_url, target_extension): + """Convert the item via OnlyOffice and return the converted bytes.""" + key = f"{item.id}-{uuid4()}" + payload = { + "async": False, + "filetype": (item.extension or "").lower(), + "outputtype": target_extension, + "key": key, + "title": item.filename, + "url": source_url, + } + headers = {"Accept": "application/json"} + if self.jwt_secret: + token = jwt.encode({"payload": payload}, self.jwt_secret, algorithm="HS256") + payload = {**payload, "token": token} + headers["Authorization"] = f"Bearer {token}" + + data = self._post_convert(payload, headers) + return self._download(data["fileUrl"], target_extension) diff --git a/src/backend/wopi/conversion/exceptions.py b/src/backend/wopi/conversion/exceptions.py new file mode 100644 index 000000000..3ed2d2a4d --- /dev/null +++ b/src/backend/wopi/conversion/exceptions.py @@ -0,0 +1,21 @@ +"""Exceptions for server-to-server file conversion.""" + + +class ConversionError(Exception): + """Base exception for conversion service failures.""" + + +class ConversionPermissionDenied(ConversionError): + """Raised when the user cannot convert the source item.""" + + +class ConversionRejected(ConversionError): + """Raised when the conversion request cannot be satisfied.""" + + +class ConversionMisconfigured(ConversionError): + """Raised when the active conversion backend is missing required settings.""" + + +class ConversionProviderError(ConversionError): + """Raised when the document server fails to deliver the converted file.""" diff --git a/src/backend/wopi/conversion/policy.py b/src/backend/wopi/conversion/policy.py new file mode 100644 index 000000000..703572c97 --- /dev/null +++ b/src/backend/wopi/conversion/policy.py @@ -0,0 +1,42 @@ +"""Forced-conversion policy for WOPI legacy formats.""" + +from django.conf import settings + + +def _normalize(value): + return value.lower() if isinstance(value, str) else value + + +def target_extension_for(source_extension): + """Return the converted extension for a legacy source extension. + + Item.extension preserves filename case (REPORT.DOC -> "DOC"), so the + lookup must be case-insensitive. + """ + if not source_extension: + return None + + return settings.WOPI_LEGACY_CONVERSION_TARGETS.get(source_extension.lower()) + + +def is_forced_conversion(item, client_options): + """Return True when the active WOPI client forces a server-side conversion. + + The decision is product policy, not discovery-derived: a client is allowed + to advertise an "edit" action for a legacy format and still require the + Drive backend to convert it first. + """ + if not client_options: + return False + + extension = _normalize(item.extension) + forced_extensions = {_normalize(e) for e in client_options.get("ForceConvertExtensions") or []} + if extension and extension in forced_extensions: + return True + + mimetype = _normalize(item.mimetype) + forced_mimetypes = {_normalize(m) for m in client_options.get("ForceConvertMimetypes") or []} + if mimetype and mimetype in forced_mimetypes: + return True + + return False diff --git a/src/backend/wopi/conversion/services.py b/src/backend/wopi/conversion/services.py new file mode 100644 index 000000000..978df5ba0 --- /dev/null +++ b/src/backend/wopi/conversion/services.py @@ -0,0 +1,159 @@ +"""Service layer for server-to-server legacy file conversion.""" + +from os.path import splitext + +from django.conf import settings +from django.core.files.storage import default_storage +from django.db import DatabaseError, transaction + +from core import models +from core.api.utils import detect_mimetype +from core.models import Item +from core.utils.item_title import manage_unique_title +from wopi.conversion.backends.onlyoffice import OnlyOfficeConversionBackend +from wopi.conversion.exceptions import ( + ConversionMisconfigured, + ConversionPermissionDenied, + ConversionRejected, +) +from wopi.conversion.policy import is_forced_conversion, target_extension_for +from wopi.conversion.source_url import build_source_url + +MIME_SNIFF_BYTES = 2048 + + +def _validate_conversion(item, user): + """Run pre-flight checks and return the target extension and client options.""" + if item.type != models.ItemTypeChoices.FILE: + raise ConversionRejected("Source item is not a file.") + + if item.upload_state != models.ItemUploadStateChoices.READY: + raise ConversionRejected("Source item is not ready.") + + if not item.get_abilities(user).get("update"): + raise ConversionPermissionDenied("User cannot update the source item.") + + target_extension = target_extension_for(item.extension) + if not target_extension: + raise ConversionRejected("No target conversion for this extension.") + + onlyoffice_config = settings.WOPI_CLIENTS_CONFIGURATION.get("onlyoffice") + if onlyoffice_config is None: + raise ConversionRejected("No OnlyOffice client configured.") + + client_options = onlyoffice_config.get("options", {}) + if not is_forced_conversion(item, client_options): + raise ConversionRejected("Conversion not forced by the active WOPI client.") + + return target_extension, client_options + + +def resolve_backend(client_options): + """Build the OnlyOffice conversion backend from WOPI client options.""" + convert_service_url = client_options.get("ConvertServiceUrl") + if not convert_service_url: + raise ConversionMisconfigured("Missing OnlyOffice ConvertServiceUrl") + + return OnlyOfficeConversionBackend( + convert_service_url=convert_service_url, + jwt_secret=client_options.get("ConvertJwtSecret"), + jwt_required=bool(client_options.get("ConvertJwtRequired", False)), + ) + + +def _resolve_destination_parent(source_item, user): + """Return the source parent when the user can still update it, else None.""" + if source_item.depth <= 1: + return None + + parent = source_item.parent() + if not parent.get_abilities(user).get("children_create"): + return None + + return parent + + +def _target_filename(item, target_extension, parent, user): + """Return a sibling-free filename for the converted item.""" + if parent: + siblings = Item.objects.children(parent.path) + else: + siblings = Item.objects.filter(path__depth=1, accesses__user=user).distinct() + + base, _ = splitext(item.filename) + target = f"{base}.{target_extension}" + + return manage_unique_title(siblings, target) + + +def prepare_conversion(source_item, user): + """Validate inputs and create the placeholder Item receiving the conversion. + + The placeholder is created synchronously and returned to the caller so the + UI can display the converting state right away. The actual OnlyOffice + conversion happens later in a celery task. + """ + target_extension, _ = _validate_conversion(source_item, user) + parent = _resolve_destination_parent(source_item, user) + target_filename = _target_filename(source_item, target_extension, parent, user) + + with transaction.atomic(): + placeholder = Item.objects.create_child( + creator=user, + link_reach=None if parent else models.LinkReachChoices.RESTRICTED, + parent=parent, + title=target_filename, + type=models.ItemTypeChoices.FILE, + filename=target_filename, + upload_state=models.ItemUploadStateChoices.CONVERTING, + ) + if placeholder.is_root: + models.ItemAccess.objects.create( + item=placeholder, + user=user, + role=models.RoleChoices.OWNER, + ) + + return placeholder + + +def perform_conversion(source_item, placeholder, user): + """Run the OnlyOffice conversion and attach the result to the placeholder. + + Called from the celery task. The network call to OnlyOffice happens outside + of any DB transaction; only the final READY update is wrapped in one. + """ + target_extension, client_options = _validate_conversion(source_item, user) + source_url = build_source_url(source_item, user) + converted_file = resolve_backend(client_options).convert( + source_item, source_url, target_extension + ) + + converted_file.seek(0) + mimetype = detect_mimetype( + converted_file.read(min(MIME_SNIFF_BYTES, converted_file.size)), + filename=placeholder.filename, + ) + converted_file.seek(0) + + default_storage.save(placeholder.file_key, converted_file) + try: + placeholder.mimetype = mimetype + placeholder.size = converted_file.size + placeholder.upload_state = models.ItemUploadStateChoices.READY + placeholder.save(update_fields=["mimetype", "size", "upload_state", "updated_at"]) + except DatabaseError: + default_storage.delete(placeholder.file_key) + raise + + return placeholder + + +def convert_item(source_item, user): + """Prepare and run a conversion synchronously. + + Used by tests and any call site that wants the new converted item right away + instead of going through the celery task. + """ + placeholder = prepare_conversion(source_item, user) + return perform_conversion(source_item, placeholder, user) diff --git a/src/backend/wopi/conversion/source_url.py b/src/backend/wopi/conversion/source_url.py new file mode 100644 index 000000000..47c3813ac --- /dev/null +++ b/src/backend/wopi/conversion/source_url.py @@ -0,0 +1,22 @@ +"""Build the short-lived WOPI URL OnlyOffice uses to fetch the source bytes.""" + +from django.conf import settings + +from wopi.conversion.exceptions import ConversionMisconfigured +from wopi.services.access import AccessUserItemService + + +def build_source_url(item, user): + """Return a short-lived WOPI GetFile URL pointing at the item for the user.""" + base_url = settings.WOPI_SRC_BASE_URL + + if not base_url: + raise ConversionMisconfigured("Missing WOPI_SRC_BASE_URL for conversion source URL") + + access_token, _ttl_ms = AccessUserItemService().insert_new_access( + item, user, ttl=settings.WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT + ) + return ( + f"{base_url.rstrip('/')}/api/{settings.API_VERSION}" + f"/wopi/files/{item.id}/contents/?access_token={access_token}" + ) diff --git a/src/backend/wopi/services/access.py b/src/backend/wopi/services/access.py index 857e85c01..537ba0089 100644 --- a/src/backend/wopi/services/access.py +++ b/src/backend/wopi/services/access.py @@ -68,22 +68,25 @@ def generate_token(): """Generate a random access token""" return token_urlsafe() - def insert_new_access(self, item: Item, user: AbstractUser) -> tuple[str, int]: + def insert_new_access( + self, item: Item, user: AbstractUser, ttl: int | None = None + ) -> tuple[str, int]: """ Insert a new access token for the user and item. Return an access_token and access_token_ttl - access_token_ttl must be a timestamp in milliseconds + access_token_ttl must be a timestamp in milliseconds. + + ttl overrides the default WOPI_ACCESS_TOKEN_TIMEOUT lifetime. Pass a short + ttl for one-shot uses (server-to-server conversion source download, etc.). """ abilities = item.get_abilities(user) if not abilities["retrieve"]: raise AccessUserItemNotAllowed() + + effective_ttl = ttl if ttl is not None else settings.WOPI_ACCESS_TOKEN_TIMEOUT token = self.generate_token() access_user_item = AccessUserItem(item=item, user=user) - token_eol = timezone.now() + timedelta(seconds=settings.WOPI_ACCESS_TOKEN_TIMEOUT) - cache.set( - token, - access_user_item.to_dict(), - timeout=settings.WOPI_ACCESS_TOKEN_TIMEOUT, - ) + token_eol = timezone.now() + timedelta(seconds=effective_ttl) + cache.set(token, access_user_item.to_dict(), timeout=effective_ttl) return token, int(round(token_eol.timestamp())) * 1000 def get_access_user_item(self, token: str) -> AccessUserItem: diff --git a/src/backend/wopi/tasks/__init__.py b/src/backend/wopi/tasks/__init__.py index e69de29bb..91e916ad7 100644 --- a/src/backend/wopi/tasks/__init__.py +++ b/src/backend/wopi/tasks/__init__.py @@ -0,0 +1 @@ +"""Celery task package for the WOPI app.""" diff --git a/src/backend/wopi/tasks/conversion.py b/src/backend/wopi/tasks/conversion.py new file mode 100644 index 000000000..655b5e120 --- /dev/null +++ b/src/backend/wopi/tasks/conversion.py @@ -0,0 +1,61 @@ +"""Celery tasks for WOPI conversion.""" + +import logging + +from django.contrib.auth import get_user_model + +from core import models +from wopi.conversion.services import perform_conversion + +from drive.celery_app import app + +logger = logging.getLogger(__name__) + + +@app.task +def convert_file(source_item_id, converted_item_id, user_id): + """Convert the source item and attach the result to the placeholder. + + Mirrors the duplicate_file task pattern: the placeholder Item already + exists in CONVERTING state; on failure it is hard-deleted so the folder + stops showing a stuck spinner. + """ + User = get_user_model() # pylint: disable=invalid-name + + try: + source = models.Item.objects.get(id=source_item_id) + except models.Item.DoesNotExist: + logger.error("convert_file: source item %s does not exist, aborting", source_item_id) + return + + try: + placeholder = models.Item.objects.get(id=converted_item_id) + except models.Item.DoesNotExist: + logger.error( + "convert_file: placeholder item %s does not exist, aborting", converted_item_id + ) + return + + if placeholder.upload_state != models.ItemUploadStateChoices.CONVERTING: + logger.error( + "convert_file: placeholder %s upload_state is %s, not converting; aborting", + placeholder.id, + placeholder.upload_state, + ) + return + + try: + user = User.objects.get(id=user_id) + except User.DoesNotExist: + logger.error("convert_file: user %s does not exist, aborting", user_id) + placeholder.soft_delete() + placeholder.delete() + return + + try: + perform_conversion(source, placeholder, user) + except Exception: + logger.exception("convert_file: conversion failed for placeholder %s", placeholder.id) + placeholder.soft_delete() + placeholder.delete() + raise diff --git a/src/backend/wopi/tests/tasks/test_conversion.py b/src/backend/wopi/tests/tasks/test_conversion.py new file mode 100644 index 000000000..7bfe2c287 --- /dev/null +++ b/src/backend/wopi/tests/tasks/test_conversion.py @@ -0,0 +1,121 @@ +"""Tests for the conversion celery tasks.""" + +import pytest + +from core import factories, models +from wopi.conversion import exceptions +from wopi.tasks import conversion + +pytestmark = pytest.mark.django_db + + +def _source_and_placeholder(): + """Build a user with a ready source file and a converting placeholder.""" + user = factories.UserFactory() + source = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.READY, + ) + placeholder = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FILE, + filename="document.docx", + update_upload_state=models.ItemUploadStateChoices.CONVERTING, + ) + return user, source, placeholder + + +def test_convert_file_calls_perform_conversion(monkeypatch): + """Delegate to perform_conversion with the resolved entities on success.""" + user, source, placeholder = _source_and_placeholder() + calls = {} + + def fake_perform(src, ph, usr): + calls["args"] = (src.id, ph.id, usr.id) + + monkeypatch.setattr(conversion, "perform_conversion", fake_perform) + + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + assert calls["args"] == (source.id, placeholder.id, user.id) + + +def test_convert_file_deletes_placeholder_on_conversion_error(monkeypatch): + """Hard-delete the placeholder when perform_conversion raises ConversionError.""" + user, source, placeholder = _source_and_placeholder() + + def fail(*_args, **_kwargs): + raise exceptions.ConversionError("boom") + + monkeypatch.setattr(conversion, "perform_conversion", fail) + + with pytest.raises(exceptions.ConversionError): + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + assert not models.Item.objects.filter(id=placeholder.id).exists() + + +def test_convert_file_deletes_placeholder_on_unexpected_exception(monkeypatch): + """Clean up the placeholder and re-raise on any uncaught exception.""" + user, source, placeholder = _source_and_placeholder() + + def fail(*_args, **_kwargs): + raise RuntimeError("unexpected") + + monkeypatch.setattr(conversion, "perform_conversion", fail) + + with pytest.raises(RuntimeError): + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + assert not models.Item.objects.filter(id=placeholder.id).exists() + + +def test_convert_file_aborts_when_placeholder_state_changed(monkeypatch): + """Skip work when the placeholder is no longer CONVERTING.""" + user, source, placeholder = _source_and_placeholder() + placeholder.upload_state = models.ItemUploadStateChoices.READY + placeholder.save(update_fields=["upload_state", "updated_at"]) + + def boom(*_args, **_kwargs): + raise AssertionError("perform_conversion should not be called") + + monkeypatch.setattr(conversion, "perform_conversion", boom) + + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + +def test_convert_file_ignores_missing_source(): + """Ignore a missing source item without crashing.""" + conversion.convert_file( + source_item_id="00000000-0000-0000-0000-000000000000", + converted_item_id="00000000-0000-0000-0000-000000000000", + user_id="00000000-0000-0000-0000-000000000000", + ) + + +def test_convert_file_ignores_missing_placeholder(): + """Ignore a missing placeholder without crashing.""" + user, source, _ = _source_and_placeholder() + + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id="00000000-0000-0000-0000-000000000000", + user_id=str(user.id), + ) diff --git a/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py b/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py new file mode 100644 index 000000000..6ec3ef659 --- /dev/null +++ b/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py @@ -0,0 +1,188 @@ +"""Tests for the OnlyOffice server-to-server conversion backend.""" + +import json +from unittest import mock + +from django.conf import settings + +import jwt +import pytest +import requests +import responses + +from core import factories +from wopi.conversion import exceptions +from wopi.conversion.backends.onlyoffice import OnlyOfficeConversionBackend + +CONVERT_URL = "https://office.example/converter" +FILE_URL = "https://office.example/result/converted.docx" +SOURCE_URL = "https://drive.example/source" +JWT_SECRET = "test-secret-with-enough-length-for-hs256" + + +def _item(filename="document.doc"): + """Build a minimal in-memory Item for backend tests.""" + return factories.ItemFactory.build(filename=filename) + + +def _ok_body(): + """Return a successful OnlyOffice /converter response body.""" + return json.dumps({"endConvert": True, "fileUrl": FILE_URL, "percent": 100}) + + +@responses.activate +def test_convert_sends_synchronous_payload_with_shardkey(): + """Send a synchronous /converter call with required fields and shardkey.""" + responses.add(responses.POST, CONVERT_URL, body=_ok_body(), status=200) + responses.add(responses.GET, FILE_URL, body=b"converted", status=200) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL) + item = _item() + + backend.convert(item, SOURCE_URL, "docx") + + request = responses.calls[0].request + payload = json.loads(request.body) + key = payload.pop("key") + assert payload == { + "async": False, + "filetype": "doc", + "outputtype": "docx", + "title": "document.doc", + "url": SOURCE_URL, + } + assert str(item.id) in key + # OnlyOffice routes synchronous conversions by the ``shardkey`` query param, + # not by the body — load balancers can pin a key to a worker. + assert f"shardkey={key}" in request.url + + +@responses.activate +def test_convert_signs_payload_when_jwt_secret_is_set(): + """Wrap the payload in a JWT and send it in body and Authorization header.""" + responses.add(responses.POST, CONVERT_URL, body=_ok_body(), status=200) + responses.add(responses.GET, FILE_URL, body=b"converted", status=200) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL, jwt_secret=JWT_SECRET) + backend.convert(_item(), SOURCE_URL, "docx") + + request = responses.calls[0].request + payload = json.loads(request.body) + token = payload.pop("token") + + decoded = jwt.decode(token, JWT_SECRET, algorithms=["HS256"]) + assert decoded["payload"] == payload + assert request.headers.get("Authorization") == f"Bearer {token}" + + +@responses.activate +def test_convert_does_not_sign_when_no_secret(): + """Skip the token field when the backend has no JWT secret.""" + responses.add(responses.POST, CONVERT_URL, body=_ok_body(), status=200) + responses.add(responses.GET, FILE_URL, body=b"converted", status=200) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL) + backend.convert(_item(), SOURCE_URL, "docx") + + request = responses.calls[0].request + payload = json.loads(request.body) + assert "token" not in payload + + +def test_constructor_raises_when_jwt_required_but_secret_missing(): + """Fail fast at construction time when JWT is required but the secret is missing.""" + with pytest.raises(exceptions.ConversionMisconfigured, match="OnlyOffice JWT is required"): + OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL, jwt_required=True) + + +@responses.activate +def test_convert_returns_content_file_with_downloaded_bytes(): + """Wrap the downloaded bytes in a ContentFile named after the target.""" + responses.add(responses.POST, CONVERT_URL, body=_ok_body(), status=200) + responses.add(responses.GET, FILE_URL, body=b"converted-bytes", status=200) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL) + result = backend.convert(_item(), SOURCE_URL, "docx") + + assert result.read() == b"converted-bytes" + assert result.name.endswith(".docx") + + +def test_convert_uses_separate_connect_and_read_timeouts(): + """Apply the configured connect/read timeouts to /converter and the download.""" + with mock.patch("wopi.conversion.backends.onlyoffice.requests") as requests_mock: + post_response = mock.Mock(ok=True) + post_response.json.return_value = {"endConvert": True, "fileUrl": FILE_URL} + get_response = mock.Mock(ok=True, content=b"x") + requests_mock.post.return_value = post_response + requests_mock.get.return_value = get_response + requests_mock.exceptions = requests.exceptions + + OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL).convert( + _item(), SOURCE_URL, "docx" + ) + + assert requests_mock.post.call_args.kwargs["timeout"] == ( + settings.WOPI_ONLYOFFICE_CONVERT_HTTP_CONNECT_TIMEOUT, + settings.WOPI_ONLYOFFICE_CONVERT_HTTP_READ_TIMEOUT, + ) + assert requests_mock.get.call_args.kwargs["timeout"] == ( + settings.WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_CONNECT_TIMEOUT, + settings.WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_READ_TIMEOUT, + ) + + +@pytest.mark.parametrize( + ("post_kwargs", "get_kwargs", "expected_message"), + [ + ({"body": requests.exceptions.Timeout()}, None, None), + ({"body": "not json", "status": 200}, None, "non-JSON body"), + ( + {"body": json.dumps({"endConvert": False, "percent": 50}), "status": 200}, + None, + "did not report a completed", + ), + ( + {"body": "boom", "status": 500}, + None, + "/converter returned status 500", + ), + ( + {"body": json.dumps({"error": -3}), "status": 200}, + None, + "error code -3", + ), + ( + {"body": _ok_body(), "status": 200}, + {"body": requests.exceptions.Timeout()}, + None, + ), + ( + {"body": _ok_body(), "status": 200}, + {"body": "boom", "status": 500}, + "file download returned status 500", + ), + ], + ids=[ + "convert_timeout", + "convert_non_json_body", + "convert_incomplete_sync_response", + "convert_5xx", + "convert_error_code", + "download_timeout", + "download_5xx", + ], +) +@responses.activate +def test_convert_maps_provider_failures_to_provider_error( + post_kwargs, get_kwargs, expected_message +): + """Map any transport or HTTP failure from OnlyOffice to ConversionProviderError.""" + responses.add(responses.POST, CONVERT_URL, **post_kwargs) + if get_kwargs is not None: + responses.add(responses.GET, FILE_URL, **get_kwargs) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL) + + with pytest.raises(exceptions.ConversionProviderError, match=expected_message): + backend.convert(_item(), SOURCE_URL, "docx") diff --git a/src/backend/wopi/tests/test_conversion_policy.py b/src/backend/wopi/tests/test_conversion_policy.py new file mode 100644 index 000000000..e26f06bd2 --- /dev/null +++ b/src/backend/wopi/tests/test_conversion_policy.py @@ -0,0 +1,74 @@ +"""Tests for the conversion policy helpers.""" + +from django.conf import settings + +from core import factories +from wopi.conversion.policy import is_forced_conversion, target_extension_for + + +def _item(filename="document.doc", mimetype="application/msword"): + """Build a minimal in-memory Item for policy tests.""" + return factories.ItemFactory.build(filename=filename, mimetype=mimetype) + + +def test_target_extension_for_known_legacy_formats(): + """Map known legacy Office extensions to their modern equivalents.""" + assert target_extension_for("doc") == "docx" + assert target_extension_for("xls") == "xlsx" + assert target_extension_for("ppt") == "pptx" + + +def test_target_extension_for_unknown_format_returns_none(): + """Return None for unsupported or empty source extensions.""" + assert target_extension_for("docx") is None + assert target_extension_for("") is None + assert target_extension_for(None) is None + + +def test_target_extension_for_is_case_insensitive(): + """Match the target extension regardless of source extension case.""" + assert target_extension_for("DOC") == "docx" + assert target_extension_for("Xls") == "xlsx" + + +def test_legacy_conversion_targets_only_lists_legacy_formats(): + """Restrict the conversion policy to legacy Office formats.""" + assert set(settings.WOPI_LEGACY_CONVERSION_TARGETS) == {"doc", "xls", "ppt"} + + +def test_is_forced_conversion_true_when_extension_listed(): + """Force conversion when the item extension is listed.""" + options = {"ForceConvertExtensions": ["doc", "xls", "ppt"]} + assert is_forced_conversion(_item(filename="document.doc"), options) is True + + +def test_is_forced_conversion_true_when_mimetype_listed(): + """Force conversion when the item mimetype is listed.""" + options = {"ForceConvertMimetypes": ["application/msword"]} + item = _item(filename="document.unknown", mimetype="application/msword") + assert is_forced_conversion(item, options) is True + + +def test_is_forced_conversion_false_when_neither_matches(): + """Skip conversion when neither extension nor mimetype matches.""" + options = { + "ForceConvertExtensions": ["doc"], + "ForceConvertMimetypes": ["application/msword"], + } + item = _item(filename="document.docx", mimetype="application/vnd.openxmlformats") + assert is_forced_conversion(item, options) is False + + +def test_is_forced_conversion_false_when_options_missing(): + """Skip conversion when client options are missing.""" + assert is_forced_conversion(_item(), {}) is False + assert is_forced_conversion(_item(), None) is False + + +def test_is_forced_conversion_extension_check_is_case_insensitive(): + """Match forced extensions regardless of case.""" + options = {"ForceConvertExtensions": ["doc"]} + assert is_forced_conversion(_item(filename="REPORT.DOC"), options) is True + + options = {"ForceConvertExtensions": ["DOC"]} + assert is_forced_conversion(_item(filename="document.doc"), options) is True diff --git a/src/backend/wopi/tests/test_conversion_services.py b/src/backend/wopi/tests/test_conversion_services.py new file mode 100644 index 000000000..3239ada6d --- /dev/null +++ b/src/backend/wopi/tests/test_conversion_services.py @@ -0,0 +1,343 @@ +"""Tests for the conversion service.""" + +from django.core.files.base import ContentFile +from django.db import DatabaseError + +import pytest + +from core import factories, models +from wopi.conversion import exceptions, services +from wopi.conversion.backends.onlyoffice import OnlyOfficeConversionBackend + +pytestmark = pytest.mark.django_db + + +class FakeBackend: + """Conversion backend returning fixed converted content.""" + + def convert(self, _item, _source_url, target_extension): + """Return fake converted content.""" + return ContentFile(b"converted", name=f"converted.{target_extension}") + + +@pytest.fixture(autouse=True) +def _fake_backend(request, monkeypatch): + """Use a fake backend except in tests that exercise backend resolution.""" + if request.node.name.startswith("test_resolve_backend"): + return + monkeypatch.setattr(services, "resolve_backend", lambda _options: FakeBackend()) + + +def _configure_wopi(settings, client="onlyoffice", options=None): + """Wire a WOPI client config covering .doc files.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example" + settings.WOPI_CLIENTS_CONFIGURATION = { + client: { + "options": options + or { + "ForceConvertExtensions": ["doc"], + "ConvertServiceUrl": "http://onlyoffice/converter", + }, + } + } + + +def _file(user, **kwargs): + """Build a ready .doc file the user can update; kwargs override defaults.""" + defaults = { + "users": [(user, models.RoleChoices.EDITOR)], + "type": models.ItemTypeChoices.FILE, + "filename": "document.doc", + "mimetype": "application/msword", + "update_upload_state": models.ItemUploadStateChoices.READY, + } + defaults.update(kwargs) + return factories.ItemFactory(**defaults) + + +def test_convert_item_creates_converted_file_copy(settings): + """Create a modern-format copy for a forced legacy file conversion.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + item = _file( + user, + parent=parent, + ) + + converted = services.convert_item(item, user) + + assert converted.filename == "document.docx" + assert converted.parent().id == parent.id + assert converted.upload_state == models.ItemUploadStateChoices.READY + + +def test_convert_item_keeps_original_file_unchanged(settings): + """Leave the source file unchanged after conversion.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, size=123) + original_parent = item.parent() + + services.convert_item(item, user) + + item.refresh_from_db() + assert item.filename == "document.doc" + assert item.parent() == original_parent + assert item.upload_state == models.ItemUploadStateChoices.READY + assert item.size == 123 + + +def test_convert_item_does_not_copy_explicit_source_accesses(settings): + """Skip copying explicit source accesses to the converted file.""" + _configure_wopi(settings) + user = factories.UserFactory() + other_user = factories.UserFactory() + item = _file(user) + factories.UserItemAccessFactory( + item=item, + user=other_user, + role=models.RoleChoices.READER, + ) + + converted = services.convert_item(item, user) + + assert not converted.accesses.filter(user=other_user).exists() + + +def test_convert_item_keeps_title_and_filename_aligned_on_collision(settings): + """Keep the converted title and filename aligned when the target name exists.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + factories.ItemFactory( + parent=parent, + type=models.ItemTypeChoices.FILE, + title="document.docx", + filename="document.docx", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + item = _file(user, parent=parent) + + converted = services.convert_item(item, user) + + assert converted.title == "document_01.docx" + assert converted.filename == "document_01.docx" + + +def test_convert_item_uses_source_parent_when_user_can_update_it_via_link(settings): + """Keep the converted file in the parent when a writable link is present.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory( + type=models.ItemTypeChoices.FOLDER, + link_reach=models.LinkReachChoices.AUTHENTICATED, + link_role=models.LinkRoleChoices.EDITOR, + ) + item = _file(user, parent=parent) + + converted = services.convert_item(item, user) + + assert converted.parent().id == parent.id + + +def test_convert_item_falls_back_to_root_when_parent_is_not_writable(settings): + """Fall back to the user's root when the parent is read-only.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + item = _file(user, parent=parent) + + converted = services.convert_item(item, user) + + assert converted.parent() is None + + +def test_convert_item_rejects_non_file_items(settings): + """Reject non-file items.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + + with pytest.raises(exceptions.ConversionRejected, match="not a file"): + services.convert_item(item, user) + + +def test_convert_item_rejects_items_that_are_not_ready(settings): + """Reject files that are not yet ready.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, update_upload_state=models.ItemUploadStateChoices.PENDING) + + with pytest.raises(exceptions.ConversionRejected, match="not ready"): + services.convert_item(item, user) + + +def test_convert_item_rejects_users_without_update_permission(settings): + """Reject users without update permission on the source file.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = factories.ItemFactory( + type=models.ItemTypeChoices.FILE, + filename="document.doc", + mimetype="application/msword", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + + with pytest.raises(exceptions.ConversionPermissionDenied, match="cannot update"): + services.convert_item(item, user) + + +def test_convert_item_rejects_unsupported_extensions(settings): + """Reject extensions outside the configured legacy set.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, filename="document.docx") + + with pytest.raises(exceptions.ConversionRejected, match="No target conversion"): + services.convert_item(item, user) + + +def test_convert_item_rejects_non_onlyoffice_clients(settings): + """Reject WOPI clients other than OnlyOffice.""" + _configure_wopi(settings, client="collabora") + user = factories.UserFactory() + item = _file(user) + + with pytest.raises(exceptions.ConversionRejected, match="No OnlyOffice client"): + services.convert_item(item, user) + + +def test_convert_item_rejects_files_without_forced_conversion(settings): + """Reject files outside the forced-conversion policy.""" + _configure_wopi(settings, options={"ForceConvertExtensions": ["xls"]}) + user = factories.UserFactory() + item = _file(user) + + with pytest.raises(exceptions.ConversionRejected, match="not forced"): + services.convert_item(item, user) + + +def test_convert_item_rejects_missing_onlyoffice_client_config(settings): + """Reject conversion when WOPI_CLIENTS_CONFIGURATION has no OnlyOffice entry.""" + settings.WOPI_CLIENTS_CONFIGURATION = {} + user = factories.UserFactory() + item = _file(user) + + with pytest.raises(exceptions.ConversionRejected, match="No OnlyOffice client"): + services.convert_item(item, user) + + +def test_resolve_backend_rejects_missing_convert_service_url(): + """Reject missing ConvertServiceUrl in OnlyOffice options.""" + with pytest.raises( + exceptions.ConversionMisconfigured, match="Missing OnlyOffice ConvertServiceUrl" + ): + services.resolve_backend({}) + + +def test_perform_conversion_removes_saved_file_when_database_save_fails(settings, monkeypatch): + """Delete saved bytes when flipping the placeholder to READY fails.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user) + placeholder = services.prepare_conversion(item, user) + saved_keys = [] + deleted_keys = [] + + class FakeStorage: + """Storage fake tracking saved and deleted keys.""" + + def save(self, key, _content): + """Track saved keys.""" + saved_keys.append(key) + return key + + def delete(self, key): + """Track deleted keys.""" + deleted_keys.append(key) + + original_save = models.Item.save + + def fail_ready_save(instance, *args, **kwargs): + """Raise when the placeholder is flipped to READY, pass otherwise.""" + if instance.id == placeholder.id and kwargs.get("update_fields"): + raise DatabaseError("database write failed") + return original_save(instance, *args, **kwargs) + + monkeypatch.setattr(services, "default_storage", FakeStorage()) + monkeypatch.setattr(models.Item, "save", fail_ready_save) + + with pytest.raises(DatabaseError, match="database write failed"): + services.perform_conversion(item, placeholder, user) + + assert deleted_keys == saved_keys + + +def test_resolve_backend_builds_onlyoffice_backend_from_options(): + """Build the OnlyOffice backend from the WOPI client options.""" + backend = services.resolve_backend( + { + "ConvertServiceUrl": "https://office.example/converter", + "ConvertJwtSecret": "test-secret", + }, + ) + + assert isinstance(backend, OnlyOfficeConversionBackend) + assert backend.convert_service_url == "https://office.example/converter" + assert backend.jwt_secret == "test-secret" + + +def test_resolve_backend_raises_when_onlyoffice_url_is_missing(): + """Raise when ConvertServiceUrl is missing from OnlyOffice options.""" + with pytest.raises( + exceptions.ConversionMisconfigured, match="Missing OnlyOffice ConvertServiceUrl" + ): + services.resolve_backend({}) + + +def test_prepare_conversion_returns_placeholder_in_converting_state(settings): + """Create the placeholder in the destination folder with upload_state CONVERTING.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + item = _file(user, parent=parent) + + placeholder = services.prepare_conversion(item, user) + + assert placeholder.filename == "document.docx" + assert placeholder.upload_state == models.ItemUploadStateChoices.CONVERTING + assert placeholder.parent().id == parent.id + + +def test_prepare_conversion_rejects_unsupported_extensions(settings): + """Skip placeholder creation for files that cannot be converted.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, filename="document.docx") + + with pytest.raises(exceptions.ConversionRejected, match="No target conversion"): + services.prepare_conversion(item, user) + + +def test_convert_item_accepts_uppercase_extension(settings): + """Convert REPORT.DOC the same way as report.doc.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, filename="REPORT.DOC") + + converted = services.convert_item(item, user) + + assert converted.filename.endswith(".docx") diff --git a/src/backend/wopi/tests/test_conversion_source_url.py b/src/backend/wopi/tests/test_conversion_source_url.py new file mode 100644 index 000000000..035e38817 --- /dev/null +++ b/src/backend/wopi/tests/test_conversion_source_url.py @@ -0,0 +1,64 @@ +"""Tests for the WOPI source-URL builder used by the conversion service.""" + +from unittest import mock + +import pytest + +from core import factories +from wopi.conversion import exceptions +from wopi.conversion.source_url import build_source_url + + +@pytest.fixture +def _access_service(monkeypatch): + """Mock the AccessUserItemService used by build_source_url.""" + service = mock.Mock() + service.return_value.insert_new_access.return_value = ("tok-abc", 1_700_000_000_000) + monkeypatch.setattr("wopi.conversion.source_url.AccessUserItemService", service) + return service + + +def test_build_source_url_uses_configured_wopi_base_url(settings, _access_service): + """Start the built URL with WOPI_SRC_BASE_URL and embed the access token.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example" + item = factories.ItemFactory.build() + user = factories.UserFactory.build() + + url = build_source_url(item, user) + + assert url == ( + f"https://drive.example/api/v1.0/wopi/files/{item.id}/contents/?access_token=tok-abc" + ) + + +def test_build_source_url_strips_trailing_slash_on_base_url(settings, _access_service): + """Strip a trailing slash on the base URL to avoid a double slash.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example/" + item = factories.ItemFactory.build() + user = factories.UserFactory.build() + + url = build_source_url(item, user) + + assert "//api" not in url + + +def test_build_source_url_raises_when_base_url_is_missing(settings, _access_service): + """Raise when WOPI_SRC_BASE_URL is missing.""" + settings.WOPI_SRC_BASE_URL = None + item = factories.ItemFactory.build() + user = factories.UserFactory.build() + + with pytest.raises(exceptions.ConversionMisconfigured, match="Missing WOPI_SRC_BASE_URL"): + build_source_url(item, user) + + +def test_build_source_url_delegates_to_access_user_item_service(settings, _access_service): + """Issue a short-lived WOPI access token via the existing service.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example" + settings.WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT = 90 + item = factories.ItemFactory.build() + user = factories.UserFactory.build() + + build_source_url(item, user) + + _access_service.return_value.insert_new_access.assert_called_once_with(item, user, ttl=90) diff --git a/src/backend/wopi/tests/test_utils.py b/src/backend/wopi/tests/test_utils.py index 8a098e791..e1e297311 100644 --- a/src/backend/wopi/tests/test_utils.py +++ b/src/backend/wopi/tests/test_utils.py @@ -352,6 +352,29 @@ def test_get_wopi_client_config(): assert get_wopi_client_config(item, user) is None +def test_get_wopi_client_config_normalizes_uppercase_extension(): + """Match the WOPI client config when the item extension is uppercase.""" + cache.set( + WOPI_CONFIGURATION_CACHE_KEY, + { + "mimetypes": {}, + "extensions": { + "doc": {"url": "https://vendorA.com/launch_url", "client": "vendorA"}, + }, + }, + ) + user = UserFactory() + item = ItemFactory( + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.READY, + filename="REPORT.DOC", + ) + assert get_wopi_client_config(item, user) == { + "url": "https://vendorA.com/launch_url", + "client": "vendorA", + } + + @pytest.mark.parametrize( "upload_state", [ diff --git a/src/backend/wopi/utils/__init__.py b/src/backend/wopi/utils/__init__.py index 4893cf9e5..bf2834b09 100644 --- a/src/backend/wopi/utils/__init__.py +++ b/src/backend/wopi/utils/__init__.py @@ -39,9 +39,11 @@ def get_wopi_client_config(item, user): return None result = None - # Extension must always be checked first. - if item.extension in wopi_configuration["extensions"]: - result = wopi_configuration["extensions"][item.extension] + # Extension must always be checked first. Filenames preserve case (REPORT.DOC), + # while the discovery stores extensions in lowercase, so normalize the lookup. + extension = item.extension.lower() if item.extension else None + if extension and extension in wopi_configuration["extensions"]: + result = wopi_configuration["extensions"][extension] elif item.mimetype in wopi_configuration["mimetypes"]: result = wopi_configuration["mimetypes"][item.mimetype] diff --git a/src/frontend/apps/drive/src/features/drivers/Driver.ts b/src/frontend/apps/drive/src/features/drivers/Driver.ts index fba2a84a3..2cbc8c74a 100644 --- a/src/frontend/apps/drive/src/features/drivers/Driver.ts +++ b/src/frontend/apps/drive/src/features/drivers/Driver.ts @@ -159,6 +159,7 @@ export abstract class Driver { abstract deleteItems(ids: string[]): Promise; abstract hardDeleteItems(ids: string[]): Promise; abstract getWopiInfo(itemId: string): Promise; + abstract convertItem(itemId: string): Promise; abstract getEntitlements(): Promise; diff --git a/src/frontend/apps/drive/src/features/drivers/implementations/StandardDriver.ts b/src/frontend/apps/drive/src/features/drivers/implementations/StandardDriver.ts index 25cfd5a5b..0f6d75d0f 100644 --- a/src/frontend/apps/drive/src/features/drivers/implementations/StandardDriver.ts +++ b/src/frontend/apps/drive/src/features/drivers/implementations/StandardDriver.ts @@ -454,6 +454,13 @@ export class StandardDriver extends Driver { return jsonToItem(await response.json()); } + async convertItem(itemId: string): Promise { + const response = await fetchAPI(`items/${itemId}/convert/`, { + method: "POST", + }); + return jsonToItem(await response.json()); + } + async deleteItems(ids: string[]): Promise { for (const id of ids) { await fetchAPI(`items/${id}/`, { diff --git a/src/frontend/apps/drive/src/features/drivers/types.ts b/src/frontend/apps/drive/src/features/drivers/types.ts index 7a6b20ac2..1744e0c1a 100644 --- a/src/frontend/apps/drive/src/features/drivers/types.ts +++ b/src/frontend/apps/drive/src/features/drivers/types.ts @@ -20,12 +20,18 @@ export enum LinkRole { export enum ItemUploadState { PENDING = "pending", DUPLICATING = "duplicating", + CONVERTING = "converting", ANALYZING = "analyzing", SUSPICIOUS = "suspicious", FILE_TOO_LARGE_TO_ANALYZE = "file_too_large_to_analyze", READY = "ready", } +export const TRANSIENT_UPLOAD_STATES: string[] = [ + ItemUploadState.DUPLICATING, + ItemUploadState.CONVERTING, +]; + export type ItemBreadcrumb = { id: string; originalId?: string; // Used to identify all occurrences of the same item in the tree @@ -93,6 +99,7 @@ export type Item = { tree: boolean; update: boolean; upload_ended: boolean; + must_convert?: boolean; }; policy?: string; }; diff --git a/src/frontend/apps/drive/src/features/explorer/components/app-view/AppExplorerGrid.tsx b/src/frontend/apps/drive/src/features/explorer/components/app-view/AppExplorerGrid.tsx index 206d61b9e..bac36f55a 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/app-view/AppExplorerGrid.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/app-view/AppExplorerGrid.tsx @@ -13,11 +13,14 @@ import { import { InfiniteScroll } from "@/features/ui/components/infinite-scroll/InfiniteScroll"; import { useRouter } from "next/router"; import { DefaultRoute, getDefaultRouteId } from "@/utils/defaultRoutes"; -import { useMemo } from "react"; +import { useMemo, useState } from "react"; import { canCreateChildren } from "@/features/items/utils"; import { Spinner } from "@gouvfr-lasuite/ui-kit"; import { openWopiInNewTab } from "@/features/wopi/openWopi"; import { itemToPreviewFile } from "@/features/explorer/utils/utils"; +import { useModal } from "@gouvfr-lasuite/cunningham-react"; +import { ConvertLegacyFileModal } from "@/features/explorer/components/modals/ConvertLegacyFileModal"; +import { useRefreshQueryCacheAfterMutation } from "@/features/explorer/hooks/useRefreshItems"; /** * Wrapper around EmbeddedExplorerGrid to display a list of items in a table. @@ -45,9 +48,18 @@ export const AppExplorerGrid = () => { const effectiveOnNavigate = appExplorer.onNavigate ?? onNavigate; + const convertModal = useModal(); + const [itemToConvert, setItemToConvert] = useState(null); + const refresh = useRefreshQueryCacheAfterMutation(); + const handleFileClick = appExplorer.onFileClick ?? ((item: Item) => { + if (item.abilities.must_convert) { + setItemToConvert(item); + convertModal.open(); + return; + } if (item.is_wopi_supported) { openWopiInNewTab(itemToPreviewFile(item)); return; @@ -171,6 +183,14 @@ export const AppExplorerGrid = () => { )} + {convertModal.isOpen && itemToConvert && ( + refresh(item?.originalId ?? item?.id)} + /> + )} ); }; diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx index 6955e998d..f2d4aa02b 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx @@ -1,4 +1,8 @@ -import { Item, ItemType, ItemUploadState } from "@/features/drivers/types"; +import { + Item, + ItemType, + TRANSIENT_UPLOAD_STATES, +} from "@/features/drivers/types"; import { createContext, useCallback, @@ -41,7 +45,7 @@ import { } from "../../types/columns"; import { ColumnHeader } from "./headers/ColumnHeader"; import { CustomizableColumnHeader } from "./headers/CustomizableColumnHeader"; -import { useDuplicatingItemsPoller } from "../../hooks/useDuplicatingItemsPoller"; +import { useTransientItemsPoller } from "../../hooks/useTransientItemsPoller"; import { EmbeddedExplorerGridRow } from "./EmbeddedExplorerGridRow"; import posthog from "posthog-js"; @@ -124,7 +128,7 @@ export const EmbeddedExplorerGrid = (props: EmbeddedExplorerGridProps) => { }); const contextMenu = useContextMenuContext(); - useDuplicatingItemsPoller(props.items ?? EMPTY_ARRAY); + useTransientItemsPoller(props.items ?? EMPTY_ARRAY); const selectionStore = useSelectionStore(); // TODO: This hook makes use of the ExplorerContext to manage the overred items. So, this component is not really standalone as it should be. @@ -296,7 +300,7 @@ export const EmbeddedExplorerGrid = (props: EmbeddedExplorerGridProps) => { const handleRowClick = useCallback( (e: React.MouseEvent, row: Row) => { - if (row.original.upload_state === ItemUploadState.DUPLICATING) { + if (TRANSIENT_UPLOAD_STATES.includes(row.original.upload_state)) { return; } @@ -355,7 +359,7 @@ export const EmbeddedExplorerGrid = (props: EmbeddedExplorerGridProps) => { e.preventDefault(); e.stopPropagation(); - if (row.original.upload_state === ItemUploadState.DUPLICATING) { + if (TRANSIENT_UPLOAD_STATES.includes(row.original.upload_state)) { return; } diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridActionsCell.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridActionsCell.tsx index 642cab23b..034349454 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridActionsCell.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridActionsCell.tsx @@ -1,5 +1,5 @@ import { CellContext } from "@tanstack/react-table"; -import { Item, ItemUploadState } from "@/features/drivers/types"; +import { Item, TRANSIENT_UPLOAD_STATES } from "@/features/drivers/types"; import { memo, useState } from "react"; import { Button } from "@gouvfr-lasuite/cunningham-react"; import { Draggable } from "@/features/explorer/components/Draggable"; @@ -21,7 +21,7 @@ const EmbeddedExplorerGridActionsCellComponent = ( const { setIsActionModalOpen, isActionModalOpen } = useEmbeddedExplorerGirdContext(); - if (item.upload_state === ItemUploadState.DUPLICATING) { + if (TRANSIENT_UPLOAD_STATES.includes(item.upload_state)) { return null; } diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridMobileCell.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridMobileCell.tsx index 28bf4a087..9c78b2de5 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridMobileCell.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridMobileCell.tsx @@ -1,5 +1,9 @@ import { CellContext } from "@tanstack/react-table"; -import { Item, ItemUploadState } from "@/features/drivers/types"; +import { + Item, + ItemUploadState, + TRANSIENT_UPLOAD_STATES, +} from "@/features/drivers/types"; import { memo } from "react"; import { ItemIcon } from "@/features/explorer/components/icons/ItemIcon"; import { timeAgo } from "@/features/explorer/utils/utils"; @@ -15,11 +19,15 @@ const EmbeddedExplorerGridMobileCellComponent = ( ) => { const item = params.row.original; const { t } = useTranslation(); - const isDuplicating = item.upload_state === ItemUploadState.DUPLICATING; + const isConverting = item.upload_state === ItemUploadState.CONVERTING; + const isTransient = TRANSIENT_UPLOAD_STATES.includes(item.upload_state); + const transientLabel = isConverting + ? t("explorer.item.converting") + : t("explorer.item.duplicating"); return (
- {isDuplicating ? ( + {isTransient ? (
@@ -30,14 +38,14 @@ const EmbeddedExplorerGridMobileCellComponent = (
{removeFileExtension(item.title)} - {isDuplicating && ( + {isTransient && ( {" "} - ({t("explorer.item.duplicating")}) + ({transientLabel}) )} diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridNameCell.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridNameCell.tsx index d0192af30..99f7ac9b8 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridNameCell.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridNameCell.tsx @@ -1,5 +1,10 @@ import { CellContext } from "@tanstack/react-table"; -import { Item, ItemUploadState, LinkReach } from "@/features/drivers/types"; +import { + Item, + ItemUploadState, + LinkReach, + TRANSIENT_UPLOAD_STATES, +} from "@/features/drivers/types"; import { memo, useEffect, useMemo, useRef, useState } from "react"; import { Draggable } from "@/features/explorer/components/Draggable"; import { Tooltip } from "@gouvfr-lasuite/cunningham-react"; @@ -24,7 +29,11 @@ const EmbeddedExplorerGridNameCellComponent = ( const [isOverflown, setIsOverflown] = useState(false); const { disableItemDragAndDrop } = useEmbeddedExplorerGirdContext(); const isSelected = useIsItemSelected(item.id); - const isDuplicating = item.upload_state === ItemUploadState.DUPLICATING; + const isConverting = item.upload_state === ItemUploadState.CONVERTING; + const isTransient = TRANSIENT_UPLOAD_STATES.includes(item.upload_state); + const transientLabel = isConverting + ? t("explorer.item.converting") + : t("explorer.item.duplicating"); const disableDrag = useDisableDragGridItem(item); @@ -36,20 +45,20 @@ const EmbeddedExplorerGridNameCellComponent = ( id={params.cell.id + "-title"} item={item} className="explorer__grid__item__name__title-wrapper" - disabled={isDuplicating || disableItemDragAndDrop || isSelected} // If it's selected then we can drag on the entire cell + disabled={isTransient || disableItemDragAndDrop || isSelected} // If it's selected then we can drag on the entire cell >
{removeFileExtension(item.title)} - {isDuplicating && ( + {isTransient && ( {" "} - ({t("explorer.item.duplicating")}) + ({transientLabel}) )} {params.children} @@ -90,14 +99,14 @@ const EmbeddedExplorerGridNameCellComponent = (
- {isDuplicating ? ( + {isTransient ? (
diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridRow.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridRow.tsx index 5a8cb1a22..af417d135 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridRow.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridRow.tsx @@ -1,7 +1,7 @@ import { memo } from "react"; import { Row, flexRender } from "@tanstack/react-table"; import clsx from "clsx"; -import { Item, ItemType, ItemUploadState } from "@/features/drivers/types"; +import { Item, ItemType, TRANSIENT_UPLOAD_STATES } from "@/features/drivers/types"; import { Droppable } from "@/features/explorer/components/Droppable"; import { useIsItemSelected } from "@/features/explorer/stores/selectionStore"; @@ -25,14 +25,15 @@ const EmbeddedExplorerGridRowComponent = ({ }: EmbeddedExplorerGridRowProps) => { const item = row.original; const isSelected = useIsItemSelected(item.id); + const isTransient = TRANSIENT_UPLOAD_STATES.includes(item.upload_state); return ( void; + onSuccess: () => void; +}; + +export const ConvertLegacyFileModal = ({ item, isOpen, onClose, onSuccess }: Props) => { + const { t } = useTranslation(); + const [isSubmitting, setIsSubmitting] = useState(false); + const [error, setError] = useState(null); + + const handleClose = () => { + if (isSubmitting) return; + setError(null); + onClose(); + }; + + const handleConvert = async () => { + setIsSubmitting(true); + setError(null); + try { + await getDriver().convertItem(item.id); + onSuccess(); + onClose(); + } catch { + setError(t("explorer.actions.convert.modal.error")); + setIsSubmitting(false); + } + }; + + return ( + + + + + } + > +
+ {error ? ( +

{error}

+ ) : ( + t("explorer.actions.convert.modal.content") + )} +
+ + ); +}; diff --git a/src/frontend/apps/drive/src/features/explorer/hooks/useDuplicatingItemsPoller.ts b/src/frontend/apps/drive/src/features/explorer/hooks/useTransientItemsPoller.ts similarity index 64% rename from src/frontend/apps/drive/src/features/explorer/hooks/useDuplicatingItemsPoller.ts rename to src/frontend/apps/drive/src/features/explorer/hooks/useTransientItemsPoller.ts index 466dcee0c..d412a909c 100644 --- a/src/frontend/apps/drive/src/features/explorer/hooks/useDuplicatingItemsPoller.ts +++ b/src/frontend/apps/drive/src/features/explorer/hooks/useTransientItemsPoller.ts @@ -1,48 +1,49 @@ import { useMemo, useEffect, useRef } from "react"; import { useQueries } from "@tanstack/react-query"; import { getDriver } from "@/features/config/Config"; -import { Item, ItemUploadState } from "@/features/drivers/types"; +import { Item, TRANSIENT_UPLOAD_STATES } from "@/features/drivers/types"; import { useRefreshItemCache } from "./useRefreshItems"; const POLL_INTERVAL = 3000; const POLL_TIMEOUT = 10 * 60 * 1000; -export const useDuplicatingItemsPoller = (items: Item[]) => { +export const useTransientItemsPoller = (items: Item[]) => { const refreshItemCache = useRefreshItemCache(); const startTimesRef = useRef>(new Map()); - const duplicatingItems = useMemo( - () => items.filter((i) => i.upload_state === ItemUploadState.DUPLICATING), + const transientItems = useMemo( + () => + items.filter((i) => TRANSIENT_UPLOAD_STATES.includes(i.upload_state)), [items], ); useEffect(() => { - for (const item of duplicatingItems) { + for (const item of transientItems) { if (!startTimesRef.current.has(item.id)) { startTimesRef.current.set(item.id, Date.now()); } } - const duplicatingIds = new Set(duplicatingItems.map((i) => i.id)); + const transientIds = new Set(transientItems.map((i) => i.id)); for (const id of startTimesRef.current.keys()) { - if (!duplicatingIds.has(id)) { + if (!transientIds.has(id)) { startTimesRef.current.delete(id); } } - }, [duplicatingItems]); + }, [transientItems]); useQueries({ - queries: duplicatingItems.map((item) => ({ - queryKey: ["items", item.id, "duplicate-poll"], + queries: transientItems.map((item) => ({ + queryKey: ["items", item.id, "transient-poll"], queryFn: async () => { const updatedItem = await getDriver().getItem(item.id); - if (updatedItem.upload_state !== ItemUploadState.DUPLICATING) { + if (!TRANSIENT_UPLOAD_STATES.includes(updatedItem.upload_state)) { await refreshItemCache(item.id, updatedItem); } return updatedItem; }, refetchInterval: (query: { state: { data: Item | undefined } }) => { const data = query.state.data; - if (data && data.upload_state !== ItemUploadState.DUPLICATING) { + if (data && !TRANSIENT_UPLOAD_STATES.includes(data.upload_state)) { return false; } const startTime = startTimesRef.current.get(item.id) ?? Date.now(); diff --git a/src/frontend/apps/drive/src/features/i18n/translations.json b/src/frontend/apps/drive/src/features/i18n/translations.json index 4b6614ca2..ec3597470 100644 --- a/src/frontend/apps/drive/src/features/i18n/translations.json +++ b/src/frontend/apps/drive/src/features/i18n/translations.json @@ -483,6 +483,15 @@ "cancel": "Cancel", "submit": "Rename" } + }, + "convert": { + "modal": { + "title": "Convert to open this file", + "content": "Drive will create an editable copy. Your original file stays untouched.", + "cancel": "Cancel", + "confirm": "Convert", + "error": "Conversion failed. Please try again." + } } }, "item": { @@ -501,7 +510,8 @@ "duplicate_error": "An error occurred while duplicating the item.", "delete": "Delete" }, - "duplicating": "duplication in progress" + "duplicating": "duplication in progress", + "converting": "conversion in progress" } }, "time": { @@ -1073,6 +1083,15 @@ "cancel": "Annuler", "submit": "Renommer" } + }, + "convert": { + "modal": { + "title": "Convertir pour ouvrir ce fichier", + "content": "Drive va créer une copie modifiable. Votre fichier d'origine reste intact.", + "cancel": "Annuler", + "confirm": "Convertir", + "error": "La conversion a échoué. Veuillez réessayer." + } } }, "item": { @@ -1091,7 +1110,8 @@ "duplicate_error": "Une erreur est survenue lors de la duplication.", "delete": "Supprimer" }, - "duplicating": "duplication en cours" + "duplicating": "duplication en cours", + "converting": "conversion en cours" } }, "time": { @@ -1657,6 +1677,15 @@ "cancel": "Annuleren", "submit": "Hernoem" } + }, + "convert": { + "modal": { + "title": "Converteren om dit bestand te openen", + "content": "Drive maakt een bewerkbare kopie. Uw originele bestand blijft ongewijzigd.", + "cancel": "Annuleren", + "confirm": "Converteren", + "error": "Conversie mislukt. Probeer het opnieuw." + } } }, "item": { @@ -1675,7 +1704,8 @@ "duplicate_error": "Er is een fout opgetreden bij het dupliceren.", "delete": "Verwijder" }, - "duplicating": "duplicatie bezig" + "duplicating": "duplicatie bezig", + "converting": "conversie bezig" } }, "time": { diff --git a/src/frontend/apps/e2e/__tests__/app-drive/wopi.spec.ts b/src/frontend/apps/e2e/__tests__/app-drive/wopi.spec.ts index fc20b6a84..fd041c9a4 100644 --- a/src/frontend/apps/e2e/__tests__/app-drive/wopi.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-drive/wopi.spec.ts @@ -1,5 +1,5 @@ import path from "path"; -import test, { expect } from "@playwright/test"; +import test, { expect, Page } from "@playwright/test"; import { clearDb, login } from "./utils-common"; import { clickToMyFiles } from "./utils-navigate"; import { getRowItem } from "./utils-embedded-grid"; @@ -9,6 +9,165 @@ import { grantClipboardPermissions } from "./utils/various-utils"; const IMAGE_FILE_PATH = path.join(__dirname, "/assets/test-image.png"); const DOCX_FILE_PATH = path.join(__dirname, "/assets/empty_doc.docx"); +/** + * Intercepts items API responses to inject must_convert: true on all items. + */ +const mockRequiresConversion = async (page: Page) => { + const inject = (item: Record) => { + item.abilities = { + ...(item.abilities as Record), + must_convert: true, + }; + }; + await page.route( + (url) => /\/api\/v1\.0\/items(\/|$)/.test(url.pathname), + async (route) => { + if (route.request().method() !== "GET") { + await route.continue(); + return; + } + const response = await route.fetch(); + const json = await response.json(); + if (Array.isArray(json?.results)) { + json.results.forEach(inject); + } else if (json?.id) { + inject(json); + } + await route.fulfill({ response, json }); + }, + ); +}; + +test("Double-clicking a file with must_convert ability opens the conversion modal", async ({ + page, + browserName, +}) => { + test.skip(browserName !== "chromium", "Only runs on chromium"); + await clearDb(); + await login(page, "drive@example.com"); + await mockRequiresConversion(page); + await page.goto("/"); + await clickToMyFiles(page); + await expect(page.getByText("This tab is empty")).toBeVisible(); + + await uploadFile(page, DOCX_FILE_PATH); + await expect(page.getByText("Drop your files here")).not.toBeVisible(); + + const row = await getRowItem(page, "empty_doc"); + await row.dblclick(); + + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).toBeVisible(); +}); + +test("Cancelling the conversion modal (must_convert mocked) does not call the convert API", async ({ + page, + browserName, +}) => { + test.skip(browserName !== "chromium", "Only runs on chromium"); + await clearDb(); + await login(page, "drive@example.com"); + await mockRequiresConversion(page); + await page.goto("/"); + await clickToMyFiles(page); + await expect(page.getByText("This tab is empty")).toBeVisible(); + + await uploadFile(page, DOCX_FILE_PATH); + await expect(page.getByText("Drop your files here")).not.toBeVisible(); + + let convertCalled = false; + await page.route("**/api/v1.0/items/*/convert/", async (route) => { + convertCalled = true; + await route.continue(); + }); + + const row = await getRowItem(page, "empty_doc"); + await row.dblclick(); + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).toBeVisible(); + + await page.getByRole("button", { name: "Cancel" }).click(); + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).not.toBeVisible(); + expect(convertCalled).toBe(false); +}); + +test("Confirming the conversion modal (must_convert mocked) shows the converting placeholder in the folder", async ({ + page, + browserName, +}) => { + test.skip(browserName !== "chromium", "Only runs on chromium"); + await clearDb(); + await login(page, "drive@example.com"); + await mockRequiresConversion(page); + await page.goto("/"); + await clickToMyFiles(page); + await expect(page.getByText("This tab is empty")).toBeVisible(); + + await uploadFile(page, DOCX_FILE_PATH); + await expect(page.getByText("Drop your files here")).not.toBeVisible(); + + // The POST /convert/ now returns the placeholder Item directly. We mock it + // and let the front-end refresh the folder so the placeholder appears. + const placeholderId = "00000000-0000-0000-0000-000000000001"; + await page.route("**/api/v1.0/items/*/convert/", async (route) => { + await route.fulfill({ + status: 201, + contentType: "application/json", + body: JSON.stringify({ + id: placeholderId, + title: "empty_doc.docx", + filename: "empty_doc.docx", + type: "file", + upload_state: "converting", + abilities: {}, + }), + }); + }); + + // Inject the placeholder in the folder list returned by /items/ so it shows + // up after the post-conversion refresh. + await page.route( + (url) => /\/api\/v1\.0\/items(\/|$)/.test(url.pathname), + async (route) => { + if (route.request().method() !== "GET") { + await route.continue(); + return; + } + const response = await route.fetch(); + const json = await response.json(); + const placeholder = { + id: placeholderId, + title: "empty_doc.docx", + filename: "empty_doc.docx", + type: "file", + upload_state: "converting", + abilities: {}, + }; + if (Array.isArray(json?.results)) { + json.results.push(placeholder); + } + await route.fulfill({ response, json }); + }, + ); + + const row = await getRowItem(page, "empty_doc"); + await row.dblclick(); + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).toBeVisible(); + + await page.getByRole("button", { name: "Convert" }).click(); + + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).not.toBeVisible({ timeout: 5000 }); + await expect(page.getByText("conversion in progress")).toBeVisible(); +}); + test("Double-clicking a WOPI file opens the editor in a new tab", async ({ page, context,