Backport pull request #12934 from jellyfin/release-10.10.z

Fix playlists

Original-merge: 8bee67f1f8dab604d745b3d077330085f7f111d4

Merged-by: crobibero <cody@robibe.ro>

Backported-by: Joshua M. Boniface <joshua@boniface.me>

9 files changed, 107 insertions, 42 deletions

diff --git a/Emby.Server.Implementations/ConfigurationOptions.cs b/Emby.Server.Implementations/ConfigurationOptions.cs
index 91791a1c8..a06f6e7fe 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 47ff22c0b..daeb7fed8 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 newList = playlist.LinkedChildren.ToList();
+            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 e6f23b136..1ab36ccc6 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<BaseItemDto>(
diff --git a/Jellyfin.Server/Migrations/MigrationRunner.cs b/Jellyfin.Server/Migrations/MigrationRunner.cs
index 9d4441ac3..2ab130eef 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)
         };
 
         /// <summary>
diff --git a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs
index 3655a610d..192c170b2 100644
--- a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs
+++ b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs
@@ -15,12 +15,12 @@ namespace Jellyfin.Server.Migrations.Routines;
 /// </summary>
 internal class FixPlaylistOwner : IMigrationRoutine
 {
-    private readonly ILogger<RemoveDuplicateExtras> _logger;
+    private readonly ILogger<FixPlaylistOwner> _logger;
     private readonly ILibraryManager _libraryManager;
     private readonly IPlaylistManager _playlistManager;
 
     public FixPlaylistOwner(
-        ILogger<RemoveDuplicateExtras> logger,
+        ILogger<FixPlaylistOwner> 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 000000000..99047b2a2
--- /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;
+
+/// <summary>
+/// Remove duplicate playlist entries.
+/// </summary>
+internal class RemoveDuplicatePlaylistChildren : IMigrationRoutine
+{
+    private readonly ILogger<RemoveDuplicatePlaylistChildren> _logger;
+    private readonly ILibraryManager _libraryManager;
+    private readonly IPlaylistManager _playlistManager;
+
+    public RemoveDuplicatePlaylistChildren(
+        ILogger<RemoveDuplicatePlaylistChildren> logger,
+        ILibraryManager libraryManager,
+        IPlaylistManager playlistManager)
+    {
+        _logger = logger;
+        _libraryManager = libraryManager;
+        _playlistManager = playlistManager;
+    }
+
+    /// <inheritdoc/>
+    public Guid Id => Guid.Parse("{96C156A2-7A13-4B3B-A8B8-FB80C94D20C0}");
+
+    /// <inheritdoc/>
+    public string Name => "RemoveDuplicatePlaylistChildren";
+
+    /// <inheritdoc/>
+    public bool PerformOnNewInstall => false;
+
+    /// <inheritdoc/>
+    public void Perform()
+    {
+        var playlists = _libraryManager.GetItemList(new InternalItemsQuery
+        {
+            IncludeItemTypes = [BaseItemKind.Playlist]
+        })
+        .Cast<Playlist>()
+        .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 fd5fef3dc..98e4f525f 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; }
-
         /// <summary>
         /// Gets or sets the linked item id.
         /// </summary>
@@ -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 f8049cd48..e4806109a 100644
--- a/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs
+++ b/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs
@@ -50,11 +50,6 @@ namespace MediaBrowser.Controller.Extensions
         public const string FfmpegPathKey = "ffmpeg";
 
         /// <summary>
-        /// The key for a setting that indicates whether playlists should allow duplicate entries.
-        /// </summary>
-        public const string PlaylistsAllowDuplicatesKey = "playlists:allowDuplicates";
-
-        /// <summary>
         /// The key for a setting that indicates whether kestrel should bind to a unix socket.
         /// </summary>
         public const string BindToUnixSocketKey = "kestrel:socket";
@@ -121,14 +116,6 @@ namespace MediaBrowser.Controller.Extensions
             => configuration.GetValue<bool>(FfmpegImgExtractPerfTradeoffKey);
 
         /// <summary>
-        /// Gets a value indicating whether playlists should allow duplicate entries from the <see cref="IConfiguration"/>.
-        /// </summary>
-        /// <param name="configuration">The configuration to read the setting from.</param>
-        /// <returns>True if playlists should allow duplicates, otherwise false.</returns>
-        public static bool DoPlaylistsAllowDuplicates(this IConfiguration configuration)
-            => configuration.GetValue<bool>(PlaylistsAllowDuplicatesKey);
-
-        /// <summary>
         /// Gets a value indicating whether kestrel should bind to a unix socket from the <see cref="IConfiguration" />.
         /// </summary>
         /// <param name="configuration">The configuration to read the setting from.</param>
diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
index 038cbd2d6..497c4a511 100644
--- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
+++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
@@ -92,8 +92,9 @@ namespace MediaBrowser.Controller.Playlists
         /// <param name="playlistId">The playlist identifier.</param>
         /// <param name="entryId">The entry identifier.</param>
         /// <param name="newIndex">The new index.</param>
+        /// <param name="callingUserId">The calling user.</param>
         /// <returns>Task.</returns>
-        Task MoveItemAsync(string playlistId, string entryId, int newIndex);
+        Task MoveItemAsync(string playlistId, string entryId, int newIndex, Guid callingUserId);
 
         /// <summary>
         /// Removed all playlists of a user.