Fix a bunch of things in the tracking collection

See https://github.com/shana/handy-things/commits/79d3baa for a list of
fixes

Author: shana

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

@@ -15,7 +15,6 @@
 using System.Reactive.Linq;
 using System.Reactive.Subjects;
 using System.Threading;
-using System.Threading.Tasks;
 
 namespace GitHub.Collections
 {
@@ -30,15 +29,16 @@ namespace GitHub.Collections
     /// </summary>
     /// <typeparam name="T"></typeparam>
     public class TrackingCollection<T> : ObservableCollection<T>, ITrackingCollection<T>, IDisposable
-        where T : class, ICopyable<T>, IComparable<T>
+        where T : class, ICopyable<T>
     {
         enum TheAction
         {
             None,
             Move,
             Add,
             Insert,
-            Remove
+            Remove,
+            Ignore
         }
 
         bool isChanging;
@@ -83,10 +83,7 @@ enum TheAction
         public TimeSpan ProcessingDelay
         {
             get { return requestedDelay; }
-            set
-            {
-                requestedDelay = value;
-            }
+            set { requestedDelay = value; }
         }
 
         bool ManualProcessing => cache.IsEmpty && originalSourceIsCompleted;
@@ -105,7 +102,7 @@ public TrackingCollection(Func<T, T, int> comparer = null, Func<T, int, IList<T>
 #endif
             this.comparer = comparer ?? Comparer<T>.Default.Compare;
             this.filter = filter;
-            this.newer = newer ?? Comparer<T>.Default.Compare;
+            this.newer = newer;
         }
 
         public TrackingCollection(IObservable<T> source,
@@ -132,8 +129,12 @@ public IObservable<T> Listen(IObservable<T> obs)
 
             Reset();
 
+            // ManualResetEvent uses the realtime clock for accurate <50ms delays
             var waitHandle = new ManualResetEventSlim();
 
+            // empty the source observable as fast as possible
+            // to the cache queue, and signal that data is available
+            // for processing
             dataPump = obs
                 .Do(data =>
                 {
@@ -147,7 +148,6 @@ public IObservable<T> Listen(IObservable<T> obs)
                 })
                 .Publish();
 
-
             // when both signalHaveData and signalNeedData produce a value, dataListener gets a value
             // this will empty the queue of items that have been cached in regular intervals according
             // to the requested delay
@@ -167,7 +167,8 @@ public IObservable<T> Listen(IObservable<T> obs)
             source = dataListener
                 .Where(data => data.Item != null)
                 .ObserveOn(scheduler)
-                .Select(data => {
+                .Select(data =>
+                {
                     data = ProcessItem(data, original);
 
                     // if we're removing an item that doesn't exist, ignore it
@@ -197,7 +198,6 @@ public IObservable<T> Listen(IObservable<T> obs)
                             signalOriginalSourceCompletion = false;
                             originalSourceCompleted.OnNext(Unit.Default);
                         }
-
                     }
                     else
                         signalNeedData.OnNext(Unit.Default);
@@ -339,16 +339,6 @@ void SetAndRecalculateFilter(Func<T, int, IList<T>, bool> newFilter)
 
         #region Source pipeline processing
 
-        ActionData CheckFilter(ActionData data)
-        {
-            var isIncluded = true;
-            if (data.TheAction == TheAction.Remove)
-                isIncluded = false;
-            else if (filter != null)
-                isIncluded = filter(data.Item, data.Position, this);
-            return new ActionData(data, isIncluded);
-        }
-
         int StartQueue()
         {
             disposables.Add(cachePump.Connect());
@@ -382,11 +372,12 @@ ActionData ProcessItem(ActionData data, List<T> list)
             if (idx >= 0)
             {
                 var old = list[idx];
-                var isNewer = newer(item, old);
-
-                // the object is "older" than the one we have, ignore it
-                if (isNewer > 0)
-                    ret = new ActionData(TheAction.None, list, item, null, idx, idx);
+                if (newer != null)
+                {
+                    // the object is "older" than the one we have, ignore it
+                    if (newer(item, old) > 0)
+                        return new ActionData(TheAction.Ignore, list, item, null, idx, idx);
+                }
 
                 var comparison = comparer(item, old);
 
@@ -466,10 +457,20 @@ ActionData SortedRemove(ActionData data)
             // unfiltered list update
             sortedIndexCache.Remove(data.Item);
             UpdateIndexCache(data.List.Count - 1, data.OldPosition, data.List, sortedIndexCache);
-            original.Remove(data.Item);
+            data.List.Remove(data.Item);
             return data;
         }
 
+        ActionData CheckFilter(ActionData data)
+        {
+            var isIncluded = true;
+            if (data.TheAction == TheAction.Remove)
+                isIncluded = false;
+            else if (filter != null)
+                isIncluded = filter(data.Item, data.Position, this);
+            return new ActionData(data, isIncluded);
+        }
+
         ActionData FilteredAdd(ActionData data)
         {
             if (data.TheAction != TheAction.Add)

src/TrackingCollectionTests/TrackingCollectionTests.cs

@@ -77,7 +77,7 @@ public void OrderByUpdatedFilter()
             Observable.Never<Thing>(),
             OrderedComparer<Thing>.OrderBy(x => x.UpdatedAt).Compare,
             (item, position, list) => true,
-            OrderedComparer<Thing>.OrderBy(x => x.UpdatedAt).Compare);
+            OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare);
         col.ProcessingDelay = TimeSpan.Zero;
 
         var list1 = new List<Thing>(Enumerable.Range(1, count).Select(i => GetThing(i, i, count - i, "Run 1")).ToList());
@@ -475,7 +475,6 @@ public void SortingTest()
         ITrackingCollection<Thing> col = new TrackingCollection<Thing>(
             source,
             OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare);
-        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;
@@ -671,7 +670,6 @@ public void SortingTestWithFilterTrue()
             source,
             OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare,
             (item, position, list) => true);
-        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;
@@ -860,7 +858,6 @@ public void SortingTestWithFilterBetween6And12()
             source,
             OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare,
             (item, position, list) => item.UpdatedAt.Minute >= 6 && item.UpdatedAt.Minute <= 12);
-        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;
@@ -1015,7 +1012,6 @@ public void SortingTestWithFilterPosition2to4()
             source,
             OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare,
             (item, position, list) => position >= 2 && position <= 4);
-        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;
@@ -1163,7 +1159,6 @@ public void SortingTestWithFilterPosition1And3to4()
             source,
             OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare,
             (item, position, list) => position == 1 || (position >= 3 && position <= 4));
-        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;
@@ -1327,7 +1322,6 @@ public void SortingTestWithFilterMoves()
             source,
             OrderedComparer<Thing>.OrderBy(x => x.UpdatedAt).Compare,
             (item, position, list) => (position >= 1 && position <= 2) || (position >= 5 && position <= 7));
-        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;
@@ -1445,7 +1439,6 @@ public void ChangingItemContentRemovesItFromFilteredList2()
             source,
             OrderedComparer<Thing>.OrderBy(x => x.CreatedAt).Compare,
             (item, position, list) => item.UpdatedAt > now + TimeSpan.FromMinutes(2) && item.UpdatedAt < now + TimeSpan.FromMinutes(8));
-        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;
@@ -1760,7 +1753,6 @@ public void Removing()
             source,
             OrderedComparer<Thing>.OrderBy(x => x.UpdatedAt).Compare,
             (item, position, list) => (position > 2 && position < 5) || (position > 6 && position < 8));
-        col.NewerComparer = OrderedComparer<Thing>.OrderByDescending(x => x.UpdatedAt).Compare;
         col.ProcessingDelay = TimeSpan.Zero;
 
         var count = 0;