From c70aca799f2f20a0e002a5e84a60cf0b3b8eaf4c Mon Sep 17 00:00:00 2001 From: Bungeefan Date: Sat, 9 Mar 2024 20:35:34 +0100 Subject: [PATCH] [refactor] further migrate to correctly handle uris --- package/lib/src/beam_location.dart | 4 +- package/lib/src/beam_page.dart | 5 +- package/lib/src/beam_state.dart | 9 +-- package/lib/src/beamer_delegate.dart | 7 +- package/lib/src/utils.dart | 66 +++++++++---------- package/test/utils_test.dart | 95 +++++++++++++++++----------- 6 files changed, 99 insertions(+), 87 deletions(-) diff --git a/package/lib/src/beam_location.dart b/package/lib/src/beam_location.dart index 15348d0..84c8029 100644 --- a/package/lib/src/beam_location.dart +++ b/package/lib/src/beam_location.dart @@ -520,9 +520,7 @@ class RoutesBeamLocation extends BeamLocation { final matched = {}; var overrideNotFound = false; - final uri = routeInformation.uri.replace( - path: Utils.trimmed(routeInformation.uri.path), - ); + final uri = Utils.removeTrailingSlash(routeInformation.uri); for (final route in routes) { if (route is String) { diff --git a/package/lib/src/beam_page.dart b/package/lib/src/beam_page.dart index 31aa105..d7cf97c 100644 --- a/package/lib/src/beam_page.dart +++ b/package/lib/src/beam_page.dart @@ -1,9 +1,8 @@ +import 'package:beamer/beamer.dart'; import 'package:beamer/src/utils.dart'; import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; -import 'package:beamer/beamer.dart'; - /// Types for how to route should be built. /// /// See [BeamPage.type] @@ -145,7 +144,7 @@ class BeamPage extends Page { delegate.update( configuration: delegate.configuration.copyWith( - location: popUri.toString(), + uri: popUri, state: lastRouteInformation?.state, ), data: data, diff --git a/package/lib/src/beam_state.dart b/package/lib/src/beam_state.dart index 5d79780..997e016 100644 --- a/package/lib/src/beam_state.dart +++ b/package/lib/src/beam_state.dart @@ -1,9 +1,8 @@ import 'dart:convert'; -import 'package:flutter/widgets.dart'; - -import 'package:beamer/src/utils.dart'; import 'package:beamer/src/beam_location.dart'; +import 'package:beamer/src/utils.dart'; +import 'package:flutter/widgets.dart'; /// A class to mix with when defining a custom state for [BeamLocation]. /// @@ -71,10 +70,8 @@ class BeamState with RouteInformationSerializable { BeamLocation? beamLocation, Object? routeState, }) { - uriString = Utils.trimmed(uriString); - final uri = Uri.parse(uriString); return BeamState.fromUri( - uri, + Utils.removeTrailingSlash(Uri.parse(uriString)), beamLocation: beamLocation, routeState: routeState, ); diff --git a/package/lib/src/beamer_delegate.dart b/package/lib/src/beamer_delegate.dart index 95047ef..4d521bf 100644 --- a/package/lib/src/beamer_delegate.dart +++ b/package/lib/src/beamer_delegate.dart @@ -803,8 +803,9 @@ class BeamerDelegate extends RouterDelegate configuration = currentBeamLocation.state.routeInformation; } else if (uri.path == '/') { configuration = RouteInformation( - uri: Uri.parse( - initialPath + (uri.query.isNotEmpty ? '?${uri.query}' : ''), + uri: uri.replace( + path: initialPath, + query: uri.hasQuery ? uri.query : null, ), ); } @@ -815,7 +816,7 @@ class BeamerDelegate extends RouterDelegate @override SynchronousFuture setNewRoutePath(RouteInformation configuration) { if (configuration.uri.path == '/' && initialPath != '/') { - configuration = configuration.copyWith(location: initialPath); + configuration = configuration.copyWith(uri: Uri.parse(initialPath)); } update(configuration: configuration); return SynchronousFuture(null); diff --git a/package/lib/src/utils.dart b/package/lib/src/utils.dart index 3cade38..51dbc01 100644 --- a/package/lib/src/utils.dart +++ b/package/lib/src/utils.dart @@ -231,43 +231,42 @@ abstract class Utils { } } - /// Removes the trailing / in an URI String and returns the result. + /// Removes the trailing / in an URI path and returns the new URI. /// - /// If there is no trailing /, returns the input. - static String trimmed(String? uriString) { - if (uriString == null) { - return ''; + /// If there is no trailing /, returns the input URI. + static Uri removeTrailingSlash(Uri uri) { + String path = uri.path; + if (path.length > 1 && path.endsWith('/')) { + return uri.replace(path: path.substring(0, path.length - 1)); } - if (uriString.length > 1 && uriString.endsWith('/')) { - return uriString.substring(0, uriString.length - 1); + return uri; + } + + /// If [incoming] is a relative URI append it to [current]. + /// Else, return incoming URI. + static Uri maybeAppend(Uri current, Uri incoming) { + if (!incoming.hasAbsolutePath && !incoming.hasEmptyPath) { + String currentPath = + current.path.endsWith('/') ? current.path : '${current.path}/'; + return current.replace( + path: currentPath + incoming.path, + query: incoming.hasQuery ? incoming.query : null, + fragment: incoming.hasFragment ? incoming.fragment : null, + ); } - return uriString; + return incoming; } - /// If incoming RouteInformation doesn't start with slash, - /// append it to the current RouteInformation.location. + /// Merges the URIs of the RouteInformation's. /// - /// Else, return incoming RouteInformation - static RouteInformation maybeAppend( + /// See [maybeAppend] and [removeTrailingSlash]. + static RouteInformation mergeConfiguration( RouteInformation current, RouteInformation incoming, ) { - if (!incoming.uri.hasAbsolutePath && !incoming.uri.hasEmptyPath) { - String currentPath = current.uri.path.endsWith('/') - ? current.uri.path - : '${current.uri.path}/'; - return current.copyWith( - location: current.uri - .replace( - path: currentPath + incoming.uri.path, - query: incoming.uri.hasQuery ? incoming.uri.query : null, - fragment: incoming.uri.hasFragment ? incoming.uri.fragment : null, - ) - .toString(), - state: incoming.state, - ); - } - return incoming; + return incoming.copyWith( + uri: maybeAppend(current.uri, removeTrailingSlash(incoming.uri)), + ); } /// Creates a new configuration for [BeamerDelegate.update]. @@ -275,15 +274,12 @@ abstract class Utils { /// Takes into consideration trimming and potentially appending /// the incoming to current (used in relative beaming). /// - /// See [trimmed] and [maybeAppend]. + /// See [removeTrailingSlash] and [maybeAppend]. static RouteInformation createNewConfiguration( RouteInformation current, RouteInformation incoming, ) { - incoming = incoming.copyWith( - location: trimmed(incoming.uri.toString()), - ); - return maybeAppend(current, incoming); + return mergeConfiguration(current, incoming); } } @@ -291,11 +287,11 @@ abstract class Utils { extension BeamerRouteInformationExtension on RouteInformation { /// Returns a new [RouteInformation] created from this. RouteInformation copyWith({ - String? location, + Uri? uri, Object? state, }) { return RouteInformation( - uri: Uri.parse(location ?? this.uri.toString()), + uri: uri ?? this.uri, state: state ?? this.state, ); } diff --git a/package/test/utils_test.dart b/package/test/utils_test.dart index 6331d5d..c4d90d6 100644 --- a/package/test/utils_test.dart +++ b/package/test/utils_test.dart @@ -125,83 +125,104 @@ void main() { }); group('Creating new configuration for BeamerDelegate', () { - test('Trimming', () { - expect(Utils.trimmed(null), ''); - expect(Utils.trimmed('/'), '/'); - expect(Utils.trimmed('/xxx/'), '/xxx'); + test('Remove trailing slashes', () { + expect(Utils.removeTrailingSlash(Uri.parse('')), Uri.parse('')); + expect(Utils.removeTrailingSlash(Uri.parse('/')), Uri.parse('/')); + expect(Utils.removeTrailingSlash(Uri.parse('/xxx/')), Uri.parse('/xxx')); + expect( + Utils.removeTrailingSlash(Uri.parse('https://example.com/')), + Uri.parse('https://example.com/'), + ); + expect( + Utils.removeTrailingSlash(Uri.parse('https://example.com/test/')), + Uri.parse('https://example.com/test'), + ); }); - test('Appending without new routeState', () { - final current = RouteInformation(uri: Uri.parse('/current')); + test('Appending URIs to relative URI', () { + final current = Uri.parse('/current'); expect( - Utils.maybeAppend(current, RouteInformation(uri: Uri.parse('incoming'))) - .uri - .toString(), + Utils.maybeAppend(current, Uri.parse('incoming')).toString(), '/current/incoming', ); expect( - Utils.maybeAppend( - current, RouteInformation(uri: Uri.parse('/incoming'))) - .uri - .toString(), + Utils.maybeAppend(current, Uri.parse('/incoming')).toString(), '/incoming', ); expect( - Utils.maybeAppend(current, RouteInformation(uri: Uri.parse(''))) - .uri - .toString(), + Utils.maybeAppend(current, Uri.parse('')).toString(), '', ); expect( - Utils.maybeAppend(current, RouteInformation(uri: Uri.parse('/'))) - .uri - .toString(), + Utils.maybeAppend(current, Uri.parse('/')).toString(), '/', ); expect( - Utils.maybeAppend(current, - RouteInformation(uri: Uri.parse('example://app/incoming'))) - .uri - .toString(), + Utils.maybeAppend( + current, + Uri.parse('example://app/incoming'), + ).toString(), 'example://app/incoming', ); expect( - Utils.maybeAppend(current, - RouteInformation(uri: Uri.parse('example://app/incoming'))) - .uri - .toString(), - 'example://app/incoming', + Utils.maybeAppend(current, Uri.parse('//app/incoming')).toString(), + '//app/incoming', + ); + }); + + test('Appending URIs to absolute URI', () { + final current = Uri.parse('example://app/current'); + expect( + Utils.maybeAppend(current, Uri.parse('incoming')).toString(), + 'example://app/current/incoming', + ); + expect( + Utils.maybeAppend(current, Uri.parse('/incoming')).toString(), + '/incoming', + ); + expect( + Utils.maybeAppend(current, Uri.parse('')).toString(), + '', + ); + expect( + Utils.maybeAppend(current, Uri.parse('/')).toString(), + '/', ); expect( Utils.maybeAppend( - current, RouteInformation(uri: Uri.parse('//app/incoming'))) - .uri - .toString(), + current, + Uri.parse('example://app/incoming'), + ).toString(), + 'example://app/incoming', + ); + expect( + Utils.maybeAppend(current, Uri.parse('//app/incoming')).toString(), '//app/incoming', ); }); - test('Appending with new routeState', () { + test('Merging with new routeState', () { final current = RouteInformation(uri: Uri.parse('/current')); expect( - Utils.maybeAppend(current, RouteInformation(uri: Uri(), state: 42)) + Utils.mergeConfiguration( + current, RouteInformation(uri: Uri(), state: 42)) .state, 42, ); expect( - Utils.maybeAppend(current, + Utils.mergeConfiguration(current, RouteInformation(uri: Uri.parse('incoming'), state: 42)) .state, 42, ); expect( - Utils.maybeAppend(current, + Utils.mergeConfiguration(current, RouteInformation(uri: Uri.parse('/incoming'), state: 42)) .state, 42, ); expect( - Utils.maybeAppend( + Utils.mergeConfiguration( current, RouteInformation( uri: Uri.parse('example://app/incoming'), @@ -210,7 +231,7 @@ void main() { 42, ); expect( - Utils.maybeAppend( + Utils.mergeConfiguration( current, RouteInformation( uri: Uri.parse('//app/incoming'),