From ecf716dcab4ad9ba01910111414e72651d2afb4a Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Thu, 26 Aug 2021 16:14:29 -0700 Subject: [PATCH] navigation_and_routing: a bunch of cleanup (#886) Fail early on nullable context objects - no code paths allow null Eliminate superfluous private function Use a function over an abstract class with one method --- navigation_and_routing/lib/src/app.dart | 19 ++++++-- navigation_and_routing/lib/src/auth.dart | 46 +++++++++++++++++- navigation_and_routing/lib/src/auth/auth.dart | 48 ------------------- .../lib/src/auth/auth_guard.dart | 32 ------------- .../lib/src/routing/parser.dart | 37 +++++--------- .../lib/src/routing/route_state.dart | 4 +- .../lib/src/screens/author_details.dart | 2 +- .../lib/src/screens/authors.dart | 2 +- .../lib/src/screens/books.dart | 2 +- .../lib/src/screens/navigator.dart | 4 +- .../lib/src/screens/scaffold.dart | 2 +- .../lib/src/screens/scaffold_body.dart | 2 +- .../lib/src/screens/settings.dart | 6 +-- 13 files changed, 84 insertions(+), 122 deletions(-) delete mode 100644 navigation_and_routing/lib/src/auth/auth.dart delete mode 100644 navigation_and_routing/lib/src/auth/auth_guard.dart diff --git a/navigation_and_routing/lib/src/app.dart b/navigation_and_routing/lib/src/app.dart index 1269d8846..e59976b6f 100644 --- a/navigation_and_routing/lib/src/app.dart +++ b/navigation_and_routing/lib/src/app.dart @@ -48,8 +48,6 @@ class _BookstoreState extends State { @override void initState() { - final guard = BookstoreRouteGuard(auth: _auth); - /// Configure the parser with all of the app's allowed path templates. _routeParser = TemplateRouteParser( allowedPaths: [ @@ -62,7 +60,7 @@ class _BookstoreState extends State { '/book/:bookId', '/author/:authorId', ], - guard: guard, + guard: _guard, initialRoute: '/signin', ); @@ -97,6 +95,21 @@ class _BookstoreState extends State { ), ); + Future _guard(ParsedRoute from) async { + final signedIn = _auth.signedIn; + final signInRoute = ParsedRoute('/signin', '/signin', {}, {}); + + // Go to /signin if the user is not signed in + if (!signedIn && from != signInRoute) { + return signInRoute; + } + // Go to /books if the user is signed in and tries to go to /signin. + else if (signedIn && from == signInRoute) { + return ParsedRoute('/books/popular', '/books/popular', {}, {}); + } + return from; + } + void _handleAuthStateChanged() { if (!_auth.signedIn) { _routeState.go('/signin'); diff --git a/navigation_and_routing/lib/src/auth.dart b/navigation_and_routing/lib/src/auth.dart index aca584a6b..bc82bbeda 100644 --- a/navigation_and_routing/lib/src/auth.dart +++ b/navigation_and_routing/lib/src/auth.dart @@ -2,5 +2,47 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -export 'auth/auth.dart'; -export 'auth/auth_guard.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart'; + +/// A mock authentication service +class BookstoreAuth extends ChangeNotifier { + bool _signedIn = false; + + bool get signedIn => _signedIn; + + Future signOut() async { + await Future.delayed(const Duration(milliseconds: 200)); + // Sign out. + _signedIn = false; + notifyListeners(); + } + + Future signIn(String username, String password) async { + await Future.delayed(const Duration(milliseconds: 200)); + + // Sign in. Allow any password. + _signedIn = true; + notifyListeners(); + return _signedIn; + } + + @override + bool operator ==(Object other) => + other is BookstoreAuth && other._signedIn == _signedIn; + + @override + int get hashCode => _signedIn.hashCode; +} + +class BookstoreAuthScope extends InheritedNotifier { + const BookstoreAuthScope({ + required BookstoreAuth notifier, + required Widget child, + Key? key, + }) : super(key: key, notifier: notifier, child: child); + + static BookstoreAuth of(BuildContext context) => context + .dependOnInheritedWidgetOfExactType()! + .notifier!; +} diff --git a/navigation_and_routing/lib/src/auth/auth.dart b/navigation_and_routing/lib/src/auth/auth.dart deleted file mode 100644 index 70f73433d..000000000 --- a/navigation_and_routing/lib/src/auth/auth.dart +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2021, the Flutter project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'package:flutter/foundation.dart'; -import 'package:flutter/widgets.dart'; - -/// A mock authentication service -class BookstoreAuth extends ChangeNotifier { - bool _signedIn = false; - - bool get signedIn => _signedIn; - - Future signOut() async { - await Future.delayed(const Duration(milliseconds: 200)); - // Sign out. - _signedIn = false; - notifyListeners(); - } - - Future signIn(String username, String password) async { - await Future.delayed(const Duration(milliseconds: 200)); - - // Sign in. Allow any password. - _signedIn = true; - notifyListeners(); - return _signedIn; - } - - @override - bool operator ==(Object other) => - other is BookstoreAuth && other._signedIn == _signedIn; - - @override - int get hashCode => _signedIn.hashCode; -} - -class BookstoreAuthScope extends InheritedNotifier { - const BookstoreAuthScope({ - required BookstoreAuth notifier, - required Widget child, - Key? key, - }) : super(key: key, notifier: notifier, child: child); - - static BookstoreAuth? of(BuildContext context) => context - .dependOnInheritedWidgetOfExactType() - ?.notifier; -} diff --git a/navigation_and_routing/lib/src/auth/auth_guard.dart b/navigation_and_routing/lib/src/auth/auth_guard.dart deleted file mode 100644 index 6ecbcd470..000000000 --- a/navigation_and_routing/lib/src/auth/auth_guard.dart +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright 2021, the Flutter project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import '../routing.dart'; -import 'auth.dart'; - -/// An implementation of [RouteGuard] that redirects to /signIn -class BookstoreRouteGuard implements RouteGuard { - final BookstoreAuth auth; - - BookstoreRouteGuard({ - required this.auth, - }); - - /// Redirect to /signin if the user isn't signed in. - @override - Future redirect(ParsedRoute from) async { - final signedIn = auth.signedIn; - final signInRoute = ParsedRoute('/signin', '/signin', {}, {}); - - // Go to /signin if the user is not signed in - if (!signedIn && from != signInRoute) { - return signInRoute; - } - // Go to /books if the user is signed in and tries to go to /signin. - else if (signedIn && from == signInRoute) { - return ParsedRoute('/books/popular', '/books/popular', {}, {}); - } - return from; - } -} diff --git a/navigation_and_routing/lib/src/routing/parser.dart b/navigation_and_routing/lib/src/routing/parser.dart index 1953e3428..0c57d5dc6 100644 --- a/navigation_and_routing/lib/src/routing/parser.dart +++ b/navigation_and_routing/lib/src/routing/parser.dart @@ -8,18 +8,12 @@ import 'package:path_to_regexp/path_to_regexp.dart'; import 'parsed_route.dart'; /// Used by [TemplateRouteParser] to guard access to routes. -/// -/// Override this class to change the route that is returned by -/// [TemplateRouteParser.parseRouteInformation] if a condition is not met, for -/// example, if the user is not signed in. -abstract class RouteGuard { - Future redirect(T from); -} +typedef RouteGuard = Future Function(T from); /// Parses the URI path into a [ParsedRoute]. class TemplateRouteParser extends RouteInformationParser { - final List _pathTemplates = []; - RouteGuard? guard; + final List _pathTemplates; + final RouteGuard? guard; final ParsedRoute initialRoute; TemplateRouteParser({ @@ -27,27 +21,20 @@ class TemplateRouteParser extends RouteInformationParser { required List allowedPaths, /// The initial route - String? initialRoute = '/', + String initialRoute = '/', /// [RouteGuard] used to redirect. this.guard, - }) : initialRoute = - ParsedRoute(initialRoute ?? '/', initialRoute ?? '/', {}, {}) { - for (var template in allowedPaths) { - _addRoute(template); - } - } - - void _addRoute(String pathTemplate) { - _pathTemplates.add(pathTemplate); - } + }) : initialRoute = ParsedRoute(initialRoute, initialRoute, {}, {}), + _pathTemplates = [ + ...allowedPaths, + ], + assert(allowedPaths.contains(initialRoute)); @override Future parseRouteInformation( - RouteInformation routeInformation) async => - await _parse(routeInformation); - - Future _parse(RouteInformation routeInformation) async { + RouteInformation routeInformation, + ) async { final path = routeInformation.location!; final queryParams = Uri.parse(path).queryParameters; var parsedRoute = initialRoute; @@ -66,7 +53,7 @@ class TemplateRouteParser extends RouteInformationParser { // Redirect if a guard is present var guard = this.guard; if (guard != null) { - return guard.redirect(parsedRoute); + return guard(parsedRoute); } return parsedRoute; diff --git a/navigation_and_routing/lib/src/routing/route_state.dart b/navigation_and_routing/lib/src/routing/route_state.dart index 7c4de181e..d65d0b8e3 100644 --- a/navigation_and_routing/lib/src/routing/route_state.dart +++ b/navigation_and_routing/lib/src/routing/route_state.dart @@ -44,6 +44,6 @@ class RouteStateScope extends InheritedNotifier { Key? key, }) : super(key: key, notifier: notifier, child: child); - static RouteState? of(BuildContext context) => - context.dependOnInheritedWidgetOfExactType()?.notifier; + static RouteState of(BuildContext context) => + context.dependOnInheritedWidgetOfExactType()!.notifier!; } diff --git a/navigation_and_routing/lib/src/screens/author_details.dart b/navigation_and_routing/lib/src/screens/author_details.dart index abf411db1..3de7a4968 100644 --- a/navigation_and_routing/lib/src/screens/author_details.dart +++ b/navigation_and_routing/lib/src/screens/author_details.dart @@ -28,7 +28,7 @@ class AuthorDetailsScreen extends StatelessWidget { child: BookList( books: author.books, onTap: (book) { - RouteStateScope.of(context)!.go('/book/${book.id}'); + RouteStateScope.of(context).go('/book/${book.id}'); }, ), ), diff --git a/navigation_and_routing/lib/src/screens/authors.dart b/navigation_and_routing/lib/src/screens/authors.dart index 94fe80780..24fccb002 100644 --- a/navigation_and_routing/lib/src/screens/authors.dart +++ b/navigation_and_routing/lib/src/screens/authors.dart @@ -21,7 +21,7 @@ class AuthorsScreen extends StatelessWidget { body: AuthorList( authors: LibraryScope.of(context).allAuthors, onTap: (author) { - RouteStateScope.of(context)!.go('/author/${author.id}'); + RouteStateScope.of(context).go('/author/${author.id}'); }, ), ); diff --git a/navigation_and_routing/lib/src/screens/books.dart b/navigation_and_routing/lib/src/screens/books.dart index 4d654a7c5..c1108ba1d 100644 --- a/navigation_and_routing/lib/src/screens/books.dart +++ b/navigation_and_routing/lib/src/screens/books.dart @@ -93,7 +93,7 @@ class _BooksScreenState extends State ); } - RouteState get _routeState => RouteStateScope.of(context)!; + RouteState get _routeState => RouteStateScope.of(context); void _handleBookTapped(Book book) { _routeState.go('/book/${book.id}'); diff --git a/navigation_and_routing/lib/src/screens/navigator.dart b/navigation_and_routing/lib/src/screens/navigator.dart index 442b816da..3d91dc4a1 100644 --- a/navigation_and_routing/lib/src/screens/navigator.dart +++ b/navigation_and_routing/lib/src/screens/navigator.dart @@ -37,8 +37,8 @@ class _BookstoreNavigatorState extends State { @override Widget build(BuildContext context) { - final routeState = RouteStateScope.of(context)!; - final authState = BookstoreAuthScope.of(context)!; + final routeState = RouteStateScope.of(context); + final authState = BookstoreAuthScope.of(context); final pathTemplate = routeState.route.pathTemplate; final library = LibraryScope.of(context); diff --git a/navigation_and_routing/lib/src/screens/scaffold.dart b/navigation_and_routing/lib/src/screens/scaffold.dart index 5da034527..f13734375 100644 --- a/navigation_and_routing/lib/src/screens/scaffold.dart +++ b/navigation_and_routing/lib/src/screens/scaffold.dart @@ -15,7 +15,7 @@ class BookstoreScaffold extends StatelessWidget { @override Widget build(BuildContext context) { - final routeState = RouteStateScope.of(context)!; + final routeState = RouteStateScope.of(context); final selectedIndex = _getSelectedIndex(routeState.route.pathTemplate); return Scaffold( diff --git a/navigation_and_routing/lib/src/screens/scaffold_body.dart b/navigation_and_routing/lib/src/screens/scaffold_body.dart index b222fabf0..6b784ec0e 100644 --- a/navigation_and_routing/lib/src/screens/scaffold_body.dart +++ b/navigation_and_routing/lib/src/screens/scaffold_body.dart @@ -21,7 +21,7 @@ class BookstoreScaffoldBody extends StatelessWidget { @override Widget build(BuildContext context) { - var currentRoute = RouteStateScope.of(context)!.route; + var currentRoute = RouteStateScope.of(context).route; // A nested Router isn't necessary because the back button behavior doesn't // need to be customized. diff --git a/navigation_and_routing/lib/src/screens/settings.dart b/navigation_and_routing/lib/src/screens/settings.dart index 9927e9a46..0e7368408 100644 --- a/navigation_and_routing/lib/src/screens/settings.dart +++ b/navigation_and_routing/lib/src/screens/settings.dart @@ -5,7 +5,7 @@ import 'package:flutter/material.dart'; import 'package:url_launcher/link.dart'; -import '../auth/auth.dart'; +import '../auth.dart'; import '../routing.dart'; class SettingsScreen extends StatefulWidget { @@ -52,7 +52,7 @@ class SettingsContent extends StatelessWidget { ), ElevatedButton( onPressed: () { - BookstoreAuthScope.of(context)!.signOut(); + BookstoreAuthScope.of(context).signOut(); }, child: const Text('Sign out'), ), @@ -66,7 +66,7 @@ class SettingsContent extends StatelessWidget { TextButton( child: const Text('Go directly to /book/0 (RouteState)'), onPressed: () { - RouteStateScope.of(context)!.go('/book/0'); + RouteStateScope.of(context).go('/book/0'); }, ), ].map((w) => Padding(padding: const EdgeInsets.all(8), child: w)),