From b5313e28ca0c44b54e878463239ae55b4fe49556 Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 8 Jul 2026 22:54:07 +0100 Subject: [PATCH] refactor: replace duplicated string literals with constants docs: explain the intentionally empty default of RelayUnderTest.prepare fix: surface failed checkpoint deletion in CorpusDownloader --- .../com/vitorpamplona/amethyst/cli/Output.kt | 6 +++++ .../amethyst/cli/commands/AdminCommand.kt | 2 +- .../amethyst/cli/commands/RelayCommands.kt | 22 ++++++++++++------- .../amethyst/cli/commands/SyncCommand.kt | 2 +- .../kotlin/com/vitorpamplona/geode/Main.kt | 20 ++++++++++------- .../com/vitorpamplona/relaybench/Main.kt | 8 ++++--- .../relaybench/corpus/CorpusDownloader.kt | 4 +++- .../relaybench/relays/RelayUnderTest.kt | 6 ++++- 8 files changed, 47 insertions(+), 23 deletions(-) diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/Output.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/Output.kt index 7f30d19349..d24b83a3dc 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/Output.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/Output.kt @@ -86,6 +86,12 @@ object Output { return 1 } + /** + * Shared `bad_args` failure for any command that takes a relay-URL + * argument, so every command names the offending input the same way. + */ + fun invalidRelayUrl(raw: String): Int = error("bad_args", "invalid relay url: $raw") + private fun renderText(value: Any?): String { val color = Ansi.forStream(isStderr = false) val out = StringBuilder() diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/AdminCommand.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/AdminCommand.kt index d377861911..1c80df3c3d 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/AdminCommand.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/AdminCommand.kt @@ -55,7 +55,7 @@ object AdminCommand { val args = Args(rest) val relayArg = args.positionalOrNull(0) ?: return Output.error("bad_args", "usage: admin RELAY METHOD [args]") val method = args.positionalOrNull(1) ?: return Output.error("bad_args", "missing method; e.g. supported-methods") - val relay = RelayUrlNormalizer.normalizeOrNull(relayArg) ?: return Output.error("bad_args", "invalid relay url: $relayArg") + val relay = RelayUrlNormalizer.normalizeOrNull(relayArg) ?: return Output.invalidRelayUrl(relayArg) val p2 = args.positionalOrNull(2) val reason = args.flag("reason") diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/RelayCommands.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/RelayCommands.kt index 1ea2698858..74f48c1506 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/RelayCommands.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/RelayCommands.kt @@ -237,7 +237,7 @@ object RelayCommands { val raw = args.positional(0, "relay-url") val normalized = raw.normalizeRelayUrlOrNull() - ?: return Output.error("bad_args", "invalid relay url: $raw") + ?: return Output.invalidRelayUrl(raw) val httpUrl = normalized.toHttp() val request = @@ -286,21 +286,21 @@ object RelayCommands { val self = ctx.identity.pubKeyHex when (verb) { "add" -> { - val url = parseUrl(args.positional(0, "url")) ?: return Output.error("bad_args", "invalid relay url") + val url = urlArg(args) ?: return Output.invalidRelayUrl(args.positional(0, "url")) val existing = flat.read(ctx, self) val added = existing.none { it.url == url.url } if (added) ctx.verifyAndStore(flat.build(ctx, existing + url)) Output.emit(mapOf("noun" to flat.noun, "kind" to flat.kind, "url" to url.url, "added" to added)) } "remove", "rm" -> { - val url = parseUrl(args.positional(0, "url")) ?: return Output.error("bad_args", "invalid relay url") + val url = urlArg(args) ?: return Output.invalidRelayUrl(args.positional(0, "url")) val existing = flat.read(ctx, self) val removed = existing.any { it.url == url.url } if (removed) ctx.verifyAndStore(flat.build(ctx, existing.filterNot { it.url == url.url })) Output.emit(mapOf("noun" to flat.noun, "kind" to flat.kind, "url" to url.url, "removed" to removed)) } "set" -> { - val relays = parseUrls(args.positional) ?: return Output.error("bad_args", "invalid relay url") + val relays = parseUrls(args.positional) ?: return badUrlIn(args.positional) if (relays.isEmpty()) return Output.error("bad_args", "set needs at least one URL; use `relay ${flat.noun} clear` to empty it") val signed = flat.build(ctx, relays) ctx.verifyAndStore(signed) @@ -335,7 +335,7 @@ object RelayCommands { when (verb) { "add", "remove", "rm" -> { val present = verb == "add" - val url = parseUrl(args.positional(0, "url")) ?: return Output.error("bad_args", "invalid relay url") + val url = urlArg(args) ?: return Output.invalidRelayUrl(args.positional(0, "url")) val changed = mutateNip65(ctx, self) { applyFacet(it, url, facet, present) } Output.emit( mapOf( @@ -352,7 +352,7 @@ object RelayCommands { if (verb == "clear") { emptyList() } else { - val parsed = parseUrls(args.positional) ?: return Output.error("bad_args", "invalid relay url") + val parsed = parseUrls(args.positional) ?: return badUrlIn(args.positional) if (parsed.isEmpty()) return Output.error("bad_args", "set needs at least one URL; use `relay ${facet.noun} clear` to empty it") parsed } @@ -388,7 +388,7 @@ object RelayCommands { ) } "remove", "rm" -> { - val url = parseUrl(args.positional(0, "url")) ?: return Output.error("bad_args", "invalid relay url") + val url = urlArg(args) ?: return Output.invalidRelayUrl(args.positional(0, "url")) val removed = mutateNip65(ctx, self) { infos -> infos.filterNot { it.relayUrl.url == url.url } } Output.emit(mapOf("noun" to "nip65", "kind" to AdvertisedRelayListEvent.KIND, "url" to url.url, "removed" to removed)) } @@ -416,7 +416,7 @@ object RelayCommands { args: Args, add: Boolean, ): Int { - val url = parseUrl(args.positional(0, "url")) ?: return Output.error("bad_args", "invalid relay url") + val url = urlArg(args) ?: return Output.invalidRelayUrl(args.positional(0, "url")) Context.open(dataDir).use { ctx -> val self = ctx.identity.pubKeyHex val changed = linkedMapOf() @@ -518,6 +518,9 @@ object RelayCommands { private fun parseUrl(raw: String): NormalizedRelayUrl? = raw.normalizeRelayUrlOrNull() + /** The single relay-URL argument every add/remove verb takes, or null if it doesn't parse. */ + private fun urlArg(args: Args): NormalizedRelayUrl? = parseUrl(args.positional(0, "url")) + /** Normalize + dedupe (order-preserving) a list of raw URLs, or null on any bad one. */ private fun parseUrls(raws: List): List? { val out = mutableListOf() @@ -525,6 +528,9 @@ object RelayCommands { return out.distinctBy { it.url } } + /** Error exit naming the first URL in [raws] that made [parseUrls] fail. */ + private fun badUrlIn(raws: List): Int = Output.invalidRelayUrl(raws.first { parseUrl(it) == null }) + private suspend fun readNip65( ctx: Context, self: HexKey, diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/SyncCommand.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/SyncCommand.kt index 35bd5df2c1..ff74354184 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/SyncCommand.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/SyncCommand.kt @@ -113,7 +113,7 @@ object SyncCommand { ?: return Output.error("bad_args", "sync requires --relay URL") val relay = RelayUrlNormalizer.normalizeOrNull(relayUrl) - ?: return Output.error("bad_args", "invalid relay url: $relayUrl") + ?: return Output.invalidRelayUrl(relayUrl) val timeoutMs = (args.flag("timeout")?.toLongOrNull() ?: 30L) * 1000 // Default direction is download; --up adds upload. val up = args.bool("up") diff --git a/geode/src/main/kotlin/com/vitorpamplona/geode/Main.kt b/geode/src/main/kotlin/com/vitorpamplona/geode/Main.kt index 3ccb80af07..5c30fc2f60 100644 --- a/geode/src/main/kotlin/com/vitorpamplona/geode/Main.kt +++ b/geode/src/main/kotlin/com/vitorpamplona/geode/Main.kt @@ -128,9 +128,9 @@ private class StoreContext( ) private fun openStore(a: Args): StoreContext { - val config = a.opt("--config")?.let { StaticConfig.fromFile(File(it)) } ?: StaticConfig() + val config = a.opt(CONFIG_FLAG)?.let { StaticConfig.fromFile(File(it)) } ?: StaticConfig() val dbFile = a.opt("--db") ?: config.database.file?.takeUnless { config.database.in_memory } - val fullTextSearch = !a.flag("--no-search") && config.options.full_text_search + val fullTextSearch = !a.flag(NO_SEARCH_FLAG) && config.options.full_text_search val store = EventStore( dbName = dbFile, @@ -142,12 +142,12 @@ private fun openStore(a: Args): StoreContext { private fun runImport(args: Array) { val a = parseArgs(args) - val config = a.opt("--config")?.let { StaticConfig.fromFile(File(it)) } ?: StaticConfig() + val config = a.opt(CONFIG_FLAG)?.let { StaticConfig.fromFile(File(it)) } ?: StaticConfig() // Verify by default, matching the relay's stance — `import` won't trust a // file's signatures any more than the relay trusts a client's. `--no-verify` // is the trusted-input escape hatch (fixture replay, a dump from a relay you // already trust). - val verify = !a.flag("--no-verify") && config.options.verify_signatures + val verify = !a.flag(NO_VERIFY_FLAG) && config.options.verify_signatures val ctx = openStore(a) try { val stats = @@ -190,7 +190,7 @@ private fun serve(args: Array) { val config: StaticConfig = a - .opt("--config") + .opt(CONFIG_FLAG) ?.let { StaticConfig.fromFile(File(it)) } ?: StaticConfig() config.validate() @@ -208,7 +208,7 @@ private fun serve(args: Array) { // Verify is on by default; only disable when the operator explicitly // opts out (CLI `--no-verify` or `[options].verify_signatures = false` // in the config). - val verifySigs = !a.flag("--no-verify") && config.options.verify_signatures + val verifySigs = !a.flag(NO_VERIFY_FLAG) && config.options.verify_signatures // Parallel verify is on whenever signature checking is on; the // IngestQueue handles it instead of VerifyPolicy. Operators can // force the legacy in-policy path with `--no-parallel-verify` or @@ -218,7 +218,7 @@ private fun serve(args: Array) { // NIP-50 search is on by default; `--no-search` (or // `[options].full_text_search = false`) trades it for cheaper ingest — // e.g. to match relays that don't implement NIP-50 at all. - val fullTextSearch = !a.flag("--no-search") && config.options.full_text_search + val fullTextSearch = !a.flag(NO_SEARCH_FLAG) && config.options.full_text_search // Advertised URL: explicit `info.relay_url` wins, then build from // host/port/path. 0.0.0.0 bind → 127.0.0.1 in the URL so NIP-42 @@ -466,13 +466,17 @@ private class Args( fun flag(k: String) = k in flags } +private const val CONFIG_FLAG = "--config" +private const val NO_VERIFY_FLAG = "--no-verify" +private const val NO_SEARCH_FLAG = "--no-search" + /** * Boolean flags that never take a value. Listing them explicitly is what lets a * trailing positional survive after a flag — `import --no-verify corpus.ndjson` * must read `corpus.ndjson` as a file, not as `--no-verify`'s value. */ private val BOOLEAN_FLAGS = - setOf("--auth", "--optional-auth", "--no-verify", "--no-parallel-verify", "--no-search") + setOf("--auth", "--optional-auth", NO_VERIFY_FLAG, "--no-parallel-verify", NO_SEARCH_FLAG) private fun parseArgs(args: Array): Args { val opts = mutableMapOf() diff --git a/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/Main.kt b/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/Main.kt index 7fb340ef63..d1ae574128 100644 --- a/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/Main.kt +++ b/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/Main.kt @@ -246,6 +246,8 @@ private fun loadCorpus( ) } +private const val DOWNLOAD_FLAG = "--download" + private fun parseArgs(args: Array): Options? { val map = HashMap>() val flags = HashSet() @@ -260,7 +262,7 @@ private fun parseArgs(args: Array): Options? { "--base-time", "--corpus", "--limit", - "--download", + DOWNLOAD_FLAG, "--max-event-bytes", "--max-tags", "--samples", @@ -284,7 +286,7 @@ private fun parseArgs(args: Array): Options? { if (next != null && !next.startsWith("--")) { map.getOrPut(arg) { mutableListOf() }.add(next) i++ - } else if (arg == "--download") { + } else if (arg == DOWNLOAD_FLAG) { map.getOrPut(arg) { mutableListOf() }.add("") } else { System.err.println("Missing value for $arg") @@ -330,7 +332,7 @@ private fun parseArgs(args: Array): Options? { else -> t.toLongOrNull() ?: CorpusSpec.DEFAULT_BASE_TIME }, corpusFile = one("--corpus")?.let { File(it) }, - downloadFrom = map["--download"]?.lastOrNull()?.split(',')?.filter { it.isNotBlank() }, + downloadFrom = map[DOWNLOAD_FLAG]?.lastOrNull()?.split(',')?.filter { it.isNotBlank() }, limit = int("--limit", 0), maxEventBytes = int("--max-event-bytes", CorpusSource.DEFAULT_MAX_EVENT_BYTES), maxTags = int("--max-tags", CorpusSource.DEFAULT_MAX_TAGS), diff --git a/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/corpus/CorpusDownloader.kt b/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/corpus/CorpusDownloader.kt index d3c3b3965f..7a30334fca 100644 --- a/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/corpus/CorpusDownloader.kt +++ b/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/corpus/CorpusDownloader.kt @@ -137,7 +137,9 @@ object CorpusDownloader { val raw = CorpusIO.read(spill).events val corpus = CorpusSource.prepare(raw, target, "download:${relayUrls.joinToString(",")}", log) CorpusIO.write(cached, corpus) - checkpoint.delete() + if (!checkpoint.delete() && checkpoint.exists()) { + log(" ! could not delete stale checkpoint ${checkpoint.name}") + } log(" cached prepared corpus to ${cached.path}") return corpus } diff --git a/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/relays/RelayUnderTest.kt b/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/relays/RelayUnderTest.kt index a4ee0fa9c8..5d2d0a67d2 100644 --- a/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/relays/RelayUnderTest.kt +++ b/relayBench/src/main/kotlin/com/vitorpamplona/relaybench/relays/RelayUnderTest.kt @@ -49,7 +49,11 @@ abstract class RelayUnderTest( open fun prepare( port: Int, dataDir: File, - ) {} + ) { + // No-op by default: most relays are configured entirely through + // command-line flags. Overridden by relays that need config files + // on disk before launch (e.g. StrfryRelay). + } fun start(workDir: File): RunningRelay { val port = ServerSocket(0).use { it.localPort }