Check also availability of internet access when using online storage

This moves these availability checks into the Storage class, so they can be used in various places without duplicating code.

app/src/main/java/com/stevesoltys/seedvault/settings/SettingsFragment.kt

@@ -151,6 +151,7 @@ class SettingsFragment : PreferenceFragmentCompat() {
         setAutoRestoreState()
         lifecycleScope.launch { setMenuItemStates() }
 
+        // TODO we should also monitor network changes, if storage requires network
         if (storage?.isUsb == true) context?.registerReceiver(usbReceiver, usbFilter)
     }
 
@@ -233,7 +234,7 @@ class SettingsFragment : PreferenceFragmentCompat() {
 
     private suspend fun storageAvailable(storage: Storage) = withContext(Dispatchers.IO) {
         val context = context ?: return@withContext false
-        (!storage.isUsb || storage.getDocumentFile(context).isDirectory)
+        !storage.isUnavailableUsb(context) && !storage.isUnavailableNetwork(context)
     }
 
 }

app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt

@@ -2,8 +2,11 @@ package com.stevesoltys.seedvault.settings
 
 import android.content.Context
 import android.hardware.usb.UsbDevice
+import android.net.ConnectivityManager
+import android.net.NetworkCapabilities
 import android.net.Uri
 import androidx.annotation.UiThread
+import androidx.annotation.WorkerThread
 import androidx.documentfile.provider.DocumentFile
 import androidx.preference.PreferenceManager
 import com.stevesoltys.seedvault.permitDiskReads
@@ -127,6 +130,30 @@ data class Storage(
 ) {
     fun getDocumentFile(context: Context) = DocumentFile.fromTreeUri(context, uri)
         ?: throw AssertionError("Should only happen on API < 21.")
+
+    /**
+     * Returns true if this is USB storage that is not available, false otherwise.
+     *
+     * Must be run off UI thread (ideally I/O).
+     */
+    @WorkerThread
+    fun isUnavailableUsb(context: Context): Boolean {
+        return isUsb && !getDocumentFile(context).isDirectory
+    }
+
+    /**
+     * Returns true if this is storage that requires network access,
+     * but it isn't available right now.
+     */
+    fun isUnavailableNetwork(context: Context): Boolean {
+        return requiresNetwork && !hasInternet(context)
+    }
+
+    private fun hasInternet(context: Context): Boolean {
+        val cm = context.getSystemService(ConnectivityManager::class.java)
+        val capabilities = cm.getNetworkCapabilities(cm.activeNetwork) ?: return false
+        return capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+    }
 }
 
 data class FlashDrive(

app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt

@@ -14,8 +14,6 @@ import android.app.backup.RestoreSet
 import android.content.Context
 import android.content.pm.PackageInfo
 import android.content.pm.PackageManager.GET_SIGNING_CERTIFICATES
-import android.net.ConnectivityManager
-import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET
 import android.os.ParcelFileDescriptor
 import android.util.Log
 import androidx.annotation.VisibleForTesting
@@ -431,24 +429,18 @@ internal class BackupCoordinator(
     }
 
     private fun getBackupBackoff(): Long {
-        val noBackoff = 0L
         val longBackoff = DAYS.toMillis(30)
 
         // back off if there's no storage set
         val storage = settingsManager.getStorage() ?: return longBackoff
-
+        return when {
-        // back off if storage is removable and not available right now
+            // back off if storage is removable and not available right now
-        return if (storage.isUsb && !storage.getDocumentFile(context).isDirectory) longBackoff
+            storage.isUnavailableUsb(context) -> longBackoff
-        // back off if storage is on network, but we have no access
+            // back off if storage is on network, but we have no access
-        else if (storage.requiresNetwork && !hasInternet()) HOURS.toMillis(1)
+            storage.isUnavailableNetwork(context) -> HOURS.toMillis(1)
-        // otherwise no back off
+            // otherwise no back off
-        else noBackoff
+            else -> 0L
-    }
+        }
-
-    private fun hasInternet(): Boolean {
-        val cm = context.getSystemService(ConnectivityManager::class.java)
-        val capabilities = cm.getNetworkCapabilities(cm.activeNetwork) ?: return false
-        return capabilities.hasCapability(NET_CAPABILITY_INTERNET)
     }
 
 }

app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt

@@ -283,7 +283,7 @@ internal class RestoreCoordinator(
     // TODO this is plugin specific, needs to be factored out when supporting different plugins
     private fun isStorageRemovableAndNotAvailable(): Boolean {
         val storage = settingsManager.getStorage() ?: return false
-        return storage.isUsb && !storage.getDocumentFile(context).isDirectory
+        return storage.isUnavailableUsb(context)
     }
 
 }

app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt

@@ -9,7 +9,6 @@ import android.content.pm.ApplicationInfo.FLAG_STOPPED
 import android.content.pm.PackageInfo
 import android.net.Uri
 import android.os.ParcelFileDescriptor
-import androidx.documentfile.provider.DocumentFile
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.coAssertThrows
 import com.stevesoltys.seedvault.getRandomString
@@ -63,7 +62,10 @@ internal class BackupCoordinatorTest : BackupTest() {
     private val metadataOutputStream = mockk<OutputStream>()
     private val fileDescriptor: ParcelFileDescriptor = mockk()
     private val packageMetadata: PackageMetadata = mockk()
-    private val storage = Storage(Uri.EMPTY, getRandomString(), false, false)
+    private val storage = Storage(Uri.EMPTY, getRandomString(),
+        isUsb = false,
+        requiresNetwork = false
+    )
 
     @Test
     fun `starting a new restore set works as expected`() = runBlocking {
@@ -121,14 +123,11 @@ internal class BackupCoordinatorTest : BackupTest() {
     fun `no error notification when device initialization fails on unplugged USB storage`() =
         runBlocking {
             val storage = mockk<Storage>()
-            val documentFile = mockk<DocumentFile>()
 
             every { settingsManager.getToken() } returns token
             coEvery { plugin.initializeDevice() } throws IOException()
             every { settingsManager.getStorage() } returns storage
-            every { storage.isUsb } returns true
+            every { storage.isUnavailableUsb(context) } returns true
-            every { storage.getDocumentFile(context) } returns documentFile
-            every { documentFile.isDirectory } returns false
 
             assertEquals(TRANSPORT_ERROR, backup.initializeDevice())
 

app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt

@@ -8,7 +8,6 @@ import android.app.backup.RestoreDescription.TYPE_FULL_STREAM
 import android.app.backup.RestoreDescription.TYPE_KEY_VALUE
 import android.content.pm.PackageInfo
 import android.os.ParcelFileDescriptor
-import androidx.documentfile.provider.DocumentFile
 import com.stevesoltys.seedvault.coAssertThrows
 import com.stevesoltys.seedvault.getRandomString
 import com.stevesoltys.seedvault.metadata.BackupMetadata
@@ -57,7 +56,6 @@ internal class RestoreCoordinatorTest : TransportTest() {
 
     private val inputStream = mockk<InputStream>()
     private val storage: Storage = mockk()
-    private val documentFile: DocumentFile = mockk()
     private val packageInfo2 = PackageInfo().apply { packageName = "org.example2" }
     private val packageInfoArray = arrayOf(packageInfo)
     private val packageInfoArray2 = arrayOf(packageInfo, packageInfo2)
@@ -124,9 +122,7 @@ internal class RestoreCoordinatorTest : TransportTest() {
     @Test
     fun `startRestore() optimized auto-restore with removed storage shows notification`() {
         every { settingsManager.getStorage() } returns storage
-        every { storage.isUsb } returns true
+        every { storage.isUnavailableUsb(context) } returns true
-        every { storage.getDocumentFile(context) } returns documentFile
-        every { documentFile.isDirectory } returns false
         every { metadataManager.getPackageMetadata(packageName) } returns PackageMetadata(42L)
         every { storage.name } returns storageName
         every {
@@ -149,9 +145,7 @@ internal class RestoreCoordinatorTest : TransportTest() {
     @Test
     fun `startRestore() optimized auto-restore with available storage shows no notification`() {
         every { settingsManager.getStorage() } returns storage
-        every { storage.isUsb } returns true
+        every { storage.isUnavailableUsb(context) } returns false
-        every { storage.getDocumentFile(context) } returns documentFile
-        every { documentFile.isDirectory } returns true
 
         assertEquals(TRANSPORT_OK, restore.startRestore(token, pmPackageInfoArray))
 
@@ -166,9 +160,7 @@ internal class RestoreCoordinatorTest : TransportTest() {
     @Test
     fun `startRestore() with removed storage shows no notification`() {
         every { settingsManager.getStorage() } returns storage
-        every { storage.isUsb } returns true
+        every { storage.isUnavailableUsb(context) } returns true
-        every { storage.getDocumentFile(context) } returns documentFile
-        every { documentFile.isDirectory } returns false
         every { metadataManager.getPackageMetadata(packageName) } returns null
 
         assertEquals(TRANSPORT_ERROR, restore.startRestore(token, pmPackageInfoArray))