From 56a2721db24172b676f308c465db47caa10c722d Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sat, 30 Dec 2023 22:41:24 +0100 Subject: [PATCH] General backend-worker comms improvements Change imports/exports --- lib/flutter_map_tile_caching.dart | 6 +- lib/fmtc_module_api.dart | 36 ----- lib/src/backend/export_plus.dart | 4 - lib/src/backend/export_std.dart | 2 - lib/src/backend/exports.dart | 5 + lib/src/backend/impls/objectbox/backend.dart | 51 ++++-- .../impls/objectbox/models/models.dart | 2 - lib/src/backend/impls/objectbox/worker.dart | 1 + lib/src/backend/interfaces/backend.dart | 2 - lib/src/backend/interfaces/models.dart | 2 - .../{impl_tools => interfaces}/no_sync.dart | 6 +- .../backend/{impl_tools => utils}/errors.dart | 0 lib/src/errors/store_not_ready.dart | 39 ----- lib/src/store/manage.dart | 149 ++++-------------- 14 files changed, 81 insertions(+), 224 deletions(-) delete mode 100644 lib/fmtc_module_api.dart delete mode 100644 lib/src/backend/export_plus.dart delete mode 100644 lib/src/backend/export_std.dart create mode 100644 lib/src/backend/exports.dart rename lib/src/backend/{impl_tools => interfaces}/no_sync.dart (97%) rename lib/src/backend/{impl_tools => utils}/errors.dart (100%) delete mode 100644 lib/src/errors/store_not_ready.dart diff --git a/lib/flutter_map_tile_caching.dart b/lib/flutter_map_tile_caching.dart index 0f32c267..94d7fe2b 100644 --- a/lib/flutter_map_tile_caching.dart +++ b/lib/flutter_map_tile_caching.dart @@ -33,7 +33,7 @@ import 'package:path_provider/path_provider.dart'; import 'package:stream_transform/stream_transform.dart'; import 'package:watcher/watcher.dart'; -import 'src/backend/export_std.dart'; +import 'src/backend/exports.dart'; import 'src/bulk_download/instance.dart'; import 'src/bulk_download/rate_limited_stream.dart'; import 'src/bulk_download/tile_loops/shared.dart'; @@ -45,18 +45,16 @@ import 'src/db/registry.dart'; import 'src/db/tools.dart'; import 'src/errors/browsing.dart'; import 'src/errors/initialisation.dart'; -import 'src/errors/store_not_ready.dart'; import 'src/misc/exts.dart'; import 'src/misc/int_extremes.dart'; import 'src/misc/obscure_query_params.dart'; import 'src/misc/typedefs.dart'; import 'src/providers/image_provider.dart'; -export 'src/backend/export_std.dart'; +export 'src/backend/exports.dart'; export 'src/errors/browsing.dart'; export 'src/errors/damaged_store.dart'; export 'src/errors/initialisation.dart'; -export 'src/errors/store_not_ready.dart'; export 'src/misc/typedefs.dart'; part 'src/bulk_download/download_progress.dart'; diff --git a/lib/fmtc_module_api.dart b/lib/fmtc_module_api.dart deleted file mode 100644 index dfbf0ae6..00000000 --- a/lib/fmtc_module_api.dart +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright © Luka S (JaffaKetchup) under GPL-v3 -// A full license can be found at .\LICENSE - -// ignore_for_file: invalid_export_of_internal_element - -/// Restricted API which exports internal functionality, necessary for the FMTC -/// modules to work correctly -/// -/// When importing this library, also import 'flutter_map_tile_caching.dart' for -/// the full functionality set. -/// -/// --- -/// -/// "With great power comes great responsibility" - Someone -/// -/// This library forms part of a layer of abstraction between you, FMTC -/// internals, and underlying databases. Importing this library removes that -/// abstraction, making it easy to disrupt FMTC's normal operations with -/// incorrect usage. For example, it is possible to force close an open Isar -/// database, leading to an erroneous & invalid state. -/// -/// If you are using this to create a custom module, go ahead! Please do get in -/// touch, I'm always interested to hear what the community is making, and I may -/// be able to offer some insight into the darker corners and workings of FMTC. -/// Note that not necessarily all internal APIs are exposed through this library. -/// -/// **Do not use in normal applications. I may be unable to offer support.** -library fmtc_module_api; - -export 'src/backend/export_plus.dart'; -export 'src/db/defs/metadata.dart'; -export 'src/db/defs/store_descriptor.dart'; -export 'src/db/defs/tile.dart'; -export 'src/db/registry.dart'; -export 'src/db/tools.dart'; -export 'src/misc/exts.dart'; diff --git a/lib/src/backend/export_plus.dart b/lib/src/backend/export_plus.dart deleted file mode 100644 index 07a1d7f8..00000000 --- a/lib/src/backend/export_plus.dart +++ /dev/null @@ -1,4 +0,0 @@ -export 'export_std.dart'; -export 'impl_tools/errors.dart'; -export 'impl_tools/no_sync.dart'; -export 'interfaces/models.dart'; diff --git a/lib/src/backend/export_std.dart b/lib/src/backend/export_std.dart deleted file mode 100644 index 22aca207..00000000 --- a/lib/src/backend/export_std.dart +++ /dev/null @@ -1,2 +0,0 @@ -export 'impls/objectbox/backend.dart'; -export 'interfaces/backend.dart'; diff --git a/lib/src/backend/exports.dart b/lib/src/backend/exports.dart new file mode 100644 index 00000000..f5f38c1a --- /dev/null +++ b/lib/src/backend/exports.dart @@ -0,0 +1,5 @@ +export 'impls/objectbox/backend.dart'; +export 'interfaces/backend.dart'; +export 'interfaces/models.dart'; +export 'interfaces/no_sync.dart'; +export 'utils/errors.dart'; diff --git a/lib/src/backend/impls/objectbox/backend.dart b/lib/src/backend/impls/objectbox/backend.dart index 1941868c..f68500e4 100644 --- a/lib/src/backend/impls/objectbox/backend.dart +++ b/lib/src/backend/impls/objectbox/backend.dart @@ -8,9 +8,9 @@ import 'package:meta/meta.dart' as meta; import 'package:path_provider/path_provider.dart'; import '../../../misc/exts.dart'; -import '../../impl_tools/errors.dart'; -import '../../impl_tools/no_sync.dart'; import '../../interfaces/backend.dart'; +import '../../interfaces/no_sync.dart'; +import '../../utils/errors.dart'; import 'models/generated/objectbox.g.dart'; import 'models/models.dart'; @@ -42,11 +42,12 @@ class _ObjectBoxBackendImpl final Map?>> _workerRes = {}; late int _workerId; late Completer _workerComplete; + late StreamSubscription _workerHandler; // `deleteOldestTile` tracking & debouncing - int _dotLength = 0; - Timer? _dotDebouncer; - String? _dotStore; + late int _dotLength; + late Timer _dotDebouncer; + late String? _dotStore; Future?> _sendCmd({ required _WorkerCmdType type, @@ -92,10 +93,13 @@ class _ObjectBoxBackendImpl }) async { if (_sendPort != null) throw RootAlreadyInitialised(); - // Setup worker isolate - final receivePort = ReceivePort(); + // Reset non-comms-related non-resource-intensive state _workerId = 0; _workerRes.clear(); + _dotStore = null; + _dotLength = 0; + + // Prepare to recieve `SendPort` from worker _workerRes[0] = Completer(); unawaited( _workerRes[0]!.future.then((res) { @@ -103,14 +107,19 @@ class _ObjectBoxBackendImpl _workerRes.remove(0); }), ); + + // Setup worker comms/response handler + final receivePort = ReceivePort(); _workerComplete = Completer(); - receivePort.listen( + _workerHandler = receivePort.listen( (evt) { evt as ({int id, Map? data}); _workerRes[evt.id]!.complete(evt.data); }, onDone: () => _workerComplete.complete(), ); + + // Spawn worker isolate await Isolate.spawn( _worker, ( @@ -133,20 +142,30 @@ class _ObjectBoxBackendImpl }) async { expectInitialised; - if (!immediate) { - await Future.wait(_workerRes.values.map((e) => e.future)); - } + // Wait for all currently underway operations to complete before destroying + // the isolate (if not `immediate`) + if (!immediate) await Future.wait(_workerRes.values.map((e) => e.future)); + // Send self-destruct cmd to worker, and don't wait for any response unawaited( _sendCmd( type: _WorkerCmdType.destroy_, args: {'deleteRoot': deleteRoot}, ), ); + + // Wait for worker to exit (worker handler will exit and signal) await _workerComplete.future; - _sendPort = null; + // Resource-intensive state cleanup only (other cleanup done during init) + _sendPort = null; // Indicate ready for re-init + await _workerHandler.cancel(); + _dotDebouncer.cancel(); + + print('passed _workerHandler cancel'); + // Kill any remaining operations with an error (they'll never recieve a + // response from the worker) for (final completer in _workerRes.values) { completer.complete({'error': RootUnavailable()}); } @@ -289,14 +308,14 @@ class _ObjectBoxBackendImpl // If the store has changed, failing to reset the batch/queue will mean // tiles are removed from the wrong store _dotStore = storeName; - if (_dotDebouncer != null && _dotDebouncer!.isActive) { - _dotDebouncer!.cancel(); + if (_dotDebouncer.isActive) { + _dotDebouncer.cancel(); _sendRemoveOldestTileCmd(storeName); } } - if (_dotDebouncer != null && _dotDebouncer!.isActive) { - _dotDebouncer!.cancel(); + if (_dotDebouncer.isActive) { + _dotDebouncer.cancel(); _dotDebouncer = Timer( const Duration(milliseconds: 500), () => _sendRemoveOldestTileCmd(storeName), diff --git a/lib/src/backend/impls/objectbox/models/models.dart b/lib/src/backend/impls/objectbox/models/models.dart index 5db648e1..6614096b 100644 --- a/lib/src/backend/impls/objectbox/models/models.dart +++ b/lib/src/backend/impls/objectbox/models/models.dart @@ -14,10 +14,8 @@ base class ObjectBoxStore extends BackendStore { @Unique() String name; - @override int numberOfTiles; - @override double numberOfBytes; @override diff --git a/lib/src/backend/impls/objectbox/worker.dart b/lib/src/backend/impls/objectbox/worker.dart index 7296dd25..ff928bed 100644 --- a/lib/src/backend/impls/objectbox/worker.dart +++ b/lib/src/backend/impls/objectbox/worker.dart @@ -154,6 +154,7 @@ Future _worker( storeQuery.close(); assert(store.tiles.isEmpty); + // TODO: Hits & misses stores.put( store diff --git a/lib/src/backend/interfaces/backend.dart b/lib/src/backend/interfaces/backend.dart index 1e8e0c1c..c23d0798 100644 --- a/lib/src/backend/interfaces/backend.dart +++ b/lib/src/backend/interfaces/backend.dart @@ -5,8 +5,6 @@ import 'package:meta/meta.dart'; import 'package:path_provider/path_provider.dart'; import '../../../flutter_map_tile_caching.dart'; -import '../impl_tools/errors.dart'; -import 'models.dart'; /// An abstract interface that FMTC will use to communicate with a storage /// 'backend' (usually one root) diff --git a/lib/src/backend/interfaces/models.dart b/lib/src/backend/interfaces/models.dart index bbc67d33..9697f221 100644 --- a/lib/src/backend/interfaces/models.dart +++ b/lib/src/backend/interfaces/models.dart @@ -4,8 +4,6 @@ import 'package:meta/meta.dart'; abstract base class BackendStore { abstract String name; - abstract int numberOfTiles; - abstract double numberOfBytes; abstract int hits; abstract int misses; diff --git a/lib/src/backend/impl_tools/no_sync.dart b/lib/src/backend/interfaces/no_sync.dart similarity index 97% rename from lib/src/backend/impl_tools/no_sync.dart rename to lib/src/backend/interfaces/no_sync.dart index a730054a..ab9a6a42 100644 --- a/lib/src/backend/impl_tools/no_sync.dart +++ b/lib/src/backend/interfaces/no_sync.dart @@ -1,8 +1,8 @@ import 'dart:typed_data'; -import '../interfaces/backend.dart'; -import '../interfaces/models.dart'; -import 'errors.dart'; +import '../utils/errors.dart'; +import 'backend.dart'; +import 'models.dart'; /// A shortcut to declare that an [FMTCBackend] does not support any synchronous /// versions of methods diff --git a/lib/src/backend/impl_tools/errors.dart b/lib/src/backend/utils/errors.dart similarity index 100% rename from lib/src/backend/impl_tools/errors.dart rename to lib/src/backend/utils/errors.dart diff --git a/lib/src/errors/store_not_ready.dart b/lib/src/errors/store_not_ready.dart deleted file mode 100644 index 2d374d38..00000000 --- a/lib/src/errors/store_not_ready.dart +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright © Luka S (JaffaKetchup) under GPL-v3 -// A full license can be found at .\LICENSE - -import 'package:meta/meta.dart'; - -/// An [Error] indicating that a store did not exist when it was expected to -/// -/// Commonly thrown by statistic operations, but can be thrown from multiple -/// other places. -class FMTCStoreNotReady extends Error { - /// The store name that the method tried to access - final String storeName; - - /// A human readable description of the error, and steps that may be taken to - /// avoid this error being thrown again - final String message; - - /// Whether this store was registered internally. - /// - /// Represents a serious internal FMTC error if `true`, as represented by - /// [message]. - final bool registered; - - /// An [Error] indicating that a store did not exist when it was expected to - /// - /// Commonly thrown by statistic operations, but can be thrown from multiple - /// other places. - @internal - FMTCStoreNotReady({ - required this.storeName, - required this.registered, - }) : message = registered - ? "The store ('$storeName') was registered, but the underlying database was not open, at this time. This is an erroneous state in FMTC: if this error appears in your application, please open an issue on GitHub immediately." - : "The store ('$storeName') does not exist at this time, and is not ready. Ensure that your application does not use the method that triggered this error unless it is sure that the store will exist at this point."; - - /// Similar to [message], but suitable for console output in an unknown context - @override - String toString() => 'FMTCStoreNotReady: $message'; -} diff --git a/lib/src/store/manage.dart b/lib/src/store/manage.dart index a231151a..86c53914 100644 --- a/lib/src/store/manage.dart +++ b/lib/src/store/manage.dart @@ -5,111 +5,51 @@ part of flutter_map_tile_caching; /// Manages a [StoreDirectory]'s representation on the filesystem, such as /// creation and deletion +/// +/// If the store is not in the expected state (of existence) when invoking an +/// operation, then an error will be thrown (likely [StoreNotExists] or +/// [StoreAlreadyExists]). It is recommended to check [ready] or [readySync] when +/// necessary. final class StoreManagement extends _WithBackendAccess { - StoreManagement._(super.store) - : _rootDirectory = FMTC.instance.rootDirectory.directory; - - final Directory _rootDirectory; + StoreManagement._(super.store); /// Whether this store exists Future get ready => _backend.storeExists(storeName: _storeName); - /// Create this store asynchronously - /// - /// Does nothing if the store already exists. - Future createAsync() async { - if (ready) return; - - final db = await Isar.open( - [DbStoreDescriptorSchema, DbTileSchema, DbMetadataSchema], - name: _id.toString(), - directory: _rootDirectory.path, - maxSizeMiB: FMTC.instance.settings.databaseMaxSize, - compactOnLaunch: FMTC.instance.settings.databaseCompactCondition, - inspector: false, - ); - await db.writeTxn( - () => db.storeDescriptor.put(DbStoreDescriptor(name: _name)), - ); - _registry.register(_id, db); - } + /// Whether this store exists + bool get readySync => _backend.storeExistsSync(storeName: _storeName); - /// Create this store synchronously - /// - /// Prefer [createAsync] to avoid blocking the UI thread. Otherwise, this has - /// slightly better performance. - /// - /// Does nothing if the store already exists. - void create() { - if (ready) return; + /// Create this store + Future create() => _backend.createStore(storeName: _storeName); - final db = Isar.openSync( - [DbStoreDescriptorSchema, DbTileSchema, DbMetadataSchema], - name: _id.toString(), - directory: _rootDirectory.path, - maxSizeMiB: FMTC.instance.settings.databaseMaxSize, - compactOnLaunch: FMTC.instance.settings.databaseCompactCondition, - inspector: false, - ); - db.writeTxnSync( - () => db.storeDescriptor.putSync(DbStoreDescriptor(name: _name)), - ); - _registry.register(_id, db); - } + /// Create this store + void createSync() => _backend.createStoreSync(storeName: _storeName); /// Delete this store /// - /// This will remove all traces of this store from the user's device. Use with - /// caution! - /// - /// Does nothing if the store does not already exist. - Future delete() async { - if (!ready) return; - - final store = _registry.unregister(_id); - if (store?.isOpen ?? false) await store!.close(deleteFromDisk: true); - } + /// This operation cannot be undone! Ensure you confirm with the user that + /// this action is expected. + Future delete() => _backend.deleteStore(storeName: _storeName); - /// Removes all tiles from this store synchronously - /// - /// Also resets the cache hits & misses statistic. + /// Delete this store /// - /// This method requires the store to be [ready], else an [FMTCStoreNotReady] - /// error will be raised. - Future resetAsync() async { - final db = _registry(_name); - await db.writeTxn(() async { - await db.tiles.clear(); - await db.storeDescriptor.put( - (await db.descriptor) - ..hits = 0 - ..misses = 0, - ); - }); - } + /// This operation cannot be undone! Ensure you confirm with the user that + /// this action is expected. + void deleteSync() => _backend.deleteStoreSync(storeName: _storeName); - /// Removes all tiles from this store asynchronously - /// - /// Also resets the cache hits & misses statistic. + /// Removes all tiles from this store /// - /// Prefer [resetAsync] to avoid blocking the UI thread. Otherwise, this has - /// slightly better performance. + /// This operation cannot be undone! Ensure you confirm with the user that + /// this action is expected. + Future reset() => _backend.resetStore(storeName: _storeName); + + /// Removes all tiles from this store /// - /// This method requires the store to be [ready], else an [FMTCStoreNotReady] - /// error will be raised. - void reset() { - final db = _registry(_name); - db.writeTxnSync(() { - db.tiles.clearSync(); - db.storeDescriptor.putSync( - db.descriptorSync - ..hits = 0 - ..misses = 0, - ); - }); - } + /// This operation cannot be undone! Ensure you confirm with the user that + /// this action is expected. + void resetSync() => _backend.resetStoreSync(storeName: _storeName); - /// Rename the store directory asynchronously + /// Rename the store directory /// /// The old [StoreDirectory] will still retain it's link to the old store, so /// always use the new returned value instead: returns a new [StoreDirectory] @@ -118,33 +58,14 @@ final class StoreManagement extends _WithBackendAccess { /// This method requires the store to be [ready], else an [FMTCStoreNotReady] /// error will be raised. Future rename(String newStoreName) async { - // Unregister and close old database without deleting it - final store = _registry.unregister(_id); - if (store == null) { - _registry(_name); - throw StateError( - 'This error represents a serious internal error in FMTC. Please raise a bug report if seen in any application', - ); - } - await store.close(); - - // Manually change the database's filename - await (_rootDirectory >>> '$_id.isar').rename( - (_rootDirectory >>> '${DatabaseTools.hash(newStoreName)}.isar').path, - ); - - // Register the new database (it will be re-opened) - final newStore = StoreDirectory._(newStoreName, autoCreate: false); - await newStore.manage.createAsync(); - - // Update the name stored inside the database - await _registry(newStoreName).writeTxn( - () => _registry(newStoreName) - .storeDescriptor - .put(DbStoreDescriptor(name: newStoreName)), + await _backend.renameStore( + currentStoreName: _storeName, + newStoreName: newStoreName, ); - return newStore; + // TODO: `autoCreate` and entire shortcut will now be broken by default + // consider whether this bi-synchronousable approach is sustainable + return StoreDirectory._(newStoreName, autoCreate: false); } /// Delete all tiles older that were last modified before [expiry]