Clear the sort cache when changing the sorter

Fixes #737

Also make sure we scan the whole range of items that have potentially
been affected by an item move due to updated sorting fields.
Clean up some tests and add more tests.

src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs

@@ -492,6 +492,7 @@ void SetAndRecalculateSort(Func<T, T, int> theComparer)
 
         void RecalculateSort(List<T> list, int start, int end)
         {
+            sortedIndexCache.Clear();
             list.Sort(start, end, new LambdaComparer<T>(comparer));
         }
 
@@ -872,11 +873,9 @@ void MoveAndRecalculate(IList<T> list, int from, int to, int start, int end)
                 throw new ArgumentOutOfRangeException(nameof(start), "Start cannot be bigger than end, evaluation of the filter goes forward.");
 
             InternalMoveItem(from, to);
-            start++;
-            RecalculateFilter(list, (from < to ? from : to) + 1, start, end);
+            RecalculateFilter(list, (from < to ? from : to), start, end);
         }
 
-
         /// <summary>
         /// Go through the list of objects and adjust their "visibility" in the live list
         /// (by removing/inserting as needed). 

src/TrackingCollectionTests/TestBase.cs

@@ -74,24 +74,29 @@ protected void Add(Subject<Thing> source, Thing item)
         source.OnNext(item);
     }
 
+    /// <summary>
+    /// This will create a new Thing with CreatedAt and UpdatedAt set to 0
+    /// </summary>
+    /// <param name="id"></param>
+    /// <returns></returns>
     protected Thing GetThing(int id)
     {
-        return new Thing { Number = id };
+        return GetThing(id, 0, 0, "Run 1");
     }
 
     protected Thing GetThing(int id, int minutes)
     {
-        return new Thing { Number = id, Title = "Run 1", CreatedAt = Now + TimeSpan.FromMinutes(minutes), UpdatedAt = Now + TimeSpan.FromMinutes(minutes) };
+        return GetThing(id, minutes, minutes, "Run 1");
     }
 
     protected Thing GetThing(int id, int minutesc, int minutesu)
     {
-        return new Thing { Number = id, Title = "Run 1", CreatedAt = Now + TimeSpan.FromMinutes(minutesc), UpdatedAt = Now + TimeSpan.FromMinutes(minutesu) };
+        return GetThing(id, minutesc, minutesu, "Run 1");
     }
 
     protected Thing GetThing(int id, string title)
     {
-        return new Thing { Number = id, Title = "Run 1" };
+        return GetThing(id, 0, 0, title);
     }
 
     protected Thing GetThing(int id, int minutesc, int minutesu, string title)

src/TrackingCollectionTests/TrackingCollectionTests.cs

@@ -13,6 +13,7 @@
 using System.Reactive;
 using System.Threading.Tasks;
 using System.Reactive.Threading.Tasks;
+using GitHub;
 
 [TestFixture]
 public class TrackingTests : TestBase
@@ -1323,7 +1324,7 @@ public void SortingTestWithFilterMoves()
         ITrackingCollection<Thing> col = new TrackingCollection<Thing>(
             source,
             OrderedComparer<Thing>.OrderBy(x => x.UpdatedAt).Compare,
-            (item, position, list) => (position >= 1 && position <= 2) || (position >= 5 && position <= 7));
+            (item, position, list) => position == 1 || position == 2 || position == 5 || position == 6 || position == 7);
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;
@@ -1882,46 +1883,25 @@ public async Task MultipleSortingAndFiltering()
         var expectedTotal = 20;
         var rnd = new Random(214748364);
 
-        var titles1 = Enumerable.Range(1, expectedTotal).Select(x => ((char)('a' + x)).ToString()).ToList();
-        var dates1 = Enumerable.Range(1, expectedTotal).Select(x => Now + TimeSpan.FromMinutes(x)).ToList();
-
-        var idstack1 = new Stack<int>(Enumerable.Range(1, expectedTotal).OrderBy(rnd.Next));
-        var datestack1 = new Stack<DateTimeOffset>(dates1);
-        var titlestack1 = new Stack<string>(titles1.OrderBy(_ => rnd.Next()));
-
-        var titles2 = Enumerable.Range(1, expectedTotal).Select(x => ((char)('c' + x)).ToString()).ToList();
-        var dates2 = Enumerable.Range(1, expectedTotal).Select(x => Now + TimeSpan.FromMinutes(x)).ToList();
-        var dates2mixed = new List<DateTimeOffset>() {
-                dates2[2],  dates2[0],  dates2[1],  dates2[3],  dates2[5],
-                dates2[9],  dates2[15], dates2[6],  dates2[7],  dates2[8],
-                dates2[13], dates2[10], dates2[16], dates2[11], dates2[12],
-                dates2[14], dates2[17], dates2[18], dates2[19], dates2[4],
-        };
-        var idstack2 = new Stack<int>(Enumerable.Range(1, expectedTotal).OrderBy(rnd.Next));
-        var datestack2 = new Stack<DateTimeOffset>(dates2mixed);
-        var titlestack2 = new Stack<string>(titles2.OrderBy(_ => rnd.Next()));
-
-        var datestack3 = new Stack<DateTimeOffset>();
-        for (int i = 0; i < datestack1.Count; i++)
-            datestack3.Push(new DateTimeOffset(Math.Max(dates1[i].Ticks, dates2mixed[i].Ticks), TimeSpan.Zero));
+        var updatedAtMinutesStack = new Stack<int>(Enumerable.Range(1, expectedTotal).OrderBy(rnd.Next));
 
         var list1 = Observable.Defer(() => Enumerable.Range(1, expectedTotal)
             .OrderBy(rnd.Next)
-            .Select(x => new Thing(idstack1.Pop(), titlestack1.Pop(), datestack1.Pop()))
+            .Select(x => GetThing(x, x, x, ((char)('a' + x)).ToString()))
             .ToObservable())
             .Replay()
             .RefCount();
 
         var list2 = Observable.Defer(() => Enumerable.Range(1, expectedTotal)
             .OrderBy(rnd.Next)
-            .Select(x => new Thing(idstack2.Pop(), titlestack2.Pop(), datestack2.Pop()))
+            .Select(x => GetThing(x, x, updatedAtMinutesStack.Pop(), ((char)('c' + x)).ToString()))
             .ToObservable())
             .Replay()
             .RefCount();
 
         ITrackingCollection<Thing> col = new TrackingCollection<Thing>(
             list1.Concat(list2),
-            OrderedComparer<Thing>.OrderByDescending(x => x.CreatedAt).Compare,
+            OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare,
             (item, idx, list) => idx < 5
         );
         col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
@@ -1933,17 +1913,21 @@ public async Task MultipleSortingAndFiltering()
         CollectionAssert.AreNotEqual(list1.Select(x => x.Number).ToEnumerable(), list2.Select(x => x.Number).ToEnumerable());
 
         var sortlist = col.ToArray();
-        Array.Sort(sortlist, new LambdaComparer<Thing>(OrderedComparer<Thing>.OrderByDescending(x => x.CreatedAt).Compare));
+        Array.Sort(sortlist, new LambdaComparer<Thing>(OrderedComparer<Thing>
+            .OrderByDescending(x => x.UpdatedAt)
+            .ThenByDescending(x => x.CreatedAt).Compare));
         CollectionAssert.AreEqual(sortlist.Take(5), col);
 
         col.Comparer = OrderedComparer<Thing>.OrderBy(x => x.Number).Compare;
         sortlist = col.ToArray();
         Array.Sort(sortlist, new LambdaComparer<Thing>(OrderedComparer<Thing>.OrderBy(x => x.Number).Compare));
         CollectionAssert.AreEqual(sortlist.Take(5), col);
 
-        col.Comparer = OrderedComparer<Thing>.OrderBy(x => x.CreatedAt).Compare;
+        col.Comparer = OrderedComparer<Thing>.OrderBy(x => x.UpdatedAt).Compare;
         sortlist = col.ToArray();
-        Array.Sort(sortlist, new LambdaComparer<Thing>(OrderedComparer<Thing>.OrderBy(x => x.CreatedAt).Compare));
+        Array.Sort(sortlist, new LambdaComparer<Thing>(OrderedComparer<Thing>
+            .OrderBy(x => x.UpdatedAt)
+            .ThenBy(x => x.CreatedAt).Compare));
         CollectionAssert.AreEqual(sortlist.Take(5), col);
 
         col.Comparer = OrderedComparer<Thing>.OrderByDescending(x => x.Title).Compare;
@@ -2136,4 +2120,122 @@ public void DoesUpdateThingIfTimeIsNewer()
 
         col.Dispose();
     }
+
+
+    [Test]
+    public void ChangingSortingAndUpdatingItemsUpdatesSortCorrectly()
+    {
+        var source = new Subject<Thing>();
+
+        ITrackingCollection<Thing> col = new TrackingCollection<Thing>(
+            source);
+        col.Comparer = OrderedComparer<Thing>.OrderBy(x => x.UpdatedAt).Compare;
+        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
+        col.Filter = (item, position, list) =>
+            position == 2 || position == 3 || position == 5 || position == 7;
+        col.ProcessingDelay = TimeSpan.Zero;
+
+        var count = 0;
+        var expectedCount = 0;
+        var evt = new ManualResetEvent(false);
+
+        col.Subscribe(t =>
+        {
+            if (++count == expectedCount)
+                evt.Set();
+        }, () => { });
+
+        expectedCount = 9;
+        Enumerable.Range(0, expectedCount)
+            .Select(x => GetThing(x, x))
+            .ForEach(x => Add(source, x));
+
+        evt.WaitOne();
+        evt.Reset();
+        CollectionAssert.AreEqual(new List<Thing> {
+            GetThing(2, 2),
+            GetThing(3, 3),
+            GetThing(5, 5),
+            GetThing(7, 7),
+        }, col);
+
+        expectedCount = 10;
+        Add(source, GetThing(3, 3, 2));
+        evt.WaitOne();
+        evt.Reset();
+        CollectionAssert.AreEqual(new List<Thing> {
+            GetThing(2, 2),
+            GetThing(3, 3),
+            GetThing(5, 5),
+            GetThing(7, 7),
+        }, col);
+
+        expectedCount = 11;
+        Add(source, GetThing(3, 3, 4));
+        evt.WaitOne();
+        evt.Reset();
+        CollectionAssert.AreEqual(new List<Thing> {
+            GetThing(2, 2),
+            GetThing(3, 3, 4),
+            GetThing(5, 5),
+            GetThing(7, 7),
+        }, col);
+
+        expectedCount = 12;
+        Add(source, GetThing(3, 3, 6));
+        evt.WaitOne();
+        evt.Reset();
+        CollectionAssert.AreEqual(new List<Thing> {
+            GetThing(2, 2),
+            GetThing(4, 4),
+            GetThing(3, 3, 6),
+            GetThing(7, 7),
+        }, col);
+
+        col.Comparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
+        CollectionAssert.AreEqual(new List<Thing> {
+            GetThing(3, 3, 6),
+            GetThing(6, 6),
+            GetThing(4, 4),
+            GetThing(1, 1),
+        }, col);
+
+        expectedCount = 13;
+        Add(source, GetThing(4, 4));
+        evt.WaitOne();
+        evt.Reset();
+
+        CollectionAssert.AreEqual(new List<Thing> {
+            GetThing(3, 3, 6),
+            GetThing(6, 6),
+            GetThing(4, 4),
+            GetThing(1, 1),
+        }, col);
+
+        expectedCount = 14;
+        Add(source, GetThing(4, 4, 6));
+        evt.WaitOne();
+        evt.Reset();
+
+        CollectionAssert.AreEqual(new List<Thing> {
+            GetThing(3, 3, 6),
+            GetThing(6, 6),
+            GetThing(5, 5),
+            GetThing(1, 1),
+        }, col);
+
+        expectedCount = 15;
+        Add(source, GetThing(5, 5, 6));
+        evt.WaitOne();
+        evt.Reset();
+
+        CollectionAssert.AreEqual(new List<Thing> {
+            GetThing(3, 3, 6),
+            GetThing(6, 6),
+            GetThing(5, 5, 6),
+            GetThing(1, 1),
+        }, col);
+
+        col.Dispose();
+    }
 }