Skip to content

Restore historic skeleton endpoint#1

Open
afonsobspinto wants to merge 7 commits into
metacellfrom
restore-historic-skeleton-endpoint
Open

Restore historic skeleton endpoint#1
afonsobspinto wants to merge 7 commits into
metacellfrom
restore-historic-skeleton-endpoint

Conversation

@afonsobspinto

@afonsobspinto afonsobspinto commented May 29, 2026

Copy link
Copy Markdown
Member

Overview

This change adds an API endpoint that restores the latest deleted historic
version of a single skeleton:

POST /<project_id>/skeletons/<skeleton_id>/restore
  • The skeleton to restore is identified by the URL path parameter.
  • The caller must have UserRole.Annotate.
  • The skeleton ID must not already exist in the live class_instance table.
  • The historic table should identify the skeleton to restore by a supported delete transaction (skeletons.remove).
  • The transaction must affect exactly the requested skeleton.

The existing skeleton deletion route is now wrapped with:

record_view("skeletons.remove")

Older delete transactions may still be labeled neurons.remove or may have no label; both are rejected by default. These older restore cases should remain a manual admin workflow

Finding The Latest Deleted Skeleton

The lookup entry point is:

find_latest_deleted_skeleton_transaction(project_id, skeleton_id)

It searches class_instance__history for historic rows where:

  • id matches the requested skeleton ID.
  • project_id matches the requested project.
  • The row belongs to the CATMAID class named skeleton.
  • sys_period is closed, meaning the row was removed or replaced historically.

The candidates are grouped by:

  • exec_transaction_id
  • upper(sys_period)

The newest candidate is selected by ordering upper(sys_period) descending and
taking one row. This is the definition of "latest" for this endpoint.

After selecting the latest candidate, the same query checks how many deleted skeleton rows exist for that transaction in class_instance__history. It only returns a row if that count is exactly one and that one skeleton is the requested skeleton.

The query also joins catmaid_transaction_info on the transaction ID and execution time to retrieve the transaction label.

The Restore View Flow

The view is:

restore_historic_skeleton(request, project_id, skeleton_id)

The flow is:

  1. Open transaction.atomic() so all checks and inserts are one database
    transaction.

  2. Take a transaction-scoped PostgreSQL advisory lock:

    SELECT pg_advisory_xact_lock(%(lock_id)s::bigint)

(The advisory lock blocks restore operations for the same skeleton id)

  1. Reject if any live class_instance already has the requested skeleton ID.
  2. Find the latest deleted historic skeleton transaction.
  3. Reject if no safe single-skeleton historic delete candidate exists.
  4. Reject if the candidate has no label or has a label that differs from skeletons.remove.
  5. Build a Transaction object from the candidate transaction ID and time.
  6. Call undelete_neuron() for the selected transaction.
  7. Return JSON describing the restored skeleton and source transaction.

The response includes:

{
  "skeleton_id": 1,
  "transaction_id": 123,
  "execution_time": "...",
  "source_label": "skeletons.remove",
  "success": "Restored skeleton 1 from history."
}

Tests

The added tests live in:

django/applications/catmaid/tests/apis/test_skeletons.py

The restore-related coverage includes:

  • Successful restore of a deleted one-skeleton neuron.
  • Verification that treenodes, class instances, neuron links, skeleton summary
    rows, and treenode edge rows are live again.
  • Multi-skeleton transaction rejection.
  • Unlabeled transaction rejection.
  • Live skeleton conflict rejection.
  • Transaction-label checks for skeletons.remove and skeletons.restore.

@afonsobspinto afonsobspinto marked this pull request as ready for review May 29, 2026 12:40
@afonsobspinto afonsobspinto requested a review from seankmartin May 29, 2026 12:41

@seankmartin seankmartin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall looks good to me, thanks! Left two comments happy to chat

})


RESTORABLE_SKELETON_DELETE_LABEL = 'skeletons.remove'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ideally I think it would be better if this is defined in one place and reused both here and in urls.py so that this label is kept in sync between the two.
If there is no good candidate place for that then feel free to leave it.

skeleton_restore_lock_namespace = base_lock_id + 3


def skeleton_restore_lock_id(skeleton_id):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'll admit that I've not fully dug into the exact uses of these pg locks in catmaid, but I think there's a exceedingly rare problem that while is so unlikely it will pretty much never occur, I'd rather we explicitly avoid it. Technically our generated unsigned_lock_id could be the spatial update event lock or the history update event lock and it also makes it a bit hard in the future to add locks since the ID space is variable.

Looking at a different signature for the pg_advisory_xact_lock(key1 integer, key2 integer) we can use two keys which could possibly work here and seems intended for this kind of case. However the base_lock_id is too large in this case as key1. And we'd have to roll over on key2 or hash into 32 bit. But I think it might still work for this use case

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.

2 participants