Fix real data loss: two catalog queries were silently truncating at 300 #2
3 changed files with 76 additions and 26 deletions
|
|
@ -96,15 +96,26 @@ export function wireTileButtons(container: ParentNode, onStart: (itemId: string)
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Renders `items` into `container` a chunk at a time, across animation
|
* Renders `items` into `container` a chunk at a time instead of one
|
||||||
* frames, instead of one `innerHTML` write for the whole list - a library
|
* `innerHTML` write for the whole list - a library grid can legitimately
|
||||||
* grid can hand back up to 300 tiles at once (CatalogService.getLibraryItems/
|
* hand back over a thousand tiles now that CatalogService.getLibraryItems/
|
||||||
* getArtists), and building + inserting/laying out ~300 posters in a
|
* getArtists no longer cap themselves (confirmed against the real server:
|
||||||
* single synchronous task is exactly what a "the page freezes while it
|
* one library alone has 1229 artists - see their own comments), and
|
||||||
* loads" report looks like on a slower device. Splitting the same HTML
|
* building + inserting/laying out that many in a single synchronous task
|
||||||
* across a few chunks keeps every individual task well under a frame's
|
* is exactly what a "the page freezes while it loads" report looks like
|
||||||
* budget, so the tab stays responsive (and the loading spinner keeps
|
* on a slower device. Splitting the same HTML across a few chunks keeps
|
||||||
* animating) while it works through the rest.
|
* every individual task well under a frame's budget, so the tab stays
|
||||||
|
* responsive (and the loading spinner keeps animating) while it works
|
||||||
|
* through the rest.
|
||||||
|
*
|
||||||
|
* Scheduled with `setTimeout(fn, 0)`, not `requestAnimationFrame` - rAF
|
||||||
|
* callbacks are deliberately paused by the browser for a backgrounded/
|
||||||
|
* hidden tab, which is a completely ordinary thing to do while a large
|
||||||
|
* library is still loading (switching tabs, minimizing the window) and
|
||||||
|
* would otherwise stall the render indefinitely, looking exactly like
|
||||||
|
* "some of the data is missing" rather than "still loading". setTimeout
|
||||||
|
* keeps running in a background tab (only throttled, never paused), so
|
||||||
|
* this always finishes on its own.
|
||||||
*
|
*
|
||||||
* Uses `insertAdjacentHTML`, not repeated `innerHTML +=` (which would
|
* Uses `insertAdjacentHTML`, not repeated `innerHTML +=` (which would
|
||||||
* re-parse and re-render everything already inserted on every chunk).
|
* re-parse and re-render everything already inserted on every chunk).
|
||||||
|
|
@ -120,7 +131,7 @@ export function renderTilesInChunks(container: HTMLElement, items: ItemDto[], ch
|
||||||
if (!chunk.length) return;
|
if (!chunk.length) return;
|
||||||
container.insertAdjacentHTML('beforeend', chunk.map(tileHtml).join(''));
|
container.insertAdjacentHTML('beforeend', chunk.map(tileHtml).join(''));
|
||||||
i += chunkSize;
|
i += chunkSize;
|
||||||
if (i < items.length) requestAnimationFrame(renderNextChunk);
|
if (i < items.length) setTimeout(renderNextChunk, 0);
|
||||||
}
|
}
|
||||||
renderNextChunk();
|
renderNextChunk();
|
||||||
}
|
}
|
||||||
|
|
@ -129,11 +140,14 @@ export function renderTilesInChunks(container: HTMLElement, items: ItemDto[], ch
|
||||||
* Wires a search <input> to show/hide `itemSelector` elements by name,
|
* Wires a search <input> to show/hide `itemSelector` elements by name,
|
||||||
* live as you type - client-side only, filtering whatever's already
|
* live as you type - client-side only, filtering whatever's already
|
||||||
* rendered rather than a server round trip per keystroke. Deliberately
|
* rendered rather than a server round trip per keystroke. Deliberately
|
||||||
* simple for that reason: a library page tops out at a few hundred items
|
* simple for that reason: even a library with well over a thousand items
|
||||||
* (see getLibraryItems's own Limit), well within "just filter the DOM"
|
* (see getLibraryItems/getArtists - no Limit any more, see their own
|
||||||
* territory - shared by views/library, views/artist, views/album so the
|
* comments) is still just a plain, already-in-the-DOM list of elements
|
||||||
* three catalog pages that asked for this all behave identically instead
|
* to a querySelectorAll/textContent check - "just filter the DOM" scales
|
||||||
* of drifting into three slightly different implementations.
|
* fine here regardless of count. Shared by views/library, views/artist,
|
||||||
|
* views/album so the three catalog pages that asked for this all behave
|
||||||
|
* identically instead of drifting into three slightly different
|
||||||
|
* implementations.
|
||||||
*
|
*
|
||||||
* `scope` should be wide enough to contain both the items themselves AND
|
* `scope` should be wide enough to contain both the items themselves AND
|
||||||
* a `.searchEmpty` notice element - typically the view's own `root`, not
|
* a `.searchEmpty` notice element - typically the view's own `root`, not
|
||||||
|
|
|
||||||
|
|
@ -2631,6 +2631,28 @@ mode a naive per-button-at-call-time wiring would have hit). **Not
|
||||||
verified**: no Emby login reachable from this sandbox to measure the
|
verified**: no Emby login reachable from this sandbox to measure the
|
||||||
actual before/after latency against a real, large library.
|
actual before/after latency against a real, large library.
|
||||||
|
|
||||||
|
**Correction, reported immediately after this shipped**: "the catalogs
|
||||||
|
are broken, not all the data is there". The `Limit: 300` both queries
|
||||||
|
above carried (`getArtists` gained one here; `getLibraryItems` already
|
||||||
|
had one, predating this section) turned out not to be the safe,
|
||||||
|
theoretical headroom it was assumed to be - confirmed directly against
|
||||||
|
the real server that this app's own "Musique" library has **1229**
|
||||||
|
distinct artists and "Films" has **421** movies, so both were silently
|
||||||
|
serving an incomplete grid, no error, no visible sign anything was
|
||||||
|
missing. Both `Limit`s removed outright, not raised to some other
|
||||||
|
guessed number - confirmed empirically that Emby returns everything
|
||||||
|
when `Limit` is omitted at all (`TotalRecordCount` matched
|
||||||
|
`Items.length` exactly for both the 421- and 1229-item cases), and the
|
||||||
|
render-side concern that motivated capping in the first place is
|
||||||
|
already `renderTilesInChunks`'s job regardless of how large the list
|
||||||
|
gets. That fix's own scheduling primitive needed a second look too:
|
||||||
|
`requestAnimationFrame` is deliberately paused by the browser for a
|
||||||
|
backgrounded/hidden tab - an ordinary thing to do while a now-possibly-
|
||||||
|
1000+-item list is still loading, and would have stalled the render
|
||||||
|
indefinitely, looking exactly like the same "data missing" symptom for
|
||||||
|
an unrelated reason. Switched to `setTimeout(fn, 0)`, which keeps
|
||||||
|
running (throttled, never paused) regardless of tab visibility.
|
||||||
|
|
||||||
## Fixing the listen-party queue's layout and its missing album art
|
## Fixing the listen-party queue's layout and its missing album art
|
||||||
|
|
||||||
Reported directly: the music bar's "File d'attente" popup looked broken,
|
Reported directly: the music bar's "File d'attente" popup looked broken,
|
||||||
|
|
|
||||||
|
|
@ -199,11 +199,13 @@ class CatalogServiceClass {
|
||||||
// structure, same reasoning as Shows -> Seasons -> Episodes above.
|
// structure, same reasoning as Shows -> Seasons -> Episodes above.
|
||||||
|
|
||||||
async getArtists(session: EmbySession, libraryId: string): Promise<ItemDto[]> {
|
async getArtists(session: EmbySession, libraryId: string): Promise<ItemDto[]> {
|
||||||
// Limit added for the same reason as getLibraryItems's own - unbounded
|
// No Limit, deliberately - see getLibraryItems's own comment below for
|
||||||
// on a large music library, this used to hand back every artist at
|
// why one very nearly shipped here and got pulled: confirmed directly
|
||||||
// once for the client to render in a single pass (see tiles.ts's
|
// against the real server that this library alone has 1229 artists,
|
||||||
// renderTilesInChunks for the render-side half of that fix).
|
// so a 300 cap would have silently dropped 75% of them, not just
|
||||||
const params = new URLSearchParams({ ParentId: libraryId, UserId: session.userId, SortBy: 'SortName', Limit: '300' });
|
// trimmed an edge case. Rendering that many is tiles.ts's
|
||||||
|
// renderTilesInChunks's job, not this query's.
|
||||||
|
const params = new URLSearchParams({ ParentId: libraryId, UserId: session.userId, SortBy: 'SortName' });
|
||||||
const data = await this.fetchJson<EmbyItemsResponse>(`/Artists?${params}`, session, 'Impossible de récupérer les artistes.');
|
const data = await this.fetchJson<EmbyItemsResponse>(`/Artists?${params}`, session, 'Impossible de récupérer les artistes.');
|
||||||
return (data.Items || []).map((a) => this.toItemDto(a, session.serverId));
|
return (data.Items || []).map((a) => this.toItemDto(a, session.serverId));
|
||||||
}
|
}
|
||||||
|
|
@ -238,17 +240,29 @@ class CatalogServiceClass {
|
||||||
// renders tileHtml's poster/name/year (views/library, tiles.ts), which
|
// renders tileHtml's poster/name/year (views/library, tiles.ts), which
|
||||||
// never reads it. Traced here after "the catalog API is too slow, the
|
// never reads it. Traced here after "the catalog API is too slow, the
|
||||||
// front freezes while it loads" - Emby happily serializes a full
|
// front freezes while it loads" - Emby happily serializes a full
|
||||||
// synopsis for all 300 items, and the client happily JSON.parses all
|
// synopsis for every item anyway, and the client happily JSON.parses
|
||||||
// of it, entirely to be thrown away unread. Contrast getSeriesEpisodes/
|
// all of it, entirely to be thrown away unread. Contrast
|
||||||
// getPlaylistItemsRaw, which keep Overview because their own views
|
// getSeriesEpisodes/getPlaylistItemsRaw, which keep Overview because
|
||||||
// (Series/Playlist) actually display it.
|
// their own views (Series/Playlist) actually display it.
|
||||||
|
//
|
||||||
|
// No Limit - there used to be one here (300), presumably meant as the
|
||||||
|
// same defensive cap getArtists briefly grew (see its own comment).
|
||||||
|
// Confirmed directly against the real server that this app's own
|
||||||
|
// "Films" library alone has 421 items - past that cap, so it was
|
||||||
|
// quietly serving an incomplete grid with no error and no hint
|
||||||
|
// anything was missing, reported as "the catalog is broken, not all
|
||||||
|
// the data is there". Emby returns everything when Limit is omitted
|
||||||
|
// (confirmed: TotalRecordCount matched Items.length exactly with no
|
||||||
|
// Limit param, for both this library and the 1229-artist one above) -
|
||||||
|
// rendering that many tiles without freezing the tab is
|
||||||
|
// renderTilesInChunks's job (tiles.ts), not something this query
|
||||||
|
// should be truncating data to work around.
|
||||||
const params = new URLSearchParams({
|
const params = new URLSearchParams({
|
||||||
ParentId: libraryId,
|
ParentId: libraryId,
|
||||||
Recursive: 'false', // top-level: Movies as movies, TV as Series tiles (not every episode)
|
Recursive: 'false', // top-level: Movies as movies, TV as Series tiles (not every episode)
|
||||||
SortBy: 'SortName',
|
SortBy: 'SortName',
|
||||||
Fields: 'ProductionYear,AlbumId',
|
Fields: 'ProductionYear,AlbumId',
|
||||||
ImageTypeLimit: '1',
|
ImageTypeLimit: '1'
|
||||||
Limit: '300'
|
|
||||||
});
|
});
|
||||||
const data = await this.fetchJson<EmbyItemsResponse>(`/Users/${session.userId}/Items?${params}`, session, 'Impossible de récupérer le contenu de cette bibliothèque.');
|
const data = await this.fetchJson<EmbyItemsResponse>(`/Users/${session.userId}/Items?${params}`, session, 'Impossible de récupérer le contenu de cette bibliothèque.');
|
||||||
return (data.Items || []).map((i) => this.toItemDto(i, session.serverId));
|
return (data.Items || []).map((i) => this.toItemDto(i, session.serverId));
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue