Code review:

- consolidate thread-root resolution
- consolidate mute-state writes
This commit is contained in:
davotoula
2026-05-12 18:30:52 +02:00
parent a99488779a
commit ff780bcb54
7 changed files with 23 additions and 68 deletions
@@ -167,11 +167,11 @@ import com.vitorpamplona.quartz.nip03Timestamp.OtsResolver
import com.vitorpamplona.quartz.nip04Dm.PrivateDMCache import com.vitorpamplona.quartz.nip04Dm.PrivateDMCache
import com.vitorpamplona.quartz.nip04Dm.messages.PrivateDmEvent import com.vitorpamplona.quartz.nip04Dm.messages.PrivateDmEvent
import com.vitorpamplona.quartz.nip09Deletions.DeletionEvent import com.vitorpamplona.quartz.nip09Deletions.DeletionEvent
import com.vitorpamplona.quartz.nip10Notes.BaseThreadedEvent
import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent
import com.vitorpamplona.quartz.nip10Notes.content.findHashtags import com.vitorpamplona.quartz.nip10Notes.content.findHashtags
import com.vitorpamplona.quartz.nip10Notes.content.findNostrUris import com.vitorpamplona.quartz.nip10Notes.content.findNostrUris
import com.vitorpamplona.quartz.nip10Notes.content.findURLs import com.vitorpamplona.quartz.nip10Notes.content.findURLs
import com.vitorpamplona.quartz.nip10Notes.threadRootIdOrSelf
import com.vitorpamplona.quartz.nip17Dm.NIP17Factory import com.vitorpamplona.quartz.nip17Dm.NIP17Factory
import com.vitorpamplona.quartz.nip17Dm.base.NIP17Group import com.vitorpamplona.quartz.nip17Dm.base.NIP17Group
import com.vitorpamplona.quartz.nip17Dm.files.ChatMessageEncryptedFileHeaderEvent import com.vitorpamplona.quartz.nip17Dm.files.ChatMessageEncryptedFileHeaderEvent
@@ -2838,17 +2838,16 @@ class Account(
} }
suspend fun muteThread(rootHex: HexKey) { suspend fun muteThread(rootHex: HexKey) {
if (isThreadMuted(rootHex)) return
sendMyPublicAndPrivateOutbox(muteList.hideThread(rootHex)) sendMyPublicAndPrivateOutbox(muteList.hideThread(rootHex))
} }
suspend fun unmuteThread(rootHex: HexKey) { suspend fun unmuteThread(rootHex: HexKey) {
if (!isThreadMuted(rootHex)) return
muteList.showThread(rootHex)?.let { sendMyPublicAndPrivateOutbox(it) } muteList.showThread(rootHex)?.let { sendMyPublicAndPrivateOutbox(it) }
} }
fun resolveThreadRoot(note: Note): HexKey { fun resolveThreadRoot(note: Note): HexKey = note.event?.threadRootIdOrSelf() ?: note.idHex
val ev = note.event
return (ev as? BaseThreadedEvent)?.root()?.eventId ?: note.idHex
}
fun isThreadMuted(rootHex: HexKey): Boolean = hiddenUsers.flow.value.isThreadMuted(rootHex) fun isThreadMuted(rootHex: HexKey): Boolean = hiddenUsers.flow.value.isThreadMuted(rootHex)
@@ -2986,7 +2985,8 @@ class Account(
} }
override fun isAcceptable(note: Note): Boolean { override fun isAcceptable(note: Note): Boolean {
if (isThreadMuted(resolveThreadRoot(note))) return false val mutedThreads = hiddenUsers.flow.value.mutedThreads
if (mutedThreads.isNotEmpty() && mutedThreads.contains(resolveThreadRoot(note))) return false
return note.author?.let { isAcceptable(it) } ?: true && return note.author?.let { isAcceptable(it) } ?: true &&
// if user hasn't hided this author // if user hasn't hided this author
isAcceptableDirect(note) && isAcceptableDirect(note) &&
@@ -31,7 +31,7 @@ import com.vitorpamplona.quartz.nip01Core.core.Event
import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
import com.vitorpamplona.quartz.nip01Core.tags.hashtags.countHashtags import com.vitorpamplona.quartz.nip01Core.tags.hashtags.countHashtags
import com.vitorpamplona.quartz.nip10Notes.BaseThreadedEvent import com.vitorpamplona.quartz.nip10Notes.threadRootIdOrSelf
import com.vitorpamplona.quartz.nip51Lists.muteList.MuteListEvent import com.vitorpamplona.quartz.nip51Lists.muteList.MuteListEvent
import com.vitorpamplona.quartz.nip51Lists.peopleList.PeopleListEvent import com.vitorpamplona.quartz.nip51Lists.peopleList.PeopleListEvent
import com.vitorpamplona.quartz.utils.TimeUtils import com.vitorpamplona.quartz.utils.TimeUtils
@@ -46,8 +46,7 @@ class FilterByListParams(
fun isNotInMutedThread(noteEvent: Event): Boolean { fun isNotInMutedThread(noteEvent: Event): Boolean {
if (hiddenLists.mutedThreads.isEmpty()) return true if (hiddenLists.mutedThreads.isEmpty()) return true
val rootId = (noteEvent as? BaseThreadedEvent)?.root()?.eventId ?: noteEvent.id return !hiddenLists.mutedThreads.contains(noteEvent.threadRootIdOrSelf())
return !hiddenLists.mutedThreads.contains(rootId)
} }
fun isNotInTheFuture(noteEvent: Event) = noteEvent.createdAt <= now fun isNotInTheFuture(noteEvent: Event) = noteEvent.createdAt <= now
@@ -225,11 +225,8 @@ class NotificationFeedFilter(
} }
} }
// Drop reactions/zaps/reposts whose target post is in a muted thread. // Reactions/zaps/reposts target a note via `replyTo`, not via thread-root tags,
// The inner-note renderer would otherwise show a misleading // so isNotInMutedThread on the wrapper event misses them.
// "This post was hidden because it mentions your hidden users or words"
// placeholder for muted-thread targets (regression from Task 6's
// Note.isHiddenFor extension).
if (noteEvent is ReactionEvent || noteEvent is LnZapEvent || if (noteEvent is ReactionEvent || noteEvent is LnZapEvent ||
noteEvent is RepostEvent || noteEvent is GenericRepostEvent noteEvent is RepostEvent || noteEvent is GenericRepostEvent
) { ) {
@@ -34,6 +34,6 @@ class MutedThreadsFeedFilter(
override fun feed(): List<Note> = override fun feed(): List<Note> =
account.hiddenUsers.flow.value.mutedThreads account.hiddenUsers.flow.value.mutedThreads
.mapNotNull { LocalCache.getNoteIfExists(it) ?: LocalCache.getOrCreateNote(it) } .map { LocalCache.getOrCreateNote(it) }
.sortedByDescending { it.createdAt() } .sortedByDescending { it.createdAt() }
} }
@@ -39,6 +39,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
import com.vitorpamplona.quartz.nip01Core.tags.hashtags.anyHashTag import com.vitorpamplona.quartz.nip01Core.tags.hashtags.anyHashTag
import com.vitorpamplona.quartz.nip01Core.tags.publishedAt.PublishedAtProvider import com.vitorpamplona.quartz.nip01Core.tags.publishedAt.PublishedAtProvider
import com.vitorpamplona.quartz.nip10Notes.BaseThreadedEvent import com.vitorpamplona.quartz.nip10Notes.BaseThreadedEvent
import com.vitorpamplona.quartz.nip10Notes.threadRootIdOrSelf
import com.vitorpamplona.quartz.nip18Reposts.GenericRepostEvent import com.vitorpamplona.quartz.nip18Reposts.GenericRepostEvent
import com.vitorpamplona.quartz.nip18Reposts.RepostEvent import com.vitorpamplona.quartz.nip18Reposts.RepostEvent
import com.vitorpamplona.quartz.nip19Bech32.entities.NAddress import com.vitorpamplona.quartz.nip19Bech32.entities.NAddress
@@ -860,10 +861,10 @@ open class Note(
return true return true
} }
// if this note belongs to a muted thread (by NIP-10 root id) if (accountChoices.mutedThreads.isNotEmpty() &&
if (accountChoices.mutedThreads.isNotEmpty()) { accountChoices.mutedThreads.contains(thisEvent.threadRootIdOrSelf())
val rootId = (thisEvent as? BaseThreadedEvent)?.root()?.eventId ?: idHex ) {
if (accountChoices.mutedThreads.contains(rootId)) return true return true
} }
// if the post is sensitive and the user doesn't want to see sensitive content // if the post is sensitive and the user doesn't want to see sensitive content
@@ -25,22 +25,11 @@ import kotlin.test.Test
import kotlin.test.assertFalse import kotlin.test.assertFalse
import kotlin.test.assertTrue import kotlin.test.assertTrue
/**
* Unit tests for Note.isHiddenFor() focusing on the mutedThreads check.
*
* Fixture approach:
* - TextNoteEvent is constructed directly via its primary constructor (no signing needed —
* the sig field is not validated in isHiddenFor).
* - A Note is created with the event's id, then note.event is set.
* - LiveHiddenUsers is a plain data class — no Android plumbing required.
*/
class NoteIsHiddenForTest { class NoteIsHiddenForTest {
// 64-char hex constants
private val rootId = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" private val rootId = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
private val replyId = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" private val replyId = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
private val authorPubKey = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" private val authorPubKey = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"
// Minimal LiveHiddenUsers with all filters off (used as a baseline)
private val noHidden = private val noHidden =
LiveHiddenUsers( LiveHiddenUsers(
showSensitiveContent = null, showSensitiveContent = null,
@@ -50,12 +39,6 @@ class NoteIsHiddenForTest {
mutedThreads = emptySet(), mutedThreads = emptySet(),
) )
/**
* Build a TextNoteEvent.
*
* @param id The event id (64 hex chars).
* @param eTags Raw `e` tag arrays included in the event's tags.
*/
private fun textNoteEvent( private fun textNoteEvent(
id: String, id: String,
pubKey: String = authorPubKey, pubKey: String = authorPubKey,
@@ -69,21 +52,12 @@ class NoteIsHiddenForTest {
sig = "sig", sig = "sig",
) )
/** A marked `e` tag array with marker="root" at index 3. */
private fun rootETag(eventId: String) = arrayOf("e", eventId, "", "root") private fun rootETag(eventId: String) = arrayOf("e", eventId, "", "root")
// -------------------------------------------------------------------------
@Test @Test
fun reply_inMutedThread_isHidden() { fun reply_inMutedThread_isHidden() {
// A reply whose NIP-10 root points to `rootId`, which is muted. val event = textNoteEvent(id = replyId, eTags = arrayOf(rootETag(rootId)))
val event =
textNoteEvent(
id = replyId,
eTags = arrayOf(rootETag(rootId)),
)
val note = Note(replyId).also { it.event = event } val note = Note(replyId).also { it.event = event }
val choices = noHidden.copy(mutedThreads = setOf(rootId)) val choices = noHidden.copy(mutedThreads = setOf(rootId))
assertTrue(note.isHiddenFor(choices), "Reply inside a muted thread must be hidden") assertTrue(note.isHiddenFor(choices), "Reply inside a muted thread must be hidden")
@@ -91,14 +65,8 @@ class NoteIsHiddenForTest {
@Test @Test
fun topLevelNote_ownIdMuted_isHidden() { fun topLevelNote_ownIdMuted_isHidden() {
// A top-level note (no e-tags) whose own id is in mutedThreads. val event = textNoteEvent(id = rootId, eTags = emptyArray())
val event =
textNoteEvent(
id = rootId,
eTags = emptyArray(),
)
val note = Note(rootId).also { it.event = event } val note = Note(rootId).also { it.event = event }
val choices = noHidden.copy(mutedThreads = setOf(rootId)) val choices = noHidden.copy(mutedThreads = setOf(rootId))
assertTrue(note.isHiddenFor(choices), "Top-level note whose id is muted must be hidden") assertTrue(note.isHiddenFor(choices), "Top-level note whose id is muted must be hidden")
@@ -106,16 +74,9 @@ class NoteIsHiddenForTest {
@Test @Test
fun note_inUnmutedThread_isNotHidden() { fun note_inUnmutedThread_isNotHidden() {
// A reply note whose root is NOT in mutedThreads, author not hidden.
val otherRoot = "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd" val otherRoot = "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
val event = val event = textNoteEvent(id = replyId, eTags = arrayOf(rootETag(otherRoot)))
textNoteEvent(
id = replyId,
eTags = arrayOf(rootETag(otherRoot)),
)
val note = Note(replyId).also { it.event = event } val note = Note(replyId).also { it.event = event }
// rootId is muted, but this note's root is otherRoot
val choices = noHidden.copy(mutedThreads = setOf(rootId)) val choices = noHidden.copy(mutedThreads = setOf(rootId))
assertFalse(note.isHiddenFor(choices), "Reply in an un-muted thread must not be hidden") assertFalse(note.isHiddenFor(choices), "Reply in an un-muted thread must not be hidden")
@@ -123,15 +84,9 @@ class NoteIsHiddenForTest {
@Test @Test
fun authorHidden_isHidden_regression() { fun authorHidden_isHidden_regression() {
// Regression guard: author-hidden notes must still return true.
val event = textNoteEvent(id = replyId) val event = textNoteEvent(id = replyId)
val note = Note(replyId).also { it.event = event } val note = Note(replyId).also { it.event = event }
val choices = noHidden.copy(hiddenUsersHashCodes = setOf(authorPubKey.hashCode()))
// Put the author's pubKey hashCode into hiddenUsersHashCodes
val choices =
noHidden.copy(
hiddenUsersHashCodes = setOf(authorPubKey.hashCode()),
)
assertTrue(note.isHiddenFor(choices), "Note whose author is hidden must still be hidden") assertTrue(note.isHiddenFor(choices), "Note whose author is hidden must still be hidden")
} }
@@ -22,6 +22,7 @@ package com.vitorpamplona.quartz.nip10Notes
import androidx.compose.runtime.Immutable import androidx.compose.runtime.Immutable
import com.vitorpamplona.quartz.experimental.nipsOnNostr.NipTextEvent import com.vitorpamplona.quartz.experimental.nipsOnNostr.NipTextEvent
import com.vitorpamplona.quartz.nip01Core.core.Event
import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.nip01Core.tags.aTag.taggedATags import com.vitorpamplona.quartz.nip01Core.tags.aTag.taggedATags
import com.vitorpamplona.quartz.nip01Core.tags.people.taggedUsers import com.vitorpamplona.quartz.nip01Core.tags.people.taggedUsers
@@ -30,6 +31,8 @@ import com.vitorpamplona.quartz.nip54Wiki.WikiNoteEvent
import com.vitorpamplona.quartz.nip72ModCommunities.definition.CommunityDefinitionEvent import com.vitorpamplona.quartz.nip72ModCommunities.definition.CommunityDefinitionEvent
import com.vitorpamplona.quartz.utils.lastNotNullOfOrNull import com.vitorpamplona.quartz.utils.lastNotNullOfOrNull
fun Event.threadRootIdOrSelf(): HexKey = (this as? BaseThreadedEvent)?.root()?.eventId ?: id
@Immutable @Immutable
open class BaseThreadedEvent( open class BaseThreadedEvent(
id: HexKey, id: HexKey,