Skip to content

Commit d852299

Browse files
Spinnichclaude
andcommitted
fix(resources): fetch each cover once instead of once per size
get_cover() called _store_cover() once for CoverSize.SMALL and once for CoverSize.BIG with the same URL, and each call performed a complete fetch. The small cover has no distinct source: it is the large image downloaded in full and then resized in place, so the second request only re-retrieved bytes already on disk. Every cover cost twice the bandwidth, doubled the load on every provider, and on ScreenScraper spent a second request from the account's quota, since media is served from the same authenticated api2 endpoints as metadata lookups. _store_cover() now takes no size: it fetches once into big.png and derives small.png from it, the way store_artwork() already did. A missing small cover no longer triggers a fetch at all. get_cover() downloads only when overwriting or when the large cover is absent, and otherwise rebuilds the small one from the large one already on disk via _derive_small_cover(). That drops the half-written pair case from one request to none, and keeps a good large cover out of the blast radius of a download that might fail. The derive path resolves its source through _get_cover_path, so an uploaded big.jpg or a converted big.webp yields a matching small file rather than assuming a .png extension. Failure paths now clean up both destinations. Undecodable bytes are discarded rather than left on disk, where they satisfy cover_exists() and stop any later scan from refetching a working cover, and a discarded chroma-key placeholder also clears a small cover left by an earlier scan. _derive_small_cover() is the exception: it did not write those bytes, so a large cover it cannot decode is left in place and only the partial small one is dropped. Fixes #4102 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 960e18d commit d852299

2 files changed

Lines changed: 426 additions & 95 deletions

File tree

backend/handler/filesystem/resources_handler.py

Lines changed: 92 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -175,18 +175,20 @@ async def _discard_partial_file(self, relative_path: str) -> None:
175175
except OSError as exc:
176176
log.error(f"Unable to remove partial file {relative_path}: {str(exc)}")
177177

178-
async def _store_cover(
179-
self, entity: Rom | Collection, url_cover: str, size: CoverSize
180-
) -> None:
181-
"""Store roms resources in filesystem
178+
async def _store_cover(self, entity: Rom | Collection, url_cover: str) -> None:
179+
"""Fetch a cover once and write both sizes.
180+
181+
The small cover is a downscale of the large one, so fetching the same
182+
URL a second time would only re-retrieve bytes we already hold, and on
183+
ScreenScraper would spend a second request from the account's quota.
182184
183185
Args:
184-
fs_slug: short name of the platform
185-
rom_name: name of rom file
186+
entity: Rom or Collection object
186187
url_cover: url to get the cover
187-
size: size of the cover
188188
"""
189189
cover_file = f"{entity.fs_resources_path}/cover"
190+
big_path = f"{cover_file}/{CoverSize.BIG.value}.png"
191+
small_path = f"{cover_file}/{CoverSize.SMALL.value}.png"
190192
await self.make_directory(cover_file)
191193

192194
# Handle local-file URIs from metadata handlers (gamelist, LaunchBox)
@@ -196,22 +198,12 @@ async def _store_cover(
196198
if resolved is None or not await AnyioPath(resolved).exists():
197199
log.warning(f"Cover file not found: {url_cover}")
198200
return None
199-
dest_path = f"{cover_file}/{size.value}.png"
200-
# Small-size covers get resized in place, which would mutate
201-
# the user's source image if the destination were a hardlink.
202-
await self.copy_file(resolved, dest_path, allow_link=False)
203-
204-
if await self._discard_if_chroma_key(dest_path):
205-
return None
206-
207-
if ENABLE_SCHEDULED_CONVERT_IMAGES_TO_WEBP:
208-
self.image_converter.convert_to_webp(
209-
self.validate_path(f"{cover_file}/{size.value}.png"),
210-
force=True,
211-
)
201+
# Covers are rewritten in place by later scans, which would
202+
# mutate the user's source image through a hardlink.
203+
await self.copy_file(resolved, big_path, allow_link=False)
212204
except Exception as exc:
213205
log.error(f"Unable to copy cover file {url_cover}: {str(exc)}")
214-
await self._discard_partial_file(f"{cover_file}/{size.value}.png")
206+
await self._discard_partial_file(big_path)
215207
return None
216208
else:
217209
# Handle HTTP URLs
@@ -233,7 +225,7 @@ async def _store_cover(
233225
)
234226

235227
async with await self.write_file_streamed(
236-
path=cover_file, filename=f"{size.value}.png"
228+
path=cover_file, filename=f"{CoverSize.BIG.value}.png"
237229
) as f:
238230
if is_gzipped:
239231
# Content is gzipped, decompress it
@@ -249,43 +241,84 @@ async def _store_cover(
249241
await f.write(chunk)
250242

251243
downloaded = True
252-
253-
# Inspecting and re-encoding the file is local work, so it runs
254-
# once the provider's request slot has been handed back.
255-
if downloaded:
256-
if await self._discard_if_chroma_key(
257-
f"{cover_file}/{size.value}.png"
258-
):
259-
return None
260-
261-
if ENABLE_SCHEDULED_CONVERT_IMAGES_TO_WEBP:
262-
self.image_converter.convert_to_webp(
263-
self.validate_path(f"{cover_file}/{size.value}.png"),
264-
force=True,
265-
)
266244
except httpx.TransportError as exc:
267245
log.error(f"Unable to fetch cover at {url_cover}: {str(exc)}")
268-
await self._discard_partial_file(f"{cover_file}/{size.value}.png")
246+
await self._discard_partial_file(big_path)
269247
return None
270248
except OSError as exc:
271249
log.error(f"Unable to write cover for {url_cover}: {str(exc)}")
272-
await self._discard_partial_file(f"{cover_file}/{size.value}.png")
250+
await self._discard_partial_file(big_path)
273251
return None
274252

275-
if size == CoverSize.SMALL:
276-
try:
277-
image_path = self.validate_path(f"{cover_file}/{size.value}.png")
278-
with Image.open(image_path) as img:
279-
self.resize_cover_to_small(img, save_path=str(image_path))
253+
if not downloaded:
254+
return None
280255

281-
if ENABLE_SCHEDULED_CONVERT_IMAGES_TO_WEBP:
282-
self.image_converter.convert_to_webp(
283-
self.validate_path(f"{cover_file}/{size.value}.png"), force=True
284-
)
285-
except UnidentifiedImageError as exc:
286-
log.error(f"Unable to identify image {cover_file}: {str(exc)}")
256+
# Inspecting and re-encoding the file is local work, so it runs once the
257+
# provider's request slot has been handed back.
258+
try:
259+
if await self._discard_if_chroma_key(big_path):
260+
# A small cover left by an earlier scan would outlive the large
261+
# one it was derived from.
262+
await self._discard_partial_file(small_path)
287263
return None
288264

265+
with Image.open(self.validate_path(big_path)) as img:
266+
self.resize_cover_to_small(
267+
img, save_path=str(self.validate_path(small_path))
268+
)
269+
270+
if ENABLE_SCHEDULED_CONVERT_IMAGES_TO_WEBP:
271+
self.image_converter.convert_to_webp(
272+
self.validate_path(big_path), force=True
273+
)
274+
self.image_converter.convert_to_webp(
275+
self.validate_path(small_path), force=True
276+
)
277+
except UnidentifiedImageError as exc:
278+
# Undecodable bytes still satisfy cover_exists(), so keeping them
279+
# would stop every later scan from refetching a working cover.
280+
log.error(f"Unable to identify image {big_path}: {str(exc)}")
281+
for path in (big_path, small_path):
282+
await self._discard_partial_file(path)
283+
except OSError as exc:
284+
log.error(f"Unable to write cover for {url_cover}: {str(exc)}")
285+
for path in (big_path, small_path):
286+
await self._discard_partial_file(path)
287+
288+
async def _derive_small_cover(self, entity: Rom | Collection) -> None:
289+
"""Rebuild a missing small cover from the large one already on disk.
290+
291+
The small cover is a downscale of the large one, so a half-written pair
292+
is repaired locally instead of spending a request on bytes we hold.
293+
294+
Args:
295+
entity: Rom or Collection object
296+
"""
297+
path_cover_l = self._get_cover_path(entity, CoverSize.BIG)
298+
if not path_cover_l:
299+
return
300+
301+
path_cover_s = (
302+
f"{entity.fs_resources_path}/cover/"
303+
f"{CoverSize.SMALL.value}{Path(path_cover_l).suffix}"
304+
)
305+
306+
try:
307+
with Image.open(self.validate_path(path_cover_l)) as img:
308+
self.resize_cover_to_small(
309+
img, save_path=str(self.validate_path(path_cover_s))
310+
)
311+
312+
if ENABLE_SCHEDULED_CONVERT_IMAGES_TO_WEBP:
313+
self.image_converter.convert_to_webp(
314+
self.validate_path(path_cover_s), force=True
315+
)
316+
except (UnidentifiedImageError, OSError) as exc:
317+
# Unlike a fresh download, these bytes weren't written here, so the
318+
# large cover stays put and only the partial small one is dropped.
319+
log.error(f"Unable to resize cover {path_cover_l}: {str(exc)}")
320+
await self._discard_partial_file(path_cover_s)
321+
289322
def _get_cover_path(self, entity: Rom | Collection, size: CoverSize) -> str | None:
290323
"""Returns rom cover filesystem path adapted to frontend folder structure
291324
@@ -305,12 +338,16 @@ async def get_cover(
305338
if not entity:
306339
return None, None
307340

308-
# Download covers if URL provided and (overwriting or covers don't exist)
309-
if url_cover:
310-
if overwrite or not self.cover_exists(entity, CoverSize.SMALL):
311-
await self._store_cover(entity, url_cover, CoverSize.SMALL)
312-
if overwrite or not self.cover_exists(entity, CoverSize.BIG):
313-
await self._store_cover(entity, url_cover, CoverSize.BIG)
341+
has_cover_l = self.cover_exists(entity, CoverSize.BIG)
342+
has_cover_s = self.cover_exists(entity, CoverSize.SMALL)
343+
344+
# A single fetch writes both sizes
345+
if url_cover and (overwrite or not has_cover_l):
346+
await self._store_cover(entity, url_cover)
347+
elif has_cover_l and not has_cover_s:
348+
# Refetching to recover the small cover would overwrite a large one
349+
# that is already good, and lose it if the fetch fails.
350+
await self._derive_small_cover(entity)
314351

315352
# Return paths for existing covers
316353
path_cover_s = (

0 commit comments

Comments
 (0)