From 7ed9d1b66fbb6d5850e1fecbc409655fb0d09322 Mon Sep 17 00:00:00 2001 From: Nikolas Rimikis Date: Tue, 26 Sep 2023 22:55:56 +0200 Subject: [PATCH] refactor(neon): make RequestManager more reliable and performant Signed-off-by: Nikolas Rimikis --- packages/neon/neon/lib/src/bloc/result.dart | 18 +- .../neon/lib/src/utils/request_manager.dart | 172 ++++++++++-------- 2 files changed, 114 insertions(+), 76 deletions(-) diff --git a/packages/neon/neon/lib/src/bloc/result.dart b/packages/neon/neon/lib/src/bloc/result.dart index d652fbbf..66240ccd 100644 --- a/packages/neon/neon/lib/src/bloc/result.dart +++ b/packages/neon/neon/lib/src/bloc/result.dart @@ -42,11 +42,19 @@ class Result { isCached: isCached, ); - Result asLoading() => Result( - data, - error, - isLoading: true, - isCached: isCached, + Result asLoading() => copyWith(isLoading: true); + + Result copyWith({ + final T? data, + final Object? error, + final bool? isLoading, + final bool? isCached, + }) => + Result( + data ?? this.data, + error ?? this.error, + isLoading: isLoading ?? this.isLoading, + isCached: isCached ?? this.isCached, ); bool get hasError => error != null; diff --git a/packages/neon/neon/lib/src/utils/request_manager.dart b/packages/neon/neon/lib/src/utils/request_manager.dart index ec1b96ed..bfe5ee4e 100644 --- a/packages/neon/neon/lib/src/utils/request_manager.dart +++ b/packages/neon/neon/lib/src/utils/request_manager.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert'; import 'package:flutter/foundation.dart'; @@ -13,7 +14,10 @@ import 'package:xml/xml.dart' as xml; typedef UnwrapCallback = T Function(R); typedef SerializeCallback = String Function(T); typedef DeserializeCallback = T Function(String); -typedef NextcloudApiCallback = Future Function(); +typedef NextcloudApiCallback = AsyncValueGetter; + +const maxRetries = 3; +const defaultTimeout = Duration(seconds: 30); class RequestManager { RequestManager(); @@ -34,26 +38,32 @@ class RequestManager { Cache? _cache; - Future wrapNextcloud( + Future wrapNextcloud( final String clientID, final String k, final BehaviorSubject> subject, - final NextcloudApiCallback call, - final UnwrapCallback unwrap, { + final DynamiteRawResponse rawResponse, + final UnwrapCallback> unwrap, { final bool disableTimeout = false, - final bool emitEmptyCache = false, }) async => - _wrap( + _wrap>( clientID, k, subject, - call, - unwrap, - (final data) => json.encode(serializeNextcloud(data)), - (final data) => deserializeNextcloud(json.decode(data) as Object), + () async { + await rawResponse.future; + + return rawResponse; + }, + (final rawResponse) => unwrap(rawResponse.response), + (final data) => json.encode(data), + (final data) => DynamiteRawResponse.fromJson( + json.decode(data) as Map, + serializers: rawResponse.serializers, + bodyType: rawResponse.bodyType, + headersType: rawResponse.headersType, + ), disableTimeout, - emitEmptyCache, - 0, ); Future wrapWebDav( @@ -75,7 +85,6 @@ class RequestManager { (final data) => WebDavMultistatus.fromXmlElement(xml.XmlDocument.parse(data).rootElement), disableTimeout, emitEmptyCache, - 0, ); Future _wrap( @@ -85,44 +94,44 @@ class RequestManager { final NextcloudApiCallback call, final UnwrapCallback unwrap, final SerializeCallback serialize, - final DeserializeCallback deserialize, - final bool disableTimeout, - final bool emitEmptyCache, - final int retries, - ) async { - if (subject.valueOrNull?.hasData ?? false) { - subject.add( - Result( - subject.value.data, - null, - isLoading: true, - isCached: true, - ), - ); - } else { - subject.add(Result.loading()); - } + final DeserializeCallback deserialize, [ + final bool disableTimeout = false, + final bool emitEmptyCache = false, + final int retries = 0, + ]) async { + // emit loading state with the current value if present + final value = subject.valueOrNull?.copyWith(isLoading: true) ?? Result.loading(); + subject.add(value); final key = '$clientID-$k'; - await _emitCached( - key, - subject, - unwrap, - deserialize, - emitEmptyCache, - true, - null, + unawaited( + _emitCached( + key, + subject, + unwrap, + deserialize, + emitEmptyCache, + ), ); try { - final response = await (disableTimeout ? call() : timeout(call)); - await _cache?.set(key, await compute(serialize, response)); + final response = await timeout(call, disableTimeout: disableTimeout); subject.add(Result.success(unwrap(response))); + + final serialized = serialize(response); + await _cache?.set(key, serialized); + } on TimeoutException catch (e, s) { + debugPrint('Request timed out ...'); + debugPrint(e.toString()); + debugPrintStack(stackTrace: s, maxFrames: 5); + + _emitError(e, subject); } catch (e, s) { debugPrint(e.toString()); - debugPrint(s.toString()); - if (e is DynamiteApiException && e.statusCode >= 500 && retries < 3) { + debugPrintStack(stackTrace: s, maxFrames: 5); + + if (e is DynamiteApiException && e.statusCode >= 500 && retries < maxRetries) { debugPrint('Retrying...'); await _wrap( clientID, @@ -136,58 +145,79 @@ class RequestManager { emitEmptyCache, retries + 1, ); - return; - } - if (!(await _emitCached( - key, - subject, - unwrap, - deserialize, - emitEmptyCache, - false, - e, - ))) { - subject.add(Result.error(e)); + } else { + _emitError(e, subject); } } } + /// Re emits the current result with the given [error]. + /// + /// Defaults to a [Result.error] if none has been emitted yet. + void _emitError( + final Object error, + final BehaviorSubject> subject, + ) { + final value = subject.valueOrNull?.copyWith(error: error, isLoading: false) ?? Result.error(error); + subject.add(value); + } + Future _emitCached( final String key, final BehaviorSubject> subject, final UnwrapCallback unwrap, final DeserializeCallback deserialize, final bool emitEmptyCache, - final bool loading, - final Object? error, ) async { - T? cached; - try { - if (_cache != null && await _cache!.has(key)) { - cached = unwrap(await compute(deserialize, (await _cache!.get(key))!)); + if (_cache != null && await _cache!.has(key)) { + try { + final cacheValue = await _cache!.get(key); + final cached = unwrap(deserialize(cacheValue!)); + + // If the network fetch is faster than fetching the cached value the + // subject can be closed before emitting. + if (subject.value.hasUncachedData) { + return true; + } + + subject.add( + subject.value.copyWith( + data: cached, + isCached: true, + ), + ); + + return true; + } catch (e, s) { + debugPrint(e.toString()); + debugPrintStack(stackTrace: s, maxFrames: 5); } - } catch (e, s) { - debugPrint(e.toString()); - debugPrint(s.toString()); } - if (cached != null || emitEmptyCache) { + + if (emitEmptyCache && !subject.value.hasUncachedData) { subject.add( - Result( - cached, - error, - isLoading: loading, + subject.value.copyWith( isCached: true, ), ); + return true; } + return false; } Future timeout( - final NextcloudApiCallback call, - ) => - call().timeout(const Duration(seconds: 30)); + final NextcloudApiCallback call, { + final bool disableTimeout = false, + final Duration timeout = const Duration(seconds: 30), + }) { + if (disableTimeout) { + return call(); + } + + return call().timeout(timeout); + } } @internal