laantungir/amethyst
1
0
Fork 0
mirror of https://github.com/vitorpamplona/amethyst.git synced 2026-10-05 19:28:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: audit findings on iOS-readiness migration

Browse Source 
Address bugs and gaps surfaced by an audit of the prior 14 commits.
JVM tests passed because of typealias / platform-type lenience that
won't hold on Native; these are real iOS compile / behavior issues.

BUG fixes (iOS compile failures):

- commons/.../Note.kt:899 — Iterable.sumOf { -> BigDecimal } is a
  JVM-stdlib-only overload. Common stdlib ships sumOf only for
  Int/Long/Double/Float/UInt/ULong. Replaced with fold(BigDecimal(0)).
- commons/.../Note.kt:889 — BigDecimal(it.event?.content): the quartz
  expect-class constructor takes String non-null; JVM accepted nullable
  via platform-type lenience and threw NPE caught downstream. Switched
  to ?.let { content -> BigDecimal(content) }.
- commons/.../Note.kt:838 — `catch (e: java.lang.Exception)` -> `Exception`.
- commons/.../feeds/custom/FeedDefinitionBuilder.kt + FeedBuilderState.kt:
  inline FQN `java.util.UUID.randomUUID().toString()` -> kotlin.uuid.Uuid.
  random().toString() (Kotlin 2.0+, @OptIn ExperimentalUuidApi).
  inline `System.currentTimeMillis() / 1000` -> TimeUtils.now() (already
  used elsewhere in the codebase).
- commons/.../viewmodels/NestViewModelTest.kt: moved from commonTest to
  jvmTest. The test imports NestViewModel + nestsclient, both of which
  the prior PR moved to jvmAndroid. commonTest depends on commonMain
  only, so the test would fail to compile for iosSimulatorArm64Test.

SUBTLE fixes:

- commons/.../UserRelaysCache.kt: the flow field used double-checked
  locking on a non-volatile var. JMM hazard on Native (ARM weak memory
  model) — outer fast-path could observe a partially-published
  WeakReference. Added @kotlin.concurrent.Volatile.
- commons/.../util/UrlValidation.ios.kt: NSURL.URLWithString("http:")
  returns non-null with scheme="http" and no host; JVM's URI.toURL()
  rejects with MalformedURLException. Reject scheme-only network URLs
  (http/https/ws/wss/ftp without a host) to match JVM behavior.
- commons/.../util/KmpLock.kt commonMain doc: corrected "NSLock" ->
  "NSRecursiveLock" to match the actual iOS implementation.

verifyKmpPurity gate extended (commons + quartz):

- Adds patterns: System.currentTimeMillis, Thread.sleep, java.util.UUID,
  kotlin.jvm.Synchronized, kotlin.jvm.Volatile.
- Each pattern paired with a hint pointing at the canonical KMP
  replacement; the error message surfaces both.
- Skips lines that start with //, *, or /* to avoid false positives on
  KDoc / migration notes.
This commit is contained in:
Claude
2026-05-25 02:17:32 +00:00
parent f1845d6a06
commit d1749c314f
9 changed files with 84 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
commons/build.gradle.kts
+23 -6
View File
@@ -212,24 +212,41 @@ val verifyKmpPurity by tasks.registering {
.filter { it.exists() }
inputs.files(checkedDirs)
doLast {
val forbidden = listOf("com.fasterxml.jackson", "okhttp3")
// Each pattern is paired with a short hint so the failure message
// points at the canonical KMP replacement.
val forbidden =
listOf(
"com.fasterxml.jackson" to "Jackson is JVM-only — use kotlinx.serialization",
"okhttp3" to "OkHttp is JVM-only — wrap behind expect/actual or use Ktor on iOS",
"System.currentTimeMillis" to "use TimeUtils.now()",
"Thread.sleep" to "use kotlinx.coroutines.delay or platform-specific actual",
"java.util.UUID" to "use kotlin.uuid.Uuid",
"kotlin.jvm.Synchronized" to "use KmpLock.withLock {}",
"kotlin.jvm.Volatile" to "use kotlin.concurrent.Volatile",
)
val offenders =
checkedDirs.flatMap { dir ->
dir.walkTopDown()
.filter { it.isFile && it.extension == "kt" }
.flatMap { file ->
file.readLines().withIndex().mapNotNull { (idx, line) ->
forbidden.firstOrNull { line.contains(it) }?.let { hit ->
"${file.relativeTo(rootDir)}:${idx + 1}: '$hit'"
val trimmed = line.trimStart()
// Skip KDoc / line-comment lines — those legitimately
// mention forbidden names (migration notes, doc refs).
if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) {
return@mapNotNull null
}
forbidden.firstOrNull { (pattern, _) -> line.contains(pattern) }?.let { (hit, hint) ->
"${file.relativeTo(rootDir)}:${idx + 1}: '$hit' — $hint"
}
}
}
}
if (offenders.isNotEmpty()) {
throw GradleException(
"iOS-targeted source sets must not reference JVM-only deps " +
"(Jackson, OkHttp). Move the offending code to jvmAndroid/ " +
"or behind an expect/actual:\n " + offenders.joinToString("\n "),
"iOS-targeted source sets must not reference JVM-only APIs. " +
"Move the offending code to jvmAndroid/ or behind an expect/actual:\n " +
offenders.joinToString("\n "),
)
}
}
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/feeds/custom/FeedBuilderState.kt
+6 -5
View File
@@ -26,7 +26,10 @@ import androidx.compose.runtime.mutableStateListOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.utils.TimeUtils
import kotlinx.collections.immutable.toImmutableList
import kotlin.uuid.ExperimentalUuidApi
import kotlin.uuid.Uuid
@Stable
class FeedBuilderState(
@@ -66,6 +69,7 @@ class FeedBuilderState(
private val editId: String? = initial?.id
@OptIn(ExperimentalUuidApi::class)
fun toDefinition(): FeedDefinition {
val source =
FeedSource.Filter(
@@ -77,17 +81,14 @@ class FeedBuilderState(
kinds = kinds.toImmutableList(),
)
return FeedDefinition(
id =
editId ?: java.util.UUID
.randomUUID()
.toString(),
id = editId ?: Uuid.random().toString(),
name = name,
emoji = emoji,
pinned = false,
pinOrder = Int.MAX_VALUE,
source = source,
refreshMode = refreshMode,
createdAt = System.currentTimeMillis() / 1000,
createdAt = TimeUtils.now(),
)
}
}
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/feeds/custom/FeedDefinitionBuilder.kt
+6 -5
View File
@@ -21,7 +21,10 @@
package com.vitorpamplona.amethyst.commons.feeds.custom
import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.utils.TimeUtils
import kotlinx.collections.immutable.toImmutableList
import kotlin.uuid.ExperimentalUuidApi
import kotlin.uuid.Uuid
class FeedDefinitionBuilder {
var name: String = ""
@@ -70,13 +73,11 @@ class FeedDefinitionBuilder {
pinOrder = Int.MAX_VALUE,
source = source ?: error("FeedDefinition requires a source"),
refreshMode = refreshMode,
createdAt = System.currentTimeMillis() / 1000,
createdAt = TimeUtils.now(),
)
private fun generateId(): String =
java.util.UUID
.randomUUID()
.toString()
@OptIn(ExperimentalUuidApi::class)
private fun generateId(): String = Uuid.random().toString()
}
class FilterBuilder {
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt
+9 -3
View File
@@ -835,7 +835,7 @@ open class Note(
val amount =
try {
LnInvoiceUtil.getAmountInSats(invoice)
} catch (e: java.lang.Exception) {
} catch (e: Exception) {
if (e is CancellationException) throw e
null
}
@@ -886,7 +886,7 @@ open class Note(
.any {
val pledgeValue =
try {
BigDecimal(it.event?.content)
it.event?.content?.let { content -> BigDecimal(content) }
} catch (e: Exception) {
if (e is CancellationException) throw e
null
@@ -896,7 +896,13 @@ open class Note(
pledgeValue != null && it.author == user
}
fun pledgedAmountByOthers(): BigDecimal = replies.sumOf { it.event?.addedRewardValue() ?: BigDecimal(0) }
// Manual fold rather than Iterable.sumOf { -> BigDecimal } because that
// overload is JVM-only; the common stdlib only ships sumOf for the
// primitive numeric types.
fun pledgedAmountByOthers(): BigDecimal =
replies.fold(BigDecimal(0)) { acc, note ->
acc + (note.event?.addedRewardValue() ?: BigDecimal(0))
}
fun hasAnyReports(): Boolean {
val dayAgo = TimeUtils.oneDayAgo()
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip01Core/UserRelaysCache.kt
+6
View File
@@ -27,6 +27,7 @@ import com.vitorpamplona.amethyst.commons.util.withLock
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.isLocalHost
import kotlinx.coroutines.flow.MutableStateFlow
import kotlin.concurrent.Volatile
@Stable
data class RelayInfo(
@@ -54,6 +55,11 @@ val DefaultOrder =
@Stable
class UserRelaysCache {
var data: Map<NormalizedRelayUrl, RelayInfo> = mapOf()
// @Volatile is required for the double-checked locking in flow() to be
// safe on weak-memory platforms (K/N + ARM). Without it, the outer
// fast-path read could observe a partially-published WeakReference.
@Volatile
private var flow: WeakReference<MutableStateFlow<Wrapper>>? = null
private val flowLock = KmpLock()
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/util/KmpLock.kt
+1 -1
View File
@@ -22,7 +22,7 @@ package com.vitorpamplona.amethyst.commons.util
/**
* KMP-friendly reentrant lock. JVM/Android map to `java.util.concurrent.locks.ReentrantLock`;
* iOS will map to `NSLock` (or a thin wrapper around it) when the target is added.
* iOS maps to `NSRecursiveLock`.
*
* Use [withLock] in preference to manual lock/unlock — it guarantees release on
* exception. The class is reentrant on every platform, so a thread that already
commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/util/UrlValidation.ios.kt
+12 -3
View File
@@ -22,12 +22,21 @@ package com.vitorpamplona.amethyst.commons.util
import platform.Foundation.NSURL
// Schemes that JVM's URI.toURL() requires a host for (network schemes).
// "http:" without a host throws MalformedURLException on JVM; NSURL accepts
// it. Explicitly reject to keep behavior aligned.
private val HOST_REQUIRED_SCHEMES = setOf("http", "https", "ws", "wss", "ftp")
actual fun isValidUrl(url: String?): Boolean {
if (url == null) return false
// NSURL.URLWithString returns null for syntactically invalid URLs. It is
// more permissive than JVM's URI(url).toURL() — it accepts scheme-less
// relative URLs ("foo", "/bar"). Match the JVM contract (absolute URL
// with a known scheme) by also requiring a non-empty scheme.
// relative URLs ("foo", "/bar") and scheme-only inputs ("http:"). Match
// the JVM contract (absolute URL with a known scheme and, for network
// schemes, a host).
val nsUrl = NSURL.URLWithString(url) ?: return false
return !nsUrl.scheme.isNullOrEmpty()
val scheme = nsUrl.scheme?.lowercase() ?: return false
if (scheme.isEmpty()) return false
if (scheme in HOST_REQUIRED_SCHEMES && nsUrl.host.isNullOrEmpty()) return false
return true
}
commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModelTest.kt → commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModelTest.kt
View File
quartz/build.gradle.kts
+21 -6
View File
@@ -386,24 +386,39 @@ val verifyKmpPurity by tasks.registering {
.filter { it.exists() }
inputs.files(checkedDirs)
doLast {
val forbidden = listOf("com.fasterxml.jackson", "okhttp3")
// Each pattern is paired with a short hint so the failure message
// points at the canonical KMP replacement.
val forbidden =
listOf(
"com.fasterxml.jackson" to "Jackson is JVM-only — use kotlinx.serialization",
"okhttp3" to "OkHttp is JVM-only — wrap behind expect/actual or use Ktor on iOS",
"System.currentTimeMillis" to "use TimeUtils.now()",
"Thread.sleep" to "use kotlinx.coroutines.delay or platform-specific actual",
"java.util.UUID" to "use kotlin.uuid.Uuid",
"kotlin.jvm.Synchronized" to "use a KMP lock primitive",
"kotlin.jvm.Volatile" to "use kotlin.concurrent.Volatile",
)
val offenders =
checkedDirs.flatMap { dir ->
dir.walkTopDown()
.filter { it.isFile && it.extension == "kt" }
.flatMap { file ->
file.readLines().withIndex().mapNotNull { (idx, line) ->
forbidden.firstOrNull { line.contains(it) }?.let { hit ->
"${file.relativeTo(rootDir)}:${idx + 1}: '$hit'"
val trimmed = line.trimStart()
if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) {
return@mapNotNull null
}
forbidden.firstOrNull { (pattern, _) -> line.contains(pattern) }?.let { (hit, hint) ->
"${file.relativeTo(rootDir)}:${idx + 1}: '$hit' — $hint"
}
}
}
}
if (offenders.isNotEmpty()) {
throw GradleException(
"iOS-targeted source sets must not reference JVM-only deps " +
"(Jackson, OkHttp). Move the offending code to jvmAndroid/ " +
"or behind an expect/actual:\n " + offenders.joinToString("\n "),
"iOS-targeted source sets must not reference JVM-only APIs. " +
"Move the offending code to jvmAndroid/ or behind an expect/actual:\n " +
offenders.joinToString("\n "),
)
}
}