Skip to content

fix: prevent HistoryItemContent orphaning during duplicate resolution#1387

Open
Project-A-E-G-I-S wants to merge 1 commit into
p0deje:masterfrom
Project-A-E-G-I-S:fix/duplicate-orphaned-contents
Open

fix: prevent HistoryItemContent orphaning during duplicate resolution#1387
Project-A-E-G-I-S wants to merge 1 commit into
p0deje:masterfrom
Project-A-E-G-I-S:fix/duplicate-orphaned-contents

Conversation

@Project-A-E-G-I-S

Copy link
Copy Markdown

Summary

Fixes a bug where adding a duplicate clipboard item could leave orphaned HistoryItemContent rows in the SwiftData store with item == nil, and cascade deletion of the existing item could delete content objects shared with the replacement item.

Root Cause

In History.add(_:) (Maccy/Observables/History.swift:147-163), when a duplicate was detected:

  1. The incoming item was already persisted to SwiftData (line 140)
  2. Its contents were replaced with the existing item's contents (line 149)
  3. The original contents were left orphaned in the store
  4. The existing item was cascade-deleted, which could wipe content objects now referenced by the replacement

Fix

  • Delete the incoming item's contents from the store before replacing them (prevents orphaning)
  • Detach the existing item's contents (existingHistoryItem.contents = []) before cascade deletion (prevents shared content from being wiped)
  • Both operations are gated behind if isModified(item) == nil, so they only run when contents are actually transferred

Testing

Added testDuplicateDoesNotOrphanContents which verifies:

  • Exactly 1 HistoryItemContent row exists after duplicate merge
  • The surviving content has a valid item reference

Closes #1368

When a duplicate clipboard item was detected, History.add() replaced the
incoming item's contents with the existing item's contents without cleanup.
The original contents (already persisted to SwiftData) were orphaned as
rows with item == nil. Additionally, cascade-deleting the existing item
would delete content objects now shared with the replacement.

Fix:
- Delete incoming item's contents from store before replacement
- Detach existing item's contents before cascade deletion
- Both scoped to the isModified(item) == nil block (contents transfer path)

Test: testDuplicateDoesNotOrphanContents verifies exact 1:1
content-to-item ratio after duplicate merge.

Closes p0deje#1368
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.

Duplicate history insertion can orphan HistoryItemContent rows

1 participant