fix(sync): an empty answer the remote's own count contradicts is not believed, one level down
E32 review round 1 (B1, B2, B3, B8). - Plex watch reconcile: a section the server SAYS is empty (totalSize 0, nothing listed) is a complete listing. It read as "cut short" forever, so one empty library stopped un-watches from ever syncing. A listing with no total at all stays incomplete (the client now returns None for it instead of 0), and a whole scan that listed nothing still clears nothing. Tests now cover completeness across two sections, both orders. - Plex collections: no members from a collection whose listing counts some (childCount) keeps the membership we hold, in the full sync and in the targeted resync after a write. - YouTube playlists: no items for a playlist whose listing counts some (itemCount) keeps the mirror in the bulk pull, and is refused as retryable in "reset to YouTube" and in the push plan/merge, where it read as "YouTube deleted them all". The rule sits beside vanished(). - A playlist deleted on YouTube answered "reconnect your account"; it now says it is gone (errors.youtube.gone, HU+EN). - The pure 3-way merge moved to sync/playlist_merge.py; playlists.py is back under 800 lines and off the size list.
This commit is contained in:
@@ -4,14 +4,26 @@ Every mirror here reconciles: what the remote no longer lists is removed locally
|
||||
(`_prune_stale`) and the subscriptions (`should_prune`) had long refused to do that on an empty
|
||||
listing that contradicts what we hold — a 200 with no items is a blip far more often than a user
|
||||
who removed everything. Two mirrors did not: the Plex collections (a row carries the user-set
|
||||
`editable` flag, and every member its keys) and the YouTube playlist mirrors. These hold both.
|
||||
`editable` flag, and every member its keys) and the YouTube playlist mirrors. These hold both — and,
|
||||
one level down (E32 review), a collection's members and a playlist's items: an empty answer that
|
||||
contradicts the remote's own count is not believed either.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.models import Playlist, PlexCollection, PlexItem, PlexLibrary
|
||||
from app.models import (
|
||||
Channel,
|
||||
Playlist,
|
||||
PlaylistItem,
|
||||
PlexCollection,
|
||||
PlexItem,
|
||||
PlexLibrary,
|
||||
Video,
|
||||
)
|
||||
from app.plex import sync as plex_sync
|
||||
from app.sync import playlists as plsync
|
||||
from app.youtube.client import YouTubeError
|
||||
|
||||
|
||||
class _Plex:
|
||||
@@ -25,6 +37,9 @@ class _Plex:
|
||||
def collection_children(self, rk):
|
||||
return self._children.get(rk, [])
|
||||
|
||||
def metadata(self, rk):
|
||||
return next((c for c in self._collections if c.get("ratingKey") == rk), {})
|
||||
|
||||
|
||||
def _library_with_a_collection(db):
|
||||
lib = PlexLibrary(plex_key="4", title="Movies", kind="movie")
|
||||
@@ -62,9 +77,40 @@ def test_a_listing_with_something_in_it_still_prunes_what_it_no_longer_has(db):
|
||||
assert db.get(PlexItem, movie.id).collection_keys is None
|
||||
|
||||
|
||||
def test_no_members_from_a_collection_plex_counts_as_non_empty_keeps_its_members(db):
|
||||
lib, movie = _library_with_a_collection(db)
|
||||
stats = {"collections": 0}
|
||||
bond = {"ratingKey": "c1", "title": "Bond", "childCount": 2}
|
||||
plex_sync._sync_collections(db, _Plex([bond], {"c1": []}), lib, stats)
|
||||
db.commit()
|
||||
db.expire_all()
|
||||
assert db.get(PlexItem, movie.id).collection_keys == ["c1"]
|
||||
|
||||
|
||||
def test_a_collection_plex_counts_as_empty_does_lose_its_members(db):
|
||||
lib, movie = _library_with_a_collection(db)
|
||||
stats = {"collections": 0}
|
||||
bond = {"ratingKey": "c1", "title": "Bond", "childCount": 0}
|
||||
plex_sync._sync_collections(db, _Plex([bond], {"c1": []}), lib, stats)
|
||||
db.commit()
|
||||
db.expire_all()
|
||||
assert db.get(PlexItem, movie.id).collection_keys is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize(("count", "kept"), [(2, ["c1"]), (0, None)])
|
||||
def test_a_collection_resync_believes_an_empty_member_list_only_with_a_zero_count(db, count, kept):
|
||||
lib, movie = _library_with_a_collection(db)
|
||||
bond = {"ratingKey": "c1", "title": "Bond", "childCount": count}
|
||||
plex_sync.resync_collection(db, _Plex([bond], {"c1": []}), lib, "c1")
|
||||
db.commit()
|
||||
db.expire_all()
|
||||
assert db.get(PlexItem, movie.id).collection_keys == kept
|
||||
|
||||
|
||||
class _YT:
|
||||
def __init__(self, playlists: list[dict]):
|
||||
def __init__(self, playlists: list[dict], snippet: dict | None = None):
|
||||
self._playlists = playlists
|
||||
self._snippet = snippet
|
||||
|
||||
def __call__(self, *_):
|
||||
return self
|
||||
@@ -84,6 +130,12 @@ class _YT:
|
||||
def iter_my_playlist_video_ids(self, _ytid):
|
||||
return []
|
||||
|
||||
def get_playlist_snippet(self, _ytid):
|
||||
return self._snippet
|
||||
|
||||
def get_videos(self, _ids):
|
||||
return []
|
||||
|
||||
|
||||
def test_an_empty_playlist_answer_keeps_the_youtube_mirrors(db, make_user, monkeypatch):
|
||||
user = make_user()
|
||||
@@ -109,3 +161,72 @@ def test_a_playlist_answer_with_something_in_it_still_drops_a_vanished_mirror(
|
||||
db.execute(select(Playlist.yt_playlist_id).where(Playlist.source == "youtube")).scalars()
|
||||
)
|
||||
assert "PL1" not in ytids
|
||||
|
||||
|
||||
def _mirror_with_an_item(db, user, **kw):
|
||||
db.add(Channel(id="UC_pl", title="C"))
|
||||
db.flush()
|
||||
db.add(Video(id="v1", channel_id="UC_pl", title="One"))
|
||||
pl = Playlist(user_id=user.id, name="Mixes", yt_playlist_id="PL1", **kw)
|
||||
db.add(pl)
|
||||
db.flush()
|
||||
db.add(PlaylistItem(playlist_id=pl.id, video_id="v1", position=0))
|
||||
db.commit()
|
||||
return pl
|
||||
|
||||
|
||||
def _items(db, pl):
|
||||
db.expire_all()
|
||||
return (
|
||||
db.execute(select(PlaylistItem.video_id).where(PlaylistItem.playlist_id == pl.id))
|
||||
.scalars()
|
||||
.all()
|
||||
)
|
||||
|
||||
|
||||
def test_no_items_from_a_playlist_youtube_counts_as_non_empty_keeps_the_mirror_items(
|
||||
db, make_user, monkeypatch
|
||||
):
|
||||
user = make_user()
|
||||
pl = _mirror_with_an_item(db, user, source="youtube")
|
||||
listing = [{"id": "PL1", "title": "Mixes", "item_count": 1}]
|
||||
monkeypatch.setattr(plsync, "YouTubeClient", _YT(listing))
|
||||
plsync.pull_user_playlists(db, user)
|
||||
assert _items(db, pl) == ["v1"]
|
||||
|
||||
|
||||
def test_a_playlist_youtube_counts_as_empty_is_emptied(db, make_user, monkeypatch):
|
||||
user = make_user()
|
||||
pl = _mirror_with_an_item(db, user, source="youtube")
|
||||
listing = [{"id": "PL1", "title": "Mixes", "item_count": 0}]
|
||||
monkeypatch.setattr(plsync, "YouTubeClient", _YT(listing))
|
||||
plsync.pull_user_playlists(db, user)
|
||||
assert _items(db, pl) == []
|
||||
|
||||
|
||||
def test_a_reset_to_youtube_refuses_a_blank_listing_and_keeps_the_items(db, make_user, monkeypatch):
|
||||
user = make_user()
|
||||
pl = _mirror_with_an_item(db, user, source="youtube")
|
||||
monkeypatch.setattr(plsync, "YouTubeClient", _YT([], {"title": "Mixes", "item_count": 1}))
|
||||
with pytest.raises(YouTubeError) as err:
|
||||
plsync.repull_playlist(db, user, pl)
|
||||
db.rollback()
|
||||
assert err.value.reason == "emptyAnswer"
|
||||
assert _items(db, pl) == ["v1"]
|
||||
|
||||
|
||||
def test_a_reset_of_a_deleted_playlist_says_it_is_gone(db, make_user, monkeypatch):
|
||||
user = make_user()
|
||||
pl = _mirror_with_an_item(db, user, source="youtube")
|
||||
monkeypatch.setattr(plsync, "YouTubeClient", _YT([], None))
|
||||
with pytest.raises(YouTubeError) as err:
|
||||
plsync.repull_playlist(db, user, pl)
|
||||
assert err.value.reason == "playlistNotFound"
|
||||
|
||||
|
||||
def test_a_push_plan_refuses_a_blank_listing_rather_than_reading_it_as_all_deleted(db, make_user):
|
||||
user = make_user()
|
||||
pl = _mirror_with_an_item(db, user, source="local")
|
||||
with pytest.raises(YouTubeError) as err:
|
||||
plsync.plan_merge(db, user, pl, live=([], {"title": "Mixes", "item_count": 1}))
|
||||
assert err.value.reason == "emptyAnswer"
|
||||
|
||||
@@ -12,8 +12,8 @@ import pytest
|
||||
|
||||
from app.models import Channel, Playlist, PlaylistItem, Video
|
||||
from app.sync import playlists as pl_mod
|
||||
from app.sync.playlist_merge import merge_plan
|
||||
from app.sync.playlists import (
|
||||
merge_plan,
|
||||
merge_playlist,
|
||||
parse_baseline,
|
||||
parse_unavailable,
|
||||
|
||||
@@ -81,6 +81,9 @@ def test_a_section_page_asks_for_guids_and_reports_the_server_total(plex):
|
||||
|
||||
plex.routes["/library/sections/7/all"] = {"Metadata": [], "size": 0}
|
||||
assert plex.section_items("7", 2) == ([], 0)
|
||||
# No total at all is not "0": the caller must not read it as a complete, empty section.
|
||||
plex.routes["/library/sections/7/all"] = {"Metadata": []}
|
||||
assert plex.section_items("7", 2) == ([], None)
|
||||
|
||||
|
||||
def test_related_flattens_the_hubs_and_degrades_to_empty(plex):
|
||||
|
||||
@@ -13,11 +13,13 @@ from app.plex import watch_sync
|
||||
|
||||
class _FakePlex:
|
||||
"""A PlexClient stand-in: one movie section whose listing reports `total` (more than it hands
|
||||
over = a walk cut short), and a record of every write the reconcile sends back to Plex."""
|
||||
over = a walk cut short), plus any `more` sections as `(type, leaves, total)` — listed AFTER
|
||||
the movies — and a record of every write the reconcile sends back to Plex."""
|
||||
|
||||
def __init__(self, leaves: list[dict], total: int | None = None):
|
||||
self.leaves = leaves
|
||||
self.total = len(leaves) if total is None else total
|
||||
def __init__(self, leaves: list[dict], total: int | None = None, more=()):
|
||||
self.listings = {"1": ("movie", leaves, len(leaves) if total is None else total)}
|
||||
for i, (stype, extra, extra_total) in enumerate(more, start=2):
|
||||
self.listings[str(i)] = (stype, extra, extra_total)
|
||||
self.calls: list[tuple] = []
|
||||
|
||||
def __enter__(self):
|
||||
@@ -27,10 +29,11 @@ class _FakePlex:
|
||||
return False
|
||||
|
||||
def sections(self):
|
||||
return [{"type": "movie", "key": "1"}]
|
||||
return [{"type": stype, "key": key} for key, (stype, _l, _t) in self.listings.items()]
|
||||
|
||||
def section_items(self, key, item_type, start, size):
|
||||
return self.leaves[start : start + size], self.total
|
||||
_stype, leaves, total = self.listings[key]
|
||||
return leaves[start : start + size], total
|
||||
|
||||
def scrobble(self, rk):
|
||||
self.calls.append(("scrobble", rk))
|
||||
@@ -136,6 +139,45 @@ def test_a_scan_cut_short_clears_nothing(db, monkeypatch, world):
|
||||
assert out["cleared"] == 0 and out["incomplete"] == 1
|
||||
|
||||
|
||||
_LISTED = [
|
||||
{"ratingKey": "m3"},
|
||||
{"ratingKey": "m4", "viewCount": 1, "lastViewedAt": 1_700_000_000},
|
||||
{"ratingKey": "m5", "viewOffset": 120_000, "lastViewedAt": 1_700_000_000},
|
||||
]
|
||||
|
||||
|
||||
def test_a_genuinely_empty_second_section_still_lets_the_reconcile_clear(db, monkeypatch, world):
|
||||
# The server SAYS the shows section holds 0: that is a complete listing, not a hiccup — before
|
||||
# the E32 review it read as "incomplete" forever and un-watches never synced again.
|
||||
state, rows = world
|
||||
_local_states(state)
|
||||
db.commit()
|
||||
out = _run(db, monkeypatch, _FakePlex(_LISTED, more=[("show", [], 0)]))
|
||||
assert "m1" not in rows()
|
||||
assert out["cleared"] == 1 and "incomplete" not in out
|
||||
|
||||
|
||||
def test_a_cut_short_section_is_not_rescued_by_a_complete_one_after_it(db, monkeypatch, world):
|
||||
# Completeness is every section's, not the last one's: movies cut short (5 said, 3 handed),
|
||||
# then a complete shows listing — m1 may simply not have been reached.
|
||||
state, rows = world
|
||||
_local_states(state)
|
||||
db.commit()
|
||||
shows = [{"ratingKey": "s1"}]
|
||||
out = _run(db, monkeypatch, _FakePlex(_LISTED, total=5, more=[("show", shows, 1)]))
|
||||
assert rows()["m1"] == ("watched", 0, True)
|
||||
assert out["cleared"] == 0 and out["incomplete"] == 1
|
||||
|
||||
|
||||
def test_a_listing_without_a_total_is_never_complete(db, monkeypatch, world):
|
||||
state, rows = world
|
||||
_local_states(state)
|
||||
db.commit()
|
||||
out = _run(db, monkeypatch, _FakePlex(_LISTED, more=[("show", [], None)]))
|
||||
assert rows()["m1"] == ("watched", 0, True)
|
||||
assert out["cleared"] == 0 and out["incomplete"] == 1
|
||||
|
||||
|
||||
def test_an_empty_listing_is_not_a_library_of_unwatched_titles(db, monkeypatch, world):
|
||||
state, rows = world
|
||||
state("m1", status="watched", synced_to_plex=True)
|
||||
|
||||
@@ -35,7 +35,6 @@ BIG_MODULES: dict[str, int] = {
|
||||
"app/routes/admin.py": 864,
|
||||
"app/routes/auth.py": 1353,
|
||||
"app/routes/downloads.py": 1237,
|
||||
"app/sync/playlists.py": 843,
|
||||
}
|
||||
|
||||
COMPLEX: dict[str, list[int]] = {
|
||||
|
||||
@@ -27,6 +27,9 @@ from app.youtube.client import YouTubeError, refusal_reason
|
||||
(YouTubeError("No refresh token; user must re-authenticate"), "reconnect"),
|
||||
(YouTubeError("m", reason="transport"), "unavailable"),
|
||||
(YouTubeError("m", reason="backendError", path="subscriptions"), "unavailable"),
|
||||
# A playlist deleted on YouTube: neither a retry nor a reconnect helps (it said "reconnect").
|
||||
(YouTubeError("m", reason="playlistNotFound", path="playlists"), "gone"),
|
||||
(YouTubeError("m", reason="emptyAnswer", path="playlistItems"), "unavailable"),
|
||||
],
|
||||
)
|
||||
def test_a_failure_is_sorted_by_what_the_user_can_do(exc, reason):
|
||||
|
||||
Reference in New Issue
Block a user