Skip to content

Wopi/background conversion poc#719

Open
kernicPanel wants to merge 20 commits into
mainfrom
wopi/background-conversion-poc
Open

Wopi/background conversion poc#719
kernicPanel wants to merge 20 commits into
mainfrom
wopi/background-conversion-poc

Conversation

@kernicPanel
Copy link
Copy Markdown
Contributor

@kernicPanel kernicPanel commented May 27, 2026

Purpose

Convert .doc / .xls / .ppt to their modern equivalents in the
background, without opening an editor. Double-clicking a legacy file
shows a confirmation modal; on confirm, a placeholder item appears as
"converting" in the listing and is replaced by the converted file when
ready.

Proposal

Backend

  • POST /api/v1.0/items/{id}/convert/ — creates a placeholder Item in
    the new CONVERTING upload state at the destination, queues
    convert_file on Celery, returns the placeholder id. Cleans up and
    returns 503 on broker failure.
  • wopi/conversion/ — policy (doc → docx, xls → xlsx, ppt → pptx),
    OnlyOfficeConversionBackend driving /converter with separate
    connect/read timeouts and optional HS256 JWT.
  • Short-lived WOPI source URL signed with the existing
    AccessUserItemService (WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT,
    default 120s).
  • New convert ability on Item, aligned with update.
  • Case-insensitive WOPI extension lookup.

Frontend

  • Confirmation modal on double-click of a legacy file.
  • Placeholder rendered as a transient "converting" row.
  • Duplicating poller generalised to transient items.
  • API client for the convert endpoint.

@kernicPanel kernicPanel force-pushed the wopi/background-conversion-poc branch 9 times, most recently from 533ebae to 1c54c2a Compare May 28, 2026 10:57
conversion_exceptions.ConversionRejected,
conversion_exceptions.ConversionMisconfigured,
) as exc:
raise drf.exceptions.ValidationError({"detail": str(exc)}) from exc
@kernicPanel kernicPanel force-pushed the wopi/background-conversion-poc branch from 1c54c2a to b51e8ab Compare May 28, 2026 12:53
Legacy Office formats need an explicit conversion policy before they can be
opened for editing.

Keep the mapping small and configuration-driven so the backend only converts
formats intentionally forced by the WOPI client.
Drive needs a server-side path to convert legacy Office files without opening an
interactive editor session.

Use OnlyOffice's conversion API as the first supported provider for the POC.
OnlyOffice must fetch the source bytes from Drive during server-side conversion.

Use short-lived WOPI access URLs so conversion does not depend on direct object
storage reachability.
Converted files should appear in the folder immediately while Celery is still
working.

Add a dedicated transient state instead of overloading upload or duplication
states.
Users need immediate feedback in the current folder while conversion continues
in the background.

Create the target item up front and complete it once converted bytes are
available.
The explorer needs to know when opening a legacy Office file should start
conversion instead of WOPI editing.

Expose the decision on item abilities so the frontend can keep the flow item-
based.
Starting a conversion should be quick for the API caller and continue in the
background.

Queue the placeholder-backed conversion task from the item endpoint.
Uploaded filenames preserve extension case, while WOPI discovery stores
extension keys in lowercase.

Normalize the lookup so legacy files like REPORT.DOC follow the same conversion
path.
The frontend needs to treat conversion as item creation rather than job polling.

Return the placeholder item from the driver so UI state stays aligned with the
explorer list.
Users should understand that opening a legacy file creates an editable copy.

Keep the modal focused on confirmation and let the folder show the ongoing
conversion state.
@kernicPanel kernicPanel force-pushed the wopi/background-conversion-poc branch 3 times, most recently from bf9bb3e to 131b11a Compare May 28, 2026 16:01
@kernicPanel kernicPanel marked this pull request as ready for review May 28, 2026 16:24
A converting file is visible but not ready for normal file actions.

Show the transient state consistently and prevent opening or acting on the item
until conversion completes.
The same polling loop fits any transient upload state.

Rename the duplicating poller to a transient one driven by
TRANSIENT_UPLOAD_STATES so converting items are picked up without copying
the hook, and the explorer refreshes automatically when background
conversion finishes.
The conversion flow crosses modal state, API calls, and explorer refreshes.

Cover the expected user path with a mocked conversion ability and placeholder
response.
@kernicPanel kernicPanel force-pushed the wopi/background-conversion-poc branch from 131b11a to c067374 Compare May 28, 2026 16:26
Copy link
Copy Markdown
Member

@lunika lunika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a global comment, I think you can replace all the usage of mokeypatch by mock.patch.

return False

extension = _normalize(item.extension)
forced_extensions = {_normalize(e) for e in client_options.get("ForceConvertExtensions") or []}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ForceConvertExtensions is created in the next commits, it is confusing to use code you will create in a future commit.

@@ -0,0 +1,189 @@
"""Tests for the OnlyOffice server-to-server conversion backend."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be in a conversion module wopi/tests/conversion/test_backends_onlyoffice.py

Comment thread src/backend/wopi/tests/test_conversion_backends_onlyoffice.py Outdated
Comment thread src/backend/wopi/tests/test_conversion_source_url.py Outdated
Comment thread src/backend/wopi/conversion/services.py Outdated
Comment thread src/backend/core/api/permissions.py Outdated
Comment thread src/backend/core/api/viewsets.py Outdated
Comment on lines +55 to +61
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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little bit abrupt. With a retry strategy, you can try the conversion multiple times before deleting the item.

Comment on lines +34 to +45
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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO you should use mock.path instead of monkeypath. The mock will allow you to change the implementation and have built assertion to check how the patch has been called.

Comment thread src/backend/wopi/apps.py
Comment on lines +11 to +14
def ready(self):
"""Import celery tasks."""
# pylint: disable=import-outside-toplevel, unused-import
from .tasks import conversion # noqa: PLC0415,F401
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With what is made in the drive.celery_app module, this code should be useless.

@kernicPanel kernicPanel force-pushed the wopi/background-conversion-poc branch from e323f8d to 338bef7 Compare June 1, 2026 16:34
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 1, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants