From b8d3791ea397ead6d11ddc3e59fc2cc79ebf4934 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sat, 4 Jul 2026 10:53:20 +0200 Subject: [PATCH] Code review: - only parse commonName when the alt tag has the Birdstar prefix - summary() on both Birdstar events now uses the canonical NIP-31 tags.alt() helper instead of a raw firstTagValue("alt") lookup - speciesReference() only returns http(s) URLs since UIs render it as a clickable link (rejects e.g. javascript: schemes), with a test - Detection card: parse tags once into a single remember slot, drop the near-dead '?: summary' title fallback, stop rebuilding the italic TextStyle every recomposition - Hoist the duplicated bird-emoji literal into a shared BIRD_PREFIX - Trim the redundant factory test to the assertIs idiom --- .../amethyst/ui/note/types/Birdex.kt | 50 ++++++++++++------- .../birdstar/BirdDetectionEvent.kt | 21 +++++--- .../experimental/birdstar/BirdexEvent.kt | 6 +-- .../birdstar/BirdDetectionEventTest.kt | 40 +++++++++++++-- 4 files changed, 85 insertions(+), 32 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt index 8f9de9df14..a73f3a2532 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt @@ -45,6 +45,9 @@ import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent /** How many species names to list before collapsing into a "+N more" suffix. */ private const val SPECIES_PREVIEW_LIMIT = 6 +/** Bird emoji prefix shared by both Birdstar card titles. */ +private const val BIRD_PREFIX = "\uD83D\uDC26 " + /** * Minimal, fixed-size summary card for a Birdstar "Birdex" (kind 12473). * @@ -65,7 +68,7 @@ fun RenderBirdex(baseNote: Note) { Column(MaterialTheme.colorScheme.replyModifier.padding(10.dp)) { Text( - text = "\uD83D\uDC26 " + pluralStringResource(R.plurals.birdex_species_count, names.size, names.size), + text = BIRD_PREFIX + pluralStringResource(R.plurals.birdex_species_count, names.size, names.size), style = MaterialTheme.typography.titleMedium, ) @@ -91,41 +94,47 @@ fun RenderBirdex(baseNote: Note) { * Minimal card for a single Birdstar bird detection (kind 2473). * * The event has no body and no images. The title is the common name parsed from - * the publisher's `alt` tag (falling back to the raw `alt`, then a generic - * label), and the scientific name renders as an italic link to the Wikidata - * species entry from the `i` tag when one is present. The `g` geohash is - * surfaced by the generic note-location UI, not here. + * the publisher's `alt` tag (a generic label when absent), and the scientific + * name renders as an italic link to the Wikidata species entry from the `i` tag + * when one is present. The `g` geohash is surfaced by the generic note-location + * UI, not here. */ @Composable fun RenderBirdDetection(baseNote: Note) { val noteEvent = baseNote.event as? BirdDetectionEvent ?: return - val commonName = remember(noteEvent) { noteEvent.commonName() } - val summary = remember(noteEvent) { noteEvent.summary() } - val species = remember(noteEvent) { noteEvent.speciesName() } - val reference = remember(noteEvent) { noteEvent.speciesReference() } + val detection = + remember(noteEvent) { + BirdDetectionInfo( + commonName = noteEvent.commonName(), + species = noteEvent.speciesName(), + reference = noteEvent.speciesReference(), + ) + } Column(MaterialTheme.colorScheme.replyModifier.padding(10.dp)) { Text( - text = "\uD83D\uDC26 " + (commonName ?: summary ?: stringResource(R.string.bird_detection_title)), + text = BIRD_PREFIX + (detection.commonName ?: stringResource(R.string.bird_detection_title)), style = MaterialTheme.typography.titleMedium, maxLines = 2, overflow = TextOverflow.Ellipsis, ) - if (species != null) { + if (detection.species != null) { Spacer(Modifier.height(6.dp)) - val speciesStyle = MaterialTheme.typography.bodyMedium.copy(fontStyle = FontStyle.Italic) - if (reference != null) { + if (detection.reference != null) { + val typography = MaterialTheme.typography + val speciesStyle = remember(typography) { typography.bodyMedium.copy(fontStyle = FontStyle.Italic) } ClickableUrl( - urlText = species, - url = reference, + urlText = detection.species, + url = detection.reference, style = speciesStyle, ) } else { Text( - text = species, - style = speciesStyle, + text = detection.species, + style = MaterialTheme.typography.bodyMedium, + fontStyle = FontStyle.Italic, color = MaterialTheme.colorScheme.placeholderText, maxLines = 1, overflow = TextOverflow.Ellipsis, @@ -134,3 +143,10 @@ fun RenderBirdDetection(baseNote: Note) { } } } + +/** Tag values parsed once per event for the detection card. */ +private class BirdDetectionInfo( + val commonName: String?, + val species: String?, + val reference: String?, +) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdDetectionEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdDetectionEvent.kt index fd1450d676..a45eef32cd 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdDetectionEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdDetectionEvent.kt @@ -24,6 +24,7 @@ import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.core.firstTagValue +import com.vitorpamplona.quartz.nip31Alts.alt import com.vitorpamplona.quartz.nip50Search.SearchableEvent /** @@ -59,20 +60,26 @@ class BirdDetectionEvent( /** Scientific name of the detected species, from the `n` tag (may be null). */ fun speciesName() = tags.firstTagValue("n") - /** External species reference (Wikidata entity URL), from the `i` tag (may be null). */ - fun speciesReference() = tags.firstTagValue("i") + /** + * External species reference (Wikidata entity URL), from the `i` tag. + * Only http(s) URLs are returned — UIs render this as a clickable link, + * so other schemes are rejected here rather than at every call site. + */ + fun speciesReference() = tags.firstTagValue("i")?.takeIf { it.startsWith("https://") || it.startsWith("http://") } - /** Publisher-provided human-readable summary, from the `alt` tag (may be null). */ - fun summary() = tags.firstTagValue("alt") + /** Publisher-provided human-readable summary, from the NIP-31 `alt` tag (may be null). */ + fun summary() = tags.alt() /** * Common (vernacular) species name, parsed out of the `alt` tag. Birdstar - * writes `Bird detection: ()`; this strips the - * fixed prefix and the trailing parenthetical. Null when the `alt` tag is - * missing or nothing is left after stripping. + * currently writes `Bird detection: ()` (in + * English); this strips the fixed prefix and the trailing parenthetical. + * Null when the `alt` tag is missing or nothing is left after stripping — + * including if a future Birdstar release rewords the alt text. */ fun commonName(): String? { val alt = summary() ?: return null + if (!alt.startsWith(ALT_PREFIX)) return null return alt .removePrefix(ALT_PREFIX) .substringBeforeLast(" (") diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt index 254635bfdc..5d15bb0896 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt @@ -23,8 +23,8 @@ package com.vitorpamplona.quartz.experimental.birdstar import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.nip01Core.core.BaseReplaceableEvent import com.vitorpamplona.quartz.nip01Core.core.HexKey -import com.vitorpamplona.quartz.nip01Core.core.firstTagValue import com.vitorpamplona.quartz.nip01Core.core.mapValueTagged +import com.vitorpamplona.quartz.nip31Alts.alt import com.vitorpamplona.quartz.nip50Search.SearchableEvent /** @@ -65,8 +65,8 @@ class BirdexEvent( /** Number of collected species (one `n` tag per species). */ fun speciesCount() = speciesNames().size - /** Publisher-provided human-readable summary, from the `alt` tag (may be null). */ - fun summary() = tags.firstTagValue("alt") + /** Publisher-provided human-readable summary, from the NIP-31 `alt` tag (may be null). */ + fun summary() = tags.alt() companion object { const val KIND = 12473 diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdDetectionEventTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdDetectionEventTest.kt index 1ce1037473..a65fb84f29 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdDetectionEventTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdDetectionEventTest.kt @@ -52,11 +52,24 @@ class BirdDetectionEventTest { @Test fun factoryBuildsBirdDetectionForKind2473() { - val event = sampleEvent() - assertTrue( - event is BirdDetectionEvent, - "Expected a BirdDetectionEvent but got ${event::class.simpleName}", - ) + assertIs(sampleEvent()) + } + + @Test + fun speciesReferenceRejectsNonHttpValues() { + val event: Event = + EventFactory.create( + id = "00".repeat(32), + pubKey = "00".repeat(32), + createdAt = 1_783_089_908L, + kind = BirdDetectionEvent.KIND, + tags = arrayOf(arrayOf("i", "javascript:alert(1)")), + content = "", + sig = "00".repeat(64), + ) + assertIs(event) + + assertEquals(null, event.speciesReference()) } @Test @@ -89,6 +102,23 @@ class BirdDetectionEventTest { assertEquals(null, event.commonName()) } + @Test + fun commonNameIsNullWhenAltLacksTheBirdstarPrefix() { + val event: Event = + EventFactory.create( + id = "00".repeat(32), + pubKey = "00".repeat(32), + createdAt = 1_783_089_908L, + kind = BirdDetectionEvent.KIND, + tags = arrayOf(arrayOf("alt", "Arbitrary attacker-controlled title (spoof)")), + content = "", + sig = "00".repeat(64), + ) + assertIs(event) + + assertEquals(null, event.commonName()) + } + @Test fun commonNameHandlesAltWithoutParenthetical() { val event: Event =