ReactiveX / rxdart

The Reactive Extensions for Dart
http://reactivex.io
Apache License 2.0
3.37k stars 270 forks source link

How can I dispose the previous value in a stream of disposables #326

Closed VerTiGoEtrex closed 3 years ago

VerTiGoEtrex commented 5 years ago

Hi there,

I'm trying to write a clean Google signin scoping bloc, as best I can describe it.

Google sign in has three states - SignedIn, SigningIn, and SignedOut. Notice that the SignedIn state owns a Client that it needs to dispose.

@sealed
class GoogleSignInStatus {}
class SigningIn extends GoogleSignInStatus {}
class SignedOut extends GoogleSignInStatus {}
class SignedIn extends GoogleSignInStatus {
  final GoogleSignInAccount account;
  final PhotoslibraryApi photosLibraryApi;
  final AuthenticatedClient _client;

  SignedIn._(this.account, this._client, this.photosLibraryApi);

  factory SignedIn(final GoogleSignInAccount account) {
    final authCache = AuthCache(
      () => account.authHeaders,
      account.clearAuthCache,
    );
    final authClient = AuthenticatedClient(
        authCache.getCachedHeaders, authCache.onReportInvalidHeaders);
    return SignedIn._(account, authClient, PhotoslibraryApi(authClient));
  }

  void dispose() {
    _client.close();
  }
}

Simple enough thus far. The part I'm not sure about is how to put these states into a stream and also ensure that as new states are generated, the old ones are disposed. Here's my class, minus some trivial details to make it as simple as possible.

class GoogleSignInModel {
  final GoogleSignIn _googleSignIn;
  final BehaviorSubject<GoogleSignInStatus> _signInStatus;

  factory GoogleSignInModel() {
    final signIn = GoogleSignIn(scopes: <String>[
      'profile',
      'https://www.googleapis.com/auth/photoslibrary',
    ]);
    final signInStatusSubject =
        BehaviorSubject<GoogleSignInStatus>.seeded(SignedOut());
    return GoogleSignInModel._(signIn, signInStatusSubject);
  }

  GoogleSignInModel._(this._googleSignIn, this._signInStatusSubject) {
    _subscriptions = [_googleSignIn.onCurrentUserChanged.listen((account) {
      if (account != null) {
        _signInStatusSubject.add(SignedIn(account));
      } else {
        _signInStatusSubject.add(SignedOut());
      }
    }),
        // ???????
    ];
  }

  Stream<GoogleSignInStatus> get signInStatus => _signInStatus.stream;

  void dispose() {
    _subscription.cancel();
    _signInStatus.close();
  }

  // ...
  // methods that sign in and out of google, triggering onCurrentUserChanged
  // ...
}

The problem I have is that my BehaviorSubject _signInStatusSubject contains state object of type GoogleSignInStatus as pasted above. The SignedIn state is disposable! What happens if I go from SignedIn to the SignedOut state. As of now, that SignedIn object will leak it's internal Client object since the behavior subject will not call dispose() on its previous value.

I see a few potential solutions, but it seems they suck.

Is there an established best practice here? How would rx experts handle streams that contain values with pseudo-destructors or pseudo java-like finalize() calls?

I hope my example was clean enough to understand - if not, let me know and I will gladly see if I can simplify it further.

Thanks!

frankpepermans commented 5 years ago

Hmmm, I would suggest using the Bloc pattern instead, this will allow you to carefully manage the disposal of past events. (see https://pub.dev/packages/bloc)

When implementing a Bloc here, you can do something like this:

GoogleSignInStatus _lastStatus;

@override
  Stream<GoogleSignInStatus> mapEventToState(GoogleSignInEvent event) async* {
    // maybe dispose the last status
    _lastStatus?.dispose();
    // create a valid status given the event
    var status = processFromEvent(event);
    // assign this to be the new last event
    _lastStatus = status;
    // finally, add it
    yield status;
  }

@override
  void dispose() {
    super.dispose();
    // maybe dispose the last status
    _lastStatus?.dispose();
  }
piotrtobolski commented 5 years ago

If you want to use the provided architecture then your first solution looks to be the best. There are no destructors in dart so you can't attach any behavior to the lifetime of an object. Call to dispose is arbitrary and you have to decide yourself when it is right to call it.

You can try implementing your own version of BehaviorSubject or a wrapper that would accept only objects that have dispose method and call it when they are replaced by something else. However it may result in issues when using replay etc. because disposed objects could be passed.

hoc081098 commented 3 years ago

filter event and use switchMap