Fix Virtualise and Page exception. Fixes #540 (#667)

reactivemarbles · Nov 25, 2022 · da46c54 · da46c54
1 parent 9f7638f
commit da46c54
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 58 deletions.
diff --git a/src/DynamicData.Tests/List/PageFixture.cs b/src/DynamicData.Tests/List/PageFixture.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Reactive.Linq;
 using System.Reactive.Subjects;
@@ -166,6 +167,24 @@ public void SimplePaging()
         pager.OnNext(new PageRequest(2, 3));
         sut.Paged.Count.Should().Be(3);
     }
+
+
+    [Fact]
+    public void DoesNotThrowWithDuplicates()
+    {
+        // see https://github.com/reactivemarbles/DynamicData/issues/540
+
+        var result = new List<string>();
+
+        var source = new SourceList<string>();
+        source.AddRange(Enumerable.Repeat("item", 10));
+        source.Connect()
+            .Page(new BehaviorSubject<IPageRequest>(new PageRequest(0, 3)))
+            .Clone(result)
+            .Subscribe();
+
+        result.Count.Should().Be(1);
+    }
 }
 
 public class SimplePaging : AbstractNotifyPropertyChanged, IDisposable

diff --git a/src/DynamicData.Tests/List/VirtualisationFixture.cs b/src/DynamicData.Tests/List/VirtualisationFixture.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Reactive.Subjects;
 
@@ -128,4 +129,22 @@ public void VirtualiseInitial()
 
         _results.Data.Items.Should().BeEquivalentTo(expected);
     }
+
+
+    [Fact]
+    public void DoesNotThrowWithDuplicates()
+    {
+        // see https://github.com/reactivemarbles/DynamicData/issues/540
+
+        var result = new List<string>();
+
+        var source = new SourceList<string>();
+        source.AddRange(Enumerable.Repeat("item", 10));
+        source.Connect()
+            .Virtualise(new BehaviorSubject<IVirtualRequest>(new VirtualRequest(0, 3)))
+            .Clone(result)
+            .Subscribe();
+
+        result.Count.Should().Be(1);
+    }
 }
diff --git a/src/DynamicData/List/ChangeAwareList.cs b/src/DynamicData/List/ChangeAwareList.cs
@@ -147,7 +147,7 @@ public IChangeSet<T> CaptureChanges()
         if (_changes.Count == 0)
             return ChangeSet<T>.Empty;
 
-        ChangeSet<T> returnValue = _changes;
+        var returnValue = _changes;
 
         // we can infer this is a Clear
         if (_innerList.Count == 0 && returnValue.Removes == returnValue.TotalChanges && returnValue.TotalChanges > 1)

diff --git a/src/DynamicData/List/Internal/Pager.cs b/src/DynamicData/List/Internal/Pager.cs
@@ -2,10 +2,7 @@
 // Roland Pheasant licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for full license information.
 
-using System;
 using System.Collections;
-using System.Collections.Generic;
-using System.Linq;
 using System.Reactive.Linq;
 
 using DynamicData.Kernel;
@@ -95,7 +92,7 @@ private static PageChangeSet<T> Page(List<T> all, ChangeAwareList<T> paged, IPag
         int page = request.Page > pages ? pages : request.Page;
         int skip = request.Size * (page - 1);
 
-        var current = all.Skip(skip)
+        var current = all.Distinct().Skip(skip)
             .Take(request.Size)
             .ToList();
 
@@ -104,39 +101,27 @@ private static PageChangeSet<T> Page(List<T> all, ChangeAwareList<T> paged, IPag
 
         paged.RemoveMany(removes);
 
-        adds.ForEach(t =>
+        foreach (var add in adds)
         {
-            var index = current.IndexOf(t);
-            paged.Insert(index, t);
-        });
+            var index = current.IndexOf(add);
+            paged.Insert(index, add);
+        }
 
         var startIndex = skip;
 
-        var moves = changeSet.EmptyIfNull()
-            .Where(change => change.Reason == ListChangeReason.Moved
-                             && change.MovedWithinRange(startIndex, startIndex + request.Size));
-
-        foreach (var change in moves)
-        {
-            // check whether an item has moved within the same page
-            var currentIndex = change.Item.CurrentIndex - startIndex;
-            var previousIndex = change.Item.PreviousIndex - startIndex;
-            paged.Move(previousIndex, currentIndex);
-        }
-
-        // find replaces [Is this ever the case that it can be reached]
-        for (int i = 0; i < current.Count; i++)
+        if (changeSet is not null && changeSet.Count != 0)
         {
-            var currentItem = current[i];
-            var previousItem = previous[i];
+            var moves = changeSet
+                .Where(change => change.Reason == ListChangeReason.Moved
+                                 && change.MovedWithinRange(startIndex, startIndex + request.Size));
 
-            if (ReferenceEquals(currentItem, previousItem))
+            foreach (var change in moves)
             {
-                continue;
+                // check whether an item has moved within the same page
+                var currentIndex = change.Item.CurrentIndex - startIndex;
+                var previousIndex = change.Item.PreviousIndex - startIndex;
+                paged.Move(previousIndex, currentIndex);
             }
-
-            var index = paged.IndexOf(currentItem);
-            paged.Move(i, index);
         }
 
         var changed = paged.CaptureChanges();

diff --git a/src/DynamicData/List/Internal/Virtualiser.cs b/src/DynamicData/List/Internal/Virtualiser.cs
@@ -2,11 +2,7 @@
 // Roland Pheasant licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for full license information.
 
-using System;
-using System.Collections.Generic;
-using System.Linq;
 using System.Reactive.Linq;
-
 using DynamicData.Kernel;
 
 namespace DynamicData.List.Internal;
@@ -69,43 +65,30 @@ private static IChangeSet<T> Virtualise(IList<T> all, ChangeAwareList<T> virtual
 
         var previous = virtualised;
 
-        var current = all.Skip(request.StartIndex).Take(request.Size).ToList();
+        var current = all.Distinct().Skip(request.StartIndex).Take(request.Size).ToList();
 
         var adds = current.Except(previous);
         var removes = previous.Except(current);
 
         virtualised.RemoveMany(removes);
 
-        adds.ForEach(
-            t =>
-            {
-                var index = current.IndexOf(t);
-                virtualised.Insert(index, t);
-            });
-
-        var moves = changeSet.EmptyIfNull().Where(change => change.Reason == ListChangeReason.Moved && change.MovedWithinRange(request.StartIndex, request.StartIndex + request.Size));
-
-        foreach (var change in moves)
+        foreach (var add in adds)
         {
-            // check whether an item has moved within the same page
-            var currentIndex = change.Item.CurrentIndex - request.StartIndex;
-            var previousIndex = change.Item.PreviousIndex - request.StartIndex;
-            virtualised.Move(previousIndex, currentIndex);
+            var index = current.IndexOf(add);
+            virtualised.Insert(index, add);
         }
 
-        // find replaces [Is this ever the case that it can be reached]
-        for (var i = 0; i < current.Count; i++)
+        if (changeSet is not null && changeSet.Count != 0)
         {
-            var currentItem = current[i];
-            var previousItem = previous[i];
+            var moves = changeSet.EmptyIfNull().Where(change => change.Reason == ListChangeReason.Moved && change.MovedWithinRange(request.StartIndex, request.StartIndex + request.Size));
 
-            if (ReferenceEquals(currentItem, previousItem))
+            foreach (var change in moves)
             {
-                continue;
+                // check whether an item has moved within the same page
+                var currentIndex = change.Item.CurrentIndex - request.StartIndex;
+                var previousIndex = change.Item.PreviousIndex - request.StartIndex;
+                virtualised.Move(previousIndex, currentIndex);
             }
-
-            var index = virtualised.IndexOf(currentItem);
-            virtualised.Move(i, index);
         }
 
         return virtualised.CaptureChanges();