Merge pull request #881 from Automattic/update/wear-network-call-factory

Prefer faster networks for downloads on watch
Automattic · Apr 18, 2023 · 1950fed · 1950fed
2 parents 3f1eef5 + d7ff7c6
commit 1950fed
Show file tree

Hide file tree

Showing 10 changed files with 421 additions and 29 deletions.
diff --git a/app/src/main/java/au/com/shiftyjelly/pocketcasts/di/AppModule.kt b/app/src/main/java/au/com/shiftyjelly/pocketcasts/di/AppModule.kt
@@ -0,0 +1,36 @@
+package au.com.shiftyjelly.pocketcasts.di
+
+import android.content.Context
+import android.net.ConnectivityManager
+import au.com.shiftyjelly.pocketcasts.repositories.di.DownloadCallFactory
+import au.com.shiftyjelly.pocketcasts.repositories.di.DownloadOkHttpClient
+import au.com.shiftyjelly.pocketcasts.repositories.di.DownloadRequestBuilder
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.android.qualifiers.ApplicationContext
+import dagger.hilt.components.SingletonComponent
+import okhttp3.Call
+import okhttp3.OkHttpClient
+import okhttp3.Request
+import javax.inject.Singleton
+
+@Module
+@InstallIn(SingletonComponent::class)
+object AppModule {
+
+    @Provides
+    fun connectivityManager(@ApplicationContext application: Context): ConnectivityManager =
+        application.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
+
+    @Provides
+    @Singleton
+    @DownloadCallFactory
+    fun downloadCallFactory(
+        @DownloadOkHttpClient phoneCallFactory: OkHttpClient,
+    ): Call.Factory = phoneCallFactory
+
+    @Provides
+    @DownloadRequestBuilder
+    fun downloadRequestBuilder(): Request.Builder = Request.Builder()
+}
diff --git a/automotive/src/main/java/au/com/shiftyjelly/pocketcasts/di/AutomotiveAppModule.kt b/automotive/src/main/java/au/com/shiftyjelly/pocketcasts/di/AutomotiveAppModule.kt
@@ -0,0 +1,29 @@
+package au.com.shiftyjelly.pocketcasts.di
+
+import au.com.shiftyjelly.pocketcasts.repositories.di.DownloadCallFactory
+import au.com.shiftyjelly.pocketcasts.repositories.di.DownloadOkHttpClient
+import au.com.shiftyjelly.pocketcasts.repositories.di.DownloadRequestBuilder
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import okhttp3.Call
+import okhttp3.OkHttpClient
+import okhttp3.Request
+import javax.inject.Singleton
+
+@Module
+@InstallIn(SingletonComponent::class)
+object AutomotiveAppModule {
+
+    @Provides
+    @Singleton
+    @DownloadCallFactory
+    fun downloadCallFactory(
+        @DownloadOkHttpClient phoneCallFactory: OkHttpClient,
+    ): Call.Factory = phoneCallFactory
+
+    @Provides
+    @DownloadRequestBuilder
+    fun downloadRequestBuilder(): Request.Builder = Request.Builder()
+}
diff --git a/modules/services/repositories/build.gradle b/modules/services/repositories/build.gradle
@@ -15,4 +15,3 @@ dependencies {
     implementation project(':modules:services:servers')
     implementation project(':modules:services:utils')
 }
-
diff --git a/.../repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/di/Annotations.kt b/.../repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/di/Annotations.kt
@@ -0,0 +1,28 @@
+package au.com.shiftyjelly.pocketcasts.repositories.di
+
+import javax.inject.Qualifier
+
+/**
+ * Annotation for providing the Call.Factory used for downloads. The provides method
+ * for this annotation must be provided in the relevant application module because
+ * the Call.Factory is different for Wear.
+ */
+@Qualifier
+@Retention(AnnotationRetention.BINARY)
+annotation class DownloadCallFactory
+
+/**
+ * Annotation for providing the OkhttpClient for download calls.
+ */
+@Qualifier
+@Retention(AnnotationRetention.BINARY)
+annotation class DownloadOkHttpClient
+
+/**
+ * Annotation for providing the Request.Builder used for download calls. The provides method
+ * for this annotation must be provided in the relevant application module because
+ * the Request.Builder is different for Wear.
+ */
+@Qualifier
+@Retention(AnnotationRetention.BINARY)
+annotation class DownloadRequestBuilder
diff --git a/.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/di/RepositoryProviderModule.kt b/.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/di/RepositoryProviderModule.kt
@@ -6,6 +6,9 @@ import dagger.Module
 import dagger.Provides
 import dagger.hilt.InstallIn
 import dagger.hilt.components.SingletonComponent
+import okhttp3.Dispatcher
+import okhttp3.OkHttpClient
+import java.util.concurrent.TimeUnit
 import javax.inject.Singleton
 
 @Module
@@ -15,4 +18,19 @@ class RepositoryProviderModule {
     @Provides
     @Singleton
     fun provideTokenHandler(syncAccountManager: SyncAccountManager): TokenHandler = syncAccountManager
+
+    @Provides
+    @Singleton
+    @DownloadOkHttpClient
+    fun downloadOkHttpClient(): OkHttpClient {
+        val dispatcher = Dispatcher().apply {
+            maxRequestsPerHost = 5
+        }
+        return OkHttpClient.Builder()
+            .dispatcher(dispatcher)
+            .connectTimeout(30, TimeUnit.SECONDS)
+            .writeTimeout(30, TimeUnit.SECONDS)
+            .readTimeout(30, TimeUnit.SECONDS)
+            .build()
+    }
 }
diff --git a/...ain/java/au/com/shiftyjelly/pocketcasts/repositories/download/task/DownloadEpisodeTask.kt b/...ain/java/au/com/shiftyjelly/pocketcasts/repositories/download/task/DownloadEpisodeTask.kt
@@ -15,6 +15,8 @@ import au.com.shiftyjelly.pocketcasts.models.entity.Playable
 import au.com.shiftyjelly.pocketcasts.models.entity.UserEpisode
 import au.com.shiftyjelly.pocketcasts.models.type.EpisodeStatusEnum
 import au.com.shiftyjelly.pocketcasts.preferences.Settings.NotificationId
+import au.com.shiftyjelly.pocketcasts.repositories.di.DownloadCallFactory
+import au.com.shiftyjelly.pocketcasts.repositories.di.DownloadRequestBuilder
 import au.com.shiftyjelly.pocketcasts.repositories.download.DownloadManager
 import au.com.shiftyjelly.pocketcasts.repositories.download.DownloadProgressUpdate
 import au.com.shiftyjelly.pocketcasts.repositories.download.ResponseValidationResult
@@ -32,10 +34,9 @@ import io.reactivex.ObservableEmitter
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.rx2.await
 import okhttp3.Call
-import okhttp3.Dispatcher
+import okhttp3.Callback
 import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
 import okhttp3.MediaType
-import okhttp3.OkHttpClient
 import okhttp3.Request
 import okhttp3.Response
 import timber.log.Timber
@@ -52,8 +53,11 @@ import java.io.RandomAccessFile
 import java.net.SocketException
 import java.net.SocketTimeoutException
 import java.net.UnknownHostException
-import java.util.concurrent.TimeUnit
+import javax.inject.Provider
 import javax.net.ssl.SSLHandshakeException
+import kotlin.coroutines.resume
+import kotlin.coroutines.resumeWithException
+import kotlin.coroutines.suspendCoroutine
 import au.com.shiftyjelly.pocketcasts.localization.R as LR
 
 private class UnderscoreInHostName : Exception("Download URL is invalid, as it contains an underscore in the hostname. Please contact the podcast author to resolve this.")
@@ -64,7 +68,9 @@ class DownloadEpisodeTask @AssistedInject constructor(
     @Assisted params: WorkerParameters,
     var downloadManager: DownloadManager,
     var episodeManager: EpisodeManager,
-    var userEpisodeManager: UserEpisodeManager
+    var userEpisodeManager: UserEpisodeManager,
+    @DownloadCallFactory private val callFactory: Call.Factory,
+    @DownloadRequestBuilder private val requestBuilderProvider: Provider<Request.Builder>
 ) : Worker(context, params) {
 
     companion object {
@@ -107,18 +113,6 @@ class DownloadEpisodeTask @AssistedInject constructor(
     private var bytesDownloadedSoFar: Long = 0
     private var bytesRemaining: Long = 0
 
-    private val okHttpClient by lazy {
-        val dispatcher = Dispatcher()
-        dispatcher.maxRequestsPerHost = 5
-        val builder = OkHttpClient.Builder()
-            .dispatcher(dispatcher)
-            .connectTimeout(30, TimeUnit.SECONDS)
-            .writeTimeout(30, TimeUnit.SECONDS)
-            .readTimeout(30, TimeUnit.SECONDS)
-
-        builder.build()
-    }
-
     override fun doWork(): Result {
         if (isStopped) {
             LogBuffer.i(LogBuffer.TAG_BACKGROUND_TASKS, "Cancelling execution of $episodeUUID download because we are already stopped")
@@ -228,7 +222,7 @@ class DownloadEpisodeTask @AssistedInject constructor(
                         emitter.onComplete()
                     }
                 } else {
-                    downloadFile(tempDownloadPath!!, okHttpClient, 1, emitter)
+                    downloadFile(tempDownloadPath!!, callFactory, 1, emitter)
                     if (!emitter.isDisposed) {
                         emitter.onComplete()
                     }
@@ -243,7 +237,7 @@ class DownloadEpisodeTask @AssistedInject constructor(
         }
     }
 
-    private fun downloadFile(tempDownloadPath: String, httpClient: OkHttpClient, tryCount: Int, emitter: ObservableEmitter<DownloadProgressUpdate>) {
+    private fun downloadFile(tempDownloadPath: String, httpClient: Call.Factory, tryCount: Int, emitter: ObservableEmitter<DownloadProgressUpdate>) {
         if (emitter.isDisposed || isStopped || pathToSaveTo == null) {
             return
         }
@@ -269,7 +263,7 @@ class DownloadEpisodeTask @AssistedInject constructor(
                 throw UnderscoreInHostName()
             }
 
-            val requestBuilder = Request.Builder()
+            val requestBuilder = requestBuilderProvider.get()
                 .url(downloadUrl)
                 .header("User-Agent", "Pocket Casts")
 
@@ -294,7 +288,7 @@ class DownloadEpisodeTask @AssistedInject constructor(
                     .header("Accept-Encoding", "identity")
                     .build()
                 call = httpClient.newCall(request)
-                response = call.execute()
+                response = call.blockingEnqueue()
 
                 if (response.code != HTTP_RESUME_SUPPORTED) {
                     LogBuffer.i(LogBuffer.TAG_BACKGROUND_TASKS, "Resuming ${episode.title} not supported, restarting download.")
@@ -313,7 +307,7 @@ class DownloadEpisodeTask @AssistedInject constructor(
             if (response == null) {
                 val request = requestBuilder.build()
                 call = httpClient.newCall(request)
-                response = call.execute()
+                response = call.blockingEnqueue()
             }
 
             if (emitter.isDisposed || isStopped) {
@@ -646,3 +640,22 @@ class DownloadEpisodeTask @AssistedInject constructor(
 
     class DownloadFailed(val exception: Exception?, message: String, val retry: Boolean) : Exception(message)
 }
+
+/**
+ * Have to use enqueue for high bandwidth requests on the watch app
+ * See https://github.com/google/horologist/blob/7bd044a4766e379f85ee3f5a01272853eec3155d/network-awareness/src/main/java/com/google/android/horologist/networks/okhttp/impl/HighBandwidthCall.kt#L93-L92
+ */
+private fun Call.blockingEnqueue(): Response =
+    runBlocking {
+        suspendCoroutine { cont ->
+            this@blockingEnqueue.enqueue(object : Callback {
+                override fun onFailure(call: Call, e: IOException) {
+                    cont.resumeWithException(e)
+                }
+
+                override fun onResponse(call: Call, response: Response) {
+                    cont.resume(response)
+                }
+            })
+        }
+    }
diff --git a/.../pocketcasts/wear/di/ApplicationModule.kt → ...elly/pocketcasts/wear/di/WearAppModule.kt b/.../pocketcasts/wear/di/ApplicationModule.kt → ...elly/pocketcasts/wear/di/WearAppModule.kt
@@ -1,6 +1,7 @@
 package au.com.shiftyjelly.pocketcasts.wear.di
 
 import android.content.Context
+import android.net.ConnectivityManager
 import au.com.shiftyjelly.pocketcasts.wear.data.service.log.Logging
 import au.com.shiftyjelly.pocketcasts.wear.ui.AppConfig
 import com.google.android.horologist.media3.config.WearMedia3Factory
@@ -20,7 +21,8 @@ import javax.inject.Singleton
 
 @Module
 @InstallIn(SingletonComponent::class)
-object ApplicationModule {
+object WearAppModule {
+
     @Singleton
     @Provides
     fun intentBuilder(
@@ -71,4 +73,10 @@ object ApplicationModule {
     fun errorReporter(
         logging: Logging,
     ): ErrorReporter = logging
+
+    @Provides
+    fun connectivityManager(
+        @ApplicationContext application: Context
+    ): ConnectivityManager =
+        application.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -15,4 +15,3 @@ dependencies {
		implementation project(':modules:services:servers')
		implementation project(':modules:services:utils')
		}