diff --git a/packages/secure_storage/amplify_secure_storage_dart/lib/src/utils/file_key_value_store.dart b/packages/secure_storage/amplify_secure_storage_dart/lib/src/utils/file_key_value_store.dart index 6ce470b42f..b87b9b1f32 100644 --- a/packages/secure_storage/amplify_secure_storage_dart/lib/src/utils/file_key_value_store.dart +++ b/packages/secure_storage/amplify_secure_storage_dart/lib/src/utils/file_key_value_store.dart @@ -1,9 +1,11 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:aws_common/aws_common.dart'; import 'package:file/file.dart' as pkg_file; import 'package:file/local.dart' as local_file; import 'package:meta/meta.dart'; @@ -18,7 +20,7 @@ import 'package:path/path.dart' as pkg_path; // without bringing in flutter as a dependency to the tests. class FileKeyValueStore { /// {@macro amplify_secure_storage_dart.file_key_value_store} - const FileKeyValueStore({ + FileKeyValueStore({ required this.path, required this.fileName, this.fs = const local_file.LocalFileSystem(), @@ -32,9 +34,14 @@ class FileKeyValueStore { /// The file will be created if it does not yet exist. final String fileName; + final TaskScheduler _scheduler = TaskScheduler(); + + final AWSLogger logger = AWSLogger('FileKeyValueStore'); + @visibleForTesting final pkg_file.FileSystem fs; + @visibleForTesting File get file => fs.file( pkg_path.join( path, @@ -42,17 +49,8 @@ class FileKeyValueStore { ), ); - /// Writes a single key to storage. - Future writeKey({ - required String key, - required Object value, - }) async { - final data = await readAll(); - data[key] = value; - return writeAll(data); - } - - /// Overwrites the existing data. + /// Overwrites the existing data in [file] with the key-value pairs in [data]. + @visibleForTesting Future writeAll( Map data, ) async { @@ -63,42 +61,104 @@ class FileKeyValueStore { await file.writeAsString(stringMap); } + /// Reads all the key-value pairs from [file]. + @visibleForTesting + Future> readAll() async { + if (await file.exists()) { + final stringMap = await file.readAsString(); + if (stringMap.isNotEmpty) { + try { + final Object? data = json.decode(stringMap); + if (data is Map) { + return data.cast(); + } + } on FormatException catch (e) { + logger.error( + 'Cannot read file. The file may be corrupted. ' + 'Clearing file contents. See error for more details. ' + 'Error: $e', + ); + await writeAll({}); + } + } + } + return {}; + } + + /// Writes a single key to storage. + Future writeKey({ + required String key, + required Object value, + }) async { + return _scheduler.schedule(() async { + final data = await readAll(); + data[key] = value; + return writeAll(data); + }); + } + /// Reads a single key from storage. Future readKey({ required String key, }) async { - final data = await readAll(); - return data[key]; + return _scheduler.schedule(() async { + final data = await readAll(); + return data[key]; + }); } /// Removes a single key from storage. Future removeKey({ required String key, }) async { - final data = await readAll(); - data.remove(key); - await writeAll(data); + return _scheduler.schedule(() async { + final data = await readAll(); + data.remove(key); + await writeAll(data); + }); } /// Returns true if the key exists in storage Future containsKey({ required String key, }) async { - final data = await readAll(); - return data.containsKey(key); + return _scheduler.schedule(() async { + final data = await readAll(); + return data.containsKey(key); + }); + } +} + +/// A class for processing async tasks one at a time in the order that they are +/// scheduled. +class TaskScheduler { + final _taskQueue = >[]; + bool _isProcessing = false; + Future schedule(Future Function() task) { + final completer = Completer(); + _taskQueue.add(Task(task, completer)); + _processTasks(); + return completer.future; } - /// Reads all the key-value pairs from storage. - Future> readAll() async { - if (await file.exists()) { - final stringMap = await file.readAsString(); - if (stringMap.isNotEmpty) { - final Object? data = json.decode(stringMap); - if (data is Map) { - return data.cast(); - } + Future _processTasks() async { + if (_isProcessing) return; + _isProcessing = true; + while (_taskQueue.isNotEmpty) { + final currentTask = _taskQueue.removeAt(0); + try { + final result = await currentTask.task(); + currentTask.completer.complete(result); + } on Object catch (e, stackTrace) { + currentTask.completer.completeError(e, stackTrace); } } - return {}; + _isProcessing = false; } } + +class Task { + Task(this.task, this.completer); + final Future Function() task; + final Completer completer; +} diff --git a/packages/secure_storage/amplify_secure_storage_test/test/file_key_value_store_test.dart b/packages/secure_storage/amplify_secure_storage_test/test/file_key_value_store_test.dart index 8fa16d436c..5f75aa4c37 100644 --- a/packages/secure_storage/amplify_secure_storage_test/test/file_key_value_store_test.dart +++ b/packages/secure_storage/amplify_secure_storage_test/test/file_key_value_store_test.dart @@ -3,83 +3,145 @@ @TestOn('vm') +import 'dart:io'; + import 'package:amplify_secure_storage_dart/src/utils/file_key_value_store.dart'; +import 'package:file/local.dart'; import 'package:file/memory.dart'; import 'package:test/test.dart'; void main() { late FileKeyValueStore storage; - group('FileKeyValueStore', () { - setUp(() { - storage = FileKeyValueStore( - path: 'path', - fileName: 'file', - fs: MemoryFileSystem(), - ); - }); - test('readKey & writeKey', () async { - // assert initial state is null - final value1 = await storage.readKey(key: 'key'); - expect(value1, isNull); + final fileSystems = [MemoryFileSystem(), const LocalFileSystem()]; - // write key-value pair - await storage.writeKey(key: 'key', value: 'value'); + for (final fileSystem in fileSystems) { + group('FileKeyValueStore (${fileSystem.runtimeType})', () { + setUp(() { + storage = FileKeyValueStore( + path: 'path', + fileName: 'file', + fs: fileSystem, + ); + }); - // assert value is updated - final value2 = await storage.readKey(key: 'key'); - expect(value2, 'value'); - }); + tearDown(() async { + await storage.file.delete(); + }); - test('removeKey', () async { - // seed storage & assert value is present - await storage.writeKey(key: 'key', value: 'value'); - final value1 = await storage.readKey(key: 'key'); - expect(value1, 'value'); + test('readKey & writeKey', () async { + // assert initial state is null + final value1 = await storage.readKey(key: 'key'); + expect(value1, isNull); - // remove key - await storage.removeKey(key: 'key'); + // write key-value pair + await storage.writeKey(key: 'key', value: 'value'); - // assert key is removed - final value2 = await storage.readKey(key: 'key'); - expect(value2, isNull); - }); + // assert value is updated + final value2 = await storage.readKey(key: 'key'); + expect(value2, 'value'); + }); - test('readAll', () async { - // write key-value pairs - await storage.writeKey(key: 'key1', value: 'value1'); - await storage.writeKey(key: 'key2', value: 'value2'); + test('removeKey', () async { + // seed storage & assert value is present + await storage.writeKey(key: 'key', value: 'value'); + final value1 = await storage.readKey(key: 'key'); + expect(value1, 'value'); - // assert values are updated - final data = await storage.readAll(); - expect(data['key1'], 'value1'); - expect(data['key2'], 'value2'); - }); + // remove key + await storage.removeKey(key: 'key'); - test('writeAll', () async { - // write key-value pairs - await storage.writeAll({ - 'key1': 'value1', - 'key2': 'value2', + // assert key is removed + final value2 = await storage.readKey(key: 'key'); + expect(value2, isNull); }); - // assert values are updated - final data = await storage.readAll(); - expect(data['key1'], 'value1'); - expect(data['key2'], 'value2'); - }); + test('readAll', () async { + // write key-value pairs + await storage.writeKey(key: 'key1', value: 'value1'); + await storage.writeKey(key: 'key2', value: 'value2'); - test('includes', () async { - // write key-value pair - await storage.writeKey(key: 'key1', value: 'value1'); + // assert values are updated + final data = await storage.readAll(); + expect(data['key1'], 'value1'); + expect(data['key2'], 'value2'); + }); + + test('writeAll', () async { + // write key-value pairs + await storage.writeAll({ + 'key1': 'value1', + 'key2': 'value2', + }); + + // assert values are updated + final data = await storage.readAll(); + expect(data['key1'], 'value1'); + expect(data['key2'], 'value2'); + }); - // assert that existing key returns true - final includesKey1 = await storage.containsKey(key: 'key1'); - expect(includesKey1, isTrue); + test('includes', () async { + // write key-value pair + await storage.writeKey(key: 'key1', value: 'value1'); - // assert that a non existing key returns false - final includesKey2 = await storage.containsKey(key: 'key2'); - expect(includesKey2, isFalse); + // assert that existing key returns true + final includesKey1 = await storage.containsKey(key: 'key1'); + expect(includesKey1, isTrue); + + // assert that a non existing key returns false + final includesKey2 = await storage.containsKey(key: 'key2'); + expect(includesKey2, isFalse); + }); + + group('parallel operations', () { + final items = List.generate(1000, ((i) => i)); + + test('should occur in the order they are called', () async { + final futures = items.map( + (i) async => storage.writeKey(key: 'key', value: i), + ); + await Future.wait(futures); + final value = await storage.readKey(key: 'key'); + expect(value, items.last); + }); + + test('should not result in stale data written to the file', () async { + final futures = items.map( + (i) async => storage.writeKey(key: 'key_$i', value: i), + ); + await Future.wait(futures); + for (final i in items) { + final value = await storage.readKey(key: 'key_$i'); + expect(value, items[i]); + } + }); + + // Reference: https://github.com/aws-amplify/amplify-flutter/issues/5190 + test('should not corrupt the file', () async { + final futures = items.map( + (i) async { + if (i % 5 == 1) { + await storage.removeKey(key: 'key_${i - 1}'); + } + return storage.writeKey(key: 'key_$i', value: 'value_$i'); + }, + ); + await Future.wait(futures); + }); + }); + + test('File is cleared when corrupted and can be re-written to', () async { + await storage.writeKey(key: 'foo', value: 'value'); + final value1 = await storage.readKey(key: 'foo'); + expect(value1, 'value'); + await storage.file + .writeAsString('{invalid json}', mode: FileMode.append); + final value2 = await storage.readKey(key: 'foo'); + expect(value2, null); + await storage.writeKey(key: 'foo', value: 'value'); + final value3 = await storage.readKey(key: 'foo'); + expect(value3, 'value'); + }); }); - }); + } }