diff --git a/lib/components/DownloadsScreen/item_file_size.dart b/lib/components/DownloadsScreen/item_file_size.dart index 2b5db1be4..3975b8bf1 100644 --- a/lib/components/DownloadsScreen/item_file_size.dart +++ b/lib/components/DownloadsScreen/item_file_size.dart @@ -22,10 +22,11 @@ class ItemFileSize extends ConsumerWidget { ref.watch(isarDownloader.itemProvider(stub).future).then((item) { switch (item?.state) { case DownloadItemState.notDownloaded: - if (isarDownloader.getStatus(item!, null).isRequired) { - return Future.value(syncingText); - } else { + if (isarDownloader.getStatus(item!, null) == + DownloadItemStatus.notNeeded) { return Future.value(deletingText); + } else { + return Future.value(syncingText); } case DownloadItemState.syncFailed: return Future.value(syncingText); diff --git a/lib/components/MusicScreen/music_screen_tab_view.dart b/lib/components/MusicScreen/music_screen_tab_view.dart index 878ca79fd..e42e56950 100644 --- a/lib/components/MusicScreen/music_screen_tab_view.dart +++ b/lib/components/MusicScreen/music_screen_tab_view.dart @@ -1,11 +1,10 @@ import 'dart:async'; import 'dart:math'; -import 'package:collection/collection.dart'; import 'package:Finamp/services/finamp_user_helper.dart'; +import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; -import 'package:flutter/src/rendering/sliver.dart'; -import 'package:flutter/src/rendering/sliver_grid.dart'; +import 'package:flutter/rendering.dart'; import 'package:get_it/get_it.dart'; import 'package:hive/hive.dart'; import 'package:infinite_scroll_pagination/infinite_scroll_pagination.dart'; @@ -182,7 +181,7 @@ class _MusicScreenTabViewState extends State Rect.fromLTRB(0, 0, 0, MediaQuery.of(context).padding.bottom), axis: Axis.vertical); _refreshStream = _isarDownloader.offlineDeletesStream.listen((event) { - _pagingController.refresh(); + _refresh(); }); super.initState(); } diff --git a/lib/models/finamp_models.dart b/lib/models/finamp_models.dart index 641e55b7f..f76b55eba 100644 --- a/lib/models/finamp_models.dart +++ b/lib/models/finamp_models.dart @@ -1057,7 +1057,8 @@ class DownloadItem extends DownloadStub { DownloadItem? copyWith( {BaseItemDto? item, List? orderedChildItems, - String? viewId}) { + String? viewId, + required bool forceCopy}) { String? json; if (type == DownloadItemType.image) { // Images do not have any attributes we might want to update @@ -1074,17 +1075,19 @@ class DownloadItem extends DownloadStub { // overwrite with null if the new item does not have them. item.mediaSources ??= baseItem?.mediaSources; item.mediaStreams ??= baseItem?.mediaStreams; - item.childCount ??= baseItem?.childCount; + item.sortName ??= baseItem?.sortName; } assert(item == null || ((item.mediaSources == null || item.mediaSources!.isNotEmpty) && (item.mediaStreams == null || item.mediaStreams!.isNotEmpty))); var orderedChildren = orderedChildItems?.map((e) => e.isarId).toList(); - if (viewId == null || viewId == this.viewId) { - if (item == null || baseItem!.mostlyEqual(item)) { - var equal = const DeepCollectionEquality().equals; - if (equal(orderedChildren, this.orderedChildren)) { - return null; + if (!forceCopy) { + if (viewId == null || viewId == this.viewId) { + if (item == null || baseItem!.mostlyEqual(item)) { + var equal = const DeepCollectionEquality().equals; + if (equal(orderedChildren, this.orderedChildren)) { + return null; + } } } } diff --git a/lib/models/jellyfin_models.dart b/lib/models/jellyfin_models.dart index 6ca8bd20e..1f29fa29d 100644 --- a/lib/models/jellyfin_models.dart +++ b/lib/models/jellyfin_models.dart @@ -7,8 +7,8 @@ /// /// These classes should be correct with Jellyfin 10.7.5 -import 'package:collection/collection.dart'; import 'package:Finamp/models/finamp_models.dart'; +import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/app_localizations.dart'; import 'package:hive/hive.dart'; @@ -2277,6 +2277,10 @@ class BaseItemDto with RunTimeTickDuration { equal(other.artists, artists) && other.albumArtist == albumArtist && other.childCount == childCount && + other.imageId == imageId && + // imageId does not necessarily change when the image is updated, so + // we must compare blurHashes as well. + other.blurHash == blurHash && other.mediaSources?.length == mediaSources?.length && other.mediaStreams?.length == mediaStreams?.length && other.normalizationGain == normalizationGain && diff --git a/lib/services/downloads_service.dart b/lib/services/downloads_service.dart index a01eeca01..7f3e991ae 100644 --- a/lib/services/downloads_service.dart +++ b/lib/services/downloads_service.dart @@ -2,10 +2,10 @@ import 'dart:async'; import 'dart:collection'; import 'dart:io'; -import 'package:background_downloader/background_downloader.dart'; -import 'package:collection/collection.dart'; import 'package:Finamp/components/global_snackbar.dart'; import 'package:Finamp/services/jellyfin_api_helper.dart'; +import 'package:background_downloader/background_downloader.dart'; +import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/app_localizations.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; diff --git a/lib/services/downloads_service_backend.dart b/lib/services/downloads_service_backend.dart index 88d4ea5c7..691db290b 100644 --- a/lib/services/downloads_service_backend.dart +++ b/lib/services/downloads_service_backend.dart @@ -3,16 +3,17 @@ import 'dart:convert'; import 'dart:core'; import 'dart:io'; -import 'package:background_downloader/background_downloader.dart'; -import 'package:collection/collection.dart'; import 'package:Finamp/components/global_snackbar.dart'; import 'package:Finamp/services/downloads_service.dart'; +import 'package:background_downloader/background_downloader.dart'; +import 'package:collection/collection.dart'; import 'package:flutter/scheduler.dart'; import 'package:get_it/get_it.dart'; import 'package:isar/isar.dart'; import 'package:json_annotation/json_annotation.dart'; import 'package:logging/logging.dart'; import 'package:path/path.dart' as path_helper; +import 'package:uuid/uuid.dart'; import '../models/finamp_models.dart'; import '../models/jellyfin_models.dart'; @@ -916,6 +917,31 @@ class DownloadsSyncService { _syncLogger.finer( "Syncing ${parent.baseItemType.name} ${parent.name} with required:$asRequired viewId:$viewId"); + // + // Fetch latest metadata from server, if needed or not quicksyncing + // + // newBaseItem must be calculated before children are determined so that the latest + // metadata can be used, especially imageId and blurhash. + BaseItemDto? newBaseItem; + //If we aren't quicksyncing, fetch the latest BaseItemDto to copy into Isar. + if (parent.type.requiresItem && + (!FinampSettingsHelper.finampSettings.preferQuickSyncs || + _downloadsService.forceFullSync || + _needsMetadataUpdate(parent))) { + newBaseItem = + (await _getCollectionInfo(parent.baseItem!.id, parent.type, true)) + ?.baseItem; + } + // We return the same BaseItemDto for all requests, so null out playlistItemId + // as it will not usually be accurate. Modifying without copying should be + // fine as this item was generated within the download service, so this value + // is not being used elsewhere. + if (parent.baseItem?.playlistItemId != null || + newBaseItem?.playlistItemId != null) { + newBaseItem ??= parent.baseItem; + newBaseItem?.playlistItemId = null; + } + // // Calculate needed children for item based on type and asRequired flag // @@ -923,15 +949,9 @@ class DownloadsSyncService { Set requiredChildren = {}; Set infoChildren = {}; List? orderedChildItems; - BaseItemDto? newBaseItem; switch (parent.type) { case DownloadItemType.collection: - var item = parent.baseItem!; - // TODO alert user that image deduplication is broken. - if ((item.blurHash ?? item.imageId) != null) { - infoChildren.add( - DownloadStub.fromItem(type: DownloadItemType.image, item: item)); - } + var item = newBaseItem ?? parent.baseItem!; try { if (asRequired) { orderedChildItems = await _getCollectionChildren(parent); @@ -942,12 +962,7 @@ class DownloadsSyncService { orderedChildItems ??= await _getCollectionChildren(parent); infoChildren.addAll(orderedChildItems); } - if (parent.baseItemType == BaseItemDtoType.playlist) { - newBaseItem = (await _getCollectionInfo( - parent.baseItem!.id, parent.type, true)) - ?.baseItem; - } else if (parent.baseItemType == BaseItemDtoType.album && - viewId == null) { + if (parent.baseItemType == BaseItemDtoType.album && viewId == null) { isarParent ??= _isar.downloadItems.getSync(parent.isarId); if (isarParent?.viewId == null) { // If we are an album and have no viewId, attempt to fetch from server @@ -958,8 +973,13 @@ class DownloadsSyncService { _syncLogger.info("Error downloading children for ${item.name}: $e"); rethrow; } + // TODO alert user if image deduplication is broken. + if ((item.blurHash ?? item.imageId) != null) { + infoChildren.add( + DownloadStub.fromItem(type: DownloadItemType.image, item: item)); + } case DownloadItemType.song: - var item = parent.baseItem!; + var item = newBaseItem ?? parent.baseItem!; if ((item.blurHash ?? item.imageId) != null) { requiredChildren.add( DownloadStub.fromItem(type: DownloadItemType.image, item: item)); @@ -1011,28 +1031,6 @@ class DownloadsSyncService { } } - final requiredAttributes = [ - parent.baseItem?.sortName, - parent.baseItem?.childCount, - parent.baseItem?.mediaSources, - parent.baseItem?.mediaStreams - ]; - //If we aren't quicksyncing, fetch the latest BaseItemDto to copy into Isar - if (parent.type.requiresItem && - (!FinampSettingsHelper.finampSettings.preferQuickSyncs || - _downloadsService.forceFullSync || - parent.baseItem?.playlistItemId != null || - requiredAttributes.any((element) => element == null))) { - newBaseItem ??= - (await _getCollectionInfo(parent.baseItem!.id, parent.type, true)) - ?.baseItem; - // We return the same BaseItemDto for all requests, so null out playlistItemId - // as it will not usually be accurate. Modifying without copying should be - // fine as this item was generated within the download service, so this value - // is not being used elsewhere. - newBaseItem?.playlistItemId = null; - } - // // Update item with latest metadata and previously calculated children. // For the anchor, just fetch current children. @@ -1052,7 +1050,8 @@ class DownloadsSyncService { var newParent = canonParent!.copyWith( item: newBaseItem, viewId: viewId, - orderedChildItems: orderedChildItems); + orderedChildItems: orderedChildItems, + forceCopy: _downloadsService.forceFullSync); // copyWith returns null if no updates to important fields are needed if (newParent != null) { _isar.downloadItems.putSync(newParent); @@ -1105,7 +1104,8 @@ class DownloadsSyncService { // successful sync. songs/images will be moved out by _initiateDownload. // If our linked children just changed, recalculate state with new children. if (!canonParent!.type.hasFiles && - (requiredChanges.$1.isNotEmpty || + (canonParent!.state == DownloadItemState.syncFailed || + requiredChanges.$1.isNotEmpty || requiredChanges.$2.isNotEmpty || requiredChanges.$3.isNotEmpty || infoChanges.$1.isNotEmpty || @@ -1296,10 +1296,25 @@ class DownloadsSyncService { fields: fields) ?? []; _downloadsService.resetConnectionErrors(); - itemFetch.complete(childItems.map((e) => e.id).toList()); var childStubs = childItems .map((e) => DownloadStub.fromItem(type: childType, item: e)) .toList(); + // If we are a library, we need to get orphan songs to download in addition to + // songs which are contained in albums. + if (parent.baseItemType == BaseItemDtoType.library) { + var songChildItems = await _jellyfinApiData.getItems( + parentItem: item, + includeItemTypes: BaseItemDtoType.song.idString, + recursive: false, + fields: + "${_jellyfinApiData.defaultFields},MediaSources,MediaStreams,SortName") ?? + []; + childItems.addAll(songChildItems); + var songChildStubs = songChildItems.map( + (e) => DownloadStub.fromItem(type: DownloadItemType.song, item: e)); + childStubs.addAll(songChildStubs); + } + itemFetch.complete(childItems.map((e) => e.id).toList()); for (var element in childStubs) { _metadataCache[element.id] = Future.value(element); } @@ -1418,6 +1433,32 @@ class DownloadsSyncService { return null; } + /// This returns whether the given item needs its metadata refreshed from the server. + /// If modifying to add another required field, the requested fields to be downloaded + /// in _getCollectionInfo and _getCollectionChildren must be updated. Additionally, + /// DownloadItem.copyWith must be updated to preserve the field, and BaseItemDto.mostlyEqual + /// must be updated to check the field when determining equality. + bool _needsMetadataUpdate(DownloadStub stub) { + assert(stub.type.requiresItem); + + // childCount is expected to change frequently for playlists, so we + // always fetch a fresh copy from the server to check if the metadata + // needs updating, even when quickSyncing. + if (stub.type == DownloadItemType.collection && + stub.baseItemType == BaseItemDtoType.playlist) { + return true; + } + if (stub.baseItem?.sortName == null) { + return true; + } + if (stub.type == DownloadItemType.song && + (stub.baseItem?.mediaSources == null || + stub.baseItem?.mediaStreams == null)) { + return true; + } + return false; + } + /// Ensures the given node is downloaded. Called on all required nodes with files /// by [_syncDownload]. Items enqueued/downloading/failed are validated and cleaned /// up before re-initiating download if needed. @@ -1571,9 +1612,8 @@ class DownloadsSyncService { path_helper.join(downloadLocation.currentPath, subDirectory); } - // We still use imageIds for filenames despite switching to blurhashes as - // blurhashes can include characters that filesystems don't support - final fileName = "${_filesystemSafe(item.imageId)!}.image"; + // Always use a new, unique filename when creating image downloads + final fileName = "${const Uuid().v4()}.image"; _isar.writeTxnSync(() { DownloadItem? canonItem = diff --git a/lib/services/jellyfin_api_helper.dart b/lib/services/jellyfin_api_helper.dart index 3864e4b36..8573857d0 100644 --- a/lib/services/jellyfin_api_helper.dart +++ b/lib/services/jellyfin_api_helper.dart @@ -106,6 +106,7 @@ class JellyfinApiHelper { List? itemIds, String? filters, String? fields, + bool? recursive, /// The record index to start at. All items with a lower index will be /// dropped from the results. @@ -127,6 +128,8 @@ class JellyfinApiHelper { assert(itemIds == null || parentItem == null); fields ??= defaultFields; // explicitly set the default fields, if we pass `null` to [JellyfinAPI.getItems] it will **not** apply the default fields, since the argument *is* provided. + recursive ??= true; + if (parentItem != null) { _jellyfinApiHelperLogger.fine("Getting children of ${parentItem.name}"); } else if (itemIds != null) { @@ -148,14 +151,14 @@ class JellyfinApiHelper { userId: currentUserId, parentId: parentItem.id, includeItemTypes: includeItemTypes, - recursive: true, + recursive: recursive, fields: fields, ); } else if (includeItemTypes == "MusicArtist") { // For artists, we need to use a different endpoint response = await api.getAlbumArtists( parentId: parentItem?.id, - recursive: true, + recursive: recursive, sortBy: sortBy, sortOrder: sortOrder, searchTerm: searchTerm, @@ -172,7 +175,7 @@ class JellyfinApiHelper { userId: currentUserId, albumArtistIds: parentItem?.id, includeItemTypes: includeItemTypes, - recursive: true, + recursive: recursive, sortBy: sortBy, sortOrder: sortOrder, searchTerm: searchTerm, @@ -195,7 +198,7 @@ class JellyfinApiHelper { userId: currentUserId, genreIds: parentItem?.id, includeItemTypes: includeItemTypes, - recursive: true, + recursive: recursive, sortBy: sortBy, sortOrder: sortOrder, searchTerm: searchTerm, @@ -211,7 +214,7 @@ class JellyfinApiHelper { userId: currentUserId, parentId: parentItem?.id, includeItemTypes: includeItemTypes, - recursive: true, + recursive: recursive, sortBy: sortBy, sortOrder: sortOrder, searchTerm: searchTerm,