diff --git a/app/repositories/polymorphic.py b/app/repositories/polymorphic.py index b532cea..0e4ec9d 100644 --- a/app/repositories/polymorphic.py +++ b/app/repositories/polymorphic.py @@ -120,44 +120,57 @@ async def sync_artwork( .scalars() .all() ) - existing_by_asset_id = { + by_asset_id: dict[str, Artwork] = { row.provider_asset_id: row for row in existing if row.provider_asset_id is not None } + # The incoming batch is authoritative for which image(s) are primary + # this sync — reset up front so a provider dropping its "primary" pick + # on an existing image actually clears here, not just adds. + for row in existing: + row.is_primary = False seen_asset_ids: set[str] = set() for art in artwork_list: art_type = art.artwork_type if art.artwork_type in _VALID_ARTWORK_TYPES else "poster" - matched = ( - existing_by_asset_id.get(art.provider_asset_id) if art.provider_asset_id else None - ) + asset_id = art.provider_asset_id + matched = by_asset_id.get(asset_id) if asset_id else None if matched is not None: - seen_asset_ids.add(art.provider_asset_id) # type: ignore[arg-type] matched.artwork_type = ArtworkType(art_type) matched.width = art.width matched.height = art.height matched.aspect_ratio = art.aspect_ratio matched.vote_average = art.vote_average - matched.is_primary = art.is_primary + # OR, never a plain overwrite: the *same* image commonly + # appears twice in one provider fetch (TMDb's selected + # poster_path also shows up in the general posters alternates + # list) — once any occurrence in this batch says "primary", + # a later, non-primary occurrence of that same image must not + # un-set it. + matched.is_primary = matched.is_primary or art.is_primary matched.original_url = art.url # local_path/checksum deliberately untouched — see docstring. else: - session.add( - Artwork( - entity_type=entity_type, - entity_id=entity_id, - artwork_type=ArtworkType(art_type), - width=art.width, - height=art.height, - aspect_ratio=art.aspect_ratio, - provider_id=provider_id, - provider_asset_id=art.provider_asset_id, - original_url=art.url, - vote_average=art.vote_average, - is_primary=art.is_primary, - ) + new_row = Artwork( + entity_type=entity_type, + entity_id=entity_id, + artwork_type=ArtworkType(art_type), + width=art.width, + height=art.height, + aspect_ratio=art.aspect_ratio, + provider_id=provider_id, + provider_asset_id=asset_id, + original_url=art.url, + vote_average=art.vote_average, + is_primary=art.is_primary, ) - if art.provider_asset_id: - seen_asset_ids.add(art.provider_asset_id) + session.add(new_row) + if asset_id: + # Register immediately so a second occurrence of this same + # new asset_id later in the batch updates this row instead + # of inserting a duplicate. + by_asset_id[asset_id] = new_row + if asset_id: + seen_asset_ids.add(asset_id) for row in existing: if row.provider_asset_id not in seen_asset_ids: diff --git a/tests/integration/test_artwork_sync.py b/tests/integration/test_artwork_sync.py index e8e4163..d4b9f60 100644 --- a/tests/integration/test_artwork_sync.py +++ b/tests/integration/test_artwork_sync.py @@ -160,3 +160,116 @@ async def test_sync_artwork_adds_new_rows_on_resync(db_session: AsyncSession): .all() ) assert {r.provider_asset_id for r in rows} == {"a.jpg", "c.jpg"} + + +@pytest.mark.asyncio +async def test_sync_artwork_duplicate_entry_in_same_batch_does_not_clobber_primary( + db_session: AsyncSession, +): + """Regression test: TMDb's images.posters/backdrops alternates list + commonly includes the *same* image as poster_path/backdrop_path, so one + normalize_movie() call can legitimately produce the same + provider_asset_id twice in one artwork_list -- once from the top-level + field (is_primary=True), once from the general alternates loop + (is_primary=False, the default). Whichever occurrence sync_artwork + processes second must not un-set what the first one set, regardless of + which order they appear in.""" + + provider = await make_provider(db_session) + entity_id = uuid.uuid4() + + # Primary occurrence first, duplicate non-primary occurrence second -- + # matches normalize_movie()'s actual ordering (poster_path is appended + # before the images.posters loop). + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[ + _artwork("shared.jpg", is_primary=True), + _artwork("shared.jpg", is_primary=False), + ], + ) + await db_session.commit() + + row = ( + await db_session.execute(select(Artwork).where(Artwork.entity_id == entity_id)) + ).scalar_one() + assert row.is_primary is True + + +@pytest.mark.asyncio +async def test_sync_artwork_duplicate_new_entry_does_not_create_two_rows( + db_session: AsyncSession, +): + """Same as above, but for an image that doesn't already have a row -- + the second occurrence in the batch must update the row the first + occurrence just created, not insert a second one for the same + provider_asset_id.""" + + provider = await make_provider(db_session) + entity_id = uuid.uuid4() + + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[ + _artwork("new.jpg", is_primary=False), + _artwork("new.jpg", is_primary=True), + ], + ) + await db_session.commit() + + rows = ( + ( + await db_session.execute( + select(Artwork).where( + Artwork.entity_id == entity_id, Artwork.provider_asset_id == "new.jpg" + ) + ) + ) + .scalars() + .all() + ) + assert len(rows) == 1 + assert rows[0].is_primary is True + + +@pytest.mark.asyncio +async def test_sync_artwork_clears_primary_when_provider_no_longer_marks_it( + db_session: AsyncSession, +): + provider = await make_provider(db_session) + entity_id = uuid.uuid4() + + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg", is_primary=True)], + ) + await db_session.flush() + + # Re-sync where the provider now picks a *different* image as primary. + await sync_artwork( + db_session, + entity_type="movie", + entity_id=entity_id, + provider_id=provider.id, + artwork_list=[_artwork("a.jpg", is_primary=False), _artwork("b.jpg", is_primary=True)], + ) + await db_session.commit() + + rows = { + r.provider_asset_id: r.is_primary + for r in ( + await db_session.execute(select(Artwork).where(Artwork.entity_id == entity_id)) + ) + .scalars() + .all() + } + assert rows == {"a.jpg": False, "b.jpg": True}