✨(backend) duplicate a document through the collaboration server

its stateis fetched from yhub and seeded into the copy instead
of being copied from the content stored by Django

CHANGELOG.md

@@ -99,6 +99,7 @@ and this project adheres to
   instead of giving up. The built-in endpoints can also answer JSON on
   `Accept: application/json`
 - ✨(collaboration) add a get-ydoc endpoint on yhub
+- ✨(backend) duplicate a document through the collaboration server
 - ✨(backend) call YHubService to seed initial document content
 - ✨(backend) reset the yhub connections of a document and its descendants
   when an access or the link configuration changes

src/backend/core/api/viewsets.py

@@ -81,7 +81,7 @@ from core.utils.s3 import get_s3_client
 from core.utils.s3_response_stream import content_stream
 from core.utils.treebeard import create_tree_node_with_retry
 from core.utils.users import users_sharing_documents_with
-from core.utils.yjs import extract_attachments
+from core.utils.yjs import extract_attachments, extract_attachments_from_update
 
 from ..enums import FeatureFlag, SearchType
 from . import permissions, serializers, utils
@@ -757,6 +757,45 @@ class DocumentViewSet(
                 {"file": ["Could not save the imported file content"]}
             ) from err
 
+    def _get_collaboration_document(self, document):
+        """
+        Return the content of a document, as held by the collaboration server.
+
+        It is the source of truth for the content, the one Django may still
+        store is ignored. A document it holds nothing for has no content to
+        copy, it answers None.
+        """
+        try:
+            return YHubService(user=self.request.user).get_ydoc(document)
+        except YHubError as err:
+            logger.error(
+                "could not fetch the content of document %s with error: %s",
+                document.id,
+                err,
+            )
+            raise drf.exceptions.APIException(
+                "Failed to fetch the document content"
+            ) from err
+
+    def _copy_collaboration_document(self, document, update):
+        """
+        Seed a duplicated document with the content of the one it copies.
+
+        The duplicate is worthless without it, so a failure is reported to the
+        caller rather than leaving an empty copy behind.
+        """
+        try:
+            YHubService(user=self.request.user).create_ydoc(document, update)
+        except YHubError as err:
+            logger.error(
+                "could not copy the content into document %s with error: %s",
+                document.id,
+                err,
+            )
+            raise drf.exceptions.APIException(
+                "Failed to duplicate the document content"
+            ) from err
+
     def perform_create(self, serializer):
         """Set the current user as creator and owner of the newly created object."""
 
@@ -1286,11 +1325,12 @@ class DocumentViewSet(
         serializer.is_valid(raise_exception=True)
         user = request.user
 
-        duplicated_document = self._duplicate_document(
-            document_to_duplicate=document_to_duplicate,
-            serializer=serializer,
-            user=user,
-        )
+        with transaction.atomic():
+            duplicated_document = self._duplicate_document(
+                document_to_duplicate=document_to_duplicate,
+                serializer=serializer,
+                user=user,
+            )
 
         posthog_capture(
             PosthogEventName.DOC_DUPLICATED,
@@ -1332,7 +1372,9 @@ class DocumentViewSet(
         user_role = document_to_duplicate.get_role(user)
         is_owner_or_admin = user_role in models.PRIVILEGED_ROLES
 
-        base64_yjs_content = document_to_duplicate.content or ""
+        # The collaboration server holds the content, the duplicate is seeded
+        # with it once it exists
+        ydoc_update = self._get_collaboration_document(document_to_duplicate)
 
         # Duplicate the document instance
         link_kwargs = (
@@ -1343,7 +1385,7 @@ class DocumentViewSet(
             if with_accesses
             else {}
         )
-        extracted_attachments = set(extract_attachments(document_to_duplicate.content))
+        extracted_attachments = set(extract_attachments_from_update(ydoc_update))
         attachments = list(
             extracted_attachments & set(document_to_duplicate.attachments)
         )
@@ -1352,7 +1394,6 @@ class DocumentViewSet(
         if new_parent is not None:
             duplicated_document = new_parent.add_child(
                 title=title,
-                content=base64_yjs_content,
                 attachments=attachments,
                 duplicated_from=document_to_duplicate,
                 creator=user,
@@ -1384,7 +1425,6 @@ class DocumentViewSet(
             duplicated_document = models.Document.add_root(
                 creator=user,
                 title=title,
-                content=base64_yjs_content,
                 attachments=attachments,
                 duplicated_from=document_to_duplicate,
                 **link_kwargs,
@@ -1398,7 +1438,6 @@ class DocumentViewSet(
             duplicated_document = document_to_duplicate.add_sibling(
                 "last-sibling",
                 title=title,
-                content=base64_yjs_content,
                 attachments=attachments,
                 duplicated_from=document_to_duplicate,
                 creator=user,
@@ -1435,6 +1474,11 @@ class DocumentViewSet(
                 # Bulk create all the duplicated accesses
                 models.DocumentAccess.objects.bulk_create(accesses_to_create)
 
+        # the accesses exist by now, so the content is only served to the users
+        # the duplicate is meant for
+        if ydoc_update:
+            self._copy_collaboration_document(duplicated_document, ydoc_update)
+
         if with_descendants:
             for child in document_to_duplicate.get_children().filter(
                 ancestors_deleted_at__isnull=True

src/backend/core/tests/documents/test_api_documents_duplicate.py

@@ -19,9 +19,30 @@ from freezegun import freeze_time
 from rest_framework.test import APIClient
 
 from core import factories, models
+from core.services.yhub_services import (
+    ServiceUnavailableError as YHubServiceUnavailableError,
+)
 
 pytestmark = pytest.mark.django_db
 
+
+@pytest.fixture(autouse=True, name="mock_yhub")
+def mock_yhub_fixture():
+    """
+    The content of a document is held by the collaboration server.
+
+    It stands for a server holding the very content the factories gave the
+    documents, which is what an editor connected to it would have saved.
+    """
+
+    def get_ydoc(document):
+        return base64.b64decode(document.content) if document.content else None
+
+    with mock.patch("core.api.viewsets.YHubService") as mock_service:
+        mock_service.return_value.get_ydoc.side_effect = get_ydoc
+        yield mock_service
+
+
 PIXEL = (
     b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06\x00"
     b"\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\xf8\xff\xff?\x00\x05\xfe\x02\xfe"
@@ -75,7 +96,7 @@ def test_api_documents_duplicate_anonymous():
 
 
 @pytest.mark.parametrize("index", range(3))
-def test_api_documents_duplicate_success(index):
+def test_api_documents_duplicate_success(index, mock_yhub):
     """
     Anonymous users should be able to retrieve attachments linked to a public document.
     Accesses should not be duplicated if the user does not request it specifically.
@@ -120,7 +141,11 @@ def test_api_documents_duplicate_success(index):
 
     duplicated_document = models.Document.objects.get(id=response.json()["id"])
     assert duplicated_document.title == "Copy of document with an image"
-    assert duplicated_document.content == document.content
+    # the content is copied through the collaboration server
+    assert duplicated_document.content is None
+    mock_yhub.return_value.create_ydoc.assert_called_once_with(
+        duplicated_document, base64.b64decode(document.content)
+    )
     assert duplicated_document.creator == user
     assert duplicated_document.link_reach == "restricted"
     assert duplicated_document.link_role == "reader"
@@ -185,7 +210,7 @@ def test_api_documents_duplicate_success(index):
 
 
 @pytest.mark.parametrize("role", ["owner", "administrator"])
-def test_api_documents_duplicate_with_accesses_admin(role):
+def test_api_documents_duplicate_with_accesses_admin(role, mock_yhub):
     """
     Accesses should be duplicated if the user requests it specifically and is owner or admin.
     """
@@ -219,7 +244,11 @@ def test_api_documents_duplicate_with_accesses_admin(role):
 
     duplicated_document = models.Document.objects.get(id=response.json()["id"])
     assert duplicated_document.title == "Copy of document with accesses"
-    assert duplicated_document.content == document.content
+    # the content is copied through the collaboration server
+    assert duplicated_document.content is None
+    mock_yhub.return_value.create_ydoc.assert_called_once_with(
+        duplicated_document, base64.b64decode(document.content)
+    )
     assert duplicated_document.link_reach == document.link_reach
     assert duplicated_document.link_role == document.link_role
     assert duplicated_document.creator == user
@@ -246,7 +275,7 @@ def test_api_documents_duplicate_with_accesses_admin(role):
 
 
 @pytest.mark.parametrize("role", ["editor", "reader"])
-def test_api_documents_duplicate_with_accesses_non_admin(role):
+def test_api_documents_duplicate_with_accesses_non_admin(role, mock_yhub):
     """
     Accesses should not be duplicated if the user requests it specifically and is not owner
     or admin.
@@ -274,7 +303,11 @@ def test_api_documents_duplicate_with_accesses_non_admin(role):
 
     duplicated_document = models.Document.objects.get(id=response.json()["id"])
     assert duplicated_document.title == "Copy of document with accesses"
-    assert duplicated_document.content == document.content
+    # the content is copied through the collaboration server
+    assert duplicated_document.content is None
+    mock_yhub.return_value.create_ydoc.assert_called_once_with(
+        duplicated_document, base64.b64decode(document.content)
+    )
     assert duplicated_document.link_reach == document.link_reach
     assert duplicated_document.link_role == document.link_role
     assert duplicated_document.creator == user
@@ -295,7 +328,7 @@ def test_api_documents_duplicate_with_accesses_non_admin(role):
 
 
 @pytest.mark.parametrize("role", ["editor", "reader"])
-def test_api_documents_duplicate_non_root_document(role):
+def test_api_documents_duplicate_non_root_document(role, mock_yhub):
     """
     Non-root documents can be duplicated but without accesses.
     """
@@ -322,7 +355,11 @@ def test_api_documents_duplicate_non_root_document(role):
 
     duplicated_document = models.Document.objects.get(id=response.json()["id"])
     assert duplicated_document.title == "Copy of document with accesses"
-    assert duplicated_document.content == child.content
+    # the content is copied through the collaboration server
+    assert duplicated_document.content is None
+    mock_yhub.return_value.create_ydoc.assert_called_once_with(
+        duplicated_document, base64.b64decode(child.content)
+    )
     assert duplicated_document.link_reach == child.link_reach
     assert duplicated_document.link_role == child.link_role
     assert duplicated_document.creator == user
@@ -517,7 +554,7 @@ def test_api_documents_duplicate_with_descendants_multi_level():
 
 
 # pylint: disable=too-many-locals
-def test_api_documents_duplicate_with_descendants_and_attachments():
+def test_api_documents_duplicate_with_descendants_and_attachments(mock_yhub):
     """
     Duplicating with descendants should properly handle attachments in all children.
     """
@@ -590,14 +627,20 @@ def test_api_documents_duplicate_with_descendants_and_attachments():
 
     # Check root attachments
     assert duplicated_root.attachments == [image_key_root]
-    assert duplicated_root.content == root_content
 
     # Check child attachments
     dup_children = duplicated_root.get_children()
     assert dup_children.count() == 1
     dup_child = dup_children.first()
     assert dup_child.attachments == [image_key_child]
-    assert dup_child.content == child_content
+
+    # the content of the whole subtree is copied through the collaboration server
+    assert duplicated_root.content is None
+    assert dup_child.content is None
+    assert mock_yhub.return_value.create_ydoc.call_args_list == [
+        mock.call(duplicated_root, base64.b64decode(root_content)),
+        mock.call(dup_child, base64.b64decode(child_content)),
+    ]
 
 
 def test_api_documents_duplicate_with_descendants_and_accesses():
@@ -864,33 +907,57 @@ def test_api_documents_duplicate_with_descendants_complex_tree():
     assert dup_grandchildren2.first().title == "Copy of GrandChild 3"
 
 
-def test_api_documents_duplicate_document_without_content():
+def test_api_documents_duplicate_content_from_collaboration_server(mock_yhub):
     """
-    A document that was never edited has no content in object storage. Duplicating it
-    should succeed and produce an empty duplicate.
+    The content held by the collaboration server is the one duplicated, the
+    content Django may still store for the document is ignored.
     """
     user = factories.UserFactory()
     client = APIClient()
     client.force_login(user)
 
-    # Create through the API so nothing is ever written to object storage
-    with mock.patch("core.api.viewsets.posthog_capture"):
-        response = client.post(
-            "/api/v1.0/documents/", {"title": "never edited"}, format="json"
-        )
+    image_key, image_url = get_image_refs(uuid.uuid4())
+
+    # what the collaboration server holds, an image Django never saw
+    ydoc = pycrdt.Doc()
+    ydoc["document-store"] = pycrdt.XmlFragment(
+        [pycrdt.XmlElement("img", {"src": image_url})]
+    )
+    edited_update = ydoc.get_update()
+    mock_yhub.return_value.get_ydoc.side_effect = None
+    mock_yhub.return_value.get_ydoc.return_value = edited_update
+
+    document = factories.DocumentFactory(
+        users=[(user, "owner")],
+        title="an edited document",
+        attachments=[image_key],
+    )
+
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/duplicate/")
 
     assert response.status_code == 201
-    document = models.Document.objects.get(id=response.json()["id"])
-    assert document.content is None
-
-    with mock.patch("core.api.viewsets.posthog_capture"):
-        response = client.post(f"/api/v1.0/documents/{document.id!s}/duplicate/")
-
-    assert response.status_code == 201
-
     duplicated_document = models.Document.objects.get(id=response.json()["id"])
-    assert duplicated_document.title == "Copy of never edited"
-    assert duplicated_document.content is None
-    assert duplicated_document.creator == user
-    assert duplicated_document.duplicated_from == document
-    assert duplicated_document.attachments == []
+
+    mock_yhub.return_value.get_ydoc.assert_called_once_with(document)
+    mock_yhub.return_value.create_ydoc.assert_called_once_with(
+        duplicated_document, edited_update
+    )
+    # the attachments are the ones of the duplicated state, not of Django's
+    assert duplicated_document.attachments == [image_key]
+
+
+def test_api_documents_duplicate_collaboration_server_unavailable(mock_yhub):
+    """A document whose content cannot be copied should not be duplicated."""
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(users=[(user, "owner")], title="my document")
+    mock_yhub.return_value.create_ydoc.side_effect = YHubServiceUnavailableError(
+        "Failed to connect to the yhub service"
+    )
+
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/duplicate/")
+
+    assert response.status_code == 500
+    assert models.Document.objects.count() == 1

src/backend/core/tests/external_api/test_external_api_documents.py

@@ -429,9 +429,12 @@ def test_external_api_documents_duplicate_allowed(
         role=models.RoleChoices.OWNER,
     )
 
-    response = client.post(
-        f"/external_api/v1.0/documents/{document.id!s}/duplicate/",
-    )
+    with patch("core.api.viewsets.YHubService") as mock_yhub:
+        # the collaboration server holds no content for this document
+        mock_yhub.return_value.get_ydoc.return_value = None
+        response = client.post(
+            f"/external_api/v1.0/documents/{document.id!s}/duplicate/",
+        )
 
     assert response.status_code == 201
 

src/backend/core/utils/yjs.py

@@ -9,14 +9,18 @@ from bs4 import BeautifulSoup
 from core import enums
 
 
+def yjs_to_xml(update):
+    """Extract xml from a raw yjs update."""
+
+    doc = pycrdt.Doc()
+    doc.apply_update(update)
+    return str(doc.get("document-store", type=pycrdt.XmlFragment))
+
+
 def base64_yjs_to_xml(base64_string):
     """Extract xml from base64 yjs document."""
 
-    decoded_bytes = base64.b64decode(base64_string)
-
-    doc = pycrdt.Doc()
-    doc.apply_update(decoded_bytes)
-    return str(doc.get("document-store", type=pycrdt.XmlFragment))
+    return yjs_to_xml(base64.b64decode(base64_string))
 
 
 def base64_yjs_to_text(base64_string):
@@ -34,3 +38,11 @@ def extract_attachments(content):
 
     xml_content = base64_yjs_to_xml(content)
     return re.findall(enums.MEDIA_STORAGE_URL_EXTRACT, xml_content)
+
+
+def extract_attachments_from_update(update):
+    """Helper method to extract media paths from a raw yjs update."""
+    if not update:
+        return []
+
+    return re.findall(enums.MEDIA_STORAGE_URL_EXTRACT, yjs_to_xml(update))