From fbe163e8f28db1ab40e4ec22ed6ecccdfb7595f7 Mon Sep 17 00:00:00 2001 From: nrobi144 Date: Mon, 10 Aug 2026 11:00:52 +0300 Subject: [PATCH] test+docs: commons finder test + document per-visible loading (Phase 5) - EventFinderFilterAssemblyTest (commons jvmTest): filterMissingEvents batches one ids-filter per relay with sorted ids; ICacheProvider.checkGetOrCreateUser default tolerates a throwing/null getOrCreateUser via a fake cache. - relay-client skill + commons/ARCHITECTURE.md: document observeUser*/ EventFinderFilterAssemblerSubscription/observeNote* as the canonical per-visible loading entry points and the LocalUserFinder/LocalUserFinderAccount/LocalEventFinder seams. Co-Authored-By: Claude Opus 4.8 --- .claude/skills/relay-client/SKILL.md | 26 ++++ commons/ARCHITECTURE.md | 2 +- .../event/EventFinderFilterAssemblyTest.kt | 119 ++++++++++++++++++ 3 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/relayClient/event/EventFinderFilterAssemblyTest.kt diff --git a/.claude/skills/relay-client/SKILL.md b/.claude/skills/relay-client/SKILL.md index c1019cde8f..1b72fb6977 100644 --- a/.claude/skills/relay-client/SKILL.md +++ b/.claude/skills/relay-client/SKILL.md @@ -94,6 +94,32 @@ class MetadataFilterAssembler( Assemblers stay pure — no state, no I/O. They're the composition seam: `FeedMetadataCoordinator` takes a list of visible notes and assembles a single metadata filter covering every referenced pubkey. +## Per-visible loading — the canonical entry points (`observeUser*` / `observeNote*`) + +Prefer these over hand-rolled "load metadata for this list" calls. They are the shared, +KMP way to load data **only for what's on screen** — a composable subscribes while it is in +composition and unsubscribes ~30s after it leaves (or the app backgrounds). Both live in +`commons/relayClient/`: + +- **Per user** (`relayClient/user/`): `observeUserInfo/Picture/Banner/AboutMe/Name(user)` + each open a composition-scoped `UserFinderFilterAssemblerSubscription(user)` **and** return + reactive `State`. Metadata (kind 0 + relay lists) loads for on-screen users only, coalesced + into one batched REQ per relay for the whole visible set. +- **Per note** (`relayClient/event/`): `EventFinderFilterAssemblerSubscription(note)` loads a + note's interactions (reactions / zaps / reposts / replies) while it is composed. Android's + `observeNote*` display observers layer on top of the same subscription. + +Both read front-end-provided CompositionLocals — `LocalUserFinder` / `LocalUserFinderAccount` +(reused by the event finder) / `LocalEventFinder` — provided once near the composition root +(Android `AppModules`, Desktop `Main.kt` via its subscriptions coordinator). The account seam +is the narrow `UserFinderAccount` (snapshot relay-hint getters), NOT the fat `IAccount`. +`error()` defaults mean these must never be reached from a composition without a relay client +(e.g. the Android `:napplet` sandbox). + +The load-once, viewport-batch path (`FeedMetadataCoordinator.loadMetadataForNotes` / +`loadMetadataBatched`) is superseded for foreground loading; `MetadataPreloader` remains only +as an optional off-screen background warmer. + ## Preloaders `MetadataPreloader` is the "I need metadata for 200 pubkeys, but don't melt my CPU or the relay" path. It uses `MetadataRateLimiter` (token bucket) to throttle bulk fetches and group them into relay-friendly chunks. diff --git a/commons/ARCHITECTURE.md b/commons/ARCHITECTURE.md index b86c122c6f..25c594b532 100644 --- a/commons/ARCHITECTURE.md +++ b/commons/ARCHITECTURE.md @@ -101,7 +101,7 @@ they are shared across the GUI apps. Treat as GUI-shared, not strictly headless. ### Relay client | Package | UI? | Purpose | |----------------|-----|---------| -| `relayClient` | no | Compose-scoped subscription managers, filter assemblers, EOSE managers, preloaders. (Despite a `composeSubscriptionManagers` subpackage name, this is subscription-lifecycle logic, not UI.) | +| `relayClient` | no | Compose-scoped subscription managers, filter assemblers, EOSE managers, preloaders. (Despite a `composeSubscriptionManagers` subpackage name, this is subscription-lifecycle logic, not UI.) The canonical **per-visible loading** entry points live here: `relayClient/user/` (`observeUser*` — kind-0 metadata) and `relayClient/event/` (`EventFinderFilterAssemblerSubscription`/`observeNote*` — reactions/zaps/reposts). See the `relay-client` skill. | | `relays` | no | Low-level EOSE/relay-timing bookkeeping (`EOSECache`, `EOSERelayList`). | ### Platform abstractions (`expect`/`actual`) diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/relayClient/event/EventFinderFilterAssemblyTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/relayClient/event/EventFinderFilterAssemblyTest.kt new file mode 100644 index 0000000000..034565a5d5 --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/relayClient/event/EventFinderFilterAssemblyTest.kt @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.relayClient.event + +import com.vitorpamplona.amethyst.commons.model.Channel +import com.vitorpamplona.amethyst.commons.model.Note +import com.vitorpamplona.amethyst.commons.model.User +import com.vitorpamplona.amethyst.commons.model.cache.ICacheEventStream +import com.vitorpamplona.amethyst.commons.model.cache.ICacheProvider +import com.vitorpamplona.amethyst.commons.relayClient.event.loaders.filterMissingEvents +import com.vitorpamplona.quartz.nip01Core.core.Address +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.hints.HintIndexer +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for the extracted (Phase 2b) per-note event-finder filter assembly + * and the [ICacheProvider] seam it relies on. + */ +class EventFinderFilterAssemblyTest { + private val relay1 = NormalizedRelayUrl("wss://relay1.test/") + private val relay2 = NormalizedRelayUrl("wss://relay2.test/") + + /** + * One batched `ids` filter per relay — NOT one filter per event id — and the + * ids are sorted deterministically so REQs dedup across rebuilds. + */ + @Test + fun `filterMissingEvents batches one filter per relay with sorted ids`() { + val filters = + filterMissingEvents( + mapOf( + relay1 to setOf("bbbb", "aaaa", "cccc"), + relay2 to setOf("dddd"), + ), + ) + + assertEquals("one batched filter per relay", 2, filters.size) + + val r1 = filters.first { it.relay == relay1 } + assertEquals(listOf("aaaa", "bbbb", "cccc"), r1.filter.ids) + + val r2 = filters.first { it.relay == relay2 } + assertEquals(listOf("dddd"), r2.filter.ids) + } + + @Test + fun `filterMissingEvents skips relays with no ids and empty input`() { + assertTrue(filterMissingEvents(emptyMap()).isEmpty()) + assertTrue(filterMissingEvents(mapOf(relay1 to emptySet())).isEmpty()) + } + + /** + * The new default [ICacheProvider.checkGetOrCreateUser] must swallow a + * malformed-key throw and return null (the event-finder follows pubkey hints + * parsed out of arbitrary events, some of which are junk). + */ + @Test + fun `checkGetOrCreateUser tolerates a throwing getOrCreateUser`() { + val throwing = + object : StubCache() { + override fun getOrCreateUser(pubkey: HexKey): User? = throw IllegalArgumentException("bad key") + } + assertNull(throwing.checkGetOrCreateUser("not-a-key")) + } + + @Test + fun `checkGetOrCreateUser passes through a null result`() { + assertNull(StubCache().checkGetOrCreateUser("00")) + } + + /** Minimal [ICacheProvider] that returns nothing; override per test. */ + private open class StubCache : ICacheProvider { + override val relayHints = HintIndexer() + + override fun getAnyChannel(note: Note): Channel? = null + + override fun getUserIfExists(pubkey: HexKey): User? = null + + override fun countUsers(predicate: (String, User) -> Boolean): Int = 0 + + override fun getNoteIfExists(hexKey: HexKey): Note? = null + + override fun checkGetOrCreateNote(hexKey: HexKey): Note? = null + + override fun getOrCreateAddressableNote(key: Address): com.vitorpamplona.amethyst.commons.model.AddressableNote = error("unused") + + override fun getEventStream(): ICacheEventStream = error("unused") + + override fun hasBeenDeleted(event: Any): Boolean = false + + override fun getOrCreateUser(pubkey: HexKey): User? = null + + override fun justConsumeMyOwnEvent(event: Event): Boolean = false + } +}