fix(playback): harden watch progress edge cases

2026-05-29 21:16:01 +02:00
parent d93ea9813f
commit dba01f14bb
20 changed files with 593 additions and 163 deletions
@@ -0,0 +1,111 @@
+import 'package:drift/native.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:plezy/database/app_database.dart';
+import 'package:plezy/media/media_backend.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/media/playback_report_metadata.dart';
+import 'package:plezy/services/external_player_service.dart';
+import 'package:plezy/services/jellyfin_api_cache.dart';
+import 'package:plezy/services/multi_server_manager.dart';
+import 'package:plezy/services/offline_watch_sync_service.dart';
+
+class _RecordingClient implements MediaServerClient {
+  bool failStart = false;
+  bool failStop = false;
+  final started = <({int positionMs, int? durationMs})>[];
+  final stopped = <({int positionMs, int? durationMs})>[];
+
+  @override
+  String get serverId => 'srv';
+
+  @override
+  MediaBackend get backend => MediaBackend.plex;
+
+  @override
+  double get watchedThreshold => 0.9;
+
+  @override
+  Future<void> reportPlaybackStarted({
+    required String itemId,
+    required Duration position,
+    Duration? duration,
+    String? playSessionId,
+    String? playMethod,
+    String? mediaSourceId,
+    int? audioStreamIndex,
+    int? subtitleStreamIndex,
+  }) async {
+    started.add((positionMs: position.inMilliseconds, durationMs: duration?.inMilliseconds));
+    if (failStart) throw StateError('start failed');
+  }
+
+  @override
+  Future<void> reportPlaybackStopped({
+    required String itemId,
+    required Duration position,
+    Duration? duration,
+    String? playSessionId,
+    String? mediaSourceId,
+    PlaybackReportMetadata report = const PlaybackReportMetadata.live(),
+  }) async {
+    stopped.add((positionMs: position.inMilliseconds, durationMs: duration?.inMilliseconds));
+    if (failStop) throw StateError('stop failed');
+  }
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
+}
+
+MediaItem _item({int? durationMs}) {
+  return MediaItem(
+    id: 'item-1',
+    backend: MediaBackend.plex,
+    kind: MediaKind.movie,
+    serverId: 'srv',
+    durationMs: durationMs,
+  );
+}
+
+void main() {
+  test('Android external progress preserves null duration and still stops after start failure', () async {
+    final client = _RecordingClient()..failStart = true;
+
+    await ExternalPlayerService.reportAndroidExternalProgressForTesting(
+      positionMs: 5000,
+      durationMs: null,
+      metadata: _item(),
+      client: client,
+    );
+
+    expect(client.started, [(positionMs: 5000, durationMs: null)]);
+    expect(client.stopped, [(positionMs: 5000, durationMs: null)]);
+  });
+
+  test('Android external progress queues unknown-duration resume when no client is available', () async {
+    final db = AppDatabase.forTesting(NativeDatabase.memory());
+    JellyfinApiCache.initialize(db);
+    final manager = MultiServerManager();
+    final service = OfflineWatchSyncService(database: db, serverManager: manager);
+    addTearDown(() async {
+      service.dispose();
+      manager.dispose();
+      await db.close();
+    });
+
+    await ExternalPlayerService.reportAndroidExternalProgressForTesting(
+      positionMs: 5000,
+      durationMs: null,
+      metadata: _item(),
+      client: null,
+      offlineWatchService: service,
+    );
+
+    final action = await db.getLatestWatchAction('srv:item-1');
+    expect(action, isNotNull);
+    expect(action!.viewOffset, 5000);
+    expect(action.duration, isNull);
+    expect(action.shouldMarkWatched, isFalse);
+  });
+}
@@ -357,6 +357,30 @@ void main() {
      expect(await svc.getPendingSyncCount(), 0);
    });

+    test('unknown-duration offline progress still replays stopped position', () async {
+      final (svc: svc, db: db, mgr: mgr) = _makeService();
+      addTearDown(() async {
+        svc.dispose();
+        mgr.dispose();
+        await db.close();
+      });
+
+      final client = _RecordingMediaClient(serverId: 'srv', backend: MediaBackend.plex);
+      mgr.debugRegisterClientForTesting(client);
+      await svc.queueProgressUpdate(serverId: 'srv', itemId: '42', viewOffset: 50000, duration: null);
+
+      await svc.syncPendingItems();
+
+      expect(client.started, hasLength(1));
+      expect(client.started.single.positionMs, 50000);
+      expect(client.started.single.durationMs, isNull);
+      expect(client.stopped, hasLength(1));
+      expect(client.stopped.single.positionMs, 50000);
+      expect(client.stopped.single.durationMs, isNull);
+      expect(client.watched, isEmpty);
+      expect(await svc.getPendingSyncCount(), 0);
+    });
+
    test('completed Plex offline progress replays at duration and marks watched', () async {
      final (svc: svc, db: db, mgr: mgr) = _makeService();
      addTearDown(() async {
@@ -409,6 +433,24 @@ void main() {
      expect(action.shouldMarkWatched, isFalse);
    });

+    test('persists unknown-duration progress without marking watched', () async {
+      final (svc: svc, db: db, mgr: mgr) = _makeService();
+      addTearDown(() async {
+        svc.dispose();
+        mgr.dispose();
+        await db.close();
+      });
+
+      await svc.queueProgressUpdate(serverId: 'srv', itemId: '42', viewOffset: 50, duration: null);
+
+      final action = await db.getLatestWatchAction('srv:42');
+      expect(action, isNotNull);
+      expect(action!.actionType, 'progress');
+      expect(action.viewOffset, 50);
+      expect(action.duration, isNull);
+      expect(action.shouldMarkWatched, isFalse);
+    });
+
    test('persists shouldMarkWatched=true at/above the default 0.9 threshold', () async {
      final (svc: svc, db: db, mgr: mgr) = _makeService();
      addTearDown(() async {
@@ -486,9 +528,9 @@ void main() {
        await db.close();
      });

-      // Below threshold is resume-only, not an explicit unwatched override.
+      // Below threshold is explicit local progress, so it overrides stale watched metadata.
      await svc.queueProgressUpdate(serverId: 'srv', itemId: '1', viewOffset: 50, duration: 100);
-      expect(await svc.getLocalWatchStatus('srv:1'), isNull);
+      expect(await svc.getLocalWatchStatus('srv:1'), isFalse);

      // Above threshold → shouldMarkWatched=true → status=true.
      await svc.queueProgressUpdate(serverId: 'srv', itemId: '2', viewOffset: 99, duration: 100);
@@ -767,7 +809,7 @@ void main() {

      expect(await svc.getLocalWatchStatus('jf-machine:item-1'), isTrue);
      expect(await svc.getLocalViewOffset('jf-machine:item-1'), isNull);
-      expect(await svc.getLocalWatchStatus('jf-machine:item-1', clientScopeId: 'jf-machine/user-a'), isNull);
+      expect(await svc.getLocalWatchStatus('jf-machine:item-1', clientScopeId: 'jf-machine/user-a'), isFalse);
      expect(await svc.getLocalViewOffset('jf-machine:item-1', clientScopeId: 'jf-machine/user-a'), 5000);
    });

@@ -134,6 +134,22 @@ void main() {
    expect(result.mediaInfo?.audioTracks.single.languageCode, 'fre');
  });

+  test('offline path falls back to media index when caller has no source id', () async {
+    await _insertDownloaded(
+      db,
+      serverId: 'srv-1',
+      ratingKey: 'movie-1',
+      videoFilePath: 'content://offline/movie-1-v1',
+      mediaIndex: 0,
+      mediaSourceId: 'source-a',
+    );
+
+    final service = PlaybackInitializationService(database: db);
+
+    expect(await service.getOfflineVideoPath('srv-1', 'movie-1', mediaIndex: 1), null);
+    expect(await service.getOfflineVideoPath('srv-1', 'movie-1', mediaIndex: 0), 'content://offline/movie-1-v1');
+  });
+
  test('pure-offline Jellyfin cache works without a connection row', () async {
    await _insertDownloaded(
      db,
@@ -312,6 +328,7 @@ Future<void> _insertDownloaded(
  required String ratingKey,
  required String videoFilePath,
  int mediaIndex = 0,
+  String? mediaSourceId,
 }) async {
  await db
      .into(db.downloadedMedia)
@@ -325,6 +342,7 @@ Future<void> _insertDownloaded(
          status: DownloadStatus.completed.index,
          videoFilePath: Value(videoFilePath),
          mediaIndex: Value(mediaIndex),
+          mediaSourceId: Value(mediaSourceId),
        ),
      );
 }
@@ -8,6 +8,7 @@ import 'package:plezy/services/playback_report_session.dart';
 class _RecordingClient implements MediaServerClient {
  final calls = <String>[];
  Completer<void>? startGate;
+  Completer<void>? stopGate;
  bool failNextStop = false;

  @override
@@ -51,6 +52,8 @@ class _RecordingClient implements MediaServerClient {
    PlaybackReportMetadata report = const PlaybackReportMetadata.live(),
  }) async {
    calls.add('stopped-attempt:${position.inMilliseconds}:$mediaSourceId');
+    final gate = stopGate;
+    if (gate != null) await gate.future;
    if (failNextStop) {
      failNextStop = false;
      throw StateError('stop failed');
@@ -174,4 +177,24 @@ void main() {

    expect(client.calls, ['stopped-attempt:1000:null', 'stopped-attempt:3000:null', 'stopped:3000:null']);
  });
+
+  test('resetAfterStop during in-flight stop reopens reporting after stop completes', () async {
+    final client = _RecordingClient()..stopGate = Completer<void>();
+    final session = PlaybackReportSession(client: client, itemId: 'item-1');
+
+    await session.report(_snapshot('playing', positionMs: 1000));
+    client.calls.clear();
+
+    final stopFuture = session.report(_snapshot('stopped', positionMs: 3000));
+    await Future<void>.delayed(Duration.zero);
+    expect(client.calls, ['stopped-attempt:3000:null']);
+
+    session.resetAfterStop();
+    client.stopGate!.complete();
+    await stopFuture;
+
+    expect(session.isIdle, isTrue);
+    expect(await session.report(_snapshot('playing', positionMs: 4000)), isTrue);
+    expect(client.calls, ['stopped-attempt:3000:null', 'stopped:3000:null', 'started:4000:null:null:null']);
+  });
 }
@@ -0,0 +1,62 @@
+import 'package:drift/native.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:plezy/database/app_database.dart';
+import 'package:plezy/media/media_backend.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/transcode_quality_preset.dart';
+import 'package:plezy/services/multi_server_manager.dart';
+import 'package:plezy/services/playback_context.dart';
+import 'package:plezy/services/playback_initialization_types.dart';
+import 'package:plezy/services/playback_source_resolver.dart';
+
+class _PlaybackClient implements MediaServerClient {
+  @override
+  String get serverId => 'srv';
+
+  @override
+  MediaBackend get backend => MediaBackend.plex;
+
+  @override
+  double get watchedThreshold => 0.9;
+
+  @override
+  Map<String, String> get streamHeaders => const {'X-Test': 'token'};
+
+  @override
+  void close() {}
+
+  @override
+  Future<PlaybackInitializationResult> getPlaybackInitialization(PlaybackInitializationOptions options) async {
+    return PlaybackInitializationResult(availableVersions: const [], videoUrl: 'https://example.com/video.mp4');
+  }
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
+}
+
+void main() {
+  test('online playback uses registered client even when status is stale offline', () async {
+    final db = AppDatabase.forTesting(NativeDatabase.memory());
+    final manager = MultiServerManager();
+    addTearDown(() async {
+      manager.dispose();
+      await db.close();
+    });
+
+    final client = _PlaybackClient();
+    manager.debugRegisterClientForTesting(client, online: false);
+
+    final context = await PlaybackSourceResolver(serverManager: manager, database: db).resolve(
+      metadata: MediaItem(id: 'item-1', backend: MediaBackend.plex, kind: MediaKind.movie, serverId: 'srv'),
+      selectedMediaIndex: 0,
+      offlineLibraryMode: false,
+      qualityPreset: TranscodeQualityPreset.original,
+    );
+
+    expect(context.result.videoUrl, 'https://example.com/video.mp4');
+    expect(context.reportingClient, same(client));
+    expect(context.reportingMode, PlaybackReportingMode.online);
+  });
+}
@@ -0,0 +1,81 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:plezy/database/app_database.dart';
+import 'package:plezy/services/watch_state_resolver.dart';
+import 'package:plezy/utils/watch_state_notifier.dart';
+
+OfflineWatchProgressItem _action({
+  required String actionType,
+  required int updatedAt,
+  int? viewOffset,
+  int? duration,
+  bool shouldMarkWatched = false,
+}) {
+  return OfflineWatchProgressItem(
+    id: updatedAt,
+    serverId: 'srv',
+    ratingKey: 'item-1',
+    globalKey: 'srv:item-1',
+    actionType: actionType,
+    viewOffset: viewOffset,
+    duration: duration,
+    shouldMarkWatched: shouldMarkWatched,
+    createdAt: updatedAt,
+    updatedAt: updatedAt,
+    syncAttempts: 0,
+  );
+}
+
+void main() {
+  test('newer sub-threshold progress overrides older watched state without watched-plus-resume', () {
+    final snapshot = WatchStateResolver.fromActions([
+      _action(actionType: 'watched', updatedAt: 1),
+      _action(actionType: 'progress', updatedAt: 2, viewOffset: 5000, duration: 100000),
+    ]);
+
+    expect(snapshot.isWatched, isFalse);
+    expect(snapshot.hasViewOffsetMs, isTrue);
+    expect(snapshot.viewOffsetMs, 5000);
+  });
+
+  test('newer watched action clears older progress offset', () {
+    final snapshot = WatchStateResolver.fromActions([
+      _action(actionType: 'progress', updatedAt: 1, viewOffset: 5000, duration: 100000),
+      _action(actionType: 'watched', updatedAt: 2),
+    ]);
+
+    expect(snapshot.isWatched, isTrue);
+    expect(snapshot.hasViewOffsetMs, isTrue);
+    expect(snapshot.viewOffsetMs, 0);
+  });
+
+  test('sub-threshold progress events explicitly clear watched state', () {
+    final snapshot = WatchStateResolver.fromEvent(
+      WatchStateEvent(
+        itemId: 'item-1',
+        serverId: 'srv',
+        changeType: WatchStateChangeType.progressUpdate,
+        parentChain: const [],
+        mediaType: 'movie',
+        viewOffset: 5000,
+        isNowWatched: false,
+      ),
+    );
+
+    expect(snapshot.isWatched, isFalse);
+    expect(snapshot.viewOffsetMs, 5000);
+  });
+
+  test('removed from continue watching is not a watch-state overlay patch', () {
+    final snapshot = WatchStateResolver.fromEvent(
+      WatchStateEvent(
+        itemId: 'item-1',
+        serverId: 'srv',
+        changeType: WatchStateChangeType.removedFromContinueWatching,
+        parentChain: const [],
+        mediaType: 'movie',
+      ),
+    );
+
+    expect(snapshot.isEmpty, isTrue);
+  });
+}