Add regression tests for async station refresh race conditions

Libs/PandoraSharp/Pandora.cs

@@ -282,7 +282,7 @@ protected internal async Task<string> CallRPC_Internal(string method, JObject re
             return response;
         }
 
-        protected internal async Task<JSONResult> CallRPC(string method, JObject request = null, bool isAuth = false, bool useSSL = false)
+        protected internal virtual async Task<JSONResult> CallRPC(string method, JObject request = null, bool isAuth = false, bool useSSL = false)
         {
             try
             {
@@ -343,77 +343,81 @@ protected internal object CallRPC(string method, object[] args, bool b_url_args
         public async Task<List<Station>> RefreshStationsAsync()
         {
             Log.O("RefreshStations");
-            if (StationsUpdatingEvent != null)
-                StationsUpdatingEvent(this);
+            StationsUpdatingEvent?.Invoke(this);
+
+            JObject req = new JObject
+            {
+                ["includeStationArtUrl"] = true
+            };
 
-            JObject req = new JObject();
-            req["includeStationArtUrl"] = true;
             var stationList = await CallRPC("user.getStationList", req);
-            // Use this local variable throughout the method.
-            List<Station> localStations = new List<Station>();
 
-            // Lock all modifications to the station lists
+            List<Station> quickMixes;
+            List<Station> normalStations;
+
             lock (_stationListLock)
             {
-
                 QuickMixStationIDs.Clear();
 
-                Stations = new List<Station>();
-                var stations = stationList.Result["stations"];
-                foreach (JToken d in stations)
+                var fetchedStations = new List<Station>();
+                var stationsToken = stationList?.Result?["stations"] as JArray;
+
+                if (stationsToken != null)
                 {
-                    Stations.Add(new Station(this, d));
+                    foreach (JToken d in stationsToken)
+                    {
+                        fetchedStations.Add(new Station(this, d));
+                    }
                 }
-                //foreach (PDict s in stationList)
-                //    Stations.Add(new Station(this, s));
 
                 if (QuickMixStationIDs.Count > 0)
                 {
-                    foreach (Station s in Stations)
+                    foreach (Station s in fetchedStations)
                     {
                         if (QuickMixStationIDs.Contains(s.ID))
                             s.UseQuickMix = true;
                     }
                 }
 
-                List<Station> quickMixes = Stations.FindAll(x => x.IsQuickMix);
-                Stations = Stations.FindAll(x => !x.IsQuickMix);
+                quickMixes = fetchedStations.Where(x => x.IsQuickMix).ToList();
+                normalStations = fetchedStations.Where(x => !x.IsQuickMix).ToList();
+            }
 
-                switch (StationSortOrder)
-                {
-                    case SortOrder.DateDesc:
-                        //Stations = Stations.OrderByDescending(x => x.ID).ToList();
-                        Stations = Stations.OrderByDescending(x => Convert.ToInt64(x.ID)).ToList();
-                        break;
-                    case SortOrder.DateAsc:
-                        //Stations = Stations.OrderBy(x => x.ID).ToList();
-                        Stations = Stations.OrderBy(x => Convert.ToInt64(x.ID)).ToList();
-                        break;
-                    case SortOrder.AlphaDesc:
-                        Stations = Stations.OrderByDescending(x => x.Name).ToList();
-                        break;
-                    case SortOrder.AlphaAsc:
-                        Stations = Stations.OrderBy(x => x.Name).ToList();
-                        break;
-                    case SortOrder.RatingAsc:
-                        GetStationMetaData();
-                        Stations = Stations.OrderBy(x => x.ThumbsUp).ToList();
-                        break;
-                    case SortOrder.RatingDesc:
-                        GetStationMetaData();
-                        Stations = Stations.OrderByDescending(x => x.ThumbsUp).ToList();
-                        break;
+            switch (StationSortOrder)
+            {
+                case SortOrder.DateDesc:
+                    normalStations = normalStations.OrderByDescending(x => Convert.ToInt64(x.ID)).ToList();
+                    break;
+                case SortOrder.DateAsc:
+                    normalStations = normalStations.OrderBy(x => Convert.ToInt64(x.ID)).ToList();
+                    break;
+                case SortOrder.AlphaDesc:
+                    normalStations = normalStations.OrderByDescending(x => x.Name).ToList();
+                    break;
+                case SortOrder.AlphaAsc:
+                    normalStations = normalStations.OrderBy(x => x.Name).ToList();
+                    break;
+                case SortOrder.RatingAsc:
+                    await GetStationMetaData(normalStations);
+                    normalStations = normalStations.OrderBy(x => x.ThumbsUp).ToList();
+                    break;
+                case SortOrder.RatingDesc:
+                    await GetStationMetaData(normalStations);
+                    normalStations = normalStations.OrderByDescending(x => x.ThumbsUp).ToList();
+                    break;
+            }
 
-                }
+            var orderedStations = new List<Station>(quickMixes.Count + normalStations.Count);
+            orderedStations.AddRange(quickMixes);
+            orderedStations.AddRange(normalStations);
 
-                localStations.InsertRange(0, quickMixes);
-                // Also update the public property for other parts of your app
-                this.Stations = localStations;
+            lock (_stationListLock)
+            {
+                Stations = orderedStations;
+            }
 
-            } // end of lock
-            if (StationUpdateEvent != null)
-                StationUpdateEvent(this);
-            return localStations;
+            StationUpdateEvent?.Invoke(this);
+            return orderedStations;
         }
 
         //private string getSyncKey()
@@ -677,11 +681,14 @@ private async Task<Station> CreateStation(Song song, string type)
             return station;
         }
 
-        private async Task GetStationMetaData()
+        private async Task GetStationMetaData(IEnumerable<Station> stationsToProcess)
         {
             Log.O("RetrieveStationMetaData");
 
-            foreach (var station in Stations)
+            if (stationsToProcess == null)
+                return;
+
+            foreach (var station in stationsToProcess)
             {
                 JObject req = new JObject
                 {

UnitTestProject1/BassAudioEngineTest.csproj

@@ -46,10 +46,14 @@
     <Reference Include="Microsoft.VisualStudio.TestPlatform.TestFramework.Extensions, Version=14.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
       <HintPath>..\packages\MSTest.TestFramework.2.2.10\lib\net45\Microsoft.VisualStudio.TestPlatform.TestFramework.Extensions.dll</HintPath>
     </Reference>
+    <Reference Include="Newtonsoft.Json, Version=7.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">
+      <HintPath>..\packages\Newtonsoft.Json.7.0.1\lib\net40\Newtonsoft.Json.dll</HintPath>
+    </Reference>
     <Reference Include="System" />
     <Reference Include="System.Core" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="PandoraStationRefreshTests.cs" />
     <Compile Include="UnitTest1.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
   </ItemGroup>
@@ -61,6 +65,10 @@
       <Project>{d054f1ed-6a9c-4c4d-b500-7c1af01f16c9}</Project>
       <Name>BassPlayer</Name>
     </ProjectReference>
+    <ProjectReference Include="..\Libs\PandoraSharp\PandoraSharp.csproj">
+      <Project>{64DD2B7D-9069-41FF-A4E4-732F6DF82F09}</Project>
+      <Name>PandoraSharp</Name>
+    </ProjectReference>
   </ItemGroup>
   <ItemGroup>
     <WCFMetadata Include="Connected Services\" />

UnitTestProject1/PandoraStationRefreshTests.cs

@@ -0,0 +1,187 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Newtonsoft.Json.Linq;
+using PandoraSharp;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace UnitTestProject1
+{
+    [TestClass]
+    public class PandoraStationRefreshTests
+    {
+        private sealed class StubPandora : Pandora
+        {
+            private readonly Dictionary<string, Queue<Func<JObject, Task<JSONResult>>>> _handlers;
+            private readonly object _handlerLock = new object();
+
+            public StubPandora(Dictionary<string, Queue<Func<JObject, Task<JSONResult>>>> handlers)
+            {
+                _handlers = handlers;
+            }
+
+            protected internal override Task<JSONResult> CallRPC(string method, JObject request = null, bool isAuth = false, bool useSSL = false)
+            {
+                Func<JObject, Task<JSONResult>> handler = null;
+
+                lock (_handlerLock)
+                {
+                    if (_handlers.TryGetValue(method, out var queue) && queue.Count > 0)
+                    {
+                        handler = queue.Dequeue();
+                    }
+                }
+
+                if (handler == null)
+                {
+                    throw new InvalidOperationException($"No handler registered for method {method}.");
+                }
+
+                return handler(request ?? new JObject());
+            }
+        }
+
+        private static JSONResult BuildStationListResponse(params JObject[] stations)
+        {
+            var payload = new JObject
+            {
+                ["stat"] = "ok",
+                ["result"] = new JObject
+                {
+                    ["stations"] = new JArray(stations)
+                }
+            };
+
+            return new JSONResult(payload.ToString());
+        }
+
+        private static JObject BuildStationPayload(string id, string name, bool isQuickMix = false, string[] quickMixStationIds = null)
+        {
+            var obj = new JObject
+            {
+                ["stationId"] = id,
+                ["stationToken"] = $"token-{id}",
+                ["isShared"] = false,
+                ["isQuickMix"] = isQuickMix,
+                ["stationName"] = name,
+                ["stationDetailUrl"] = "http://station"
+            };
+
+            if (isQuickMix)
+            {
+                quickMixStationIds = quickMixStationIds ?? Array.Empty<string>();
+                obj["quickMixStationIds"] = new JArray(quickMixStationIds);
+            }
+
+            return obj;
+        }
+
+        private static JSONResult BuildStationMetadataResponse(int thumbsUp, int thumbsDown)
+        {
+            var payload = new JObject
+            {
+                ["stat"] = "ok",
+                ["result"] = new JObject
+                {
+                    ["feedback"] = new JObject
+                    {
+                        ["totalThumbsUp"] = thumbsUp,
+                        ["totalThumbsDown"] = thumbsDown
+                    }
+                }
+            };
+
+            return new JSONResult(payload.ToString());
+        }
+
+        [TestMethod]
+        public async Task RefreshStationsAsync_WaitsForMetadataBeforePublishing()
+        {
+            var quickMix = BuildStationPayload("1", "Quick Mix", true, new[] { "2", "3" });
+            var betaStation = BuildStationPayload("2", "Beta Station");
+            var alphaStation = BuildStationPayload("3", "Alpha Station");
+
+            var metadataTcs = new TaskCompletionSource<JSONResult>();
+
+            var handlers = new Dictionary<string, Queue<Func<JObject, Task<JSONResult>>>>
+            {
+                ["user.getStationList"] = new Queue<Func<JObject, Task<JSONResult>>>(new[]
+                {
+                    new Func<JObject, Task<JSONResult>>(async _ =>
+                    {
+                        await Task.Delay(10).ConfigureAwait(false);
+                        return BuildStationListResponse(quickMix, betaStation, alphaStation);
+                    })
+                }),
+                ["station.getStation"] = new Queue<Func<JObject, Task<JSONResult>>>(new[]
+                {
+                    new Func<JObject, Task<JSONResult>>(_ => metadataTcs.Task),
+                    new Func<JObject, Task<JSONResult>>(_ => Task.FromResult(BuildStationMetadataResponse(1, 0)))
+                })
+            };
+
+            var pandora = new StubPandora(handlers)
+            {
+                StationSortOrder = Pandora.SortOrder.RatingDesc
+            };
+
+            var updateEvents = 0;
+            pandora.StationUpdateEvent += _ => Interlocked.Increment(ref updateEvents);
+
+            var refreshTask = pandora.RefreshStationsAsync();
+
+            await Task.Delay(50).ConfigureAwait(false);
+
+            Assert.AreEqual(0, updateEvents, "Station updates should wait for metadata fetches to complete.");
+            Assert.IsNull(pandora.Stations, "Stations must not be published before metadata resolves.");
+
+            metadataTcs.SetResult(BuildStationMetadataResponse(3, 0));
+
+            var stations = await refreshTask.ConfigureAwait(false);
+
+            Assert.AreEqual(1, updateEvents, "Station update event should fire exactly once.");
+            Assert.IsNotNull(stations, "Refresh should return a station list.");
+            Assert.AreEqual(3, stations.Count, "Quick mix and normal stations should be included.");
+            CollectionAssert.AreEqual(
+                new[] { "Quick Mix", "Beta Station", "Alpha Station" },
+                stations.Select(s => s.Name).ToArray(),
+                "Stations should keep quick mix first and sort by rating thereafter.");
+        }
+
+        [TestMethod]
+        public async Task RefreshStationsAsync_ReturnsAllStationsAfterAsyncDelay()
+        {
+            var quickMix = BuildStationPayload("10", "Quick Mix", true, new[] { "20", "30" });
+            var firstStation = BuildStationPayload("20", "First Station");
+            var secondStation = BuildStationPayload("30", "Second Station");
+
+            var handlers = new Dictionary<string, Queue<Func<JObject, Task<JSONResult>>>>
+            {
+                ["user.getStationList"] = new Queue<Func<JObject, Task<JSONResult>>>(new[]
+                {
+                    new Func<JObject, Task<JSONResult>>(async _ =>
+                    {
+                        await Task.Delay(25).ConfigureAwait(false);
+                        return BuildStationListResponse(quickMix, firstStation, secondStation);
+                    })
+                })
+            };
+
+            var pandora = new StubPandora(handlers)
+            {
+                StationSortOrder = Pandora.SortOrder.DateDesc
+            };
+
+            var stations = await pandora.RefreshStationsAsync().ConfigureAwait(false);
+
+            Assert.AreEqual(3, stations.Count, "Refresh should publish every station returned by the API.");
+            Assert.IsNotNull(pandora.Stations, "Stations property should be populated after refresh.");
+            Assert.AreEqual(3, pandora.Stations.Count, "Stations property should hold the combined list.");
+            Assert.IsTrue(stations.Any(s => s.IsQuickMix), "Quick mix entry should be present.");
+            Assert.IsTrue(stations.Any(s => !s.IsQuickMix), "Non quick-mix stations should be present.");
+            Assert.AreEqual("Quick Mix", stations.First().Name, "Quick mix station should remain at the top of the list.");
+        }
+    }
+}