Skip to content

Commit

Permalink
List filter fix (#514)
Browse files Browse the repository at this point in the history 
* Fix auto refresh / dynamic filter issue for observable list.  Fixes #393.
* Optimise joins by using ChangeAwareCache directly
* Removed some of the fundamentalist fxcop rules as I have a different opinion and I am fed up with fx cop telling me what makes code more readable.
  • Loading branch information
@RolandPheasant
RolandPheasant authored Oct 5, 2021
1 parent f41fae7 commit 7522537
Show file tree
Hide file tree
Showing 9 changed files with 511 additions and 421 deletions.
113 changes: 112 additions & 1 deletion src/DynamicData.Tests/AutoRefreshFilter.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
using System;
using System.Collections.ObjectModel;
using System.ComponentModel;

using System.Reactive.Linq;
using System.Reactive.Subjects;
using DynamicData.Binding;
using FluentAssertions;

using Xunit;
Expand Down Expand Up @@ -33,6 +36,97 @@ public void Test()
obsListDerived.Count.Should().Be(3);
obsListDerived.Items.Should().BeEquivalentTo(a0, i2, i3);
}

[Fact]
public void AutoRefreshWithObservablePredicate1()
{
var item1 = new ActivableItem
{
Activated = false
};

var items = new SourceList<ActivableItem>();
items.Add(item1);

var filterSubject = new BehaviorSubject<Func<ActivableItem, bool>>(_ => false);

var obsListDerived = items
.Connect()
.AutoRefresh(i => i.Activated)
.Filter(filterSubject)
.Do(x => Console.WriteLine("Changes" + x.TotalChanges))
.AsObservableList();

// Default filter predicate denies all items
// The binding collection should stay empty, until the predicate changes
obsListDerived.Count.Should().Be(0);

item1.Activated = true;
obsListDerived.Count.Should().Be(0);

item1.Activated = false;
obsListDerived.Count.Should().Be(0);

item1.Activated = true;
obsListDerived.Count.Should().Be(0);

// Changing predicate, all "Activated" items should added to the binding collection
filterSubject.OnNext(i => i.Activated);

obsListDerived.Count.Should().Be(1);
obsListDerived.Items.Should().BeEquivalentTo(new[] { item1 });

// Changing property value
item1.Activated = false;
obsListDerived.Count.Should().Be(0);
}

[Fact]
public void AutoRefreshWithObservablePredicate2()
{
var item1 = new ActivableItem
{
Activated = false
};

var items = new ObservableCollection<ActivableItem>();
items.Add(item1);

var filterSubject = new BehaviorSubject<Func<ActivableItem, bool>>(_ => false);

var obsListDerived = items
.ToObservableChangeSet()
.AutoRefresh(i => i.Activated)
.Filter(filterSubject)
.AsObservableList();

// Default filter predicate denies all items
// The binding collection should stay empty, until the predicate changes
obsListDerived.Count.Should().Be(0);

item1.Activated = true;
obsListDerived.Count.Should().Be(0);

item1.Activated = false;
obsListDerived.Count.Should().Be(0);

item1.Activated = true;
obsListDerived.Count.Should().Be(0);

// Changing predicate, all "Activated" items should added to the binding collection
filterSubject.OnNext(i => i.Activated);

obsListDerived.Count.Should().Be(1);
obsListDerived.Items.Should().BeEquivalentTo(new[] { item1 });

// Changing property value multiple times
item1.Activated = false;
item1.Activated = true;
item1.Activated = false;
item1.Activated = true;
obsListDerived.Count.Should().Be(1);
obsListDerived.Items.Should().BeEquivalentTo(new[] { item1 });
}
}

public class Item : INotifyPropertyChanged
Expand All @@ -59,4 +153,21 @@ public string Name
}
}
}

public class ActivableItem : INotifyPropertyChanged
{
private bool _activated;

public bool Activated
{
get => _activated;
set
{
_activated = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Activated)));
}
}

public event PropertyChangedEventHandler? PropertyChanged;
}
}
148 changes: 71 additions & 77 deletions src/DynamicData/Cache/Internal/FullJoin.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,97 +42,91 @@ public IObservable<IChangeSet<TDestination, TLeftKey>> Run()
var rightCache = _right.Synchronize(locker).ChangeKey(_rightKeySelector).AsObservableCache(false);
// joined is the final cache
var joinedCache = new LockFreeObservableCache<TDestination, TLeftKey>();
var joinedCache = new ChangeAwareCache<TDestination, TLeftKey>();
var leftLoader = leftCache.Connect().Subscribe(
changes =>
var leftLoader = leftCache.Connect().Select(changes =>
{
joinedCache.Edit(
innerCache =>
{
foreach (var change in changes.ToConcreteType())
{
var left = change.Current;
var right = rightCache.Lookup(change.Key);
foreach (var change in changes.ToConcreteType())
{
var left = change.Current;
var right = rightCache.Lookup(change.Key);
switch (change.Reason)
{
case ChangeReason.Add:
case ChangeReason.Update:
innerCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key);
break;
case ChangeReason.Remove:
if (!right.HasValue)
{
// remove from result because there is no left and no rights
innerCache.Remove(change.Key);
}
else
{
// update with no left value
innerCache.AddOrUpdate(_resultSelector(change.Key, Optional<TLeft>.None, right), change.Key);
}
break;
case ChangeReason.Refresh:
// propagate upstream
innerCache.Refresh(change.Key);
break;
}
switch (change.Reason)
{
case ChangeReason.Add:
case ChangeReason.Update:
joinedCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key);
break;
case ChangeReason.Remove:
if (!right.HasValue)
{
// remove from result because there is no left and no rights
joinedCache.Remove(change.Key);
}
else
{
// update with no left value
joinedCache.AddOrUpdate(_resultSelector(change.Key, Optional<TLeft>.None, right), change.Key);
}
});
break;
case ChangeReason.Refresh:
// propagate upstream
joinedCache.Refresh(change.Key);
break;
}
}
return joinedCache.CaptureChanges();
});
var rightLoader = rightCache.Connect().Subscribe(
changes =>
var rightLoader = rightCache.Connect().Select(changes =>
{
joinedCache.Edit(
innerCache =>
{
foreach (var change in changes.ToConcreteType())
foreach (var change in changes.ToConcreteType())
{
var right = change.Current;
var left = leftCache.Lookup(change.Key);
switch (change.Reason)
{
case ChangeReason.Add:
case ChangeReason.Update:
{
var right = change.Current;
var left = leftCache.Lookup(change.Key);
joinedCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key);
}
break;
switch (change.Reason)
case ChangeReason.Remove:
{
if (!left.HasValue)
{
case ChangeReason.Add:
case ChangeReason.Update:
{
innerCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key);
}
break;
case ChangeReason.Remove:
{
if (!left.HasValue)
{
// remove from result because there is no left and no rights
innerCache.Remove(change.Key);
}
else
{
// update with no right value
innerCache.AddOrUpdate(_resultSelector(change.Key, left, Optional<TRight>.None), change.Key);
}
}
break;
case ChangeReason.Refresh:
// propagate upstream
innerCache.Refresh(change.Key);
break;
// remove from result because there is no left and no rights
joinedCache.Remove(change.Key);
}
else
{
// update with no right value
joinedCache.AddOrUpdate(_resultSelector(change.Key, left, Optional<TRight>.None), change.Key);
}
}
});
break;
case ChangeReason.Refresh:
// propagate upstream
joinedCache.Refresh(change.Key);
break;
}
}
return joinedCache.CaptureChanges();
});
return new CompositeDisposable(joinedCache.Connect().NotEmpty().SubscribeSafe(observer), leftCache, rightCache, leftLoader, joinedCache, rightLoader);
return new CompositeDisposable(leftLoader.Merge(rightLoader).SubscribeSafe(observer), leftCache, rightCache);
});
}
}
Expand Down
Loading

0 comments on commit 7522537

Please sign in to comment.