Skip to content

[SYNPY-1875] Copy and deprecate copyWiki function - #1434

Open
andrewelamb wants to merge 9 commits into
developfrom
SYNPY-1875
Open

[SYNPY-1875] Copy and deprecate copyWiki function#1434
andrewelamb wants to merge 9 commits into
developfrom
SYNPY-1875

Conversation

@andrewelamb

@andrewelamb andrewelamb commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Problem:

synapseutils.copyWiki() was the only way to copy a wiki tree from one entity to
another, and it is legacy code that sits outside the object-oriented model layer:

  • It is a free-standing, sync-only function that takes a syn client as its first
    positional argument, so it cannot be used from an async context and does not follow
    the synapse_client keyword pattern used everywhere else in the new API.
  • It uses camelCase arguments (entitySubPageId, destinationSubPageId,
    updateLinks, entityMap) that are inconsistent with the rest of the OOP models.
  • It has no counterpart on the WikiPage model, so users who have adopted
    synapseclient.models had to drop back down to synapseutils to copy a wiki —
    the same gap that was closed for DownloadList, sync_to_synapse /
    sync_from_synapse, storage locations, and migration in earlier work.

This is the copyWiki half of the ongoing migration of synapseutils bulk
operations onto the object-oriented models (SYNPY-1875).

Solution:

Add WikiPage.copy_async() (and its generated sync WikiPage.copy()) as the
object-oriented replacement for copyWiki(), and deprecate the legacy function.

New API functionsynapseclient/api/file_services.py

Added post_file_handles_copy(), wrapping POST /filehandles/copy, used to copy
wiki attachment file handles to the destination. It batches requests by
MAX_FILE_HANDLE_PER_COPY_REQUEST internally so callers do not have to, and it is
re-exported from synapseclient.api. This puts the file-handle-copy REST call in the
api/ layer where the model can reuse it, rather than reimplementing the batching
that synapseutils.copyFileHandles() does.

Deprecationsynapseutils/copy_functions.py

copyWiki() is marked @deprecated(version="5.0.0") for removal in 6.0.0, with a
docstring migration example pointing at WikiPage.copy / WikiPage.copy_async. The
function itself is unchanged, so existing callers keep working.

Supporting changes

  • synapseclient/models/protocols/wikipage_protocol.py — sync copy() signature
    added so IDE type hints resolve.
  • docs/reference/experimental/sync/wiki.md and
    docs/reference/experimental/async/wiki.mdcopy / copy_async added to the
    WikiPage members lists so mkdocstrings generates the anchors and
    [synapseclient.models.WikiPage.copy] cross-references resolve.

The orchestration is split into small module-level helpers
(_validate_and_format_copy_inputs, _copy_wiki_pages,
_collect_wiki_sub_tree_headers, _update_internal_links,
_update_synapse_id_references, _ensure_destination_has_no_root_wiki,
_get_existing_destination_wiki_page) so each piece is unit-testable without
standing up a full wiki tree.

Testing:

Unit teststests/unit/synapseclient/models/async/unit_test_wiki_async.py

Added a TestWikiPageCopy class covering copy_async end to end with mocks, plus a
test class per helper. Coverage includes:

  • Validation failures: missing owner_id, invalid destination_owner_id, invalid
    entity_map keys and values, non-numeric sub-page IDs.
  • Source edge cases: source entity with no wiki returns an empty list, unknown
    entity_sub_page_id raises ValueError, sub-page ID given for a source with no
    wiki raises ValueError, sub-page ID type coercion.
  • Destination edge cases: destination that already has a root wiki without
    destination_sub_page_id, nonexistent destination_sub_page_id, and error
    propagation from the destination-page and header-tree lookups.
  • The four rewrite combinations: update_links only (the default), entity_map
    only, both, and neither.
  • Helper behavior: sub-tree collection ordering (parents before children) and
    non-mutation of the input headers, link and Synapse-ID rewriting across every
    copied page, None markdown becoming an empty string, and hierarchy preservation
    in _copy_wiki_pages including the create-under and write-into destination cases.

Integration teststests/integration/synapseclient/models/async/test_wiki_async.py

Added a TestWikiPageCopy class that copies against a real Synapse backend:

  • test_copy_entire_wiki_tree — full tree copy, verifying hierarchy, markdown,
    attachments, and rewritten internal links.
  • test_copy_wiki_sub_tree — copying only the sub-tree rooted at a given page.
  • test_copy_wiki_into_existing_destination_page — copying into an existing
    destination wiki page via destination_sub_page_id.
  • test_copy_wiki_from_entity_without_wiki — source with no wiki returns an empty
    list.

@andrewelamb
andrewelamb requested a review from a team as a code owner July 27, 2026 15:49
@andrewelamb
andrewelamb marked this pull request as draft July 27, 2026 15:49
@andrewelamb
andrewelamb marked this pull request as ready for review July 27, 2026 16:00
@andrewelamb andrewelamb changed the title create copy wiki method [SYNPY-1875] Copy and deprecate copyWiki function Jul 27, 2026

@thomasyu888 thomasyu888 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔥 From a product perspective, the usage of this makes sense. I wonder if we should take this opportunity to update the parameters from "destination_" to "target_" which would mimic the unix copy, and thoughts from the team about this?

I'll defer to @BryanFauble / @danlu1 for final review. PInging Dan especially as this will be new copy functionality that exists for objects.

Comment thread synapseclient/api/file_services.py Outdated
Comment thread synapseclient/models/wiki.py Outdated
Comment thread synapseclient/models/wiki.py Outdated
Comment thread synapseclient/models/wiki.py Outdated
Comment thread synapseclient/models/wiki.py Outdated
Comment thread synapseclient/models/wiki.py Outdated
Comment thread synapseclient/models/wiki.py
root_wiki = await root_wiki.store_async(synapse_client=syn)

# Allow the wiki header tree to become consistent
await asyncio.sleep(5)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

https://sagebionetworks.jira.com/browse/SYNPY-1901

If you want to take on the jira, this would be updated to use the new path. At a minimum it would be nice to have a tenacity wait for consistency instead of sleep statements for just your code here, and then applied to existing files as a part of SYNPY-1901

@andrewelamb

andrewelamb commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

🔥 From a product perspective, the usage of this makes sense. I wonder if we should take this opportunity to update the parameters from "destination_" to "target_" which would mimic the unix copy, and thoughts from the team about this?

I would say no to this, unless we wanted to do it across the entire project.

andrewelamb and others added 6 commits July 30, 2026 08:35
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) <noreply@anthropic.com>
… 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…unds

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) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants