Repository navigation
ADFA-5646: Hide the discover-plugins download arrow while offline #2083
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| package com.itsaky.androidide.viewmodels | ||
|
|
||
| import com.google.common.truth.Truth.assertThat | ||
| import com.itsaky.androidide.repositories.PluginRepository | ||
| import com.itsaky.androidide.utils.ConnectivityObserver | ||
| import com.itsaky.androidide.viewmodel.MainDispatcherRule | ||
| import io.mockk.every | ||
| import io.mockk.mockk | ||
| import kotlinx.coroutines.ExperimentalCoroutinesApi | ||
| import kotlinx.coroutines.flow.MutableStateFlow | ||
| import kotlinx.coroutines.test.advanceUntilIdle | ||
| import kotlinx.coroutines.test.runTest | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
| import org.junit.runners.JUnit4 | ||
| import java.io.File | ||
|
|
||
| /** A fake [ConnectivityObserver] whose emissions are driven by [state], for tests that need to simulate a change over time. */ | ||
| private class FakeConnectivityObserver( | ||
| initialValue: Boolean, | ||
| ) : ConnectivityObserver { | ||
| val state = MutableStateFlow(initialValue) | ||
|
|
||
| override fun observe() = state | ||
| } | ||
|
|
||
| /** | ||
| * Covers only [PluginManagerViewModel.uiState]'s `isOnline` field (ADFA-5646) - the rest of this | ||
| * ViewModel has no test coverage yet, which is out of scope here. | ||
| */ | ||
| @RunWith(JUnit4::class) | ||
| @OptIn(ExperimentalCoroutinesApi::class) | ||
| class PluginManagerViewModelTest { | ||
| @get:Rule | ||
| val mainDispatcherRule = MainDispatcherRule() | ||
|
|
||
| private val pluginRepository = | ||
| mockk<PluginRepository> { | ||
| // Keeps init{}'s loadPlugins() a no-op past this check, so the test doesn't need to | ||
| // stub the rest of the load path - irrelevant to what's under test here. | ||
| every { isPluginManagerAvailable() } returns false | ||
| } | ||
|
|
||
| private fun viewModel(connectivityObserver: ConnectivityObserver) = | ||
| PluginManagerViewModel( | ||
| pluginRepository = pluginRepository, | ||
| contentResolver = mockk(relaxed = true), | ||
| filesDir = File("/tmp"), | ||
| connectivityObserver = connectivityObserver, | ||
| ) | ||
|
|
||
| @Test | ||
| fun uiState_isOnline_reflectsConnectivityObserverOnInit() = | ||
| runTest { | ||
| val viewModel = viewModel(FakeConnectivityObserver(initialValue = false)) | ||
| advanceUntilIdle() | ||
|
|
||
| assertThat(viewModel.uiState.value.isOnline).isFalse() | ||
| } | ||
|
|
||
| @Test | ||
| fun uiState_isOnline_updatesWhenConnectivityChanges() = | ||
| runTest { | ||
| val connectivityObserver = FakeConnectivityObserver(initialValue = true) | ||
| val viewModel = viewModel(connectivityObserver) | ||
| advanceUntilIdle() | ||
| assertThat(viewModel.uiState.value.isOnline).isTrue() | ||
|
|
||
| connectivityObserver.state.value = false | ||
| advanceUntilIdle() | ||
|
|
||
| assertThat(viewModel.uiState.value.isOnline).isFalse() | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| package com.itsaky.androidide.utils | ||
|
|
||
| import android.content.Context | ||
| import android.net.ConnectivityManager | ||
| import android.net.Network | ||
| import android.net.NetworkCapabilities | ||
| import android.net.NetworkRequest | ||
| import kotlinx.coroutines.channels.awaitClose | ||
| import kotlinx.coroutines.flow.Flow | ||
| import kotlinx.coroutines.flow.callbackFlow | ||
| import kotlinx.coroutines.flow.distinctUntilChanged | ||
|
|
||
| /** | ||
| * Live internet-connectivity state, for UI that must react to connectivity changes rather than | ||
| * check it once - e.g. hiding an action that requires the network before the user taps it and | ||
| * hits a dead end, instead of only refusing the tap after the fact. | ||
| */ | ||
| interface ConnectivityObserver { | ||
| /** Emits the current state immediately upon collection, then again on every change. */ | ||
| fun observe(): Flow<Boolean> | ||
| } | ||
|
|
||
| /** | ||
| * [ConnectivityManager]-backed [ConnectivityObserver]. [Context.isNetworkConnected] answers "is | ||
| * the active network capable of internet right now"; this wraps that same check in a | ||
| * [NetworkCallback] so callers get updates instead of having to poll. | ||
| */ | ||
| class AndroidConnectivityObserver( | ||
| private val context: Context, | ||
| ) : ConnectivityObserver { | ||
| override fun observe(): Flow<Boolean> = | ||
| callbackFlow { | ||
| val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager | ||
| if (connectivityManager == null) { | ||
| trySend(false) | ||
| close() | ||
| return@callbackFlow | ||
| } | ||
|
|
||
| // Re-derives from the active network on every callback rather than trusting | ||
| // onAvailable/onLost's own network in isolation: onLost fires for the network that | ||
| // was lost, not the device's overall state, so switching Wi-Fi -> cellular must not | ||
| // be reported as "offline" just because the Wi-Fi network specifically went away. | ||
| val callback = | ||
| object : ConnectivityManager.NetworkCallback() { | ||
| override fun onAvailable(network: Network) { | ||
| trySend(context.isNetworkConnected()) | ||
| } | ||
|
|
||
| override fun onLost(network: Network) { | ||
|
Collaborator
There was a problem hiding this comment. @jimturner-adfa IMPORTANT: Android doesn't guarantee Fix: track the networks the callback has reported in a set, remove |
||
| trySend(context.isNetworkConnected()) | ||
| } | ||
|
|
||
| override fun onCapabilitiesChanged( | ||
| network: Network, | ||
| networkCapabilities: NetworkCapabilities, | ||
| ) { | ||
| trySend(context.isNetworkConnected()) | ||
|
Collaborator
There was a problem hiding this comment. @jimturner-adfa MINOR: the defect is at INTERNET means the network claims to offer internet. It doesn't mean Android confirmed the connection works. Behind a captive portal (a Wi-Fi login page), or on Wi-Fi with no working uplink, the arrow stays visible and tapping it hits the dead end ADFA-5646 targets. This check existed before the PR; graded MINOR because the PR doesn't make it worse, but the arrow now relies on it. Fix: also require |
||
| } | ||
| } | ||
|
|
||
| trySend(context.isNetworkConnected()) | ||
|
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Register before relying on the initial connectivity sample. If the last network disconnects after 🤖 Prompt for AI Agents |
||
|
|
||
| val request = | ||
| NetworkRequest | ||
| .Builder() | ||
| .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) | ||
| .build() | ||
| connectivityManager.registerNetworkCallback(request, callback) | ||
|
|
||
| awaitClose { connectivityManager.unregisterNetworkCallback(callback) } | ||
| }.distinctUntilChanged() | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 1986
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 6239
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 6239
Derive callback state from delivered network data.
All three callbacks call
context.isNetworkConnected(). That helper performs synchronousConnectivityManagerqueries. Android warns that these queries can race withNetworkCallbackdelivery, so the flow can emit a stale Boolean.distinctUntilChanged()does not correct that state.Track callback-delivered networks and capabilities in callback order, then emit the resulting internet state. Do not use
onLost'sNetworkalone because another network may already be available.🤖 Prompt for AI Agents