From 0199951cd2416f2cf35e778f8454661a6958781a Mon Sep 17 00:00:00 2001 From: Dillon Nys Date: Fri, 7 Jun 2024 15:16:26 -0700 Subject: [PATCH] chore(native_storage): Various fixes - Ensure only one `NativeStorage` instance exists for any namespace/scope pair - Buffer calls to `close` - Ensure consistent validation of namespaces --- packages/native/storage/CHANGELOG.md | 6 + .../integration_test/storage_shared.dart | 225 +++++++++++++----- .../integration_test/storage_test.dart | 10 +- .../native/storage/example/ios/Podfile.lock | 2 +- .../native/storage/lib/native_storage.dart | 2 +- .../lib/src/isolated/isolated_storage.dart | 2 +- .../isolated_storage_platform.vm.dart | 19 +- .../isolated_storage_platform.web.dart | 5 +- .../lib/src/local/local_storage.android.dart | 2 +- .../storage/lib/src/local/local_storage.dart | 8 +- .../lib/src/local/local_storage.linux.dart | 2 +- .../lib/src/local/local_storage.windows.dart | 2 +- .../lib/src/local/local_storage_darwin.dart | 2 +- .../src/local/local_storage_platform.vm.dart | 24 +- .../src/local/local_storage_platform.web.dart | 25 +- .../lib/src/native/windows/windows.dart | 8 +- ...torage.dart => native_memory_storage.dart} | 55 +++-- .../storage/lib/src/native_storage.dart | 30 ++- .../storage/lib/src/native_storage_base.dart | 30 +++ .../lib/src/native_storage_extended.dart | 17 -- .../src/secure/secure_storage.android.dart | 2 +- .../lib/src/secure/secure_storage.dart | 7 +- .../lib/src/secure/secure_storage.darwin.dart | 2 +- .../lib/src/secure/secure_storage.linux.dart | 2 +- .../src/secure/secure_storage.windows.dart | 2 +- .../secure/secure_storage_platform.vm.dart | 20 +- .../secure/secure_storage_platform.web.dart | 2 +- .../storage/lib/src/util/namespace.dart | 23 ++ packages/native/storage/pubspec.yaml | 2 +- .../storage/test/native_storage_test.dart | 10 +- 30 files changed, 358 insertions(+), 190 deletions(-) rename packages/native/storage/lib/src/{memory_storage.dart => native_memory_storage.dart} (56%) create mode 100644 packages/native/storage/lib/src/native_storage_base.dart delete mode 100644 packages/native/storage/lib/src/native_storage_extended.dart create mode 100644 packages/native/storage/lib/src/util/namespace.dart diff --git a/packages/native/storage/CHANGELOG.md b/packages/native/storage/CHANGELOG.md index 105f2be..fdd24b5 100644 --- a/packages/native/storage/CHANGELOG.md +++ b/packages/native/storage/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.1.7 + +- fix: Ensure only one `NativeStorage` instance exists for any namespace/scope pair +- fix: Buffer calls to `close` +- chore: Ensure consistent validation of namespaces + ## 0.1.6 - feat: Support absolute scopes in `NativeStorage.scoped` diff --git a/packages/native/storage/example/integration_test/storage_shared.dart b/packages/native/storage/example/integration_test/storage_shared.dart index 6856e79..5a217c1 100644 --- a/packages/native/storage/example/integration_test/storage_shared.dart +++ b/packages/native/storage/example/integration_test/storage_shared.dart @@ -1,11 +1,66 @@ +// ignore_for_file: invalid_use_of_internal_member + import 'dart:math'; import 'package:native_storage/native_storage.dart'; -import 'package:native_storage/src/native_storage_extended.dart'; +import 'package:native_storage/src/native_storage_base.dart'; import 'package:test/test.dart'; -void sharedTests(String name, NativeStorageExtendedFactory factory) { - group(name, () { +enum NativeStorageType { + memory, + secure, + local; + + String get name => switch (this) { + memory => 'NativeMemoryStorage', + secure => 'NativeSecureStorage', + local => 'NativeLocalStorage', + }; +} + +void sharedTests(NativeStorageType type, NativeStorageFactory factory_) { + NativeStorageBase factory({ + String? namespace, + String? scope, + }) => + factory_(namespace: namespace, scope: scope) as NativeStorageBase; + + const bool isWeb = bool.fromEnvironment('dart.library.js_interop'); + final isMemoryStorage = type == NativeStorageType.memory; + final isSecureStorage = type == NativeStorageType.secure; + final hasIsolatedStorage = + isWeb ? (isSecureStorage || isMemoryStorage) : true; + final hasFullyIsolatedStorage = isWeb ? hasIsolatedStorage : isMemoryStorage; + + // ignore: unused_element + void dumpInstances() { + String dump(Map<(String, String?), NativeStorage> instances) { + return instances + .map((k, v) => MapEntry(k, '${v.runtimeType}(${v.hashCode})')) + .toString(); + } + + print('Memory: ${dump(NativeMemoryStorage.instances)}'); + print('Instances: ${dump(NativeStorage.instances)}'); + } + + void reset() { + for (final instance in List.of(NativeStorage.instances.values)) { + instance.close(); + } + for (final instance in List.of(NativeMemoryStorage.instances.values)) { + instance.close(); + } + } + + group(type.name, () { + const invalidNamespaces = ['com.domain/myapp', 'com.domain.myapp/']; + for (final namespace in invalidNamespaces) { + test('throws for invalid namespace: $namespace', () { + expect(() => factory(namespace: namespace), throwsArgumentError); + }); + } + const allowedNamespaces = [null, 'com.domain.myapp']; const allowedScopes = [null, 'scope', 'scope1/scope2']; for (final namespace in allowedNamespaces) { @@ -13,15 +68,16 @@ void sharedTests(String name, NativeStorageExtendedFactory factory) { group('namespace=$namespace', () { group('scope=$scope', () { late String key; - late final storage = factory(namespace: namespace, scope: scope); + late NativeStorageBase storage; setUp(() { + storage = factory(namespace: namespace, scope: scope); storage.clear(); // Add some randomness to prevent overlap between concurrent tests. key = _randomString(10); }); - tearDownAll(() { + tearDown(() { storage.clear(); storage.close(); }); @@ -119,23 +175,19 @@ void sharedTests(String name, NativeStorageExtendedFactory factory) { group('isolated', () { late String key; - late final isolated = storage.isolated; + late IsolatedNativeStorage isolated; setUp(() async { + isolated = storage.isolated; await isolated.clear(); // Add some randomness to prevent overlap between concurrent tests. key = _randomString(10); }); - tearDownAll(() async { - await isolated.clear(); - await isolated.close(); - }); - test( 'shares with non-isolated storage', // The NativeMemoryStorage does not share. - skip: storage is NativeMemoryStorage, + skip: hasFullyIsolatedStorage, () async { storage.write(key, 'value'); expect(await isolated.read(key), 'value'); @@ -148,6 +200,22 @@ void sharedTests(String name, NativeStorageExtendedFactory factory) { }, ); + test( + 'does not share with non-isolated storage', + // The NativeMemoryStorage does not share. + skip: !hasFullyIsolatedStorage, + () async { + storage.write(key, 'value'); + expect(await isolated.read(key), null); + + await isolated.write(key, 'isolated'); + expect(storage.read(key), 'value'); + + await isolated.clear(); + expect(storage.read(key), 'value'); + }, + ); + group('write', () { test('writes a new key-value pair to storage', () async { await isolated.write(key, 'value'); @@ -233,70 +301,113 @@ void sharedTests(String name, NativeStorageExtendedFactory factory) { } } - test('parent clears child', () { - final parent = factory(namespace: 'com.domain', scope: 'scope'); - final child = parent.scoped('child'); + group('fixes', () { + setUp(reset); - parent.write('key', 'parentValue'); - child.write('key', 'childValue'); + test('parent clears child', () { + final parent = factory(namespace: 'com.domain', scope: 'scope'); + final child = parent.scoped('child'); - expect(parent.read('key'), 'parentValue'); - expect(child.read('key'), 'childValue'); + parent.write('key', 'parentValue'); + child.write('key', 'childValue'); - expect(parent.allKeys, unorderedEquals(['key', 'child/key'])); - expect((child as NativeStorageExtended).allKeys, ['key']); + expect(parent.read('key'), 'parentValue'); + expect(child.read('key'), 'childValue'); - parent.clear(); + expect(parent.allKeys, unorderedEquals(['key', 'child/key'])); + expect((child as NativeStorageBase).allKeys, ['key']); - expect(parent.read('key'), isNull); - expect(child.read('key'), isNull); + parent.clear(); - expect(parent.allKeys, isEmpty); - expect(child.allKeys, isEmpty); - }); + expect(parent.read('key'), isNull); + expect(child.read('key'), isNull); - test('child does not clear parent', () { - final parent = factory(namespace: 'com.domain', scope: 'scope'); - final child = parent.scoped('child'); + expect(parent.allKeys, isEmpty); + expect(child.allKeys, isEmpty); + }); - parent.write('key', 'parentValue'); - child.write('key', 'childValue'); + test('child does not clear parent', () { + final parent = factory(namespace: 'com.domain', scope: 'scope'); + final child = parent.scoped('child'); - expect(parent.read('key'), 'parentValue'); - expect(child.read('key'), 'childValue'); + parent.write('key', 'parentValue'); + child.write('key', 'childValue'); - expect(parent.allKeys, unorderedEquals(['key', 'child/key'])); - expect((child as NativeStorageExtended).allKeys, ['key']); + expect(parent.read('key'), 'parentValue'); + expect(child.read('key'), 'childValue'); - child.clear(); + expect(parent.allKeys, unorderedEquals(['key', 'child/key'])); + expect((child as NativeStorageBase).allKeys, ['key']); - expect(parent.read('key'), 'parentValue'); - expect(child.read('key'), isNull); + child.clear(); - expect(parent.allKeys, ['key']); - expect(child.allKeys, isEmpty); + expect(parent.read('key'), 'parentValue'); + expect(child.read('key'), isNull); - parent.clear(); - }); + expect(parent.allKeys, ['key']); + expect(child.allKeys, isEmpty); + + parent.clear(); + }); + + test('rescope', () { + final nested = factory( + namespace: 'com.domain', + scope: 'scope/child/nested', + ); + expect(nested.scope, 'scope/child/nested'); + + final root = nested.scoped('/'); + expect(root.scope, null); + + final child = nested.scoped('/scope/child'); + expect(child.scope, 'scope/child'); + + final current = nested.scoped(''); + expect(current.scope, 'scope/child/nested'); + + final drill = child.scoped('nested'); + expect(drill.scope, 'scope/child/nested'); + }); + + test('instance caching', () { + void compare(NativeStorage instance1, NativeStorage instance2) { + expect(instance1, same(instance2)); + + final secure1 = instance1.secure; + final secure2 = instance2.secure; + expect(secure1, same(secure2)); + + final isolated1 = instance1.isolated; + final isolated2 = instance2.isolated; + expect(isolated1, same(isolated2)); + } - test('rescope', () { - final nested = factory( - namespace: 'com.domain', - scope: 'scope/child/nested', - ); - expect(nested.scope, 'scope/child/nested'); + final instance1 = factory(); + final instance2 = factory(); + compare(instance1, instance2); - final root = nested.scoped('/'); - expect(root.scope, null); + final scope1 = factory(namespace: 'com.domain', scope: 'scope'); + final scope2 = factory(namespace: 'com.domain', scope: 'scope'); + compare(scope1, scope2); - final child = nested.scoped('/scope/child'); - expect(child.scope, 'scope/child'); + final child1 = scope1.scoped('child'); + final child2 = scope2.scoped('child'); + compare(child1, child2); + }); - final current = nested.scoped(''); - expect(current.scope, 'scope/child/nested'); + test('buffers close calls', skip: !hasIsolatedStorage, () async { + final instance = factory(); + final isolated = instance.isolated; + instance.close(); + expect(isolated, same(instance.isolated)); + await expectLater(isolated.read('key'), throwsStateError); - final drill = child.scoped('nested'); - expect(drill.scope, 'scope/child/nested'); + final newInstance = factory(); + expect(instance, isNot(same(newInstance))); + expect(isolated, isNot(same(newInstance.isolated))); + await expectLater(newInstance.isolated.read('key'), completion(null)); + }); }); }); } diff --git a/packages/native/storage/example/integration_test/storage_test.dart b/packages/native/storage/example/integration_test/storage_test.dart index 472f6a0..36b040b 100644 --- a/packages/native/storage/example/integration_test/storage_test.dart +++ b/packages/native/storage/example/integration_test/storage_test.dart @@ -1,15 +1,11 @@ import 'package:integration_test/integration_test.dart'; import 'package:native_storage/native_storage.dart'; -import 'package:native_storage/src/local/local_storage_platform.vm.dart' - if (dart.library.js_interop) 'package:native_storage/src/local/local_storage_platform.web.dart'; -import 'package:native_storage/src/secure/secure_storage_platform.vm.dart' - if (dart.library.js_interop) 'package:native_storage/src/secure/secure_storage_platform.web.dart'; import 'storage_shared.dart'; void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); - sharedTests('NativeMemoryStorage', NativeMemoryStorage.new); - sharedTests('NativeSecureStorage', NativeSecureStoragePlatform.new); - sharedTests('NativeLocalStorage', NativeLocalStoragePlatform.new); + sharedTests(NativeStorageType.memory, NativeMemoryStorage.new); + sharedTests(NativeStorageType.secure, NativeSecureStorage.new); + sharedTests(NativeStorageType.local, NativeLocalStorage.new); } diff --git a/packages/native/storage/example/ios/Podfile.lock b/packages/native/storage/example/ios/Podfile.lock index d0dbce6..bc8361b 100644 --- a/packages/native/storage/example/ios/Podfile.lock +++ b/packages/native/storage/example/ios/Podfile.lock @@ -15,7 +15,7 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: Flutter: e0871f40cf51350855a761d2e70bf5af5b9b5de7 - integration_test: 13825b8a9334a850581300559b8839134b124670 + integration_test: ce0a3ffa1de96d1a89ca0ac26fca7ea18a749ef4 PODFILE CHECKSUM: 819463e6a0290f5a72f145ba7cde16e8b6ef0796 diff --git a/packages/native/storage/lib/native_storage.dart b/packages/native/storage/lib/native_storage.dart index 11d3ca3..0849bf5 100644 --- a/packages/native/storage/lib/native_storage.dart +++ b/packages/native/storage/lib/native_storage.dart @@ -1,6 +1,6 @@ export 'src/isolated/isolated_storage.dart'; export 'src/local/local_storage.dart'; -export 'src/memory_storage.dart'; +export 'src/native_memory_storage.dart'; export 'src/native_storage.dart'; export 'src/native_storage_exception.dart'; export 'src/secure/secure_storage.dart'; diff --git a/packages/native/storage/lib/src/isolated/isolated_storage.dart b/packages/native/storage/lib/src/isolated/isolated_storage.dart index 236ad5e..dca1ff8 100644 --- a/packages/native/storage/lib/src/isolated/isolated_storage.dart +++ b/packages/native/storage/lib/src/isolated/isolated_storage.dart @@ -4,7 +4,7 @@ import 'package:native_storage/src/isolated/isolated_storage_platform.unsupporte if (dart.library.isolate) 'package:native_storage/src/isolated/isolated_storage_platform.vm.dart'; /// A [NativeStorage] constructor. -typedef NativeStorageFactory = NativeStorage Function({ +typedef NativeStorageFactory = T Function({ String? namespace, String? scope, }); diff --git a/packages/native/storage/lib/src/isolated/isolated_storage_platform.vm.dart b/packages/native/storage/lib/src/isolated/isolated_storage_platform.vm.dart index d5b841a..5d1101c 100644 --- a/packages/native/storage/lib/src/isolated/isolated_storage_platform.vm.dart +++ b/packages/native/storage/lib/src/isolated/isolated_storage_platform.vm.dart @@ -11,6 +11,14 @@ typedef StorageConfig = ({ String? scope, }); +extension on StorageConfig { + String get qualifiedName { + final namespace = this.namespace ?? 'default'; + if (scope == null) return namespace; + return '$namespace/$scope'; + } +} + /// The VM implementation of [IsolatedNativeStorage] which uses an [Isolate] /// to handle storage operations. final class IsolatedNativeStoragePlatform implements IsolatedNativeStorage { @@ -23,7 +31,7 @@ final class IsolatedNativeStoragePlatform implements IsolatedNativeStorage { namespace: namespace, scope: scope, ) { - _spawned = spawn().then((_) { + _spawned = _spawn().then((_) { _listener = _channel!.stream.listen((response) { final completer = _pendingRequests.remove(response.id); if (completer == null) { @@ -42,13 +50,13 @@ final class IsolatedNativeStoragePlatform implements IsolatedNativeStorage { final _pendingRequests = >{}; var _closed = false; - Future spawn() async { + Future _spawn() async { final port = ReceivePort(); _channel = IsolateChannel.connectReceive(port); _isolate = await Isolate.spawn( _handleRequests, (port.sendPort, _config), - debugName: 'IsolatedStoragePlatform', + debugName: 'IsolatedNativeStorage(${_config.qualifiedName})', ); } @@ -126,10 +134,12 @@ final class IsolatedNativeStoragePlatform implements IsolatedNativeStorage { return writtenValue!; } + final _closeCompleter = Completer(); + @override Future close({bool force = false}) async { if (_closed) { - return; + return _closeCompleter.future; } _closed = true; try { @@ -153,6 +163,7 @@ final class IsolatedNativeStoragePlatform implements IsolatedNativeStorage { _isolate = null; _spawned?.ignore(); _spawned = null; + _closeCompleter.complete(); } } } diff --git a/packages/native/storage/lib/src/isolated/isolated_storage_platform.web.dart b/packages/native/storage/lib/src/isolated/isolated_storage_platform.web.dart index 66df7a2..5839a20 100644 --- a/packages/native/storage/lib/src/isolated/isolated_storage_platform.web.dart +++ b/packages/native/storage/lib/src/isolated/isolated_storage_platform.web.dart @@ -195,10 +195,12 @@ self.postMessage('ready'); return writtenValue!; } + final _closeCompleter = Completer(); + @override Future close({bool force = false}) async { if (_closed) { - return; + return _closeCompleter.future; } _closed = true; try { @@ -218,6 +220,7 @@ self.postMessage('ready'); _worker = null; _spawned?.ignore(); _spawned = null; + _closeCompleter.future; } } } diff --git a/packages/native/storage/lib/src/local/local_storage.android.dart b/packages/native/storage/lib/src/local/local_storage.android.dart index 0eb062d..bd91d4c 100644 --- a/packages/native/storage/lib/src/local/local_storage.android.dart +++ b/packages/native/storage/lib/src/local/local_storage.android.dart @@ -4,7 +4,7 @@ import 'package:native_storage/src/native/android/android.dart'; final class LocalStoragePlatformAndroid extends NativeLocalStoragePlatform { LocalStoragePlatformAndroid({ - String? namespace, + super.namespace, super.scope, }) : _namespace = namespace, super.base(); diff --git a/packages/native/storage/lib/src/local/local_storage.dart b/packages/native/storage/lib/src/local/local_storage.dart index 689dfc0..521e9e0 100644 --- a/packages/native/storage/lib/src/local/local_storage.dart +++ b/packages/native/storage/lib/src/local/local_storage.dart @@ -1,6 +1,4 @@ import 'package:native_storage/native_storage.dart'; -import 'package:native_storage/src/local/local_storage_platform.vm.dart' - if (dart.library.js_interop) 'package:native_storage/src/local/local_storage_platform.web.dart'; /// {@template native_storage.native_local_storage} /// Provides app-local storage of key-value pairs. @@ -16,7 +14,11 @@ abstract interface class NativeLocalStorage implements NativeStorage { factory NativeLocalStorage({ String? namespace, String? scope, - }) = NativeLocalStoragePlatform; + }) { + // Route through [NativeStorage] to ensure de-duplication of instances. + return NativeStorage(namespace: namespace, scope: scope) + as NativeLocalStorage; + } @override NativeLocalStorage scoped(String scope); diff --git a/packages/native/storage/lib/src/local/local_storage.linux.dart b/packages/native/storage/lib/src/local/local_storage.linux.dart index a44f540..ff07f7e 100644 --- a/packages/native/storage/lib/src/local/local_storage.linux.dart +++ b/packages/native/storage/lib/src/local/local_storage.linux.dart @@ -7,7 +7,7 @@ import 'package:path/path.dart' as p; final class LocalStorageLinux extends NativeLocalStoragePlatform { LocalStorageLinux({ - String? namespace, + super.namespace, super.scope, }) : super.base(); diff --git a/packages/native/storage/lib/src/local/local_storage.windows.dart b/packages/native/storage/lib/src/local/local_storage.windows.dart index 54f0282..e1dbb41 100644 --- a/packages/native/storage/lib/src/local/local_storage.windows.dart +++ b/packages/native/storage/lib/src/local/local_storage.windows.dart @@ -5,7 +5,7 @@ import 'package:win32_registry/win32_registry.dart'; final class LocalStorageWindows extends NativeLocalStoragePlatform with NativeStorageWindows { LocalStorageWindows({ - String? namespace, + super.namespace, super.scope, }) : namespaceOverride = namespace, super.base(); diff --git a/packages/native/storage/lib/src/local/local_storage_darwin.dart b/packages/native/storage/lib/src/local/local_storage_darwin.dart index 3a71f21..09c5910 100644 --- a/packages/native/storage/lib/src/local/local_storage_darwin.dart +++ b/packages/native/storage/lib/src/local/local_storage_darwin.dart @@ -8,7 +8,7 @@ import 'package:path/path.dart' as p; final class LocalStoragePlatformDarwin extends NativeLocalStoragePlatform { LocalStoragePlatformDarwin({ - String? namespace, + super.namespace, super.scope, }) : _namespace = namespace, super.base(); diff --git a/packages/native/storage/lib/src/local/local_storage_platform.vm.dart b/packages/native/storage/lib/src/local/local_storage_platform.vm.dart index ad35331..3617bfb 100644 --- a/packages/native/storage/lib/src/local/local_storage_platform.vm.dart +++ b/packages/native/storage/lib/src/local/local_storage_platform.vm.dart @@ -6,15 +6,14 @@ import 'package:native_storage/src/local/local_storage.android.dart'; import 'package:native_storage/src/local/local_storage.linux.dart'; import 'package:native_storage/src/local/local_storage.windows.dart'; import 'package:native_storage/src/local/local_storage_darwin.dart'; -import 'package:native_storage/src/native_storage_extended.dart'; +import 'package:native_storage/src/native_storage_base.dart'; +import 'package:native_storage/src/secure/secure_storage_platform.vm.dart'; import 'package:native_storage/src/util/rescope.dart'; /// The VM implementation of [NativeLocalStorage]. -abstract base class NativeLocalStoragePlatform - implements - NativeLocalStorage, - // ignore: invalid_use_of_visible_for_testing_member - NativeStorageExtended { +// ignore: invalid_use_of_visible_for_testing_member +abstract base class NativeLocalStoragePlatform extends NativeStorageBase + implements NativeLocalStorage { factory NativeLocalStoragePlatform({ String? namespace, String? scope, @@ -36,6 +35,7 @@ abstract base class NativeLocalStoragePlatform @protected NativeLocalStoragePlatform.base({ + required super.namespace, this.scope, }); @@ -44,28 +44,26 @@ abstract base class NativeLocalStoragePlatform @override @mustCallSuper - void close() { + void closeInternal() { _secure?.close(); - _secure = null; _isolated?.close().ignore(); - _isolated = null; } NativeSecureStorage? _secure; @override - NativeSecureStorage get secure => - _secure ??= NativeSecureStorage(namespace: namespace, scope: scope); + NativeSecureStorage get secure => _secure ??= + NativeSecureStoragePlatform(namespace: namespace, scope: scope); IsolatedNativeStorage? _isolated; @override IsolatedNativeStorage get isolated => _isolated ??= IsolatedNativeStorage( - factory: NativeLocalStoragePlatform.new, + factory: NativeLocalStorage.new, namespace: namespace, scope: scope, ); @override - NativeLocalStorage scoped(String scope) => NativeLocalStoragePlatform( + NativeLocalStorage scoped(String scope) => NativeLocalStorage( namespace: namespace, scope: rescope(scope), ); diff --git a/packages/native/storage/lib/src/local/local_storage_platform.web.dart b/packages/native/storage/lib/src/local/local_storage_platform.web.dart index e8db6cb..26626f2 100644 --- a/packages/native/storage/lib/src/local/local_storage_platform.web.dart +++ b/packages/native/storage/lib/src/local/local_storage_platform.web.dart @@ -1,17 +1,16 @@ import 'package:native_storage/native_storage.dart'; import 'package:native_storage/src/isolated/isolated_storage_platform.unsupported.dart' as unsupported; -import 'package:native_storage/src/native_storage_extended.dart'; +import 'package:native_storage/src/native_storage_base.dart'; +import 'package:native_storage/src/secure/secure_storage_platform.web.dart'; import 'package:native_storage/src/util/rescope.dart'; import 'package:web/web.dart' as web; /// The browser implementation of [NativeLocalStorage]. -final class NativeLocalStoragePlatform - implements - NativeLocalStorage, - // ignore: invalid_use_of_visible_for_testing_member - NativeStorageExtended { - NativeLocalStoragePlatform({String? namespace, this.scope}) +// ignore: invalid_use_of_visible_for_testing_member +final class NativeLocalStoragePlatform extends NativeStorageBase + implements NativeLocalStorage { + NativeLocalStoragePlatform({super.namespace, this.scope}) : namespace = namespace ?? web.window.location.hostname; @override @@ -56,17 +55,15 @@ final class NativeLocalStoragePlatform ]; @override - void close() { + void closeInternal() { _secure?.close(); - _secure = null; - _isolated?.close().ignore(); - _isolated = null; + _isolated?.close(); } NativeSecureStorage? _secure; @override - NativeSecureStorage get secure => - _secure ??= NativeSecureStorage(namespace: namespace, scope: scope); + NativeSecureStorage get secure => _secure ??= + NativeSecureStoragePlatform(namespace: namespace, scope: scope); IsolatedNativeStorage? _isolated; @override @@ -74,7 +71,7 @@ final class NativeLocalStoragePlatform _isolated ??= unsupported.IsolatedNativeStoragePlatform.from(this); @override - NativeLocalStorage scoped(String scope) => NativeLocalStoragePlatform( + NativeLocalStorage scoped(String scope) => NativeLocalStorage( namespace: namespace, scope: rescope(scope), ); diff --git a/packages/native/storage/lib/src/native/windows/windows.dart b/packages/native/storage/lib/src/native/windows/windows.dart index 5b175e9..813ab92 100644 --- a/packages/native/storage/lib/src/native/windows/windows.dart +++ b/packages/native/storage/lib/src/native/windows/windows.dart @@ -4,7 +4,7 @@ import 'dart:io'; import 'package:ffi/ffi.dart'; import 'package:meta/meta.dart'; import 'package:native_storage/native_storage.dart'; -import 'package:native_storage/src/native_storage_extended.dart'; +import 'package:native_storage/src/native_storage_base.dart'; import 'package:native_storage/src/util/functional.dart'; import 'package:path/path.dart' as p; import 'package:win32/win32.dart'; @@ -135,7 +135,7 @@ final class WindowsCommon { } // ignore: invalid_use_of_visible_for_testing_member -mixin NativeStorageWindows on NativeStorage implements NativeStorageExtended { +mixin NativeStorageWindows on NativeStorageBase { @protected abstract final String? namespaceOverride; @@ -214,8 +214,8 @@ mixin NativeStorageWindows on NativeStorage implements NativeStorageExtended { } @override - void close() { + void closeInternal() { registry.close(); - super.close(); + super.closeInternal(); } } diff --git a/packages/native/storage/lib/src/memory_storage.dart b/packages/native/storage/lib/src/native_memory_storage.dart similarity index 56% rename from packages/native/storage/lib/src/memory_storage.dart rename to packages/native/storage/lib/src/native_memory_storage.dart index d5b7ea8..8118689 100644 --- a/packages/native/storage/lib/src/memory_storage.dart +++ b/packages/native/storage/lib/src/native_memory_storage.dart @@ -1,27 +1,37 @@ +import 'package:meta/meta.dart'; import 'package:native_storage/src/isolated/isolated_storage.dart'; import 'package:native_storage/src/native_storage.dart'; -import 'package:native_storage/src/native_storage_extended.dart'; +import 'package:native_storage/src/native_storage_base.dart'; import 'package:native_storage/src/secure/secure_storage.dart'; +import 'package:native_storage/src/util/namespace.dart'; import 'package:native_storage/src/util/rescope.dart'; /// An in-memory implementation of [NativeStorage] and [NativeSecureStorage]. -final class NativeMemoryStorage - implements - NativeStorage, - NativeSecureStorage, - // ignore: invalid_use_of_visible_for_testing_member - NativeStorageExtended { - NativeMemoryStorage({ +// ignore: invalid_use_of_visible_for_testing_member +final class NativeMemoryStorage extends NativeStorageBase + implements NativeStorage, NativeSecureStorage { + factory NativeMemoryStorage({ String? namespace, - this.scope, - }) : namespace = namespace ?? '', - _storage = {}; + String? scope, + }) { + namespace ??= 'default'; + validateNamespace(namespace); + return instances[(namespace, scope)] ??= NativeMemoryStorage._( + namespace: namespace, + scope: scope, + ); + } NativeMemoryStorage._({ - required this.namespace, + super.namespace, this.scope, Map? storage, - }) : _storage = storage ?? {}; + }) : namespace = namespace ?? 'default', + _storage = storage ?? {}; + + @visibleForTesting + static final Map<(String namespace, String? scope), NativeMemoryStorage> + instances = {}; @override final String namespace; @@ -52,10 +62,10 @@ final class NativeMemoryStorage ]; @override - void close() { + void closeInternal() { clear(); _isolated?.close().ignore(); - _isolated = null; + instances.remove((namespace, scope)); } @override @@ -64,15 +74,18 @@ final class NativeMemoryStorage IsolatedNativeStorage? _isolated; @override IsolatedNativeStorage get isolated => _isolated ??= IsolatedNativeStorage( - factory: NativeMemoryStorage.new, + factory: NativeMemoryStorage._, namespace: namespace, scope: scope, ); @override - NativeMemoryStorage scoped(String scope) => NativeMemoryStorage._( - namespace: namespace, - scope: rescope(scope), - storage: _storage, - ); + NativeMemoryStorage scoped(String scope) { + final newScope = rescope(scope); + return instances[(namespace, newScope)] ??= NativeMemoryStorage._( + namespace: namespace, + scope: newScope, + storage: _storage, + ); + } } diff --git a/packages/native/storage/lib/src/native_storage.dart b/packages/native/storage/lib/src/native_storage.dart index 67eaa18..69c5342 100644 --- a/packages/native/storage/lib/src/native_storage.dart +++ b/packages/native/storage/lib/src/native_storage.dart @@ -1,6 +1,9 @@ +import 'package:meta/meta.dart'; import 'package:native_storage/src/isolated/isolated_storage.dart'; -import 'package:native_storage/src/local/local_storage.dart'; +import 'package:native_storage/src/local/local_storage_platform.vm.dart' + if (dart.library.js_interop) 'package:native_storage/src/local/local_storage_platform.web.dart'; import 'package:native_storage/src/secure/secure_storage.dart'; +import 'package:native_storage/src/util/namespace.dart'; /// An interface for native storage implementations. /// @@ -20,22 +23,17 @@ abstract interface class NativeStorage { String? namespace, String? scope, }) { - final isValidNamespace = - namespace == null || _validNamespace.hasMatch(namespace); - if (!isValidNamespace) { - throw ArgumentError.value( - namespace, - 'namespace', - 'Invalid namespace. Must match $_validNamespace', - ); - } - return NativeLocalStorage(namespace: namespace, scope: scope); + validateNamespace(namespace); + final instance = NativeLocalStoragePlatform( + namespace: namespace, + scope: scope, + ); + return instances[(instance.namespace, scope)] ??= instance; } - static final _validNamespace = RegExp( - r'^[a-z0-9]+(\.[a-z0-9]+)+$', - caseSensitive: false, - ); + @visibleForTesting + static final Map<(String namespace, String? scope), NativeStorage> instances = + {}; /// {@template native_storage.native_storage.namespace} /// The main identifier all values are stored under. @@ -45,7 +43,7 @@ abstract interface class NativeStorage { /// application or bundle identifier, which is the default if not passed. /// /// If provided, it must match the regular expression - /// `^[a-z0-9]+(\.[a-z0-9]+)+$`, which is that of an bundle identifier. + /// `^[a-z0-9]+(\.[a-z0-9]+)*$`, which is that of an bundle identifier. /// {@endtemplate} String get namespace; diff --git a/packages/native/storage/lib/src/native_storage_base.dart b/packages/native/storage/lib/src/native_storage_base.dart new file mode 100644 index 0000000..3c8bc1b --- /dev/null +++ b/packages/native/storage/lib/src/native_storage_base.dart @@ -0,0 +1,30 @@ +import 'package:meta/meta.dart'; +import 'package:native_storage/native_storage.dart'; + +/// Base implementation for [NativeStorage]. +@internal +abstract class NativeStorageBase implements NativeStorage { + NativeStorageBase({ + required String? namespace, + }); + + @visibleForTesting + List get allKeys; + + var _closed = false; + + @override + @nonVirtual + void close() { + if (_closed) { + return; + } + _closed = true; + closeInternal(); + // ignore: invalid_use_of_visible_for_testing_member + NativeStorage.instances.remove((namespace, scope)); + } + + @protected + void closeInternal(); +} diff --git a/packages/native/storage/lib/src/native_storage_extended.dart b/packages/native/storage/lib/src/native_storage_extended.dart deleted file mode 100644 index a9486a7..0000000 --- a/packages/native/storage/lib/src/native_storage_extended.dart +++ /dev/null @@ -1,17 +0,0 @@ -@visibleForTesting -library; - -import 'package:meta/meta.dart'; -import 'package:native_storage/native_storage.dart'; - -typedef NativeStorageExtendedFactory = NativeStorageExtended Function({ - String? namespace, - String? scope, -}); - -/// Extended interface for [NativeStorage], used in tests. -@visibleForTesting -abstract interface class NativeStorageExtended implements NativeStorage { - @visibleForTesting - List get allKeys; -} diff --git a/packages/native/storage/lib/src/secure/secure_storage.android.dart b/packages/native/storage/lib/src/secure/secure_storage.android.dart index 62a0f67..02ab8e0 100644 --- a/packages/native/storage/lib/src/secure/secure_storage.android.dart +++ b/packages/native/storage/lib/src/secure/secure_storage.android.dart @@ -4,7 +4,7 @@ import 'package:native_storage/src/secure/secure_storage_platform.vm.dart'; final class SecureStorageAndroid extends NativeSecureStoragePlatform { SecureStorageAndroid({ - String? namespace, + super.namespace, super.scope, }) : _namespace = namespace, super.base(); diff --git a/packages/native/storage/lib/src/secure/secure_storage.dart b/packages/native/storage/lib/src/secure/secure_storage.dart index 6c46722..46f49b2 100644 --- a/packages/native/storage/lib/src/secure/secure_storage.dart +++ b/packages/native/storage/lib/src/secure/secure_storage.dart @@ -1,6 +1,4 @@ import 'package:native_storage/native_storage.dart'; -import 'package:native_storage/src/secure/secure_storage_platform.vm.dart' - if (dart.library.js_interop) 'package:native_storage/src/secure/secure_storage_platform.web.dart'; /// {@template native_storage.native_secure_storage} /// Provides platform-specific secure storage, typically using the OS's secure @@ -14,7 +12,10 @@ abstract interface class NativeSecureStorage implements NativeStorage { factory NativeSecureStorage({ String? namespace, String? scope, - }) = NativeSecureStoragePlatform; + }) { + // Route through [NativeStorage] to ensure de-duplication of instances. + return NativeStorage(namespace: namespace, scope: scope).secure; + } @override NativeSecureStorage scoped(String scope); diff --git a/packages/native/storage/lib/src/secure/secure_storage.darwin.dart b/packages/native/storage/lib/src/secure/secure_storage.darwin.dart index b6de911..d9ade85 100644 --- a/packages/native/storage/lib/src/secure/secure_storage.darwin.dart +++ b/packages/native/storage/lib/src/secure/secure_storage.darwin.dart @@ -15,7 +15,7 @@ import 'package:path/path.dart' as p; final class SecureStorageDarwin extends NativeSecureStoragePlatform { SecureStorageDarwin({ - String? namespace, + super.namespace, super.scope, }) : _namespace = namespace, super.base(); diff --git a/packages/native/storage/lib/src/secure/secure_storage.linux.dart b/packages/native/storage/lib/src/secure/secure_storage.linux.dart index 5d25e6d..f6cd1d4 100644 --- a/packages/native/storage/lib/src/secure/secure_storage.linux.dart +++ b/packages/native/storage/lib/src/secure/secure_storage.linux.dart @@ -11,7 +11,7 @@ import 'package:native_storage/src/util/functional.dart'; final class SecureStorageLinux extends NativeSecureStoragePlatform { SecureStorageLinux({ - String? namespace, + super.namespace, super.scope, }) : _namespace = namespace, super.base(); diff --git a/packages/native/storage/lib/src/secure/secure_storage.windows.dart b/packages/native/storage/lib/src/secure/secure_storage.windows.dart index 0293b1a..2429a8d 100644 --- a/packages/native/storage/lib/src/secure/secure_storage.windows.dart +++ b/packages/native/storage/lib/src/secure/secure_storage.windows.dart @@ -12,7 +12,7 @@ import 'package:win32_registry/win32_registry.dart'; final class SecureStorageWindows extends NativeSecureStoragePlatform with NativeStorageWindows { SecureStorageWindows({ - String? namespace, + super.namespace, super.scope, }) : namespaceOverride = namespace, super.base(); diff --git a/packages/native/storage/lib/src/secure/secure_storage_platform.vm.dart b/packages/native/storage/lib/src/secure/secure_storage_platform.vm.dart index 5f2b6b1..86427d0 100644 --- a/packages/native/storage/lib/src/secure/secure_storage_platform.vm.dart +++ b/packages/native/storage/lib/src/secure/secure_storage_platform.vm.dart @@ -2,18 +2,16 @@ import 'dart:io'; import 'package:meta/meta.dart'; import 'package:native_storage/native_storage.dart'; -import 'package:native_storage/src/native_storage_extended.dart'; +import 'package:native_storage/src/native_storage_base.dart'; import 'package:native_storage/src/secure/secure_storage.android.dart'; import 'package:native_storage/src/secure/secure_storage.darwin.dart'; import 'package:native_storage/src/secure/secure_storage.linux.dart'; import 'package:native_storage/src/secure/secure_storage.windows.dart'; import 'package:native_storage/src/util/rescope.dart'; -abstract base class NativeSecureStoragePlatform - implements - NativeSecureStorage, - // ignore: invalid_use_of_visible_for_testing_member - NativeStorageExtended { +// ignore: invalid_use_of_visible_for_testing_member +abstract base class NativeSecureStoragePlatform extends NativeStorageBase + implements NativeSecureStorage { factory NativeSecureStoragePlatform({ String? namespace, String? scope, @@ -35,6 +33,7 @@ abstract base class NativeSecureStoragePlatform @protected NativeSecureStoragePlatform.base({ + required super.namespace, this.scope, }); @@ -43,24 +42,25 @@ abstract base class NativeSecureStoragePlatform @override @mustCallSuper - void close() { + void closeInternal() { _isolated?.close().ignore(); - _isolated = null; } @override + @nonVirtual NativeSecureStorage get secure => this; IsolatedNativeStorage? _isolated; @override + @nonVirtual IsolatedNativeStorage get isolated => _isolated ??= IsolatedNativeStorage( - factory: NativeSecureStoragePlatform.new, + factory: NativeSecureStorage.new, namespace: namespace, scope: scope, ); @override - NativeSecureStorage scoped(String scope) => NativeSecureStoragePlatform( + NativeSecureStorage scoped(String scope) => NativeSecureStorage( namespace: namespace, scope: rescope(scope), ); diff --git a/packages/native/storage/lib/src/secure/secure_storage_platform.web.dart b/packages/native/storage/lib/src/secure/secure_storage_platform.web.dart index 034b838..f45af26 100644 --- a/packages/native/storage/lib/src/secure/secure_storage_platform.web.dart +++ b/packages/native/storage/lib/src/secure/secure_storage_platform.web.dart @@ -1,3 +1,3 @@ -import 'package:native_storage/src/memory_storage.dart'; +import 'package:native_storage/src/native_memory_storage.dart'; typedef NativeSecureStoragePlatform = NativeMemoryStorage; diff --git a/packages/native/storage/lib/src/util/namespace.dart b/packages/native/storage/lib/src/util/namespace.dart new file mode 100644 index 0000000..e32f8af --- /dev/null +++ b/packages/native/storage/lib/src/util/namespace.dart @@ -0,0 +1,23 @@ +@internal +library; + +import 'package:meta/meta.dart'; + +final _validNamespace = RegExp( + r'^[a-z0-9]+(\.[a-z0-9]+)*$', + caseSensitive: false, +); + +String? validateNamespace(String? namespace) { + if (namespace == null) { + return namespace; + } + if (!_validNamespace.hasMatch(namespace)) { + throw ArgumentError.value( + namespace, + 'namespace', + 'Must match pattern: ${_validNamespace.pattern}', + ); + } + return namespace; +} diff --git a/packages/native/storage/pubspec.yaml b/packages/native/storage/pubspec.yaml index 7c82e95..808dd4c 100644 --- a/packages/native/storage/pubspec.yaml +++ b/packages/native/storage/pubspec.yaml @@ -1,6 +1,6 @@ name: native_storage description: A Dart-only package for accessing platform-native storage functionality. -version: 0.1.6 +version: 0.1.7 repository: https://github.com/celest-dev/dart-packages/tree/main/packages/native/storage environment: diff --git a/packages/native/storage/test/native_storage_test.dart b/packages/native/storage/test/native_storage_test.dart index 71fef44..4dcffcf 100644 --- a/packages/native/storage/test/native_storage_test.dart +++ b/packages/native/storage/test/native_storage_test.dart @@ -1,13 +1,9 @@ import 'package:native_storage/native_storage.dart'; -import 'package:native_storage/src/local/local_storage_platform.vm.dart' - if (dart.library.js_interop) 'package:native_storage/src/local/local_storage_platform.web.dart'; -import 'package:native_storage/src/secure/secure_storage_platform.vm.dart' - if (dart.library.js_interop) 'package:native_storage/src/secure/secure_storage_platform.web.dart'; import '../example/integration_test/storage_shared.dart'; void main() { - sharedTests('NativeMemoryStorage', NativeMemoryStorage.new); - sharedTests('NativeSecureStorage', NativeSecureStoragePlatform.new); - sharedTests('NativeLocalStorage', NativeLocalStoragePlatform.new); + sharedTests(NativeStorageType.memory, NativeMemoryStorage.new); + sharedTests(NativeStorageType.secure, NativeSecureStorage.new); + sharedTests(NativeStorageType.local, NativeLocalStorage.new); }