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

TransformMany with Refresh #173

Closed
Liklainy opened this issue Nov 1, 2018 · 4 comments · Fixed by #174
Closed

TransformMany with Refresh #173

Liklainy opened this issue Nov 1, 2018 · 4 comments · Fixed by #174
Labels

Comments

@Liklainy
Copy link
Contributor

Liklainy commented Nov 1, 2018

Hello, I have an issue with TransformMany:

[Fact]
public void Refresh()
{
    var relations = new List<Person> { new Person("Child1", 1) };
    var parent = new PersonWithChildren("parent", 50, relations);
    _source.AddOrUpdate(parent);

    relations.Add(new Person("Child2", 2));
    _source.Refresh(parent); //_source.AddOrUpdate(parent);

    _results.Data.Count.Should().Be(2, "Should be 2 in the cache");
    _results.Data.Lookup("Child1").HasValue.Should().BeTrue();
    _results.Data.Lookup("Child2").HasValue.Should().BeTrue();
}

The test will fail.
Because TransformMany on Refresh doesn't add or remove items it only propagates Refresh.
TransformMany internally uses Transform with transformOnRefresh = false by default.

I can see that changing Relations from IEnumerable to ObservableCollection will fix it and there will be no need for explicit Refresh.

public ObservableCollection<Person> Relations { get; }
parent.Relations.Add(new Person("Child2", 2));

Are there other ways to do it?
Probably add transformOnRefresh parameter to TransformMany?

@RolandPheasant
Copy link
Collaborator

I recommend using an observable collection as the API explicitly states that the contents of the collection can change. However for completeness it would be best to add the transformOnRefresh overload.

@samirem
Copy link

samirem commented Nov 1, 2018

I stumbled upon a similar issue in my code where an AutoRefresh triggered an update in the TransformMany which then threw an ArgumentOutOfRangeException, see #172.

My PR simply makes sure that the exception is not thrown, but the desired functionality from my point of view is that the TransformMany should be reevaluated whenever a ListChangeReason.Refresh is triggered from an item in the stream. I'm new to this library, so not sure how it should be implemented. @Liklainy does your PR adress this?

@Liklainy
Copy link
Contributor Author

Liklainy commented Nov 2, 2018

@samirem, current commits in PR is only for cache.
But I was intended to fix list in the same request.

@Liklainy
Copy link
Contributor Author

Liklainy commented Nov 2, 2018

I ended up trying to fix list's behavior in the same pull request

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants