Merge pull request #6947 from Bnyro/master

refactor: improve proxy url rewriting, store only unproxied URLs in database

app/src/main/java/com/github/libretube/api/obj/PipedStream.kt

@@ -3,6 +3,7 @@ package com.github.libretube.api.obj
 import android.os.Parcelable
 import com.github.libretube.db.obj.DownloadItem
 import com.github.libretube.enums.FileType
+import com.github.libretube.helpers.ProxyHelper
 import kotlinx.parcelize.Parcelize
 import kotlinx.serialization.Serializable
 import kotlin.io.path.Path
@@ -40,7 +41,7 @@ data class PipedStream(
         videoId = videoId,
         fileName = getQualityString(fileName),
         path = Path(""),
-        url = url,
+        url = url?.let { ProxyHelper.unwrapUrl(it) },
         format = format,
         quality = quality,
         language = audioTrackLocale,

app/src/main/java/com/github/libretube/api/obj/Playlist.kt

@@ -1,6 +1,7 @@
 package com.github.libretube.api.obj
 
 import com.github.libretube.db.obj.PlaylistBookmark
+import com.github.libretube.helpers.ProxyHelper
 import kotlinx.serialization.Serializable
 
 @Serializable
@@ -20,9 +21,9 @@ data class Playlist(
         return PlaylistBookmark(
             playlistId = playlistId,
             playlistName = name,
-            thumbnailUrl = thumbnailUrl,
+            thumbnailUrl = thumbnailUrl?.let { ProxyHelper.unwrapUrl(it) },
             uploader = uploader,
-            uploaderAvatar = uploaderAvatar,
+            uploaderAvatar = uploaderAvatar?.let { ProxyHelper.unwrapUrl(it) },
             uploaderUrl = uploaderUrl,
             videos = videos
         )

app/src/main/java/com/github/libretube/api/obj/StreamItem.kt

@@ -3,7 +3,10 @@ package com.github.libretube.api.obj
 import android.os.Parcelable
 import com.github.libretube.db.obj.LocalPlaylistItem
 import com.github.libretube.db.obj.SubscriptionsFeedItem
+import com.github.libretube.db.obj.WatchHistoryItem
 import com.github.libretube.extensions.toID
+import com.github.libretube.extensions.toLocalDate
+import com.github.libretube.helpers.ProxyHelper
 import kotlinx.parcelize.Parcelize
 import kotlinx.serialization.Serializable
 
@@ -32,10 +35,10 @@ data class StreamItem(
             playlistId = playlistId.toInt(),
             videoId = url!!.toID(),
             title = title,
-            thumbnailUrl = thumbnail,
+            thumbnailUrl = thumbnail?.let { ProxyHelper.unwrapUrl(it) },
             uploader = uploaderName,
             uploaderUrl = uploaderUrl,
-            uploaderAvatar = uploaderAvatar,
+            uploaderAvatar = uploaderAvatar?.let { ProxyHelper.unwrapUrl(it) },
             uploadDate = uploadedDate,
             duration = duration
         )
@@ -55,6 +58,17 @@ data class StreamItem(
         views = views,
         isShort = isShort
     )
+    
+    fun toWatchHistoryItem(videoId: String) = WatchHistoryItem(
+        videoId = videoId,
+        title = title,
+        uploadDate = uploaded.toLocalDate(),
+        uploader = uploaderName,
+        uploaderUrl = uploaderUrl?.toID(),
+        uploaderAvatar = uploaderAvatar?.let { ProxyHelper.unwrapUrl(it) },
+        thumbnailUrl = thumbnail?.let { ProxyHelper.unwrapUrl(it) },
+        duration = duration
+    )
 
     companion object {
         const val TYPE_STREAM = "stream"

app/src/main/java/com/github/libretube/api/obj/Streams.kt

@@ -4,7 +4,6 @@ import android.os.Parcelable
 import com.github.libretube.db.obj.DownloadItem
 import com.github.libretube.enums.FileType
 import com.github.libretube.extensions.toLocalDate
-import com.github.libretube.helpers.ProxyHelper
 import com.github.libretube.json.SafeInstantSerializer
 import com.github.libretube.parcelable.DownloadData
 import kotlinx.datetime.Instant
@@ -12,7 +11,6 @@ import kotlinx.parcelize.IgnoredOnParcel
 import kotlinx.parcelize.Parcelize
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
-import kotlin.io.path.Path
 
 @Serializable
 @Parcelize
@@ -75,17 +73,8 @@ data class Streams(
         }
 
         if (!subCode.isNullOrEmpty()) {
-            items.add(
-                DownloadItem(
-                    type = FileType.SUBTITLE,
-                    videoId = id,
-                    fileName = "${name}_$subCode.srt",
-                    path = Path(""),
-                    url = subtitles.find {
-                        it.code == subCode
-                    }?.url?.let { ProxyHelper.unwrapUrl(it) }
-                )
-            )
+            val subtitle = subtitles.find { it.code == subCode }
+            subtitle?.toDownloadItem(id)?.let { items.add(it) }
         }
 
         return items

app/src/main/java/com/github/libretube/api/obj/Subtitle.kt

@@ -3,8 +3,12 @@ package com.github.libretube.api.obj
 import android.content.Context
 import android.os.Parcelable
 import com.github.libretube.R
+import com.github.libretube.db.obj.DownloadItem
+import com.github.libretube.enums.FileType
+import com.github.libretube.helpers.ProxyHelper
 import kotlinx.parcelize.Parcelize
 import kotlinx.serialization.Serializable
+import kotlin.io.path.Path
 
 @Serializable
 @Parcelize
@@ -20,4 +24,12 @@ data class Subtitle(
     } else {
         "$name (${context.getString(R.string.auto_generated)})"
     }
+
+    fun toDownloadItem(videoId: String) = DownloadItem(
+        type = FileType.SUBTITLE,
+        videoId = videoId,
+        fileName = "${name}_${code}.srt",
+        path = Path(""),
+        url = url?.let { ProxyHelper.unwrapUrl(it) }
+    )
 }

app/src/main/java/com/github/libretube/db/DatabaseHelper.kt

@@ -1,14 +1,12 @@
 package com.github.libretube.db
 
 import com.github.libretube.api.obj.StreamItem
-import com.github.libretube.api.obj.Streams
 import com.github.libretube.constants.PreferenceKeys
 import com.github.libretube.db.DatabaseHolder.Database
 import com.github.libretube.db.obj.SearchHistoryItem
 import com.github.libretube.db.obj.WatchHistoryItem
 import com.github.libretube.enums.ContentFilter
 import com.github.libretube.extensions.toID
-import com.github.libretube.extensions.toLocalDate
 import com.github.libretube.helpers.PreferenceHelper
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.runBlocking
@@ -17,26 +15,6 @@ import kotlinx.coroutines.withContext
 object DatabaseHelper {
     private const val MAX_SEARCH_HISTORY_SIZE = 20
 
-    suspend fun addToWatchHistory(videoId: String, streams: Streams) = addToWatchHistory(
-        videoId,
-        streams.toStreamItem(videoId)
-    )
-
-    suspend fun addToWatchHistory(videoId: String, stream: StreamItem) {
-        val watchHistoryItem = WatchHistoryItem(
-            videoId,
-            stream.title,
-            stream.uploaded.toLocalDate(),
-            stream.uploaderName,
-            stream.uploaderUrl?.toID(),
-            stream.uploaderAvatar,
-            stream.thumbnail,
-            stream.duration
-        )
-
-        addToWatchHistory(watchHistoryItem)
-    }
-
     suspend fun addToWatchHistory(watchHistoryItem: WatchHistoryItem) =
         withContext(Dispatchers.IO) {
             Database.watchHistoryDao().insert(watchHistoryItem)

app/src/main/java/com/github/libretube/db/obj/LocalPlaylistItem.kt

@@ -4,7 +4,6 @@ import androidx.room.ColumnInfo
 import androidx.room.Entity
 import androidx.room.PrimaryKey
 import com.github.libretube.api.obj.StreamItem
-import com.github.libretube.helpers.ProxyHelper
 import kotlinx.serialization.Serializable
 
 @Serializable
@@ -25,10 +24,10 @@ data class LocalPlaylistItem(
         return StreamItem(
             url = videoId,
             title = title,
-            thumbnail = ProxyHelper.rewriteUrl(thumbnailUrl),
+            thumbnail = thumbnailUrl,
             uploaderName = uploader,
             uploaderUrl = uploaderUrl,
-            uploaderAvatar = ProxyHelper.rewriteUrl(uploaderAvatar),
+            uploaderAvatar = uploaderAvatar,
             uploadedDate = uploadDate,
             duration = duration
         )

app/src/main/java/com/github/libretube/helpers/DashHelper.kt

@@ -57,7 +57,7 @@ object DashHelper {
             }
 
             // only unwraps the url if the preference is set in the settings
-            stream.url = ProxyHelper.unwrapStreamUrl(stream.url.orEmpty())
+            stream.url = ProxyHelper.rewriteUrlUsingProxyPreference(stream.url.orEmpty())
 
             val adapSetInfo = adapSetInfos.find { it.mimeType == stream.mimeType }
             if (adapSetInfo != null) {
@@ -83,7 +83,7 @@ object DashHelper {
             }
 
             // only unwraps the url if the preference is set in the settings
-            stream.url = ProxyHelper.unwrapStreamUrl(stream.url.orEmpty())
+            stream.url = ProxyHelper.rewriteUrlUsingProxyPreference(stream.url.orEmpty())
 
             adapSetInfos.add(
                 AdapSetInfo(

app/src/main/java/com/github/libretube/helpers/ImageHelper.kt

@@ -87,7 +87,7 @@ object ImageHelper {
 
         // only load the image if the data saver mode is disabled
         if (DataSaverMode.isEnabled(target.context) || url.isNullOrEmpty()) return
-        val urlToLoad = ProxyHelper.unwrapImageUrl(url)
+        val urlToLoad = ProxyHelper.rewriteUrlUsingProxyPreference(url)
 
         getImageWithCallback(target.context, urlToLoad) { result ->
             // set the background to white for transparent images

app/src/main/java/com/github/libretube/helpers/ProxyHelper.kt

@@ -18,58 +18,50 @@ object ProxyHelper {
         }
     }
 
-    fun rewriteUrl(url: String?): String? {
+    /**
+     * Decide whether the proxy should be used or not for a given stream URL based on user preferences
+     */
+    fun rewriteUrlUsingProxyPreference(url: String): String {
+        if (PlayerHelper.disablePipedProxy) {
+            return unwrapUrl(url)
+        }
+
+        return proxyRewriteUrl(url) ?: url
+    }
+
+    /**
+     * Rewrite the URL to use the stored image proxy url of the selected instance.
+     * Can handle both Piped links and normal YouTube links.
+     */
+    private fun proxyRewriteUrl(url: String?): String? {
         if (url == null) return null
 
         val proxyUrl = PreferenceHelper.getString(PreferenceKeys.IMAGE_PROXY_URL, "")
-            .toHttpUrlOrNull() ?: return url
+            .toHttpUrlOrNull()
 
-        val parsedUrl = url.toHttpUrlOrNull() ?: return url
-        if (parsedUrl.queryParameter("host").isNullOrEmpty()) {
-            return parsedUrl.newBuilder()
-                .host(proxyUrl.host)
-                .port(proxyUrl.port)
-                .setQueryParameter("host", parsedUrl.host)
-                .build()
-                .toString()
-        }
+        // parsedUrl should now be a plain YouTube URL without using any proxy
+        val parsedUrl = unwrapUrl(url).toHttpUrlOrNull()
+        if (proxyUrl == null || parsedUrl == null) return null
 
         return parsedUrl.newBuilder()
             .host(proxyUrl.host)
             .port(proxyUrl.port)
+            .setQueryParameter("host", parsedUrl.host)
             .build()
             .toString()
     }
 
-    /**
-     * Detect whether the proxy should be used or not for a given stream URL based on user preferences
-     */
-    fun unwrapStreamUrl(url: String): String {
-        return if (PlayerHelper.disablePipedProxy && !PlayerHelper.localStreamExtraction) {
-            unwrapUrl(url)
-        } else {
-            url
-        }
-    }
-
-    fun unwrapImageUrl(url: String): String {
-        return if (PlayerHelper.disablePipedProxy) {
-            unwrapUrl(url)
-        } else {
-            url
-        }
-    }
-
     /**
      * Convert a proxied Piped url to a YouTube url that's not proxied
+     *
+     * Should not be called directly in most cases, use [rewriteUrlUsingProxyPreference] instead
      */
-    fun unwrapUrl(url: String, unwrap: Boolean = true): String {
-        val parsedUrl = url.toHttpUrlOrNull()
+    fun unwrapUrl(url: String): String {
+        val parsedUrl = url.toHttpUrlOrNull() ?: return url
 
-        val host = parsedUrl?.queryParameter("host")
-        // if there's no host parameter specified, there's no way to unwrap the URL
-        // and the proxied one must be used. That's the case if using LBRY.
-        if (!unwrap || parsedUrl == null || host.isNullOrEmpty()) {
+        val host = parsedUrl.queryParameter("host")
+        // If the host is not set, the URL is probably already unwrapped
+        if (host.isNullOrEmpty()) {
             return url
         }
 

app/src/main/java/com/github/libretube/repo/LocalPlaylistsRepository.kt

@@ -10,7 +10,6 @@ import com.github.libretube.api.obj.StreamItem
 import com.github.libretube.db.DatabaseHolder
 import com.github.libretube.db.obj.LocalPlaylist
 import com.github.libretube.extensions.parallelMap
-import com.github.libretube.helpers.ProxyHelper
 import com.github.libretube.obj.PipedImportPlaylist
 
 class LocalPlaylistsRepository: PlaylistRepository {
@@ -21,7 +20,7 @@ class LocalPlaylistsRepository: PlaylistRepository {
         return Playlist(
             name = relation.playlist.name,
             description = relation.playlist.description,
-            thumbnailUrl = ProxyHelper.rewriteUrl(relation.playlist.thumbnailUrl),
+            thumbnailUrl = relation.playlist.thumbnailUrl,
             videos = relation.videos.size,
             relatedStreams = relation.videos.map { it.toStreamItem() }
         )
@@ -34,7 +33,7 @@ class LocalPlaylistsRepository: PlaylistRepository {
                     id = it.playlist.id.toString(),
                     name = it.playlist.name,
                     shortDescription = it.playlist.description,
-                    thumbnail = ProxyHelper.rewriteUrl(it.playlist.thumbnailUrl),
+                    thumbnail = it.playlist.thumbnailUrl,
                     videos = it.videos.size.toLong()
                 )
             }

app/src/main/java/com/github/libretube/services/DownloadService.kt

@@ -214,7 +214,7 @@ class DownloadService : LifecycleService() {
         setResumeNotification(notificationBuilder, item)
 
         var totalRead = item.path.fileSize()
-        val url = URL(ProxyHelper.unwrapStreamUrl(item.url ?: return))
+        val url = URL(ProxyHelper.rewriteUrlUsingProxyPreference(item.url ?: return))
 
         // only fetch the content length if it's not been returned by the API
         if (item.downloadSize <= 0L) {

app/src/main/java/com/github/libretube/services/OnlinePlayerService.kt

@@ -90,7 +90,11 @@ open class OnlinePlayerService : AbstractPlayerService() {
                     // waiting for the player to be ready since the video can't be claimed to be watched
                     // while it did not yet start actually, but did buffer only so far
                     scope.launch(Dispatchers.IO) {
-                        streams?.let { DatabaseHelper.addToWatchHistory(videoId, it) }
+                        streams?.let { streams ->
+                            val watchHistoryItem =
+                                streams.toStreamItem(videoId).toWatchHistoryItem(videoId)
+                            DatabaseHelper.addToWatchHistory(watchHistoryItem)
+                        }
                     }
                 }
             }
@@ -241,7 +245,8 @@ open class OnlinePlayerService : AbstractPlayerService() {
         if (args.containsKey(PlayerCommand.SET_SB_AUTO_SKIP_ENABLED.name)) {
             sponsorBlockAutoSkip = args.getBoolean(PlayerCommand.SET_SB_AUTO_SKIP_ENABLED.name)
         } else if (args.containsKey(PlayerCommand.SET_AUTOPLAY_COUNTDOWN_ENABLED.name)) {
-            autoPlayCountdownEnabled = args.getBoolean(PlayerCommand.SET_AUTOPLAY_COUNTDOWN_ENABLED.name)
+            autoPlayCountdownEnabled =
+                args.getBoolean(PlayerCommand.SET_AUTOPLAY_COUNTDOWN_ENABLED.name)
         }
     }
 
@@ -272,7 +277,7 @@ open class OnlinePlayerService : AbstractPlayerService() {
                 // only use the dash manifest generated by YT if either it's a livestream or no other source is available
                 val dashUri =
                     if (streams.isLive && streams.dash != null) {
-                        ProxyHelper.unwrapStreamUrl(
+                        ProxyHelper.rewriteUrlUsingProxyPreference(
                             streams.dash
                         ).toUri()
                     } else {
@@ -289,7 +294,7 @@ open class OnlinePlayerService : AbstractPlayerService() {
                     .setPlaylistParserFactory(YoutubeHlsPlaylistParser.Factory())
 
                 val mediaItem = createMediaItem(
-                    ProxyHelper.unwrapStreamUrl(streams.hls).toUri(),
+                    ProxyHelper.rewriteUrlUsingProxyPreference(streams.hls).toUri(),
                     MimeTypes.APPLICATION_M3U8,
                     streams
                 )

app/src/main/java/com/github/libretube/ui/fragments/LibraryFragment.kt

@@ -29,7 +29,6 @@ import com.github.libretube.extensions.ceilHalf
 import com.github.libretube.extensions.dpToPx
 import com.github.libretube.helpers.NavBarHelper
 import com.github.libretube.helpers.PreferenceHelper
-import com.github.libretube.helpers.ProxyHelper
 import com.github.libretube.ui.adapters.PlaylistBookmarkAdapter
 import com.github.libretube.ui.adapters.PlaylistsAdapter
 import com.github.libretube.ui.base.DynamicLayoutManagerFragment
@@ -146,9 +145,7 @@ class LibraryFragment : DynamicLayoutManagerFragment() {
     private fun initBookmarks() {
         lifecycleScope.launch {
             val bookmarks = withContext(Dispatchers.IO) {
-                DatabaseHolder.Database.playlistBookmarkDao().getAll().map {
-                    it.copy(thumbnailUrl = ProxyHelper.rewriteUrl(it.thumbnailUrl))
-                }
+                DatabaseHolder.Database.playlistBookmarkDao().getAll()
             }
 
             val binding = _binding ?: return@launch

app/src/main/java/com/github/libretube/ui/fragments/PlayerFragment.kt

@@ -683,7 +683,7 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions {
             val context = requireContext()
             lifecycleScope.launch {
                 val hlsStream = withContext(Dispatchers.IO) {
-                    ProxyHelper.unwrapStreamUrl(streams.hls!!).toUri()
+                    ProxyHelper.rewriteUrlUsingProxyPreference(streams.hls!!).toUri()
                 }
                 IntentHelper.openWithExternalPlayer(
                     context,

app/src/main/java/com/github/libretube/ui/fragments/WatchHistoryFragment.kt

@@ -29,7 +29,6 @@ import com.github.libretube.extensions.setOnDismissListener
 import com.github.libretube.helpers.NavBarHelper
 import com.github.libretube.helpers.NavigationHelper
 import com.github.libretube.helpers.PreferenceHelper
-import com.github.libretube.helpers.ProxyHelper
 import com.github.libretube.ui.adapters.WatchHistoryAdapter
 import com.github.libretube.ui.base.DynamicLayoutManagerFragment
 import com.github.libretube.ui.extensions.addOnBottomReachedListener
@@ -168,12 +167,6 @@ class WatchHistoryFragment : DynamicLayoutManagerFragment() {
 
     private fun showWatchHistory(history: List<WatchHistoryItem>) {
         val watchHistory = history.filterByStatusAndWatchPosition()
-
-        watchHistory.forEach {
-            it.thumbnailUrl = ProxyHelper.rewriteUrl(it.thumbnailUrl)
-            it.uploaderAvatar = ProxyHelper.rewriteUrl(it.uploaderAvatar)
-        }
-
         val watchHistoryAdapter = WatchHistoryAdapter(watchHistory.toMutableList())
 
         binding.playAll.setOnClickListener {

app/src/main/java/com/github/libretube/ui/sheets/VideoOptionsBottomSheet.kt

@@ -103,7 +103,7 @@ class VideoOptionsBottomSheet : BaseBottomSheet() {
                         DatabaseHolder.Database.watchPositionDao().insert(watchPosition)
                         if (!PlayerHelper.watchHistoryEnabled) return@withContext
                         // add video to watch history
-                        DatabaseHelper.addToWatchHistory(videoId, streamItem)
+                        DatabaseHelper.addToWatchHistory(streamItem.toWatchHistoryItem(videoId))
                     }
                     if (PreferenceHelper.getBoolean(PreferenceKeys.HIDE_WATCHED_FROM_FEED, false)) {
                         // get the host fragment containing the current fragment