Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Virtualise and Page exception #667

Merged
merged 1 commit into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/DynamicData.Tests/List/PageFixture.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reactive.Linq;
using System.Reactive.Subjects;
Expand Down Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions src/DynamicData.Tests/List/VirtualisationFixture.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reactive.Subjects;

Expand Down Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/DynamicData/List/ChangeAwareList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
43 changes: 14 additions & 29 deletions src/DynamicData/List/Internal/Pager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand All @@ -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();
Expand Down
39 changes: 11 additions & 28 deletions src/DynamicData/List/Internal/Virtualiser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down