Fix code issues

8 files changed, 86 insertions, 54 deletions

diff --git a/Emby.Server.Implementations/Session/SessionWebSocketListener.cs b/Emby.Server.Implementations/Session/SessionWebSocketListener.cs
index d1ee22ea8..3704445ab 100644
--- a/Emby.Server.Implementations/Session/SessionWebSocketListener.cs
+++ b/Emby.Server.Implementations/Session/SessionWebSocketListener.cs
@@ -22,17 +22,17 @@ namespace Emby.Server.Implementations.Session
         /// <summary>
         /// The timeout in seconds after which a WebSocket is considered to be lost.
         /// </summary>
-        public readonly int WebSocketLostTimeout = 60;
+        public const int WebSocketLostTimeout = 60;
 
         /// <summary>
         /// The keep-alive interval factor; controls how often the watcher will check on the status of the WebSockets.
         /// </summary>
-        public readonly double IntervalFactor = 0.2;
+        public const float IntervalFactor = 0.2f;
 
         /// <summary>
         /// The ForceKeepAlive factor; controls when a ForceKeepAlive is sent.
         /// </summary>
-        public readonly double ForceKeepAliveFactor = 0.75;
+        public const float ForceKeepAliveFactor = 0.75f;
 
         /// <summary>
         /// The _session manager
@@ -213,7 +213,7 @@ namespace Emby.Server.Implementations.Session
                 {
                     _keepAliveCancellationToken = new CancellationTokenSource();
                     // Start KeepAlive watcher
-                    var task = RepeatAsyncCallbackEvery(
+                    _ = RepeatAsyncCallbackEvery(
                         KeepAliveSockets,
                         TimeSpan.FromSeconds(WebSocketLostTimeout * IntervalFactor),
                         _keepAliveCancellationToken.Token);
@@ -241,6 +241,7 @@ namespace Emby.Server.Implementations.Session
                 {
                     webSocket.Closed -= OnWebSocketClosed;
                 }
+
                 _webSockets.Clear();
             }
         }
@@ -250,8 +251,8 @@ namespace Emby.Server.Implementations.Session
         /// </summary>
         private async Task KeepAliveSockets()
         {
-            IEnumerable<IWebSocketConnection> inactive;
-            IEnumerable<IWebSocketConnection> lost;
+            List<IWebSocketConnection> inactive;
+            List<IWebSocketConnection> lost;
 
             lock (_webSocketsLock)
             {
@@ -261,8 +262,8 @@ namespace Emby.Server.Implementations.Session
                 {
                     var elapsed = (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds;
                     return (elapsed > WebSocketLostTimeout * ForceKeepAliveFactor) && (elapsed < WebSocketLostTimeout);
-                });
-                lost = _webSockets.Where(i => (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds >= WebSocketLostTimeout);
+                }).ToList();
+                lost = _webSockets.Where(i => (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds >= WebSocketLostTimeout).ToList();
             }
 
             if (inactive.Any())
@@ -279,7 +280,7 @@ namespace Emby.Server.Implementations.Session
                 catch (WebSocketException exception)
                 {
                     _logger.LogInformation(exception, "Error sending ForceKeepAlive message to WebSocket.");
-                    lost = lost.Append(webSocket);
+                    lost.Add(webSocket);
                 }
             }
 
@@ -288,7 +289,7 @@ namespace Emby.Server.Implementations.Session
                 if (lost.Any())
                 {
                     _logger.LogInformation("Lost {0} WebSockets.", lost.Count());
-                    foreach (var webSocket in lost.ToList())
+                    foreach (var webSocket in lost)
                     {
                         // TODO: handle session relative to the lost webSocket
                         RemoveWebSocket(webSocket);
diff --git a/Emby.Server.Implementations/SyncPlay/SyncPlayController.cs b/Emby.Server.Implementations/SyncPlay/SyncPlayController.cs
index 9c9758de1..c7bd242a7 100644
--- a/Emby.Server.Implementations/SyncPlay/SyncPlayController.cs
+++ b/Emby.Server.Implementations/SyncPlay/SyncPlayController.cs
@@ -144,7 +144,7 @@ namespace Emby.Server.Implementations.SyncPlay
                         session => session.Session
                     ).ToArray();
                 default:
-                    return new SessionInfo[] { };
+                    return Array.Empty<SessionInfo>();
             }
         }
 
@@ -541,7 +541,7 @@ namespace Emby.Server.Implementations.SyncPlay
                 PlayingItemName = _group.PlayingItem.Name,
                 PlayingItemId = _group.PlayingItem.Id.ToString(),
                 PositionTicks = _group.PositionTicks,
-                Participants = _group.Participants.Values.Select(session => session.Session.UserName).Distinct().ToArray()
+                Participants = _group.Participants.Values.Select(session => session.Session.UserName).Distinct().ToList().AsReadOnly()
             };
         }
     }
diff --git a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs
index d3197d97b..93cec1304 100644
--- a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs
+++ b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs
@@ -47,8 +47,8 @@ namespace Emby.Server.Implementations.SyncPlay
         /// <summary>
         /// The groups.
         /// </summary>
-        private readonly Dictionary<string, ISyncPlayController> _groups =
-            new Dictionary<string, ISyncPlayController>(StringComparer.OrdinalIgnoreCase);
+        private readonly Dictionary<Guid, ISyncPlayController> _groups =
+            new Dictionary<Guid, ISyncPlayController>();
 
         /// <summary>
         /// Lock used for accesing any group.
@@ -113,14 +113,22 @@ namespace Emby.Server.Implementations.SyncPlay
         private void OnSessionManagerSessionEnded(object sender, SessionEventArgs e)
         {
             var session = e.SessionInfo;
-            if (!IsSessionInGroup(session)) return;
+            if (!IsSessionInGroup(session))
+            {
+                return;
+            }
+
             LeaveGroup(session, CancellationToken.None);
         }
 
         private void OnSessionManagerPlaybackStopped(object sender, PlaybackStopEventArgs e)
         {
             var session = e.Session;
-            if (!IsSessionInGroup(session)) return;
+            if (!IsSessionInGroup(session))
+            {
+                return;
+            }
+
             LeaveGroup(session, CancellationToken.None);
         }
 
@@ -193,14 +201,14 @@ namespace Emby.Server.Implementations.SyncPlay
                 }
 
                 var group = new SyncPlayController(_sessionManager, this);
-                _groups[group.GetGroupId().ToString()] = group;
+                _groups[group.GetGroupId()] = group;
 
                 group.InitGroup(session, cancellationToken);
             }
         }
 
         /// <inheritdoc />
-        public void JoinGroup(SessionInfo session, string groupId, JoinGroupRequest request, CancellationToken cancellationToken)
+        public void JoinGroup(SessionInfo session, Guid groupId, JoinGroupRequest request, CancellationToken cancellationToken)
         {
             var user = _userManager.GetUserById(session.UserId);
 
@@ -248,7 +256,11 @@ namespace Emby.Server.Implementations.SyncPlay
 
                 if (IsSessionInGroup(session))
                 {
-                    if (GetSessionGroup(session).Equals(groupId)) return;
+                    if (GetSessionGroup(session).Equals(groupId))
+                    {
+                        return;
+                    }
+
                     LeaveGroup(session, cancellationToken);
                 }
 
@@ -282,7 +294,7 @@ namespace Emby.Server.Implementations.SyncPlay
                 if (group.IsGroupEmpty())
                 {
                     _logger.LogInformation("LeaveGroup: removing empty group {0}.", group.GetGroupId());
-                    _groups.Remove(group.GetGroupId().ToString(), out _);
+                    _groups.Remove(group.GetGroupId(), out _);
                 }
             }
         }
diff --git a/MediaBrowser.Api/SyncPlay/SyncPlayService.cs b/MediaBrowser.Api/SyncPlay/SyncPlayService.cs
index bcdc833e4..9137faf9f 100644
--- a/MediaBrowser.Api/SyncPlay/SyncPlayService.cs
+++ b/MediaBrowser.Api/SyncPlay/SyncPlayService.cs
@@ -171,31 +171,35 @@ namespace MediaBrowser.Api.SyncPlay
         public void Post(SyncPlayJoinGroup request)
         {
             var currentSession = GetSession(_sessionContext);
-            var joinRequest = new JoinGroupRequest()
+
+            Guid groupId;
+            Guid playingItemId = Guid.Empty;
+
+            var valid = Guid.TryParse(request.GroupId, out groupId);
+            if (!valid)
             {
-                GroupId = Guid.Parse(request.GroupId)
-            };
+                Logger.LogError("JoinGroup: {0} is not a valid format for GroupId. Ignoring request.", request.GroupId);
+                return;
+            }
 
             // Both null and empty strings mean that client isn't playing anything
             if (!String.IsNullOrEmpty(request.PlayingItemId))
             {
-                try
-                {
-                    joinRequest.PlayingItemId = Guid.Parse(request.PlayingItemId);
-                }
-                catch (ArgumentNullException)
-                {
-                    // Should never happen, but just in case
-                    Logger.LogError("JoinGroup: null value for PlayingItemId. Ignoring request.");
-                    return;
-                }
-                catch (FormatException)
+                valid = Guid.TryParse(request.PlayingItemId, out playingItemId);
+                if (!valid)
                 {
                     Logger.LogError("JoinGroup: {0} is not a valid format for PlayingItemId. Ignoring request.", request.PlayingItemId);
                     return;
                 }
             }
-            _syncPlayManager.JoinGroup(currentSession, request.GroupId, joinRequest, CancellationToken.None);
+
+            var joinRequest = new JoinGroupRequest()
+            {
+                GroupId = groupId,
+                PlayingItemId = playingItemId
+            };
+
+            _syncPlayManager.JoinGroup(currentSession, groupId, joinRequest, CancellationToken.None);
         }
 
         /// <summary>
@@ -217,21 +221,16 @@ namespace MediaBrowser.Api.SyncPlay
         {
             var currentSession = GetSession(_sessionContext);
             var filterItemId = Guid.Empty;
+
             if (!String.IsNullOrEmpty(request.FilterItemId))
             {
-                try
-                {
-                    filterItemId = Guid.Parse(request.FilterItemId);
-                }
-                catch (ArgumentNullException)
-                {
-                    Logger.LogWarning("ListGroups: null value for FilterItemId. Ignoring filter.");
-                }
-                catch (FormatException)
+                var valid = Guid.TryParse(request.FilterItemId, out filterItemId);
+                if (!valid)
                 {
                     Logger.LogWarning("ListGroups: {0} is not a valid format for FilterItemId. Ignoring filter.", request.FilterItemId);
                 }
             }
+
             return _syncPlayManager.ListGroups(currentSession, filterItemId);
         }
 
diff --git a/MediaBrowser.Controller/Net/IWebSocketConnection.cs b/MediaBrowser.Controller/Net/IWebSocketConnection.cs
index fb766ab57..b371a59e9 100644
--- a/MediaBrowser.Controller/Net/IWebSocketConnection.cs
+++ b/MediaBrowser.Controller/Net/IWebSocketConnection.cs
@@ -30,7 +30,7 @@ namespace MediaBrowser.Controller.Net
         /// Gets or sets the date of last Keeplive received.
         /// </summary>
         /// <value>The date of last Keeplive received.</value>
-        public DateTime LastKeepAliveDate { get; set; }
+        DateTime LastKeepAliveDate { get; set; }
 
         /// <summary>
         /// Gets or sets the URL.
diff --git a/MediaBrowser.Controller/SyncPlay/GroupInfo.cs b/MediaBrowser.Controller/SyncPlay/GroupInfo.cs
index 087748de0..bda49bd1b 100644
--- a/MediaBrowser.Controller/SyncPlay/GroupInfo.cs
+++ b/MediaBrowser.Controller/SyncPlay/GroupInfo.cs
@@ -69,7 +69,11 @@ namespace MediaBrowser.Controller.SyncPlay
         /// <param name="session">The session.</param>
         public void AddSession(SessionInfo session)
         {
-            if (ContainsSession(session.Id.ToString())) return;
+            if (ContainsSession(session.Id.ToString()))
+            {
+                return;
+            }
+
             var member = new GroupMember();
             member.Session = session;
             member.Ping = DefaulPing;
@@ -84,9 +88,12 @@ namespace MediaBrowser.Controller.SyncPlay
 
         public void RemoveSession(SessionInfo session)
         {
-            if (!ContainsSession(session.Id.ToString())) return;
-            GroupMember member;
-            Participants.Remove(session.Id.ToString(), out member);
+            if (!ContainsSession(session.Id.ToString()))
+            {
+                return;
+            }
+
+            Participants.Remove(session.Id.ToString(), out _);
         }
 
         /// <summary>
@@ -96,7 +103,11 @@ namespace MediaBrowser.Controller.SyncPlay
         /// <param name="ping">The ping.</param>
         public void UpdatePing(SessionInfo session, long ping)
         {
-            if (!ContainsSession(session.Id.ToString())) return;
+            if (!ContainsSession(session.Id.ToString()))
+            {
+                return;
+            }
+
             Participants[session.Id.ToString()].Ping = ping;
         }
 
@@ -121,7 +132,11 @@ namespace MediaBrowser.Controller.SyncPlay
         /// <param name="isBuffering">The state.</param>
         public void SetBuffering(SessionInfo session, bool isBuffering)
         {
-            if (!ContainsSession(session.Id.ToString())) return;
+            if (!ContainsSession(session.Id.ToString()))
+            {
+                return;
+            }
+
             Participants[session.Id.ToString()].IsBuffering = isBuffering;
         }
 
@@ -133,7 +148,10 @@ namespace MediaBrowser.Controller.SyncPlay
         {
             foreach (var session in Participants.Values)
             {
-                if (session.IsBuffering) return true;
+                if (session.IsBuffering)
+                {
+                    return true;
+                }
             }
             return false;
         }
diff --git a/MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs b/MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs
index 6c962ec85..006fb687b 100644
--- a/MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs
+++ b/MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs
@@ -25,7 +25,7 @@ namespace MediaBrowser.Controller.SyncPlay
         /// <param name="groupId">The group id.</param>
         /// <param name="request">The request.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
-        void JoinGroup(SessionInfo session, string groupId, JoinGroupRequest request, CancellationToken cancellationToken);
+        void JoinGroup(SessionInfo session, Guid groupId, JoinGroupRequest request, CancellationToken cancellationToken);
 
         /// <summary>
         /// Removes the session from a group.
diff --git a/MediaBrowser.Model/SyncPlay/GroupInfoView.cs b/MediaBrowser.Model/SyncPlay/GroupInfoView.cs
index 7b833506b..f28ecf16d 100644
--- a/MediaBrowser.Model/SyncPlay/GroupInfoView.cs
+++ b/MediaBrowser.Model/SyncPlay/GroupInfoView.cs
@@ -1,3 +1,5 @@
+using System.Collections.Generic;
+
 namespace MediaBrowser.Model.SyncPlay
 {
     /// <summary>
@@ -33,6 +35,6 @@ namespace MediaBrowser.Model.SyncPlay
         /// Gets or sets the participants.
         /// </summary>
         /// <value>The participants.</value>
-        public string[] Participants { get; set; }
+        public IReadOnlyList<string> Participants { get; set; }
     }
 }