From 39d086ebb89297dcd2bae7c216b0f78a64ea3ef8 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 27 Jul 2026 08:49:03 -0700 Subject: [PATCH 1/7] create copy wiki method --- docs/reference/experimental/async/wiki.md | 1 + docs/reference/experimental/sync/wiki.md | 1 + synapseclient/api/__init__.py | 2 + synapseclient/api/file_services.py | 40 + .../models/protocols/wikipage_protocol.py | 64 + synapseclient/models/wiki.py | 692 ++++++++- synapseutils/copy_functions.py | 33 + .../models/async/test_wiki_async.py | 353 +++++ .../models/async/unit_test_wiki_async.py | 1255 ++++++++++++++++- 9 files changed, 2439 insertions(+), 2 deletions(-) diff --git a/docs/reference/experimental/async/wiki.md b/docs/reference/experimental/async/wiki.md index d1cd829ab..c09bb5af4 100644 --- a/docs/reference/experimental/async/wiki.md +++ b/docs/reference/experimental/async/wiki.md @@ -26,6 +26,7 @@ - restore_async - get_async - delete_async + - copy_async - get_attachment_handles_async - get_attachment_async - get_attachment_preview_async diff --git a/docs/reference/experimental/sync/wiki.md b/docs/reference/experimental/sync/wiki.md index 46310a411..0cc567964 100644 --- a/docs/reference/experimental/sync/wiki.md +++ b/docs/reference/experimental/sync/wiki.md @@ -26,6 +26,7 @@ - restore - get - delete + - copy - get_attachment_handles - get_attachment - get_attachment_preview diff --git a/synapseclient/api/__init__.py b/synapseclient/api/__init__.py index c55e43410..af177be8d 100644 --- a/synapseclient/api/__init__.py +++ b/synapseclient/api/__init__.py @@ -106,6 +106,7 @@ post_external_filehandle, post_external_object_store_filehandle, post_external_s3_file_handle, + post_file_handles_copy, post_file_multipart, post_file_multipart_presigned_urls, put_file_multipart_add, @@ -216,6 +217,7 @@ "get_file_handle", "get_file_handle_presigned_url", "post_external_filehandle", + "post_file_handles_copy", "post_file_multipart_presigned_urls", "put_file_multipart_add", "AddPartResponse", diff --git a/synapseclient/api/file_services.py b/synapseclient/api/file_services.py index 75e451dec..58ca67c2b 100644 --- a/synapseclient/api/file_services.py +++ b/synapseclient/api/file_services.py @@ -544,3 +544,43 @@ def get_file_handle_for_download( f"associated with the Synapse {entity_type}: {synapse_id}" ) return result + + +async def post_file_handles_copy( + copy_requests: list[dict[str, Any]], + *, + synapse_client: Optional["Synapse"] = None, +) -> list[dict[str, Any]]: + """ + Copy a batch of file handles. Requests are automatically submitted in batches + of MAX_FILE_HANDLE_PER_COPY_REQUEST. + + + + Arguments: + copy_requests: A list of copy requests, each matching + + synapse_client: If not passed in and caching was not disabled by + `Synapse.allow_client_caching(False)` this will use the last created + instance from the Synapse class constructor. + + Returns: + A list of copy results matching + . + Failed copies include a failureCode of UNAUTHORIZED or NOT_FOUND. + """ + from synapseclient import Synapse + from synapseclient.core.constants.limits import MAX_FILE_HANDLE_PER_COPY_REQUEST + + client = Synapse.get_client(synapse_client=synapse_client) + + copy_results = [] + for start in range(0, len(copy_requests), MAX_FILE_HANDLE_PER_COPY_REQUEST): + batch = copy_requests[start : start + MAX_FILE_HANDLE_PER_COPY_REQUEST] + response = await client.rest_post_async( + "/filehandles/copy", + body=json.dumps({"copyRequests": batch}), + endpoint=client.fileHandleEndpoint, + ) + copy_results.extend(response.get("copyResults", [])) + return copy_results diff --git a/synapseclient/models/protocols/wikipage_protocol.py b/synapseclient/models/protocols/wikipage_protocol.py index ace77f9bd..650f7580f 100644 --- a/synapseclient/models/protocols/wikipage_protocol.py +++ b/synapseclient/models/protocols/wikipage_protocol.py @@ -340,6 +340,70 @@ def get_attachment_preview( """ return "" + def copy( + self, + destination_owner_id: str, + destination_sub_page_id: Optional[str] = None, + update_links: bool = True, + entity_map: Optional[Dict[str, str]] = None, + *, + synapse_client: Optional["Synapse"] = None, + ) -> List["WikiHeader"]: + """ + Copy the wiki page tree of the owner entity to another entity and + update internal links. + + If id is set on this WikiPage, only the sub-tree rooted at that wiki page + is copied. Otherwise the entire wiki of the owner entity is copied. + + Arguments: + destination_owner_id: The Synapse ID of the entity that the wiki + will be copied to. + destination_sub_page_id: Optional ID of a wiki page that already + exists in the destination. The root of the copied tree is written + into that page, replacing its title, markdown, and attachments, + and the rest of the copied pages are created beneath it. + update_links: Update all the internal links so that they point at the + copied wiki pages. For example, syn1234/wiki/34345 becomes + syn3345/wiki/49508. Defaults to True. + entity_map: A mapping of old Synapse IDs to new Synapse IDs, for + example {"syn1234": "syn2345"}. If provided, the Synapse IDs + referenced in the markdown of the copied wiki pages are updated, + for example syn1234 becomes syn2345. If omitted, Synapse IDs + are left unchanged. + synapse_client: If not passed in and caching was not disabled by + Synapse.allow_client_caching(False) this will use the last created + instance from the Synapse class constructor. + Returns: + A list of WikiHeader objects for the destination entity. + + Example: Copy the entire wiki of an entity to another entity + This example shows how to copy all wiki pages from one project to another. + ```python + from synapseclient import Synapse + from synapseclient.models import WikiPage + + syn = Synapse() + syn.login() + + new_wiki_headers = WikiPage(owner_id="syn123").copy( + destination_owner_id="syn456" + ) + print(new_wiki_headers) + ``` + Example: Copy a wiki sub-tree and update Synapse ID references + This example shows how to copy a specific wiki page and its sub-pages, + rewriting references to syn1234 so they point at syn2345. + ```python + new_wiki_headers = WikiPage(owner_id="syn123", id="34345").copy( + destination_owner_id="syn456", + entity_map={"syn1234": "syn2345"}, + ) + print(new_wiki_headers) + ``` + """ + return [] + def get_markdown_file( self, *, diff --git a/synapseclient/models/wiki.py b/synapseclient/models/wiki.py index b79b5f169..484dc3ced 100644 --- a/synapseclient/models/wiki.py +++ b/synapseclient/models/wiki.py @@ -4,6 +4,7 @@ import gzip import os import pprint +import re from dataclasses import dataclass, field from typing import Any, AsyncGenerator, Dict, Generator, List, Literal, Optional, Union @@ -18,6 +19,7 @@ get_wiki_history, get_wiki_order_hint, get_wiki_page, + post_file_handles_copy, post_wiki_page, put_wiki_order_hint, put_wiki_page, @@ -37,7 +39,11 @@ ) from synapseclient.core.exceptions import SynapseHTTPError from synapseclient.core.upload.upload_functions_async import upload_file_handle -from synapseclient.core.utils import delete_none_keys, merge_dataclass_entities +from synapseclient.core.utils import ( + delete_none_keys, + is_synapse_id_str, + merge_dataclass_entities, +) from synapseclient.models.protocols.wikipage_protocol import ( WikiHeaderSynchronousProtocol, WikiHistorySnapshotSynchronousProtocol, @@ -1368,3 +1374,687 @@ async def get_markdown_file_async( return unzipped_file_path else: return markdown_url + + async def _get_markdown_text(self, synapse_client: Synapse) -> str: + """Download the markdown of this wiki page and return it as text. + + Arguments: + synapse_client: The Synapse client to use for the download. + + Returns: + The markdown content of this wiki page as a string. + """ + markdown_url = await get_markdown_url( + owner_id=self.owner_id, + wiki_id=self.id, + wiki_version=self.wiki_version, + synapse_client=synapse_client, + ) + cache_dir = os.path.join(synapse_client.cache.cache_root_dir, "wiki_content") + if not os.path.exists(cache_dir): + os.makedirs(cache_dir) + downloaded_file_path = download_from_url( + url=markdown_url, + destination=cache_dir, + url_is_presigned=True, + synapse_client=synapse_client, + ) + try: + with gzip.open(downloaded_file_path, "rt", encoding="utf-8") as f_in: + return f_in.read() + finally: + os.remove(downloaded_file_path) + + async def _copy_attachment_file_handles(self, synapse_client: Synapse) -> list[str]: + """Copy the attachment file handles of this wiki page. + + Arguments: + synapse_client: The Synapse client to use for the copy. + + Returns: + The IDs of the newly copied file handles. + + Raises: + ValueError: If any file handle copy fails. + """ + if not self.attachment_file_handle_ids: + return [] + + attachment_handles = await self.get_attachment_handles_async( + synapse_client=synapse_client + ) + # Get rid of the previews + no_previews = [ + file_handle + for file_handle in attachment_handles.get("list", []) + if not file_handle.get("isPreview") + ] + if not no_previews: + return [] + + copy_requests = [ + { + "originalFile": { + "fileHandleId": file_handle["id"], + "associateObjectId": self.id, + "associateObjectType": "WikiAttachment", + }, + "newContentType": file_handle.get("contentType"), + "newFileName": file_handle.get("fileName"), + } + for file_handle in no_previews + ] + copy_results = await post_file_handles_copy( + copy_requests=copy_requests, + synapse_client=synapse_client, + ) + for copy_result in copy_results: + if copy_result.get("failureCode") is not None: + raise ValueError( + f"{copy_result['failureCode']} dataFileHandleId: " + f"{copy_result['originalFileHandleId']}" + ) + return [copy_result["newFileHandle"]["id"] for copy_result in copy_results] + + @otel_trace_method( + method_to_trace_name=lambda self, **kwargs: f"Copy_Wiki: Owner ID {self.owner_id}, Wiki ID {self.id}" + ) + async def copy_async( + self, + destination_owner_id: str, + destination_sub_page_id: str | None = None, + update_links: bool = True, + entity_map: dict[str, str] | None = None, + *, + synapse_client: Synapse | None = None, + ) -> list["WikiHeader"]: + """Copy the wiki page tree of the owner entity to another entity and + update internal links. + + If id is set on this WikiPage, only the sub-tree rooted at that wiki page + is copied. Otherwise the entire wiki of the owner entity is copied. + + Arguments: + destination_owner_id: The Synapse ID of the entity that the wiki + will be copied to. + destination_sub_page_id: Optional ID of a wiki page that already + exists in the destination. The root of the copied tree is written + into that page, replacing its title, markdown, and attachments, + and the rest of the copied pages are created beneath it. + Required when the destination entity already has a root wiki + page. + update_links: Update all the internal links so that they point at the + copied wiki pages. For example, syn1234/wiki/34345 becomes + syn3345/wiki/49508. Defaults to True. + entity_map: A mapping of old Synapse IDs to new Synapse IDs, for + example {"syn1234": "syn2345"}. If provided, the Synapse IDs + referenced in the markdown of the copied wiki pages are updated, + for example syn1234 becomes syn2345. If omitted, Synapse IDs + are left unchanged. + synapse_client: If not passed in and caching was not disabled by + Synapse.allow_client_caching(False) this will use the last created + instance from the Synapse class constructor. + + Returns: + A list of WikiHeader objects for the destination entity. + + Raises: + ValueError: If owner_id is not provided or is not a Synapse ID, + if destination_owner_id is not a Synapse ID, if a key or + value of entity_map is not + a Synapse ID, if id is set but no wiki page with that ID + exists in the owner entity, if destination_sub_page_id does + not exist in the destination entity, if the destination + entity already has a root wiki page and + destination_sub_page_id is not provided, or if copying an + attachment file handle fails. + + Example: Copy the entire wiki of an entity to another entity + This example shows how to copy all wiki pages from one project to another. + ```python + from synapseclient import Synapse + from synapseclient.models import WikiPage + + syn = Synapse() + syn.login() + + new_wiki_headers = await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456" + ) + print(new_wiki_headers) + ``` + Example: Copy a wiki sub-tree and update Synapse ID references + This example shows how to copy a specific wiki page and its sub-pages, + rewriting references to syn1234 so they point at syn2345. + ```python + new_wiki_headers = await WikiPage(owner_id="syn123", id="34345").copy_async( + destination_owner_id="syn456", + entity_map={"syn1234": "syn2345"}, + ) + print(new_wiki_headers) + ``` + """ + entity_sub_page_id, destination_sub_page_id = _validate_and_format_copy_inputs( + owner_id=self.owner_id, + destination_owner_id=destination_owner_id, + entity_sub_page_id=self.id, + destination_sub_page_id=destination_sub_page_id, + entity_map=entity_map, + ) + + client = Synapse.get_client(synapse_client=synapse_client) + + # Getting the wiki header tree fails when there is no wiki + old_wiki_headers = [] + try: + async for item in get_wiki_header_tree( + owner_id=self.owner_id, + synapse_client=client, + ): + old_wiki_headers.append(item) + except SynapseHTTPError as e: + if e.response.status_code == 404: + if entity_sub_page_id: + raise ValueError( + f"The wiki page {entity_sub_page_id} does not exist " + f"in the owner entity {self.owner_id}." + ) from e + return [] + raise + + if destination_sub_page_id: + destination_wiki_page = await _get_existing_destination_wiki_page( + destination_owner_id=destination_owner_id, + destination_sub_page_id=destination_sub_page_id, + synapse_client=client, + ) + else: + await _ensure_destination_has_no_root_wiki( + destination_owner_id=destination_owner_id, + synapse_client=client, + ) + destination_wiki_page = None + + if entity_sub_page_id: + old_wiki_headers = _collect_wiki_sub_tree_headers( + wiki_headers=old_wiki_headers, + sub_page_id=entity_sub_page_id, + ) + if not old_wiki_headers: + raise ValueError( + f"The wiki page {entity_sub_page_id} does not exist " + f"in the owner entity {self.owner_id}." + ) + + if not old_wiki_headers: + return [] + + new_wikis, wiki_id_map = await _copy_wiki_pages( + old_wiki_headers=old_wiki_headers, + source_owner_id=self.owner_id, + destination_owner_id=destination_owner_id, + destination_wiki_page=destination_wiki_page, + destination_sub_page_id=destination_sub_page_id, + synapse_client=client, + ) + + if update_links: + client.logger.info("Updating internal links") + new_wikis = _update_internal_links( + new_wikis=new_wikis, + wiki_id_map=wiki_id_map, + source_owner_id=self.owner_id, + destination_owner_id=destination_owner_id, + ) + client.logger.info("Done updating internal links.") + + if entity_map: + client.logger.info("Updating Synapse references") + new_wikis = _update_synapse_id_references( + new_wikis=new_wikis, + wiki_id_map=wiki_id_map, + entity_map=entity_map, + ) + client.logger.info("Done updating Synapse IDs.") + + if update_links or entity_map: + client.logger.info("Storing new wiki pages") + for new_wiki_id in wiki_id_map.values(): + new_wiki = new_wikis[new_wiki_id] + new_wiki = await new_wiki._get_markdown_file_handle( + synapse_client=client + ) + wiki_data = await put_wiki_page( + owner_id=destination_owner_id, + wiki_id=new_wiki_id, + request=new_wiki.to_synapse_request(), + synapse_client=client, + ) + new_wikis[new_wiki_id] = new_wiki.fill_from_dict(synapse_wiki=wiki_data) + client.logger.info(f"Stored wiki page {new_wiki_id}") + + new_wiki_headers = [] + async for item in get_wiki_header_tree( + owner_id=destination_owner_id, + synapse_client=client, + ): + new_wiki_headers.append(WikiHeader().fill_from_dict(wiki_header=item)) + return new_wiki_headers + + +async def _copy_wiki_pages( + old_wiki_headers: list[dict[str, str]], + source_owner_id: str, + destination_owner_id: str, + destination_wiki_page: Optional["WikiPage"], + destination_sub_page_id: str | None, + synapse_client: Synapse, +) -> tuple[dict[str, "WikiPage"], dict[str, str]]: + """Create a copy of each source wiki page in the destination entity. + + Iterates over the source wiki headers in order, creating a corresponding + wiki page in the destination for each one and copying its markdown and + attachment file handles. The header list is expected to be ordered so that + a page appears before any of its children, since each child is created with + a parent_id looked up from the pages copied earlier. + + A page with a parentId is created beneath its already-copied parent. The + root of the copied tree is written into destination_wiki_page when one was + provided, otherwise it is created as a new page under destination_sub_page_id + (which may be None to create a root page). + + Arguments: + old_wiki_headers: The source wiki headers to copy, ordered parent before + child. + source_owner_id: The Synapse ID of the entity whose wiki is copied. + destination_owner_id: The Synapse ID of the entity the wiki is copied to. + destination_wiki_page: An existing destination wiki page that the root of + the copied tree is written into, or None to create a new root page. + destination_sub_page_id: The ID of the destination page the copied root + is created beneath when destination_wiki_page is None, or None to + create a root page. + synapse_client: The Synapse client to use for the copy. + + Returns: + A tuple of the copied wiki pages keyed by their new wiki ID and a + mapping of old wiki IDs to new wiki IDs. + """ + wiki_id_map = {} + new_wikis: dict[str, WikiPage] = {} + for wiki_header in old_wiki_headers: + old_wiki = await WikiPage( + owner_id=source_owner_id, id=wiki_header["id"] + ).get_async(synapse_client=synapse_client) + synapse_client.logger.info(f"Got wiki {wiki_header['id']}") + markdown = await old_wiki._get_markdown_text(synapse_client=synapse_client) + new_file_handle_ids = await old_wiki._copy_attachment_file_handles( + synapse_client=synapse_client + ) + + if wiki_header.get("parentId"): + new_wiki = WikiPage( + owner_id=destination_owner_id, + title=old_wiki.title or "", + markdown=markdown, + parent_id=wiki_id_map[wiki_header["parentId"]], + attachment_file_handle_ids=new_file_handle_ids, + ) + new_wiki = await new_wiki._get_markdown_file_handle( + synapse_client=synapse_client + ) + wiki_data = await post_wiki_page( + owner_id=destination_owner_id, + request=new_wiki.to_synapse_request(), + synapse_client=synapse_client, + ) + new_wiki.fill_from_dict(synapse_wiki=wiki_data) + elif destination_wiki_page is not None: + # Write the root of the copied tree into the existing + # destination wiki page + destination_wiki_page.title = old_wiki.title or "" + destination_wiki_page.markdown = markdown + destination_wiki_page.attachment_file_handle_ids = new_file_handle_ids + destination_wiki_page = ( + await destination_wiki_page._get_markdown_file_handle( + synapse_client=synapse_client + ) + ) + wiki_data = await put_wiki_page( + owner_id=destination_owner_id, + wiki_id=destination_wiki_page.id, + request=destination_wiki_page.to_synapse_request(), + synapse_client=synapse_client, + ) + new_wiki = destination_wiki_page.fill_from_dict(synapse_wiki=wiki_data) + else: + new_wiki = WikiPage( + owner_id=destination_owner_id, + title=old_wiki.title or "", + markdown=markdown, + parent_id=destination_sub_page_id, + attachment_file_handle_ids=new_file_handle_ids, + ) + new_wiki = await new_wiki._get_markdown_file_handle( + synapse_client=synapse_client + ) + wiki_data = await post_wiki_page( + owner_id=destination_owner_id, + request=new_wiki.to_synapse_request(), + synapse_client=synapse_client, + ) + new_wiki.fill_from_dict(synapse_wiki=wiki_data) + + new_wikis[new_wiki.id] = new_wiki + wiki_id_map[old_wiki.id] = new_wiki.id + + return new_wikis, wiki_id_map + + +def _coerce_sub_page_id( + sub_page_id: str | int | float | None, argument_description: str +) -> str | None: + """Coerce a wiki sub page ID to an integer string. + + Rejects values that a plain int() conversion would silently mangle: + fractional floats truncate, and bools and negative numbers convert to + nonsense page IDs. Integer-valued floats such as 4.0 are accepted since + they convert losslessly. + + Arguments: + sub_page_id: The wiki page ID to coerce. + argument_description: How to refer to the argument in error messages. + + Returns: + The ID coerced to an integer string, or None if not provided. + + Raises: + ValueError: If the ID is not a non-negative whole number or numeric + string. + """ + if sub_page_id is None: + return None + if isinstance(sub_page_id, float) and sub_page_id.is_integer(): + sub_page_id = int(sub_page_id) + if isinstance(sub_page_id, bool) or not str(sub_page_id).isdecimal(): + raise ValueError( + f"{argument_description} must be a numeric wiki page ID or None, " + f"got {sub_page_id}." + ) + return str(int(sub_page_id)) + + +def _validate_and_format_copy_inputs( + owner_id: str | None, + destination_owner_id: str, + entity_sub_page_id: str | int | float | None, + destination_sub_page_id: str | int | float | None, + entity_map: dict[str, str] | None = None, +) -> tuple[str | None, str | None]: + """Validate the inputs of a wiki copy and coerce the sub page IDs. + + Arguments: + owner_id: The Synapse ID of the entity whose wiki is copied. + destination_owner_id: The Synapse ID of the entity that the wiki is + copied to. + entity_sub_page_id: Optional ID of the wiki page that is the root of + the sub-tree to copy. + destination_sub_page_id: Optional ID of a wiki page that already + exists in the destination. + entity_map: Optional mapping of old Synapse IDs to new Synapse IDs. + + Returns: + A tuple of entity_sub_page_id and destination_sub_page_id, each + coerced to an integer string, or None if not provided. + + Raises: + ValueError: If owner_id is not provided or is not a Synapse ID, if + destination_owner_id is not a Synapse ID, if a key or value of + entity_map is not a Synapse ID, or if a sub page ID is not + numeric. + """ + if not owner_id: + raise ValueError("Must provide owner_id to copy a wiki.") + if not is_synapse_id_str(owner_id): + raise ValueError( + f"Wiki owner_id must be a Synapse ID such as syn123, got {owner_id}." + ) + + if not is_synapse_id_str(destination_owner_id): + raise ValueError( + "destination_owner_id must be a Synapse ID such as syn123, " + f"got {destination_owner_id}." + ) + + for old_synapse_id, new_synapse_id in (entity_map or {}).items(): + if not is_synapse_id_str(old_synapse_id): + raise ValueError( + "entity_map keys must be Synapse IDs such as syn123, " + f"got {old_synapse_id}." + ) + if not is_synapse_id_str(new_synapse_id): + raise ValueError( + "entity_map values must be Synapse IDs such as syn123, " + f"got {new_synapse_id}." + ) + + destination_sub_page_id = _coerce_sub_page_id( + sub_page_id=destination_sub_page_id, + argument_description="destination_sub_page_id", + ) + entity_sub_page_id = _coerce_sub_page_id( + sub_page_id=entity_sub_page_id, + argument_description="The id of the WikiPage", + ) + + return (entity_sub_page_id, destination_sub_page_id) + + +async def _ensure_destination_has_no_root_wiki( + destination_owner_id: str, + synapse_client: Synapse, +) -> None: + """Verify that a copied wiki tree can become the root wiki of the destination. + + A Synapse entity can have at most one root wiki page, so copying a wiki + without a destination_sub_page_id is only possible when the destination + has no wiki yet. Checking upfront fails fast before any attachment file + handles are copied. + + Arguments: + destination_owner_id: The Synapse ID of the entity the wiki is copied to. + synapse_client: The Synapse client to use for the lookup. + + Raises: + ValueError: If the destination entity already has a root wiki page. + """ + root_wiki_data = await _fetch_wiki_page_data( + owner_id=destination_owner_id, + wiki_id=None, + synapse_client=synapse_client, + ) + if root_wiki_data is not None: + raise ValueError( + f"The destination entity {destination_owner_id} already has a " + "root wiki page. Provide destination_sub_page_id to copy the " + "wiki beneath one of its existing pages." + ) + + +async def _get_existing_destination_wiki_page( + destination_owner_id: str, + destination_sub_page_id: str, + synapse_client: Synapse, +) -> "WikiPage": + """Fetch the destination wiki page that a copied wiki tree is written into. + + Arguments: + destination_owner_id: The Synapse ID of the entity the wiki is copied to. + destination_sub_page_id: The ID of a wiki page that already exists in + the destination. + synapse_client: The Synapse client to use for the lookup. + + Returns: + The destination wiki page. + + Raises: + ValueError: If no wiki page with the given ID exists in the destination. + """ + destination_wiki_data = await _fetch_wiki_page_data( + owner_id=destination_owner_id, + wiki_id=destination_sub_page_id, + synapse_client=synapse_client, + ) + if destination_wiki_data is None: + raise ValueError( + f"The destination_sub_page_id {destination_sub_page_id} does not " + f"exist in the destination entity {destination_owner_id}." + ) + destination_wiki_page = WikiPage().fill_from_dict( + synapse_wiki=destination_wiki_data + ) + destination_wiki_page.owner_id = destination_owner_id + return destination_wiki_page + + +async def _fetch_wiki_page_data( + owner_id: str, + wiki_id: str | None, + synapse_client: Synapse, +) -> dict | None: + """Fetch a wiki page, returning None instead of raising if it does not exist. + + Arguments: + owner_id: The Synapse ID of the entity that owns the wiki. + wiki_id: The ID of the wiki page to fetch. If None, the root wiki page + of the entity is fetched. + synapse_client: The Synapse client to use for the lookup. + + Returns: + The wiki page data, or None if the page does not exist. + """ + try: + return await get_wiki_page( + owner_id=owner_id, + wiki_id=wiki_id, + synapse_client=synapse_client, + ) + except SynapseHTTPError as e: + if e.response.status_code == 404: + return None + raise + + +def _collect_wiki_sub_tree_headers( + wiki_headers: list[dict[str, str]], + sub_page_id: str, + collected_headers: list[dict[str, str]] | None = None, +) -> list[dict[str, str]] | None: + """Collect the wiki header for sub_page_id and all of its descendants. + + Synapse returns a whole wiki as one flat list of headers, where each header + only records its parent. This function picks out the requested page, then + every page underneath it by recursing on each child it finds, and returns + that branch as a list with each parent appearing before its children. All + other pages are ignored. The requested page has its parentId removed since + it becomes the root of the copied tree. + + Arguments: + wiki_headers: The flat list of all wiki headers for the owner entity. + sub_page_id: The ID of the wiki page that is the root of the sub-tree. + collected_headers: Used internally to accumulate matches during recursion. + + Returns: + The wiki headers for the sub-tree rooted at sub_page_id. The root header + has its parentId removed so it is treated as a root page when copied. + """ + sub_page_id = str(sub_page_id) + for wiki_header in wiki_headers: + if wiki_header["id"] == sub_page_id: + if collected_headers is None: + # The root of the sub-tree is treated as a root page (no parent) + root_header = { + key: value + for key, value in wiki_header.items() + if key != "parentId" + } + collected_headers = [root_header] + else: + collected_headers.append(wiki_header) + elif wiki_header.get("parentId") == sub_page_id: + collected_headers = _collect_wiki_sub_tree_headers( + wiki_headers=wiki_headers, + sub_page_id=wiki_header["id"], + collected_headers=collected_headers, + ) + return collected_headers + + +def _update_internal_links( + new_wikis: dict[str, "WikiPage"], + wiki_id_map: dict[str, str], + source_owner_id: str, + destination_owner_id: str, +) -> dict[str, "WikiPage"]: + """Rewrite internal wiki links in the markdown of copied wiki pages. + + Copied markdown still contains links to the source wiki, written as paths + like source_owner_id/wiki/old_wiki_id. Since the copied pages have new IDs, + this function must run after all pages are copied, when the complete + old-to-new ID mapping is known. It replaces each such path with + destination_owner_id/wiki/new_wiki_id, then replaces any remaining references + to the source owner ID with the destination owner ID. Only the in-memory + WikiPage objects are modified; the caller is responsible for storing the + updated markdown. + + Arguments: + new_wikis: The copied wiki pages keyed by their new wiki ID. + wiki_id_map: A mapping of old wiki IDs to new wiki IDs. + source_owner_id: The Synapse ID of the entity the wiki was copied from. + destination_owner_id: The Synapse ID of the entity the wiki was copied to. + + Returns: + The copied wiki pages with updated markdown. + """ + for new_wiki_id in wiki_id_map.values(): + markdown = new_wikis[new_wiki_id].markdown or "" + for old_wiki_id, mapped_wiki_id in wiki_id_map.items(): + old_reference = f"{source_owner_id}/wiki/{old_wiki_id}\\b" + new_reference = f"{destination_owner_id}/wiki/{mapped_wiki_id}" + markdown = re.sub(old_reference, new_reference, markdown) + markdown = re.sub(source_owner_id + "\\b", destination_owner_id, markdown) + new_wikis[new_wiki_id].markdown = markdown + return new_wikis + + +def _update_synapse_id_references( + new_wikis: dict[str, "WikiPage"], + wiki_id_map: dict[str, str], + entity_map: dict[str, str], +) -> dict[str, "WikiPage"]: + """Rewrite Synapse ID references in the markdown of copied wiki pages. + + Wiki markdown often mentions other entities (files, folders, tables) by + their Synapse IDs. When those entities were also copied, the copied + markdown still points at the originals. This function replaces each old + entity ID from entity_map with its new counterpart, wherever it appears + in the markdown of the copied pages. A trailing word boundary in the + pattern prevents a shorter ID from matching inside a longer one. While + _update_internal_links handles the wiki's links to its own pages, this + function handles references to everything else that was copied. Only the + in-memory WikiPage objects are modified; the caller is responsible for + storing the updated markdown. + + Arguments: + new_wikis: The copied wiki pages keyed by their new wiki ID. + wiki_id_map: A mapping of old wiki IDs to new wiki IDs. + entity_map: A mapping of old Synapse IDs to new Synapse IDs. + + Returns: + The copied wiki pages with updated markdown. + """ + for new_wiki_id in wiki_id_map.values(): + markdown = new_wikis[new_wiki_id].markdown or "" + for old_synapse_id, new_synapse_id in entity_map.items(): + markdown = re.sub(old_synapse_id + "\\b", new_synapse_id, markdown) + new_wikis[new_wiki_id].markdown = markdown + return new_wikis diff --git a/synapseutils/copy_functions.py b/synapseutils/copy_functions.py index 26494247e..8f3ad3ff6 100644 --- a/synapseutils/copy_functions.py +++ b/synapseutils/copy_functions.py @@ -4,6 +4,8 @@ import re import typing +from deprecated import deprecated + import synapseclient from synapseclient import ( Activity, @@ -806,6 +808,13 @@ def _updateInternalLinks(newWikis, wikiIdMap, entity, destinationId): return newWikis +@deprecated( + version="5.0.0", + reason=( + "To be removed in 6.0.0. Use WikiPage.copy or WikiPage.copy_async from " + "synapseclient.models instead." + ), +) def copyWiki( syn, entity, @@ -819,6 +828,10 @@ def copyWiki( """ Copies wikis and updates internal links + This function is deprecated. Use WikiPage.copy or WikiPage.copy_async from + synapseclient.models instead, where owner_id is the source entity and id is + the optional sub-page to copy from. + Arguments: syn: A Synapse object with user's login, e.g. syn = synapseclient.login() entity: A synapse ID of an entity whose wiki you want to copy @@ -838,6 +851,26 @@ def copyWiki( Returns: A list of Objects with three fields: id, title and parentId. + + Example: Migration to the new method +   + + This function is deprecated. Use the WikiPage model from + synapseclient.models instead, where owner_id is the source entity and + id is the optional sub-page to copy from. + + ```python + # Old approach (DEPRECATED) + # import synapseutils + # new_wiki_headers = synapseutils.copyWiki(syn, "syn123", "syn456") + + # New approach (RECOMMENDED) + from synapseclient.models import WikiPage + + new_wiki_headers = WikiPage(owner_id="syn123").copy( + destination_owner_id="syn456" + ) + ``` """ # Validate input parameters diff --git a/tests/integration/synapseclient/models/async/test_wiki_async.py b/tests/integration/synapseclient/models/async/test_wiki_async.py index 971be086e..92469d3ea 100644 --- a/tests/integration/synapseclient/models/async/test_wiki_async.py +++ b/tests/integration/synapseclient/models/async/test_wiki_async.py @@ -3,6 +3,7 @@ import asyncio import gzip import os +import re import tempfile import uuid from typing import Callable @@ -801,6 +802,358 @@ async def test_get_wiki_header_tree( schedule_for_cleanup(headers) +class TestWikiPageCopy: + """Tests for WikiPage copy operations.""" + + OLD_ENTITY_ID = "syn000000123" + NEW_ENTITY_ID = "syn000000999" + + @pytest.fixture(scope="class") + async def source_wiki_tree( + self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + ) -> dict: + """Create a source project with a three-level wiki tree. + + The tree is root -> sub -> sub_sub. The sub and sub_sub pages each have + a file attachment. The root page markdown contains an internal link to + the sub page and a reference to a fake entity ID used to test + entity_map rewriting. + """ + project = Project(name=f"Test Wiki Copy Source_" + str(uuid.uuid4())) + project = await project.store_async(synapse_client=syn) + schedule_for_cleanup(project.id) + + root_wiki = await WikiPage( + owner_id=project.id, + title=f"Copy Root {str(uuid.uuid4())}", + markdown="# Root\n\nPlaceholder.", + ).store_async(synapse_client=syn) + + attachment_file = utils.make_bogus_uuid_file() + schedule_for_cleanup(attachment_file) + sub_wiki = await WikiPage( + owner_id=project.id, + parent_id=root_wiki.id, + title=f"Copy Sub {str(uuid.uuid4())}", + markdown="# Sub\n\nThis is the sub wiki page.", + attachments=[attachment_file], + ).store_async(synapse_client=syn) + + sub_sub_attachment_file = utils.make_bogus_uuid_file() + schedule_for_cleanup(sub_sub_attachment_file) + sub_sub_wiki = await WikiPage( + owner_id=project.id, + parent_id=sub_wiki.id, + title=f"Copy Sub Sub {str(uuid.uuid4())}", + markdown="# Sub Sub\n\nThis is the sub sub wiki page.", + attachments=[sub_sub_attachment_file], + ).store_async(synapse_client=syn) + + # Update the root markdown now that the sub wiki ID is known so it + # contains an internal wiki link and an entity reference + root_wiki.markdown = ( + "# Root\n\n" + f"See the sub page: {project.id}/wiki/{sub_wiki.id}\n\n" + f"Data is stored at {self.OLD_ENTITY_ID}." + ) + root_wiki = await root_wiki.store_async(synapse_client=syn) + + # Allow the wiki header tree to become consistent + await asyncio.sleep(5) + + # The source tree is never modified by the tests, so its markdown and + # attachment names are read once here and shared instead of being + # downloaded again by every test that compares against them + pages = [root_wiki, sub_wiki, sub_sub_wiki] + source_markdown = { + page.id: await self._read_markdown( + owner_id=project.id, + wiki_id=page.id, + syn=syn, + schedule_for_cleanup=schedule_for_cleanup, + ) + for page in pages + } + source_attachment_names = { + page.id: await self._attachment_file_names( + owner_id=project.id, wiki_id=page.id, syn=syn + ) + for page in pages + } + + return { + "project": project, + "root": root_wiki, + "sub": sub_wiki, + "sub_sub": sub_sub_wiki, + "attachment_name": os.path.basename(attachment_file), + "markdown": source_markdown, + "attachment_names": source_attachment_names, + } + + @pytest.fixture(scope="function") + async def destination_project( + self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + ) -> Project: + """Create a fresh destination project for each test.""" + project = Project(name=f"Test Wiki Copy Destination_" + str(uuid.uuid4())) + project = await project.store_async(synapse_client=syn) + schedule_for_cleanup(project.id) + return project + + @staticmethod + async def _read_markdown( + owner_id: str, + wiki_id: str, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> str: + """Download the markdown file of a wiki page and return its content.""" + download_dir = tempfile.mkdtemp() + schedule_for_cleanup(download_dir) + downloaded_path = await WikiPage( + owner_id=owner_id, id=wiki_id + ).get_markdown_file_async( + download_file=True, + download_location=download_dir, + synapse_client=syn, + ) + with open(downloaded_path, "r", encoding="utf-8") as f: + return f.read() + + @staticmethod + async def _attachment_file_names( + owner_id: str, wiki_id: str, syn: Synapse + ) -> list[str]: + """Return the sorted non-preview attachment file names of a wiki page.""" + attachment_handles = await WikiPage( + owner_id=owner_id, id=wiki_id + ).get_attachment_handles_async(synapse_client=syn) + return sorted( + handle.get("fileName") + for handle in attachment_handles["list"] + if not handle.get("isPreview") + ) + + async def test_copy_entire_wiki_tree( + self, + source_wiki_tree: dict, + destination_project: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> None: + """Test copying an entire wiki tree to another entity in a single copy, + verifying the hierarchy, that internal links and entity IDs are + rewritten while the rest of every page's markdown stays byte-for-byte + identical, and that attachments are copied at every level of the tree. + """ + # GIVEN a source project with a wiki tree and an empty destination project + source_project = source_wiki_tree["project"] + source_pages = [ + source_wiki_tree["root"], + source_wiki_tree["sub"], + source_wiki_tree["sub_sub"], + ] + + # WHEN copying the entire wiki with link updating and an entity_map + new_headers = await WikiPage(owner_id=source_project.id).copy_async( + destination_owner_id=destination_project.id, + entity_map={self.OLD_ENTITY_ID: self.NEW_ENTITY_ID}, + synapse_client=syn, + ) + + # THEN all three pages should be copied with their titles preserved + assert len(new_headers) == 3 + assert all(isinstance(header, WikiHeader) for header in new_headers) + headers_by_title = {header.title: header for header in new_headers} + new_root = headers_by_title[source_wiki_tree["root"].title] + new_sub = headers_by_title[source_wiki_tree["sub"].title] + new_sub_sub = headers_by_title[source_wiki_tree["sub_sub"].title] + + # AND the page hierarchy should be preserved + assert new_root.parent_id is None + assert new_sub.parent_id == new_root.id + assert new_sub_sub.parent_id == new_sub.id + + # AND every copied page's markdown should equal the source markdown + # with only the expected link and entity ID substitutions applied + wiki_id_map = { + page.id: headers_by_title[page.title].id for page in source_pages + } + copied_markdown_by_source_id = {} + for page in source_pages: + expected_markdown = source_wiki_tree["markdown"][page.id] + for old_wiki_id, new_wiki_id in wiki_id_map.items(): + expected_markdown = expected_markdown.replace( + f"{source_project.id}/wiki/{old_wiki_id}", + f"{destination_project.id}/wiki/{new_wiki_id}", + ) + expected_markdown = re.sub( + self.OLD_ENTITY_ID + r"\b", self.NEW_ENTITY_ID, expected_markdown + ) + + copied_markdown = await self._read_markdown( + owner_id=destination_project.id, + wiki_id=wiki_id_map[page.id], + syn=syn, + schedule_for_cleanup=schedule_for_cleanup, + ) + assert copied_markdown == expected_markdown + copied_markdown_by_source_id[page.id] = copied_markdown + + # AND the internal wiki link should point at the copied sub page rather + # than the source + copied_root_markdown = copied_markdown_by_source_id[source_wiki_tree["root"].id] + assert f"{destination_project.id}/wiki/{new_sub.id}" in copied_root_markdown + assert source_project.id not in copied_root_markdown + + # AND each copied page should have the same non-preview attachment file + # names as its source page + pages_with_attachments = 0 + for page in source_pages: + source_names = source_wiki_tree["attachment_names"][page.id] + copied_names = await self._attachment_file_names( + owner_id=destination_project.id, + wiki_id=wiki_id_map[page.id], + syn=syn, + ) + assert copied_names == source_names + if source_names: + pages_with_attachments += 1 + + # AND the comparison is not vacuous - the source tree has attachments + # at two different levels. Text attachments are gzipped on upload, so + # the stored file name has a .gz suffix. + assert pages_with_attachments == 2 + assert source_wiki_tree["attachment_names"][source_wiki_tree["sub"].id] == [ + f"{source_wiki_tree['attachment_name']}.gz" + ] + + async def test_copy_wiki_sub_tree( + self, + source_wiki_tree: dict, + destination_project: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> None: + """Test copying only a wiki sub-tree, verifying the sub page becomes + the root of the destination wiki.""" + # GIVEN a source project with a wiki tree and an empty destination project + source_project = source_wiki_tree["project"] + sub_wiki = source_wiki_tree["sub"] + + # WHEN copying only the sub-tree rooted at the sub wiki page + new_headers = await WikiPage( + owner_id=source_project.id, id=sub_wiki.id + ).copy_async( + destination_owner_id=destination_project.id, + synapse_client=syn, + ) + + # THEN only the sub page and its child should be copied + assert len(new_headers) == 2 + headers_by_title = {header.title: header for header in new_headers} + new_sub = headers_by_title[sub_wiki.title] + new_sub_sub = headers_by_title[source_wiki_tree["sub_sub"].title] + + # AND the copied sub page should become the root of the destination wiki + assert new_sub.parent_id is None + assert new_sub_sub.parent_id == new_sub.id + + async def test_copy_wiki_into_existing_destination_page( + self, + source_wiki_tree: dict, + destination_project: Project, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> None: + """Test copying a wiki sub-tree into an existing destination wiki page + via destination_sub_page_id, overwriting that page with the copied root.""" + # GIVEN a destination project with an existing root wiki page + source_project = source_wiki_tree["project"] + sub_wiki = source_wiki_tree["sub"] + destination_root = await WikiPage( + owner_id=destination_project.id, + title=f"Destination Root {str(uuid.uuid4())}", + markdown="# Destination Root\n\nThis page will be overwritten.", + ).store_async(synapse_client=syn) + + # WHEN copying the sub-tree into the existing destination page + new_headers = await WikiPage( + owner_id=source_project.id, id=sub_wiki.id + ).copy_async( + destination_owner_id=destination_project.id, + destination_sub_page_id=destination_root.id, + synapse_client=syn, + ) + + # THEN the root of the copied tree should be written into the + # existing destination page + assert len(new_headers) == 2 + updated_destination_root = await WikiPage( + owner_id=destination_project.id, id=destination_root.id + ).get_async(synapse_client=syn) + assert updated_destination_root.title == sub_wiki.title + + # AND the child page should be created under the destination page + headers_by_title = {header.title: header for header in new_headers} + new_sub_sub = headers_by_title[source_wiki_tree["sub_sub"].title] + assert new_sub_sub.parent_id == destination_root.id + + # AND the copied pages' markdown should match the source pages, + # replacing the original destination page content. Neither source + # page contains links or entity IDs, so the markdown should be + # copied verbatim. + destination_root_markdown = await self._read_markdown( + owner_id=destination_project.id, + wiki_id=destination_root.id, + syn=syn, + schedule_for_cleanup=schedule_for_cleanup, + ) + assert destination_root_markdown == source_wiki_tree["markdown"][sub_wiki.id] + + new_sub_sub_markdown = await self._read_markdown( + owner_id=destination_project.id, + wiki_id=new_sub_sub.id, + syn=syn, + schedule_for_cleanup=schedule_for_cleanup, + ) + assert ( + new_sub_sub_markdown + == source_wiki_tree["markdown"][source_wiki_tree["sub_sub"].id] + ) + + async def test_copy_wiki_from_entity_without_wiki( + self, + source_wiki_tree: dict, + syn: Synapse, + schedule_for_cleanup: Callable[..., None], + ) -> None: + """Test that copying from an entity that has no wiki returns an + empty list instead of raising an error.""" + # GIVEN a source project without any wiki pages + empty_source_project = Project( + name=f"Test Wiki Copy Empty Source_" + str(uuid.uuid4()) + ) + empty_source_project = await empty_source_project.store_async( + synapse_client=syn + ) + schedule_for_cleanup(empty_source_project.id) + + # WHEN copying its wiki to another entity. No destination project is + # created because the copy returns before the destination is contacted. + # The class source project is reused as the destination ID, and because + # it already has a root wiki the copy would fail rather than silently + # write anything if that short-circuit ever stopped happening. + new_headers = await WikiPage(owner_id=empty_source_project.id).copy_async( + destination_owner_id=source_wiki_tree["project"].id, + synapse_client=syn, + ) + + # THEN an empty list should be returned + assert new_headers == [] + + class TestWikiOrderHint: """Tests for WikiOrderHint operations.""" diff --git a/tests/unit/synapseclient/models/async/unit_test_wiki_async.py b/tests/unit/synapseclient/models/async/unit_test_wiki_async.py index dd4549144..4d6182f3b 100644 --- a/tests/unit/synapseclient/models/async/unit_test_wiki_async.py +++ b/tests/unit/synapseclient/models/async/unit_test_wiki_async.py @@ -1,9 +1,10 @@ """Tests for the synapseclient.models.wiki classes.""" +import contextlib import copy import os from typing import Any, AsyncGenerator, Dict -from unittest.mock import AsyncMock, Mock, call, mock_open, patch +from unittest.mock import ANY, AsyncMock, Mock, call, mock_open, patch import pytest @@ -15,6 +16,13 @@ WikiHistorySnapshot, WikiOrderHint, WikiPage, + _collect_wiki_sub_tree_headers, + _copy_wiki_pages, + _ensure_destination_has_no_root_wiki, + _get_existing_destination_wiki_page, + _update_internal_links, + _update_synapse_id_references, + _validate_and_format_copy_inputs, ) @@ -2166,3 +2174,1248 @@ async def test_get_markdown_file_async_with_none_wiki_version(self) -> None: # AND the result should be the markdown URL assert results == "https://example.com/markdown_latest.md" + + +class TestWikiPageCopy: + """Tests for the WikiPage.copy_async method.""" + + @pytest.fixture(autouse=True, scope="function") + def init_syn(self, syn: Synapse) -> None: + self.syn = syn + + @staticmethod + def _header_generator( + headers: list, + ) -> AsyncGenerator[Dict[str, str], None]: + """Return an async generator yielding the given wiki headers.""" + + async def generator() -> AsyncGenerator[Dict[str, str], None]: + for header in headers: + yield header + + return generator() + + async def test_copy_async_missing_owner_id(self) -> None: + # WHEN I call `copy_async` on a WikiPage without owner_id + # THEN it should raise ValueError before calling the API + with patch("synapseclient.models.wiki.get_wiki_header_tree") as mocked_get: + with pytest.raises( + ValueError, match="Must provide owner_id to copy a wiki." + ): + await WikiPage().copy_async( + destination_owner_id="syn456", synapse_client=self.syn + ) + mocked_get.assert_not_called() + + @pytest.mark.parametrize( + "destination_owner_id", [None, "", "project123", "syn", "syn123abc"] + ) + async def test_copy_async_invalid_destination_owner_id( + self, destination_owner_id + ) -> None: + # WHEN I call `copy_async` with a missing or malformed destination_owner_id + # THEN it should raise ValueError before calling the API + with patch("synapseclient.models.wiki.get_wiki_header_tree") as mocked_get: + with pytest.raises( + ValueError, match="destination_owner_id must be a Synapse ID" + ): + await WikiPage(owner_id="syn123").copy_async( + destination_owner_id=destination_owner_id, + synapse_client=self.syn, + ) + mocked_get.assert_not_called() + + async def test_copy_async_invalid_entity_map(self) -> None: + # WHEN I call `copy_async` with an entity_map containing a value that + # is not a Synapse ID + # THEN it should raise ValueError before calling the API + with patch("synapseclient.models.wiki.get_wiki_header_tree") as mocked_get: + with pytest.raises( + ValueError, match="entity_map values must be Synapse IDs" + ): + await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", + entity_map={"syn111": "not_an_id"}, + synapse_client=self.syn, + ) + mocked_get.assert_not_called() + + async def test_copy_async_source_without_wiki_returns_empty_list(self) -> None: + # GIVEN a source entity whose wiki header tree request fails with a 404 + with patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=SynapseHTTPError(response=Mock(status_code=404)), + ): + # WHEN I call `copy_async` + results = await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", synapse_client=self.syn + ) + + # THEN an empty list should be returned instead of raising + assert results == [] + + async def test_copy_async_header_tree_error_propagates(self) -> None: + # GIVEN a source entity whose wiki header tree request fails with a + # non-404 error + with ( + patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=SynapseHTTPError(response=Mock(status_code=500)), + ), + # WHEN I call `copy_async` + # THEN the error should be re-raised + pytest.raises(SynapseHTTPError), + ): + await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", synapse_client=self.syn + ) + + async def test_copy_async_unknown_sub_page_id_raises_value_error(self) -> None: + # GIVEN a source wiki whose header tree does not contain the requested + # page and a destination without an existing wiki + with ( + patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=lambda **kwargs: self._header_generator( + [{"id": "8688", "title": "Root"}] + ), + ), + patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + side_effect=SynapseHTTPError(response=Mock(status_code=404)), + ), + # WHEN I call `copy_async` with an id that is not in the tree + # THEN it should raise ValueError + pytest.raises( + ValueError, + match="The wiki page 9999 does not exist in the owner entity syn123.", + ), + ): + await WikiPage(owner_id="syn123", id="9999").copy_async( + destination_owner_id="syn456", synapse_client=self.syn + ) + + async def test_copy_async_sub_page_id_on_source_without_wiki_raises_value_error( + self, + ) -> None: + # GIVEN a source entity without a wiki, so the header tree request + # fails with a 404 + with ( + patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=SynapseHTTPError(response=Mock(status_code=404)), + ), + # WHEN I call `copy_async` with an id set + # THEN it should raise ValueError instead of returning an empty list + pytest.raises( + ValueError, + match="The wiki page 9999 does not exist in the owner entity syn123.", + ), + ): + await WikiPage(owner_id="syn123", id="9999").copy_async( + destination_owner_id="syn456", synapse_client=self.syn + ) + + @pytest.mark.parametrize( + "entity_sub_page_id,destination_sub_page_id", + [("8688", "4"), (8688.0, 4.0)], + ) + async def test_copy_async_sub_page_id_coercion( + self, entity_sub_page_id, destination_sub_page_id + ) -> None: + # GIVEN a copy request with sub page IDs given as numeric strings or floats + headers = [{"id": "8688", "title": "Root"}] + with ( + patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=lambda **kwargs: self._header_generator(headers), + ), + patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + return_value={"id": "4", "title": "Existing"}, + ) as mocked_get_page, + patch( + "synapseclient.models.wiki._collect_wiki_sub_tree_headers", + return_value=None, + ) as mocked_collect, + ): + # WHEN I call `copy_async` + with pytest.raises( + ValueError, + match="The wiki page 8688 does not exist in the owner entity syn123.", + ): + await WikiPage(owner_id="syn123", id=entity_sub_page_id).copy_async( + destination_owner_id="syn456", + destination_sub_page_id=destination_sub_page_id, + synapse_client=self.syn, + ) + + # THEN both IDs should be coerced to integer strings + mocked_get_page.assert_called_once_with( + owner_id="syn456", wiki_id="4", synapse_client=self.syn + ) + mocked_collect.assert_called_once_with( + wiki_headers=headers, sub_page_id="8688" + ) + + @pytest.mark.parametrize( + "entity_sub_page_id,destination_sub_page_id", + [("some_string", None), (None, "some_string")], + ) + async def test_copy_async_non_numeric_ids_raise_value_error( + self, entity_sub_page_id, destination_sub_page_id + ) -> None: + # WHEN I call `copy_async` with a non-numeric sub page ID + # THEN it should raise ValueError before calling the API + with patch("synapseclient.models.wiki.get_wiki_header_tree") as mocked_get: + with pytest.raises(ValueError): + await WikiPage(owner_id="syn123", id=entity_sub_page_id).copy_async( + destination_owner_id="syn456", + destination_sub_page_id=destination_sub_page_id, + synapse_client=self.syn, + ) + mocked_get.assert_not_called() + + async def test_copy_async_destination_page_error_propagates(self) -> None: + # GIVEN a destination page check that fails with a non-404 error + with ( + patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=lambda **kwargs: self._header_generator( + [{"id": "8688", "title": "Root"}] + ), + ), + patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + side_effect=SynapseHTTPError(response=Mock(status_code=403)), + ), + # WHEN I call `copy_async` + # THEN the error should be re-raised + pytest.raises(SynapseHTTPError), + ): + await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", + destination_sub_page_id="4", + synapse_client=self.syn, + ) + + async def test_copy_async_nonexistent_destination_sub_page_raises_value_error( + self, + ) -> None: + # GIVEN a source wiki with a single root page and a destination sub + # page ID that does not exist in the destination + with ( + patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=lambda **kwargs: self._header_generator( + [{"id": "8688", "title": "Root"}] + ), + ), + patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + side_effect=SynapseHTTPError(response=Mock(status_code=404)), + ) as mocked_get_page, + patch( + "synapseclient.models.wiki.WikiPage._copy_attachment_file_handles", + new_callable=AsyncMock, + ) as mocked_copy_attachments, + patch( + "synapseclient.models.wiki.post_wiki_page", + new_callable=AsyncMock, + ) as mocked_post, + ): + # WHEN I call `copy_async` + # THEN a ValueError should be raised + with pytest.raises(ValueError, match="does not exist"): + await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", + destination_sub_page_id="4", + synapse_client=self.syn, + ) + + # AND the destination page should have been checked + mocked_get_page.assert_called_once_with( + owner_id="syn456", wiki_id="4", synapse_client=self.syn + ) + + # AND no attachments should have been copied and no pages created + mocked_copy_attachments.assert_not_called() + mocked_post.assert_not_called() + + @staticmethod + @contextlib.contextmanager + def _patched_flag_copy(): + """Patch what copy_async does around the link and entity ID rewrites. + + The source wiki is a single root page 1 that is copied to new1. The + rewrite helpers are patched to return the new_wikis dict they were + handed so the tests can check which of them ran and what they were + given, and the re-store loop is patched at the markdown upload and + put boundaries. + + Yields: + A dict with the mocks under the keys "links", "entity_ids", and + "put". + """ + source_headers = [{"id": "1", "title": "Root"}] + destination_headers = [{"id": "new1", "title": "Root"}] + new_wikis = {"new1": WikiPage(owner_id="syn456", id="new1", markdown="md-1")} + wiki_id_map = {"1": "new1"} + + def fake_header_tree(**kwargs): + headers = ( + destination_headers + if kwargs["owner_id"] == "syn456" + else source_headers + ) + + async def generator(): + for header in headers: + yield header + + return generator() + + with ( + patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=fake_header_tree, + ), + patch( + "synapseclient.models.wiki._ensure_destination_has_no_root_wiki", + new_callable=AsyncMock, + ), + patch( + "synapseclient.models.wiki._copy_wiki_pages", + new_callable=AsyncMock, + return_value=(new_wikis, wiki_id_map), + ), + patch( + "synapseclient.models.wiki._update_internal_links", + side_effect=lambda **kwargs: kwargs["new_wikis"], + ) as mock_links, + patch( + "synapseclient.models.wiki._update_synapse_id_references", + side_effect=lambda **kwargs: kwargs["new_wikis"], + ) as mock_entity_ids, + patch.object( + WikiPage, + "_get_markdown_file_handle", + autospec=True, + side_effect=lambda self, *args, **kwargs: self, + ), + patch( + "synapseclient.models.wiki.put_wiki_page", + new_callable=AsyncMock, + return_value={"id": "new1", "title": "Root"}, + ) as mock_put, + ): + yield { + "links": mock_links, + "entity_ids": mock_entity_ids, + "put": mock_put, + } + + async def test_copy_async_updates_links_by_default(self) -> None: + # GIVEN a source wiki with a single page + with self._patched_flag_copy() as mocks: + # WHEN I call `copy_async` without passing update_links or entity_map + new_headers = await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", synapse_client=self.syn + ) + + # THEN the internal links should be rewritten + mocks["links"].assert_called_once_with( + new_wikis=ANY, + wiki_id_map={"1": "new1"}, + source_owner_id="syn123", + destination_owner_id="syn456", + ) + + # AND the Synapse ID references should be left unchanged + mocks["entity_ids"].assert_not_called() + + # AND the rewritten page should be stored back once + mocks["put"].assert_called_once() + assert mocks["put"].call_args.kwargs["owner_id"] == "syn456" + assert mocks["put"].call_args.kwargs["wiki_id"] == "new1" + + # AND the destination header tree should be returned + assert [header.id for header in new_headers] == ["new1"] + + async def test_copy_async_without_update_links_or_entity_map_skips_rewrites( + self, + ) -> None: + # GIVEN a source wiki with a single page + with self._patched_flag_copy() as mocks: + # WHEN I call `copy_async` with update_links disabled and no entity_map + new_headers = await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", + update_links=False, + synapse_client=self.syn, + ) + + # THEN neither rewrite should be applied + mocks["links"].assert_not_called() + mocks["entity_ids"].assert_not_called() + + # AND the copied pages should not be stored a second time + mocks["put"].assert_not_called() + + # AND the destination header tree should still be returned + assert [header.id for header in new_headers] == ["new1"] + + async def test_copy_async_entity_map_without_update_links(self) -> None: + # GIVEN a source wiki with a single page + with self._patched_flag_copy() as mocks: + # WHEN I call `copy_async` with an entity_map but update_links disabled + await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", + update_links=False, + entity_map={"syn111": "syn222"}, + synapse_client=self.syn, + ) + + # THEN only the Synapse ID references should be rewritten + mocks["links"].assert_not_called() + mocks["entity_ids"].assert_called_once_with( + new_wikis=ANY, + wiki_id_map={"1": "new1"}, + entity_map={"syn111": "syn222"}, + ) + + # AND the rewritten page should be stored back once + mocks["put"].assert_called_once() + + async def test_copy_async_update_links_and_entity_map(self) -> None: + # GIVEN a source wiki with a single page + with self._patched_flag_copy() as mocks: + # WHEN I call `copy_async` with both link updating and an entity_map + await WikiPage(owner_id="syn123").copy_async( + destination_owner_id="syn456", + entity_map={"syn111": "syn222"}, + synapse_client=self.syn, + ) + + # THEN both rewrites should be applied + mocks["links"].assert_called_once() + mocks["entity_ids"].assert_called_once() + + # AND the entity ID rewrite should operate on the pages returned by + # the link rewrite rather than on a separate copy + assert ( + mocks["entity_ids"].call_args.kwargs["new_wikis"] + is mocks["links"].call_args.kwargs["new_wikis"] + ) + + # AND the page should be stored back only once for both rewrites + mocks["put"].assert_called_once() + + +class TestValidateAndFormatCopyInputs: + """Tests for the _validate_and_format_copy_inputs helper function.""" + + @pytest.mark.parametrize( + "entity_sub_page_id,destination_sub_page_id,expected", + [ + (None, None, (None, None)), + ("8688", None, ("8688", None)), + (None, "4", (None, "4")), + (8688, 4, ("8688", "4")), + (8688.0, 4.0, ("8688", "4")), + ], + ) + def test_valid_inputs_return_coerced_sub_page_ids( + self, entity_sub_page_id, destination_sub_page_id, expected + ) -> None: + # WHEN I validate copy inputs with valid owner IDs and sub page IDs + result = _validate_and_format_copy_inputs( + owner_id="syn123", + destination_owner_id="syn456", + entity_sub_page_id=entity_sub_page_id, + destination_sub_page_id=destination_sub_page_id, + ) + + # THEN the sub page IDs should be coerced to integer strings or None + assert result == expected + + def test_missing_owner_id_raises_value_error(self) -> None: + # WHEN I validate copy inputs without an owner ID + # THEN it should raise ValueError + with pytest.raises(ValueError, match="Must provide owner_id to copy a wiki."): + _validate_and_format_copy_inputs( + owner_id=None, + destination_owner_id="syn456", + entity_sub_page_id=None, + destination_sub_page_id=None, + ) + + @pytest.mark.parametrize( + "destination_owner_id", [None, "", "project123", "123", "syn123abc"] + ) + def test_invalid_destination_owner_id_raises_value_error( + self, destination_owner_id + ) -> None: + # WHEN I validate copy inputs with a missing or malformed destination + # owner ID + # THEN it should raise ValueError + with pytest.raises( + ValueError, match="destination_owner_id must be a Synapse ID" + ): + _validate_and_format_copy_inputs( + owner_id="syn123", + destination_owner_id=destination_owner_id, + entity_sub_page_id=None, + destination_sub_page_id=None, + ) + + def test_valid_entity_map_passes(self) -> None: + # WHEN I validate copy inputs with an entity_map of Synapse IDs + # THEN no error should be raised + result = _validate_and_format_copy_inputs( + owner_id="syn123", + destination_owner_id="syn456", + entity_sub_page_id=None, + destination_sub_page_id=None, + entity_map={"syn111": "syn222", "syn333": "syn444"}, + ) + assert result == (None, None) + + @pytest.mark.parametrize( + "entity_map,offender", + [ + ({"not_an_id": "syn222"}, "keys"), + ({"111": "syn222"}, "keys"), + ({None: "syn222"}, "keys"), + ({"syn111": "not_an_id"}, "values"), + ({"syn111": "222"}, "values"), + ({"syn111": None}, "values"), + ({"syn111": "syn222", "syn333": "oops"}, "values"), + ], + ) + def test_invalid_entity_map_raises_value_error(self, entity_map, offender) -> None: + # WHEN I validate copy inputs with an entity_map containing a key or + # value that is not a Synapse ID + # THEN it should raise ValueError naming keys or values + with pytest.raises( + ValueError, match=f"entity_map {offender} must be Synapse IDs" + ): + _validate_and_format_copy_inputs( + owner_id="syn123", + destination_owner_id="syn456", + entity_sub_page_id=None, + destination_sub_page_id=None, + entity_map=entity_map, + ) + + @pytest.mark.parametrize( + "entity_sub_page_id,destination_sub_page_id,argument_name", + [ + ("some_string", None, "The id of the WikiPage"), + (None, "some_string", "destination_sub_page_id"), + (8688.9, None, "The id of the WikiPage"), + (None, 4.9, "destination_sub_page_id"), + (True, None, "The id of the WikiPage"), + (None, True, "destination_sub_page_id"), + (-8688, None, "The id of the WikiPage"), + (None, "-4", "destination_sub_page_id"), + ], + ) + def test_non_numeric_sub_page_id_raises_value_error( + self, entity_sub_page_id, destination_sub_page_id, argument_name + ) -> None: + # WHEN I validate copy inputs with a non-numeric, fractional, + # boolean, or negative sub page ID + # THEN it should raise ValueError naming the offending argument + with pytest.raises( + ValueError, match=f"{argument_name} must be a numeric wiki page ID" + ): + _validate_and_format_copy_inputs( + owner_id="syn123", + destination_owner_id="syn456", + entity_sub_page_id=entity_sub_page_id, + destination_sub_page_id=destination_sub_page_id, + ) + + +class TestGetExistingDestinationWikiPage: + """Tests for the _get_existing_destination_wiki_page helper function.""" + + @pytest.fixture(autouse=True, scope="function") + def init_syn(self, syn: Synapse) -> None: + self.syn = syn + + async def test_existing_destination_sub_page_is_returned(self) -> None: + # GIVEN a destination sub page that exists in the destination entity + destination_wiki_data = {"id": "4", "title": "Existing", "etag": "etag1"} + with patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + return_value=destination_wiki_data, + ) as mocked_get_page: + # WHEN I fetch the destination wiki page + result = await _get_existing_destination_wiki_page( + destination_owner_id="syn456", + destination_sub_page_id="4", + synapse_client=self.syn, + ) + + # THEN the page should be fetched from the destination entity + mocked_get_page.assert_called_once_with( + owner_id="syn456", wiki_id="4", synapse_client=self.syn + ) + + # AND returned as a WikiPage owned by the destination entity + assert result.id == "4" + assert result.title == "Existing" + assert result.owner_id == "syn456" + + async def test_destination_page_error_propagates(self) -> None: + # GIVEN a destination page lookup that fails with a non-404 error + with ( + patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + side_effect=SynapseHTTPError(response=Mock(status_code=403)), + ), + # WHEN I fetch the destination wiki page + # THEN the error should be re-raised + pytest.raises(SynapseHTTPError), + ): + await _get_existing_destination_wiki_page( + destination_owner_id="syn456", + destination_sub_page_id="4", + synapse_client=self.syn, + ) + + async def test_nonexistent_destination_sub_page_raises_value_error(self) -> None: + # GIVEN a destination_sub_page_id that does not exist in the + # destination entity + with ( + patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + side_effect=SynapseHTTPError(response=Mock(status_code=404)), + ), + # WHEN I fetch the destination wiki page + # THEN a ValueError should be raised before any wiki content is + # copied, instead of silently returning None and letting the copy + # proceed toward a confusing server-side failure + pytest.raises(ValueError, match="does not exist"), + ): + await _get_existing_destination_wiki_page( + destination_owner_id="syn456", + destination_sub_page_id="4", + synapse_client=self.syn, + ) + + +class TestEnsureDestinationHasNoRootWiki: + """Tests for the _ensure_destination_has_no_root_wiki helper function.""" + + @pytest.fixture(autouse=True, scope="function") + def init_syn(self, syn: Synapse) -> None: + self.syn = syn + + async def test_existing_root_wiki_raises_value_error(self) -> None: + # GIVEN a destination entity that already has a root wiki page + with ( + patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + return_value={"id": "1", "title": "Existing root"}, + ), + # WHEN I check the destination for an existing root wiki + # THEN a ValueError should be raised, since the server would + # reject creating a second root wiki page after attachments have + # already been copied + pytest.raises(ValueError, match="already has a root wiki"), + ): + await _ensure_destination_has_no_root_wiki( + destination_owner_id="syn456", + synapse_client=self.syn, + ) + + async def test_no_existing_wiki_passes(self) -> None: + # GIVEN a destination entity without an existing wiki + with patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + side_effect=SynapseHTTPError(response=Mock(status_code=404)), + ): + # WHEN I check the destination for an existing root wiki + # THEN no error should be raised so the copy creates a new root page + await _ensure_destination_has_no_root_wiki( + destination_owner_id="syn456", + synapse_client=self.syn, + ) + + async def test_root_wiki_check_error_propagates(self) -> None: + # GIVEN a root wiki lookup that fails with a non-404 error + with ( + patch( + "synapseclient.models.wiki.get_wiki_page", + new_callable=AsyncMock, + side_effect=SynapseHTTPError(response=Mock(status_code=403)), + ), + # WHEN I check the destination for an existing root wiki + # THEN the error should be re-raised + pytest.raises(SynapseHTTPError), + ): + await _ensure_destination_has_no_root_wiki( + destination_owner_id="syn456", + synapse_client=self.syn, + ) + + +class TestCollectWikiSubTreeHeaders: + """Tests for the _collect_wiki_sub_tree_headers helper function. + + The sample wiki header tree used by these tests: + + root (1) + |-- methods (2) + | |-- sequencing (4) + | | `-- deep (6) + | `-- analysis (5) + `-- results (3) + """ + + def get_wiki_headers(self) -> list: + return [ + {"id": "1", "title": "root"}, + {"id": "2", "title": "methods", "parentId": "1"}, + {"id": "4", "title": "sequencing", "parentId": "2"}, + {"id": "6", "title": "deep", "parentId": "4"}, + {"id": "5", "title": "analysis", "parentId": "2"}, + {"id": "3", "title": "results", "parentId": "1"}, + ] + + @pytest.mark.parametrize( + "sub_page_id,expected", + [ + ( + # Mid-level page: the page itself (re-rooted) plus all descendants + "2", + [ + {"id": "2", "title": "methods"}, + {"id": "4", "title": "sequencing", "parentId": "2"}, + {"id": "6", "title": "deep", "parentId": "4"}, + {"id": "5", "title": "analysis", "parentId": "2"}, + ], + ), + ( + # The root of the whole wiki: everything is returned + "1", + [ + {"id": "1", "title": "root"}, + {"id": "2", "title": "methods", "parentId": "1"}, + {"id": "4", "title": "sequencing", "parentId": "2"}, + {"id": "6", "title": "deep", "parentId": "4"}, + {"id": "5", "title": "analysis", "parentId": "2"}, + {"id": "3", "title": "results", "parentId": "1"}, + ], + ), + ( + # A leaf page: only the page itself, re-rooted + "6", + [{"id": "6", "title": "deep"}], + ), + ( + # An integer ID is coerced to a string before matching + 2, + [ + {"id": "2", "title": "methods"}, + {"id": "4", "title": "sequencing", "parentId": "2"}, + {"id": "6", "title": "deep", "parentId": "4"}, + {"id": "5", "title": "analysis", "parentId": "2"}, + ], + ), + ], + ) + def test_collects_sub_tree(self, sub_page_id, expected) -> None: + # GIVEN a flat list of wiki headers + wiki_headers = self.get_wiki_headers() + + # WHEN I collect the sub-tree rooted at sub_page_id + result = _collect_wiki_sub_tree_headers( + wiki_headers=wiki_headers, + sub_page_id=sub_page_id, + ) + + # THEN only the requested page and its descendants are returned, + # with the requested page re-rooted (no parentId) + assert result == expected + + def test_parents_appear_before_children(self) -> None: + # GIVEN a flat list of wiki headers + wiki_headers = self.get_wiki_headers() + + # WHEN I collect a sub-tree with nested descendants + result = _collect_wiki_sub_tree_headers( + wiki_headers=wiki_headers, + sub_page_id="2", + ) + + # THEN every page with a parent appears after that parent in the list + positions = {header["id"]: index for index, header in enumerate(result)} + for header in result: + parent_id = header.get("parentId") + if parent_id is not None: + assert positions[parent_id] < positions[header["id"]] + + def test_unknown_sub_page_id_returns_none(self) -> None: + # GIVEN a flat list of wiki headers + wiki_headers = self.get_wiki_headers() + + # WHEN I collect a sub-tree for an ID that is not in the tree + result = _collect_wiki_sub_tree_headers( + wiki_headers=wiki_headers, + sub_page_id="999", + ) + + # THEN nothing is returned + assert result is None + + def test_input_headers_are_not_mutated(self) -> None: + # GIVEN a flat list of wiki headers + wiki_headers = self.get_wiki_headers() + original = copy.deepcopy(wiki_headers) + + # WHEN I collect a sub-tree whose root has a parentId + _collect_wiki_sub_tree_headers( + wiki_headers=wiki_headers, + sub_page_id="2", + ) + + # THEN the input headers are unchanged, including the parentId + # of the requested page + assert wiki_headers == original + + +class TestUpdateInternalLinks: + """Tests for the _update_internal_links helper function. + + The scenario used by these tests: a wiki was copied from syn123 to syn456, + where source page 8688 became 9901, page 8689 became 9902, and page 8 + became 1000. + """ + + wiki_id_map = {"8688": "9901", "8689": "9902", "8": "1000"} + + def get_new_wikis(self, markdown) -> dict[str, WikiPage]: + return { + "9901": WikiPage(owner_id="syn456", id="9901", markdown=markdown), + "9902": WikiPage(owner_id="syn456", id="9902", markdown=""), + "1000": WikiPage(owner_id="syn456", id="1000", markdown=""), + } + + @pytest.mark.parametrize( + "markdown,expected", + [ + ( + # A link to a copied page is retargeted to the copy + "See syn123/wiki/8688 for details.", + "See syn456/wiki/9901 for details.", + ), + ( + # Multiple links in one page are all retargeted + "syn123/wiki/8688 and syn123/wiki/8689", + "syn456/wiki/9901 and syn456/wiki/9902", + ), + ( + # The rule for page 8 must not clobber the longer page ID 8688, + # and a link to page 8 itself is still retargeted + "syn123/wiki/8688 then syn123/wiki/8 end", + "syn456/wiki/9901 then syn456/wiki/1000 end", + ), + ( + # A link to a page that was not copied keeps its wiki ID but + # still gets the destination owner ID + "syn123/wiki/7777", + "syn456/wiki/7777", + ), + ( + # A bare reference to the source entity is replaced even when + # it is not a wiki link + "Data originally from syn123.", + "Data originally from syn456.", + ), + ( + # Markdown without any source references is left unchanged + "No links here.", + "No links here.", + ), + ( + # A longer entity ID that merely starts with the source owner + # ID must not be corrupted + "Related data in syn1234.", + "Related data in syn1234.", + ), + ], + ) + def test_rewrites_markdown(self, markdown: str, expected: str) -> None: + # GIVEN copied wiki pages where one page contains the markdown + new_wikis = self.get_new_wikis(markdown=markdown) + + # WHEN I update the internal links + result = _update_internal_links( + new_wikis=new_wikis, + wiki_id_map=self.wiki_id_map, + source_owner_id="syn123", + destination_owner_id="syn456", + ) + + # THEN the markdown points at the destination wiki + assert result["9901"].markdown == expected + + def test_updates_every_copied_page(self) -> None: + # GIVEN copied wiki pages that link to each other and to the source entity + new_wikis = { + "9901": WikiPage( + owner_id="syn456", id="9901", markdown="Next: syn123/wiki/8689" + ), + "9902": WikiPage( + owner_id="syn456", id="9902", markdown="Back: syn123/wiki/8688" + ), + "1000": WikiPage(owner_id="syn456", id="1000", markdown="Home: syn123"), + } + + # WHEN I update the internal links + result = _update_internal_links( + new_wikis=new_wikis, + wiki_id_map=self.wiki_id_map, + source_owner_id="syn123", + destination_owner_id="syn456", + ) + + # THEN every page's markdown is updated in place + assert result is new_wikis + assert new_wikis["9901"].markdown == "Next: syn456/wiki/9902" + assert new_wikis["9902"].markdown == "Back: syn456/wiki/9901" + assert new_wikis["1000"].markdown == "Home: syn456" + + def test_none_markdown_becomes_empty_string(self) -> None: + # GIVEN a copied wiki page without markdown + new_wikis = self.get_new_wikis(markdown=None) + + # WHEN I update the internal links + result = _update_internal_links( + new_wikis=new_wikis, + wiki_id_map=self.wiki_id_map, + source_owner_id="syn123", + destination_owner_id="syn456", + ) + + # THEN the markdown is normalized to an empty string without error + assert result["9901"].markdown == "" + + +class TestUpdateSynapseIdReferences: + """Tests for the _update_synapse_id_references helper function. + + The scenario used by these tests: a wiki was copied from one project to + another, where source wiki page 8688 became 9901 and page 8689 became + 9902. Alongside the wiki, entity syn111 was copied as syn999 and entity + syn1112 was copied as syn888. + """ + + wiki_id_map = {"8688": "9901", "8689": "9902"} + entity_map = {"syn111": "syn999", "syn1112": "syn888"} + + def get_new_wikis(self, markdown) -> dict: + return { + "9901": WikiPage(owner_id="syn456", id="9901", markdown=markdown), + "9902": WikiPage(owner_id="syn456", id="9902", markdown=""), + } + + @pytest.mark.parametrize( + "markdown,expected", + [ + ( + # A reference to a copied entity is rewritten to the copy + "Data in syn111.", + "Data in syn999.", + ), + ( + # The rule for syn111 must not clobber the longer ID syn1112, + # and both references are rewritten + "See syn111 and syn1112.", + "See syn999 and syn888.", + ), + ( + # An entity that is not in the entity map is left unchanged + "Not copied: syn777.", + "Not copied: syn777.", + ), + ( + # Markdown without any entity references is left unchanged + "No ids here.", + "No ids here.", + ), + ], + ) + def test_rewrites_markdown(self, markdown: str, expected: str) -> None: + # GIVEN copied wiki pages where one page contains the markdown + new_wikis = self.get_new_wikis(markdown=markdown) + + # WHEN I update the Synapse ID references + result = _update_synapse_id_references( + new_wikis=new_wikis, + wiki_id_map=self.wiki_id_map, + entity_map=self.entity_map, + ) + + # THEN the markdown points at the copied entities + assert result["9901"].markdown == expected + + def test_updates_every_copied_page(self) -> None: + # GIVEN copied wiki pages that both reference copied entities + new_wikis = { + "9901": WikiPage(owner_id="syn456", id="9901", markdown="Raw data: syn111"), + "9902": WikiPage( + owner_id="syn456", id="9902", markdown="Results table: syn1112" + ), + } + + # WHEN I update the Synapse ID references + result = _update_synapse_id_references( + new_wikis=new_wikis, + wiki_id_map=self.wiki_id_map, + entity_map=self.entity_map, + ) + + # THEN every page's markdown is updated in place + assert result is new_wikis + assert new_wikis["9901"].markdown == "Raw data: syn999" + assert new_wikis["9902"].markdown == "Results table: syn888" + + def test_none_markdown_becomes_empty_string(self) -> None: + # GIVEN a copied wiki page without markdown + new_wikis = self.get_new_wikis(markdown=None) + + # WHEN I update the Synapse ID references + result = _update_synapse_id_references( + new_wikis=new_wikis, + wiki_id_map=self.wiki_id_map, + entity_map=self.entity_map, + ) + + # THEN the markdown is normalized to an empty string without error + assert result["9901"].markdown == "" + + +class TestCopyWikiPages: + """Tests for the _copy_wiki_pages helper function. + + These tests stub out the network boundary (get_async, markdown download, + attachment copy, markdown file handle upload, and the wiki create/update + calls) so the helper's own bookkeeping can be checked: the old-to-new ID + map it builds, which pages are created versus written into an existing + destination page, and the parent links it sends for child pages. + + The stubs are deterministic functions of a page's ID: get_async returns a + source page whose title is title-, its markdown is md-, and its + copied attachment file handle is fh-. New IDs assigned by the create + call are looked up from new_id_by_title. + """ + + @pytest.fixture(autouse=True, scope="function") + def init_syn(self, syn: Synapse) -> None: + self.syn = syn + + @staticmethod + @contextlib.contextmanager + def _patched_copy(new_id_by_title: dict): + """Patch the network boundary of _copy_wiki_pages. + + Arguments: + new_id_by_title: Maps the title sent to post_wiki_page to the new + wiki ID the server should return for it. + + Yields: + A dict with the post_wiki_page and put_wiki_page mocks under the + keys "post" and "put". + """ + + def fake_get(self, *args, **kwargs): + self.title = f"title-{self.id}" + self.markdown = f"md-{self.id}" + return self + + def fake_markdown(self, *args, **kwargs): + return f"md-{self.id}" + + def fake_attachments(self, *args, **kwargs): + return [f"fh-{self.id}"] + + def fake_get_fh(self, *args, **kwargs): + return self + + def fake_post(**kwargs): + request = kwargs["request"] + title = request["title"] + return { + "id": new_id_by_title[title], + "title": title, + "parentWikiId": request.get("parentWikiId"), + } + + def fake_put(**kwargs): + request = kwargs["request"] + return { + "id": kwargs["wiki_id"], + "title": request["title"], + "parentWikiId": request.get("parentWikiId"), + } + + with ( + patch.object(WikiPage, "get_async", autospec=True, side_effect=fake_get), + patch.object( + WikiPage, + "_get_markdown_text", + autospec=True, + side_effect=fake_markdown, + ), + patch.object( + WikiPage, + "_copy_attachment_file_handles", + autospec=True, + side_effect=fake_attachments, + ), + patch.object( + WikiPage, + "_get_markdown_file_handle", + autospec=True, + side_effect=fake_get_fh, + ), + patch( + "synapseclient.models.wiki.post_wiki_page", + new_callable=AsyncMock, + side_effect=fake_post, + ) as mock_post, + patch( + "synapseclient.models.wiki.put_wiki_page", + new_callable=AsyncMock, + side_effect=fake_put, + ) as mock_put, + ): + yield {"post": mock_post, "put": mock_put} + + async def test_copies_whole_tree_preserving_hierarchy(self) -> None: + # GIVEN a three level wiki tree with root -> methods -> analysis + headers = [ + {"id": "1"}, + {"id": "2", "parentId": "1"}, + {"id": "3", "parentId": "2"}, + ] + new_id_by_title = { + "title-1": "new1", + "title-2": "new2", + "title-3": "new3", + } + + # WHEN copying the whole tree with no existing destination page + with self._patched_copy(new_id_by_title) as mocks: + new_wikis, wiki_id_map = await _copy_wiki_pages( + old_wiki_headers=headers, + source_owner_id="syn123", + destination_owner_id="syn456", + destination_wiki_page=None, + destination_sub_page_id=None, + synapse_client=self.syn, + ) + + # THEN every source page is mapped to its new ID and returned keyed + # by that new ID + assert wiki_id_map == {"1": "new1", "2": "new2", "3": "new3"} + assert set(new_wikis) == {"new1", "new2", "new3"} + + # AND all three pages are created with post, none with put + assert mocks["put"].call_count == 0 + requests = { + mock_call.kwargs["request"]["title"]: mock_call.kwargs["request"] + for mock_call in mocks["post"].call_args_list + } + # AND the root is created as a root page with no parent + assert "parentWikiId" not in requests["title-1"] + # AND each child is linked to the new ID of its parent + assert requests["title-2"]["parentWikiId"] == "new1" + assert requests["title-3"]["parentWikiId"] == "new2" + # AND each page carries its own copied attachment file handle + assert requests["title-1"]["attachmentFileHandleIds"] == ["fh-1"] + assert requests["title-2"]["attachmentFileHandleIds"] == ["fh-2"] + assert requests["title-3"]["attachmentFileHandleIds"] == ["fh-3"] + + async def test_root_created_under_destination_sub_page_id(self) -> None: + # GIVEN a single root page and a destination sub page to nest it under + headers = [{"id": "1"}] + + # WHEN copying with destination_sub_page_id but no existing page object + with self._patched_copy({"title-1": "new1"}) as mocks: + _, wiki_id_map = await _copy_wiki_pages( + old_wiki_headers=headers, + source_owner_id="syn123", + destination_owner_id="syn456", + destination_wiki_page=None, + destination_sub_page_id="900", + synapse_client=self.syn, + ) + + # THEN the copied root is created beneath the destination sub page + assert wiki_id_map == {"1": "new1"} + assert mocks["put"].call_count == 0 + assert mocks["post"].call_args.kwargs["request"]["parentWikiId"] == "900" + + async def test_root_written_into_existing_destination_page(self) -> None: + # GIVEN a root -> child tree and an existing destination page + headers = [{"id": "1"}, {"id": "2", "parentId": "1"}] + destination_page = WikiPage(owner_id="syn456", id="500") + + # WHEN copying into that existing destination page + with self._patched_copy({"title-2": "new2"}) as mocks: + new_wikis, wiki_id_map = await _copy_wiki_pages( + old_wiki_headers=headers, + source_owner_id="syn123", + destination_owner_id="syn456", + destination_wiki_page=destination_page, + destination_sub_page_id="500", + synapse_client=self.syn, + ) + + # THEN the root maps to the existing page ID and the child to a new ID + assert wiki_id_map == {"1": "500", "2": "new2"} + assert set(new_wikis) == {"500", "new2"} + + # AND the root is written with put into the existing page + mocks["put"].assert_called_once() + put_request = mocks["put"].call_args.kwargs + assert put_request["wiki_id"] == "500" + assert put_request["request"]["title"] == "title-1" + assert put_request["request"]["markdown"] == "md-1" + + # AND the child is created with post and linked to the existing page + mocks["post"].assert_called_once() + assert mocks["post"].call_args.kwargs["request"]["parentWikiId"] == "500" + + async def test_returns_empty_when_no_headers(self) -> None: + # GIVEN no wiki headers to copy + # WHEN copying + with self._patched_copy({}) as mocks: + new_wikis, wiki_id_map = await _copy_wiki_pages( + old_wiki_headers=[], + source_owner_id="syn123", + destination_owner_id="syn456", + destination_wiki_page=None, + destination_sub_page_id=None, + synapse_client=self.syn, + ) + + # THEN nothing is created and empty results are returned + assert new_wikis == {} + assert wiki_id_map == {} + assert mocks["post"].call_count == 0 + assert mocks["put"].call_count == 0 From 67d79fc3d296c3afce314499a31e81def7132e3a Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 30 Jul 2026 08:35:40 -0700 Subject: [PATCH 2/7] post_file_handles_copy now copies in batches --- synapseclient/api/file_services.py | 42 ++-- .../api/unit_test_file_services.py | 183 ++++++++++++++++++ 2 files changed, 214 insertions(+), 11 deletions(-) create mode 100644 tests/unit/synapseclient/api/unit_test_file_services.py diff --git a/synapseclient/api/file_services.py b/synapseclient/api/file_services.py index 58ca67c2b..dd719433c 100644 --- a/synapseclient/api/file_services.py +++ b/synapseclient/api/file_services.py @@ -2,6 +2,7 @@ """ +import asyncio import json import mimetypes import os @@ -552,8 +553,9 @@ async def post_file_handles_copy( synapse_client: Optional["Synapse"] = None, ) -> list[dict[str, Any]]: """ - Copy a batch of file handles. Requests are automatically submitted in batches - of MAX_FILE_HANDLE_PER_COPY_REQUEST. + Copy a batch of file handles. Requests are automatically split into batches of + MAX_FILE_HANDLE_PER_COPY_REQUEST, and the batches are submitted concurrently, + with at most max_threads requests in flight at a time. @@ -566,7 +568,8 @@ async def post_file_handles_copy( Returns: A list of copy results matching - . + , + in the same order as copy_requests. Failed copies include a failureCode of UNAUTHORIZED or NOT_FOUND. """ from synapseclient import Synapse @@ -574,13 +577,30 @@ async def post_file_handles_copy( client = Synapse.get_client(synapse_client=synapse_client) - copy_results = [] - for start in range(0, len(copy_requests), MAX_FILE_HANDLE_PER_COPY_REQUEST): - batch = copy_requests[start : start + MAX_FILE_HANDLE_PER_COPY_REQUEST] - response = await client.rest_post_async( - "/filehandles/copy", - body=json.dumps({"copyRequests": batch}), - endpoint=client.fileHandleEndpoint, + if not copy_requests: + return [] + + semaphore = asyncio.Semaphore(client.max_threads) + + async def copy_batch(batch: list[dict[str, Any]]) -> list[dict[str, Any]]: + async with semaphore: + response = await client.rest_post_async( + "/filehandles/copy", + body=json.dumps({"copyRequests": batch}), + endpoint=client.fileHandleEndpoint, + ) + return response.get("copyResults", []) + + tasks = [ + asyncio.create_task( + copy_batch(copy_requests[start : start + MAX_FILE_HANDLE_PER_COPY_REQUEST]) ) - copy_results.extend(response.get("copyResults", [])) + for start in range(0, len(copy_requests), MAX_FILE_HANDLE_PER_COPY_REQUEST) + ] + + batched_results = await asyncio.gather(*tasks) + + copy_results = [] + for batch_results in batched_results: + copy_results.extend(batch_results) return copy_results diff --git a/tests/unit/synapseclient/api/unit_test_file_services.py b/tests/unit/synapseclient/api/unit_test_file_services.py new file mode 100644 index 000000000..9c0419c8d --- /dev/null +++ b/tests/unit/synapseclient/api/unit_test_file_services.py @@ -0,0 +1,183 @@ +"""Unit tests for file_services utility functions.""" + +import asyncio +import json +from unittest.mock import AsyncMock, patch + +import synapseclient.api.file_services as file_services +from synapseclient.core.constants.limits import MAX_FILE_HANDLE_PER_COPY_REQUEST + +FILE_HANDLE_ENDPOINT = "https://repo-prod.prod.sagebase.org/file/v1" +MAX_THREADS = 4 + + +def copy_request(index: int) -> dict: + """Build a file handle copy request for the file handle with the given index.""" + return { + "originalFile": { + "fileHandleId": str(index), + "associateObjectId": "syn123", + "associateObjectType": "WikiAttachment", + } + } + + +def copy_result(index: int) -> dict: + """Build the copy result that the API would return for the given index.""" + return { + "originalFileHandleId": str(index), + "newFileHandle": {"id": f"new-{index}"}, + } + + +def mock_client() -> AsyncMock: + """Build a mock Synapse client for the file handle copy endpoint.""" + client = AsyncMock() + client.fileHandleEndpoint = FILE_HANDLE_ENDPOINT + # max_threads sizes the concurrency semaphore, so it must be a real int + client.max_threads = MAX_THREADS + return client + + +def requested_file_handle_ids(mock_rest_post: AsyncMock) -> list[list[str]]: + """Extract the file handle IDs of each request sent to the copy endpoint.""" + return [ + [ + request["originalFile"]["fileHandleId"] + for request in json.loads(call.kwargs["body"])["copyRequests"] + ] + for call in mock_rest_post.call_args_list + ] + + +class TestPostFileHandlesCopy: + """Tests for post_file_handles_copy function.""" + + @patch("synapseclient.Synapse") + async def test_single_batch(self, mock_synapse): + """Test that a batch under the limit is sent as one request.""" + # GIVEN a mock client that copies the requested file handles + client = mock_client() + mock_synapse.get_client.return_value = client + client.rest_post_async.return_value = { + "copyResults": [copy_result(0), copy_result(1)] + } + + # WHEN I copy two file handles + results = await file_services.post_file_handles_copy( + copy_requests=[copy_request(0), copy_request(1)] + ) + + # THEN the copy results are returned + assert results == [copy_result(0), copy_result(1)] + + # AND a single request was sent to the file handle endpoint + client.rest_post_async.assert_called_once_with( + "/filehandles/copy", + body=json.dumps( + {"copyRequests": [copy_request(0), copy_request(1)]}, + ), + endpoint=FILE_HANDLE_ENDPOINT, + ) + + @patch("synapseclient.Synapse") + async def test_empty_requests(self, mock_synapse): + """Test that no request is sent when there is nothing to copy.""" + # GIVEN a mock client + client = mock_client() + mock_synapse.get_client.return_value = client + + # WHEN I copy an empty list of file handles + results = await file_services.post_file_handles_copy(copy_requests=[]) + + # THEN no results are returned + assert results == [] + + # AND no request was sent + client.rest_post_async.assert_not_called() + + @patch("synapseclient.Synapse") + async def test_batches_are_split_and_results_stay_ordered(self, mock_synapse): + """Test that oversized requests are split and results keep their order.""" + # GIVEN more copy requests than fit in a single request + total = MAX_FILE_HANDLE_PER_COPY_REQUEST * 2 + 1 + copy_requests = [copy_request(index) for index in range(total)] + + # AND a mock client that responds to later batches first + client = mock_client() + mock_synapse.get_client.return_value = client + + async def respond(*_, body: str, **__) -> dict: + requests = json.loads(body)["copyRequests"] + first_id = int(requests[0]["originalFile"]["fileHandleId"]) + # Sleep longer for earlier batches so that responses arrive out of order + await asyncio.sleep((total - first_id) / total / 100) + return { + "copyResults": [ + copy_result(int(request["originalFile"]["fileHandleId"])) + for request in requests + ] + } + + client.rest_post_async.side_effect = respond + + # WHEN I copy the file handles + results = await file_services.post_file_handles_copy( + copy_requests=copy_requests + ) + + # THEN the results are in the order of the requests + assert results == [copy_result(index) for index in range(total)] + + # AND the requests were split into full batches plus a remainder + assert requested_file_handle_ids(client.rest_post_async) == [ + [ + str(index) + for index in range( + start, min(start + MAX_FILE_HANDLE_PER_COPY_REQUEST, total) + ) + ] + for start in range(0, total, MAX_FILE_HANDLE_PER_COPY_REQUEST) + ] + + @patch("synapseclient.Synapse") + async def test_batches_are_sent_concurrently(self, mock_synapse): + """Test that batches are in flight together, up to max_threads.""" + # GIVEN enough copy requests to fill more batches than max_threads allows + batches = MAX_THREADS + 2 + copy_requests = [ + copy_request(index) + for index in range(MAX_FILE_HANDLE_PER_COPY_REQUEST * batches) + ] + + # AND a mock client that tracks how many requests are in flight at once + client = mock_client() + mock_synapse.get_client.return_value = client + in_flight = 0 + max_in_flight = 0 + + async def respond(*_, body: str, **__) -> dict: + nonlocal in_flight, max_in_flight + in_flight += 1 + max_in_flight = max(max_in_flight, in_flight) + try: + await asyncio.sleep(0.01) + return { + "copyResults": [ + copy_result(int(request["originalFile"]["fileHandleId"])) + for request in json.loads(body)["copyRequests"] + ] + } + finally: + in_flight -= 1 + + client.rest_post_async.side_effect = respond + + # WHEN I copy the file handles + await file_services.post_file_handles_copy(copy_requests=copy_requests) + + # THEN every batch was sent + assert client.rest_post_async.call_count == batches + + # AND they were sent concurrently, without exceeding max_threads + assert max_in_flight == MAX_THREADS From d757e9ba1dd2f633ba2e955d4df156a8b99e787c Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 30 Jul 2026 08:44:20 -0700 Subject: [PATCH 3/7] Run synchronous wiki downloads in a worker thread download_from_url is synchronous and its HTTP path blocks on requests, so calling it directly from the async wiki methods stalled the asyncio event loop for the duration of the transfer. Route the four call sites through loop.run_in_executor with the client thread pool, matching the pattern already used in download_by_file_handle. Co-Authored-By: Claude Opus 5 (1M context) --- synapseclient/models/wiki.py | 64 +++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/synapseclient/models/wiki.py b/synapseclient/models/wiki.py index 484dc3ced..41ed86593 100644 --- a/synapseclient/models/wiki.py +++ b/synapseclient/models/wiki.py @@ -1192,11 +1192,17 @@ async def get_attachment_async( ) file_size = int(WikiPage._get_file_size(filehandle_dict, file_name)) if file_size < SINGLE_THREAD_DOWNLOAD_SIZE_LIMIT: - downloaded_file_path = download_from_url( - url=presigned_url_info.url, - destination=download_location, - url_is_presigned=True, - synapse_client=client, + # download_from_url is synchronous, run it in a worker thread so that + # the blocking HTTP request does not stall the asyncio event loop + loop = asyncio.get_running_loop() + downloaded_file_path = await loop.run_in_executor( + client._get_thread_pool_executor(asyncio_event_loop=loop), + lambda: download_from_url( + url=presigned_url_info.url, + destination=download_location, + url_is_presigned=True, + synapse_client=client, + ), ) else: downloaded_file_path = await download_from_url_multi_threaded( @@ -1289,11 +1295,17 @@ async def get_attachment_preview_async( ) file_size = int(WikiPage._get_file_size(filehandle_dict, file_name)) if file_size < SINGLE_THREAD_DOWNLOAD_SIZE_LIMIT: - downloaded_file_path = download_from_url( - url=presigned_url_info.url, - destination=download_location, - url_is_presigned=True, - synapse_client=client, + # download_from_url is synchronous, run it in a worker thread so that + # the blocking HTTP request does not stall the asyncio event loop + loop = asyncio.get_running_loop() + downloaded_file_path = await loop.run_in_executor( + client._get_thread_pool_executor(asyncio_event_loop=loop), + lambda: download_from_url( + url=presigned_url_info.url, + destination=download_location, + url_is_presigned=True, + synapse_client=client, + ), ) else: downloaded_file_path = await download_from_url_multi_threaded( @@ -1359,11 +1371,17 @@ async def get_markdown_file_async( if not download_location: raise ValueError("Must provide download_location to download a file.") - downloaded_file_path = download_from_url( - url=markdown_url, - destination=download_location, - url_is_presigned=True, - synapse_client=client, + # download_from_url is synchronous, run it in a worker thread so that the + # blocking HTTP request does not stall the asyncio event loop + loop = asyncio.get_running_loop() + downloaded_file_path = await loop.run_in_executor( + client._get_thread_pool_executor(asyncio_event_loop=loop), + lambda: download_from_url( + url=markdown_url, + destination=download_location, + url_is_presigned=True, + synapse_client=client, + ), ) unzipped_file_path = WikiPage.unzip_gzipped_file(downloaded_file_path) client.logger.info( @@ -1393,11 +1411,17 @@ async def _get_markdown_text(self, synapse_client: Synapse) -> str: cache_dir = os.path.join(synapse_client.cache.cache_root_dir, "wiki_content") if not os.path.exists(cache_dir): os.makedirs(cache_dir) - downloaded_file_path = download_from_url( - url=markdown_url, - destination=cache_dir, - url_is_presigned=True, - synapse_client=synapse_client, + # download_from_url is synchronous, run it in a worker thread so that the + # blocking HTTP request does not stall the asyncio event loop + loop = asyncio.get_running_loop() + downloaded_file_path = await loop.run_in_executor( + synapse_client._get_thread_pool_executor(asyncio_event_loop=loop), + lambda: download_from_url( + url=markdown_url, + destination=cache_dir, + url_is_presigned=True, + synapse_client=synapse_client, + ), ) try: with gzip.open(downloaded_file_path, "rt", encoding="utf-8") as f_in: From 9e72b7d7003ad98bed0fa869a54da05d8e85bdb6 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 30 Jul 2026 08:51:59 -0700 Subject: [PATCH 4/7] Add identifying IDs to wiki log messages and demote copy internals to debug Every log statement in wiki.py now carries an owner/wiki ID prefix so messages can be attributed to a specific wiki page. The per-page copy chatter is demoted from info to debug, leaving info for the higher level operations. Co-Authored-By: Claude Opus 5 (1M context) --- synapseclient/models/wiki.py | 83 ++++++++++++++----- .../models/async/unit_test_wiki_async.py | 52 +++++++----- 2 files changed, 92 insertions(+), 43 deletions(-) diff --git a/synapseclient/models/wiki.py b/synapseclient/models/wiki.py index 41ed86593..bae54dbb1 100644 --- a/synapseclient/models/wiki.py +++ b/synapseclient/models/wiki.py @@ -747,13 +747,16 @@ async def _get_markdown_file_handle(self, synapse_client: Synapse) -> "WikiPage" path=file_path, ) synapse_client.logger.info( - f"Uploaded file handle {file_handle.get('id')} for wiki page markdown." + f"[{self.owner_id}:{self.id}]: Uploaded file handle " + f"{file_handle.get('id')} for wiki page markdown." ) self.markdown_file_handle_id = file_handle.get("id") finally: if os.path.exists(file_path): os.remove(file_path) - synapse_client.logger.debug(f"Deleted temp directory {file_path}") + synapse_client.logger.debug( + f"[{self.owner_id}:{self.id}]: Deleted temp directory {file_path}" + ) return self @otel_trace_method( @@ -788,14 +791,16 @@ async def task_of_uploading_attachment(attachment: str) -> tuple[str, str]: path=file_path, ) synapse_client.logger.info( - f"Uploaded file handle {file_handle.get('id')} for wiki page attachment." + f"[{self.owner_id}:{self.id}]: Uploaded file handle " + f"{file_handle.get('id')} for wiki page attachment." ) return file_handle.get("id") finally: if os.path.exists(file_path): os.remove(file_path) synapse_client.logger.debug( - f"Deleted temp directory {file_path}" + f"[{self.owner_id}:{self.id}]: Deleted temp directory " + f"{file_path}" ) tasks = [ @@ -891,7 +896,8 @@ async def store_async( if wiki_action == "create_root_wiki_page": client.logger.info( - "No wiki page exists within the owner. Create a new wiki page." + f"[{self.owner_id}]: No wiki page exists within the owner. " + "Create a new wiki page." ) wiki_data = await post_wiki_page( owner_id=self.owner_id, @@ -899,11 +905,13 @@ async def store_async( synapse_client=client, ) client.logger.info( - f"Created wiki page: {wiki_data.get('title')} with ID: {wiki_data.get('id')}." + f"[{self.owner_id}]: Created wiki page: {wiki_data.get('title')} " + f"with ID: {wiki_data.get('id')}." ) elif wiki_action == "update_existing_wiki_page": client.logger.info( - "A wiki page already exists within the owner. Update the existing wiki page." + f"[{self.owner_id}:{self.id}]: A wiki page already exists within the " + "owner. Update the existing wiki page." ) existing_wiki_dict = await get_wiki_page( owner_id=self.owner_id, @@ -931,12 +939,14 @@ async def store_async( synapse_client=client, ) client.logger.info( - f"Updated wiki page: {wiki_data.get('title')} with ID: {wiki_data.get('id')}." + f"[{self.owner_id}]: Updated wiki page: {wiki_data.get('title')} " + f"with ID: {wiki_data.get('id')}." ) else: client.logger.info( - f"Creating sub-wiki page under parent ID: {self.parent_id}" + f"[{self.owner_id}]: Creating sub-wiki page under parent ID: " + f"{self.parent_id}" ) wiki_data = await post_wiki_page( owner_id=self.owner_id, @@ -944,7 +954,8 @@ async def store_async( synapse_client=client, ) client.logger.info( - f"Created sub-wiki page: {wiki_data.get('title')} with ID: {wiki_data.get('id')} under parent: {self.parent_id}" + f"[{self.owner_id}]: Created sub-wiki page: {wiki_data.get('title')} " + f"with ID: {wiki_data.get('id')} under parent: {self.parent_id}" ) self.fill_from_dict(wiki_data) return self @@ -1212,10 +1223,15 @@ async def get_attachment_async( ) unzipped_file_path = WikiPage.unzip_gzipped_file(downloaded_file_path) client.logger.info( - f"Downloaded file {presigned_url_info.file_name.replace('.gz', '')} to {unzipped_file_path}." + f"[{self.owner_id}:{self.id}]: Downloaded file " + f"{presigned_url_info.file_name.replace('.gz', '')} to " + f"{unzipped_file_path}." ) os.remove(downloaded_file_path) - client.logger.debug(f"Removed the gzipped file {downloaded_file_path}.") + client.logger.debug( + f"[{self.owner_id}:{self.id}]: Removed the gzipped file " + f"{downloaded_file_path}." + ) return unzipped_file_path else: return attachment_url @@ -1314,7 +1330,9 @@ async def get_attachment_preview_async( synapse_client=client, ) client.logger.info( - f"Downloaded the preview file {presigned_url_info.file_name.replace('.gz', '')} to {downloaded_file_path}." + f"[{self.owner_id}:{self.id}]: Downloaded the preview file " + f"{presigned_url_info.file_name.replace('.gz', '')} to " + f"{downloaded_file_path}." ) return downloaded_file_path else: @@ -1385,10 +1403,14 @@ async def get_markdown_file_async( ) unzipped_file_path = WikiPage.unzip_gzipped_file(downloaded_file_path) client.logger.info( - f"Downloaded and unzipped the markdown file for wiki page {self.id} to {unzipped_file_path}." + f"[{self.owner_id}:{self.id}]: Downloaded and unzipped the markdown " + f"file to {unzipped_file_path}." ) os.remove(downloaded_file_path) - client.logger.debug(f"Removed the gzipped file {downloaded_file_path}.") + client.logger.debug( + f"[{self.owner_id}:{self.id}]: Removed the gzipped file " + f"{downloaded_file_path}." + ) return unzipped_file_path else: return markdown_url @@ -1623,26 +1645,39 @@ async def copy_async( ) if update_links: - client.logger.info("Updating internal links") + client.logger.debug( + f"[{self.owner_id} -> {destination_owner_id}]: Updating internal links" + ) new_wikis = _update_internal_links( new_wikis=new_wikis, wiki_id_map=wiki_id_map, source_owner_id=self.owner_id, destination_owner_id=destination_owner_id, ) - client.logger.info("Done updating internal links.") + client.logger.debug( + f"[{self.owner_id} -> {destination_owner_id}]: Done updating internal " + "links." + ) if entity_map: - client.logger.info("Updating Synapse references") + client.logger.debug( + f"[{self.owner_id} -> {destination_owner_id}]: Updating Synapse " + "references" + ) new_wikis = _update_synapse_id_references( new_wikis=new_wikis, wiki_id_map=wiki_id_map, entity_map=entity_map, ) - client.logger.info("Done updating Synapse IDs.") + client.logger.debug( + f"[{self.owner_id} -> {destination_owner_id}]: Done updating Synapse " + "IDs." + ) if update_links or entity_map: - client.logger.info("Storing new wiki pages") + client.logger.debug( + f"[{self.owner_id} -> {destination_owner_id}]: Storing new wiki pages" + ) for new_wiki_id in wiki_id_map.values(): new_wiki = new_wikis[new_wiki_id] new_wiki = await new_wiki._get_markdown_file_handle( @@ -1655,7 +1690,9 @@ async def copy_async( synapse_client=client, ) new_wikis[new_wiki_id] = new_wiki.fill_from_dict(synapse_wiki=wiki_data) - client.logger.info(f"Stored wiki page {new_wiki_id}") + client.logger.debug( + f"[{destination_owner_id}:{new_wiki_id}]: Stored wiki page" + ) new_wiki_headers = [] async for item in get_wiki_header_tree( @@ -1709,7 +1746,9 @@ async def _copy_wiki_pages( old_wiki = await WikiPage( owner_id=source_owner_id, id=wiki_header["id"] ).get_async(synapse_client=synapse_client) - synapse_client.logger.info(f"Got wiki {wiki_header['id']}") + synapse_client.logger.debug( + f"[{source_owner_id}:{wiki_header['id']}]: Got wiki page to copy" + ) markdown = await old_wiki._get_markdown_text(synapse_client=synapse_client) new_file_handle_ids = await old_wiki._copy_attachment_file_handles( synapse_client=synapse_client diff --git a/tests/unit/synapseclient/models/async/unit_test_wiki_async.py b/tests/unit/synapseclient/models/async/unit_test_wiki_async.py index a720d2cd8..29326cad8 100644 --- a/tests/unit/synapseclient/models/async/unit_test_wiki_async.py +++ b/tests/unit/synapseclient/models/async/unit_test_wiki_async.py @@ -708,10 +708,10 @@ async def test_get_markdown_file_handle_success_with_markdown(self) -> WikiPage: path="test.txt.gz", ) mock_logger_info.assert_called_once_with( - "Uploaded file handle handle1 for wiki page markdown." + "[syn123:wiki1]: Uploaded file handle handle1 for wiki page markdown." ) mock_logger_debug.assert_called_once_with( - "Deleted temp directory test.txt.gz" + "[syn123:wiki1]: Deleted temp directory test.txt.gz" ) # AND the temp gzipped file should be deleted assert mock_remove.call_count == 1 @@ -790,14 +790,22 @@ async def test_get_attachment_file_handles_success_multiple_attachments( ) mock_logger_info.assert_has_calls( [ - call("Uploaded file handle handle1 for wiki page attachment."), - call("Uploaded file handle handle2 for wiki page attachment."), + call( + "[syn123:wiki1]: Uploaded file handle handle1 for wiki page attachment." + ), + call( + "[syn123:wiki1]: Uploaded file handle handle2 for wiki page attachment." + ), ] ) mock_logger_debug.assert_has_calls( [ - call("Deleted temp directory /tmp/cache1/test_1.txt.gz"), - call("Deleted temp directory /tmp/cache1/test_2.txt.gz"), + call( + "[syn123:wiki1]: Deleted temp directory /tmp/cache1/test_1.txt.gz" + ), + call( + "[syn123:wiki1]: Deleted temp directory /tmp/cache1/test_2.txt.gz" + ), ] ) @@ -875,10 +883,10 @@ async def test_get_attachment_file_handles_single_attachment(self) -> WikiPage: path="/tmp/cache/test_1.txt.gz", ) mock_logger_info.assert_called_once_with( - "Uploaded file handle handle1 for wiki page attachment." + "[syn123:wiki1]: Uploaded file handle handle1 for wiki page attachment." ) mock_logger_debug.assert_called_once_with( - "Deleted temp directory /tmp/cache/test_1.txt.gz" + "[syn123:wiki1]: Deleted temp directory /tmp/cache/test_1.txt.gz" ) # AND the temp directory should be cleaned up mock_remove.assert_called_once_with("/tmp/cache/test_1.txt.gz") @@ -922,7 +930,7 @@ async def test_get_attachment_file_handles_cache_dir_not_exists(self) -> WikiPag # THEN the function should complete successfully assert results.attachment_file_handle_ids == ["handle1"] mock_logger_info.assert_called_once_with( - "Uploaded file handle handle1 for wiki page attachment." + "[syn123:wiki1]: Uploaded file handle handle1 for wiki page attachment." ) mock_logger_debug.assert_not_called() # AND cleanup should not be attempted since directory doesn't exist @@ -959,7 +967,7 @@ async def test_get_attachment_file_handles_upload_failure(self) -> WikiPage: # AND cleanup should still be attempted mock_remove.assert_called_once_with("/tmp/cache/test_1.txt.gz") mock_logger_debug.assert_called_once_with( - "Deleted temp directory /tmp/cache/test_1.txt.gz" + "[syn123:wiki1]: Deleted temp directory /tmp/cache/test_1.txt.gz" ) async def test_determine_wiki_action_error_no_owner_id(self) -> None: @@ -1127,10 +1135,10 @@ async def test_store_async_new_root_wiki_success(self) -> None: mock_logger.assert_has_calls( [ call( - "No wiki page exists within the owner. Create a new wiki page." + "[syn123]: No wiki page exists within the owner. Create a new wiki page." ), call( - f"Created wiki page: {post_api_response['title']} with ID: {post_api_response['id']}." + f"[syn123]: Created wiki page: {post_api_response['title']} with ID: {post_api_response['id']}." ), ] ) @@ -1247,10 +1255,10 @@ async def test_store_async_update_existing_wiki_success(self) -> None: mock_logger.assert_has_calls( [ call( - "A wiki page already exists within the owner. Update the existing wiki page." + "[syn123:wiki1]: A wiki page already exists within the owner. Update the existing wiki page." ), call( - f"Updated wiki page: {mock_put_wiki_response['title']} with ID: {self.api_response['id']}." + f"[syn123]: Updated wiki page: {mock_put_wiki_response['title']} with ID: {self.api_response['id']}." ), ] ) @@ -1305,9 +1313,11 @@ async def test_store_async_create_sub_wiki_success(self) -> None: assert mock_logger.call_count == 2 mock_logger.assert_has_calls( [ - call("Creating sub-wiki page under parent ID: parent_wiki"), call( - f"Created sub-wiki page: {post_api_response['title']} with ID: {post_api_response['id']} under parent: parent_wiki" + "[syn123]: Creating sub-wiki page under parent ID: parent_wiki" + ), + call( + f"[syn123]: Created sub-wiki page: {post_api_response['title']} with ID: {post_api_response['id']} under parent: parent_wiki" ), ] ) @@ -1713,7 +1723,7 @@ async def test_get_attachment_async_download_file_success(self, file_size) -> No # AND debug log should be called once (only the general one) mock_logger_info.assert_called_once_with( - f"Downloaded file test.txt to {result}." + f"[syn123:wiki1]: Downloaded file test.txt to {result}." ) # AND the file should be unzipped mock_unzip_gzipped_file.assert_called_once_with("/tmp/download/test.txt.gz") @@ -1721,7 +1731,7 @@ async def test_get_attachment_async_download_file_success(self, file_size) -> No mock_remove.assert_called_once_with("/tmp/download/test.txt.gz") # AND debug log should be called mock_logger_debug.assert_called_once_with( - "Removed the gzipped file /tmp/download/test.txt.gz." + "[syn123:wiki1]: Removed the gzipped file /tmp/download/test.txt.gz." ) async def test_get_attachment_async_no_file_download(self) -> None: @@ -1913,7 +1923,7 @@ async def test_get_attachment_preview_async_download_file_success( # AND debug log should be called once (only the general one) mock_logger_info.assert_called_once_with( - f"Downloaded the preview file test.txt to {result}." + f"[syn123:wiki1]: Downloaded the preview file test.txt to {result}." ) async def test_get_attachment_preview_async_no_file_download(self) -> None: @@ -2049,13 +2059,13 @@ async def test_get_markdown_file_async_download_file_success(self) -> None: "/tmp/download/markdown.md.gz" ) mock_logger_info.assert_called_once_with( - f"Downloaded and unzipped the markdown file for wiki page wiki1 to {result}." + f"[syn123:wiki1]: Downloaded and unzipped the markdown file to {result}." ) # AND the gzipped file should be removed mock_remove.assert_called_once_with("/tmp/download/markdown.md.gz") # AND debug log should be called mock_logger_debug.assert_called_once_with( - f"Removed the gzipped file /tmp/download/markdown.md.gz." + f"[syn123:wiki1]: Removed the gzipped file /tmp/download/markdown.md.gz." ) async def test_get_markdown_file_async_no_file_download(self) -> None: From 1577efe921f1b99a021f4fc169e100a30938c994 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 30 Jul 2026 08:55:00 -0700 Subject: [PATCH 5/7] Drop float support from wiki sub page ID coercion Both sub page ID inputs are typed str | None in copy_async, so a float can only arrive from a caller ignoring the type hints. The float branch was speculative parity with the legacy copyWiki, which called int() on an untyped argument. Int is still accepted since wiki page IDs are numeric. Co-Authored-By: Claude Opus 5 (1M context) --- synapseclient/models/wiki.py | 15 ++++++--------- .../models/async/unit_test_wiki_async.py | 9 ++++----- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/synapseclient/models/wiki.py b/synapseclient/models/wiki.py index bae54dbb1..9b1b809c4 100644 --- a/synapseclient/models/wiki.py +++ b/synapseclient/models/wiki.py @@ -1814,14 +1814,13 @@ async def _copy_wiki_pages( def _coerce_sub_page_id( - sub_page_id: str | int | float | None, argument_description: str + sub_page_id: str | int | None, argument_description: str ) -> str | None: """Coerce a wiki sub page ID to an integer string. - Rejects values that a plain int() conversion would silently mangle: - fractional floats truncate, and bools and negative numbers convert to - nonsense page IDs. Integer-valued floats such as 4.0 are accepted since - they convert losslessly. + Wiki page IDs are numeric strings, but an integer is accepted as a + convenience. Values that a plain int() conversion would silently mangle + are rejected: bools and negative numbers convert to nonsense page IDs. Arguments: sub_page_id: The wiki page ID to coerce. @@ -1836,8 +1835,6 @@ def _coerce_sub_page_id( """ if sub_page_id is None: return None - if isinstance(sub_page_id, float) and sub_page_id.is_integer(): - sub_page_id = int(sub_page_id) if isinstance(sub_page_id, bool) or not str(sub_page_id).isdecimal(): raise ValueError( f"{argument_description} must be a numeric wiki page ID or None, " @@ -1849,8 +1846,8 @@ def _coerce_sub_page_id( def _validate_and_format_copy_inputs( owner_id: str | None, destination_owner_id: str, - entity_sub_page_id: str | int | float | None, - destination_sub_page_id: str | int | float | None, + entity_sub_page_id: str | int | None, + destination_sub_page_id: str | int | None, entity_map: dict[str, str] | None = None, ) -> tuple[str | None, str | None]: """Validate the inputs of a wiki copy and coerce the sub page IDs. diff --git a/tests/unit/synapseclient/models/async/unit_test_wiki_async.py b/tests/unit/synapseclient/models/async/unit_test_wiki_async.py index 29326cad8..ac425d92b 100644 --- a/tests/unit/synapseclient/models/async/unit_test_wiki_async.py +++ b/tests/unit/synapseclient/models/async/unit_test_wiki_async.py @@ -2333,12 +2333,12 @@ async def test_copy_async_sub_page_id_on_source_without_wiki_raises_value_error( @pytest.mark.parametrize( "entity_sub_page_id,destination_sub_page_id", - [("8688", "4"), (8688.0, 4.0)], + [("8688", "4"), (8688, 4)], ) async def test_copy_async_sub_page_id_coercion( self, entity_sub_page_id, destination_sub_page_id ) -> None: - # GIVEN a copy request with sub page IDs given as numeric strings or floats + # GIVEN a copy request with sub page IDs given as numeric strings or integers headers = [{"id": "8688", "title": "Root"}] with ( patch( @@ -2639,7 +2639,6 @@ class TestValidateAndFormatCopyInputs: ("8688", None, ("8688", None)), (None, "4", (None, "4")), (8688, 4, ("8688", "4")), - (8688.0, 4.0, ("8688", "4")), ], ) def test_valid_inputs_return_coerced_sub_page_ids( @@ -2730,7 +2729,7 @@ def test_invalid_entity_map_raises_value_error(self, entity_map, offender) -> No [ ("some_string", None, "The id of the WikiPage"), (None, "some_string", "destination_sub_page_id"), - (8688.9, None, "The id of the WikiPage"), + (8688.0, None, "The id of the WikiPage"), (None, 4.9, "destination_sub_page_id"), (True, None, "The id of the WikiPage"), (None, True, "destination_sub_page_id"), @@ -2741,7 +2740,7 @@ def test_invalid_entity_map_raises_value_error(self, entity_map, offender) -> No def test_non_numeric_sub_page_id_raises_value_error( self, entity_sub_page_id, destination_sub_page_id, argument_name ) -> None: - # WHEN I validate copy inputs with a non-numeric, fractional, + # WHEN I validate copy inputs with a non-numeric, float, # boolean, or negative sub page ID # THEN it should raise ValueError naming the offending argument with pytest.raises( From 8d49971b564cb399b28911383404483a30cf4390 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 30 Jul 2026 09:01:49 -0700 Subject: [PATCH 6/7] Log wiki pages skipped by a sub-tree copy at debug level When copying a sub-tree, Synapse returns the owner's entire flat header list and everything outside the requested branch is discarded silently. The skipped page IDs are now logged once in copy_async, after the recursive collection has finished, so a missing page is traceable. Co-Authored-By: Claude Opus 5 (1M context) --- synapseclient/models/wiki.py | 15 ++++- .../models/async/unit_test_wiki_async.py | 63 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/synapseclient/models/wiki.py b/synapseclient/models/wiki.py index 9b1b809c4..efa627e15 100644 --- a/synapseclient/models/wiki.py +++ b/synapseclient/models/wiki.py @@ -1622,8 +1622,9 @@ async def copy_async( destination_wiki_page = None if entity_sub_page_id: + all_wiki_headers = old_wiki_headers old_wiki_headers = _collect_wiki_sub_tree_headers( - wiki_headers=old_wiki_headers, + wiki_headers=all_wiki_headers, sub_page_id=entity_sub_page_id, ) if not old_wiki_headers: @@ -1631,6 +1632,18 @@ async def copy_async( f"The wiki page {entity_sub_page_id} does not exist " f"in the owner entity {self.owner_id}." ) + collected_ids = {header["id"] for header in old_wiki_headers} + ignored_ids = [ + header["id"] + for header in all_wiki_headers + if header["id"] not in collected_ids + ] + if ignored_ids: + client.logger.debug( + f"[{self.owner_id}]: Not copying {len(ignored_ids)} wiki page(s) " + f"outside the sub-tree rooted at {entity_sub_page_id}: " + f"{ignored_ids}" + ) if not old_wiki_headers: return [] diff --git a/tests/unit/synapseclient/models/async/unit_test_wiki_async.py b/tests/unit/synapseclient/models/async/unit_test_wiki_async.py index ac425d92b..a844dffee 100644 --- a/tests/unit/synapseclient/models/async/unit_test_wiki_async.py +++ b/tests/unit/synapseclient/models/async/unit_test_wiki_async.py @@ -2374,6 +2374,69 @@ async def test_copy_async_sub_page_id_coercion( wiki_headers=headers, sub_page_id="8688" ) + @pytest.mark.parametrize( + "sub_tree_headers,expected_log", + [ + ( + [{"id": "8688", "title": "Root"}], + "[syn123]: Not copying 2 wiki page(s) outside the sub-tree rooted " + "at 8688: ['1', '2']", + ), + ( + [ + {"id": "8688", "title": "Root"}, + {"id": "1", "title": "Other"}, + {"id": "2", "title": "Another"}, + ], + None, + ), + ], + ids=["some_pages_ignored", "no_pages_ignored"], + ) + async def test_copy_async_logs_ignored_pages( + self, sub_tree_headers, expected_log + ) -> None: + # GIVEN a source wiki whose header tree contains pages outside the + # requested sub-tree + headers = [ + {"id": "8688", "title": "Root"}, + {"id": "1", "title": "Other"}, + {"id": "2", "title": "Another"}, + ] + with ( + patch( + "synapseclient.models.wiki.get_wiki_header_tree", + side_effect=lambda **kwargs: self._header_generator(headers), + ), + patch( + "synapseclient.models.wiki._ensure_destination_has_no_root_wiki", + new_callable=AsyncMock, + ), + patch( + "synapseclient.models.wiki._collect_wiki_sub_tree_headers", + return_value=sub_tree_headers, + ), + patch( + "synapseclient.models.wiki._copy_wiki_pages", + new_callable=AsyncMock, + return_value=({}, {}), + ), + patch.object(self.syn.logger, "debug") as mock_logger_debug, + ): + # WHEN I call `copy_async` for that sub-tree + await WikiPage(owner_id="syn123", id="8688").copy_async( + destination_owner_id="syn456", + update_links=False, + synapse_client=self.syn, + ) + + # THEN the pages outside the sub-tree should be logged at debug level + debug_messages = [call.args[0] for call in mock_logger_debug.call_args_list] + if expected_log is None: + assert not any("Not copying" in message for message in debug_messages) + else: + assert expected_log in debug_messages + @pytest.mark.parametrize( "entity_sub_page_id,destination_sub_page_id", [("some_string", None), (None, "some_string")], From 6e96b0418e83c36d2041e625902960a55f787cdb Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 30 Jul 2026 09:22:18 -0700 Subject: [PATCH 7/7] Reference SYNPY-1903 from the download_from_url worker thread workarounds Add TODO comments at the four sites in wiki.py that push the synchronous download_from_url onto a worker thread, pointing at the ticket tracking the async httpx-based replacement. Co-Authored-By: Claude Opus 5 (1M context) --- synapseclient/models/wiki.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/synapseclient/models/wiki.py b/synapseclient/models/wiki.py index efa627e15..9630baa38 100644 --- a/synapseclient/models/wiki.py +++ b/synapseclient/models/wiki.py @@ -1205,6 +1205,8 @@ async def get_attachment_async( if file_size < SINGLE_THREAD_DOWNLOAD_SIZE_LIMIT: # download_from_url is synchronous, run it in a worker thread so that # the blocking HTTP request does not stall the asyncio event loop + # TODO: SYNPY-1903 - replace with the async download_from_url once + # it is available and drop the worker thread loop = asyncio.get_running_loop() downloaded_file_path = await loop.run_in_executor( client._get_thread_pool_executor(asyncio_event_loop=loop), @@ -1313,6 +1315,8 @@ async def get_attachment_preview_async( if file_size < SINGLE_THREAD_DOWNLOAD_SIZE_LIMIT: # download_from_url is synchronous, run it in a worker thread so that # the blocking HTTP request does not stall the asyncio event loop + # TODO: SYNPY-1903 - replace with the async download_from_url once + # it is available and drop the worker thread loop = asyncio.get_running_loop() downloaded_file_path = await loop.run_in_executor( client._get_thread_pool_executor(asyncio_event_loop=loop), @@ -1391,6 +1395,8 @@ async def get_markdown_file_async( # download_from_url is synchronous, run it in a worker thread so that the # blocking HTTP request does not stall the asyncio event loop + # TODO: SYNPY-1903 - replace with the async download_from_url once it is + # available and drop the worker thread loop = asyncio.get_running_loop() downloaded_file_path = await loop.run_in_executor( client._get_thread_pool_executor(asyncio_event_loop=loop), @@ -1435,6 +1441,8 @@ async def _get_markdown_text(self, synapse_client: Synapse) -> str: os.makedirs(cache_dir) # download_from_url is synchronous, run it in a worker thread so that the # blocking HTTP request does not stall the asyncio event loop + # TODO: SYNPY-1903 - replace with the async download_from_url once it is + # available and drop the worker thread loop = asyncio.get_running_loop() downloaded_file_path = await loop.run_in_executor( synapse_client._get_thread_pool_executor(asyncio_event_loop=loop),