fix(catalog): keep library copies held by a server that was never asked

A title's copy disappeared from a catalog item's library matches when the
server holding it went offline, and the screen then claimed the title was not
in the library at all.

The reverse-lookup fan-out only reaches online clients, so a registered
server that is offline lands in neither the succeeded, failed nor cancelled
set. The fold read that absence as "left the account" and dropped the
server's verified copies. Worse, the surviving wave looked complete, so it
was memoized for the rest of the profile session and never asked again. With
no server online at all, every server's copies were erased and the detail
screen asserted "Not in your library" over servers nobody had queried.

LibraryLookupResult now names the registered servers a wave could not even
reach, which needs a registered-server set on MultiServerManager because an
auth-rejected Plex server holds no client at all. The fold's rule is stated
positively -- only a server that answered may replace its own entry -- a wave
that skipped a server is never memoized past the TTL, and the detail screen
counts those servers as unchecked alongside the ones that failed.
This commit is contained in:
edde746
2026-09-06 00:53:55 +02:00
parent e35f6b3a64
commit d6f0aa8936
8 changed files with 300 additions and 24 deletions
@@ -189,9 +189,13 @@ class _CatalogItemDetailScreenState extends State<CatalogItemDetailScreen> {
}
if (!mounted) return;
_checkedServerIds.addAll(result.succeededServerIds);
// A server that was never asked — offline, so absent from the wave — is
// as unchecked as one that failed; without it "not in your library"
// would be asserted over a server that never answered.
_uncheckedServerIds
..addAll(result.failedServerIds)
..addAll(result.cancelledServerIds)
..addAll(result.unqueriedServerIds)
..removeAll(_checkedServerIds);
_mergeMatches(result.items);
}
@@ -13,12 +13,14 @@ import '../data_aggregation_service.dart';
/// One reverse-lookup fan-out per tap (see
/// `DataAggregationService.findByExternalIdsAcrossServers`), memoized for
/// the session per server: a server's answer is replaced only by that
/// server. A complete positive wave is kept (library membership rarely
/// shrinks mid-session); negatives expire so newly-added media is picked up,
/// and so does any wave a server sat out — a slow or unreachable server is
/// not evidence of absence, and the next tap after [negativeTtl] asks it
/// again while its last-known copies stay on screen. Profile-scoped via the
/// provider subtree, so a profile switch drops the cache by construction.
/// server. A wave every registered server answered is kept when positive
/// (library membership rarely shrinks mid-session); negatives expire so
/// newly-added media is picked up, and so does any wave a server sat out —
/// a server that failed, was cancelled, or was never asked at all because
/// it is offline is not evidence of absence, and the next tap after
/// [negativeTtl] asks it again while its last-known copies stay on screen.
/// Profile-scoped via the provider subtree, so a profile switch drops the
/// cache by construction.
class CatalogLibraryMatcher {
static const Duration negativeTtl = Duration(minutes: 10);
@@ -68,18 +70,29 @@ class CatalogLibraryMatcher {
return entry.result;
}
/// Fold a wave into the last-known per-server answers. Servers in the
/// failed or cancelled sets keep their previous copies — a timeout is not
/// evidence of removal (#2098) — and their sets pass through so the UI
/// still names the outage and the wave is retried after [negativeTtl].
/// Every other server's entry is replaced by what it returned this time:
/// nothing for a server named in no set, which has left the account.
/// Fold a wave into the last-known per-server answers.
///
/// Stated positively: only a server in the succeeded set may replace its
/// own entry, because it is the only one that just answered. Servers in
/// the failed, cancelled or unqueried sets keep their previous copies — a
/// timeout, a client abort, and a registered server that was offline and
/// so never asked at all are none of them evidence of removal (#2098) —
/// and their sets pass through so the UI still names them unchecked and
/// the wave is retried after [negativeTtl]. Everything else is dropped:
/// a server in none of the four sets is no longer on the account.
_Entry _fold(Map<String?, List<MediaItem>>? previous, LibraryLookupResult result) {
bool sawNoAnswerFrom(String? serverId) =>
result.failedServerIds.contains(serverId) ||
result.cancelledServerIds.contains(serverId) ||
result.unqueriedServerIds.contains(serverId);
final byServer = <String?, List<MediaItem>>{
if (previous != null)
for (final MapEntry(:key, :value) in previous.entries)
if (result.failedServerIds.contains(key) || result.cancelledServerIds.contains(key)) key: value,
if (sawNoAnswerFrom(key)) key: value,
};
// Carried copies are absent from `result.items`, so a wave that kept any
// has to be rebuilt around them; with nothing carried the wave already
// is the whole answer.
final carried = byServer.isNotEmpty;
for (final item in result.items) {
(byServer[item.serverId] ??= []).add(item);
@@ -93,6 +106,7 @@ class CatalogLibraryMatcher {
succeededServerIds: result.succeededServerIds,
cancelledServerIds: result.cancelledServerIds,
failedServerIds: result.failedServerIds,
unqueriedServerIds: result.unqueriedServerIds,
)
: result,
);
@@ -114,14 +128,22 @@ class CatalogLibraryMatcher {
/// to ask again.
static List<String> lookupTitles(CatalogItem item) => titleMatchCandidates([item.originalTitle, item.title]);
/// Whether [result] may be memoized for the rest of the session: a hit
/// every registered server took part in. One that sat out — failed,
/// cancelled or never asked — leaves the answer incomplete, so it expires
/// with [negativeTtl] instead of being cached until the profile switches.
static bool _isAuthoritativeHit(LibraryLookupResult result) =>
result.items.isNotEmpty && result.failedServerIds.isEmpty && result.cancelledServerIds.isEmpty;
result.items.isNotEmpty &&
result.failedServerIds.isEmpty &&
result.cancelledServerIds.isEmpty &&
result.unqueriedServerIds.isEmpty;
static const LibraryLookupResult _nothing = (
items: <MediaItem>[],
succeededServerIds: <String>{},
cancelledServerIds: <String>{},
failedServerIds: <String>{},
unqueriedServerIds: <String>{},
);
/// The exact `plex://` guid for a Plex Discover item, which its own rating
+32 -8
View File
@@ -65,12 +65,16 @@ typedef SearchAggregationResult = ({
/// One reverse-lookup wave over every online server. `items` are the
/// id-verified copies, deduped and ordered best-first; a server in
/// `failedServerIds` or `cancelledServerIds` contributed nothing and says
/// nothing about what it holds.
/// nothing about what it holds. `unqueriedServerIds` names the registered
/// servers the wave could not even reach out to — offline, or auth-rejected
/// with no client at all — which say just as little: a caller must treat
/// them as unchecked, never as evidence of absence.
typedef LibraryLookupResult = ({
List<MediaItem> items,
Set<String> succeededServerIds,
Set<String> cancelledServerIds,
Set<String> failedServerIds,
Set<String> unqueriedServerIds,
});
typedef _FanOutResult<T> = ({
List<T> items,
@@ -805,11 +809,14 @@ class DataAggregationService {
/// (#1754). Results are deduped by global key and ordered best-first with
/// [compareLibraryCopies] so the chooser is stable across repeated passes.
///
/// A server that could not be asked — slow past its lookup deadline,
/// unreachable, or aborted client-side — is reported in the failed or
/// cancelled set rather than folded into an empty answer (#2098): the
/// caller shows it as unchecked, and a caller holding earlier results must
/// merge rather than replace (see [mergeLibraryCopies]).
/// A server that could not be asked is reported rather than folded into an
/// empty answer (#2098): slow past its lookup deadline, unreachable, or
/// aborted client-side lands in the failed or cancelled set, and a
/// registered server the wave never even reached — offline, or auth-
/// rejected before a client existed — lands in `unqueriedServerIds`, since
/// [MultiServerManager.onlineClients] is the whole world a wave sees. The
/// caller shows all of them as unchecked, and a caller holding earlier
/// results must merge rather than replace (see [mergeLibraryCopies]).
Future<LibraryLookupResult> findByExternalIdsAcrossServers(
ExternalIds ids, {
required MediaKind kind,
@@ -818,13 +825,29 @@ class DataAggregationService {
String? plexGuid,
ExternalSeasonRef? season,
}) async {
final clients = _serverManager.onlineClients;
if ((!ids.hasAny && plexGuid == null) || clients.isEmpty) {
// Nothing to ask with: no server was skipped, the query itself is empty.
if (!ids.hasAny && plexGuid == null) {
return (
items: const <MediaItem>[],
succeededServerIds: const <String>{},
cancelledServerIds: const <String>{},
failedServerIds: const <String>{},
unqueriedServerIds: const <String>{},
);
}
final clients = _serverManager.onlineClients;
final unqueriedServerIds = {
for (final serverId in _serverManager.registeredServerIds)
if (!clients.containsKey(serverId)) serverId,
};
if (clients.isEmpty) {
return (
items: const <MediaItem>[],
succeededServerIds: const <String>{},
cancelledServerIds: const <String>{},
failedServerIds: const <String>{},
unqueriedServerIds: unqueriedServerIds,
);
}
@@ -839,6 +862,7 @@ class DataAggregationService {
succeededServerIds: fetched.succeededServerIds,
cancelledServerIds: fetched.cancelledServerIds,
failedServerIds: fetched.failedServerIds,
unqueriedServerIds: unqueriedServerIds,
);
}
+8
View File
@@ -186,6 +186,14 @@ class MultiServerManager {
/// MediaBrowser-only profiles.
List<String> get serverIds => _clients.keys.toList();
/// Every server this profile has registered, whether or not it currently
/// holds a client. A startup auth failure ([markPlexConnectionAuthError])
/// and a token-refresh/connect failure register a Plex server with no
/// [_clients] entry at all, so [serverIds] cannot see it. Callers that must
/// distinguish "this server said nothing" from "this server was never
/// asked" — a fan-out only ever reaches [onlineClients] — need this set.
Set<String> get registeredServerIds => {..._clients.keys, ..._plexServers.keys, ..._authErrorServers};
List<String> get onlineServerIds => _serverStatus.entries.where((e) => e.value).map((e) => e.key).toList();
List<String> get offlineServerIds => _serverStatus.entries.where((e) => !e.value).map((e) => e.key).toList();
@@ -534,6 +534,25 @@ void main() {
expect(find.text(t.explore.libraryCheckFailed(n: 1)), findsOneWidget);
});
testWidgets('a server that was never asked is reported instead of counted as a miss', (tester) async {
// An offline server is not in the fan-out at all, so it lands in no
// failed or cancelled set — but "Not in your library" is still a false
// claim about a server that never answered.
final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow));
await _pumpDetail(
tester,
source,
item: _bareRow,
matcherBuilder: (multiServer) => _ScriptedMatcher(multiServer, [
() => libraryLookupResult(const [], unqueried: {'server-1'}),
]),
);
expect(find.text(t.explore.notInLibrary), findsNothing);
expect(find.text(t.explore.libraryCheckFailed(n: 1)), findsOneWidget);
});
testWidgets('an unchecked server is noted under the copies other servers found', (tester) async {
final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow));
@@ -1,5 +1,8 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:plezy/media/ids.dart';
import 'package:plezy/media/media_item.dart';
import 'package:plezy/media/media_kind.dart';
import 'package:plezy/media/media_server_client.dart';
import 'package:plezy/models/catalog/catalog_item.dart';
import 'package:plezy/providers/multi_server_provider.dart';
import 'package:plezy/services/catalog/catalog_library_matcher.dart';
@@ -69,6 +72,61 @@ class _Harness {
}
}
/// A server that answers the reverse lookup with [copies], or throws [error]
/// to model one that could not be reached.
class _LookupClient implements MediaServerClient {
_LookupClient(String id, {this.copies = const [], this.error}) : serverId = ServerId(id);
@override
final ServerId serverId;
List<MediaItem> copies;
Object? error;
int calls = 0;
@override
Future<List<MediaItem>> findByExternalIds(
ExternalIds ids, {
required MediaKind kind,
List<String> titles = const [],
int? year,
String? plexGuid,
ExternalSeasonRef? season,
}) async {
calls++;
final failure = error;
if (failure != null) throw failure;
return copies;
}
@override
void close() {}
@override
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
/// A matcher over the real [DataAggregationService] and [MultiServerManager].
/// Only they can tell a registered-but-offline server (never asked) from one
/// that left the account, so a canned-response harness cannot model it.
class _LiveHarness {
final MultiServerManager manager = MultiServerManager();
late final MultiServerProvider multiServer;
late final CatalogLibraryMatcher matcher;
_LiveHarness(DateTime Function() now) {
multiServer = MultiServerProvider(manager, DataAggregationService(manager));
matcher = CatalogLibraryMatcher.withClock(multiServer, now);
}
void register(_LookupClient client, {bool online = true}) =>
manager.debugRegisterClientForTesting(client, online: online);
void dispose() {
multiServer.dispose();
manager.dispose();
}
}
void main() {
test('season entries sharing canonical ids keep independent cached matches', () async {
final harness = _Harness();
@@ -228,6 +286,104 @@ void main() {
expect(harness.aggregation.calls, hasLength(3));
});
test('a registered server that is offline for the retry keeps its copies and stays retryable', () async {
// An offline server is not in the fan-out at all, so it answers in no
// set — indistinguishable, to the fold, from one that left the account.
// It never said the title was gone, and the surviving wave must not be
// memoized as a complete answer for the session either.
var now = DateTime.utc(2026, 7, 28, 12);
final harness = _LiveHarness(() => now);
addTearDown(harness.dispose);
final aCopy = testMediaItem(id: 'a-copy', serverId: 'a');
final bCopy = testMediaItem(id: 'b-copy', serverId: 'b');
final a = _LookupClient('a', copies: [aCopy]);
final b = _LookupClient('b', error: StateError('unreachable'));
harness.register(a);
harness.register(b);
const item = CatalogItem(
source: CatalogSourceId.trakt,
kind: MediaKind.movie,
title: 'Offline Server Movie',
ids: CatalogItemIds(trakt: 11, tmdb: 79),
);
expect((await harness.matcher.match(item)).items, [aCopy]);
// A drops offline, B recovers and answers for itself.
harness.register(a, online: false);
b
..error = null
..copies = [bCopy];
now = now.add(CatalogLibraryMatcher.negativeTtl);
final retained = await harness.matcher.match(item);
expect(retained.items, [aCopy, bCopy], reason: 'A was never asked, so its verified copy stands');
expect(a.calls, 1);
// Only A's own answer may drop A's copy, so the wave stays retryable.
harness.register(a);
a.copies = const [];
now = now.add(CatalogLibraryMatcher.negativeTtl);
expect((await harness.matcher.match(item)).items, [bCopy]);
expect(a.calls, 2);
});
test('a wave with no online server keeps known copies and is not an authoritative absence', () async {
var now = DateTime.utc(2026, 7, 28, 12);
final harness = _LiveHarness(() => now);
addTearDown(harness.dispose);
final aCopy = testMediaItem(id: 'a-copy', serverId: 'a');
final a = _LookupClient('a', copies: [aCopy]);
final b = _LookupClient('b', error: StateError('unreachable'));
harness.register(a);
harness.register(b);
const item = CatalogItem(
source: CatalogSourceId.trakt,
kind: MediaKind.movie,
title: 'All Servers Offline Movie',
ids: CatalogItemIds(trakt: 12, tmdb: 80),
);
expect((await harness.matcher.match(item)).items, [aCopy]);
harness.register(a, online: false);
harness.register(b, online: false);
now = now.add(CatalogLibraryMatcher.negativeTtl);
final blind = await harness.matcher.match(item);
expect(blind.items, [aCopy], reason: 'a wave nobody answered erases nothing');
expect(blind.succeededServerIds, isEmpty);
harness.register(a);
now = now.add(CatalogLibraryMatcher.negativeTtl);
expect((await harness.matcher.match(item)).items, [aCopy]);
expect(a.calls, 2, reason: 'the blind wave is no hit, so the next tap asks again');
});
test('a server that left the account loses its copies', () async {
// The one wave shape that means removal: the server is in no set at all
// — it neither answered nor sat the wave out, so it is off the account.
var now = DateTime.utc(2026, 7, 28, 12);
final harness = _Harness(now: () => now);
addTearDown(harness.dispose);
final aCopy = testMediaItem(id: 'a-copy', serverId: 'a');
final bCopy = testMediaItem(id: 'b-copy', serverId: 'b');
harness.aggregation.responses.addAll([
libraryLookupResult([aCopy], succeeded: {'a'}, failed: {'b'}),
libraryLookupResult([bCopy], succeeded: {'b'}),
]);
const item = CatalogItem(
source: CatalogSourceId.trakt,
kind: MediaKind.movie,
title: 'Removed Server Movie',
ids: CatalogItemIds(trakt: 13, tmdb: 81),
);
expect((await harness.matcher.match(item)).items, [aCopy]);
now = now.add(CatalogLibraryMatcher.negativeTtl);
expect((await harness.matcher.match(item)).items, [bCopy]);
});
test('forwards season-stripped title candidates and season reference', () async {
final harness = _Harness();
addTearDown(harness.dispose);
@@ -127,6 +127,41 @@ void main() {
});
});
group('registeredServerIds', () {
test('covers an auth-rejected Plex server that never got a client', () {
// A startup auth failure registers the server before any client can
// exist, so `serverIds` — sourced from `_clients` — cannot see it.
// A fan-out that has to report the servers it never asked still must.
final m = MultiServerManager();
addTearDown(m.dispose);
m.markPlexConnectionAuthError(
_plexAccount('account-1', [
PlexServer(
name: 'Plex',
clientIdentifier: 'server-1',
accessToken: 'rejected-token',
connections: const [],
owned: true,
),
]),
);
expect(m.serverIds, isEmpty);
expect(m.onlineClients, isEmpty);
expect(m.registeredServerIds, {'server-1'});
});
test('covers a registered client that is offline', () {
final m = MultiServerManager();
addTearDown(m.dispose);
m.debugRegisterClientForTesting(_jellyfinClient('user-a'), online: false);
expect(m.onlineClients, isEmpty);
expect(m.registeredServerIds, {'jf-machine'});
});
});
group('updateServerStatus + statusStream', () {
test('emits a snapshot when status flips for a tracked server', () async {
final m = MultiServerManager();
+10 -2
View File
@@ -2,10 +2,18 @@ import 'package:plezy/media/media_item.dart';
import 'package:plezy/services/data_aggregation_service.dart';
/// A reverse-lookup wave fixture. By default every listed server answered;
/// name [failed] / [cancelled] servers to model a wave a server sat out.
/// name [failed] / [cancelled] servers to model a wave a server sat out, and
/// [unqueried] ones to model a registered server the wave never asked.
LibraryLookupResult libraryLookupResult(
List<MediaItem> items, {
Set<String> succeeded = const {},
Set<String> failed = const {},
Set<String> cancelled = const {},
}) => (items: items, succeededServerIds: succeeded, cancelledServerIds: cancelled, failedServerIds: failed);
Set<String> unqueried = const {},
}) => (
items: items,
succeededServerIds: succeeded,
cancelledServerIds: cancelled,
failedServerIds: failed,
unqueriedServerIds: unqueried,
);