Fix playlists library and migration (#9770)

7 files changed, 30 insertions, 51 deletions

diff --git a/Emby.Server.Implementations/Library/UserViewManager.cs b/Emby.Server.Implementations/Library/UserViewManager.cs
index 17f1d1905..2c3dc1857 100644
--- a/Emby.Server.Implementations/Library/UserViewManager.cs
+++ b/Emby.Server.Implementations/Library/UserViewManager.cs
@@ -46,10 +46,9 @@ namespace Emby.Server.Implementations.Library
         public Folder[] GetUserViews(UserViewQuery query)
         {
             var user = _userManager.GetUserById(query.UserId);
-
             if (user is null)
             {
-                throw new ArgumentException("User Id specified in the query does not exist.", nameof(query));
+                throw new ArgumentException("User id specified in the query does not exist.", nameof(query));
             }
 
             var folders = _libraryManager.GetUserRootFolder()
@@ -58,7 +57,6 @@ namespace Emby.Server.Implementations.Library
                 .ToList();
 
             var groupedFolders = new List<ICollectionFolder>();
-
             var list = new List<Folder>();
 
             foreach (var folder in folders)
@@ -66,6 +64,20 @@ namespace Emby.Server.Implementations.Library
                 var collectionFolder = folder as ICollectionFolder;
                 var folderViewType = collectionFolder?.CollectionType;
 
+                // Playlist library requires special handling because the folder only refrences user playlists
+                if (string.Equals(folderViewType, CollectionType.Playlists, StringComparison.OrdinalIgnoreCase))
+                {
+                    var items = folder.GetItemList(new InternalItemsQuery(user)
+                    {
+                        ParentId = folder.ParentId
+                    });
+
+                    if (!items.Any(item => item.IsVisible(user)))
+                    {
+                        continue;
+                    }
+                }
+
                 if (UserView.IsUserSpecific(folder))
                 {
                     list.Add(_libraryManager.GetNamedView(user, folder.Name, folder.Id, folderViewType, null));
@@ -132,14 +144,12 @@ namespace Emby.Server.Implementations.Library
             }
 
             var sorted = _libraryManager.Sort(list, user, new[] { ItemSortBy.SortName }, SortOrder.Ascending).ToList();
-
             var orders = user.GetPreferenceValues<Guid>(PreferenceKind.OrderedViews);
 
             return list
                 .OrderBy(i =>
                 {
                     var index = Array.IndexOf(orders, i.Id);
-
                     if (index == -1
                         && i is UserView view
                         && !view.DisplayParentId.Equals(default))
diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs
index adb8ac732..702f8d45b 100644
--- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs
+++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs
@@ -67,9 +67,8 @@ namespace Emby.Server.Implementations.Playlists
         public async Task<PlaylistCreationResult> CreatePlaylist(PlaylistCreationRequest options)
         {
             var name = options.Name;
-
             var folderName = _fileSystem.GetValidFilename(name);
-            var parentFolder = GetPlaylistsFolder(Guid.Empty);
+            var parentFolder = GetPlaylistsFolder(options.UserId);
             if (parentFolder is null)
             {
                 throw new ArgumentException(nameof(parentFolder));
@@ -80,7 +79,6 @@ namespace Emby.Server.Implementations.Playlists
                 foreach (var itemId in options.ItemIdList)
                 {
                     var item = _libraryManager.GetItemById(itemId);
-
                     if (item is null)
                     {
                         throw new ArgumentException("No item exists with the supplied Id");
@@ -121,7 +119,6 @@ namespace Emby.Server.Implementations.Playlists
             }
 
             var user = _userManager.GetUserById(options.UserId);
-
             var path = Path.Combine(parentFolder.Path, folderName);
             path = GetTargetPath(path);
 
@@ -130,7 +127,6 @@ namespace Emby.Server.Implementations.Playlists
             try
             {
                 Directory.CreateDirectory(path);
-
                 var playlist = new Playlist
                 {
                     Name = name,
@@ -140,7 +136,6 @@ namespace Emby.Server.Implementations.Playlists
                 };
 
                 playlist.SetMediaType(options.MediaType);
-
                 parentFolder.AddChild(playlist);
 
                 await playlist.RefreshMetadata(new MetadataRefreshOptions(new DirectoryService(_fileSystem)) { ForceSave = true }, CancellationToken.None)
@@ -326,7 +321,8 @@ namespace Emby.Server.Implementations.Playlists
             }
         }
 
-        private void SavePlaylistFile(Playlist item)
+        /// <inheritdoc />
+        public void SavePlaylistFile(Playlist item)
         {
             // this is probably best done as a metadata provider
             // saving a file over itself will require some work to prevent this from happening when not needed
@@ -564,20 +560,5 @@ namespace Emby.Server.Implementations.Playlists
                 }
             }
         }
-
-        /// <inheritdoc />
-        public async Task UpdatePlaylistAsync(Playlist playlist)
-        {
-            var currentPlaylist = (Playlist)_libraryManager.GetItemById(playlist.Id);
-            currentPlaylist.OwnerUserId = playlist.OwnerUserId;
-            currentPlaylist.Shares = playlist.Shares;
-
-            await playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).ConfigureAwait(false);
-
-            if (currentPlaylist.IsFile)
-            {
-                SavePlaylistFile(currentPlaylist);
-            }
-        }
     }
 }
diff --git a/Emby.Server.Implementations/Playlists/PlaylistsFolder.cs b/Emby.Server.Implementations/Playlists/PlaylistsFolder.cs
index e2f2e436f..549209715 100644
--- a/Emby.Server.Implementations/Playlists/PlaylistsFolder.cs
+++ b/Emby.Server.Implementations/Playlists/PlaylistsFolder.cs
@@ -27,11 +27,6 @@ namespace Emby.Server.Implementations.Playlists
         [JsonIgnore]
         public override string CollectionType => MediaBrowser.Model.Entities.CollectionType.Playlists;
 
-        public override bool IsVisible(User user)
-        {
-            return base.IsVisible(user) && GetChildren(user, true).Any();
-        }
-
         protected override IEnumerable<BaseItem> GetEligibleChildrenForRecursiveChildren(User user)
         {
             return base.GetEligibleChildrenForRecursiveChildren(user).OfType<Playlist>();
@@ -47,7 +42,6 @@ namespace Emby.Server.Implementations.Playlists
 
             query.Recursive = true;
             query.IncludeItemTypes = new[] { BaseItemKind.Playlist };
-            query.Parent = null;
             return LibraryManager.GetItemsResult(query);
         }
 
diff --git a/Jellyfin.Api/Controllers/ItemsController.cs b/Jellyfin.Api/Controllers/ItemsController.cs
index 377526729..d4116116b 100644
--- a/Jellyfin.Api/Controllers/ItemsController.cs
+++ b/Jellyfin.Api/Controllers/ItemsController.cs
@@ -503,6 +503,7 @@ public class ItemsController : BaseJellyfinApiController
                 }
             }
 
+            query.Parent = null;
             result = folder.GetItems(query);
         }
         else
@@ -511,10 +512,12 @@ public class ItemsController : BaseJellyfinApiController
             result = new QueryResult<BaseItem>(itemsArray);
         }
 
+        // result might include items not accessible by the user, DtoService will remove them
+        var accessibleItems = _dtoService.GetBaseItemDtos(result.Items, dtoOptions, user);
         return new QueryResult<BaseItemDto>(
             startIndex,
-            result.TotalRecordCount,
-            _dtoService.GetBaseItemDtos(result.Items, dtoOptions, user));
+            accessibleItems.Count,
+            accessibleItems);
     }
 
     /// <summary>
diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs
index 20995bf1b..8d2a738d4 100644
--- a/Jellyfin.Api/Controllers/PlaylistsController.cs
+++ b/Jellyfin.Api/Controllers/PlaylistsController.cs
@@ -65,14 +65,12 @@ public class PlaylistsController : BaseJellyfinApiController
     /// <param name="mediaType">The media type.</param>
     /// <param name="createPlaylistRequest">The create playlist payload.</param>
     /// <response code="200">Playlist created.</response>
-    /// <response code="403">User does not have permission to create playlists.</response>
     /// <returns>
     /// A <see cref="Task" /> that represents the asynchronous operation to create a playlist.
     /// The task result contains an <see cref="OkResult"/> indicating success.
     /// </returns>
     [HttpPost]
     [ProducesResponseType(StatusCodes.Status200OK)]
-    [ProducesResponseType(StatusCodes.Status403Forbidden)]
     public async Task<ActionResult<PlaylistCreationResult>> CreatePlaylist(
         [FromQuery, ParameterObsolete] string? name,
         [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder)), ParameterObsolete] IReadOnlyList<Guid> ids,
@@ -105,11 +103,9 @@ public class PlaylistsController : BaseJellyfinApiController
     /// <param name="ids">Item id, comma delimited.</param>
     /// <param name="userId">The userId.</param>
     /// <response code="204">Items added to playlist.</response>
-    /// <response code="403">User does not have permission to add items to playlist.</response>
     /// <returns>An <see cref="NoContentResult"/> on success.</returns>
     [HttpPost("{playlistId}/Items")]
     [ProducesResponseType(StatusCodes.Status204NoContent)]
-    [ProducesResponseType(StatusCodes.Status403Forbidden)]
     public async Task<ActionResult> AddToPlaylist(
         [FromRoute, Required] Guid playlistId,
         [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] Guid[] ids,
@@ -127,11 +123,9 @@ public class PlaylistsController : BaseJellyfinApiController
     /// <param name="itemId">The item id.</param>
     /// <param name="newIndex">The new index.</param>
     /// <response code="204">Item moved to new index.</response>
-    /// <response code="403">User does not have permission to move item.</response>
     /// <returns>An <see cref="NoContentResult"/> on success.</returns>
     [HttpPost("{playlistId}/Items/{itemId}/Move/{newIndex}")]
     [ProducesResponseType(StatusCodes.Status204NoContent)]
-    [ProducesResponseType(StatusCodes.Status403Forbidden)]
     public async Task<ActionResult> MoveItem(
         [FromRoute, Required] string playlistId,
         [FromRoute, Required] string itemId,
@@ -147,11 +141,9 @@ public class PlaylistsController : BaseJellyfinApiController
     /// <param name="playlistId">The playlist id.</param>
     /// <param name="entryIds">The item ids, comma delimited.</param>
     /// <response code="204">Items removed.</response>
-    /// <response code="403">User does not have permission to get playlist.</response>
     /// <returns>An <see cref="NoContentResult"/> on success.</returns>
     [HttpDelete("{playlistId}/Items")]
     [ProducesResponseType(StatusCodes.Status204NoContent)]
-    [ProducesResponseType(StatusCodes.Status403Forbidden)]
     public async Task<ActionResult> RemoveFromPlaylist(
         [FromRoute, Required] string playlistId,
         [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] string[] entryIds)
@@ -173,12 +165,10 @@ public class PlaylistsController : BaseJellyfinApiController
     /// <param name="imageTypeLimit">Optional. The max number of images to return, per image type.</param>
     /// <param name="enableImageTypes">Optional. The image types to include in the output.</param>
     /// <response code="200">Original playlist returned.</response>
-    /// <response code="403">User does not have permission to get playlist items.</response>
     /// <response code="404">Playlist not found.</response>
     /// <returns>The original playlist items.</returns>
     [HttpGet("{playlistId}/Items")]
     [ProducesResponseType(StatusCodes.Status200OK)]
-    [ProducesResponseType(StatusCodes.Status403Forbidden)]
     [ProducesResponseType(StatusCodes.Status404NotFound)]
     public ActionResult<QueryResult<BaseItemDto>> GetPlaylistItems(
         [FromRoute, Required] Guid playlistId,
diff --git a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs
index 1dd938b1b..cf3182003 100644
--- a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs
+++ b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Linq;
+using System.Threading;
 
 using Jellyfin.Data.Enums;
 using MediaBrowser.Controller.Entities;
@@ -60,13 +61,14 @@ internal class FixPlaylistOwner : IMigrationRoutine
                     {
                         playlist.OwnerUserId = guid;
                         playlist.Shares = shares.Where(x => x != firstEditShare).ToArray();
-                        _playlistManager.UpdatePlaylistAsync(playlist).GetAwaiter().GetResult();
+                        playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).GetAwaiter().GetResult();
+                        _playlistManager.SavePlaylistFile(playlist);
                     }
                 }
                 else
                 {
                     playlist.OpenAccess = true;
-                    _playlistManager.UpdatePlaylistAsync(playlist).GetAwaiter().GetResult();
+                    playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).GetAwaiter().GetResult();
                 }
             }
         }
diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
index d88943662..d1a51c2cf 100644
--- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
+++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs
@@ -66,10 +66,9 @@ namespace MediaBrowser.Controller.Playlists
         Task RemovePlaylistsAsync(Guid userId);
 
         /// <summary>
-        /// Updates a playlist.
+        /// Saves a playlist.
         /// </summary>
-        /// <param name="playlist">The updated playlist.</param>
-        /// <returns>Task.</returns>
-        Task UpdatePlaylistAsync(Playlist playlist);
+        /// <param name="item">The playlist.</param>
+        void SavePlaylistFile(Playlist item);
     }
 }