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

Feature: TransformOnObservable Operator for SourceCache #841

Merged

Conversation

dwcullop
Copy link
Member

@dwcullop dwcullop commented Jan 29, 2024

Description

As requested by #672, this creates a new operator TransformOnObservable that will transform each item in a Cache ChangeSet into an Observable and use the latest value from that Observable as the value the corresponding key in the resulting change set.

Uses an update counter to minimize the number of changesets emitted downstream. If the source emits a changeset with 5 add events, and each of the sub-observables emits a value upon subscription, then the downstream should see a single changeset with 5 add events.

Notes

  • The Key will not be observed until the new Observable emits at least one value.
  • Subsequent values emitted from the Observable will show up as Update changes for the same Key value.
  • If the Key is removed from the source, the removal will be reflected downstream
  • If the source completes and all of the sub-observables completes, then the result observable changeset will also complete.

Bonus Changes

  • Improvements to OnItem* operators:
    • Added overloads that use an Action that will take the key as a parameter
    • Updated all to a common implementation (optimized)
    • Changed OnItemRemoved to not use the special class unless the "invokeOnUnsub" flag is set because it uses an extra Cache and lock that isn't needed

@dwcullop dwcullop self-assigned this Jan 29, 2024
// Helper to emit any pending changes when all the updates have been handled
void EmitChanges()
{
if (Interlocked.Decrement(ref updateCounter) == 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the concept of an update counter to batch results is inspired. The same concept would probably benefit several other operations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the concept of an update counter to batch results is inspired. The same concept would probably benefit several other operations.

Thank you! 😊
I used it for TransformManyAsync to get behavior similar to TransformMany after you stressed the importance of minimizing the number of ChangeSets. I was also thinking that it might be beneficial to use in other places, so I'll keep a lookout for those.

There is some nuance to using it. For example, you have to increment the counter before the Synchronize so that updates coming in on other threads are counted and the emission downstream is deferred until it can be handled.

There are also some downsides:

  • More difficult (sometimes impossible) to deterministically know how many changesets you'll get
  • Possible to never get a downstream changeset (if updates happen constantly)

@RolandPheasant
Copy link
Collaborator

Doh, I've approved but I just noticed a test is failing,.

@dwcullop
Copy link
Member Author

Doh, I've approved but I just noticed a test is failing,.

Yeah, I changed something to use Lookup which is returning an Optional<string> and they don't automatically compare to a string... Easy fix.

- Overloads that take an Action that uses the Key
- Now use common implementation
- OnItemRemoved doesn't use special operator unless necessary
Update to use new OnItemRemoved operator that gives the key to the callback
@dwcullop dwcullop merged commit 38c6a38 into reactivemarbles:main Jan 30, 2024
1 check passed
@dwcullop dwcullop deleted the feature/transform-on-observable branch January 30, 2024 20:33
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants