Skip to content

Fix: Delete entry-level file syncs when deleting a thumb asset - #4

Open
devin-ai-integration[bot] wants to merge 1 commit into
Scorpius-19.6.0from
devin/1777812270-fix-thumb-delete-entry-filesync
Open

Fix: Delete entry-level file syncs when deleting a thumb asset#4
devin-ai-integration[bot] wants to merge 1 commit into
Scorpius-19.6.0from
devin/1777812270-fix-thumb-delete-entry-filesync

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

When a thumb asset is deleted via ThumbAssetService::deleteAction, the entry-level file sync (created by setAsDefaultThumbAsset via kFileSyncUtils::createSyncFileLinkForKey) remains active. This happens because:

  1. deleteAction soft-deletes the thumb asset (status → DELETED)
  2. The kObjectDeletedEvent triggers syncableDeleted which deletes the asset-level file syncs
  3. In deleteSyncFileForKey, when the source file sync has linkCount > 0, convertLinksToFiles() converts the entry's LINK file sync into an independent FILE — keeping it with status READY
  4. thumbnailAction::execute() still serves the thumbnail because it resolves the file from the entry's file sync (now an independent file)

Fix: Added deleteEntryFileSyncsLinkedToAsset() which is called before the thumb asset is soft-deleted. It:

  • Finds the thumb asset's file syncs
  • Queries for entry-level file syncs (FileSyncObjectType::ENTRY, kEntryFileSyncSubType::THUMB) that have linked_id pointing to the thumb asset's file syncs
  • Deletes those entry-level file syncs first, so convertLinksToFiles won't preserve stale thumbnail data

Review & Testing Checklist for Human

  • Verify that deleting a non-default thumb asset makes the thumbnail inaccessible via the entry-level URL (/thumbnail/entry_id/{id}/version/{version})
  • Verify that deleting a thumb asset does NOT affect the current default thumb asset's accessibility
  • Verify that after deleting a thumb asset, the entry's current default thumbnail (different version) still works
  • Check that when a thumb asset has file syncs across multiple DCs, the entry-level file syncs for all DCs are cleaned up
  • Test the scenario where the thumb asset was previously the default but is no longer (TAG_DEFAULT_THUMB was removed before deletion)

Notes

  • The fix only affects non-default thumb assets (the deleteAction already throws THUMB_ASSET_IS_DEFAULT if the asset has TAG_DEFAULT_THUMB)
  • The deleteEntryFileSyncsLinkedToAsset method queries by LINKED_ID to find entry file syncs that are links to this specific thumb asset, so it won't affect unrelated entry file syncs
  • The method is called before save() to ensure the entry file syncs are deleted before the kObjectDeletedEvent cascade runs convertLinksToFiles

Link to Devin session: https://app.devin.ai/sessions/fdc0eae331b54163b0359394b033c83f
Requested by: @bw-kaltura

When a thumb asset is set as default, a file sync link is created on the
entry (kEntryFileSyncSubType::THUMB) pointing to the thumb asset's file
sync. When the thumb asset is later deleted, only the asset-level file
syncs are cleaned up. The entry-level file sync link is converted to an
independent file by convertLinksToFiles, keeping the thumbnail accessible
via the entry's thumbnail URL even after deletion.

This fix adds deleteEntryFileSyncsLinkedToAsset() which is called before
the thumb asset is soft-deleted. It finds entry-level file syncs that are
linked to the thumb asset's file syncs and deletes them first, preventing
the link-to-file conversion from preserving stale thumbnail data.

Co-Authored-By: bryna.weissman@kaltura.com <bryna.weissman@kaltura.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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.

1 participant