Merge pull request #720 from Workiva/auto-teardown-adapter-stores

CPLAT-16343 Auto-tear-down ConnectFluxAdapterStores when backing Flux stores dispose

Author: rmconsole2-wf

analysis_options.yaml

@@ -10,6 +10,7 @@ analyzer:
     - tools/analyzer_plugin/**
     - ddc_precompiled/**
   errors:
+    invalid_export_of_internal_element: warning
     unused_import: warning
     duplicate_import: warning
     missing_required_param: error

lib/over_react_flux.dart

@@ -12,6 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-export 'src/over_react_redux/over_react_flux.dart';
+export 'src/over_react_redux/over_react_flux.dart' hide ConnectFluxAdapterStoreTestingHelper;
 export 'src/over_react_redux/redux_multi_provider.dart';
 export 'src/over_react_redux/over_react_redux.dart' show ReduxProvider;

lib/src/over_react_redux/over_react_flux.dart

@@ -131,15 +131,37 @@ class ConnectFluxAdapterStore<S extends flux.Store> extends redux.Store<S> {
     });
 
     actionsForStore[store] = actions;
+
+    // This store is useless once the flux store is disposed, so for convenience,
+    // we'll tear it down for consumers.
+    //
+    // In most cases, though, from a memory management standpoint, tearing this
+    // store down shouldn't be necessary, since any components subscribed to it
+    // should have also been unmounted, leaving nothing to retain it.
+    //
+    // Use a null-aware to accommodate mock stores in unit tests that return null for `didDispose`.
+    store.didDispose?.whenComplete(teardown);
   }
 
+  bool _teardownCalled = false;
+
   @override
   Future teardown() async {
+    _teardownCalled = true;
+
     await _storeListener.cancel();
     await super.teardown();
   }
 }
 
+/// Not to be exported; only used to expose private fields for testing.
+@internal
+@visibleForTesting
+extension ConnectFluxAdapterStoreTestingHelper on ConnectFluxAdapterStore {
+  @visibleForTesting
+  bool get teardownCalled => _teardownCalled;
+}
+
 /// Adapts a Flux store to the interface of a Redux store.
 ///
 /// When using an Influx architecture, this store allows Redux, Flux, and [connectFlux]

test/over_react_redux/connect_flux_adapter_store_test.dart

@@ -0,0 +1,130 @@
+// Copyright 2021 Workiva Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import 'dart:async';
+
+import 'package:over_react/src/over_react_redux/over_react_flux.dart';
+import 'package:test/test.dart';
+import 'package:w_flux/w_flux.dart' as flux;
+
+main() {
+  // See connect_flux_integration_test.dart for more test coverage of this class
+  group('ConnectFluxAdapterStore', () {
+    group('teardown', () {
+      test('calls super, closing change subscriptions', () async {
+        final adapterStore = SimpleStore().asConnectFluxStore(null);
+        expect(() => adapterStore.onChange.listen((_) {}), returnsNormally,
+            reason: 'test setup check: the stream should be open');
+
+        await adapterStore.teardown();
+      });
+
+      test('stops listening to the flux store', () async {
+        final store = SpyStore();
+        final adapterStore = store.asConnectFluxStore(null);
+
+        expect(store.spiedSubscriptions, hasLength(1));
+        // ignore: cancel_subscriptions
+        final sub = store.spiedSubscriptions.single;
+
+        final subCalls = [];
+        sub.onData(subCalls.add);
+
+        store.trigger();
+        await pumpEventQueue();
+        expect(subCalls, hasLength(1),
+            reason: 'test setup check; subscription should have gotten event');
+
+        await adapterStore.teardown();
+        store.trigger();
+        await pumpEventQueue();
+        expect(subCalls, hasLength(1), reason: 'subscription should have been cancelled');
+      });
+
+      group('can be called multiple times without throwing', () {
+        ConnectFluxAdapterStore adapterStore;
+
+        setUp(() {
+          adapterStore = SimpleStore().asConnectFluxStore(null);
+        });
+
+        test('synchronously', () async {
+          await Future.wait([
+            adapterStore.teardown(),
+            adapterStore.teardown(),
+          ]);
+        });
+
+        test('synchronously', () async {
+          await adapterStore.teardown();
+          await adapterStore.teardown();
+        });
+      });
+    });
+
+    group('calls teardown when the flux store is disposed', () {
+      flux.Store store;
+      ConnectFluxAdapterStore adapterStore;
+
+      setUp(() {
+        store = SimpleStore();
+        adapterStore = store.asConnectFluxStore(null);
+      });
+
+      test('', () async {
+        expect(adapterStore.teardownCalled, isFalse);
+
+        await store.dispose();
+        await pumpEventQueue();
+        expect(adapterStore.teardownCalled, isTrue);
+      });
+
+      group('and doesn\'t break if teardown', () {
+        test('has already been called', () async {
+          expect(adapterStore.teardownCalled, isFalse);
+
+          await adapterStore.teardown();
+          expect(adapterStore.teardownCalled, isTrue);
+
+          await store.dispose();
+          await pumpEventQueue();
+        });
+
+        test('is called after', () async {
+          expect(adapterStore.teardownCalled, isFalse);
+
+          await store.dispose();
+          await pumpEventQueue();
+          expect(adapterStore.teardownCalled, isTrue);
+
+          await adapterStore.teardown();
+        });
+      });
+    });
+  });
+}
+
+class SimpleStore extends flux.Store {}
+
+class SpyStore extends flux.Store {
+  final spiedSubscriptions = <StreamSubscription>[];
+
+  @override
+  listen(onData, {onError, onDone, cancelOnError}) {
+    final sub =
+        super.listen(onData, onError: onError, onDone: onDone, cancelOnError: cancelOnError);
+    spiedSubscriptions.add(sub);
+    return sub;
+  }
+}

test/over_react_redux_test.dart

@@ -27,6 +27,7 @@ import 'over_react_redux/hooks/use_selector_test.dart' as use_selector_hook_test
 import 'over_react_redux/hooks/use_store_test.dart' as use_store_hook_test;
 import './over_react_redux/connect_test.dart' as connect_test;
 import './over_react_redux/connect_flux_test.dart' as connect_flux_test;
+import './over_react_redux/connect_flux_adapter_store_test.dart' as connect_flux_adapter_store_test;
 import './over_react_redux/connect_flux_integration_test.dart' as connect_flux_integration_test;
 import './over_react_redux/redux_multi_provider_test.dart' as multi_provider_test;
 import './over_react_redux/value_mutation_checker_test.dart' as value_mutation_checker_test;
@@ -39,6 +40,7 @@ void main() {
   use_store_hook_test.main();
   connect_test.main();
   connect_flux_test.main();
+  connect_flux_adapter_store_test.main();
   connect_flux_integration_test.main();
   multi_provider_test.main();
   value_mutation_checker_test.main();