diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/CalendarReminderCandidatesTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/CalendarReminderCandidatesTest.kt index 99f3195d38..1de5d9b294 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/CalendarReminderCandidatesTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/CalendarReminderCandidatesTest.kt @@ -20,8 +20,10 @@ */ package com.vitorpamplona.amethyst.calendar +import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.model.cache.LocalCache import com.vitorpamplona.amethyst.service.calendar.CalendarReminderWorker +import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip52Calendar.appt.time.CalendarTimeSlotEvent import com.vitorpamplona.quartz.nip52Calendar.rsvp.CalendarRSVPEvent import org.junit.Assert.assertFalse @@ -33,10 +35,23 @@ import org.junit.Test * chain may cancel itself. Getting it wrong in either direction is a bug: * a false "could fire" keeps waking the process forever; a false "can't fire" * silently kills a reminder the user RSVP'd to. + * + * Events go in through [consumePinned], which takes each event's note from the cache *before* + * consuming it and keeps it for the test's lifetime. `LocalCache` holds notes weakly + * (LargeSoftCache) and the worker finds RSVPs and their time slots by reading the cache, so a + * GC before that read would lose them — a lost slot reads as "not fetched yet" and flips + * `couldStillFire` to true. */ class CalendarReminderCandidatesTest { private val now = 2_000_000_000L + private val pinned = mutableListOf() + + private fun consumePinned(event: Event) { + pinned.add(LocalCache.getOrCreateNote(event)) + LocalCache.justConsume(event, null, true) + } + // Unique per-test-class identities so the shared LocalCache singleton // doesn't collide with other suites running in the same JVM. private val organizer = "b0b0000000000000000000000000000000000000000000000000000000000001" @@ -85,8 +100,8 @@ class CalendarReminderCandidatesTest { fun acceptedRsvpsInCache_returnsAcceptedAndSkipsDeclined() { val accepted = rsvp("cand-accepted", "cand-target-1") val declined = rsvp("cand-declined", "cand-target-2", status = "declined") - LocalCache.justConsume(accepted, null, true) - LocalCache.justConsume(declined, null, true) + consumePinned(accepted) + consumePinned(declined) val found = CalendarReminderWorker.acceptedRsvpsInCache() assertTrue("accepted RSVP must be found", found.any { it.dTag() == "cand-accepted" }) @@ -97,8 +112,8 @@ class CalendarReminderCandidatesTest { fun futureTarget_couldStillFire() { val slot = timeSlot("cand-future", startSeconds = now + 3600) val r = rsvp("cand-rsvp-future", "cand-future") - LocalCache.justConsume(slot, null, true) - LocalCache.justConsume(r, null, true) + consumePinned(slot) + consumePinned(r) assertTrue(CalendarReminderWorker.couldStillFire(listOf(r), now)) } @@ -107,8 +122,8 @@ class CalendarReminderCandidatesTest { fun pastTarget_cannotFireAnymore() { val slot = timeSlot("cand-past", startSeconds = now - 3600) val r = rsvp("cand-rsvp-past", "cand-past") - LocalCache.justConsume(slot, null, true) - LocalCache.justConsume(r, null, true) + consumePinned(slot) + consumePinned(r) assertFalse(CalendarReminderWorker.couldStillFire(listOf(r), now)) } @@ -118,7 +133,7 @@ class CalendarReminderCandidatesTest { // The RSVP points at an event the cache hasn't fetched yet: the start // is unknown, so the worker must NOT cancel its chain. val r = rsvp("cand-rsvp-unresolved", "cand-never-fetched") - LocalCache.justConsume(r, null, true) + consumePinned(r) assertTrue(CalendarReminderWorker.couldStillFire(listOf(r), now)) } @@ -128,8 +143,8 @@ class CalendarReminderCandidatesTest { // Target resolved but carries no start tag: still unknown, keep alive. val slot = timeSlot("cand-no-start", startSeconds = null) val r = rsvp("cand-rsvp-no-start", "cand-no-start") - LocalCache.justConsume(slot, null, true) - LocalCache.justConsume(r, null, true) + consumePinned(slot) + consumePinned(r) assertTrue(CalendarReminderWorker.couldStillFire(listOf(r), now)) } @@ -137,7 +152,7 @@ class CalendarReminderCandidatesTest { @Test fun rsvpWithoutTargetAddress_doesNotKeepTheChainAlive() { val r = rsvp("cand-rsvp-no-a-tag", targetDTag = null) - LocalCache.justConsume(r, null, true) + consumePinned(r) assertFalse(CalendarReminderWorker.couldStillFire(listOf(r), now)) } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/CalendarsViewModelFlowTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/CalendarsViewModelFlowTest.kt index aaf1c26d9b..95b7f70d16 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/CalendarsViewModelFlowTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/calendar/CalendarsViewModelFlowTest.kt @@ -120,10 +120,14 @@ class CalendarsViewModelFlowTest { * Consumes [calendars] and hands back the notes they landed in, so the caller can hold them * for the length of the test: `LocalCache.addressables` is a weak cache, and with nothing * holding a reference a calendar can be collected out from under the assertions. + * + * The notes are taken *before* consuming: fetching them afterwards leaves the window between + * the consume and the fetch open, and a GC there drops the calendar before it is ever held. */ - private fun consume(vararg calendars: CalendarCollectionEvent): List { + private fun consume(vararg calendars: CalendarCollectionEvent): List { + val held = calendars.map { LocalCache.getOrCreateAddressableNote(it.address()) } calendars.forEach { LocalCache.justConsumeMyOwnEvent(it) } - return calendars.map { LocalCache.getAddressableNoteIfExists(it.address()) } + return held } /** diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/BuzzWorkspaceChannelTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/BuzzWorkspaceChannelTest.kt index 97512fd09b..6aa5e58758 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/BuzzWorkspaceChannelTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/BuzzWorkspaceChannelTest.kt @@ -168,6 +168,10 @@ class BuzzWorkspaceChannelTest { // overlay its message — it is anchored on the message note, independent of any relay. val channelId = newChannelId() val original = streamMessage(channelId, "original") + // Taken before consuming and held: with no relay the message joins no channel + // timeline, so nothing else holds its note in LocalCache's weak store, and a GC + // before the read-back would drop it along with its edit overlay. + val target = LocalCache.getOrCreateNote(original.id) LocalCache.checkDeletionAndConsume(original, null, true) val edit = @@ -176,7 +180,6 @@ class BuzzWorkspaceChannelTest { ) LocalCache.checkDeletionAndConsume(edit, null, true) - val target = LocalCache.getNoteIfExists(original.id)!! val newest = target.edits.filter { it.event is StreamMessageEditEvent }.maxByOrNull { it.createdAt() ?: 0L } assertEquals("edited offline", newest?.event?.content) } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/DvmHeartbeatTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/DvmHeartbeatTest.kt index f5b0752a11..93f9ca4a9f 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/DvmHeartbeatTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/DvmHeartbeatTest.kt @@ -20,12 +20,14 @@ */ package com.vitorpamplona.amethyst.model +import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.model.cache.LocalCache import com.vitorpamplona.amethyst.commons.model.cachedDvmAnnouncements import com.vitorpamplona.amethyst.commons.model.dvmHeartbeatOf import com.vitorpamplona.amethyst.commons.model.hasFreshDvmHeartbeat import com.vitorpamplona.amethyst.commons.model.nip90DVMs.DvmHeartbeatRegistry import com.vitorpamplona.quartz.nip01Core.core.Address +import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip89AppHandlers.definition.AppDefinitionEvent import com.vitorpamplona.quartz.nip90Dvms.dvmHeartbeat.DvmHeartbeatEvent import com.vitorpamplona.quartz.utils.TimeUtils @@ -38,10 +40,23 @@ import org.junit.Test /** * `LocalCache` is a process-wide object and JUnit 4 runs methods in hash order, so every test * uses its own pubkeys/dTags/ids (same discipline as ReportNamingIndexIngestionTest). + * + * Events go in through [consumePinned], which takes each event's note from the cache *before* + * consuming it and keeps it for the test's lifetime. Beat and announcement notes live in + * `LocalCache`'s weakly-held store, and `dvmHeartbeatOf` / `cachedDvmAnnouncements` read them from + * there, so a GC between the consume and the read would make them vanish. Fetching the note after + * the consume is too late: the window is already open. */ class DvmHeartbeatTest { private val appDefPubKey = "f1".repeat(32) + private val pinned = mutableListOf() + + private fun consumePinned(event: Event) { + pinned.add(LocalCache.getOrCreateNote(event)) + LocalCache.justConsume(event, null, true) + } + private fun appDef( dTag: String, pubKey: String = appDefPubKey, @@ -76,7 +91,7 @@ class DvmHeartbeatTest { @Test fun aConsumedHeartbeatLandsAtTheAnnouncementMirrorAddress() { val app = appDef("dvm-one") - LocalCache.justConsume(beat("dvm-one", createdAt = 1_760_000_100L, id = "f3".repeat(32)), null, true) + consumePinned(beat("dvm-one", createdAt = 1_760_000_100L, id = "f3".repeat(32))) val found = LocalCache.dvmHeartbeatOf(app) assertTrue("heartbeat should be found via the announcement's address", found != null) @@ -93,8 +108,8 @@ class DvmHeartbeatTest { val staleApp = appDef("dvm-two-stale") assertNull("no beat yet", LocalCache.dvmHeartbeatOf(freshApp)) - LocalCache.justConsume(beat("dvm-two-fresh", createdAt = now - 900, id = "f4".repeat(32)), null, true) - LocalCache.justConsume(beat("dvm-two-stale", createdAt = now - 901, id = "f5".repeat(32)), null, true) + consumePinned(beat("dvm-two-fresh", createdAt = now - 900, id = "f4".repeat(32))) + consumePinned(beat("dvm-two-stale", createdAt = now - 901, id = "f5".repeat(32))) assertTrue("exactly 900s old counts as fresh", LocalCache.hasFreshDvmHeartbeat(freshApp, now)) assertFalse("901s old is stale", LocalCache.hasFreshDvmHeartbeat(staleApp, now)) @@ -104,8 +119,8 @@ class DvmHeartbeatTest { fun theNewestBeatPerAddressWins() { val now = 1_760_000_000L val app = appDef("dvm-three") - LocalCache.justConsume(beat("dvm-three", createdAt = now - 600, id = "f6".repeat(32)), null, true) - LocalCache.justConsume(beat("dvm-three", createdAt = now - 60, id = "f7".repeat(32)), null, true) + consumePinned(beat("dvm-three", createdAt = now - 600, id = "f6".repeat(32))) + consumePinned(beat("dvm-three", createdAt = now - 60, id = "f7".repeat(32))) assertEquals(now - 60, LocalCache.dvmHeartbeatOf(app)?.createdAt) } @@ -115,7 +130,7 @@ class DvmHeartbeatTest { val now = 1_760_000_000L val appA = appDef("dvm-a") val appB = appDef("dvm-b") - LocalCache.justConsume(beat("dvm-a", createdAt = now - 60, id = "f8".repeat(32)), null, true) + consumePinned(beat("dvm-a", createdAt = now - 60, id = "f8".repeat(32))) assertTrue(LocalCache.hasFreshDvmHeartbeat(appA, now)) assertFalse("no beat for dvm-b", LocalCache.hasFreshDvmHeartbeat(appB, now)) @@ -177,11 +192,10 @@ class DvmHeartbeatTest { // scanning for them clears the lot and the scan returns nothing — which is exactly how // this failed on CI, where the heap is tighter than a dev box's. A fixture has to hold // what it expects to find, the same discipline LargeCacheAddressableFilterTest spells - // out; the forced GC below is what keeps that honest rather than assumed. - val held = - listOf(alive, appDef("dvm-x"), subscriptionApp, nonDiscoveryApp, secondSubscriptionApp) - .onEach { LocalCache.justConsume(it, null, true) } - .map { LocalCache.getOrCreateNote(it) } + // out, and it has to take hold BEFORE consuming (see [consumePinned]); the forced GC + // below is what keeps that honest rather than assumed. + listOf(alive, appDef("dvm-x"), subscriptionApp, nonDiscoveryApp, secondSubscriptionApp) + .forEach { consumePinned(it) } System.gc() @@ -192,6 +206,6 @@ class DvmHeartbeatTest { assertFalse("k=9999 apps are not content-discovery DVMs", scanned.any { it.dTag() == "other" }) assertEquals("newest-first so the cap keeps the most relevant announcements", scanned.sortedByDescending { it.createdAt }, scanned) assertTrue("capped", scanned.size <= 100) - assertEquals("the fixture notes must stay reachable for the whole test", 5, held.size) + assertEquals("the fixture notes must stay reachable for the whole test", 5, pinned.size) } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/EntityRatingIngestionTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/EntityRatingIngestionTest.kt index 609857aaf5..13cc3ce03f 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/EntityRatingIngestionTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/EntityRatingIngestionTest.kt @@ -21,10 +21,12 @@ package com.vitorpamplona.amethyst.model import com.vitorpamplona.amethyst.commons.model.HomeFeedType +import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.model.cache.LocalCache import com.vitorpamplona.quartz.experimental.publications.PublicationIndexEvent import com.vitorpamplona.quartz.experimental.ratings.EntityRatingEvent import com.vitorpamplona.quartz.nip01Core.core.Address +import com.vitorpamplona.quartz.nip01Core.core.Event import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull @@ -38,10 +40,23 @@ import org.junit.Test * * `LocalCache` is a process-wide object and JUnit 4's method order is hash-based, so every test * below uses its own author and identifier. + * + * Events go in through [consumePinned], which takes each event's note from the cache *before* + * consuming it and keeps it for the test's lifetime. `LocalCache` holds notes weakly + * (LargeSoftCache), so a note nothing references can be collected between two consumes — the + * older rating then lands on a fresh, empty slot — or before the read-back. In the app the + * screen showing the rating is that strong referent. */ class EntityRatingIngestionTest { private val publisher = "b1".repeat(32) + private val pinned = mutableListOf() + + private fun consumePinned(event: Event): Boolean { + pinned.add(LocalCache.getOrCreateNote(event)) + return LocalCache.justConsume(event, null, true) + } + private fun rating( id: String, author: String, @@ -75,7 +90,7 @@ class EntityRatingIngestionTest { val author = "b2".repeat(32) val event = rating("b3".repeat(32), author, "consumed-book", 5) - assertTrue("LocalCache must accept kind 34259", LocalCache.justConsume(event, null, true)) + assertTrue("LocalCache must accept kind 34259", consumePinned(event)) val note = LocalCache.getAddressableNoteIfExists(event.addressTag()) assertNotNull("the rating must land in the addressable index", note) @@ -88,8 +103,8 @@ class EntityRatingIngestionTest { val first = rating("b5".repeat(32), author, "replaced-book", 1, createdAt = 1_000_000L) val second = rating("b6".repeat(32), author, "replaced-book", 5, createdAt = 2_000_000L) - LocalCache.justConsume(first, null, true) - LocalCache.justConsume(second, null, true) + consumePinned(first) + consumePinned(second) val note = LocalCache.getAddressableNoteIfExists(first.addressTag()) assertEquals("the later rating wins the (pubkey, d) slot", second.id, note?.event?.id) @@ -102,8 +117,8 @@ class EntityRatingIngestionTest { val newer = rating("b8".repeat(32), author, "ordered-book", 5, createdAt = 2_000_000L) val older = rating("b9".repeat(32), author, "ordered-book", 1, createdAt = 1_000_000L) - LocalCache.justConsume(newer, null, true) - LocalCache.justConsume(older, null, true) + consumePinned(newer) + consumePinned(older) val note = LocalCache.getAddressableNoteIfExists(newer.addressTag()) assertEquals(newer.id, note?.event?.id) @@ -114,8 +129,8 @@ class EntityRatingIngestionTest { val one = rating("c1".repeat(32), "c2".repeat(32), "shared-book", 5) val two = rating("c3".repeat(32), "c4".repeat(32), "shared-book", 2) - LocalCache.justConsume(one, null, true) - LocalCache.justConsume(two, null, true) + consumePinned(one) + consumePinned(two) assertEquals(one.id, LocalCache.getAddressableNoteIfExists(one.addressTag())?.event?.id) assertEquals(two.id, LocalCache.getAddressableNoteIfExists(two.addressTag())?.event?.id) @@ -134,7 +149,7 @@ class EntityRatingIngestionTest { sig = "sig", ) - assertTrue("LocalCache must accept kind 30040", LocalCache.justConsume(index, null, true)) + assertTrue("LocalCache must accept kind 30040", consumePinned(index)) assertEquals("Wuthering Heights", (LocalCache.getAddressableNoteIfExists(index.addressTag())?.event as PublicationIndexEvent).title()) } @@ -146,7 +161,7 @@ class EntityRatingIngestionTest { val author = "c6".repeat(32) val event = rating("c7".repeat(32), author, "new-thread-book", 4) - LocalCache.justConsume(event, null, true) + consumePinned(event) val note = LocalCache.getAddressableNoteIfExists(event.addressTag())!! assertTrue("replyTo must stay empty", note.replyTo.isNullOrEmpty()) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/LocalCacheSearchParityTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/LocalCacheSearchParityTest.kt index 6bf410abc4..08e2c30f86 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/LocalCacheSearchParityTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/LocalCacheSearchParityTest.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.model +import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.model.cache.LocalCache import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter @@ -45,6 +46,11 @@ import java.io.File * * `LocalCache` is a process-wide object and JUnit's method order is hash-based, so the corpus is * loaded once and every assertion here is read-only. + * + * The corpus' notes are taken from the cache *before* consuming and kept in [pinned] for the whole + * class. `LocalCache` holds notes weakly (LargeSoftCache) and nothing else references a corpus note, + * so without the pin any GC between loading and a query drops notes and the query comes back short. + * In the app the screen showing a note is that strong referent. */ class LocalCacheSearchParityTest { companion object { @@ -53,6 +59,9 @@ class LocalCacheSearchParityTest { private lateinit var corpus: List + /** Strong references to every corpus note. See the class KDoc. */ + private lateinit var pinned: List + @BeforeClass @JvmStatic fun loadCorpus() { @@ -69,6 +78,8 @@ class LocalCacheSearchParityTest { .map { Event.fromJson(it.toString()) } .distinctBy { it.id } + pinned = corpus.map { LocalCache.getOrCreateNote(it) } + // LocalCache.consume refuses the main thread; a plain JVM test has no Looper, so the // check passes and the events land synchronously. corpus.forEach { LocalCache.justConsumeMyOwnEvent(it) } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/ReportNamingIndexIngestionTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/ReportNamingIndexIngestionTest.kt index 5d64a594db..7556ca7a14 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/ReportNamingIndexIngestionTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/ReportNamingIndexIngestionTest.kt @@ -24,14 +24,20 @@ import com.vitorpamplona.amethyst.commons.model.cache.LocalCache import com.vitorpamplona.quartz.nip56Reports.ReportEvent import com.vitorpamplona.quartz.nip56Reports.ReportType import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue import org.junit.Test /** * `LocalCache` is a process-wide object (`LocalCache.kt:353`) and JUnit 4's default method order is * hash-based, not source order. So each test method uses its **own** reporter, target and event ids — * sharing them across methods would let one test's consumed report inflate another's count depending - * on which ran first. + * on which ran first. The ids must also be unused by every OTHER test class in the JVM: this suite + * once shared `c3…`/`d3…`/`e5…` with the rating, appointment and calendar suites, and whenever one + * of their notes was still alive the report here was treated as already seen and never indexed. + * + * Each test takes the reported users from the cache *before* consuming and keeps them. `LocalCache` + * holds users weakly (LargeSoftCache) and nothing else references a user that is only named by a + * report, so a GC before the read-back would drop the user and the report index with it. In the app + * the screen showing the user is that strong referent. */ class ReportNamingIndexIngestionTest { private fun reportNamingBothNoteAndAuthor( @@ -54,48 +60,45 @@ class ReportNamingIndexIngestionTest { @Test fun aReportFiledFromANoteStillReachesTheUserNamingIndex() { - val reporter = "c1".repeat(32) - val target = "c2".repeat(32) + val reporter = "7a".repeat(32) + val target = "7b".repeat(32) + val reported = LocalCache.getOrCreateUser(target) LocalCache.consume( - reportNamingBothNoteAndAuthor("c3".repeat(32), reporter, target, "c4".repeat(32)), + reportNamingBothNoteAndAuthor("7c".repeat(32), reporter, target, "7d".repeat(32)), null, true, ) - val reported = LocalCache.getUserIfExists(target) - assertTrue("the reported author must exist in the cache", reported != null) - - assertEquals(1, reported!!.reports().reportsNaming(setOf(reporter)).size) + assertEquals(1, reported.reports().reportsNaming(setOf(reporter)).size) } @Test fun theHideThresholdCountIsUnaffectedByNoteFiledReports() { - val reporter = "d1".repeat(32) - val target = "d2".repeat(32) + val reporter = "8a".repeat(32) + val target = "8b".repeat(32) + val reported = LocalCache.getOrCreateUser(target) LocalCache.consume( - reportNamingBothNoteAndAuthor("d3".repeat(32), reporter, target, "d4".repeat(32)), + reportNamingBothNoteAndAuthor("8c".repeat(32), reporter, target, "8d".repeat(32)), null, true, ) - val reported = LocalCache.getUserIfExists(target)!! - assertEquals(1, reported.reports().reportsNaming(setOf(reporter)).size) assertEquals(0, reported.reports().countReportAuthorsBy(setOf(reporter))) } @Test fun aBareCoMentionedPTagIsNotIndexedWhileTheExplicitlyTypedOffenderIs() { - val reporter = "e1".repeat(32) - val offender = "e2".repeat(32) - val bystander = "e3".repeat(32) - val reportedNoteId = "e4".repeat(32) + val reporter = "9a".repeat(32) + val offender = "9b".repeat(32) + val bystander = "9c".repeat(32) + val reportedNoteId = "9e".repeat(32) val report = ReportEvent( - id = "e5".repeat(32), + id = "7e".repeat(32), pubKey = reporter, createdAt = 1_700_000_000L, tags = @@ -110,14 +113,12 @@ class ReportNamingIndexIngestionTest { sig = "sig", ) + val reportedOffender = LocalCache.getOrCreateUser(offender) + val reportedBystander = LocalCache.getOrCreateUser(bystander) + LocalCache.consume(report, null, true) - val reportedOffender = LocalCache.getUserIfExists(offender) - assertTrue("the offender must exist in the cache", reportedOffender != null) - assertEquals(1, reportedOffender!!.reports().reportsNaming(setOf(reporter)).size) - - val reportedBystander = LocalCache.getUserIfExists(bystander) - assertTrue("the bystander must exist in the cache", reportedBystander != null) - assertEquals(0, reportedBystander!!.reports().reportsNaming(setOf(reporter)).size) + assertEquals(1, reportedOffender.reports().reportsNaming(setOf(reporter)).size) + assertEquals(0, reportedBystander.reports().reportsNaming(setOf(reporter)).size) } } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/CoordinatorPipelineTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/CoordinatorPipelineTest.kt index 9b8a88949a..b4bcddcca5 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/CoordinatorPipelineTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/CoordinatorPipelineTest.kt @@ -55,6 +55,12 @@ import kotlin.test.assertTrue * * Key invariant being tested: when coordinator.consumeEvent() is called, * the event should flow through cache → eventStream → ViewModel.feedState. + * + * Tests that expect a note in a feed take it from the cache *before* it is consumed and + * keep it ([pinned]). [DesktopLocalCache] holds notes weakly (LargeSoftCache) and the feed + * filters find notes by scanning it, so a note nothing references can be collected during + * the bundler wait and never reach the feed. In the app the screen showing it is that + * strong referent. */ class CoordinatorPipelineTest { private val userPubKey = "a".repeat(64) @@ -64,6 +70,13 @@ class CoordinatorPipelineTest { private suspend fun waitForBundler() = delay(600) + /** Strong references to notes a test expects to see in a feed. See the class KDoc. */ + private val pinned = mutableListOf() + + private fun DesktopLocalCache.pinNote(id: HexKey) { + pinned.add(getOrCreateNote(id)) + } + /** * Stub INostrClient — records subscription calls but doesn't connect to any relay. * This lets us test the coordinator's event routing without network dependencies. @@ -171,6 +184,7 @@ class CoordinatorPipelineTest { content = "Hello from relay", sig = dummySig, ) + cache.pinNote(event.id) coordinator.consumeEvent(event, relayUrl, wasVerified = true) waitForBundler() @@ -278,6 +292,7 @@ class CoordinatorPipelineTest { content = "Note from followed user", sig = dummySig, ) + cache.pinNote(textEvent.id) coordinator.consumeEvent(textEvent, relayUrl, wasVerified = true) waitForBundler() @@ -352,6 +367,8 @@ class CoordinatorPipelineTest { sig = dummySig, ) + cache.pinNote(event.id) + // Consume same event twice (can happen with multiple relays) coordinator.consumeEvent(event, relayUrl, wasVerified = true) coordinator.consumeEvent(event, NormalizedRelayUrl("wss://relay2.test/"), wasVerified = true) diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCachePollTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCachePollTest.kt index a5b55291df..e1dfa6c2e3 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCachePollTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCachePollTest.kt @@ -33,6 +33,12 @@ import kotlin.test.assertTrue * NIP-88 poll consumption: a kind-1068 poll becomes a renderable Note, and a kind-1018 * response is linked into that poll Note's `pollState()` tally. Second identical response * (a relay echo) must not double-count. + * + * Each test takes the poll [com.vitorpamplona.amethyst.commons.model.Note] from the cache + * *before* consuming and keeps it. [DesktopLocalCache] holds notes weakly (LargeSoftCache), + * so a poll note nothing references can be collected between the poll's consume and the + * response's — the response would then tally into a fresh, empty note — or before the + * read-back. In the app the screen showing the poll is that strong referent. */ class DesktopLocalCachePollTest { private val relayUrl = NormalizedRelayUrl("wss://relay.test/") @@ -77,13 +83,13 @@ class DesktopLocalCachePollTest { val voter = NostrSignerSync(KeyPair()) val poll = signedPoll(author, createdAt = 1_700_000_000) + val pollNote = cache.getOrCreateNote(poll.id) assertTrue(cache.consume(poll, relayUrl, wasVerified = true), "poll should be consumed") val response = signedResponse(voter, poll.id, option = "0", createdAt = 1_700_000_100) assertTrue(cache.consume(response, relayUrl, wasVerified = true), "response should be consumed") - val pollNote = cache.getNoteIfExists(poll.id) - assertTrue(pollNote != null, "poll note must exist") + assertTrue(pollNote.event is PollEvent, "poll note must hold the poll") val tally = pollNote.pollState().responses.value assertEquals(1, tally.totalVoters()) assertEquals("0", tally.winning()) @@ -96,6 +102,7 @@ class DesktopLocalCachePollTest { val voter = NostrSignerSync(KeyPair()) val poll = signedPoll(author, createdAt = 1_700_000_000) + val pollNote = cache.getOrCreateNote(poll.id) cache.consume(poll, relayUrl, wasVerified = true) val response = signedResponse(voter, poll.id, option = "1", createdAt = 1_700_000_100) @@ -103,11 +110,7 @@ class DesktopLocalCachePollTest { // Same signed event echoed back by another relay — id-dedup must reject it. assertTrue(!cache.consume(response, relayUrl, wasVerified = true)) - val tally = - cache - .getNoteIfExists(poll.id)!! - .pollState() - .responses.value + val tally = pollNote.pollState().responses.value assertEquals(1, tally.totalVoters()) } } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCacheQuoteBoostTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCacheQuoteBoostTest.kt index 2641907f78..438c5dafb8 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCacheQuoteBoostTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCacheQuoteBoostTest.kt @@ -36,6 +36,12 @@ import kotlin.test.assertTrue * only collected kind:6/kind:16 reposts — so quote-reposts never showed in the * quoted note's reaction row. These tests pin the fix: consuming a `q`-tagged note * adds it as a boost of the quoted note. + * + * Each test takes the quoted note from the cache *before* consuming and keeps it. + * [DesktopLocalCache] holds notes weakly (LargeSoftCache), and nothing else references the + * quoted note — a quote is kept out of `replyTo` — so a GC before the read-back would drop + * it along with the boost it just received. In the app the screen showing it is that + * strong referent. */ class DesktopLocalCacheQuoteBoostTest { private val relayUrl = NormalizedRelayUrl("wss://relay.test/") @@ -65,13 +71,12 @@ class DesktopLocalCacheQuoteBoostTest { fun `a quote-repost counts as a boost of the quoted note`() { val cache = DesktopLocalCache() val quote = Event.fromJson(quoteJson) + val quotedNote = cache.getOrCreateNote(quotedId) // wasVerified = true: this test pins the boost wiring, not signature checks. val consumed = cache.consume(quote, relayUrl, wasVerified = true) assertTrue(consumed, "The quote-repost should be consumed") - val quotedNote = cache.getNoteIfExists(quotedId) - assertTrue(quotedNote != null, "The quoted note placeholder should exist") assertEquals(1, quotedNote.boosts.size, "The quote should count as one boost") assertEquals(quote.id, quotedNote.boosts.first().idHex) } @@ -88,6 +93,7 @@ class DesktopLocalCacheQuoteBoostTest { tags = emptyArray(), content = "the original post", ) + val originalNote = cache.getOrCreateNote(original.id) cache.consume(original, relayUrl, wasVerified = true) val quote = @@ -99,8 +105,6 @@ class DesktopLocalCacheQuoteBoostTest { ) cache.consume(quote, relayUrl, wasVerified = true) - val originalNote = cache.getNoteIfExists(original.id) - assertTrue(originalNote != null) assertEquals(1, originalNote.boosts.size, "The quote should boost the original") assertEquals(quote.id, originalNote.boosts.first().idHex) } @@ -117,6 +121,7 @@ class DesktopLocalCacheQuoteBoostTest { tags = emptyArray(), content = "target", ) + val targetNote = cache.getOrCreateNote(target.id) cache.consume(target, relayUrl, wasVerified = true) val plain = @@ -128,6 +133,6 @@ class DesktopLocalCacheQuoteBoostTest { ) cache.consume(plain, relayUrl, wasVerified = true) - assertEquals(0, cache.getNoteIfExists(target.id)?.boosts?.size) + assertEquals(0, targetNote.boosts.size) } } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCacheVerifyTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCacheVerifyTest.kt index 3ed2085d90..7d7f005eec 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCacheVerifyTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCacheVerifyTest.kt @@ -27,7 +27,6 @@ import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse -import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -37,6 +36,11 @@ import kotlin.test.assertTrue * desktop equivalent of Amethyst Android's `LocalCache.justVerify` guard * and closes the receive-time verification gap flagged in the Quartz * security review (item 2.1, finding #1). + * + * The acceptance tests take the note from the cache *before* consuming and keep it: + * [DesktopLocalCache] holds notes weakly (LargeSoftCache), so a note nothing references + * can be collected before the read-back and look as if it was never stored. The + * rejection tests look notes up without pinning on purpose — pinning would create one. */ class DesktopLocalCacheVerifyTest { private val relayUrl = NormalizedRelayUrl("wss://relay.test/") @@ -102,13 +106,12 @@ class DesktopLocalCacheVerifyTest { fun `consume accepts a properly signed event`() { val cache = DesktopLocalCache() val signed = signedTextNote(content = "authentic") + val note = cache.getOrCreateNote(signed.id) val consumed = cache.consume(signed, relayUrl) assertTrue(consumed, "Signed event should be accepted") - val note = cache.getNoteIfExists(signed.id) - assertNotNull(note, "Signed event must reach the cache") - assertEquals(signed.id, note.event?.id) + assertEquals(signed.id, note.event?.id, "Signed event must reach the cache") } @Test @@ -141,10 +144,11 @@ class DesktopLocalCacheVerifyTest { content = "synthetic", sig = "0".repeat(128), ) + val note = cache.getOrCreateNote(event.id) val consumed = cache.consume(event, relayUrl, wasVerified = true) assertTrue(consumed, "wasVerified=true must skip the signature check") - assertNotNull(cache.getNoteIfExists(event.id)) + assertEquals(event.id, note.event?.id) } } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/FindUsersTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/FindUsersTest.kt index e1fe6a991b..c134bb9edf 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/FindUsersTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/cache/FindUsersTest.kt @@ -40,10 +40,13 @@ class FindUsersTest { * and `findUsersStartingWith` could collect them and make the search return fewer results * (flaky failure at line 115). Every test keeps its users in a strong-reference list that * stays reachable through the assertions to pin them in the cache. + * + * The pin must be taken BEFORE `consumeMetadata`: pinning afterwards leaves the window + * between the consume and the pin open, and a GC there drops the user with its metadata. */ - private val retained = mutableListOf() + private val retained = mutableListOf() - private fun DesktopLocalCache.retainUser(pubkey: String): User? = getUserIfExists(pubkey).also { retained.add(it) } + private fun DesktopLocalCache.pinUser(pubkey: String): User = getOrCreateUser(pubkey).also { retained.add(it) } private fun fakeMetadata( pubKey: String, @@ -64,7 +67,7 @@ class FindUsersTest { val cache = createCache() val pubkey = KeyPair().pubKey.toHexKey() - retained.add(cache.getOrCreateUser(pubkey)) + cache.pinUser(pubkey) val results = cache.findUsersStartingWith("test", 10) assertTrue(results.isEmpty(), "User without metadata should not match name search") @@ -75,8 +78,8 @@ class FindUsersTest { val cache = createCache() val pubkey = KeyPair().pubKey.toHexKey() + cache.pinUser(pubkey) cache.consumeMetadata(fakeMetadata(pubkey, "vitor", "Vitor Pamplona")) - cache.retainUser(pubkey) val results = cache.findUsersStartingWith("Vitor", 10) assertEquals(1, results.size, "Should find user by display name") @@ -88,8 +91,8 @@ class FindUsersTest { val cache = createCache() val pubkey = KeyPair().pubKey.toHexKey() + cache.pinUser(pubkey) cache.consumeMetadata(fakeMetadata(pubkey, "vitor")) - cache.retainUser(pubkey) val results = cache.findUsersStartingWith("vit", 10) assertEquals(1, results.size, "Should find user by name prefix") @@ -100,8 +103,8 @@ class FindUsersTest { val cache = createCache() val pubkey = KeyPair().pubKey.toHexKey() + cache.pinUser(pubkey) cache.consumeMetadata(fakeMetadata(pubkey, "Vitor", "Vitor Pamplona")) - cache.retainUser(pubkey) val lower = cache.findUsersStartingWith("vitor", 10) assertEquals(1, lower.size, "Should find case-insensitively (lowercase)") @@ -115,7 +118,7 @@ class FindUsersTest { val cache = createCache() val pubkey = KeyPair().pubKey.toHexKey() - retained.add(cache.getOrCreateUser(pubkey)) + cache.pinUser(pubkey) val results = cache.findUsersStartingWith(pubkey.take(8), 10) assertEquals(1, results.size, "Should find user by pubkey prefix") @@ -127,8 +130,8 @@ class FindUsersTest { listOf("alice", "bob", "alex").forEach { name -> val pubkey = KeyPair().pubKey.toHexKey() + cache.pinUser(pubkey) cache.consumeMetadata(fakeMetadata(pubkey, name)) - cache.retainUser(pubkey) } val results = cache.findUsersStartingWith("al", 10) @@ -140,7 +143,7 @@ class FindUsersTest { val cache = createCache() // Simulate users created from kind 1 notes (no metadata) - repeat(10) { retained.add(cache.getOrCreateUser(KeyPair().pubKey.toHexKey())) } + repeat(10) { cache.pinUser(KeyPair().pubKey.toHexKey()) } assertEquals(10, cache.userCount()) @@ -153,10 +156,10 @@ class FindUsersTest { val cache = createCache() val pubkey = KeyPair().pubKey.toHexKey() + val user = cache.pinUser(pubkey) cache.consumeMetadata(fakeMetadata(pubkey, "testuser", "Test User")) - val user = cache.retainUser(pubkey) - val metadata = user?.metadataOrNull() + val metadata = user.metadataOrNull() assertNotNull(metadata, "Metadata should exist after consumeMetadata") assertTrue( diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/relay/LocalRelayStoreHydrationTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/relay/LocalRelayStoreHydrationTest.kt index f039e7deb7..25eea26bea 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/relay/LocalRelayStoreHydrationTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/relay/LocalRelayStoreHydrationTest.kt @@ -37,7 +37,6 @@ import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -208,14 +207,20 @@ class LocalRelayStoreHydrationTest { val cache = DesktopLocalCache().apply { accountPubkey = ownerPubKey } val store = newStore() + + // Pin the note across hydrate -> assert: the cache holds notes weakly + // (LargeSoftCache) and nothing else references a hydrated note here, so a + // GC in between would drop it and read as "never hydrated". In the app the + // feed showing the note is that strong referent. + val pinned = cache.getOrCreateNote(recentNote.id) + try { store.hydrate(cache) } finally { store.close() } - val note = cache.getNoteIfExists(recentNote.id) - assertNotNull(note, "Recent text note must be hydrated into cache") + assertEquals(recentNote.id, pinned.event?.id, "Recent text note must be hydrated into cache") } @Test @@ -258,15 +263,16 @@ class LocalRelayStoreHydrationTest { val cache = DesktopLocalCache().apply { accountPubkey = ownerPubKey } val store = newStore() + + // Pinned for the same reason as in recentTextNotesWithinSevenDayWindowAreHydrated. + val pinned = cache.getOrCreateNote(note.id) + try { store.hydrate(cache) } finally { store.close() } - assertFalse( - cache.getNoteIfExists(note.id) == null, - "Round-trip through hydrate must not drop a valid note", - ) + assertEquals(note.id, pinned.event?.id, "Round-trip through hydrate must not drop a valid note") } }