merge: bug/channel-details-refresh into E28
# Conflicts: # frontend/src/components/ChannelPage.tsx
This commit is contained in:
@@ -28,7 +28,7 @@ from app.models import (
|
|||||||
from app.routes._common import get_or_404, not_found, owned_or_404, youtube_refusal
|
from app.routes._common import get_or_404, not_found, owned_or_404, youtube_refusal
|
||||||
from app.sync.explore import explore_ingest_page
|
from app.sync.explore import explore_ingest_page
|
||||||
from app.sync.runner import backfill_channel_now, run_recent_backfill
|
from app.sync.runner import backfill_channel_now, run_recent_backfill
|
||||||
from app.sync.subscriptions import apply_channel_details
|
from app.sync.subscriptions import apply_channel_details, details_stale
|
||||||
from app.utils import iso
|
from app.utils import iso
|
||||||
from app.youtube.client import YouTubeClient, YouTubeError
|
from app.youtube.client import YouTubeClient, YouTubeError
|
||||||
from app.refusal import refused
|
from app.refusal import refused
|
||||||
@@ -297,10 +297,13 @@ def channel_detail(
|
|||||||
relationship to it (subscribed / explored) and how many of its videos are in the catalog.
|
relationship to it (subscribed / explored) and how many of its videos are in the catalog.
|
||||||
Lazily fills the About metadata (view count, join date, banner, links) the first time it's
|
Lazily fills the About metadata (view count, join date, banner, links) the first time it's
|
||||||
needed — a stub, or a channel last enriched before those fields existed (published_at is a
|
needed — a stub, or a channel last enriched before those fields existed (published_at is a
|
||||||
reliable sentinel: every real channel has a creation date). The enrich uses the API key
|
reliable sentinel: every real channel has a creation date) — and refreshes it once it is stale
|
||||||
(1 unit, cached after), so it's skipped for the quota-less demo account."""
|
(`DETAILS_MAX_AGE`), which is how a channel nobody subscribes to (an explored one, outside the
|
||||||
|
subscription import) gets a banner that still resolves. The enrich uses the API key (1 unit,
|
||||||
|
cached after), so it's skipped for the quota-less demo account."""
|
||||||
channel = get_or_404(db, Channel, channel_id, what="channel")
|
channel = get_or_404(db, Channel, channel_id, what="channel")
|
||||||
if channel.published_at is None and not user.is_demo:
|
stale = channel.published_at is None or details_stale(channel, datetime.now(timezone.utc))
|
||||||
|
if stale and not user.is_demo:
|
||||||
try:
|
try:
|
||||||
with (
|
with (
|
||||||
quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER),
|
quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER),
|
||||||
|
|||||||
@@ -1,8 +1,8 @@
|
|||||||
"""Import the authenticated user's YouTube subscriptions and channel metadata."""
|
"""Import the authenticated user's YouTube subscriptions and channel metadata."""
|
||||||
|
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
|
|
||||||
from sqlalchemy import select
|
from sqlalchemy import or_, select
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from app import logs
|
from app import logs
|
||||||
@@ -47,6 +47,27 @@ def _external_links(branding: dict) -> list | None:
|
|||||||
return out or None
|
return out or None
|
||||||
|
|
||||||
|
|
||||||
|
# How old a channel's metadata may get before it is fetched again. Banners, avatars, descriptions and
|
||||||
|
# counts change on YouTube, and an old banner URL stops resolving at all — without a refresh a channel
|
||||||
|
# kept whatever it had the day it was first seen. A week keeps counts roughly current at almost no
|
||||||
|
# cost: `channels.list` is 1 quota unit per 50 channels, and only the stale ones are asked for.
|
||||||
|
DETAILS_MAX_AGE = timedelta(days=7)
|
||||||
|
|
||||||
|
|
||||||
|
def details_stale(channel: Channel, now: datetime) -> bool:
|
||||||
|
"""Whether `channel`'s metadata is missing or older than DETAILS_MAX_AGE (see `details_stale_clause`)."""
|
||||||
|
synced = channel.details_synced_at
|
||||||
|
return synced is None or synced < now - DETAILS_MAX_AGE
|
||||||
|
|
||||||
|
|
||||||
|
def details_stale_clause(now: datetime):
|
||||||
|
"""`details_stale` as a WHERE clause, so the batch import and a single channel agree on "stale"."""
|
||||||
|
return or_(
|
||||||
|
Channel.details_synced_at.is_(None),
|
||||||
|
Channel.details_synced_at < now - DETAILS_MAX_AGE,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def apply_channel_details(db: Session, items: list[dict]) -> None:
|
def apply_channel_details(db: Session, items: list[dict]) -> None:
|
||||||
now = datetime.now(timezone.utc)
|
now = datetime.now(timezone.utc)
|
||||||
for item in items:
|
for item in items:
|
||||||
@@ -92,7 +113,7 @@ def should_prune(fetched_count: int, existing_count: int) -> bool:
|
|||||||
def import_subscriptions(db: Session, user: User) -> dict:
|
def import_subscriptions(db: Session, user: User) -> dict:
|
||||||
"""Pull all subscriptions for `user`, upsert channels + subscription links, prune
|
"""Pull all subscriptions for `user`, upsert channels + subscription links, prune
|
||||||
channels the user has unsubscribed from on YouTube, and fetch channel details for
|
channels the user has unsubscribed from on YouTube, and fetch channel details for
|
||||||
any channel we don't have details for yet."""
|
any channel whose details are missing or stale (`DETAILS_MAX_AGE`)."""
|
||||||
fetched: dict[str, dict] = {}
|
fetched: dict[str, dict] = {}
|
||||||
new_channels = 0
|
new_channels = 0
|
||||||
|
|
||||||
@@ -145,12 +166,12 @@ def import_subscriptions(db: Session, user: User) -> dict:
|
|||||||
removed += 1
|
removed += 1
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|
||||||
# Fetch details for channels that don't have them yet.
|
# Fetch details for channels that don't have them yet, or had them too long ago.
|
||||||
need_details = (
|
need_details = (
|
||||||
db.execute(
|
db.execute(
|
||||||
select(Channel).where(
|
select(Channel).where(
|
||||||
Channel.id.in_(fetched.keys()),
|
Channel.id.in_(fetched.keys()),
|
||||||
Channel.details_synced_at.is_(None),
|
details_stale_clause(datetime.now(timezone.utc)),
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
.scalars()
|
.scalars()
|
||||||
|
|||||||
@@ -0,0 +1,107 @@
|
|||||||
|
"""A channel's metadata is fetched again once it is stale, not only the first time it is seen.
|
||||||
|
|
||||||
|
Banners, avatars, descriptions and counts change on YouTube, and an old banner URL stops resolving:
|
||||||
|
a channel that kept the metadata of the day it was first imported showed a dead banner forever. The
|
||||||
|
subscription import and the channel page both refresh what is older than `DETAILS_MAX_AGE`, and
|
||||||
|
leave fresh metadata alone (each refresh spends quota). DB lane.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from datetime import datetime, timedelta, timezone
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from app.models import Channel, Subscription
|
||||||
|
from app.routes import channels
|
||||||
|
from app.sync import subscriptions
|
||||||
|
from app.sync.subscriptions import DETAILS_MAX_AGE, import_subscriptions
|
||||||
|
|
||||||
|
NOW = datetime.now(timezone.utc)
|
||||||
|
FRESH = NOW - timedelta(days=1)
|
||||||
|
STALE = NOW - DETAILS_MAX_AGE - timedelta(hours=1)
|
||||||
|
|
||||||
|
|
||||||
|
def _item(cid: str) -> dict:
|
||||||
|
return {
|
||||||
|
"id": cid,
|
||||||
|
"snippet": {"title": f"{cid} renamed", "publishedAt": "2010-01-01T00:00:00Z"},
|
||||||
|
"brandingSettings": {"image": {"bannerExternalUrl": f"https://yt3.example/{cid}-new"}},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _youtube(asked: list[list[str]], subscribed: list[str]):
|
||||||
|
class _Client:
|
||||||
|
def __init__(self, db, user):
|
||||||
|
pass
|
||||||
|
|
||||||
|
def __enter__(self):
|
||||||
|
return self
|
||||||
|
|
||||||
|
def __exit__(self, *exc):
|
||||||
|
return False
|
||||||
|
|
||||||
|
def iter_subscriptions(self):
|
||||||
|
for cid in subscribed:
|
||||||
|
yield {"channel_id": cid, "title": cid, "yt_subscription_id": f"sub-{cid}"}
|
||||||
|
|
||||||
|
def get_channels(self, ids):
|
||||||
|
asked.append(sorted(ids))
|
||||||
|
return [_item(cid) for cid in ids]
|
||||||
|
|
||||||
|
return _Client
|
||||||
|
|
||||||
|
|
||||||
|
def _channel(db, cid: str, synced: datetime | None) -> None:
|
||||||
|
db.add(
|
||||||
|
Channel(
|
||||||
|
id=cid,
|
||||||
|
title=cid,
|
||||||
|
banner_url=f"https://yt3.example/{cid}-old",
|
||||||
|
published_at=datetime(2010, 1, 1, tzinfo=timezone.utc),
|
||||||
|
details_synced_at=synced,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_import_refreshes_stale_and_missing_details_and_leaves_fresh_ones(
|
||||||
|
db, make_user, monkeypatch
|
||||||
|
):
|
||||||
|
user = make_user()
|
||||||
|
_channel(db, "UCfresh", FRESH)
|
||||||
|
_channel(db, "UCstale", STALE)
|
||||||
|
_channel(db, "UCnever", None)
|
||||||
|
db.flush()
|
||||||
|
for cid in ("UCfresh", "UCstale", "UCnever"):
|
||||||
|
db.add(Subscription(user_id=user.id, channel_id=cid))
|
||||||
|
db.commit()
|
||||||
|
asked: list[list[str]] = []
|
||||||
|
monkeypatch.setattr(
|
||||||
|
subscriptions, "YouTubeClient", _youtube(asked, ["UCfresh", "UCstale", "UCnever"])
|
||||||
|
)
|
||||||
|
|
||||||
|
result = import_subscriptions(db, user)
|
||||||
|
|
||||||
|
assert asked == [["UCnever", "UCstale"]]
|
||||||
|
assert result["channels_detailed"] == 2
|
||||||
|
stale = db.get(Channel, "UCstale")
|
||||||
|
assert stale is not None
|
||||||
|
assert stale.banner_url == "https://yt3.example/UCstale-new"
|
||||||
|
assert stale.details_synced_at is not None and stale.details_synced_at > STALE
|
||||||
|
fresh = db.get(Channel, "UCfresh")
|
||||||
|
assert fresh is not None and fresh.banner_url == "https://yt3.example/UCfresh-old"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(("synced", "refreshed"), [(STALE, True), (FRESH, False)])
|
||||||
|
def test_the_channel_page_refreshes_a_stale_channel_only(
|
||||||
|
db, make_user, monkeypatch, synced, refreshed
|
||||||
|
):
|
||||||
|
user = make_user()
|
||||||
|
_channel(db, "UCpage", synced)
|
||||||
|
db.commit()
|
||||||
|
asked: list[list[str]] = []
|
||||||
|
monkeypatch.setattr(channels, "YouTubeClient", _youtube(asked, []))
|
||||||
|
|
||||||
|
body = channels.channel_detail("UCpage", user=user, db=db)
|
||||||
|
|
||||||
|
assert asked == ([["UCpage"]] if refreshed else [])
|
||||||
|
suffix = "new" if refreshed else "old"
|
||||||
|
assert body["banner_url"] == f"https://yt3.example/UCpage-{suffix}"
|
||||||
@@ -0,0 +1,89 @@
|
|||||||
|
// @vitest-environment jsdom
|
||||||
|
//
|
||||||
|
// The channel banner is served from Google's image CDN (yt3.googleusercontent.com), which refuses a
|
||||||
|
// request that carries a Referer from our origin — every time, not only under load (measured
|
||||||
|
// 2026-09-26: 6/6 refused with the page's referrer or its bare origin, 8/8 served without one). So the
|
||||||
|
// banner <img> must send none, as ui/Avatar does for the same CDN; with a referrer the banner never
|
||||||
|
// showed and the page fell back to the blank bar.
|
||||||
|
import type { ReactNode } from "react";
|
||||||
|
import { describe, it, expect, vi, beforeAll, afterEach } from "vitest";
|
||||||
|
import { render, cleanup, waitFor } from "@testing-library/react";
|
||||||
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||||
|
|
||||||
|
vi.mock("react-i18next", async (importOriginal) => ({
|
||||||
|
...(await importOriginal<typeof import("react-i18next")>()),
|
||||||
|
useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
|
||||||
|
}));
|
||||||
|
vi.mock("./navigationContext", () => ({
|
||||||
|
useNavigationActions: () => ({ closeChannel: vi.fn(), openChannel: vi.fn() }),
|
||||||
|
}));
|
||||||
|
vi.mock("./confirmContext", () => ({ useConfirm: () => async () => true }));
|
||||||
|
vi.mock("./feedFiltersContext", () => ({
|
||||||
|
useFeedFilters: () => ({ includeNormal: true, includeShorts: false, includeLive: false }),
|
||||||
|
}));
|
||||||
|
vi.mock("./Feed", () => ({ default: () => null }));
|
||||||
|
vi.mock("./ChannelAbout", () => ({ default: () => null }));
|
||||||
|
// The page's chrome portals into the shell's fixed toolbar band, which this test has no shell for.
|
||||||
|
vi.mock("./ui/PageShell", async (importOriginal) => ({
|
||||||
|
...(await importOriginal<typeof import("./ui/PageShell")>()),
|
||||||
|
PageToolbar: ({ children }: { children?: ReactNode }) => <>{children}</>,
|
||||||
|
}));
|
||||||
|
|
||||||
|
import ChannelPage from "./ChannelPage";
|
||||||
|
import { api, type ChannelDetail, type Me } from "../lib/api";
|
||||||
|
|
||||||
|
beforeAll(() => {
|
||||||
|
class Stub {
|
||||||
|
observe() {}
|
||||||
|
unobserve() {}
|
||||||
|
disconnect() {}
|
||||||
|
takeRecords() {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver;
|
||||||
|
globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver;
|
||||||
|
});
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
const CH = "UCqoAEDirJPjEUFcF2FklnBA";
|
||||||
|
|
||||||
|
describe("ChannelPage — the banner", () => {
|
||||||
|
it("is requested without a referrer, which Google's image CDN refuses", async () => {
|
||||||
|
vi.spyOn(api, "channelDetail").mockResolvedValue({
|
||||||
|
id: CH,
|
||||||
|
title: "StarTalk",
|
||||||
|
banner_url: "https://yt3.googleusercontent.com/banner",
|
||||||
|
subscribed: true,
|
||||||
|
blocked: false,
|
||||||
|
hidden: false,
|
||||||
|
deep_requested: false,
|
||||||
|
backfill_done: true,
|
||||||
|
stored_videos: 10,
|
||||||
|
} as unknown as ChannelDetail);
|
||||||
|
vi.spyOn(api, "tags").mockResolvedValue([] as never);
|
||||||
|
const { container } = render(
|
||||||
|
<QueryClientProvider
|
||||||
|
client={new QueryClient({ defaultOptions: { queries: { retry: false } } })}
|
||||||
|
>
|
||||||
|
<ChannelPage
|
||||||
|
channelId={CH}
|
||||||
|
initialName="StarTalk"
|
||||||
|
me={{ id: 1, can_write: true } as unknown as Me}
|
||||||
|
view="cards"
|
||||||
|
setView={() => {}}
|
||||||
|
onShowInFeed={() => {}}
|
||||||
|
/>
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
const banner = await waitFor(() => {
|
||||||
|
const img = container.querySelector<HTMLImageElement>('img[src^="https://yt3."]');
|
||||||
|
expect(img).not.toBeNull();
|
||||||
|
return img!;
|
||||||
|
});
|
||||||
|
expect(banner.getAttribute("referrerpolicy")).toBe("no-referrer");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -81,11 +81,11 @@ export default function ChannelPage({
|
|||||||
// where the channelScoped Feed portals ITS toolbar (via the overridden PageToolbarSlot below), so
|
// where the channelScoped Feed portals ITS toolbar (via the overridden PageToolbarSlot below), so
|
||||||
// the toolbar lands under the tabs inside the fixed band rather than in the scroller.
|
// the toolbar lands under the tabs inside the fixed band rather than in the scroller.
|
||||||
const [chromeSlot, setChromeSlot] = useState<HTMLDivElement | null>(null);
|
const [chromeSlot, setChromeSlot] = useState<HTMLDivElement | null>(null);
|
||||||
// The banner URL is valid, but googleusercontent intermittently throttles it when the channel
|
// The banner comes from Google's image CDN, which refuses any request carrying a Referer from our
|
||||||
// page fires the banner + avatar + ~16 thumbnails in one burst — the request is dropped and the
|
// origin — every time, not only in a burst (measured 2026-09-26) — so the <img> sends none
|
||||||
// browser then NEGATIVE-CACHES the miss, so the banner stays broken even though a retry would
|
// (`referrerPolicy`, as ui/Avatar). A load can still drop, and the browser NEGATIVE-CACHES the miss,
|
||||||
// succeed. So on error we retry a few times with a cache-buster (forcing a fresh request), and
|
// so on error we retry a few times with a cache-buster (forcing a fresh request), and only fall
|
||||||
// only fall back to the blank bar if it keeps failing (a genuinely dead URL — rare).
|
// back to the blank bar if it keeps failing (a genuinely dead URL — rare).
|
||||||
const MAX_BANNER_RETRIES = 3;
|
const MAX_BANNER_RETRIES = 3;
|
||||||
const [bannerAttempt, setBannerAttempt] = useState(0);
|
const [bannerAttempt, setBannerAttempt] = useState(0);
|
||||||
// The banner source that has actually loaded. Until then the <img> is invisible: an attempt that
|
// The banner source that has actually loaded. Until then the <img> is invisible: an attempt that
|
||||||
@@ -338,6 +338,7 @@ export default function ChannelPage({
|
|||||||
// browser's negative cache (googleusercontent accepts an extra query param).
|
// browser's negative cache (googleusercontent accepts an extra query param).
|
||||||
key={bannerAttempt}
|
key={bannerAttempt}
|
||||||
src={bannerSrc}
|
src={bannerSrc}
|
||||||
|
referrerPolicy="no-referrer"
|
||||||
alt=""
|
alt=""
|
||||||
className={`w-full h-full object-cover object-center ${
|
className={`w-full h-full object-cover object-center ${
|
||||||
bannerLoaded === bannerSrc ? "" : "opacity-0"
|
bannerLoaded === bannerSrc ? "" : "opacity-0"
|
||||||
|
|||||||
@@ -3,10 +3,11 @@ import { useState } from "react";
|
|||||||
// Avatar image with a graceful fallback.
|
// Avatar image with a graceful fallback.
|
||||||
//
|
//
|
||||||
// Uses referrerPolicy="no-referrer" because Google's avatar CDNs
|
// Uses referrerPolicy="no-referrer" because Google's avatar CDNs
|
||||||
// (yt3.ggpht.com / lh3.googleusercontent.com) rate-limit bursts of
|
// (yt3.ggpht.com / lh3.googleusercontent.com) refuse referrer-bearing requests
|
||||||
// referrer-bearing requests — when a feed page fires dozens of avatar loads at
|
// from our origin — yt3 refuses even a single load, not only a burst (measured
|
||||||
// once, a random subset gets throttled (HTTP 429) and the browser shows its
|
// 2026-09-26) — and the browser shows its broken-image icon. Dropping the
|
||||||
// broken-image icon. Dropping the referrer makes those CDNs serve the images.
|
// referrer makes those CDNs serve the images. The channel banner, from the same
|
||||||
|
// CDN, does the same (ChannelPage).
|
||||||
// If a load still fails we render a neutral placeholder (optionally an initial)
|
// If a load still fails we render a neutral placeholder (optionally an initial)
|
||||||
// instead of the broken-image icon.
|
// instead of the broken-image icon.
|
||||||
export default function Avatar({
|
export default function Avatar({
|
||||||
|
|||||||
Reference in New Issue
Block a user