From 6e7118eff1e6bc9c5ca70d80e5ff5e6eff7c90e5 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 19 Nov 2024 15:43:18 -0500 Subject: [PATCH] Backport pull request #12934 from jellyfin/release-10.10.z Fix playlists Original-merge: 8bee67f1f8dab604d745b3d077330085f7f111d4 Merged-by: crobibero Backported-by: Joshua M. Boniface --- .../ConfigurationOptions.cs | 1 - .../Playlists/PlaylistManager.cs | 40 +++++++---- .../Controllers/PlaylistsController.cs | 10 +-- Jellyfin.Server/Migrations/MigrationRunner.cs | 3 +- .../Migrations/Routines/FixPlaylistOwner.cs | 4 +- .../RemoveDuplicatePlaylistChildren.cs | 68 +++++++++++++++++++ .../Entities/LinkedChild.cs | 7 +- .../Extensions/ConfigurationExtensions.cs | 13 ---- .../Playlists/IPlaylistManager.cs | 3 +- 9 files changed, 107 insertions(+), 42 deletions(-) create mode 100644 Jellyfin.Server/Migrations/Routines/RemoveDuplicatePlaylistChildren.cs diff --git a/Emby.Server.Implementations/ConfigurationOptions.cs b/Emby.Server.Implementations/ConfigurationOptions.cs index 91791a1c82..a06f6e7fe9 100644 --- a/Emby.Server.Implementations/ConfigurationOptions.cs +++ b/Emby.Server.Implementations/ConfigurationOptions.cs @@ -17,7 +17,6 @@ namespace Emby.Server.Implementations { DefaultRedirectKey, "web/" }, { FfmpegProbeSizeKey, "1G" }, { FfmpegAnalyzeDurationKey, "200M" }, - { PlaylistsAllowDuplicatesKey, bool.FalseString }, { BindToUnixSocketKey, bool.FalseString }, { SqliteCacheSizeKey, "20000" }, { FfmpegSkipValidationKey, bool.FalseString }, diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 47ff22c0b3..daeb7fed88 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -216,14 +216,11 @@ namespace Emby.Server.Implementations.Playlists var newItems = GetPlaylistItems(newItemIds, user, options) .Where(i => i.SupportsAddingToPlaylist); - // Filter out duplicate items, if necessary - if (!_appConfig.DoPlaylistsAllowDuplicates()) - { - var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet(); - newItems = newItems - .Where(i => !existingIds.Contains(i.Id)) - .Distinct(); - } + // Filter out duplicate items + var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet(); + newItems = newItems + .Where(i => !existingIds.Contains(i.Id)) + .Distinct(); // Create a list of the new linked children to add to the playlist var childrenToAdd = newItems @@ -269,7 +266,7 @@ namespace Emby.Server.Implementations.Playlists var idList = entryIds.ToList(); - var removals = children.Where(i => idList.Contains(i.Item1.Id)); + var removals = children.Where(i => idList.Contains(i.Item1.ItemId?.ToString("N", CultureInfo.InvariantCulture))); playlist.LinkedChildren = children.Except(removals) .Select(i => i.Item1) @@ -286,26 +283,39 @@ namespace Emby.Server.Implementations.Playlists RefreshPriority.High); } - public async Task MoveItemAsync(string playlistId, string entryId, int newIndex) + public async Task MoveItemAsync(string playlistId, string entryId, int newIndex, Guid callingUserId) { if (_libraryManager.GetItemById(playlistId) is not Playlist playlist) { throw new ArgumentException("No Playlist exists with the supplied Id"); } + var user = _userManager.GetUserById(callingUserId); var children = playlist.GetManageableItems().ToList(); + var accessibleChildren = children.Where(c => c.Item2.IsVisible(user)).ToArray(); - var oldIndex = children.FindIndex(i => string.Equals(entryId, i.Item1.Id, StringComparison.OrdinalIgnoreCase)); + var oldIndexAll = children.FindIndex(i => string.Equals(entryId, i.Item1.ItemId?.ToString("N", CultureInfo.InvariantCulture), StringComparison.OrdinalIgnoreCase)); + var oldIndexAccessible = accessibleChildren.FindIndex(i => string.Equals(entryId, i.Item1.ItemId?.ToString("N", CultureInfo.InvariantCulture), StringComparison.OrdinalIgnoreCase)); - if (oldIndex == newIndex) + if (oldIndexAccessible == newIndex) { return; } - var item = playlist.LinkedChildren[oldIndex]; + var newPriorItemIndex = newIndex > oldIndexAccessible ? newIndex : newIndex - 1 < 0 ? 0 : newIndex - 1; + var newPriorItemId = accessibleChildren[newPriorItemIndex].Item1.ItemId; + var newPriorItemIndexOnAllChildren = children.FindIndex(c => c.Item1.ItemId.Equals(newPriorItemId)); + var adjustedNewIndex = newPriorItemIndexOnAllChildren + 1; + + var item = playlist.LinkedChildren.FirstOrDefault(i => string.Equals(entryId, i.ItemId?.ToString("N", CultureInfo.InvariantCulture), StringComparison.OrdinalIgnoreCase)); + if (item is null) + { + _logger.LogWarning("Modified item not found in playlist. ItemId: {ItemId}, PlaylistId: {PlaylistId}", item.ItemId, playlistId); + + return; + } var newList = playlist.LinkedChildren.ToList(); - newList.Remove(item); if (newIndex >= newList.Count) @@ -314,7 +324,7 @@ namespace Emby.Server.Implementations.Playlists } else { - newList.Insert(newIndex, item); + newList.Insert(adjustedNewIndex, item); } playlist.LinkedChildren = [.. newList]; diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index e6f23b1364..1ab36ccc64 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using System.Globalization; using System.Linq; using System.Threading.Tasks; using Jellyfin.Api.Attributes; @@ -426,7 +427,7 @@ public class PlaylistsController : BaseJellyfinApiController return Forbid(); } - await _playlistManager.MoveItemAsync(playlistId, itemId, newIndex).ConfigureAwait(false); + await _playlistManager.MoveItemAsync(playlistId, itemId, newIndex, callingUserId).ConfigureAwait(false); return NoContent(); } @@ -514,7 +515,8 @@ public class PlaylistsController : BaseJellyfinApiController return Forbid(); } - var items = playlist.GetManageableItems().ToArray(); + var user = _userManager.GetUserById(callingUserId); + var items = playlist.GetManageableItems().Where(i => i.Item2.IsVisible(user)).ToArray(); var count = items.Length; if (startIndex.HasValue) { @@ -529,11 +531,11 @@ public class PlaylistsController : BaseJellyfinApiController var dtoOptions = new DtoOptions { Fields = fields } .AddClientFields(User) .AddAdditionalDtoOptions(enableImages, enableUserData, imageTypeLimit, enableImageTypes); - var user = _userManager.GetUserById(callingUserId); + var dtos = _dtoService.GetBaseItemDtos(items.Select(i => i.Item2).ToList(), dtoOptions, user); for (int index = 0; index < dtos.Count; index++) { - dtos[index].PlaylistItemId = items[index].Item1.Id; + dtos[index].PlaylistItemId = items[index].Item1.ItemId?.ToString("N", CultureInfo.InvariantCulture); } var result = new QueryResult( diff --git a/Jellyfin.Server/Migrations/MigrationRunner.cs b/Jellyfin.Server/Migrations/MigrationRunner.cs index 9d4441ac39..2ab130eefb 100644 --- a/Jellyfin.Server/Migrations/MigrationRunner.cs +++ b/Jellyfin.Server/Migrations/MigrationRunner.cs @@ -47,7 +47,8 @@ namespace Jellyfin.Server.Migrations typeof(Routines.AddDefaultCastReceivers), typeof(Routines.UpdateDefaultPluginRepository), typeof(Routines.FixAudioData), - typeof(Routines.MoveTrickplayFiles) + typeof(Routines.MoveTrickplayFiles), + typeof(Routines.RemoveDuplicatePlaylistChildren) }; /// diff --git a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs index 3655a610d3..192c170b26 100644 --- a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs +++ b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs @@ -15,12 +15,12 @@ namespace Jellyfin.Server.Migrations.Routines; /// internal class FixPlaylistOwner : IMigrationRoutine { - private readonly ILogger _logger; + private readonly ILogger _logger; private readonly ILibraryManager _libraryManager; private readonly IPlaylistManager _playlistManager; public FixPlaylistOwner( - ILogger logger, + ILogger logger, ILibraryManager libraryManager, IPlaylistManager playlistManager) { diff --git a/Jellyfin.Server/Migrations/Routines/RemoveDuplicatePlaylistChildren.cs b/Jellyfin.Server/Migrations/Routines/RemoveDuplicatePlaylistChildren.cs new file mode 100644 index 0000000000..99047b2a2a --- /dev/null +++ b/Jellyfin.Server/Migrations/Routines/RemoveDuplicatePlaylistChildren.cs @@ -0,0 +1,68 @@ +using System; +using System.Linq; +using System.Threading; + +using Jellyfin.Data.Enums; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Playlists; +using Microsoft.Extensions.Logging; + +namespace Jellyfin.Server.Migrations.Routines; + +/// +/// Remove duplicate playlist entries. +/// +internal class RemoveDuplicatePlaylistChildren : IMigrationRoutine +{ + private readonly ILogger _logger; + private readonly ILibraryManager _libraryManager; + private readonly IPlaylistManager _playlistManager; + + public RemoveDuplicatePlaylistChildren( + ILogger logger, + ILibraryManager libraryManager, + IPlaylistManager playlistManager) + { + _logger = logger; + _libraryManager = libraryManager; + _playlistManager = playlistManager; + } + + /// + public Guid Id => Guid.Parse("{96C156A2-7A13-4B3B-A8B8-FB80C94D20C0}"); + + /// + public string Name => "RemoveDuplicatePlaylistChildren"; + + /// + public bool PerformOnNewInstall => false; + + /// + public void Perform() + { + var playlists = _libraryManager.GetItemList(new InternalItemsQuery + { + IncludeItemTypes = [BaseItemKind.Playlist] + }) + .Cast() + .ToArray(); + + if (playlists.Length > 0) + { + foreach (var playlist in playlists) + { + var linkedChildren = playlist.LinkedChildren; + if (linkedChildren.Length > 0) + { + var nullItemChildren = linkedChildren.Where(c => c.ItemId is null); + var deduplicatedChildren = linkedChildren.DistinctBy(c => c.ItemId); + var newLinkedChildren = nullItemChildren.Concat(deduplicatedChildren); + playlist.LinkedChildren = linkedChildren; + playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).GetAwaiter().GetResult(); + _playlistManager.SavePlaylistFile(playlist); + } + } + } + } +} diff --git a/MediaBrowser.Controller/Entities/LinkedChild.cs b/MediaBrowser.Controller/Entities/LinkedChild.cs index fd5fef3dc5..98e4f525f5 100644 --- a/MediaBrowser.Controller/Entities/LinkedChild.cs +++ b/MediaBrowser.Controller/Entities/LinkedChild.cs @@ -4,7 +4,6 @@ using System; using System.Globalization; -using System.Text.Json.Serialization; namespace MediaBrowser.Controller.Entities { @@ -12,7 +11,6 @@ namespace MediaBrowser.Controller.Entities { public LinkedChild() { - Id = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture); } public string Path { get; set; } @@ -21,9 +19,6 @@ namespace MediaBrowser.Controller.Entities public string LibraryItemId { get; set; } - [JsonIgnore] - public string Id { get; set; } - /// /// Gets or sets the linked item id. /// @@ -31,6 +26,8 @@ namespace MediaBrowser.Controller.Entities public static LinkedChild Create(BaseItem item) { + ArgumentNullException.ThrowIfNull(item); + var child = new LinkedChild { Path = item.Path, diff --git a/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs b/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs index f8049cd488..e4806109a1 100644 --- a/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs +++ b/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs @@ -49,11 +49,6 @@ namespace MediaBrowser.Controller.Extensions /// public const string FfmpegPathKey = "ffmpeg"; - /// - /// The key for a setting that indicates whether playlists should allow duplicate entries. - /// - public const string PlaylistsAllowDuplicatesKey = "playlists:allowDuplicates"; - /// /// The key for a setting that indicates whether kestrel should bind to a unix socket. /// @@ -120,14 +115,6 @@ namespace MediaBrowser.Controller.Extensions public static bool GetFFmpegImgExtractPerfTradeoff(this IConfiguration configuration) => configuration.GetValue(FfmpegImgExtractPerfTradeoffKey); - /// - /// Gets a value indicating whether playlists should allow duplicate entries from the . - /// - /// The configuration to read the setting from. - /// True if playlists should allow duplicates, otherwise false. - public static bool DoPlaylistsAllowDuplicates(this IConfiguration configuration) - => configuration.GetValue(PlaylistsAllowDuplicatesKey); - /// /// Gets a value indicating whether kestrel should bind to a unix socket from the . /// diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index 038cbd2d67..497c4a511e 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -92,8 +92,9 @@ namespace MediaBrowser.Controller.Playlists /// The playlist identifier. /// The entry identifier. /// The new index. + /// The calling user. /// Task. - Task MoveItemAsync(string playlistId, string entryId, int newIndex); + Task MoveItemAsync(string playlistId, string entryId, int newIndex, Guid callingUserId); /// /// Removed all playlists of a user.