State management: Migrate to bloc pattern/library (#10)

Migrate `ChangeNotifier` implementations to Cubits from the `flutter_bloc` library. Cubits were chosen over Blocs as the latter seems like it would just add the overhead of events (as compared to a direct function call) without added benefits that events might provide in other cases. Compared to the implementaion based on Provider, this version doesn't "crash" when resetting the valid T&C version. Instead, a listener (on `BlocConsumer`) detects that the T&C acceptance state has changed and triggers a refresh. Being straightforward with Bloc, Proivder doesn't solve problems like this. Unfortunately, we cannot get the listener to trigger on the initial value and therefore still need the component to be stateful (to use `initState`). The proper solution is probably to replace the initial loading of shared preferences with a more general initialization procedure which also fetches the valid T&C. Apart from this difference, the two implementations are clearly very similar. Still, I find that bloc does add some niceties like `onChange` and more structure like a repository abstraction. And it of course have many features that we haven't even touched here. In conclusion, Bloc seems to be strictly more powerful than Provider as the two solutions being very similar use basically the entire feature set of Provider but relatively little of that of Bloc.
Concordium · Nov 15, 2023 · e2f69fb · e2f69fb
1 parent 8e47cec
commit e2f69fb
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 50 deletions.
diff --git a/lib/main.dart b/lib/main.dart
@@ -8,7 +8,7 @@ import 'package:concordium_wallet/state/services.dart';
 import 'package:concordium_wallet/state/terms_and_conditions.dart';
 import 'package:concordium_wallet/theme.dart';
 import 'package:flutter/material.dart';
-import 'package:provider/provider.dart';
+import 'package:flutter_bloc/flutter_bloc.dart';
 import 'package:shared_preferences/shared_preferences.dart';
 
 void main() {
@@ -38,8 +38,8 @@ class App extends StatelessWidget {
           return const _LoadingSharedPreferences();
         }
         // Initialize services and provide them to the nested components
-        // (including the state components created in the child provider).
-        return Provider(
+        // (including the blocs created in the child provider).
+        return RepositoryProvider(
           create: (_) {
             final prefsSvc = SharedPreferencesService(prefs);
             const httpService = HttpService();
@@ -51,12 +51,12 @@ class App extends StatelessWidget {
               sharedPreferences: prefsSvc,
             )..enableNetwork(NetworkName.testnet);
           },
-          child: MultiProvider(
+          child: MultiBlocProvider(
             providers: [
-              ChangeNotifierProvider(
+              BlocProvider(
                 create: (_) => ActiveNetwork(config.availableNetworks[NetworkName.testnet]!),
               ),
-              ChangeNotifierProvider(
+              BlocProvider(
                 create: (context) {
                   final prefs = context.read<ServiceRepository>().sharedPreferences;
                   return TermsAndConditionAcceptance(prefs);

diff --git a/lib/screens/home/screen.dart b/lib/screens/home/screen.dart
@@ -4,7 +4,7 @@ import 'package:concordium_wallet/state/network.dart';
 import 'package:concordium_wallet/state/services.dart';
 import 'package:concordium_wallet/state/terms_and_conditions.dart';
 import 'package:flutter/material.dart';
-import 'package:provider/provider.dart';
+import 'package:flutter_bloc/flutter_bloc.dart';
 
 class HomeScreen extends StatefulWidget {
   const HomeScreen({super.key});
@@ -19,25 +19,36 @@ class _HomeScreenState extends State<HomeScreen> {
     super.initState();
 
     // Fetch currently valid T&C version unconditionally when initializing the widget.
-    final tacAcceptance = context.read<TermsAndConditionAcceptance>();
-    final network = context.read<ActiveNetwork>();
-    final services = context.read<ServiceRepository>().networkServices[network.active]!;
-    _updateValidTac(services.walletProxy, tacAcceptance);
+    // TODO: Use the 'tacAcceptance.state.valid.updatedAt' to determine whether to perform the refresh
+    //       (now and on other appropriate triggers like activating the app).
+    _refresh(context);
   }
 
   static Future<void> _updateValidTac(WalletProxyService walletProxy, TermsAndConditionAcceptance tacAcceptance) async {
     final tac = await walletProxy.fetchTermsAndConditions();
     tacAcceptance.validVersionUpdated(ValidTermsAndConditions.refreshedNow(termsAndConditions: tac));
   }
 
+  void _refresh(BuildContext context) {
+    final network = context.read<ActiveNetwork>().state;
+    final services = context.read<ServiceRepository>().networkServices[network.active]!;
+    final tacAcceptance = context.read<TermsAndConditionAcceptance>();
+    _updateValidTac(services.walletProxy, tacAcceptance);
+  }
+
   @override
   Widget build(BuildContext context) {
     return Scaffold(
       body: Container(
         padding: const EdgeInsets.fromLTRB(16, 64, 16, 16),
-        child: Builder(
-          builder: (context) {
-            final tacState = context.watch<TermsAndConditionAcceptance>();
+        child: BlocConsumer<TermsAndConditionAcceptance, TermsAndConditionsAcceptanceState>(
+          listenWhen: (previous, current) {
+            return current.valid == null;
+          },
+          listener: (context, state) {
+            _refresh(context);
+          },
+          builder: (context, tacState) {
             final validTac = tacState.valid;
             if (validTac == null) {
               // Show spinner if no valid T&C have been resolved yet (not as a result of actually ongoing fetch).
@@ -75,8 +86,8 @@ class _HomeScreenState extends State<HomeScreen> {
                 const SizedBox(height: 8),
                 ElevatedButton(
                   onPressed: () => context.read<TermsAndConditionAcceptance>().resetValid(),
-                  // NOTE: This breaks the app because no re-fetch is triggered - hot restart is necessary to recover.
-                  child: const Text('Reset valid T&C (and break the app)'),
+                  // NOTE: This resets the valid T&C which is picked up by the BlocConsumer's listener above to automatically trigger a re-fetch.
+                  child: const Text('Reset valid T&C'),
                 ),
                 const SizedBox(height: 8),
                 ElevatedButton(

diff --git a/lib/screens/terms_and_conditions/screen.dart b/lib/screens/terms_and_conditions/screen.dart
@@ -2,8 +2,8 @@ import 'package:concordium_wallet/screens/terms_and_conditions/widget.dart';
 import 'package:concordium_wallet/services/wallet_proxy/model.dart';
 import 'package:concordium_wallet/state/terms_and_conditions.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter_bloc/flutter_bloc.dart';
 import 'package:flutter_svg/flutter_svg.dart';
-import 'package:provider/provider.dart';
 import 'package:url_launcher/url_launcher.dart';
 
 class TermsAndConditionsScreen extends StatefulWidget {
@@ -121,11 +121,11 @@ class _TermsAndConditionsScreenState extends State<TermsAndConditionsScreen> {
 
   Function()? _onAcceptButtonPressed(BuildContext context) {
     if (isAccepted) {
-      return () async {
+      return () {
         final tac = AcceptedTermsAndConditions(
           version: widget.validTermsAndConditions.version,
         );
-        await context.read<TermsAndConditionAcceptance>().userAccepted(tac);
+        context.read<TermsAndConditionAcceptance>().userAccepted(tac);
       };
     }
     return null;

diff --git a/lib/state/network.dart b/lib/state/network.dart
@@ -1,6 +1,6 @@
 import 'package:concordium_wallet/state/config.dart';
 import 'package:concordium_wallet/services/wallet_proxy/service.dart';
-import 'package:flutter/foundation.dart';
+import 'package:flutter_bloc/flutter_bloc.dart';
 
 /// Name of a network.
 class NetworkName {
@@ -26,17 +26,20 @@ class Network {
   const Network({required this.name, required this.walletProxyConfig});
 }
 
-/// State component acting as the source of truth for what network is currently active in the app.
-class ActiveNetwork extends ChangeNotifier {
+class ActiveNetworkState {
   /// Currently active network.
   ///
   /// The network is guaranteed to be one of the "enabled" networks (as defined in [Config.availableNetworks]).
   Network active;
 
-  ActiveNetwork(this.active);
+  ActiveNetworkState(this.active);
+}
+
+/// State component acting as the source of truth for what network is currently active in the app.
+class ActiveNetwork extends Cubit<ActiveNetworkState> {
+  ActiveNetwork(Network active) : super(ActiveNetworkState(active));
 
   void setActive(Network n) {
-    active = n;
-    notifyListeners();
+    emit(ActiveNetworkState(n));
   }
 }
diff --git a/lib/state/terms_and_conditions.dart b/lib/state/terms_and_conditions.dart
@@ -1,6 +1,6 @@
 import 'package:concordium_wallet/services/shared_preferences/service.dart';
 import 'package:concordium_wallet/services/wallet_proxy/model.dart';
-import 'package:flutter/foundation.dart';
+import 'package:flutter_bloc/flutter_bloc.dart';
 
 /// Version of the Terms & Conditions accepted by the user.
 class AcceptedTermsAndConditions {
@@ -34,61 +34,59 @@ class ValidTermsAndConditions {
   }
 }
 
-/// State component of the currently accepted and valid Terms & Conditions.
-class TermsAndConditionAcceptance extends ChangeNotifier {
-  /// Service used to persist the accepted T&C version.
-  final SharedPreferencesService _prefs;
-
+class TermsAndConditionsAcceptanceState {
   /// Currently accepted T&C.
   ///
   /// The accepted version is persisted into shared preferences.
-  AcceptedTermsAndConditions? accepted;
+  final AcceptedTermsAndConditions? accepted;
 
   /// Currently valid T&C.
-  ValidTermsAndConditions? valid;
+  final ValidTermsAndConditions? valid;
 
-  TermsAndConditionAcceptance(this._prefs) {
+  const TermsAndConditionsAcceptanceState({required this.accepted, required this.valid});
+}
+
+/// State component of the currently accepted and valid Terms & Conditions.
+class TermsAndConditionAcceptance extends Cubit<TermsAndConditionsAcceptanceState> {
+  /// Service used to persist the accepted T&C version.
+  final SharedPreferencesService _prefs;
+
+  TermsAndConditionAcceptance(this._prefs) : super(const TermsAndConditionsAcceptanceState(accepted: null, valid: null)) {
     final acceptedVersion = _prefs.termsAndConditionsAcceptedVersion;
     if (acceptedVersion != null) {
-      accepted = AcceptedTermsAndConditions(version: acceptedVersion);
+      userAccepted(AcceptedTermsAndConditions(version: acceptedVersion));
     }
   }
 
   /// Update the currently accepted T&C and persist the new value.
   ///
   /// Use [resetAccepted] to revoke acceptance.
-  Future<void> userAccepted(AcceptedTermsAndConditions tac) async {
-    accepted = tac;
-    await _prefs.writeTermsAndConditionsAcceptedVersion(tac.version);
-    notifyListeners();
+  void userAccepted(AcceptedTermsAndConditions tac) {
+    emit(TermsAndConditionsAcceptanceState(accepted: tac, valid: state.valid));
   }
 
   /// Updates the currently valid T&C.
   void validVersionUpdated(ValidTermsAndConditions tac) {
-    valid = tac;
-    notifyListeners();
+    emit(TermsAndConditionsAcceptanceState(accepted: state.accepted, valid: tac));
   }
 
   /// Revokes T&C acceptance and delete it from persistence.
-  Future<void> resetAccepted() async {
-    accepted = null;
-    await _prefs.deleteTermsAndConditionsAcceptedVersion();
-    notifyListeners();
+  void resetAccepted() {
+    emit(TermsAndConditionsAcceptanceState(accepted: null, valid: state.valid));
   }
 
   /// Resets the valid T&C.
   ///
   /// This should trigger a reload and re-verification of the validity of the acceptance.
   void resetValid() {
-    valid = null;
-    notifyListeners();
+    emit(TermsAndConditionsAcceptanceState(accepted: state.accepted, valid: null));
   }
 
   /// Resets the update time of the currently valid T&C (if present).
   ///
   /// This method is not likely to have any uses besides maybe testing.
   void testResetValidTime() {
-    final valid = this.valid;
+    final valid = state.valid;
     if (valid != null) {
       validVersionUpdated(
         ValidTermsAndConditions(
@@ -98,4 +96,22 @@ class TermsAndConditionAcceptance extends ChangeNotifier {
       );
     }
   }
+
+  @override
+  void onChange(Change<TermsAndConditionsAcceptanceState> change) {
+    super.onChange(change);
+
+    // TODO: Pass success/failure status to notification service.
+    _persistAcceptedVersionIfChanged(change.nextState.accepted?.version, change.currentState.accepted?.version);
+  }
+
+  Future<void> _persistAcceptedVersionIfChanged(String? nextAcceptedVersion, String? currentAcceptedVersion) {
+    if (nextAcceptedVersion == currentAcceptedVersion) {
+      return Future.value();
+    }
+    if (nextAcceptedVersion == null) {
+      return _prefs.deleteTermsAndConditionsAcceptedVersion();
+    }
+    return _prefs.writeTermsAndConditionsAcceptedVersion(nextAcceptedVersion);
+  }
 }
diff --git a/pubspec.lock b/pubspec.lock
@@ -33,6 +33,14 @@ packages:
       url: "https://pub.dev"
     source: hosted
     version: "2.11.0"
+  bloc:
+    dependency: transitive
+    description:
+      name: bloc
+      sha256: "3820f15f502372d979121de1f6b97bfcf1630ebff8fe1d52fb2b0bfa49be5b49"
+      url: "https://pub.dev"
+    source: hosted
+    version: "8.1.2"
   boolean_selector:
     dependency: transitive
     description:
@@ -214,6 +222,14 @@ packages:
     description: flutter
     source: sdk
     version: "0.0.0"
+  flutter_bloc:
+    dependency: "direct main"
+    description:
+      name: flutter_bloc
+      sha256: e74efb89ee6945bcbce74a5b3a5a3376b088e5f21f55c263fc38cbdc6237faae
+      url: "https://pub.dev"
+    source: hosted
+    version: "8.1.3"
   flutter_lints:
     dependency: "direct dev"
     description:
@@ -457,7 +473,7 @@ packages:
     source: hosted
     version: "1.5.1"
   provider:
-    dependency: "direct main"
+    dependency: transitive
     description:
       name: provider
       sha256: cdbe7530b12ecd9eb455bdaa2fcb8d4dad22e80b8afb4798b41479d5ce26847f

diff --git a/pubspec.yaml b/pubspec.yaml
@@ -37,11 +37,11 @@ dependencies:
   cupertino_icons: ^1.0.2
   json_annotation: ^4.8.1
   build_runner: ^2.4.6
-  provider: ^6.0.5
   flutter_svg: ^2.0.7
   url_launcher: ^6.1.14
   shared_preferences: ^2.2.1
   http: ^1.1.0
+  flutter_bloc: ^8.1.3
 
 dev_dependencies:
   flutter_test: