Patch data-races and minor changes in SyncPlay

Emby.Server.Implementations/SyncPlay/GroupController.cs

@@ -45,11 +45,6 @@ namespace Emby.Server.Implementations.SyncPlay
         /// </summary>
         private readonly ILibraryManager _libraryManager;
 
-        /// <summary>
-        /// The SyncPlay manager.
-        /// </summary>
-        private readonly ISyncPlayManager _syncPlayManager;
-
         /// <summary>
         /// Internal group state.
         /// </summary>
@@ -63,19 +58,16 @@ namespace Emby.Server.Implementations.SyncPlay
         /// <param name="userManager">The user manager.</param>
         /// <param name="sessionManager">The session manager.</param>
         /// <param name="libraryManager">The library manager.</param>
-        /// <param name="syncPlayManager">The SyncPlay manager.</param>
         public GroupController(
             ILogger logger,
             IUserManager userManager,
             ISessionManager sessionManager,
-            ILibraryManager libraryManager,
-            ISyncPlayManager syncPlayManager)
+            ILibraryManager libraryManager)
         {
             _logger = logger;
             _userManager = userManager;
             _sessionManager = sessionManager;
             _libraryManager = libraryManager;
-            _syncPlayManager = syncPlayManager;
 
             _state = new IdleGroupState(_logger);
         }
@@ -168,7 +160,7 @@ namespace Emby.Server.Implementations.SyncPlay
         /// </summary>
         /// <param name="from">The current session.</param>
         /// <param name="type">The filtering type.</param>
-        /// <returns>The array of sessions matching the filter.</returns>
+        /// <returns>The list of sessions matching the filter.</returns>
         private IEnumerable<SessionInfo> FilterSessions(SessionInfo from, SyncPlayBroadcastType type)
         {
             return type switch
@@ -209,7 +201,7 @@ namespace Emby.Server.Implementations.SyncPlay
         /// <param name="user">The user.</param>
         /// <param name="queue">The queue.</param>
         /// <returns><c>true</c> if the user can access all the items in the queue, <c>false</c> otherwise.</returns>
-        private bool HasAccessToQueue(User user, IEnumerable<Guid> queue)
+        private bool HasAccessToQueue(User user, IReadOnlyList<Guid> queue)
         {
             // Check if queue is empty.
             if (!queue?.Any() ?? true)
@@ -234,7 +226,7 @@ namespace Emby.Server.Implementations.SyncPlay
             return true;
         }
 
-        private bool AllUsersHaveAccessToQueue(IEnumerable<Guid> queue)
+        private bool AllUsersHaveAccessToQueue(IReadOnlyList<Guid> queue)
         {
             // Check if queue is empty.
             if (!queue?.Any() ?? true)
@@ -262,7 +254,6 @@ namespace Emby.Server.Implementations.SyncPlay
         {
             GroupName = request.GroupName;
             AddSession(session);
-            _syncPlayManager.AddSessionToGroup(session, this);
 
             var sessionIsPlayingAnItem = session.FullNowPlayingItem != null;
 
@@ -270,7 +261,7 @@ namespace Emby.Server.Implementations.SyncPlay
 
             if (sessionIsPlayingAnItem)
             {
-                var playlist = session.NowPlayingQueue.Select(item => item.Id);
+                var playlist = session.NowPlayingQueue.Select(item => item.Id).ToList();
                 PlayQueue.Reset();
                 PlayQueue.SetPlaylist(playlist);
                 PlayQueue.SetPlayingItemById(session.FullNowPlayingItem.Id);
@@ -290,14 +281,13 @@ namespace Emby.Server.Implementations.SyncPlay
 
             _state.SessionJoined(this, _state.Type, session, cancellationToken);
 
-            _logger.LogInformation("InitGroup: {0} created group {1}.", session.Id, GroupId.ToString());
+            _logger.LogInformation("InitGroup: {SessionId} created group {GroupId}.", session.Id, GroupId.ToString());
         }
 
         /// <inheritdoc />
         public void SessionJoin(SessionInfo session, JoinGroupRequest request, CancellationToken cancellationToken)
         {
             AddSession(session);
-            _syncPlayManager.AddSessionToGroup(session, this);
 
             var updateSession = NewSyncPlayGroupUpdate(GroupUpdateType.GroupJoined, GetInfo());
             SendGroupUpdate(session, SyncPlayBroadcastType.CurrentSession, updateSession, cancellationToken);
@@ -307,7 +297,7 @@ namespace Emby.Server.Implementations.SyncPlay
 
             _state.SessionJoined(this, _state.Type, session, cancellationToken);
 
-            _logger.LogInformation("SessionJoin: {0} joined group {1}.", session.Id, GroupId.ToString());
+            _logger.LogInformation("SessionJoin: {SessionId} joined group {GroupId}.", session.Id, GroupId.ToString());
         }
 
         /// <inheritdoc />
@@ -321,7 +311,7 @@ namespace Emby.Server.Implementations.SyncPlay
 
             _state.SessionJoined(this, _state.Type, session, cancellationToken);
 
-            _logger.LogInformation("SessionRestore: {0} re-joined group {1}.", session.Id, GroupId.ToString());
+            _logger.LogInformation("SessionRestore: {SessionId} re-joined group {GroupId}.", session.Id, GroupId.ToString());
         }
 
         /// <inheritdoc />
@@ -330,7 +320,6 @@ namespace Emby.Server.Implementations.SyncPlay
             _state.SessionLeaving(this, _state.Type, session, cancellationToken);
 
             RemoveSession(session);
-            _syncPlayManager.RemoveSessionFromGroup(session, this);
 
             var updateSession = NewSyncPlayGroupUpdate(GroupUpdateType.GroupLeft, GroupId.ToString());
             SendGroupUpdate(session, SyncPlayBroadcastType.CurrentSession, updateSession, cancellationToken);
@@ -338,7 +327,7 @@ namespace Emby.Server.Implementations.SyncPlay
             var updateOthers = NewSyncPlayGroupUpdate(GroupUpdateType.UserLeft, session.UserName);
             SendGroupUpdate(session, SyncPlayBroadcastType.AllExceptCurrentSession, updateOthers, cancellationToken);
 
-            _logger.LogInformation("SessionLeave: {0} left group {1}.", session.Id, GroupId.ToString());
+            _logger.LogInformation("SessionLeave: {SessionId} left group {GroupId}.", session.Id, GroupId.ToString());
         }
 
         /// <inheritdoc />
@@ -347,27 +336,21 @@ namespace Emby.Server.Implementations.SyncPlay
             // The server's job is to maintain a consistent state for clients to reference
             // and notify clients of state changes. The actual syncing of media playback
             // happens client side. Clients are aware of the server's time and use it to sync.
-            _logger.LogInformation("HandleRequest: {0} requested {1}, group {2} in {3} state.", session.Id, request.Type, GroupId.ToString(), _state.Type);
+            _logger.LogInformation("HandleRequest: {SessionId} requested {RequestType}, group {GroupId} is {StateType}.", session.Id, request.Type, GroupId.ToString(), _state.Type);
             request.Apply(this, _state, session, cancellationToken);
         }
 
         /// <inheritdoc />
         public GroupInfoDto GetInfo()
         {
-            return new GroupInfoDto()
-            {
-                GroupId = GroupId.ToString(),
-                GroupName = GroupName,
-                State = _state.Type,
-                Participants = Participants.Values.Select(session => session.Session.UserName).Distinct().ToList(),
-                LastUpdatedAt = DateTime.UtcNow
-            };
+            var participants = Participants.Values.Select(session => session.Session.UserName).Distinct().ToList();
+            return new GroupInfoDto(GroupId, GroupName, _state.Type, participants, DateTime.UtcNow);
         }
 
         /// <inheritdoc />
         public bool HasAccessToPlayQueue(User user)
         {
-            var items = PlayQueue.GetPlaylist().Select(item => item.ItemId);
+            var items = PlayQueue.GetPlaylist().Select(item => item.ItemId).ToList();
             return HasAccessToQueue(user, items);
         }
 
@@ -383,7 +366,7 @@ namespace Emby.Server.Implementations.SyncPlay
         /// <inheritdoc />
         public void SetState(IGroupState state)
         {
-            _logger.LogInformation("SetState: {0} switching from {1} to {2}.", GroupId.ToString(), _state.Type, state.Type);
+            _logger.LogInformation("SetState: {GroupId} switching from {FromStateType} to {ToStateType}.", GroupId.ToString(), _state.Type, state.Type);
             this._state = state;
         }
 
@@ -418,26 +401,19 @@ namespace Emby.Server.Implementations.SyncPlay
         /// <inheritdoc />
         public SendCommand NewSyncPlayCommand(SendCommandType type)
         {
-            return new SendCommand()
-            {
-                GroupId = GroupId.ToString(),
-                PlaylistItemId = PlayQueue.GetPlayingItemPlaylistId(),
-                PositionTicks = PositionTicks,
-                Command = type,
-                When = LastActivity,
-                EmittedAt = DateTime.UtcNow
-            };
+            return new SendCommand(
+                GroupId,
+                PlayQueue.GetPlayingItemPlaylistId(),
+                LastActivity,
+                type,
+                PositionTicks,
+                DateTime.UtcNow);
         }
 
         /// <inheritdoc />
         public GroupUpdate<T> NewSyncPlayGroupUpdate<T>(GroupUpdateType type, T data)
         {
-            return new GroupUpdate<T>()
-            {
-                GroupId = GroupId.ToString(),
-                Type = type,
-                Data = data
-            };
+            return new GroupUpdate<T>(GroupId, type, data);
         }
 
         /// <inheritdoc />
@@ -501,10 +477,10 @@ namespace Emby.Server.Implementations.SyncPlay
         }
 
         /// <inheritdoc />
-        public bool SetPlayQueue(IEnumerable<Guid> playQueue, int playingItemPosition, long startPositionTicks)
+        public bool SetPlayQueue(IReadOnlyList<Guid> playQueue, int playingItemPosition, long startPositionTicks)
         {
             // Ignore on empty queue or invalid item position.
-            if (!playQueue.Any() || playingItemPosition >= playQueue.Count() || playingItemPosition < 0)
+            if (playQueue.Count == 0 || playingItemPosition >= playQueue.Count || playingItemPosition < 0)
             {
                 return false;
             }
@@ -547,7 +523,7 @@ namespace Emby.Server.Implementations.SyncPlay
         }
 
         /// <inheritdoc />
-        public bool RemoveFromPlayQueue(IEnumerable<string> playlistItemIds)
+        public bool RemoveFromPlayQueue(IReadOnlyList<string> playlistItemIds)
         {
             var playingItemRemoved = PlayQueue.RemoveFromPlaylist(playlistItemIds);
             if (playingItemRemoved)
@@ -576,10 +552,10 @@ namespace Emby.Server.Implementations.SyncPlay
         }
 
         /// <inheritdoc />
-        public bool AddToPlayQueue(IEnumerable<Guid> newItems, GroupQueueMode mode)
+        public bool AddToPlayQueue(IReadOnlyList<Guid> newItems, GroupQueueMode mode)
         {
             // Ignore on empty list.
-            if (!newItems.Any())
+            if (newItems.Count == 0)
             {
                 return false;
             }
@@ -673,16 +649,14 @@ namespace Emby.Server.Implementations.SyncPlay
                 startPositionTicks += Math.Max(elapsedTime.Ticks, 0);
             }
 
-            return new PlayQueueUpdate()
-            {
-                Reason = reason,
-                LastUpdate = PlayQueue.LastChange,
-                Playlist = PlayQueue.GetPlaylist(),
-                PlayingItemIndex = PlayQueue.PlayingItemIndex,
-                StartPositionTicks = startPositionTicks,
-                ShuffleMode = PlayQueue.ShuffleMode,
-                RepeatMode = PlayQueue.RepeatMode
-            };
+            return new PlayQueueUpdate(
+                reason,
+                PlayQueue.LastChange,
+                PlayQueue.GetPlaylist(),
+                PlayQueue.PlayingItemIndex,
+                startPositionTicks,
+                PlayQueue.ShuffleMode,
+                PlayQueue.RepeatMode);
         }
     }
 }

Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs

@@ -72,19 +72,9 @@ namespace Emby.Server.Implementations.SyncPlay
             _userManager = userManager;
             _sessionManager = sessionManager;
             _libraryManager = libraryManager;
-
             _sessionManager.SessionStarted += OnSessionManagerSessionStarted;
-            _sessionManager.SessionEnded += OnSessionManagerSessionEnded;
-            _sessionManager.PlaybackStart += OnSessionManagerPlaybackStart;
-            _sessionManager.PlaybackStopped += OnSessionManagerPlaybackStopped;
         }
 
-        /// <summary>
-        /// Gets all groups.
-        /// </summary>
-        /// <value>All groups.</value>
-        public IEnumerable<IGroupController> Groups => _groups.Values;
-
         /// <inheritdoc />
         public void Dispose()
         {
@@ -92,127 +82,6 @@ namespace Emby.Server.Implementations.SyncPlay
             GC.SuppressFinalize(this);
         }
 
-        /// <summary>
-        /// Releases unmanaged and optionally managed resources.
-        /// </summary>
-        /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
-        protected virtual void Dispose(bool disposing)
-        {
-            if (_disposed)
-            {
-                return;
-            }
-
-            _sessionManager.SessionStarted -= OnSessionManagerSessionStarted;
-            _sessionManager.SessionEnded -= OnSessionManagerSessionEnded;
-            _sessionManager.PlaybackStart -= OnSessionManagerPlaybackStart;
-            _sessionManager.PlaybackStopped -= OnSessionManagerPlaybackStopped;
-
-            _disposed = true;
-        }
-
-        private void OnSessionManagerSessionStarted(object sender, SessionEventArgs e)
-        {
-            var session = e.SessionInfo;
-            if (!IsSessionInGroup(session))
-            {
-                return;
-            }
-
-            var groupId = GetSessionGroup(session) ?? Guid.Empty;
-            var request = new JoinGroupRequest()
-            {
-                GroupId = groupId
-            };
-            JoinGroup(session, groupId, request, CancellationToken.None);
-        }
-
-        private void OnSessionManagerSessionEnded(object sender, SessionEventArgs e)
-        {
-            var session = e.SessionInfo;
-            if (!IsSessionInGroup(session))
-            {
-                return;
-            }
-
-            // TODO: probably remove this event, not used at the moment.
-        }
-
-        private void OnSessionManagerPlaybackStart(object sender, PlaybackProgressEventArgs e)
-        {
-            var session = e.Session;
-            if (!IsSessionInGroup(session))
-            {
-                return;
-            }
-
-            // TODO: probably remove this event, not used at the moment.
-        }
-
-        private void OnSessionManagerPlaybackStopped(object sender, PlaybackStopEventArgs e)
-        {
-            var session = e.Session;
-            if (!IsSessionInGroup(session))
-            {
-                return;
-            }
-
-            // TODO: probably remove this event, not used at the moment.
-        }
-
-        private bool IsRequestValid<T>(SessionInfo session, GroupRequestType requestType, T request, bool checkRequest = true)
-        {
-            if (session == null || (request == null && checkRequest))
-            {
-                return false;
-            }
-
-            var user = _userManager.GetUserById(session.UserId);
-
-            if (user.SyncPlayAccess == SyncPlayAccess.None)
-            {
-                _logger.LogWarning("IsRequestValid: {0} does not have access to SyncPlay. Requested {1}.", session.Id, requestType);
-
-                var error = new GroupUpdate<string>()
-                {
-                    // TODO: rename to a more generic error. Next PR will fix this.
-                    Type = GroupUpdateType.JoinGroupDenied
-                };
-                _sessionManager.SendSyncPlayGroupUpdate(session, error, CancellationToken.None);
-                return false;
-            }
-
-            if (requestType.Equals(GroupRequestType.NewGroup) && user.SyncPlayAccess != SyncPlayAccess.CreateAndJoinGroups)
-            {
-                _logger.LogWarning("IsRequestValid: {0} does not have permission to create groups.", session.Id);
-
-                var error = new GroupUpdate<string>
-                {
-                    Type = GroupUpdateType.CreateGroupDenied
-                };
-                _sessionManager.SendSyncPlayGroupUpdate(session, error, CancellationToken.None);
-                return false;
-            }
-
-            return true;
-        }
-
-        private bool IsRequestValid(SessionInfo session, GroupRequestType requestType)
-        {
-            return IsRequestValid(session, requestType, session, false);
-        }
-
-        private bool IsSessionInGroup(SessionInfo session)
-        {
-            return _sessionToGroupMap.ContainsKey(session.Id);
-        }
-
-        private Guid? GetSessionGroup(SessionInfo session)
-        {
-            _sessionToGroupMap.TryGetValue(session.Id, out var group);
-            return group?.GroupId;
-        }
-
         /// <inheritdoc />
         public void NewGroup(SessionInfo session, NewGroupRequest request, CancellationToken cancellationToken)
         {
@@ -229,9 +98,10 @@ namespace Emby.Server.Implementations.SyncPlay
                     LeaveGroup(session, cancellationToken);
                 }
 
-                var group = new GroupController(_logger, _userManager, _sessionManager, _libraryManager, this);
+                var group = new GroupController(_logger, _userManager, _sessionManager, _libraryManager);
                 _groups[group.GroupId] = group;
 
+                AddSessionToGroup(session, group);
                 group.CreateGroup(session, request, cancellationToken);
             }
         }
@@ -253,25 +123,18 @@ namespace Emby.Server.Implementations.SyncPlay
 
                 if (group == null)
                 {
-                    _logger.LogWarning("JoinGroup: {0} tried to join group {0} that does not exist.", session.Id, groupId);
+                    _logger.LogWarning("JoinGroup: {SessionId} tried to join group {GroupId} that does not exist.", session.Id, groupId);
 
-                    var error = new GroupUpdate<string>()
-                    {
-                        Type = GroupUpdateType.GroupDoesNotExist
-                    };
+                    var error = new GroupUpdate<string>(Guid.Empty, GroupUpdateType.GroupDoesNotExist, string.Empty);
                     _sessionManager.SendSyncPlayGroupUpdate(session, error, CancellationToken.None);
                     return;
                 }
 
                 if (!group.HasAccessToPlayQueue(user))
                 {
-                    _logger.LogWarning("JoinGroup: {0} does not have access to some content from the playing queue of group {1}.", session.Id, group.GroupId.ToString());
+                    _logger.LogWarning("JoinGroup: {SessionId} does not have access to some content from the playing queue of group {GroupId}.", session.Id, group.GroupId.ToString());
 
-                    var error = new GroupUpdate<string>()
-                    {
-                        GroupId = group.GroupId.ToString(),
-                        Type = GroupUpdateType.LibraryAccessDenied
-                    };
+                    var error = new GroupUpdate<string>(group.GroupId, GroupUpdateType.LibraryAccessDenied, string.Empty);
                     _sessionManager.SendSyncPlayGroupUpdate(session, error, CancellationToken.None);
                     return;
                 }
@@ -287,6 +150,7 @@ namespace Emby.Server.Implementations.SyncPlay
                     LeaveGroup(session, cancellationToken);
                 }
 
+                AddSessionToGroup(session, group);
                 group.SessionJoin(session, request, cancellationToken);
             }
         }
@@ -307,21 +171,19 @@ namespace Emby.Server.Implementations.SyncPlay
 
                 if (group == null)
                 {
-                    _logger.LogWarning("LeaveGroup: {0} does not belong to any group.", session.Id);
+                    _logger.LogWarning("LeaveGroup: {SessionId} does not belong to any group.", session.Id);
 
-                    var error = new GroupUpdate<string>()
-                    {
-                        Type = GroupUpdateType.NotInGroup
-                    };
+                    var error = new GroupUpdate<string>(Guid.Empty, GroupUpdateType.NotInGroup, string.Empty);
                     _sessionManager.SendSyncPlayGroupUpdate(session, error, CancellationToken.None);
                     return;
                 }
 
+                RemoveSessionFromGroup(session, group);
                 group.SessionLeave(session, cancellationToken);
 
                 if (group.IsGroupEmpty())
                 {
-                    _logger.LogInformation("LeaveGroup: removing empty group {0}.", group.GroupId);
+                    _logger.LogInformation("LeaveGroup: removing empty group {GroupId}.", group.GroupId);
                     _groups.Remove(group.GroupId, out _);
                 }
             }
@@ -338,11 +200,14 @@ namespace Emby.Server.Implementations.SyncPlay
 
             var user = _userManager.GetUserById(session.UserId);
 
-            return _groups
-                .Values
-                .Where(group => group.HasAccessToPlayQueue(user))
-                .Select(group => group.GetInfo())
-                .ToList();
+            lock (_groupsLock)
+            {
+                return _groups
+                    .Values
+                    .Where(group => group.HasAccessToPlayQueue(user))
+                    .Select(group => group.GetInfo())
+                    .ToList();
+            }
         }
 
         /// <inheritdoc />
@@ -360,12 +225,9 @@ namespace Emby.Server.Implementations.SyncPlay
 
                 if (group == null)
                 {
-                    _logger.LogWarning("HandleRequest: {0} does not belong to any group.", session.Id);
+                    _logger.LogWarning("HandleRequest: {SessionId} does not belong to any group.", session.Id);
 
-                    var error = new GroupUpdate<string>()
-                    {
-                        Type = GroupUpdateType.NotInGroup
-                    };
+                    var error = new GroupUpdate<string>(Guid.Empty, GroupUpdateType.NotInGroup, string.Empty);
                     _sessionManager.SendSyncPlayGroupUpdate(session, error, CancellationToken.None);
                     return;
                 }
@@ -374,8 +236,74 @@ namespace Emby.Server.Implementations.SyncPlay
             }
         }
 
-        /// <inheritdoc />
-        public void AddSessionToGroup(SessionInfo session, IGroupController group)
+        /// <summary>
+        /// Releases unmanaged and optionally managed resources.
+        /// </summary>
+        /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
+        protected virtual void Dispose(bool disposing)
+        {
+            if (_disposed)
+            {
+                return;
+            }
+
+            _sessionManager.SessionStarted -= OnSessionManagerSessionStarted;
+            _disposed = true;
+        }
+
+        private void OnSessionManagerSessionStarted(object sender, SessionEventArgs e)
+        {
+            var session = e.SessionInfo;
+            lock (_groupsLock)
+            {
+                if (!IsSessionInGroup(session))
+                {
+                    return;
+                }
+
+                var groupId = GetSessionGroup(session);
+                var request = new JoinGroupRequest(groupId);
+                JoinGroup(session, groupId, request, CancellationToken.None);
+            }
+        }
+
+        /// <summary>
+        /// Checks if a given session has joined a group.
+        /// </summary>
+        /// <remarks>
+        /// Not thread-safe, call only under groups-lock.
+        /// </remarks>
+        /// <param name="session">The session.</param>
+        /// <returns><c>true</c> if the session has joined a group, <c>false</c> otherwise.</returns>
+        private bool IsSessionInGroup(SessionInfo session)
+        {
+            return _sessionToGroupMap.ContainsKey(session.Id);
+        }
+
+        /// <summary>
+        /// Gets the group joined by the given session, if any.
+        /// </summary>
+        /// <remarks>
+        /// Not thread-safe, call only under groups-lock.
+        /// </remarks>
+        /// <param name="session">The session.</param>
+        /// <returns>The group identifier if the session has joined a group, an empty identifier otherwise.</returns>
+        private Guid GetSessionGroup(SessionInfo session)
+        {
+            _sessionToGroupMap.TryGetValue(session.Id, out var group);
+            return group?.GroupId ?? Guid.Empty;
+        }
+
+        /// <summary>
+        /// Maps a session to a group.
+        /// </summary>
+        /// <remarks>
+        /// Not thread-safe, call only under groups-lock.
+        /// </remarks>
+        /// <param name="session">The session.</param>
+        /// <param name="group">The group.</param>
+        /// <exception cref="InvalidOperationException">Thrown when the user is in another group already.</exception>
+        private void AddSessionToGroup(SessionInfo session, IGroupController group)
         {
             if (session == null)
             {
@@ -390,8 +318,16 @@ namespace Emby.Server.Implementations.SyncPlay
             _sessionToGroupMap[session.Id] = group ?? throw new InvalidOperationException("Group is null!");
         }
 
-        /// <inheritdoc />
-        public void RemoveSessionFromGroup(SessionInfo session, IGroupController group)
+        /// <summary>
+        /// Unmaps a session from a group.
+        /// </summary>
+        /// <remarks>
+        /// Not thread-safe, call only under groups-lock.
+        /// </remarks>
+        /// <param name="session">The session.</param>
+        /// <param name="group">The group.</param>
+        /// <exception cref="InvalidOperationException">Thrown when the user is not found in the specified group.</exception>
+        private void RemoveSessionFromGroup(SessionInfo session, IGroupController group)
         {
             if (session == null)
             {
@@ -414,5 +350,55 @@ namespace Emby.Server.Implementations.SyncPlay
                 throw new InvalidOperationException("Session was in wrong group!");
             }
         }
+
+        /// <summary>
+        /// Checks if a given session is allowed to make a given request.
+        /// </summary>
+        /// <param name="session">The session.</param>
+        /// <param name="requestType">The request type.</param>
+        /// <param name="request">The request.</param>
+        /// <param name="checkRequest">Whether to check if request is null.</param>
+        /// <returns><c>true</c> if the request is valid, <c>false</c> otherwise. Will return <c>false</c> also when session is null.</returns>
+        private bool IsRequestValid<T>(SessionInfo session, GroupRequestType requestType, T request, bool checkRequest = true)
+        {
+            if (session == null || (request == null && checkRequest))
+            {
+                return false;
+            }
+
+            var user = _userManager.GetUserById(session.UserId);
+
+            if (user.SyncPlayAccess == SyncPlayAccess.None)
+            {
+                _logger.LogWarning("IsRequestValid: {SessionId} does not have access to SyncPlay. Requested {RequestType}.", session.Id, requestType);
+
+                // TODO: rename to a more generic error. Next PR will fix this.
+                var error = new GroupUpdate<string>(Guid.Empty, GroupUpdateType.JoinGroupDenied, string.Empty);
+                _sessionManager.SendSyncPlayGroupUpdate(session, error, CancellationToken.None);
+                return false;
+            }
+
+            if (requestType.Equals(GroupRequestType.NewGroup) && user.SyncPlayAccess != SyncPlayAccess.CreateAndJoinGroups)
+            {
+                _logger.LogWarning("IsRequestValid: {SessionId} does not have permission to create groups.", session.Id);
+
+                var error = new GroupUpdate<string>(Guid.Empty, GroupUpdateType.CreateGroupDenied, string.Empty);
+                _sessionManager.SendSyncPlayGroupUpdate(session, error, CancellationToken.None);
+                return false;
+            }
+
+            return true;
+        }
+
+        /// <summary>
+        /// Checks if a given session is allowed to make a given type of request.
+        /// </summary>
+        /// <param name="session">The session.</param>
+        /// <param name="requestType">The request type.</param>
+        /// <returns><c>true</c> if the request is valid, <c>false</c> otherwise. Will return <c>false</c> also when session is null.</returns>
+        private bool IsRequestValid(SessionInfo session, GroupRequestType requestType)
+        {
+            return IsRequestValid(session, requestType, session, false);
+        }
     }
 }

Jellyfin.Api/Controllers/SyncPlayController.cs

@@ -53,10 +53,7 @@ namespace Jellyfin.Api.Controllers
             [FromQuery, Required] string groupName)
         {
             var currentSession = RequestHelpers.GetSession(_sessionManager, _authorizationContext, Request);
-            var newGroupRequest = new NewGroupRequest()
-            {
-                GroupName = groupName
-            };
+            var newGroupRequest = new NewGroupRequest(groupName);
             _syncPlayManager.NewGroup(currentSession, newGroupRequest, CancellationToken.None);
             return NoContent();
         }
@@ -73,10 +70,7 @@ namespace Jellyfin.Api.Controllers
             [FromQuery, Required] Guid groupId)
         {
             var currentSession = RequestHelpers.GetSession(_sessionManager, _authorizationContext, Request);
-            var joinRequest = new JoinGroupRequest()
-            {
-                GroupId = groupId
-            };
+            var joinRequest = new JoinGroupRequest(groupId);
             _syncPlayManager.JoinGroup(currentSession, groupId, joinRequest, CancellationToken.None);
             return NoContent();
         }
@@ -185,18 +179,18 @@ namespace Jellyfin.Api.Controllers
         /// <summary>
         /// Request to queue items to the playlist of a SyncPlay group.
         /// </summary>
-        /// <param name="items">The items to add.</param>
+        /// <param name="itemIds">The items to add.</param>
         /// <param name="mode">The mode in which to enqueue the items.</param>
         /// <response code="204">Queue update request sent to all group members.</response>
         /// <returns>A <see cref="NoContentResult"/> indicating success.</returns>
         [HttpPost("Queue")]
         [ProducesResponseType(StatusCodes.Status204NoContent)]
         public ActionResult SyncPlayQueue(
-            [FromQuery, Required] Guid[] items,
+            [FromQuery, Required] Guid[] itemIds,
             [FromQuery, Required] GroupQueueMode mode)
         {
             var currentSession = RequestHelpers.GetSession(_sessionManager, _authorizationContext, Request);
-            var syncPlayRequest = new QueueGroupRequest(items, mode);
+            var syncPlayRequest = new QueueGroupRequest(itemIds, mode);
             _syncPlayManager.HandleRequest(currentSession, syncPlayRequest, CancellationToken.None);
             return NoContent();
         }

Jellyfin.Api/Controllers/TimeSyncController.cs

@@ -13,7 +13,7 @@ namespace Jellyfin.Api.Controllers
     public class TimeSyncController : BaseJellyfinApiController
     {
         /// <summary>
-        /// Gets the current utc time.
+        /// Gets the current UTC time.
         /// </summary>
         /// <response code="200">Time returned.</response>
         /// <returns>An <see cref="UtcTimeResponse"/> to sync the client and server time.</returns>
@@ -22,18 +22,14 @@ namespace Jellyfin.Api.Controllers
         public ActionResult<UtcTimeResponse> GetUtcTime()
         {
             // Important to keep the following line at the beginning
-            var requestReceptionTime = DateTime.UtcNow.ToUniversalTime().ToString("o", DateTimeFormatInfo.InvariantInfo);
+            var requestReceptionTime = DateTime.UtcNow.ToUniversalTime();
 
-            var response = new UtcTimeResponse();
-            response.RequestReceptionTime = requestReceptionTime;
-
-            // Important to keep the following two lines at the end
-            var responseTransmissionTime = DateTime.UtcNow.ToUniversalTime().ToString("o", DateTimeFormatInfo.InvariantInfo);
-            response.ResponseTransmissionTime = responseTransmissionTime;
+            // Important to keep the following line at the end
+            var responseTransmissionTime = DateTime.UtcNow.ToUniversalTime();
 
             // Implementing NTP on such a high level results in this useless
             // information being sent. On the other hand it enables future additions.
-            return response;
+            return new UtcTimeResponse(requestReceptionTime, responseTransmissionTime);
         }
     }
 }

MediaBrowser.Controller/SyncPlay/GroupStates/AbstractGroupState.cs

@@ -68,7 +68,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
 
             if (playingItemRemoved && !context.PlayQueue.IsItemPlaying())
             {
-                Logger.LogDebug("HandleRequest: {0} in group {1}, play queue is empty.", request.Type, context.GroupId.ToString());
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, play queue is empty.", request.Type, context.GroupId.ToString());
 
                 IGroupState idleState = new IdleGroupState(Logger);
                 context.SetState(idleState);
@@ -84,7 +84,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
 
             if (!result)
             {
-                Logger.LogError("HandleRequest: {0} in group {1}, unable to move item in play queue.", request.Type, context.GroupId.ToString());
+                Logger.LogError("HandleRequest: {RequestType} in group {GroupId}, unable to move item in play queue.", request.Type, context.GroupId.ToString());
                 return;
             }
 
@@ -100,7 +100,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
 
             if (!result)
             {
-                Logger.LogError("HandleRequest: {0} in group {1}, unable to add items to play queue.", request.Type, context.GroupId.ToString());
+                Logger.LogError("HandleRequest: {RequestType} in group {GroupId}, unable to add items to play queue.", request.Type, context.GroupId.ToString());
                 return;
             }
 
@@ -203,18 +203,14 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
         protected void SendGroupStateUpdate(IGroupStateContext context, IGroupPlaybackRequest reason, SessionInfo session, CancellationToken cancellationToken)
         {
             // Notify relevant state change event.
-            var stateUpdate = new GroupStateUpdate()
-            {
-                State = Type,
-                Reason = reason.Type
-            };
+            var stateUpdate = new GroupStateUpdate(Type, reason.Type);
             var update = context.NewSyncPlayGroupUpdate(GroupUpdateType.StateUpdate, stateUpdate);
             context.SendGroupUpdate(session, SyncPlayBroadcastType.AllGroup, update, cancellationToken);
         }
 
         private void UnhandledRequest(IGroupPlaybackRequest request)
         {
-            Logger.LogWarning("HandleRequest: unhandled {0} request for {1} state.", request.Type, Type);
+            Logger.LogWarning("HandleRequest: unhandled {RequestType} request in {StateType} state.", request.Type, Type);
         }
     }
 }

MediaBrowser.Controller/SyncPlay/GroupStates/WaitingGroupState.cs

@@ -103,7 +103,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
                     var unpauseRequest = new UnpauseGroupRequest();
                     playingState.HandleRequest(context, Type, unpauseRequest, session, cancellationToken);
 
-                    Logger.LogDebug("SessionLeaving: {0} left the group {1}, notifying others to resume.", session.Id, context.GroupId.ToString());
+                    Logger.LogDebug("SessionLeaving: {SessionId} left group {GroupId}, notifying others to resume.", session.Id, context.GroupId.ToString());
                 }
                 else
                 {
@@ -111,7 +111,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
                     var pausedState = new PausedGroupState(Logger);
                     context.SetState(pausedState);
 
-                    Logger.LogDebug("SessionLeaving: {0} left the group {1}, returning to previous state.", session.Id, context.GroupId.ToString());
+                    Logger.LogDebug("SessionLeaving: {SessionId} left group {GroupId}, returning to previous state.", session.Id, context.GroupId.ToString());
                 }
             }
         }
@@ -131,7 +131,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             var setQueueStatus = context.SetPlayQueue(request.PlayingQueue, request.PlayingItemPosition, request.StartPositionTicks);
             if (!setQueueStatus)
             {
-                Logger.LogError("HandleRequest: {0} in group {1}, unable to set playing queue.", request.Type, context.GroupId.ToString());
+                Logger.LogError("HandleRequest: {RequestType} in group {GroupId}, unable to set playing queue.", request.Type, context.GroupId.ToString());
 
                 // Ignore request and return to previous state.
                 IGroupState newState = prevState switch {
@@ -151,7 +151,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             // Reset status of sessions and await for all Ready events.
             context.SetAllBuffering(true);
 
-            Logger.LogDebug("HandleRequest: {0} in group {1}, {2} set a new play queue.", request.Type, context.GroupId.ToString(), session.Id);
+            Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, {SessionId} set a new play queue.", request.Type, context.GroupId.ToString(), session.Id);
         }
 
         /// <inheritdoc />
@@ -188,7 +188,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
 
                 context.SetState(newState);
 
-                Logger.LogDebug("HandleRequest: {0} in group {1}, unable to change current playing item.", request.Type, context.GroupId.ToString());
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, unable to change current playing item.", request.Type, context.GroupId.ToString());
             }
         }
 
@@ -214,13 +214,13 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
                 // Reset status of sessions and await for all Ready events.
                 context.SetAllBuffering(true);
 
-                Logger.LogDebug("HandleRequest: {0} in group {1}, waiting for all ready events.", request.Type, context.GroupId.ToString());
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, waiting for all ready events.", request.Type, context.GroupId.ToString());
             }
             else
             {
                 if (ResumePlaying)
                 {
-                    Logger.LogDebug("HandleRequest: {0} in group {1}, ignoring sessions that are not ready and forcing the playback to start.", request.Type, context.GroupId.ToString());
+                    Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, ignoring sessions that are not ready and forcing the playback to start.", request.Type, context.GroupId.ToString());
 
                     // An Unpause request is forcing the playback to start, ignoring sessions that are not ready.
                     context.SetAllBuffering(false);
@@ -326,7 +326,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             // Make sure the client is playing the correct item.
             if (!request.PlaylistItemId.Equals(context.PlayQueue.GetPlayingItemPlaylistId(), StringComparison.OrdinalIgnoreCase))
             {
-                Logger.LogDebug("HandleRequest: {0} in group {1}, {2} has wrong playlist item.", request.Type, context.GroupId.ToString(), session.Id);
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, {SessionId} has wrong playlist item.", request.Type, context.GroupId.ToString(), session.Id);
 
                 var playQueueUpdate = context.GetPlayQueueUpdate(PlayQueueUpdateReason.SetCurrentItem);
                 var updateSession = context.NewSyncPlayGroupUpdate(GroupUpdateType.PlayQueue, playQueueUpdate);
@@ -400,7 +400,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             // Make sure the client is playing the correct item.
             if (!request.PlaylistItemId.Equals(context.PlayQueue.GetPlayingItemPlaylistId(), StringComparison.OrdinalIgnoreCase))
             {
-                Logger.LogDebug("HandleRequest: {0} in group {1}, {2} has wrong playlist item.", request.Type, context.GroupId.ToString(), session.Id);
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, {SessionId} has wrong playlist item.", request.Type, context.GroupId.ToString(), session.Id);
 
                 var playQueueUpdate = context.GetPlayQueueUpdate(PlayQueueUpdateReason.SetCurrentItem);
                 var update = context.NewSyncPlayGroupUpdate(GroupUpdateType.PlayQueue, playQueueUpdate);
@@ -420,7 +420,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             var timeSyncThresholdTicks = TimeSpan.FromMilliseconds(context.TimeSyncOffset).Ticks;
             if (Math.Abs(elapsedTime.Ticks) > timeSyncThresholdTicks)
             {
-                Logger.LogWarning("HandleRequest: {0} in group {1}, {2} is not time syncing properly. Ignoring elapsed time.", request.Type, context.GroupId.ToString(), session.Id);
+                Logger.LogWarning("HandleRequest: {RequestType} in group {GroupId}, {SessionId} is not time syncing properly. Ignoring elapsed time.", request.Type, context.GroupId.ToString(), session.Id);
 
                 elapsedTime = TimeSpan.Zero;
             }
@@ -436,7 +436,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             var delayTicks = context.PositionTicks - clientPosition.Ticks;
             var maxPlaybackOffsetTicks = TimeSpan.FromMilliseconds(context.MaxPlaybackOffset).Ticks;
 
-            Logger.LogDebug("HandleRequest: {0} in group {1}, {2} at {3} (delay of {4} seconds).", request.Type, context.GroupId.ToString(), session.Id, clientPosition, TimeSpan.FromTicks(delayTicks).TotalSeconds);
+            Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, {SessionId} at {PositionTicks} (delay of {Delay} seconds).", request.Type, context.GroupId.ToString(), session.Id, clientPosition, TimeSpan.FromTicks(delayTicks).TotalSeconds);
 
             if (ResumePlaying)
             {
@@ -454,7 +454,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
                     // Notify relevant state change event.
                     SendGroupStateUpdate(context, request, session, cancellationToken);
 
-                    Logger.LogWarning("HandleRequest: {0} in group {1}, {2} got lost in time, correcting.", request.Type, context.GroupId.ToString(), session.Id);
+                    Logger.LogWarning("HandleRequest: {RequestType} in group {GroupId}, {SessionId} got lost in time, correcting.", request.Type, context.GroupId.ToString(), session.Id);
                     return;
                 }
 
@@ -468,7 +468,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
                     command.When = currentTime.AddTicks(delayTicks);
                     context.SendCommand(session, SyncPlayBroadcastType.CurrentSession, command, cancellationToken);
 
-                    Logger.LogInformation("HandleRequest: {0} in group {1}, others still buffering, {2} will pause when ready in {3} seconds.", request.Type, context.GroupId.ToString(), session.Id, TimeSpan.FromTicks(delayTicks).TotalSeconds);
+                    Logger.LogInformation("HandleRequest: {RequestType} in group {GroupId}, others still buffering, {SessionId} will pause when ready in {Delay} seconds.", request.Type, context.GroupId.ToString(), session.Id, TimeSpan.FromTicks(delayTicks).TotalSeconds);
                 }
                 else
                 {
@@ -487,7 +487,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
 
                         context.SendCommand(session, filter, command, cancellationToken);
 
-                        Logger.LogInformation("HandleRequest: {0} in group {1}, {2} is recovering, notifying others to resume in {3} seconds.", request.Type, context.GroupId.ToString(), session.Id, TimeSpan.FromTicks(delayTicks).TotalSeconds);
+                        Logger.LogInformation("HandleRequest: {RequestType} in group {GroupId}, {SessionId} is recovering, notifying others to resume in {Delay} seconds.", request.Type, context.GroupId.ToString(), session.Id, TimeSpan.FromTicks(delayTicks).TotalSeconds);
                     }
                     else
                     {
@@ -500,7 +500,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
                         var command = context.NewSyncPlayCommand(SendCommandType.Unpause);
                         context.SendCommand(session, SyncPlayBroadcastType.AllGroup, command, cancellationToken);
 
-                        Logger.LogWarning("HandleRequest: {0} in group {1}, {2} resumed playback but did not update others in time. {3} seconds to recover.", request.Type, context.GroupId.ToString(), session.Id, TimeSpan.FromTicks(delayTicks).TotalSeconds);
+                        Logger.LogWarning("HandleRequest: {RequestType} in group {GroupId}, {SessionId} resumed playback but did not update others in time. {Delay} seconds to recover.", request.Type, context.GroupId.ToString(), session.Id, TimeSpan.FromTicks(delayTicks).TotalSeconds);
                     }
 
                     // Change state.
@@ -511,7 +511,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             }
             else
             {
-                // Check that session is really ready, tollerate player imperfections under a certain threshold.
+                // Check that session is really ready, tolerate player imperfections under a certain threshold.
                 if (Math.Abs(context.PositionTicks - requestTicks) > maxPlaybackOffsetTicks)
                 {
                     // Session still not ready.
@@ -523,7 +523,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
                     // Notify relevant state change event.
                     SendGroupStateUpdate(context, request, session, cancellationToken);
 
-                    Logger.LogWarning("HandleRequest: {0} in group {1}, {2} was seeking to wrong position, correcting.", request.Type, context.GroupId.ToString(), session.Id);
+                    Logger.LogWarning("HandleRequest: {RequestType} in group {GroupId}, {SessionId} is seeking to wrong position, correcting.", request.Type, context.GroupId.ToString(), session.Id);
                     return;
                 }
                 else
@@ -549,7 +549,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
                         pausedState.HandleRequest(context, Type, request, session, cancellationToken);
                     }
 
-                    Logger.LogDebug("HandleRequest: {0} in group {1}, {2} is ready, returning to previous state.", request.Type, context.GroupId.ToString(), session.Id);
+                    Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, {SessionId} is ready, returning to previous state.", request.Type, context.GroupId.ToString(), session.Id);
                 }
             }
         }
@@ -569,7 +569,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             // Make sure the client knows the playing item, to avoid duplicate requests.
             if (!request.PlaylistItemId.Equals(context.PlayQueue.GetPlayingItemPlaylistId(), StringComparison.OrdinalIgnoreCase))
             {
-                Logger.LogDebug("HandleRequest: {0} in group {1}, client provided the wrong playlist identifier.", request.Type, context.GroupId.ToString());
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, {SessionId} provided the wrong playlist identifier.", request.Type, context.GroupId.ToString(), session.Id);
                 return;
             }
 
@@ -596,7 +596,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
 
                 context.SetState(newState);
 
-                Logger.LogDebug("HandleRequest: {0} in group {1}, no next track available.", request.Type, context.GroupId.ToString());
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, no next track available.", request.Type, context.GroupId.ToString());
             }
         }
 
@@ -615,7 +615,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
             // Make sure the client knows the playing item, to avoid duplicate requests.
             if (!request.PlaylistItemId.Equals(context.PlayQueue.GetPlayingItemPlaylistId(), StringComparison.OrdinalIgnoreCase))
             {
-                Logger.LogDebug("HandleRequest: {0} in group {1}, client provided the wrong playlist identifier.", request.Type, context.GroupId.ToString());
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, {SessionId} provided the wrong playlist identifier.", request.Type, context.GroupId.ToString(), session.Id);
                 return;
             }
 
@@ -642,7 +642,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
 
                 context.SetState(newState);
 
-                Logger.LogDebug("HandleRequest: {0} in group {1}, no previous track available.", request.Type, context.GroupId.ToString());
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, no previous track available.", request.Type, context.GroupId.ToString());
             }
         }
 
@@ -653,7 +653,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates
 
             if (!context.IsBuffering())
             {
-                Logger.LogDebug("HandleRequest: {0} in group {1}, returning to previous state.", request.Type, context.GroupId.ToString());
+                Logger.LogDebug("HandleRequest: {RequestType} in group {GroupId}, returning to previous state.", request.Type, context.GroupId.ToString());
 
                 if (ResumePlaying)
                 {

MediaBrowser.Controller/SyncPlay/IGroupStateContext.cs

@@ -151,7 +151,7 @@ namespace MediaBrowser.Controller.SyncPlay
         /// <param name="playingItemPosition">The playing item position in the play queue.</param>
         /// <param name="startPositionTicks">The start position ticks.</param>
         /// <returns><c>true</c> if the play queue has been changed; <c>false</c> if something went wrong.</returns>
-        bool SetPlayQueue(IEnumerable<Guid> playQueue, int playingItemPosition, long startPositionTicks);
+        bool SetPlayQueue(IReadOnlyList<Guid> playQueue, int playingItemPosition, long startPositionTicks);
 
         /// <summary>
         /// Sets the playing item.
@@ -165,7 +165,7 @@ namespace MediaBrowser.Controller.SyncPlay
         /// </summary>
         /// <param name="playlistItemIds">The items to remove.</param>
         /// <returns><c>true</c> if playing item got removed; <c>false</c> otherwise.</returns>
-        bool RemoveFromPlayQueue(IEnumerable<string> playlistItemIds);
+        bool RemoveFromPlayQueue(IReadOnlyList<string> playlistItemIds);
 
         /// <summary>
         /// Moves an item in the play queue.
@@ -181,7 +181,7 @@ namespace MediaBrowser.Controller.SyncPlay
         /// <param name="newItems">The new items to add to the play queue.</param>
         /// <param name="mode">The mode with which the items will be added.</param>
         /// <returns><c>true</c> if the play queue has been changed; <c>false</c> if something went wrong.</returns>
-        bool AddToPlayQueue(IEnumerable<Guid> newItems, GroupQueueMode mode);
+        bool AddToPlayQueue(IReadOnlyList<Guid> newItems, GroupQueueMode mode);
 
         /// <summary>
         /// Restarts current item in play queue.

MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs

@@ -49,21 +49,5 @@ namespace MediaBrowser.Controller.SyncPlay
         /// <param name="request">The request.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
         void HandleRequest(SessionInfo session, IGroupPlaybackRequest request, CancellationToken cancellationToken);
-
-        /// <summary>
-        /// Maps a session to a group.
-        /// </summary>
-        /// <param name="session">The session.</param>
-        /// <param name="group">The group.</param>
-        /// <exception cref="InvalidOperationException">Thrown when the user is in another group already.</exception>
-        void AddSessionToGroup(SessionInfo session, IGroupController group);
-
-        /// <summary>
-        /// Unmaps a session from a group.
-        /// </summary>
-        /// <param name="session">The session.</param>
-        /// <param name="group">The group.</param>
-        /// <exception cref="InvalidOperationException">Thrown when the user is not found in the specified group.</exception>
-        void RemoveSessionFromGroup(SessionInfo session, IGroupController group);
     }
 }

MediaBrowser.Controller/SyncPlay/PlaybackRequests/PlayGroupRequest.cs

@@ -19,9 +19,7 @@ namespace MediaBrowser.Controller.SyncPlay.PlaybackRequests
         /// <param name="startPositionTicks">The start position ticks.</param>
         public PlayGroupRequest(Guid[] playingQueue, int playingItemPosition, long startPositionTicks)
         {
-            var list = new List<Guid>();
-            list.AddRange(playingQueue);
-            PlayingQueue = list;
+            PlayingQueue = playingQueue ?? Array.Empty<Guid>();
             PlayingItemPosition = playingItemPosition;
             StartPositionTicks = startPositionTicks;
         }

MediaBrowser.Controller/SyncPlay/PlaybackRequests/QueueGroupRequest.cs

@@ -18,9 +18,7 @@ namespace MediaBrowser.Controller.SyncPlay.PlaybackRequests
         /// <param name="mode">The enqueue mode.</param>
         public QueueGroupRequest(Guid[] items, GroupQueueMode mode)
         {
-            var list = new List<Guid>();
-            list.AddRange(items);
-            ItemIds = list;
+            ItemIds = items ?? Array.Empty<Guid>();
             Mode = mode;
         }
 

MediaBrowser.Controller/SyncPlay/PlaybackRequests/RemoveFromPlaylistGroupRequest.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Threading;
 using MediaBrowser.Controller.Session;
@@ -16,9 +17,7 @@ namespace MediaBrowser.Controller.SyncPlay.PlaybackRequests
         /// <param name="items">The playlist ids of the items to remove.</param>
         public RemoveFromPlaylistGroupRequest(string[] items)
         {
-            var list = new List<string>();
-            list.AddRange(items);
-            PlaylistItemIds = list;
+            PlaylistItemIds = items ?? Array.Empty<string>();
         }
 
         /// <summary>

MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs

@@ -94,7 +94,7 @@ namespace MediaBrowser.Controller.SyncPlay.Queue
         /// Sets a new playlist. Playing item is reset.
         /// </summary>
         /// <param name="items">The new items of the playlist.</param>
-        public void SetPlaylist(IEnumerable<Guid> items)
+        public void SetPlaylist(IReadOnlyList<Guid> items)
         {
             SortedPlaylist.Clear();
             ShuffledPlaylist.Clear();
@@ -114,7 +114,7 @@ namespace MediaBrowser.Controller.SyncPlay.Queue
         /// Appends new items to the playlist. The specified order is mantained.
         /// </summary>
         /// <param name="items">The items to add to the playlist.</param>
-        public void Queue(IEnumerable<Guid> items)
+        public void Queue(IReadOnlyList<Guid> items)
         {
             var newItems = CreateQueueItemsFromArray(items);
 
@@ -209,7 +209,7 @@ namespace MediaBrowser.Controller.SyncPlay.Queue
         /// Adds new items to the playlist right after the playing item. The specified order is mantained.
         /// </summary>
         /// <param name="items">The items to add to the playlist.</param>
-        public void QueueNext(IEnumerable<Guid> items)
+        public void QueueNext(IReadOnlyList<Guid> items)
         {
             var newItems = CreateQueueItemsFromArray(items);
 
@@ -312,13 +312,12 @@ namespace MediaBrowser.Controller.SyncPlay.Queue
         /// </summary>
         /// <param name="playlistItemIds">The items to remove.</param>
         /// <returns><c>true</c> if playing item got removed; <c>false</c> otherwise.</returns>
-        public bool RemoveFromPlaylist(IEnumerable<string> playlistItemIds)
+        public bool RemoveFromPlaylist(IReadOnlyList<string> playlistItemIds)
         {
             var playingItem = GetPlayingItem();
-            var playlistItemIdsList = playlistItemIds.ToList();
 
-            SortedPlaylist.RemoveAll(item => playlistItemIdsList.Contains(item.PlaylistItemId));
-            ShuffledPlaylist.RemoveAll(item => playlistItemIdsList.Contains(item.PlaylistItemId));
+            SortedPlaylist.RemoveAll(item => playlistItemIds.Contains(item.PlaylistItemId));
+            ShuffledPlaylist.RemoveAll(item => playlistItemIds.Contains(item.PlaylistItemId));
 
             LastChange = DateTime.UtcNow;
 
@@ -369,8 +368,7 @@ namespace MediaBrowser.Controller.SyncPlay.Queue
 
             var queueItem = playlist[oldIndex];
             playlist.RemoveAt(oldIndex);
-            newIndex = Math.Min(newIndex, playlist.Count);
-            newIndex = Math.Max(newIndex, 0);
+            newIndex = Math.Clamp(newIndex, 0, playlist.Count);
             playlist.Insert(newIndex, queueItem);
 
             LastChange = DateTime.UtcNow;
@@ -489,7 +487,7 @@ namespace MediaBrowser.Controller.SyncPlay.Queue
                 }
                 else
                 {
-                    PlayingItemIndex--;
+                    PlayingItemIndex = SortedPlaylist.Count - 1;
                     return false;
                 }
             }
@@ -519,7 +517,7 @@ namespace MediaBrowser.Controller.SyncPlay.Queue
                 }
                 else
                 {
-                    PlayingItemIndex++;
+                    PlayingItemIndex = 0;
                     return false;
                 }
             }
@@ -558,7 +556,7 @@ namespace MediaBrowser.Controller.SyncPlay.Queue
         /// Creates a list from the array of items. Each item is given an unique playlist identifier.
         /// </summary>
         /// <returns>The list of queue items.</returns>
-        private List<QueueItem> CreateQueueItemsFromArray(IEnumerable<Guid> items)
+        private List<QueueItem> CreateQueueItemsFromArray(IReadOnlyList<Guid> items)
         {
             var list = new List<QueueItem>();
             foreach (var item in items)

MediaBrowser.Model/SyncPlay/GroupInfoDto.cs

@@ -11,41 +11,48 @@ namespace MediaBrowser.Model.SyncPlay
         /// <summary>
         /// Initializes a new instance of the <see cref="GroupInfoDto"/> class.
         /// </summary>
-        public GroupInfoDto()
+        /// <param name="groupId">The group identifier.</param>
+        /// <param name="groupName">The group name.</param>
+        /// <param name="state">The group state.</param>
+        /// <param name="participants">The participants.</param>
+        /// <param name="lastUpdatedAt">The date when this DTO has been created.</param>
+        public GroupInfoDto(Guid groupId, string groupName, GroupStateType state, IReadOnlyList<string> participants, DateTime lastUpdatedAt)
         {
-            GroupId = string.Empty;
-            GroupName = string.Empty;
-            Participants = new List<string>();
+            GroupId = groupId;
+            GroupName = groupName;
+            State = state;
+            Participants = participants;
+            LastUpdatedAt = lastUpdatedAt;
         }
 
         /// <summary>
-        /// Gets or sets the group identifier.
+        /// Gets the group identifier.
         /// </summary>
         /// <value>The group identifier.</value>
-        public string GroupId { get; set; }
+        public Guid GroupId { get; }
 
         /// <summary>
-        /// Gets or sets the group name.
+        /// Gets the group name.
         /// </summary>
         /// <value>The group name.</value>
-        public string GroupName { get; set; }
+        public string GroupName { get; }
 
         /// <summary>
-        /// Gets or sets the group state.
+        /// Gets the group state.
         /// </summary>
         /// <value>The group state.</value>
-        public GroupStateType State { get; set; }
+        public GroupStateType State { get; }
 
         /// <summary>
-        /// Gets or sets the participants.
+        /// Gets the participants.
         /// </summary>
         /// <value>The participants.</value>
-        public IReadOnlyList<string> Participants { get; set; }
+        public IReadOnlyList<string> Participants { get; }
 
         /// <summary>
-        /// Gets or sets the date when this dto has been updated.
+        /// Gets the date when this DTO has been created.
         /// </summary>
-        /// <value>The date when this dto has been updated.</value>
-        public DateTime LastUpdatedAt { get; set; }
+        /// <value>The date when this DTO has been created.</value>
+        public DateTime LastUpdatedAt { get; }
     }
 }

MediaBrowser.Model/SyncPlay/GroupStateUpdate.cs

@@ -6,15 +6,26 @@ namespace MediaBrowser.Model.SyncPlay
     public class GroupStateUpdate
     {
         /// <summary>
-        /// Gets or sets the state of the group.
+        /// Initializes a new instance of the <see cref="GroupStateUpdate"/> class.
+        /// </summary>
+        /// <param name="state">The state of the group.</param>
+        /// <param name="reason">The reason of the state change.</param>
+        public GroupStateUpdate(GroupStateType state, PlaybackRequestType reason)
+        {
+            State = state;
+            Reason = reason;
+        }
+
+        /// <summary>
+        /// Gets the state of the group.
         /// </summary>
         /// <value>The state of the group.</value>
-        public GroupStateType State { get; set; }
+        public GroupStateType State { get; }
 
         /// <summary>
-        /// Gets or sets the reason of the state change.
+        /// Gets the reason of the state change.
         /// </summary>
         /// <value>The reason of the state change.</value>
-        public PlaybackRequestType Reason { get; set; }
+        public PlaybackRequestType Reason { get; }
     }
 }

MediaBrowser.Model/SyncPlay/GroupUpdate.cs

@@ -1,4 +1,4 @@
-#nullable disable
+using System;
 
 namespace MediaBrowser.Model.SyncPlay
 {
@@ -9,21 +9,34 @@ namespace MediaBrowser.Model.SyncPlay
     public class GroupUpdate<T>
     {
         /// <summary>
-        /// Gets or sets the group identifier.
+        /// Initializes a new instance of the <see cref="GroupUpdate{T}"/> class.
+        /// </summary>
+        /// <param name="groupId">The group identifier.</param>
+        /// <param name="type">The update type.</param>
+        /// <param name="data">The update data.</param>
+        public GroupUpdate(Guid groupId, GroupUpdateType type, T data)
+        {
+            GroupId = groupId;
+            Type = type;
+            Data = data;
+        }
+
+        /// <summary>
+        /// Gets the group identifier.
         /// </summary>
         /// <value>The group identifier.</value>
-        public string GroupId { get; set; }
+        public Guid GroupId { get; }
 
         /// <summary>
-        /// Gets or sets the update type.
+        /// Gets the update type.
         /// </summary>
         /// <value>The update type.</value>
-        public GroupUpdateType Type { get; set; }
+        public GroupUpdateType Type { get; }
 
         /// <summary>
-        /// Gets or sets the data.
+        /// Gets the update data.
         /// </summary>
-        /// <value>The data.</value>
-        public T Data { get; set; }
+        /// <value>The update data.</value>
+        public T Data { get; }
     }
 }

MediaBrowser.Model/SyncPlay/JoinGroupRequest.cs

@@ -8,9 +8,18 @@ namespace MediaBrowser.Model.SyncPlay
     public class JoinGroupRequest
     {
         /// <summary>
-        /// Gets or sets the group identifier.
+        /// Initializes a new instance of the <see cref="JoinGroupRequest"/> class.
+        /// </summary>
+        /// <param name="groupId">The identifier of the group to join.</param>
+        public JoinGroupRequest(Guid groupId)
+        {
+            GroupId = groupId;
+        }
+
+        /// <summary>
+        /// Gets the group identifier.
         /// </summary>
         /// <value>The identifier of the group to join.</value>
-        public Guid GroupId { get; set; }
+        public Guid GroupId { get; }
     }
 }

MediaBrowser.Model/SyncPlay/NewGroupRequest.cs

@@ -8,15 +8,16 @@ namespace MediaBrowser.Model.SyncPlay
         /// <summary>
         /// Initializes a new instance of the <see cref="NewGroupRequest"/> class.
         /// </summary>
-        public NewGroupRequest()
+        /// <param name="groupName">The name of the new group.</param>
+        public NewGroupRequest(string groupName)
         {
-            GroupName = string.Empty;
+            GroupName = groupName;
         }
 
         /// <summary>
-        /// Gets or sets the group name.
+        /// Gets the group name.
         /// </summary>
         /// <value>The name of the new group.</value>
-        public string GroupName { get; set; }
+        public string GroupName { get; }
     }
 }

MediaBrowser.Model/SyncPlay/PlayQueueUpdate.cs

@@ -11,51 +11,64 @@ namespace MediaBrowser.Model.SyncPlay
         /// <summary>
         /// Initializes a new instance of the <see cref="PlayQueueUpdate"/> class.
         /// </summary>
-        public PlayQueueUpdate()
+        /// <param name="reason">The reason for the update.</param>
+        /// <param name="lastUpdate">The UTC time of the last change to the playing queue.</param>
+        /// <param name="playlist">The playlist.</param>
+        /// <param name="playingItemIndex">The playing item index in the playlist.</param>
+        /// <param name="startPositionTicks">The start position ticks.</param>
+        /// <param name="shuffleMode">The shuffle mode.</param>
+        /// <param name="repeatMode">The repeat mode.</param>
+        public PlayQueueUpdate(PlayQueueUpdateReason reason, DateTime lastUpdate, IReadOnlyList<QueueItem> playlist, int playingItemIndex, long startPositionTicks, GroupShuffleMode shuffleMode, GroupRepeatMode repeatMode)
         {
-            Playlist = new List<QueueItem>();
+            Reason = reason;
+            LastUpdate = lastUpdate;
+            Playlist = playlist;
+            PlayingItemIndex = playingItemIndex;
+            StartPositionTicks = startPositionTicks;
+            ShuffleMode = shuffleMode;
+            RepeatMode = repeatMode;
         }
 
         /// <summary>
-        /// Gets or sets the request type that originated this update.
+        /// Gets the request type that originated this update.
         /// </summary>
         /// <value>The reason for the update.</value>
-        public PlayQueueUpdateReason Reason { get; set; }
+        public PlayQueueUpdateReason Reason { get; }
 
         /// <summary>
-        /// Gets or sets the UTC time of the last change to the playing queue.
+        /// Gets the UTC time of the last change to the playing queue.
         /// </summary>
         /// <value>The UTC time of the last change to the playing queue.</value>
-        public DateTime LastUpdate { get; set; }
+        public DateTime LastUpdate { get; }
 
         /// <summary>
-        /// Gets or sets the playlist.
+        /// Gets the playlist.
         /// </summary>
         /// <value>The playlist.</value>
-        public IReadOnlyList<QueueItem> Playlist { get; set; }
+        public IReadOnlyList<QueueItem> Playlist { get; }
 
         /// <summary>
-        /// Gets or sets the playing item index in the playlist.
+        /// Gets the playing item index in the playlist.
         /// </summary>
         /// <value>The playing item index in the playlist.</value>
-        public int PlayingItemIndex { get; set; }
+        public int PlayingItemIndex { get; }
 
         /// <summary>
-        /// Gets or sets the start position ticks.
+        /// Gets the start position ticks.
         /// </summary>
         /// <value>The start position ticks.</value>
-        public long StartPositionTicks { get; set; }
+        public long StartPositionTicks { get; }
 
         /// <summary>
-        /// Gets or sets the shuffle mode.
+        /// Gets the shuffle mode.
         /// </summary>
         /// <value>The shuffle mode.</value>
-        public GroupShuffleMode ShuffleMode { get; set; }
+        public GroupShuffleMode ShuffleMode { get; }
 
         /// <summary>
-        /// Gets or sets the repeat mode.
+        /// Gets the repeat mode.
         /// </summary>
         /// <value>The repeat mode.</value>
-        public GroupRepeatMode RepeatMode { get; set; }
+        public GroupRepeatMode RepeatMode { get; }
     }
 }

MediaBrowser.Model/SyncPlay/SendCommand.cs

@@ -10,23 +10,33 @@ namespace MediaBrowser.Model.SyncPlay
         /// <summary>
         /// Initializes a new instance of the <see cref="SendCommand"/> class.
         /// </summary>
-        public SendCommand()
+        /// <param name="groupId">The group identifier.</param>
+        /// <param name="playlistItemId">The playlist identifier of the playing item.</param>
+        /// <param name="when">The UTC time when to execute the command.</param>
+        /// <param name="command">The command.</param>
+        /// <param name="positionTicks">The position ticks, for commands that require it.</param>
+        /// <param name="emittedAt">The UTC time when this command has been emitted.</param>
+        public SendCommand(Guid groupId, string playlistItemId, DateTime when, SendCommandType command, long? positionTicks, DateTime emittedAt)
         {
-            GroupId = string.Empty;
-            PlaylistItemId = string.Empty;
+            GroupId = groupId;
+            PlaylistItemId = playlistItemId;
+            When = when;
+            Command = command;
+            PositionTicks = positionTicks;
+            EmittedAt = emittedAt;
         }
 
         /// <summary>
-        /// Gets or sets the group identifier.
+        /// Gets the group identifier.
         /// </summary>
         /// <value>The group identifier.</value>
-        public string GroupId { get; set; }
+        public Guid GroupId { get; }
 
         /// <summary>
-        /// Gets or sets the playlist identifier of the playing item.
+        /// Gets the playlist identifier of the playing item.
         /// </summary>
         /// <value>The playlist identifier of the playing item.</value>
-        public string PlaylistItemId { get; set; }
+        public string PlaylistItemId { get; }
 
         /// <summary>
         /// Gets or sets the UTC time when to execute the command.
@@ -35,21 +45,21 @@ namespace MediaBrowser.Model.SyncPlay
         public DateTime When { get; set; }
 
         /// <summary>
-        /// Gets or sets the position ticks.
+        /// Gets the position ticks.
         /// </summary>
         /// <value>The position ticks.</value>
-        public long? PositionTicks { get; set; }
+        public long? PositionTicks { get; }
 
         /// <summary>
-        /// Gets or sets the command.
+        /// Gets the command.
         /// </summary>
         /// <value>The command.</value>
-        public SendCommandType Command { get; set; }
+        public SendCommandType Command { get; }
 
         /// <summary>
-        /// Gets or sets the UTC time when this command has been emitted.
+        /// Gets the UTC time when this command has been emitted.
         /// </summary>
         /// <value>The UTC time when this command has been emitted.</value>
-        public DateTime EmittedAt { get; set; }
+        public DateTime EmittedAt { get; }
     }
 }

MediaBrowser.Model/SyncPlay/UtcTimeResponse.cs

@@ -1,4 +1,4 @@
-#nullable disable
+using System;
 
 namespace MediaBrowser.Model.SyncPlay
 {
@@ -8,15 +8,26 @@ namespace MediaBrowser.Model.SyncPlay
     public class UtcTimeResponse
     {
         /// <summary>
-        /// Gets or sets the UTC time when request has been received.
+        /// Initializes a new instance of the <see cref="UtcTimeResponse"/> class.
+        /// </summary>
+        /// <param name="requestReceptionTime">The UTC time when request has been received.</param>
+        /// <param name="responseTransmissionTime">The UTC time when response has been sent.</param>
+        public UtcTimeResponse(DateTime requestReceptionTime, DateTime responseTransmissionTime)
+        {
+            RequestReceptionTime = requestReceptionTime;
+            ResponseTransmissionTime = responseTransmissionTime;
+        }
+
+        /// <summary>
+        /// Gets the UTC time when request has been received.
         /// </summary>
         /// <value>The UTC time when request has been received.</value>
-        public string RequestReceptionTime { get; set; }
+        public DateTime RequestReceptionTime { get; }
 
         /// <summary>
-        /// Gets or sets the UTC time when response has been sent.
+        /// Gets the UTC time when response has been sent.
         /// </summary>
         /// <value>The UTC time when response has been sent.</value>
-        public string ResponseTransmissionTime { get; set; }
+        public DateTime ResponseTransmissionTime { get; }
     }
 }