Apply review suggestions

Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs

@@ -1,7 +1,5 @@
 #nullable disable
 
-#pragma warning disable CS1591
-
 using System;
 using System.IO;
 using System.Linq;
@@ -11,7 +9,6 @@ using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Playlists;
 using MediaBrowser.Controller.Resolvers;
 using MediaBrowser.LocalMetadata.Savers;
-using MediaBrowser.Model.Entities;
 
 namespace Emby.Server.Implementations.Library.Resolvers
 {
@@ -20,11 +17,11 @@ namespace Emby.Server.Implementations.Library.Resolvers
     /// </summary>
     public class PlaylistResolver : GenericFolderResolver<Playlist>
     {
-        private CollectionType?[] _musicPlaylistCollectionTypes =
+        private readonly CollectionType?[] _musicPlaylistCollectionTypes =
-        {
+        [
             null,
             CollectionType.music
-        };
+        ];
 
         /// <inheritdoc/>
         protected override Playlist Resolve(ItemResolveArgs args)

Emby.Server.Implementations/Playlists/PlaylistManager.cs

@@ -134,8 +134,8 @@ namespace Emby.Server.Implementations.Playlists
                     Name = name,
                     Path = path,
                     OwnerUserId = options.UserId,
-                    Shares = options.Shares ?? [],
+                    Shares = options.Users ?? [],
-                    OpenAccess = options.OpenAccess ?? false
+                    OpenAccess = options.Public ?? false
                 };
 
                 playlist.SetMediaType(options.MediaType);
@@ -171,9 +171,9 @@ namespace Emby.Server.Implementations.Playlists
             return path;
         }
 
-        private List<BaseItem> GetPlaylistItems(IEnumerable<Guid> itemIds, MediaType playlistMediaType, User user, DtoOptions options)
+        private IReadOnlyList<BaseItem> GetPlaylistItems(IEnumerable<Guid> itemIds, MediaType playlistMediaType, User user, DtoOptions options)
         {
-            var items = itemIds.Select(i => _libraryManager.GetItemById(i)).Where(i => i is not null);
+            var items = itemIds.Select(_libraryManager.GetItemById).Where(i => i is not null);
 
             return Playlist.GetPlaylistItems(playlistMediaType, items, user, options);
         }
@@ -556,11 +556,11 @@ namespace Emby.Server.Implementations.Playlists
             await UpdatePlaylist(playlist).ConfigureAwait(false);
         }
 
-        public async Task AddToShares(Guid playlistId, Guid userId, Share share)
+        public async Task AddToShares(Guid playlistId, Guid userId, UserPermissions share)
         {
             var playlist = GetPlaylist(userId, playlistId);
             var shares = playlist.Shares.ToList();
-            var existingUserShare = shares.FirstOrDefault(s => s.UserId?.Equals(share.UserId, StringComparison.OrdinalIgnoreCase) ?? false);
+            var existingUserShare = shares.FirstOrDefault(s => s.UserId.Equals(share.UserId, StringComparison.OrdinalIgnoreCase));
             if (existingUserShare is not null)
             {
                 shares.Remove(existingUserShare);
@@ -571,7 +571,7 @@ namespace Emby.Server.Implementations.Playlists
             await UpdatePlaylist(playlist).ConfigureAwait(false);
         }
 
-        public async Task RemoveFromShares(Guid playlistId, Guid userId, Share share)
+        public async Task RemoveFromShares(Guid playlistId, Guid userId, UserPermissions share)
         {
             var playlist = GetPlaylist(userId, playlistId);
             var shares = playlist.Shares.ToList();

Jellyfin.Api/Controllers/PlaylistsController.cs

@@ -93,32 +93,32 @@ public class PlaylistsController : BaseJellyfinApiController
             ItemIdList = ids,
             UserId = userId.Value,
             MediaType = mediaType ?? createPlaylistRequest?.MediaType,
-            Shares = createPlaylistRequest?.Shares.ToArray(),
+            Users = createPlaylistRequest?.Users.ToArray() ?? [],
-            OpenAccess = createPlaylistRequest?.OpenAccess
+            Public = createPlaylistRequest?.Public
         }).ConfigureAwait(false);
 
         return result;
     }
 
     /// <summary>
-    /// Get a playlist's shares.
+    /// Get a playlist's users.
     /// </summary>
     /// <param name="playlistId">The playlist id.</param>
     /// <returns>
-    /// A list of <see cref="Share"/> objects.
+    /// A list of <see cref="UserPermissions"/> objects.
     /// </returns>
-    [HttpGet("{playlistId}/Shares")]
+    [HttpGet("{playlistId}/User")]
     [ProducesResponseType(StatusCodes.Status200OK)]
-    public IReadOnlyList<Share> GetPlaylistShares(
+    public IReadOnlyList<UserPermissions> GetPlaylistUsers(
         [FromRoute, Required] Guid playlistId)
     {
         var userId = RequestHelpers.GetUserId(User, default);
 
         var playlist = _playlistManager.GetPlaylist(userId, playlistId);
         var isPermitted = playlist.OwnerUserId.Equals(userId)
-            || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(userId) ?? false));
+            || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(userId));
 
-        return isPermitted ? playlist.Shares : new List<Share>();
+        return isPermitted ? playlist.Shares : [];
     }
 
     /// <summary>
@@ -131,14 +131,14 @@ public class PlaylistsController : BaseJellyfinApiController
     /// </returns>
     [HttpPost("{playlistId}/ToggleOpenAccess")]
     [ProducesResponseType(StatusCodes.Status200OK)]
-    public async Task<ActionResult> ToggleopenAccess(
+    public async Task<ActionResult> ToggleOpenAccess(
         [FromRoute, Required] Guid playlistId)
     {
         var callingUserId = RequestHelpers.GetUserId(User, default);
 
         var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId);
         var isPermitted = playlist.OwnerUserId.Equals(callingUserId)
-            || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false));
+            || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId));
 
         if (!isPermitted)
         {
@@ -151,35 +151,34 @@ public class PlaylistsController : BaseJellyfinApiController
     }
 
     /// <summary>
-    /// Adds shares to a playlist's shares.
+    /// Upsert a user to a playlist's users.
     /// </summary>
     /// <param name="playlistId">The playlist id.</param>
-    /// <param name="shares">The shares.</param>
+    /// <param name="userId">The user id.</param>
+    /// <param name="canEdit">Edit permission.</param>
     /// <returns>
-    /// A <see cref="Task" /> that represents the asynchronous operation to add shares to a playlist.
+    /// A <see cref="Task" /> that represents the asynchronous operation to upsert an user to a playlist.
     /// The task result contains an <see cref="OkResult"/> indicating success.
     /// </returns>
-    [HttpPost("{playlistId}/Shares")]
+    [HttpPost("{playlistId}/User/{userId}")]
     [ProducesResponseType(StatusCodes.Status200OK)]
-    public async Task<ActionResult> AddUserToPlaylistShares(
+    public async Task<ActionResult> AddUserToPlaylist(
         [FromRoute, Required] Guid playlistId,
-        [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Disallow)] Share[] shares)
+        [FromRoute, Required] Guid userId,
+        [FromBody] bool canEdit)
     {
         var callingUserId = RequestHelpers.GetUserId(User, default);
 
         var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId);
         var isPermitted = playlist.OwnerUserId.Equals(callingUserId)
-            || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false));
+            || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId));
 
         if (!isPermitted)
         {
             return Unauthorized("Unauthorized access");
         }
 
-        foreach (var share in shares)
+        await _playlistManager.AddToShares(playlistId, callingUserId, new UserPermissions(userId.ToString(), canEdit)).ConfigureAwait(false);
-        {
-            await _playlistManager.AddToShares(playlistId, callingUserId, share).ConfigureAwait(false);
-        }
 
         return NoContent();
     }
@@ -193,24 +192,24 @@ public class PlaylistsController : BaseJellyfinApiController
     /// A <see cref="Task" /> that represents the asynchronous operation to delete a user from a playlist's shares.
     /// The task result contains an <see cref="OkResult"/> indicating success.
     /// </returns>
-    [HttpDelete("{playlistId}/Shares")]
+    [HttpDelete("{playlistId}/User/{userId}")]
     [ProducesResponseType(StatusCodes.Status200OK)]
-    public async Task<ActionResult> RemoveUserFromPlaylistShares(
+    public async Task<ActionResult> RemoveUserFromPlaylist(
         [FromRoute, Required] Guid playlistId,
-        [FromBody] Guid userId)
+        [FromRoute, Required] Guid userId)
     {
         var callingUserId = RequestHelpers.GetUserId(User, default);
 
         var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId);
         var isPermitted = playlist.OwnerUserId.Equals(callingUserId)
-            || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false));
+            || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId));
 
         if (!isPermitted)
         {
             return Unauthorized("Unauthorized access");
         }
 
-        var share = playlist.Shares.FirstOrDefault(s => s.UserId?.Equals(userId) ?? false);
+        var share = playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId));
 
         if (share is null)
         {

Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs

@@ -15,7 +15,7 @@ public class CreatePlaylistDto
     /// <summary>
     /// Gets or sets the name of the new playlist.
     /// </summary>
-    public string? Name { get; set; }
+    public required string Name { get; set; }
 
     /// <summary>
     /// Gets or sets item ids to add to the playlist.
@@ -34,12 +34,12 @@ public class CreatePlaylistDto
     public MediaType? MediaType { get; set; }
 
     /// <summary>
-    /// Gets or sets the shares.
+    /// Gets or sets the playlist users.
     /// </summary>
-    public IReadOnlyList<Share> Shares { get; set; } = [];
+    public IReadOnlyList<UserPermissions> Users { get; set; } = [];
 
     /// <summary>
-    /// Gets or sets a value indicating whether open access is enabled.
+    /// Gets or sets a value indicating whether the playlist is public.
     /// </summary>
-    public bool OpenAccess { get; set; }
+    public bool Public { get; set; } = true;
 }

MediaBrowser.Controller/Playlists/IPlaylistManager.cs

@@ -41,7 +41,7 @@ namespace MediaBrowser.Controller.Playlists
         /// <param name="userId">The user identifier.</param>
         /// <param name="share">The share.</param>
         /// <returns>Task.</returns>
-        Task AddToShares(Guid playlistId, Guid userId, Share share);
+        Task AddToShares(Guid playlistId, Guid userId, UserPermissions share);
 
         /// <summary>
         /// Rremoves a share from the playlist.
@@ -50,7 +50,7 @@ namespace MediaBrowser.Controller.Playlists
         /// <param name="userId">The user identifier.</param>
         /// <param name="share">The share.</param>
         /// <returns>Task.</returns>
-        Task RemoveFromShares(Guid playlistId, Guid userId, Share share);
+        Task RemoveFromShares(Guid playlistId, Guid userId, UserPermissions share);
 
         /// <summary>
         /// Creates the playlist.

MediaBrowser.Controller/Playlists/Playlist.cs

@@ -32,7 +32,7 @@ namespace MediaBrowser.Controller.Playlists
 
         public Playlist()
         {
-            Shares = Array.Empty<Share>();
+            Shares = [];
             OpenAccess = false;
         }
 
@@ -40,7 +40,7 @@ namespace MediaBrowser.Controller.Playlists
 
         public bool OpenAccess { get; set; }
 
-        public IReadOnlyList<Share> Shares { get; set; }
+        public IReadOnlyList<UserPermissions> Shares { get; set; }
 
         [JsonIgnore]
         public bool IsFile => IsPlaylistFile(Path);
@@ -129,7 +129,7 @@ namespace MediaBrowser.Controller.Playlists
         protected override List<BaseItem> LoadChildren()
         {
             // Save a trip to the database
-            return new List<BaseItem>();
+            return [];
         }
 
         protected override Task ValidateChildrenInternal(IProgress<double> progress, bool recursive, bool refreshChildMetadata, MetadataRefreshOptions refreshOptions, IDirectoryService directoryService, CancellationToken cancellationToken)
@@ -144,7 +144,7 @@ namespace MediaBrowser.Controller.Playlists
 
         protected override IEnumerable<BaseItem> GetNonCachedChildren(IDirectoryService directoryService)
         {
-            return new List<BaseItem>();
+            return [];
         }
 
         public override IEnumerable<BaseItem> GetRecursiveChildren(User user, InternalItemsQuery query)
@@ -166,7 +166,7 @@ namespace MediaBrowser.Controller.Playlists
             return base.GetChildren(user, true, query);
         }
 
-        public static List<BaseItem> GetPlaylistItems(MediaType playlistMediaType, IEnumerable<BaseItem> inputItems, User user, DtoOptions options)
+        public static IReadOnlyList<BaseItem> GetPlaylistItems(MediaType playlistMediaType, IEnumerable<BaseItem> inputItems, User user, DtoOptions options)
         {
             if (user is not null)
             {

MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs

@@ -519,7 +519,7 @@ namespace MediaBrowser.LocalMetadata.Parsers
 
         private void FetchFromSharesNode(XmlReader reader, IHasShares item)
         {
-            var list = new List<Share>();
+            var list = new List<UserPermissions>();
 
             reader.MoveToContent();
             reader.Read();
@@ -565,7 +565,7 @@ namespace MediaBrowser.LocalMetadata.Parsers
                 }
             }
 
-            item.Shares = list.ToArray();
+            item.Shares = [.. list];
         }
 
         /// <summary>
@@ -830,12 +830,12 @@ namespace MediaBrowser.LocalMetadata.Parsers
         /// </summary>
         /// <param name="reader">The xml reader.</param>
         /// <returns>The share.</returns>
-        protected Share? GetShare(XmlReader reader)
+        protected UserPermissions? GetShare(XmlReader reader)
         {
-            var item = new Share();
-
             reader.MoveToContent();
             reader.Read();
+            string? userId = null;
+            var canEdit = false;
 
             // Loop through each element
             while (!reader.EOF && reader.ReadState == ReadState.Interactive)
@@ -845,10 +845,10 @@ namespace MediaBrowser.LocalMetadata.Parsers
                     switch (reader.Name)
                     {
                         case "UserId":
-                            item.UserId = reader.ReadNormalizedString();
+                            userId = reader.ReadNormalizedString();
                             break;
                         case "CanEdit":
-                            item.CanEdit = string.Equals(reader.ReadElementContentAsString(), "true", StringComparison.OrdinalIgnoreCase);
+                            canEdit = string.Equals(reader.ReadElementContentAsString(), "true", StringComparison.OrdinalIgnoreCase);
                             break;
                         default:
                             reader.Skip();
@@ -862,9 +862,9 @@ namespace MediaBrowser.LocalMetadata.Parsers
             }
 
             // This is valid
-            if (!string.IsNullOrWhiteSpace(item.UserId))
+            if (!string.IsNullOrWhiteSpace(userId))
             {
-                return item;
+                return new UserPermissions(userId, canEdit);
             }
 
             return null;

MediaBrowser.Model/Entities/IHasShares.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 
 namespace MediaBrowser.Model.Entities;
 
@@ -10,5 +10,5 @@ public interface IHasShares
     /// <summary>
     /// Gets or sets the shares.
     /// </summary>
-    IReadOnlyList<Share> Shares { get; set; }
+    IReadOnlyList<UserPermissions> Shares { get; set; }
 }

MediaBrowser.Model/Entities/Share.cs

@@ -1,17 +0,0 @@
-namespace MediaBrowser.Model.Entities;
-
-/// <summary>
-/// Class to hold data on sharing permissions.
-/// </summary>
-public class Share
-{
-    /// <summary>
-    /// Gets or sets the user id.
-    /// </summary>
-    public string? UserId { get; set; }
-
-    /// <summary>
-    /// Gets or sets a value indicating whether the user has edit permissions.
-    /// </summary>
-    public bool CanEdit { get; set; }
-}

MediaBrowser.Model/Entities/UserPermissions.cs

@@ -0,0 +1,19 @@
+namespace MediaBrowser.Model.Entities;
+
+/// <summary>
+/// Class to hold data on user permissions for lists.
+/// </summary>
+/// <param name="userId">The user id.</param>
+/// <param name="canEdit">Edit permission.</param>
+public class UserPermissions(string userId, bool canEdit = false)
+{
+    /// <summary>
+    /// Gets or sets the user id.
+    /// </summary>
+    public string UserId { get; set; } = userId;
+
+    /// <summary>
+    /// Gets or sets a value indicating whether the user has edit permissions.
+    /// </summary>
+    public bool CanEdit { get; set; } = canEdit;
+}

MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs

@@ -18,7 +18,7 @@ public class PlaylistCreationRequest
     /// <summary>
     /// Gets or sets the list of items.
     /// </summary>
-    public IReadOnlyList<Guid> ItemIdList { get; set; } = Array.Empty<Guid>();
+    public IReadOnlyList<Guid> ItemIdList { get; set; } = [];
 
     /// <summary>
     /// Gets or sets the media type.
@@ -31,12 +31,12 @@ public class PlaylistCreationRequest
     public Guid UserId { get; set; }
 
     /// <summary>
-    /// Gets or sets the shares.
+    /// Gets or sets the user permissions.
     /// </summary>
-    public IReadOnlyList<Share>? Shares { get; set; }
+    public IReadOnlyList<UserPermissions> Users { get; set; } = [];
 
     /// <summary>
-    /// Gets or sets a value indicating whether open access is enabled.
+    /// Gets or sets a value indicating whether the playlist is public.
     /// </summary>
-    public bool? OpenAccess { get; set; }
+    public bool? Public { get; set; } = true;
 }