Skip to content

Commit afad69e

Browse files
committed
Wire subscriptions: fetch, reconcile, and say what happened
SubscriptionFetcher was written and tested and nothing called it, which is why the node list passed subscription = null. It is connected now, and the connection is where NW-D-04 actually lives. NodeStore learned where a node came from, because a feed may only speak for the nodes it supplied. A refresh leaves manual nodes untouched; a node the feed stops listing is kept and marked rather than deleted, since deleting it leaves a shorter list and no explanation, which is the outcome the requirement exists to prevent; and a subscription that comes back restores the node in place instead of appending a second copy. A line with no origin marker reads as manual, which is both correct and the whole migration — every node written before origins existed was one the user added by hand. The subscription URL is a bearer credential and is stored accordingly: its own file, so a future "export my nodes" cannot sweep it up; written before the first fetch is attempted, so a correct but unreachable subscription is still the subscription when the network returns; and forgetting it deletes the file rather than blanking a field. Import takes one field for both kinds of link, because a person holding a link does not necessarily know which kind it is, and the parser can answer that better than they can. Three defects a device found: A fetch launched from rememberCoroutineScope() is cancelled when the screen closes — which is the same tap. The import screen closed, the list stayed empty, and nothing reported a failure because nothing had failed: it had been cancelled. Work that outlives a screen now runs on the repository's own scope, and subscribe() takes no scope from its caller because there is no way to hand it one that dies. Cleartext is blocked at this target SDK, correctly — the URL is a password. The screen warned about http and left the button enabled, so the tap silently did nothing at all, which is worse than either refusing or allowing. A plaintext subscription is now refused at the input, with the reason and what to ask the provider for. lastRefreshFailure was kept in the repository and shown nowhere, so a subscription that could not be reached looked exactly like one with nothing to say. It is on the node list now — and a subscription failure renders as a subscription failure rather than borrowing a node's wording, so an exhausted subscription no longer reads as an unreachable node. 321 unit tests, 0 failures, 0 skipped. Verified on a device: the http refusal, and the whole import path.
1 parent 071a080 commit afad69e

13 files changed

Lines changed: 631 additions & 40 deletions

File tree

‎app/build.gradle.kts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ dependencies {
112112
implementation(libs.conscrypt.android)
113113

114114
testImplementation(libs.junit)
115+
// Needed by the tests that drive the repository, which is suspend-shaped
116+
// because a subscription fetch is a network call.
117+
testImplementation(libs.coroutines.test)
115118
// The JVM build of the same library, so the exporter can be checked against
116119
// a real TLS handshake on the host rather than only on a device.
117120
testImplementation(libs.conscrypt.openjdk)

‎app/src/main/kotlin/eu/nodepass/somewhere/SomewhereApplication.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import eu.nodepass.somewhere.data.NodeRepository
88
import eu.nodepass.somewhere.data.NodeStore
99
import eu.nodepass.somewhere.data.SubscriptionStore
1010
import eu.nodepass.somewhere.subscription.SubscriptionFetcher
11+
import kotlinx.coroutines.CoroutineScope
1112
import kotlinx.coroutines.Dispatchers
13+
import kotlinx.coroutines.SupervisorJob
1214
import java.io.File
1315

1416
/**
@@ -28,6 +30,9 @@ class SomewhereApplication : Application() {
2830
subscriptions = SubscriptionStore(File(filesDir, "subscription/subscription.txt")),
2931
fetcher = SubscriptionFetcher(clientVersion = BuildConfig.VERSION_NAME),
3032
io = Dispatchers.IO,
33+
// Application-lifetime, and supervised so one failed fetch does not
34+
// take the scope down with it.
35+
scope = CoroutineScope(SupervisorJob() + Dispatchers.Default),
3136
)
3237
}
3338
}

‎app/src/main/kotlin/eu/nodepass/somewhere/data/NodeRepository.kt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ import eu.nodepass.somewhere.subscription.Subscription
1111
import eu.nodepass.somewhere.subscription.SubscriptionFetcher
1212
import eu.nodepass.somewhere.subscription.SubscriptionReason
1313
import kotlinx.coroutines.CoroutineDispatcher
14+
import kotlinx.coroutines.CoroutineScope
1415
import kotlinx.coroutines.flow.MutableStateFlow
1516
import kotlinx.coroutines.flow.StateFlow
1617
import kotlinx.coroutines.flow.asStateFlow
1718
import kotlinx.coroutines.flow.update
19+
import kotlinx.coroutines.launch
1820
import kotlinx.coroutines.withContext
1921

2022
/**
@@ -55,6 +57,17 @@ class NodeRepository(
5557
private val fetcher: SubscriptionFetcher,
5658
private val dialer: NowhereDialer = NowhereDialer(),
5759
private val io: CoroutineDispatcher,
60+
/**
61+
* Where work that outlives a screen runs.
62+
*
63+
* A subscription fetch must not be launched from `rememberCoroutineScope()`:
64+
* that scope belongs to the composition, and the import screen closes the
65+
* instant the user taps Subscribe. The first version did exactly that, and
66+
* the fetch was cancelled before it reached the network — the screen closed,
67+
* the node list stayed empty, and nothing anywhere reported a failure,
68+
* because nothing had failed. It had been cancelled.
69+
*/
70+
private val scope: CoroutineScope,
5871
private val clock: () -> Long = System::currentTimeMillis,
5972
) {
6073
private val _nodes = MutableStateFlow(store.load())
@@ -81,6 +94,21 @@ class NodeRepository(
8194
_subscription.value = subscriptions.load()
8295
}
8396

97+
/** Fire-and-forget [setSubscription], for a caller that is about to go away. */
98+
fun subscribe(url: String) {
99+
scope.launch { setSubscription(url) }
100+
}
101+
102+
/** Fire-and-forget [refreshSubscription]. */
103+
fun refreshInBackground() {
104+
scope.launch { refreshSubscription() }
105+
}
106+
107+
/** Fire-and-forget [probe], for the same reason. */
108+
fun probeInBackground(node: NowhereUrl) {
109+
scope.launch { probe(node) }
110+
}
111+
84112
/**
85113
* Points the app at a subscription and fetches it once.
86114
*

‎app/src/main/kotlin/eu/nodepass/somewhere/ui/screens/ImportScreen.kt‎

Lines changed: 116 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import androidx.compose.foundation.layout.height
1919
import androidx.compose.foundation.layout.padding
2020
import androidx.compose.foundation.layout.size
2121
import androidx.compose.foundation.shape.RoundedCornerShape
22+
import androidx.compose.foundation.text.BasicTextField
2223
import androidx.compose.material3.Icon
2324
import androidx.compose.material3.Text
2425
import androidx.compose.runtime.Composable
@@ -29,10 +30,12 @@ import androidx.compose.runtime.setValue
2930
import androidx.compose.ui.Alignment
3031
import androidx.compose.ui.Modifier
3132
import androidx.compose.ui.draw.clip
33+
import androidx.compose.ui.graphics.SolidColor
3234
import androidx.compose.ui.graphics.vector.ImageVector
3335
import androidx.compose.ui.res.stringResource
3436
import androidx.compose.ui.text.AnnotatedString
3537
import androidx.compose.ui.text.SpanStyle
38+
import androidx.compose.ui.text.TextStyle
3639
import androidx.compose.ui.text.buildAnnotatedString
3740
import androidx.compose.ui.text.font.FontWeight
3841
import androidx.compose.ui.unit.dp
@@ -68,16 +71,20 @@ fun ImportScreen(
6871
) {
6972
val colors = SomewhereTheme.colors
7073
var acknowledged by remember { mutableStateOf(false) }
74+
var typed by remember(link) { mutableStateOf(link.orEmpty()) }
7175

72-
// The link is parsed once, by the real parser, and whatever it says is what
73-
// the screen shows. A link that arrived through the deep link and one the
74-
// user pasted go down the same path — there is no second, more forgiving
75-
// reader for text the system handed us.
76-
val parsed =
77-
remember(link) {
78-
link?.let { NowhereUrl.parse(it) }
79-
}
76+
// One field, two outcomes, and the parser decides which. A person holding a
77+
// link does not necessarily know whether it is a node or a subscription —
78+
// asking them to pick the right box first is asking them to answer a
79+
// question the app can answer itself.
80+
//
81+
// A link that arrived through the deep link and one the user pasted go down
82+
// exactly the same path. There is no second, more forgiving reader for text
83+
// the system handed us.
84+
val parsed = remember(typed) { typed.trim().takeIf { it.isNotEmpty() }?.let { NowhereUrl.parse(it) } }
8085
val node = (parsed as? DecodeResult.Ok)?.value
86+
val subscriptionUrl = remember(typed) { typed.trim().takeIf { it.looksLikeSubscription() } }
87+
val plaintext = subscriptionUrl?.startsWith("http://", ignoreCase = true) == true
8188
val verified = node?.certificateVerification?.isVerified ?: true
8289

8390
Column(Modifier.fillMaxSize()) {
@@ -124,20 +131,83 @@ fun ImportScreen(
124131
RoundedCornerShape(10.dp),
125132
).padding(horizontal = 14.dp, vertical = 13.dp),
126133
) {
127-
Text(
128-
text = if (node != null) maskedUrl(node) else AnnotatedString(link.orEmpty()),
129-
fontFamily = SomewhereType.Mono,
130-
fontSize = 12.sp,
131-
lineHeight = 19.2.sp,
132-
color = colors.inkMuted,
133-
)
134+
if (node != null) {
135+
// Once it parses, the key is masked. The field stops
136+
// being an input and becomes a statement of what was
137+
// understood — and a shared key must not sit on screen
138+
// waiting to be screenshotted.
139+
Text(
140+
text = maskedUrl(node),
141+
fontFamily = SomewhereType.Mono,
142+
fontSize = 12.sp,
143+
lineHeight = 19.2.sp,
144+
color = colors.inkMuted,
145+
)
146+
} else {
147+
BasicTextField(
148+
value = typed,
149+
onValueChange = { typed = it },
150+
textStyle =
151+
TextStyle(
152+
fontFamily = SomewhereType.Mono,
153+
fontSize = 12.sp,
154+
lineHeight = 19.2.sp,
155+
color = colors.inkMuted,
156+
),
157+
cursorBrush = SolidColor(colors.upstream),
158+
modifier = Modifier.fillMaxWidth(),
159+
decorationBox = { inner ->
160+
if (typed.isEmpty()) {
161+
Text(
162+
text = stringResource(R.string.import_paste_hint),
163+
fontFamily = SomewhereType.Mono,
164+
fontSize = 12.sp,
165+
color = colors.faint,
166+
)
167+
}
168+
inner()
169+
},
170+
)
171+
}
134172
}
135173

136174
// The parser's own reason, verbatim. It distinguishes a wrong
137175
// scheme from a bad port from a key with a password component,
138176
// and a screen that replaced all three with "invalid link"
139177
// would be throwing away the only useful part.
140-
(parsed as? DecodeResult.Invalid)?.let { failure ->
178+
if (subscriptionUrl != null) {
179+
Text(
180+
text = stringResource(R.string.import_subscription_detected),
181+
fontFamily = SomewhereType.Body,
182+
fontSize = 12.5.sp,
183+
lineHeight = 18.sp,
184+
color = colors.inkMuted,
185+
)
186+
if (plaintext) {
187+
// Refused, not warned about. The URL is the credential:
188+
// over http it is handed to every device between here
189+
// and the dashboard, and the platform blocks the request
190+
// anyway on this target SDK. Warning and then leaving
191+
// the button enabled produced the worst outcome of the
192+
// three — a tap that silently did nothing at all.
193+
Text(
194+
text = stringResource(R.string.import_subscription_plaintext),
195+
fontFamily = SomewhereType.Body,
196+
fontSize = 12.sp,
197+
lineHeight = 17.sp,
198+
color = colors.critical,
199+
)
200+
Text(
201+
text = stringResource(R.string.import_subscription_needs_https),
202+
fontFamily = SomewhereType.Body,
203+
fontSize = 12.sp,
204+
lineHeight = 17.sp,
205+
color = colors.inkMuted,
206+
)
207+
}
208+
}
209+
210+
(parsed as? DecodeResult.Invalid)?.takeIf { subscriptionUrl == null }?.let { failure ->
141211
Text(
142212
text = stringResource(R.string.import_invalid),
143213
fontFamily = SomewhereType.Body,
@@ -234,7 +304,7 @@ fun ImportScreen(
234304
// Disabled until the certificate statement has been acknowledged. The
235305
// affordance stays visible rather than hidden, so the reason the button
236306
// does nothing is on the screen next to it.
237-
val enabled = node != null && (verified || acknowledged)
307+
val enabled = (node != null && (verified || acknowledged)) || (subscriptionUrl != null && !plaintext)
238308
Box(
239309
Modifier
240310
.padding(start = 20.dp, end = 20.dp, bottom = 24.dp)
@@ -243,13 +313,29 @@ fun ImportScreen(
243313
.clip(RoundedCornerShape(12.dp))
244314
.background(if (enabled) colors.primaryAction else colors.surfaceAlt)
245315
.clickable(enabled = enabled) {
246-
node?.let(nodes::add)
247-
onClose()
316+
when {
317+
node != null -> {
318+
nodes.add(node)
319+
onClose()
320+
}
321+
322+
subscriptionUrl != null -> {
323+
// Fire and leave, on the repository's scope rather
324+
// than the composition's. This screen is about to
325+
// close, and a fetch launched from a scope that
326+
// closes with it is a fetch that never happens.
327+
nodes.subscribe(subscriptionUrl)
328+
onClose()
329+
}
330+
}
248331
},
249332
contentAlignment = Alignment.Center,
250333
) {
251334
Text(
252-
text = stringResource(R.string.import_add_node),
335+
text =
336+
stringResource(
337+
if (subscriptionUrl != null) R.string.import_subscribe else R.string.import_add_node,
338+
),
253339
fontFamily = SomewhereType.Display,
254340
fontWeight = FontWeight.SemiBold,
255341
fontSize = 15.sp,
@@ -259,6 +345,16 @@ fun ImportScreen(
259345
}
260346
}
261347

348+
/**
349+
* Anything `http`/`https` is treated as a subscription.
350+
*
351+
* Deliberately shallow: the real validation is `SubscriptionEndpoint.prepare`,
352+
* which runs on fetch and has its own reasons for refusing. Duplicating that
353+
* judgement here would give two definitions of a valid subscription URL that
354+
* can disagree, and the one the user sees would be the weaker of the two.
355+
*/
356+
private fun String.looksLikeSubscription(): Boolean = startsWith("http://", ignoreCase = true) || startsWith("https://", ignoreCase = true)
357+
262358
@Composable
263359
private fun RowLabel(
264360
text: String,

0 commit comments

Comments
 (0)