Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## Fixed
- Shorts no longer show up in Subscriptions when Subscription shorts is turned off (#1393)
- Shorts no longer slip through as new video notifications when Shorts notifications are off
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import io.github.aedev.flow.data.shorts.ChannelReelIndex
import io.github.aedev.flow.data.subscriptions.ChannelRssClient
import io.github.aedev.flow.data.subscriptions.ChannelRssEntry
import io.github.aedev.flow.data.subscriptions.ChannelUploadsClient
import io.github.aedev.flow.data.subscriptions.classifiedFromVerdicts
import io.github.aedev.flow.utils.ThumbnailUrlResolver
import io.github.aedev.flow.utils.formatYouTubeRelativeTime
import io.github.aedev.flow.utils.parsePremiereTimestamp
Expand Down Expand Up @@ -59,15 +60,20 @@ data class SubscriptionFeedChunk(
* on its own. No later chunk changes their slice, so the caller can store them right away.
*/
val rssSettledChannelIds: Set<String>? = null,
/**
* Every upload this sweep knows is a reel, including the older ones the feed leaves out (it keeps
* a channel's newest reel only). A cached row of one of them is filed as a Short again.
*/
val reelIds: Set<String> = emptySet(),
)

/**
* Two-stage subscription extraction.
*
* The RSS stage reads every channel's feed: cheap, and the only source with a trustworthy publish
* timestamp. RSS lists Shorts alongside ordinary uploads without marking either, so an entry is only
* shown once [ChannelReelIndex] or a stored verdict says what it is; a channel whose verdict is not
* at hand is held back for the checks stage rather than shown with its reels as videos (#903).
* timestamp. Each entry's link says whether it is a reel (#1393); an entry without one is only shown
* once [ChannelReelIndex] or a stored verdict says what it is, and a channel whose verdict is not at
* hand is held back for the checks stage rather than shown with its reels as videos (#903).
*
* The checks stage asks the Shorts tab for those held channels, then reads the channel tabs for the
* channels RSS could not satisfy, which is also the only way to see livestreams — their upload dates
Expand Down Expand Up @@ -111,7 +117,7 @@ class RssSubscriptionService
.map { channelId ->
async(Dispatchers.IO) {
val result = fetchRssVideos(channelId, minimumDateMillis, knownVideoIds)
Triple(channelId, result, knownReels(channelId, result.videos, storedReelVerdicts))
Triple(channelId, result, knownReels(channelId, result, storedReelVerdicts))
}
}.awaitAll()
}
Expand Down Expand Up @@ -241,6 +247,7 @@ class RssSubscriptionService
private inner class Sweep {
val regular = mutableListOf<Video>()
val shorts = mutableListOf<Video>()
val reelIds = HashSet<String>()
val rssDates = mutableMapOf<String, Long>()
val hasRecent = mutableMapOf<String, Boolean>()

Expand All @@ -255,7 +262,14 @@ class RssSubscriptionService
val incomplete = mutableSetOf<String>()

fun add(videos: List<Video>) {
videos.forEach { if (it.isShort) shorts += it else regular += it }
videos.forEach {
if (it.isShort) {
shorts += it
reelIds += it.id
} else {
regular += it
}
}
}

fun compact() {
Expand All @@ -277,6 +291,7 @@ class RssSubscriptionService
processedChannels = processed,
totalChannels = total,
rssSettledChannelIds = rssSettled,
reelIds = reelIds.toSet(),
)
}

Expand Down Expand Up @@ -311,6 +326,7 @@ class RssSubscriptionService
val videoTimestamps: Map<String, Long>,
val videos: List<Video>,
val needsChannelFallback: Boolean,
val linkVerdicts: Map<String, Boolean> = emptyMap(),
/** The feed lists an upload inside the window, whether or not it survived the filters. */
val hasRecentEntries: Boolean = false,
val failed: Boolean = false,
Expand All @@ -325,7 +341,7 @@ class RssSubscriptionService

/**
* RSS gives accurate dates for all recent uploads (including Shorts) but never a duration or a
* Shorts/live flag, so its timestamps double as the date source for the Phase 2 tab items.
* live flag, so its timestamps double as the date source for the Phase 2 tab items.
*/
private suspend fun fetchRssVideos(
channelId: String,
Expand All @@ -347,6 +363,7 @@ class RssSubscriptionService

val timestamps = mutableMapOf<String, Long>()
val videos = mutableListOf<Video>()
val linkVerdicts = mutableMapOf<String, Boolean>()
var newestTimestamp = 0L

for (entry in feed.entries) {
Expand All @@ -356,6 +373,7 @@ class RssSubscriptionService
if (publishedAt > newestTimestamp) newestTimestamp = publishedAt
if (publishedAt > minimumDateMillis && !entry.isPaidOrMembersOnly()) {
videos += entry.toVideo(channelId = channelId, channelName = feed.channelName)
entry.isShort?.let { linkVerdicts[entry.videoId] = it }
}
}

Expand All @@ -377,25 +395,23 @@ class RssSubscriptionService
videoTimestamps = timestamps,
videos = videos,
needsChannelFallback = videos.isEmpty() && newestTimestamp > minimumDateMillis,
linkVerdicts = linkVerdicts,
hasRecentEntries = newestTimestamp > minimumDateMillis,
)
}

/**
* RSS cannot tell a reel from a video. When the store already classified every entry, or the
* Shorts tab was read within the cache window, the answer is reused; otherwise the channel
* needs a Shorts check, one full browse, and null holds it for the checks stage.
* Each entry's link, else a stored verdict, classifies the slice without a browse. When some
* entry has neither, a Shorts tab read within the cache window is reused; otherwise the
* channel needs a Shorts check, one full browse, and null holds it for the checks stage.
*/
private suspend fun knownReels(
channelId: String,
videos: List<Video>,
result: RssResult,
storedReelVerdicts: Map<String, Boolean>,
): List<Video>? {
if (videos.isNotEmpty() && videos.all { it.id in storedReelVerdicts }) {
return videos.map { it.copy(isShort = storedReelVerdicts.getValue(it.id)) }
}
return channelReelIndex.markReelsIfCached(channelId, videos)
}
): List<Video>? =
classifiedFromVerdicts(result.videos, result.linkVerdicts, storedReelVerdicts)
?: channelReelIndex.markReelsIfCached(channelId, result.videos)

private fun ChannelRssEntry.toVideo(
channelId: String,
Expand All @@ -416,7 +432,7 @@ class RssSubscriptionService
timestampIsExact = true,
channelThumbnailUrl = "",
description = description.orEmpty(),
isShort = false,
isShort = isShort == true,
isLive = false,
isUpcoming = isUpcoming,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ data class ChannelRssEntry(
val publishedAtMillis: Long = 0L,
val viewCount: Long = 0L,
val description: String? = null,
/** From the entry's link: YouTube links a reel as `/shorts/`. Null when the link says neither. */
val isShort: Boolean? = null,
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ object ChannelRssParser {
var description: String? = null
var publishedAt = 0L
var viewCount = 0L
var isShort: Boolean? = null

var eventType = parser.eventType
while (eventType != XmlPullParser.END_DOCUMENT) {
Expand All @@ -67,6 +68,7 @@ object ChannelRssParser {
description = null
publishedAt = 0L
viewCount = 0L
isShort = null
} else if (insideEntry) {
when {
tagName.equals("videoId", ignoreCase = true) -> {
Expand Down Expand Up @@ -95,6 +97,10 @@ object ChannelRssParser {
tagName.equals("statistics", ignoreCase = true) && viewCount == 0L -> {
viewCount = parser.getAttributeValue(null, "views")?.toLongOrNull() ?: 0L
}

tagName.equals("link", ignoreCase = true) && isShort == null -> {
isShort = reelVerdict(parser.getAttributeValue(null, "href"))
}
}
} else if (tagName.equals("name", ignoreCase = true) && channelName == null) {
// Only the feed-level <author> carries a <name> in a channel feed.
Expand All @@ -116,6 +122,7 @@ object ChannelRssParser {
publishedAtMillis = publishedAt,
viewCount = viewCount,
description = description,
isShort = isShort,
)
}
}
Expand All @@ -129,6 +136,15 @@ object ChannelRssParser {
return ChannelRssFeed(channelName = channelName?.takeIf { it.isNotBlank() }, entries = entries)
}

private fun reelVerdict(href: String?): Boolean? {
val path = href?.substringAfter("://", "")?.substringAfter('/', "") ?: return null
return when {
path.startsWith("shorts/") -> true
path.startsWith("watch") -> false
else -> null
}
}

private fun parseTimestamp(value: String?): Long {
val text = value?.trim().orEmpty()
if (text.isEmpty()) return 0L
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ class SubscriptionFeedRepository
var failedChannelIds = emptySet<String>()
var failedChannelReasons = emptyMap<String, String>()
var incompleteChannelIds = emptySet<String>()
var reelIds = emptySet<String>()
val writtenChannelIds = HashSet<String>()

rssSubscriptionService
Expand All @@ -173,6 +174,7 @@ class SubscriptionFeedRepository
failedChannelIds = chunk.failedChannelIds
failedChannelReasons = chunk.failedChannelReasons
incompleteChannelIds = chunk.incompleteChannelIds
reelIds = chunk.reelIds
if (chunk.videos.isNotEmpty()) {
latestChunkVideos = chunk.videos
previewVideos =
Expand All @@ -194,6 +196,7 @@ class SubscriptionFeedRepository
failedChannelIds = emptySet(),
refreshTime = System.currentTimeMillis(),
dropUnfollowed = false,
reelIds = reelIds,
)
writtenChannelIds += settled
previewVideos = withContext(PerformanceDispatcher.diskIO) { loadCachedFeed(plan.scope) }
Expand All @@ -219,6 +222,7 @@ class SubscriptionFeedRepository
failedChannelIds = failedChannelIds + incompleteChannelIds,
refreshTime = System.currentTimeMillis(),
dropUnfollowed = plan.isFullRefresh,
reelIds = reelIds,
)
}
val done =
Expand Down Expand Up @@ -247,6 +251,7 @@ class SubscriptionFeedRepository
failedChannelIds: Set<String>,
refreshTime: Long,
dropUnfollowed: Boolean,
reelIds: Set<String>,
) {
val written = channelIds.toHashSet()
val subscribedChannelIds =
Expand All @@ -263,6 +268,7 @@ class SubscriptionFeedRepository
now = refreshTime,
windowMs = SUBSCRIPTION_CACHE_WINDOW_MS,
maxPerChannel = MAX_ROWS_PER_CHANNEL,
reelIds = reelIds,
)

val entities = write.rows.map { it.toEntity(refreshTime) }
Expand Down Expand Up @@ -296,9 +302,9 @@ class SubscriptionFeedRepository
* has already enriched. The channel is deliberately *not* marked as fetched: RSS alone
* cannot see livestreams, so the feed still owes it a full pass.
*
* [reelVideoIds] is the caller's reel verdict for these entries. RSS marks none of them, so
* without it every seeded reel reaches the feed as an ordinary upload and stays there until
* the channel's next full refresh — visible even with Shorts switched off (#903).
* [reelVideoIds] is the caller's reel verdict for these entries, from their links or the
* Shorts tab. Without it every seeded reel reaches the feed as an ordinary upload and stays
* there until the channel's next full refresh, visible even with Shorts switched off (#903).
*/
suspend fun seedFromNotificationCheck(
channelId: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ internal data class SubscriptionFeedWrite(
* two (#1099). The lookback window still ages rows out, and each channel keeps its newest
* [maxPerChannel].
*
* A cached row of an upload the pass knows is a reel ([reelIds]) is filed as a Short: an earlier
* pass that could not tell may have kept it as a video, and RSS repeats a reel for weeks (#1393).
*
* A channel that failed this pass is not stamped fresh, so the next pass retries it instead of
* showing it empty for the whole TTL (#1094). A channel unsubscribed while the refresh ran is
* dropped, fetched rows and all.
Expand All @@ -32,17 +35,19 @@ internal fun subscriptionFeedWrite(
now: Long,
windowMs: Long,
maxPerChannel: Int,
reelIds: Set<String> = emptySet(),
): SubscriptionFeedWrite {
val cached = cachedSlice.map { if (!it.isShort && it.id in reelIds) it.copy(isShort = true) else it }
val priorById =
cachedSlice
cached
.filter { it.id.isNotBlank() }
.groupBy { it.id }
.mapValues { (_, candidates) -> SubscriptionFeedMerger.mergeDuplicates(candidates, now) }
val rows =
SubscriptionFeedMerger
.mergeSubscriptionFeed(
freshVideos = freshVideos.map { fresh -> fresh.preservingEnrichedMetadata(priorById[fresh.id]) },
cachedVideos = cachedSlice,
cachedVideos = cached,
now = now,
windowMs = windowMs,
maxItems = Int.MAX_VALUE,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.github.aedev.flow.data.subscriptions

import io.github.aedev.flow.data.local.entity.SubscriptionFeedEntity
import io.github.aedev.flow.data.model.Video

/**
* The stored reel verdicts a refresh may reuse instead of asking a channel's Shorts tab again.
Expand All @@ -17,3 +18,20 @@ internal fun trustedReelVerdicts(
rows
.filter { row -> row.cachedAt <= (lastFeedFetchAt[row.channelId] ?: 0L) }
.associate { it.videoId to it.isShort }

/**
* Classifies a channel's RSS slice without asking its Shorts tab, or returns null when some entry
* has no verdict. The feed's own link outranks a stored verdict, so a row filed wrong by an older
* pass is repaired as soon as its entry is read again.
*/
internal fun classifiedFromVerdicts(
videos: List<Video>,
linkVerdicts: Map<String, Boolean>,
storedVerdicts: Map<String, Boolean>,
): List<Video>? {
if (videos.isEmpty()) return null
return videos.map { video ->
val isShort = linkVerdicts[video.id] ?: storedVerdicts[video.id] ?: return null
video.copy(isShort = isShort)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import kotlinx.serialization.json.JsonObject

private const val LOCKUP_CONTENT_TYPE_COURSE = "LOCKUP_CONTENT_TYPE_COURSE"

// A channel's Home tab lists the reels it collaborated on as plain video lockups of this type.
private const val LOCKUP_CONTENT_TYPE_SHORT = "LOCKUP_CONTENT_TYPE_SHORT"

internal fun JsonObject.toLockupItem(owner: FeedItemOwner): FeedItem? {
val contentId = this["contentId"].stringOrNull()?.takeIf(String::isNotBlank) ?: return null
val metadata = this["metadata"].objectOrNull()?.get("lockupMetadataViewModel").objectOrNull()
Expand Down Expand Up @@ -66,7 +69,10 @@ internal fun JsonObject.toLockupItem(owner: FeedItemOwner): FeedItem? {

else -> {
FeedItem.VideoItem(
lockupVideo(contentId, title, parts, badges, itemOwner, collaborators).copy(membersOnlyText = membersOnly),
lockupVideo(contentId, title, parts, badges, itemOwner, collaborators).copy(
membersOnlyText = membersOnly,
isShort = contentType == LOCKUP_CONTENT_TYPE_SHORT,
),
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,13 @@ class SubscriptionCheckWorker(
val latestVideo = feed.entries.firstOrNull() ?: return emptyList()
if (subscription.lastVideoId == latestVideo.videoId) return emptyList()

val reelVideoIds = dependencies.channelReelIndex().reelIds(subscription.channelId).orEmpty()
val linkedReelIds = feed.entries.filter { it.isShort == true }.mapTo(HashSet()) { it.videoId }
val reelVideoIds =
if (feed.entries.all { it.isShort != null }) {
linkedReelIds
} else {
linkedReelIds + dependencies.channelReelIndex().reelIds(subscription.channelId).orEmpty()
}

// The channel moved on, so hand the whole page to the feed cache. Rows land without a
// duration — the feed's on-demand enrichment fills that in, and the next full refresh of
Expand Down
Loading
Loading