Merge pull request #2497 from mark-monteiro/1914-prevent-duplicates-in-playlists

Check for duplicates when adding items to a playlist

6 files changed, 66 insertions, 23 deletions

diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md
index 4f3624965..f195c125f 100644
--- a/CONTRIBUTORS.md
+++ b/CONTRIBUTORS.md
@@ -91,6 +91,7 @@
  - [samuel9554](https://github.com/samuel9554)
  - [scheidleon](https://github.com/scheidleon)
  - [sebPomme](https://github.com/sebPomme)
+ - [SegiH](https://github.com/SegiH)
  - [SenorSmartyPants](https://github.com/SenorSmartyPants)
  - [shemanaev](https://github.com/shemanaev)
  - [skaro13](https://github.com/skaro13)
diff --git a/Emby.Server.Implementations/ConfigurationOptions.cs b/Emby.Server.Implementations/ConfigurationOptions.cs
index d0f3d6723..31fb5ca58 100644
--- a/Emby.Server.Implementations/ConfigurationOptions.cs
+++ b/Emby.Server.Implementations/ConfigurationOptions.cs
@@ -9,7 +9,8 @@ namespace Emby.Server.Implementations
         {
             { "HttpListenerHost:DefaultRedirectPath", "web/index.html" },
             { FfmpegProbeSizeKey, "1G" },
-            { FfmpegAnalyzeDurationKey, "200M" }
+            { FfmpegAnalyzeDurationKey, "200M" },
+            { PlaylistsAllowDuplicatesKey, bool.TrueString }
         };
     }
 }
diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs
index ad7a5005e..9b1510ac9 100644
--- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs
+++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs
@@ -8,12 +8,14 @@ using System.Threading.Tasks;
 using MediaBrowser.Controller.Dto;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Entities.Audio;
+using MediaBrowser.Controller.Extensions;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Playlists;
 using MediaBrowser.Controller.Providers;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Model.Playlists;
+using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
 using PlaylistsNET.Content;
 using PlaylistsNET.Models;
@@ -28,6 +30,7 @@ namespace Emby.Server.Implementations.Playlists
         private readonly ILogger _logger;
         private readonly IUserManager _userManager;
         private readonly IProviderManager _providerManager;
+        private readonly IConfiguration _appConfig;
 
         public PlaylistManager(
             ILibraryManager libraryManager,
@@ -35,7 +38,8 @@ namespace Emby.Server.Implementations.Playlists
             ILibraryMonitor iLibraryMonitor,
             ILogger<PlaylistManager> logger,
             IUserManager userManager,
-            IProviderManager providerManager)
+            IProviderManager providerManager,
+            IConfiguration appConfig)
         {
             _libraryManager = libraryManager;
             _fileSystem = fileSystem;
@@ -43,6 +47,7 @@ namespace Emby.Server.Implementations.Playlists
             _logger = logger;
             _userManager = userManager;
             _providerManager = providerManager;
+            _appConfig = appConfig;
         }
 
         public IEnumerable<Playlist> GetPlaylists(Guid userId)
@@ -177,7 +182,7 @@ namespace Emby.Server.Implementations.Playlists
             return Playlist.GetPlaylistItems(playlistMediaType, items, user, options);
         }
 
-        public void AddToPlaylist(string playlistId, IEnumerable<Guid> itemIds, Guid userId)
+        public void AddToPlaylist(string playlistId, ICollection<Guid> itemIds, Guid userId)
         {
             var user = userId.Equals(Guid.Empty) ? null : _userManager.GetUserById(userId);
 
@@ -187,37 +192,59 @@ namespace Emby.Server.Implementations.Playlists
             });
         }
 
-        private void AddToPlaylistInternal(string playlistId, IEnumerable<Guid> itemIds, User user, DtoOptions options)
+        private void AddToPlaylistInternal(string playlistId, ICollection<Guid> newItemIds, User user, DtoOptions options)
         {
-            var playlist = _libraryManager.GetItemById(playlistId) as Playlist;
+            // Retrieve the existing playlist
+            var playlist = _libraryManager.GetItemById(playlistId) as Playlist
+                ?? throw new ArgumentException("No Playlist exists with Id " + playlistId);
 
-            if (playlist == null)
+            // Retrieve all the items to be added to the playlist
+            var newItems = GetPlaylistItems(newItemIds, playlist.MediaType, user, options)
+                .Where(i => i.SupportsAddingToPlaylist);
+
+            // Filter out duplicate items, if necessary
+            if (!_appConfig.DoPlaylistsAllowDuplicates())
             {
-                throw new ArgumentException("No Playlist exists with the supplied Id");
+                var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet();
+                newItems = newItems
+                    .Where(i => !existingIds.Contains(i.Id))
+                    .Distinct();
             }
 
-            var list = new List<LinkedChild>();
-
-            var items = GetPlaylistItems(itemIds, playlist.MediaType, user, options)
-                .Where(i => i.SupportsAddingToPlaylist)
+            // Create a list of the new linked children to add to the playlist
+            var childrenToAdd = newItems
+                .Select(i => LinkedChild.Create(i))
                 .ToList();
 
-            foreach (var item in items)
+            // Log duplicates that have been ignored, if any
+            int numDuplicates = newItemIds.Count - childrenToAdd.Count;
+            if (numDuplicates > 0)
             {
-                list.Add(LinkedChild.Create(item));
+                _logger.LogWarning("Ignored adding {DuplicateCount} duplicate items to playlist {PlaylistName}.", numDuplicates, playlist.Name);
             }
 
-            var newList = playlist.LinkedChildren.ToList();
-            newList.AddRange(list);
-            playlist.LinkedChildren = newList.ToArray();
+            // Do nothing else if there are no items to add to the playlist
+            if (childrenToAdd.Count == 0)
+            {
+                return;
+            }
+
+            // Create a new array with the updated playlist items
+            var newLinkedChildren = new LinkedChild[playlist.LinkedChildren.Length + childrenToAdd.Count];
+            playlist.LinkedChildren.CopyTo(newLinkedChildren, 0);
+            childrenToAdd.CopyTo(newLinkedChildren, playlist.LinkedChildren.Length);
 
+            // Update the playlist in the repository
+            playlist.LinkedChildren = newLinkedChildren;
             playlist.UpdateToRepository(ItemUpdateType.MetadataEdit, CancellationToken.None);
 
+            // Update the playlist on disk
             if (playlist.IsFile)
             {
                 SavePlaylistFile(playlist);
             }
 
+            // Refresh playlist metadata
             _providerManager.QueueRefresh(
                 playlist.Id,
                 new MetadataRefreshOptions(new DirectoryService(_fileSystem))
diff --git a/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs b/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs
index 76c9b4b26..48316499a 100644
--- a/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs
+++ b/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs
@@ -13,24 +13,37 @@ namespace MediaBrowser.Controller.Extensions
         public const string FfmpegProbeSizeKey = "FFmpeg:probesize";
 
         /// <summary>
-        /// The key for the FFmpeg analyse duration option.
+        /// The key for the FFmpeg analyze duration option.
         /// </summary>
         public const string FfmpegAnalyzeDurationKey = "FFmpeg:analyzeduration";
 
         /// <summary>
-        /// Retrieves the FFmpeg probe size from the <see cref="IConfiguration" />.
+        /// The key for a setting that indicates whether playlists should allow duplicate entries.
         /// </summary>
-        /// <param name="configuration">This configuration.</param>
+        public const string PlaylistsAllowDuplicatesKey = "playlists:allowDuplicates";
+
+        /// <summary>
+        /// Gets the FFmpeg probe size from the <see cref="IConfiguration" />.
+        /// </summary>
+        /// <param name="configuration">The configuration to read the setting from.</param>
         /// <returns>The FFmpeg probe size option.</returns>
         public static string GetFFmpegProbeSize(this IConfiguration configuration)
             => configuration[FfmpegProbeSizeKey];
 
         /// <summary>
-        /// Retrieves the FFmpeg analyse duration from the <see cref="IConfiguration" />.
+        /// Gets the FFmpeg analyze duration from the <see cref="IConfiguration" />.
         /// </summary>
-        /// <param name="configuration">This configuration.</param>
-        /// <returns>The FFmpeg analyse duration option.</returns>
+        /// <param name="configuration">The configuration to read the setting from.</param>
+        /// <returns>The FFmpeg analyze duration option.</returns>
         public static string GetFFmpegAnalyzeDuration(this IConfiguration configuration)
             => configuration[FfmpegAnalyzeDurationKey];
+
+        /// <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);
     }
 }
diff --git a/MediaBrowser.Controller/MediaBrowser.Controller.csproj b/MediaBrowser.Controller/MediaBrowser.Controller.csproj
index 88e9055e8..bcca9e4a1 100644
--- a/MediaBrowser.Controller/MediaBrowser.Controller.csproj
+++ b/MediaBrowser.Controller/MediaBrowser.Controller.csproj
@@ -9,6 +9,7 @@
 
   <ItemGroup>
     <PackageReference Include="Microsoft.Extensions.Configuration.Abstractions" Version="3.1.1" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="3.1.1" />
   </ItemGroup>
 
   <ItemGroup>
diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
index 5001f6842..544cd2643 100644
--- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
+++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
@@ -29,7 +29,7 @@ namespace MediaBrowser.Controller.Playlists
         /// <param name="itemIds">The item ids.</param>
         /// <param name="userId">The user identifier.</param>
         /// <returns>Task.</returns>
-        void AddToPlaylist(string playlistId, IEnumerable<Guid> itemIds, Guid userId);
+        void AddToPlaylist(string playlistId, ICollection<Guid> itemIds, Guid userId);
 
         /// <summary>
         /// Removes from playlist.