diff --git a/lib/src/base_notifier.dart b/lib/src/base_notifier.dart index e7448a1..0c58883 100644 --- a/lib/src/base_notifier.dart +++ b/lib/src/base_notifier.dart @@ -2,27 +2,55 @@ import 'dart:async'; import 'package:async/async.dart' show CancelableOperation; import 'package:freezed_annotation/freezed_annotation.dart'; -import 'package:riverpod/riverpod.dart'; -import 'package:tapped_riverpod/src/catching_executor.dart'; -import 'package:tapped_riverpod/src/result.dart'; - -/// Base class for custom Notifiers that provides: -/// - cancelable async operations -/// - standardized error handling -/// - helper for loading/success/failure Result states +import 'package:tapped_riverpod/tapped_riverpod.dart'; +import 'package:uuid/uuid.dart'; + +/// Base class for custom Riverpod [Notifier]s with built-in support for +/// cancelable async operations, standardized error handling and +/// convenient helpers for loading/success/failure result states. +/// +/// ## What this class provides +/// - Management of **cancelable async operations** via [CancelableOperation] +/// - A unified `runCatching` helper for: +/// - loading / success / failure state transitions +/// - automatic cancellation of previous runs +/// - centralized error logging +/// - Automatic cleanup of all running operations when the provider is disposed +/// +/// +/// ** 🚨🚨🚨 ** +/// Even though [onDispose] is called when the provider is invalidated, +/// the *Notifier instance itself is reused* by Riverpod. +/// This means: +/// +/// - Member variables **are not reset automatically** +/// - Any state stored in fields will persist across rebuilds +/// +/// 👉 **Avoid keeping mutable state in member variables.** +/// If you must store state in fields, ensure it is fully cleaned up in +/// [onDispose]. abstract class BaseNotifier extends Notifier { - final _catchingExecutor = CatchingExecutor(); + // region public - @visibleForTesting - Map> get operations => - _catchingExecutor.operations; + /// This can be overridden in: + /// ProviderScope( + /// overrides: BaseNotifier.errorLogger.overrideWithValue(myNewLogger), + /// ... + /// ) + static Provider get errorLogger => _errorLogger; + + Map> get operations => _activeOperations; + + // endregion + + final Map _activeOperations = {}; @mustCallSuper @override T build() { // register cleanup when provider is disposed ref.onDispose(() { - _catchingExecutor.cancelAllOperations(); + cancelAllOperations(); onDispose(); }); @@ -30,6 +58,27 @@ abstract class BaseNotifier extends Notifier { return init(); } + /// Initializes and returns the notifier state. + /// + /// Typical override: + /// ```dart + /// @override + /// MyState init() => MyState(count: 0, result: InitialResult()); + /// ``` + /// + /// This method is usually called once when the provider is first created. + /// However, ⚠️ **Riverpod may call [build] (and therefore [init]) multiple times** + /// during the lifetime of the notifier, for example when: + /// - a dependency used with [Ref.watch] changes + /// - the provider is refreshed or invalidated + /// + /// Because of this, [init] must be **idempotent** and must not contain + /// one-time initialization logic. + /// + /// It is safe to use [ref.watch] and [ref.listen] here, but be aware that + /// changes to watched providers will cause [build] and therefor also [init] to re-run. + /// + /// See also the documentation of [Notifier.build]. @protected T init(); @@ -43,36 +92,109 @@ abstract class BaseNotifier extends Notifier { required void Function(Result result) setState, void Function()? onCancel, }) async { - return _catchingExecutor.execute( - identifier: identifier, - task: call, - setState: (result) { - if (!ref.mounted) return; - - setState(result); - }, - onCancel: onCancel, - ); + void setStateWhenMounted(Result result) { + if (!ref.mounted) return; + + setState(result); + } + + final errorLogger = ref.read(BaseNotifier.errorLogger); + + // cancel existing operation with same identifier + unawaited(_activeOperations[identifier]?.cancel()); + + // notify loading + setStateWhenMounted(ResultLoading()); + + R? result; + + try { + final operation = _createOperation( + call(), + onCancel: onCancel, + identifier: identifier, + ); + + // We have this small extra variable, because if directly do: + // result = await call(); + // and we force-unwrap the result (since call returns a NOT null value) + // a null-pointer exception will be thrown in the case that the generic type is "void". + // here is a small description: https://medium.com/flutter-community/the-curious-case-of-void-in-dart-f0535705e529 + final callResult = await operation.value; + + result = callResult; + + setStateWhenMounted(ResultSuccess(callResult)); + } catch (error, stacktrace) { + final displayableError = DisplayableError( + exception: error, + stackTrace: stacktrace, + ); + + errorLogger.logError(displayableError, runtimeType, identifier); + + setStateWhenMounted(ResultFailure(displayableError)); + } + + return result; } - void cancelRunCatchingBy({required String identifier}) { - _catchingExecutor.cancelOperationBy(identifier: identifier); + /// Cancel and clean up **all** running operations. + /// ⚠️ This need to handled by the creator of the instance + Future cancelAllOperations() async { + final list = List.from(_activeOperations.values); + + _activeOperations.clear(); + + for (final op in list) { + await op.cancel(); + } + } + + void cancelOperationBy({required String identifier}) { + _activeOperations.remove(identifier)?.cancel(); } /// Create a [CancelableOperation] that will automatically canceled when [cancelAllOperations] is called. /// [O] represents the generic type of the operation. - @protected CancelableOperation createOperation( Future result, { FutureOr Function()? onCancel, String? overrideIdentifier, }) { - return _catchingExecutor.createOperation( + final identifier = overrideIdentifier ?? const Uuid().v1(); + + return _createOperation(result, identifier: identifier, onCancel: onCancel); + } + + void onDispose() {} + + CancelableOperation _createOperation( + Future result, { + required String identifier, + FutureOr Function()? onCancel, + }) { + final operation = CancelableOperation.fromFuture( result, - overrideIdentifier: overrideIdentifier, - onCancel: onCancel, + onCancel: () => onCancel?.call(), ); + + operation.then( + (_) => _activeOperations.remove(identifier), + onCancel: () => _activeOperations.remove(identifier), + onError: (_, _) => _activeOperations.remove(identifier), + ); + + _activeOperations[identifier] = operation; + return operation; } +} - void onDispose() {} +final _errorLogger = Provider((ref) { + return _DummyOperationErrorLogger(); +}); + +class _DummyOperationErrorLogger implements OperationErrorLogger { + @override + void logError(DisplayableError error, Type providerType, String identifier) {} } diff --git a/lib/src/catching_executor.dart b/lib/src/catching_executor.dart deleted file mode 100644 index a472495..0000000 --- a/lib/src/catching_executor.dart +++ /dev/null @@ -1,118 +0,0 @@ -import 'dart:async'; -import 'package:async/async.dart' show CancelableOperation; -import 'package:tapped_riverpod/tapped_riverpod.dart'; -import 'package:uuid/uuid.dart'; - -/// A standalone utility for executing async tasks with automatic -/// error capturing, cancellation support and lifecycle hooks. -/// -/// Designed to replace runCatching inside Notifiers/Services without -/// depending on Riverpod or any state management. -/// -/// - supports CancelableOperation -/// - provides callbacks for loading/success/error/cancel -/// - reusable in UI, Services, Repositories, Controllers -class CatchingExecutor { - final Map _activeOperations = {}; - - Map> get operations => _activeOperations; - - void cancelOperationBy({required String identifier}) { - _activeOperations.remove(identifier)?.cancel(); - } - - /// Cancel and clean up **all** running operations. - /// ⚠️ This need to handled by the creator of the instance - Future cancelAllOperations() async { - final list = List.from(_activeOperations.values); - - _activeOperations.clear(); - - for (final op in list) { - await op.cancel(); - } - } - - /// Execute an async operation with catching behavior. - /// - /// Returns the result or `null` if failed or canceled. - /// ⚠️ Checking for mounted in [setState] need to happen in the implementation. - Future execute({ - required String identifier, - required Future Function() task, - required void Function(Result result) setState, - void Function()? onCancel, - }) async { - // cancel existing operation with same identifier - unawaited(_activeOperations[identifier]?.cancel()); - - // notify loading - setState(ResultLoading()); - - R? result; - - try { - final operation = _createOperation( - task(), - onCancel: onCancel, - identifier: identifier, - ); - - // We have this small extra variable, because if directly do: - // result = await call(); - // and we force-unwrap the result (since call returns a NOT null value) - // a null-pointer exception will be thrown in the case that the generic type is "void". - // here is a small description: https://medium.com/flutter-community/the-curious-case-of-void-in-dart-f0535705e529 - final callResult = await operation.value; - - result = callResult; - - setState(ResultSuccess(callResult)); - } catch (error, stacktrace) { - final displayableError = DisplayableError( - exception: error, - stackTrace: stacktrace, - ); - - setState(ResultFailure(displayableError)); - } - - return result; - } - - /// Create a [CancelableOperation] that will automatically canceled when [cancelAllOperations] is called. - /// [O] represents the generic type of the operation. - CancelableOperation createOperation( - Future result, { - FutureOr Function()? onCancel, - String? overrideIdentifier, - }) { - final identifier = overrideIdentifier ?? const Uuid().v1(); - - return _createOperation(result, identifier: identifier, onCancel: onCancel); - } - - // region helper - - CancelableOperation _createOperation( - Future result, { - required String identifier, - FutureOr Function()? onCancel, - }) { - final operation = CancelableOperation.fromFuture( - result, - onCancel: () => onCancel?.call(), - ); - - operation.then( - (_) => _activeOperations.remove(identifier), - onCancel: () => _activeOperations.remove(identifier), - onError: (_, _) => _activeOperations.remove(identifier), - ); - - _activeOperations[identifier] = operation; - return operation; - } - - // endregion -} diff --git a/lib/src/operation_error_logger.dart b/lib/src/operation_error_logger.dart new file mode 100644 index 0000000..d2b93f0 --- /dev/null +++ b/lib/src/operation_error_logger.dart @@ -0,0 +1,5 @@ +import 'package:tapped_riverpod/src/error/displayble_error.dart'; + +abstract class OperationErrorLogger { + void logError(DisplayableError error, Type providerType, String identifier); +} diff --git a/lib/src/result.dart b/lib/src/result.dart index 04190bd..3b66ce6 100644 --- a/lib/src/result.dart +++ b/lib/src/result.dart @@ -27,15 +27,19 @@ sealed class Result with _$Result { bool get isDone => isSuccess || isFailure; ResultFailure? asFailureOrNull() { - if (isFailure) { - return this as ResultFailure; + final result = this; + + if (result is ResultFailure) { + return result; } return null; } ResultSuccess? asSuccessOrNull() { - if (isSuccess) { - return this as ResultSuccess; + final result = this; + + if (result is ResultSuccess) { + return result; } return null; } diff --git a/lib/tapped_riverpod.dart b/lib/tapped_riverpod.dart index 3e6e055..74f1b03 100644 --- a/lib/tapped_riverpod.dart +++ b/lib/tapped_riverpod.dart @@ -5,7 +5,7 @@ export 'src/error/displayble_error.dart'; export 'src/base_notifier.dart'; export 'src/result_filter_not_null_notifier.dart'; export 'src/result.dart'; -export 'src/catching_executor.dart'; +export 'src/operation_error_logger.dart'; // endregion diff --git a/test/base_notifier_test.dart b/test/base_notifier_test.dart index fa94186..c94ccfa 100644 --- a/test/base_notifier_test.dart +++ b/test/base_notifier_test.dart @@ -1,12 +1,65 @@ import 'dart:async' show Completer, unawaited; import 'package:async/async.dart' show CancelableOperation; -import 'package:riverpod/riverpod.dart'; import 'package:tapped_riverpod/tapped_riverpod.dart'; import 'package:test/test.dart'; void main() { - test("dispose should cancel all active operations", () async { + test("invalidate/dispose should cancel all active operations", () async { + final container = ProviderContainer(); + + final notifier = container.read(_testNotifierProvider.notifier); + + notifier.doAsyncOperation(); + + expect(notifier.operations.values.single.isCanceled, false); + + container.invalidate(_testNotifierProvider); + + await Future.delayed(Duration.zero); + + expect(notifier.operations.isEmpty, true); + }); + + test( + "a canceled runCatching should never call setState or onSuccess", + () async { + final container = ProviderContainer(); + + Result actualResult = const ResultInitial(); + + final notifier = container.read(_testNotifierProvider.notifier); + + unawaited( + notifier.runCatching( + () async { + await Future.delayed(const Duration(seconds: 2)); + + return 1; + }, + identifier: "test", + setState: (result) { + if (result.isSuccess) { + throw Exception("Should not be called "); + } + + actualResult = result; + }, + ), + ); + + await Future.delayed(const Duration(milliseconds: 50)); + + notifier.cancelOperationBy(identifier: "test"); + + await Future.delayed(const Duration(milliseconds: 50)); + + expect(actualResult, const ResultLoading()); + expect(notifier.operations, >{}); + }, + ); + + test("invalidate/dispose should cancel all active operations", () async { final container = ProviderContainer(); final notifier = container.read(_testNotifierProvider.notifier); @@ -74,41 +127,72 @@ void main() { }); test( - "a canceled runCatching should never call setState or onSuccess", + "OperationErrorLogger.logError should be called when error occur", () async { - final container = ProviderContainer(); - - Result actualResult = const ResultInitial(); + String? emittedError; + Type? notifierType; + String? requestId; + + final container = ProviderContainer( + overrides: [ + BaseNotifier.errorLogger.overrideWithValue( + _CallbackErrorLogger( + onLog: (err, type, id) { + emittedError = err.exception.toString(); + notifierType = type; + requestId = id; + }, + ), + ), + ], + ); final notifier = container.read(_testNotifierProvider.notifier); - unawaited( - notifier.runCatching( - () async { - print("run catching"); - await Future.delayed(const Duration(seconds: 2)); + await notifier.runCatching( + () async { + await Future.delayed(const Duration(milliseconds: 23)); - return 1; - }, - identifier: "test", - setState: (result) { - if (result.isSuccess) { - throw Exception("Should not be called "); - } + throw Exception("Expected error"); + }, + setState: (result) {}, + identifier: "my-task", + ); - actualResult = result; - }, - ), + expect(emittedError, isNotEmpty); + expect(notifierType, _BaseTestNotifier().runtimeType); + expect(requestId, "my-task"); + }, + ); + + test( + "OperationErrorLogger.logError should not be called when no error occur", + () async { + String? emittedError; + + final container = ProviderContainer( + overrides: [ + BaseNotifier.errorLogger.overrideWithValue( + _CallbackErrorLogger( + onLog: (err, _, _) => emittedError = err.exception.toString(), + ), + ), + ], ); - await Future.delayed(const Duration(milliseconds: 50)); + final notifier = container.read(_testNotifierProvider.notifier); - notifier.cancelRunCatchingBy(identifier: "test"); + await notifier.runCatching( + () async { + await Future.delayed(const Duration(milliseconds: 23)); - await Future.delayed(const Duration(milliseconds: 50)); + return 0; + }, + setState: (result) {}, + identifier: "my-task", + ); - expect(actualResult, const ResultLoading()); - expect(notifier.operations, >{}); + expect(emittedError, isNull); }, ); } @@ -127,3 +211,19 @@ class _BaseTestNotifier extends BaseNotifier { asyncOperation = createOperation(Completer().future); } } + +class _CallbackErrorLogger extends OperationErrorLogger { + final void Function( + DisplayableError error, + Type providerType, + String identifier, + ) + onLog; + + _CallbackErrorLogger({required this.onLog}); + + @override + void logError(DisplayableError error, Type providerType, String identifier) { + onLog(error, providerType, identifier); + } +} diff --git a/test/lifecycle_base_notifier_test.dart b/test/lifecycle_base_notifier_test.dart new file mode 100644 index 0000000..f984f91 --- /dev/null +++ b/test/lifecycle_base_notifier_test.dart @@ -0,0 +1,79 @@ +import 'package:tapped_riverpod/tapped_riverpod.dart'; +import 'package:test/test.dart'; + +final lifeCycle = []; + +void main() { + setUp(() => lifeCycle.clear()); + + test("the lifecycle of the provider should be correct", () async { + final container = ProviderContainer(); + + expect(lifeCycle, []); + + final notifier = container.read(_testNotifierProvider.notifier); + + expect(lifeCycle, ["build", "init"]); + + await notifier.runCatching( + () async { + await Future.delayed(const Duration(milliseconds: 10)); + + return 1; + }, + identifier: "test", + setState: (result) {}, + ); + + expect(lifeCycle, ["build", "init"]); + + container.invalidate(_testNotifierProvider); + + expect(lifeCycle, ["build", "init", "onDispose"]); + + // should recreate the provider + container.read(_testNotifierProvider.notifier); + + expect(lifeCycle, ["build", "init", "onDispose", "build", "init"]); + + container.refresh(_testNotifierProvider); + + expect(lifeCycle, [ + "build", + "init", + "onDispose", + "build", + "init", + 'onDispose', + 'build', + 'init', + ]); + }); +} + +final _testNotifierProvider = NotifierProvider<_BaseTestNotifier, String>( + () => _BaseTestNotifier(), +); + +class _BaseTestNotifier extends BaseNotifier { + @override + String build() { + lifeCycle.add("build"); + + return super.build(); + } + + @override + String init() { + lifeCycle.add("init"); + + return ""; + } + + @override + void onDispose() { + lifeCycle.add("onDispose"); + + super.onDispose(); + } +}