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

Add pause and resume API's to Loop #17

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

p4checo
Copy link
Contributor

@p4checo p4checo commented Jul 6, 2020

Motivation

On some scenarios, it's useful to "pause" the Loop so that we stop processing events for some reason (e.g. to stop a Loop backed Service). Following the same reasoning, it becomes necessary to have a "resume" mechanism so that the Loop starts processing events again.

Given Loop now starts automatically and stop is designed as a tear down mechanism to be used on dealloc and dispose all observations, some new API's are required so that we can unplug/replug feedbacks to achieve the above mentioned pause/resume behavior.

⚠️ Note: Tests missing, as I wanted to validate the approach first 🙏

Changes

  • Create new plugFeedbacks and unplugFeedbacks API's in Floodgate, which establish and dispose feedbacks observations, respectively. Floodgate now retains the feedbacks passed in on bootstrap to use them on plugFeedbacks.

  • Add pause and resume API's to LoopBoxBase.

  • Implement pause and resume API's in RootLoopBox, which unplug and replug the feedbacks on the Floodgate, respectively.

  • Implement pause and resume API's in ScopedLoopBox, which forward the calls to their root, respectively.

  • Fixed .podspec to include swift files from all folders.

Copy link
Contributor Author

@p4checo p4checo left a comment

Choose a reason for hiding this comment

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

Some points where I'm not certain about the chosen approach 😇

Loop/Floodgate.swift Outdated Show resolved Hide resolved
Loop/Floodgate.swift Outdated Show resolved Hide resolved
Loop/LoopBox.swift Outdated Show resolved Hide resolved
@andersio
Copy link
Member

andersio commented Jul 13, 2020

Having given some thoughts about this over the weekend, I think making resume() and pause() available on Loop can make it potentially confusing when it comes to semantics around scoped stores (as you briefly mentioned).

I wonder if we would venture a state-driven approach instead. Say given this state definition:

struct State {
  var messages: [Message] = []
  var isPaused: Bool = false
}

Then we can define pausable feedbacks through a feedback modifier, that instructs the feedback to listen to a particular predicate. So Loop users also get to control and program the scope and granularity of cancellation.

let loop = Loop(
  initial: State(),
  reducer: ...,
  feedbacks: [
    Loop.Feedback
        .combine(
          Self.feedback_pausable_whenLoading(),
          Self.feedback_pausable_whenX(),
          Self.feedback_pausable_whenY(),
          Self.feedback_pausable_whenZ()
        )
        .autoconnect(condition: { $0.isPaused == false })
  ]
)

(... while Loop.send remains always available.)

P.S. I have a bit of hesitation about pause as the term too, since it usually implies progress preservation which would not be the case for Loop (unless it being explicitly programmed to do so). So I chose the term autoconnect in the snippet above.

@p4checo
Copy link
Contributor Author

p4checo commented Jul 21, 2020

Sorry for the delay, I was off last week 😇

I agree that the resume() and pause() APIs can be a bit confusing for users. I went with it because on initial conversations with @RuiAAPeres he suggested that it would be the preferred direction for this. My initial approach was to simply use start() and stop() APIs, however they have been deprecated. Out of curiosity: what was the reasoning for it?

On our particular case, the need for this arose because we are using Loop to model Services on our app (e.g. responsible for performing network requests and managing a local state) which we want to start/stop. Even though we currently think of the start/stop mechanics as external to the state machine, I believe that they can (and probably should) be an integral part of it. Following this state-driven approach we keep this behavior nicely packed in the state which makes everything more explicit while also being more extensible and composable. ✨

Regarding the term, I agree that pause is not ideal and I think that autoconnect is definitely an improvement, despite personally not being fully convinced by it either 😅. Since I couldn't come up with a better one, I'll go with it for now.

On some scenarios, it's useful to "pause" the `Loop` so that we stop
processing events for some reason (e.g. to stop a `Loop` backed
Service). Following the same reasoning, it becomes necessary to have a
"resume" mechanism so that the Loop starts processing events again.

Given `Loop` now starts automatically and `stop` is designed as a tear
down mechanism to be used on dealloc and dispose all observations, some
new API's are required so that we can unplug/replug feedbacks to
achieve the above mentioned pause/resume behavior.

## Changes

- Create new `plugFeedbacks` and `unplugFeedbacks` API's in
`Floodgate`, which establish and dispose feedbacks observations,
respectively. Floodgate now retains the feedbacks passed in on
`bootstrap` to use them on `plugFeedbacks`.

- Add `pause` and `resume` API's to `LoopBoxBase`.

- Implement `pause` and `resume` API's in `RootLoopBox`, which unplug
and plug the feedbacks on the `Floodgate`, respectively.

- Implement `pause` and `resume` API's in `ScopedLoopBox`, which
forward the calls to their root, respectively.
- Implement a new `autoconnect` operator  on `Feedback` that disposes
and restores its "parent" Feedback observation according to a predicate
on `State`.
Comment on lines +456 to +466
public func autoconnect(followingChangesIn predicate: @escaping (State) -> Bool) -> Feedback {
return Feedback { state, consumer in
self.events(
state
.skipRepeats { predicate($0.0) == predicate($1.0) }
.flatMap(.latest) { predicate($0.0) ? state.filter { predicate($0.0) } : .empty },
consumer
)
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what I managed to come up with, and it appears to be working. However I am not convinced this is the best approach... 🙈

More specifically, I had to add that filter on the inner state to avoid the "disconnect-triggering" state from being processed before the inner chain is replaced.

Any ideas to make this tidier and/or more idiomatic?

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

Successfully merging this pull request may close these issues.

2 participants