45321b6 · opencode-android

git.iko.soy/opencode-android

on master help sign in

Skip redundant snapshots on reconnect, batch password reads, and deduplicate URL normalization

45321b6iko3 months ago

app/src/main/java/soy/iko/opencode/data/network/HttpClientFactory.kt+5−3

compare with master

unifiedsplit

@@ -52,8 +52,10 @@ object HttpClientFactory {

52 requestTimeoutMillis = NetworkConfig.restRequestTimeoutMs

53 }

55+ val normalizedUrl = normalizeBaseUrl(profile.baseUrl)

56+ val isHttps = normalizedUrl.startsWith("https://")

57+

55 if (profile.hasAuth) {

58 if (profile.hasAuth) {

56− val isHttps = normalizeBaseUrl(profile.baseUrl).startsWith("https://")

57 if (isHttps) {

59 if (isHttps) {

58 install(Auth) {

60 install(Auth) {

59 basic {

61 basic {

@@ -80,12 +82,12 @@ object HttpClientFactory {

80 }

82 }

82 defaultRequest {

84 defaultRequest {

83− url.takeFrom(URLBuilder().takeFrom(normalizeBaseUrl(profile.baseUrl)))

85+ url.takeFrom(URLBuilder().takeFrom(normalizedUrl))

84 // For HTTP profiles with auth, attach the Basic auth header proactively on

86 // For HTTP profiles with auth, attach the Basic auth header proactively on

85 // every request (the Auth plugin is skipped for non-HTTPS). The user chose

87 // every request (the Auth plugin is skipped for non-HTTPS). The user chose

86 // cleartext explicitly, so we honor that — but without the reactive challenge

88 // cleartext explicitly, so we honor that — but without the reactive challenge

87 // logic that could re-send credentials silently.

89 // logic that could re-send credentials silently.

88− if (profile.hasAuth && !normalizeBaseUrl(profile.baseUrl).startsWith("https://")) {

90+ if (profile.hasAuth && !isHttps) {

89 val credentials = java.util.Base64.getEncoder().encodeToString(

91 val credentials = java.util.Base64.getEncoder().encodeToString(

90 "${profile.username.orEmpty()}:${profile.password.orEmpty()}".toByteArray(),

92 "${profile.username.orEmpty()}:${profile.password.orEmpty()}".toByteArray(),

91 )

93 )

app/src/main/java/soy/iko/opencode/data/repo/ProfileStore.kt+14−8

@@ -108,16 +108,21 @@ open class ProfileStore private constructor(

108 // Migrate any orphaned plaintext passwords to secure prefs on first load.

109 migrateFallbackPasswords()

110 val json = prefs[profilesKey] ?: return@map emptyList()

111− runCatching {

111+ val stored = runCatching {

112 OpencodeJson.decodeFromString(ListSerializer(StoredProfile.serializer()), json)

113 }.onFailure { Log.w("ProfileStore", "Failed to decode stored profiles, ignoring", it) }

114 .getOrDefault(emptyList())

115 .sortedByDescending { it.lastUsed }

116+ // Batch all password lookups into a single IO dispatch instead of one

117+ // withContext round-trip per profile (N dispatches → 1). DataStore already

118+ // runs this map block on IO, but each passwordFor() call still pays the

119+ // suspend + redispatch overhead.

120+ val pwPrefs = prefsForPasswords()

116− .map { it.toProfile() }

121+ stored.map { it.toProfile(pwPrefs) }

117 } ?: kotlinx.coroutines.flow.flowOf(emptyList())

122 } ?: kotlinx.coroutines.flow.flowOf(emptyList())

118

123

119− private suspend fun StoredProfile.toProfile(): ServerProfile {

124+ private fun StoredProfile.toProfile(pwPrefs: SharedPreferences): ServerProfile {

120− val pw = username?.let { passwordFor(id) }

125+ val pw = username?.let { pwPrefs.getString(passwordKey(id), null) }

121 return ServerProfile(

126 return ServerProfile(

122 id = id,

127 id = id,

123 label = label,

128 label = label,

@@ -129,13 +134,14 @@ open class ProfileStore private constructor(

129 }

134 }

130

135

131 open suspend fun resolve(profile: ServerProfile): ServerProfile {

136 open suspend fun resolve(profile: ServerProfile): ServerProfile {

132− val pw = if (profile.hasAuth) passwordFor(profile.id) else null

137+ val pw = if (profile.hasAuth) {

138+ withContext(Dispatchers.IO) { prefsForPasswords().getString(passwordKey(profile.id), null) }

139+ } else {

140+ null

141+ }

133 return profile.copy(password = pw)

142 return profile.copy(password = pw)

134 }

143 }

135

144

136− private suspend fun passwordFor(id: String): String? =

137− withContext(Dispatchers.IO) { prefsForPasswords().getString(passwordKey(id), null) }

138−

139 open suspend fun save(profile: ServerProfile) {

145 open suspend fun save(profile: ServerProfile) {

140 // Write the password first so that if the DataStore write fails, we're left

146 // Write the password first so that if the DataStore write fails, we're left

141 // with an orphaned password (harmless, cleaned up on next save/delete) rather

147 // with an orphaned password (harmless, cleaned up on next save/delete) rather

app/src/main/java/soy/iko/opencode/data/repo/SessionRepository.kt+21−7

@@ -103,6 +103,7 @@ open class SessionRepository(

103 val fresh = runCatchingCancellable { api.listMessages(sessionId) }

104 .onFailure { Log.w("SessionRepository", "Re-seed message load failed for $sessionId; relying on SSE: ${safeExceptionSummary(it)}") }

105 .getOrDefault(emptyList())

106+ var seedChanged = false

106 lock.withLock {

107 lock.withLock {

107 if (generation == seedGeneration) {

108 if (generation == seedGeneration) {

108 // Merge without pruning: SSE events arriving during the

109 // Merge without pruning: SSE events arriving during the

@@ -111,10 +112,10 @@ open class SessionRepository(

111 // Stale messages from deletions during the disconnect

112 // Stale messages from deletions during the disconnect

112 // gap are eventually removed via MessageRemoved events

113 // gap are eventually removed via MessageRemoved events

113 // or on the next app restart.

114 // or on the next app restart.

114− store.seed(fresh, prune = false)

115+ seedChanged = store.seed(fresh, prune = false)

115 }

116 }

117 }

117− publish()

118+ if (seedChanged) publish()

118 }

119 }

119 hasConnectedBefore = true

120 hasConnectedBefore = true

120 }

121 }

@@ -159,7 +160,8 @@ internal class MessageStore {

159

160

160 fun snapshot(): List<MessageWithParts> = messages.values.toList()

161 fun snapshot(): List<MessageWithParts> = messages.values.toList()

161

162

162− fun seed(initial: List<MessageWithParts>, prune: Boolean = false) {

163+ fun seed(initial: List<MessageWithParts>, prune: Boolean = false): Boolean {

164+ var changed = false

163 // On re-seed (after SSE reconnect), remove messages that are no longer in the

165 // On re-seed (after SSE reconnect), remove messages that are no longer in the

164 // server snapshot (e.g. deleted during the disconnection gap). This keeps the

166 // server snapshot (e.g. deleted during the disconnection gap). This keeps the

165 // in-memory state in sync with the authoritative REST snapshot rather than

167 // in-memory state in sync with the authoritative REST snapshot rather than

@@ -167,13 +169,16 @@ internal class MessageStore {

167 // racing with just-arrived SSE events whose messages may not yet be in REST.

169 // racing with just-arrived SSE events whose messages may not yet be in REST.

168 if (prune) {

170 if (prune) {

169 val incomingIds = initial.mapTo(mutableSetOf()) { it.info.id }

171 val incomingIds = initial.mapTo(mutableSetOf()) { it.info.id }

172+ val before = messages.size

170 messages.keys.retainAll(incomingIds)

173 messages.keys.retainAll(incomingIds)

174+ if (messages.size != before) changed = true

171 }

175 }

172

176

173 for (m in initial) {

177 for (m in initial) {

174 val existing = messages[m.info.id]

178 val existing = messages[m.info.id]

175 if (existing == null) {

179 if (existing == null) {

176 messages[m.info.id] = m

180 messages[m.info.id] = m

181+ changed = true

177 } else {

182 } else {

178 // A part streamed in between subscribe and this initial load may already

183 // A part streamed in between subscribe and this initial load may already

179 // have populated this message (see observeMessages). Merge instead of

184 // have populated this message (see observeMessages). Merge instead of

@@ -187,10 +192,15 @@ internal class MessageStore {

187 if (p.id !in snapshotIds) ordered.add(p)

192 if (p.id !in snapshotIds) ordered.add(p)

188 }

193 }

189 val info = if (existing.info is UnknownMessage) m.info else existing.info

194 val info = if (existing.info is UnknownMessage) m.info else existing.info

195+ val merged = MessageWithParts(info = info, parts = ordered)

196+ if (merged != existing) {

190− messages[m.info.id] = MessageWithParts(info = info, parts = ordered)

197+ messages[m.info.id] = merged

198+ changed = true

199+ }

191 }

200 }

192 }

201 }

193− evictOldMessages()

202+ if (evictOldMessages()) changed = true

203+ return changed

194 }

204 }

195

205

196 /** Returns true if the state changed (and a new snapshot should be published). */

206 /** Returns true if the state changed (and a new snapshot should be published). */

@@ -280,11 +290,15 @@ internal class MessageStore {

280 return true

290 return true

281 }

291 }

282

292

283− /** Evict the oldest messages when the store exceeds [maxMessages], keeping memory bounded. */

293+ /** Evict the oldest messages when the store exceeds [maxMessages], keeping memory bounded.

294+ * Returns true if any messages were evicted. */

284− private fun evictOldMessages() {

295+ private fun evictOldMessages(): Boolean {

296+ var evicted = false

285 while (messages.size > maxMessages) {

297 while (messages.size > maxMessages) {

286 val oldestKey = messages.keys.iterator().next()

298 val oldestKey = messages.keys.iterator().next()

287 messages.remove(oldestKey)

299 messages.remove(oldestKey)

300+ evicted = true

288 }

301 }

302+ return evicted

289 }

303 }

290}

304}

app/src/test/java/soy/iko/opencode/data/repo/MessageStoreTest.kt+48

@@ -249,6 +249,54 @@ class MessageStoreTest {

249 assertEquals(listOf("m1"), store.snapshot().map { it.info.id })

250 }

251

252+ // --- seed return value (skip unnecessary snapshot on reconnect) ---

253+

254+ @Test

255+ fun seedReturnsTrueWhenAddingNewMessages() {

256+ val store = MessageStore()

257+ val changed = store.seed(listOf(MessageWithParts(UserMessage("u1", session))))

258+ assertTrue(changed)

259+ }

260+

261+ @Test

262+ fun seedReturnsFalseWhenReSeedingIdenticalMessages() {

263+ val store = MessageStore()

264+ val messages = listOf(MessageWithParts(UserMessage("u1", session)))

265+ store.seed(messages)

266+ val changed = store.seed(messages)

267+ assertFalse(changed)

268+ }

269+

270+ @Test

271+ fun seedReturnsTrueWhenMessagePartsDiffer() {

272+ val store = MessageStore()

273+ store.seed(listOf(MessageWithParts(UserMessage("u1", session))))

274+ val changed = store.seed(

275+ listOf(MessageWithParts(UserMessage("u1", session), listOf(TextPart(id = "t1")))),

276+ )

277+ assertTrue(changed)

278+ }

279+

280+ @Test

281+ fun seedPruneReturnsTrueWhenMessagesRemoved() {

282+ val store = MessageStore()

283+ store.seed(listOf(

284+ MessageWithParts(UserMessage("u1", session)),

285+ MessageWithParts(AssistantMessage("a1", session)),

286+ ))

287+ val changed = store.seed(listOf(MessageWithParts(UserMessage("u1", session))), prune = true)

288+ assertTrue(changed)

289+ }

290+

291+ @Test

292+ fun seedPruneReturnsFalseWhenNoMessagesRemoved() {

293+ val store = MessageStore()

294+ val messages = listOf(MessageWithParts(UserMessage("u1", session)))

295+ store.seed(messages)

296+ val changed = store.seed(messages, prune = true)

297+ assertFalse(changed)

298+ }

299+

252 // --- seed prune (re-seed after SSE reconnect) ---

300 // --- seed prune (re-seed after SSE reconnect) ---

253

301

254 @Test

302 @Test