refactor: unify Note removal into one shared unlink path

deleteNote() (NIP-09) and removeFromCache() (prune) had drifted into two
near-duplicate "detach a note from the cache" routines. Removal really has
two halves — (1) unlink the note from everything that points AT it, and
(2) handle the note's OWN children — and only the second half differs
between the paths. removeFromCache() already implemented half (1)
completely, so deleteNote() now delegates to it and keeps only its two
delete-specific responsibilities: tearing down gift-wrap hosts and
severing (but keeping) its children via detachFromChildren().

This also fixes a real leak the duplication was hiding. computeReplyTo()
has no ReportEvent branch, so a report note's replyTo is empty and the
report→target link lives only in the explicit reported* index handling.
The old deleteNote() only undid reportedAuthor(), so deleting an
event-level report (reportedPost / reportedAddresses) left the reported
note's `.reports` map holding the removed report note — a partial deletion
that leaked the shell and risked a duplicate Note for the same id.
Delegating to removeFromCache() (author + post + addresses, all
idempotent) closes that gap.

Net behavior change is the report-leak fix only; the redundant
TorrentCommentEvent case is dropped because the torrent target is already
in replyTo (and removed via removeNote), and its @Suppress("DEPRECATION")
goes with it. Adds KDoc to both methods documenting the two-halves model.

https://claude.ai/code/session_01RqJPYzmjb1pR3NBeH2yY3s
This commit is contained in:
Claude
2026-06-04 18:35:56 +00:00
parent 23ddeba8ec
commit a3bb6fbc10
@@ -1336,60 +1336,28 @@ object LocalCache : ILocalCache, ICacheProvider {
else -> null
}
@Suppress("DEPRECATION")
/**
* NIP-09 delete of a single targeted event.
*
* Removal has two halves: unlinking the note from everything that points AT it
* (its parents, channels, and the per-user report/card/status/poll indexes —
* all handled by [removeFromCache]); and dealing with the note's OWN children
* (the notes that point at IT). The delete path and the prune path share the
* first half and differ only on the second:
* - delete (here): the children are independent events and stay in the cache;
* [Note.detachFromChildren] only severs their back-reference so the removed
* shell can neither leak (held alive by a child's `replyTo`) nor be later
* resurrected by `computeReplyTo` as a second Note for the same id.
* - prune (see [removeFromCache] callers): the whole child subtree is removed.
*
* Gift-wrapped events additionally drop their decrypted inner host.
*/
private fun deleteNote(deleteNote: Note) {
val deletedEvent = deleteNote.event
(deleteNote.event as? WrappedEvent)?.let { deleteWraps(it) }
if (deletedEvent is ReportEvent) {
deletedEvent.reportedAuthor().forEach {
getUserIfExists(it.pubkey)?.reportsOrNull()?.removeReport(deleteNote)
}
}
if (deleteNote is AddressableNote && deletedEvent is ContactCardEvent) {
getUserIfExists(deletedEvent.aboutUser())?.cardsOrNull()?.removeCard(deleteNote)
}
if (deleteNote is AddressableNote && deletedEvent is StatusEvent) {
deleteNote.author?.statusStateOrNull()?.removeStatus(deleteNote)
}
if (deletedEvent is PollResponseEvent) {
deletedEvent.poll()?.eventId?.let {
getNoteIfExists(it)?.pollStateOrNull()?.removeResponse(deleteNote)
}
}
if (deletedEvent is TorrentCommentEvent) {
deletedEvent.torrentIds()?.let {
getNoteIfExists(it)?.removeReply(deleteNote)
}
}
if (deletedEvent is WrappedEvent) {
deleteWraps(deletedEvent)
}
// Counts the replies
deleteNote.replyTo?.forEach { masterNote ->
masterNote.removeNote(deleteNote)
}
deleteNote.inGatherers?.forEach { it.removeNote(deleteNote) }
getAnyChannel(deleteNote)?.removeNote(deleteNote)
// Sever the back-references from this note's children before it leaves the
// map. Otherwise each child keeps the removed shell alive through its replyTo
// (a partial deletion / leak) and a reply resolved later via computeReplyTo
// would resurrect a second Note for the same id.
deleteNote.detachFromChildren()
notes.remove(deleteNote.idHex)
deleteNote.clearFlow()
refreshDeletedNoteObservers(deleteNote)
removeFromCache(deleteNote)
}
fun deleteWraps(event: WrappedEvent) {
@@ -2784,6 +2752,28 @@ object LocalCache : ILocalCache, ICacheProvider {
}
}
/**
* Unlinks [note] from everything in the cache that references it, then drops it
* from the [notes] map and notifies observers. This is the shared "unlink from
* above" half of removal, used by both the prune callers and [deleteNote].
*
* It detaches the note from:
* - its parent notes (their replies/reactions/zaps/boosts/reports/labels maps);
* because event-level reports and torrent comments both carry the target in
* `replyTo`, [Note.removeNote] cleans those up here too;
* - its channels/gatherers (`inGatherers` is authoritative — `Channel.addNote`
* always registers the gatherer — and `getAnyChannel` is a belt-and-suspenders
* resolve so a note can never linger in a channel after leaving the cache);
* - the per-target indexes `replyTo` does NOT reach: user-level reports and
* reported addresses, contact cards, statuses, and poll responses.
*
* It deliberately does NOT touch the note's own children: prune callers collect
* them via [Note.removeAllChildNotes] and remove the subtree, while [deleteNote]
* keeps them and severs only their back-reference. Every per-target removal is
* idempotent, so the overlap between `replyTo` and the explicit indexes (e.g. an
* event-level report reachable both ways) is harmless. Addressable notes are
* dropped from the [addressables] map by the caller; this only removes from [notes].
*/
private fun removeFromCache(note: Note) {
note.replyTo?.forEach { masterNote ->
masterNote.removeNote(note)
@@ -2791,11 +2781,6 @@ object LocalCache : ILocalCache, ICacheProvider {
note.inGatherers?.forEach { it.removeNote(note) }
// Mirror deleteNote(): inGatherers is normally authoritative for channel
// membership (Channel.addNote always calls note.addGatherer), but resolve
// the channel from the event as a belt-and-suspenders detach so a note can
// never linger in a channel's notes map after it leaves the cache — that
// would leak the note and let a relay echo mint a duplicate with the same id.
getAnyChannel(note)?.removeNote(note)
val noteEvent = note.event