From 4ba6e2568955c9d1a7f1cf434d358f2ee09b4f15 Mon Sep 17 00:00:00 2001 From: The one with the braid Date: Thu, 4 Apr 2024 11:34:32 +0200 Subject: [PATCH 1/3] fix: dart:io import in matrix_sdk_database - removes `Directory` field in high-level `MatrixSdkDatabase` - migrates `Directory fileStoragePath` to `Uri fileStorageLocation` - makes file operations in `MatrixSdkDatabase` conditional on `dart.libraries.js_util` - implements a tiny stub of the file operations used in `MatrixSdkDatabase` It seems like the Flutter tool can compile despite these imports. Sadly the Dart standalone dart2js compiler doesn't reach there. While refactorying the code, I decided it's likely cleaner to have a `Uri` as storage location provider than using some fake directory or String as relacement. The advantage of a `Uri` at this place is the explicit `Uri.directory` constructor available to ensure type and encoding safe directory locations supporting both Windows and *nix. Additionally, admitted, that's an edge-case, one could even easily extend the use of a `Uri` based descriptor to support future storage location accesses (e.g. IPFS or custom schemes for e.g. local web browser based file system APIs). Using a `Uri`, one would only need to override the three methods making use of the `fileStorageLocation` property to handle different Uri schemes too. Signed-off-by: The one with the braid --- .../database/filesystem/fake_filesystem.dart | 37 ++++++++++++++ .../database/filesystem/io_filesystem.dart | 1 + lib/src/database/matrix_sdk_database.dart | 50 +++++++++++++------ 3 files changed, 73 insertions(+), 15 deletions(-) create mode 100644 lib/src/database/filesystem/fake_filesystem.dart create mode 100644 lib/src/database/filesystem/io_filesystem.dart diff --git a/lib/src/database/filesystem/fake_filesystem.dart b/lib/src/database/filesystem/fake_filesystem.dart new file mode 100644 index 00000000..098b0ef2 --- /dev/null +++ b/lib/src/database/filesystem/fake_filesystem.dart @@ -0,0 +1,37 @@ +// This is a stub implementation of all filesystem related calls done +// by the matrix SDK database. This fake implementation ensures we can compile +// using dart2js. + +import 'dart:typed_data'; + +class File extends FileSystemEntry { + const File(super.path); + + Future readAsBytes() async => Uint8List(0); + + Future writeAsBytes(Uint8List data) async => Future.value(); +} + +class Directory extends FileSystemEntry { + const Directory(super.path); + + Stream list() async* { + return; + } +} + +abstract class FileSystemEntry { + final String path; + + const FileSystemEntry(this.path); + + Future delete() => Future.value(); + + Future stat() async => FileStat(); + + Future exists() async => false; +} + +class FileStat { + final modified = DateTime.fromMillisecondsSinceEpoch(0); +} diff --git a/lib/src/database/filesystem/io_filesystem.dart b/lib/src/database/filesystem/io_filesystem.dart new file mode 100644 index 00000000..047f132b --- /dev/null +++ b/lib/src/database/filesystem/io_filesystem.dart @@ -0,0 +1 @@ +export 'dart:io'; diff --git a/lib/src/database/matrix_sdk_database.dart b/lib/src/database/matrix_sdk_database.dart index 4a230769..83df4307 100644 --- a/lib/src/database/matrix_sdk_database.dart +++ b/lib/src/database/matrix_sdk_database.dart @@ -18,7 +18,6 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:io'; import 'dart:math'; import 'dart:typed_data'; @@ -100,11 +99,28 @@ class MatrixSdkDatabase extends DatabaseApi { late Box _seenDeviceIdsBox; late Box _seenDeviceKeysBox; + @override - bool get supportsFileStoring => fileStoragePath != null; + bool get supportsFileStoring => fileStorageLocation != null; @override final int maxFileSize; - final Directory? fileStoragePath; + + // there was a field of type `dart:io:Directory` here. This one broke the + // dart js standalone compiler. Migration via URI as file system identifier. + @Deprecated( + 'Breaks support for web standalone. Use [fileStorageLocation] instead.') + Object? get fileStoragePath => fileStorageLocation?.toFilePath(); + + /// A [Uri] defining the file storage location. + /// + /// Unless you support custom Uri schemes, this should usually be a + /// [Uri.directory] identifier. + /// + /// Using a [Uri] as type here enables users to technically extend the API to + /// support file storage on non-io platforms as well - or to use non-File + /// based storage mechanisms on io. + final Uri? fileStorageLocation; + final Duration? deleteFilesAfterDuration; static const String _clientBoxName = 'box_client'; @@ -168,9 +184,11 @@ class MatrixSdkDatabase extends DatabaseApi { this.idbFactory, this.sqfliteFactory, this.maxFileSize = 0, - this.fileStoragePath, + // TODO : remove deprecated member migration on next major release + dynamic fileStoragePath, + Uri? fileStorageLocation, this.deleteFilesAfterDuration, - }); + }) : fileStorageLocation = fileStorageLocation ?? fileStoragePath?.path; Future open() async { _collection = await BoxCollection.open( @@ -311,13 +329,15 @@ class MatrixSdkDatabase extends DatabaseApi { @override Future deleteOldFiles(int savedAt) async { - final dir = fileStoragePath; + if (_kIsWeb) return; + final path = fileStorageLocation?.toFilePath(); final deleteFilesAfterDuration = this.deleteFilesAfterDuration; if (!supportsFileStoring || - dir == null || + path == null || deleteFilesAfterDuration == null) { return; } + final dir = Directory(path); final entities = await dir.list().toList(); for (final file in entities) { if (file is! File) continue; @@ -453,11 +473,11 @@ class MatrixSdkDatabase extends DatabaseApi { @override Future getFile(Uri mxcUri) async { - final fileStoragePath = this.fileStoragePath; - if (!supportsFileStoring || fileStoragePath == null) return null; + if (_kIsWeb) return null; + final path = fileStorageLocation?.toFilePath(); + if (!supportsFileStoring || path == null) return null; - final file = - File('${fileStoragePath.path}/${mxcUri.toString().split('/').last}'); + final file = File('$path/${mxcUri.toString().split('/').last}'); if (await file.exists()) return await file.readAsBytes(); return null; @@ -1138,11 +1158,11 @@ class MatrixSdkDatabase extends DatabaseApi { @override Future storeFile(Uri mxcUri, Uint8List bytes, int time) async { - final fileStoragePath = this.fileStoragePath; - if (!supportsFileStoring || fileStoragePath == null) return; + if (_kIsWeb) return; + final path = fileStorageLocation?.toFilePath(); + if (!supportsFileStoring || path == null) return; - final file = - File('${fileStoragePath.path}/${mxcUri.toString().split('/').last}'); + final file = File('$path/${mxcUri.toString().split('/').last}'); if (await file.exists()) return; await file.writeAsBytes(bytes); From 87c0e7fbe8701feef04ab219d96b73ed3f82efe1 Mon Sep 17 00:00:00 2001 From: The one with the braid Date: Thu, 4 Apr 2024 12:29:30 +0200 Subject: [PATCH 2/3] feat: add Web build test Signed-off-by: The one with the braid --- .github/workflows/app.yml | 5 +++++ web_test/.gitignore | 3 +++ web_test/README.md | 1 + web_test/pubspec.yaml | 17 +++++++++++++++++ web_test/web/main.dart | 6 ++++++ 5 files changed, 32 insertions(+) create mode 100644 web_test/.gitignore create mode 100644 web_test/README.md create mode 100644 web_test/pubspec.yaml create mode 100644 web_test/web/main.dart diff --git a/.github/workflows/app.yml b/.github/workflows/app.yml index ceba9223..6640508e 100644 --- a/.github/workflows/app.yml +++ b/.github/workflows/app.yml @@ -47,6 +47,11 @@ jobs: rm -r example ./scripts/prepare.sh ./scripts/test.sh + - name: Ensure SDK compiles on web + run: | + pushd web_test + dart pub get + dart run webdev build coverage_without_olm: runs-on: ubuntu-latest diff --git a/web_test/.gitignore b/web_test/.gitignore new file mode 100644 index 00000000..3a857904 --- /dev/null +++ b/web_test/.gitignore @@ -0,0 +1,3 @@ +# https://dart.dev/guides/libraries/private-files +# Created by `dart pub` +.dart_tool/ diff --git a/web_test/README.md b/web_test/README.md new file mode 100644 index 00000000..c1528cc7 --- /dev/null +++ b/web_test/README.md @@ -0,0 +1 @@ +This is a bare-bone sample project in order to ensure webdev can compile the SDK. \ No newline at end of file diff --git a/web_test/pubspec.yaml b/web_test/pubspec.yaml new file mode 100644 index 00000000..af7b3090 --- /dev/null +++ b/web_test/pubspec.yaml @@ -0,0 +1,17 @@ +name: web_test +description: A test project for the webdev compiler. +version: 1.0.0 +publish_to: none + +environment: + sdk: ^3.2.0 + +# Add regular dependencies here. +dependencies: + matrix: + path: .. + +dev_dependencies: + build_runner: ^2.4.9 + build_web_compilers: ^4.0.10 + webdev: ^3.4.0 diff --git a/web_test/web/main.dart b/web_test/web/main.dart new file mode 100644 index 00000000..6873007d --- /dev/null +++ b/web_test/web/main.dart @@ -0,0 +1,6 @@ +import 'package:matrix/matrix.dart'; + +Future main() async { + final client = Client('web_test'); + await client.init(); +} From 49e6d55d323ac82bf994c5b1985fc1bd849d476e Mon Sep 17 00:00:00 2001 From: Krille Date: Thu, 11 Apr 2024 14:54:20 +0200 Subject: [PATCH 3/3] refactor: Move file storage to mixin to not import dart:io Original issue and former solution by The one with the braid Special thanks for pointing out the problem. This fixes that dart:io is imported into the SDK database by moving it into it's own mixin which makes it reusable and platform independent by using conditional imports. --- .../database/database_file_storage_io.dart | 55 +++++++++++++ .../database/database_file_storage_stub.dart | 20 +++++ .../database/filesystem/fake_filesystem.dart | 37 --------- .../database/filesystem/io_filesystem.dart | 1 - lib/src/database/matrix_sdk_database.dart | 77 ++++--------------- 5 files changed, 88 insertions(+), 102 deletions(-) create mode 100644 lib/src/database/database_file_storage_io.dart create mode 100644 lib/src/database/database_file_storage_stub.dart delete mode 100644 lib/src/database/filesystem/fake_filesystem.dart delete mode 100644 lib/src/database/filesystem/io_filesystem.dart diff --git a/lib/src/database/database_file_storage_io.dart b/lib/src/database/database_file_storage_io.dart new file mode 100644 index 00000000..6a80efd7 --- /dev/null +++ b/lib/src/database/database_file_storage_io.dart @@ -0,0 +1,55 @@ +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:matrix/matrix.dart'; + +mixin DatabaseFileStorage { + bool get supportsFileStoring => fileStorageLocation != null; + + late final Uri? fileStorageLocation; + late final Duration? deleteFilesAfterDuration; + + Future storeFile(Uri mxcUri, Uint8List bytes, int time) async { + final fileStorageLocation = this.fileStorageLocation; + if (!supportsFileStoring || fileStorageLocation == null) return; + + final dir = Directory.fromUri(fileStorageLocation); + + final file = File('${dir.path}/${mxcUri.toString().split('/').last}'); + + if (await file.exists()) return; + await file.writeAsBytes(bytes); + } + + Future getFile(Uri mxcUri) async { + final fileStorageLocation = this.fileStorageLocation; + if (!supportsFileStoring || fileStorageLocation == null) return null; + + final dir = Directory.fromUri(fileStorageLocation); + + final file = File('${dir.path}/${mxcUri.toString().split('/').last}'); + + if (await file.exists()) return await file.readAsBytes(); + return null; + } + + Future deleteOldFiles(int savedAt) async { + final dirUri = fileStorageLocation; + final deleteFilesAfterDuration = this.deleteFilesAfterDuration; + if (!supportsFileStoring || + dirUri == null || + deleteFilesAfterDuration == null) { + return; + } + final dir = Directory.fromUri(dirUri); + final entities = await dir.list().toList(); + for (final file in entities) { + if (file is! File) continue; + final stat = await file.stat(); + if (DateTime.now().difference(stat.modified) > deleteFilesAfterDuration) { + Logs().v('Delete old file', file.path); + await file.delete(); + } + } + } +} diff --git a/lib/src/database/database_file_storage_stub.dart b/lib/src/database/database_file_storage_stub.dart new file mode 100644 index 00000000..7addfe1a --- /dev/null +++ b/lib/src/database/database_file_storage_stub.dart @@ -0,0 +1,20 @@ +import 'dart:typed_data'; + +mixin DatabaseFileStorage { + bool get supportsFileStoring => false; + + late final Uri? fileStorageLocation; + late final Duration? deleteFilesAfterDuration; + + Future storeFile(Uri mxcUri, Uint8List bytes, int time) async { + return; + } + + Future getFile(Uri mxcUri) async { + return null; + } + + Future deleteOldFiles(int savedAt) async { + return; + } +} diff --git a/lib/src/database/filesystem/fake_filesystem.dart b/lib/src/database/filesystem/fake_filesystem.dart deleted file mode 100644 index 098b0ef2..00000000 --- a/lib/src/database/filesystem/fake_filesystem.dart +++ /dev/null @@ -1,37 +0,0 @@ -// This is a stub implementation of all filesystem related calls done -// by the matrix SDK database. This fake implementation ensures we can compile -// using dart2js. - -import 'dart:typed_data'; - -class File extends FileSystemEntry { - const File(super.path); - - Future readAsBytes() async => Uint8List(0); - - Future writeAsBytes(Uint8List data) async => Future.value(); -} - -class Directory extends FileSystemEntry { - const Directory(super.path); - - Stream list() async* { - return; - } -} - -abstract class FileSystemEntry { - final String path; - - const FileSystemEntry(this.path); - - Future delete() => Future.value(); - - Future stat() async => FileStat(); - - Future exists() async => false; -} - -class FileStat { - final modified = DateTime.fromMillisecondsSinceEpoch(0); -} diff --git a/lib/src/database/filesystem/io_filesystem.dart b/lib/src/database/filesystem/io_filesystem.dart deleted file mode 100644 index 047f132b..00000000 --- a/lib/src/database/filesystem/io_filesystem.dart +++ /dev/null @@ -1 +0,0 @@ -export 'dart:io'; diff --git a/lib/src/database/matrix_sdk_database.dart b/lib/src/database/matrix_sdk_database.dart index 83df4307..96b6e28e 100644 --- a/lib/src/database/matrix_sdk_database.dart +++ b/lib/src/database/matrix_sdk_database.dart @@ -19,7 +19,6 @@ import 'dart:async'; import 'dart:convert'; import 'dart:math'; -import 'dart:typed_data'; import 'package:sqflite_common/sqflite.dart'; @@ -35,6 +34,9 @@ import 'package:matrix/src/utils/run_benchmarked.dart'; import 'package:matrix/src/database/indexeddb_box.dart' if (dart.library.io) 'package:matrix/src/database/sqflite_box.dart'; +import 'package:matrix/src/database/database_file_storage_stub.dart' + if (dart.library.io) 'package:matrix/src/database/database_file_storage_io.dart'; + /// Database based on SQlite3 on native and IndexedDB on web. For native you /// have to pass a `Database` object, which can be created with the sqflite /// package like this: @@ -49,7 +51,7 @@ import 'package:matrix/src/database/indexeddb_box.dart' /// [sqflite_common_ffi](https://pub.dev/packages/sqflite_common_ffi). /// Learn more at: /// https://github.com/famedly/matrix-dart-sdk/issues/1642#issuecomment-1865827227 -class MatrixSdkDatabase extends DatabaseApi { +class MatrixSdkDatabase extends DatabaseApi with DatabaseFileStorage { static const int version = 7; final String name; late BoxCollection _collection; @@ -100,8 +102,6 @@ class MatrixSdkDatabase extends DatabaseApi { late Box _seenDeviceKeysBox; - @override - bool get supportsFileStoring => fileStorageLocation != null; @override final int maxFileSize; @@ -111,18 +111,6 @@ class MatrixSdkDatabase extends DatabaseApi { 'Breaks support for web standalone. Use [fileStorageLocation] instead.') Object? get fileStoragePath => fileStorageLocation?.toFilePath(); - /// A [Uri] defining the file storage location. - /// - /// Unless you support custom Uri schemes, this should usually be a - /// [Uri.directory] identifier. - /// - /// Using a [Uri] as type here enables users to technically extend the API to - /// support file storage on non-io platforms as well - or to use non-File - /// based storage mechanisms on io. - final Uri? fileStorageLocation; - - final Duration? deleteFilesAfterDuration; - static const String _clientBoxName = 'box_client'; static const String _accountDataBoxName = 'box_account_data'; @@ -185,10 +173,17 @@ class MatrixSdkDatabase extends DatabaseApi { this.sqfliteFactory, this.maxFileSize = 0, // TODO : remove deprecated member migration on next major release + @Deprecated( + 'Breaks support for web standalone. Use [fileStorageLocation] instead.') dynamic fileStoragePath, Uri? fileStorageLocation, - this.deleteFilesAfterDuration, - }) : fileStorageLocation = fileStorageLocation ?? fileStoragePath?.path; + Duration? deleteFilesAfterDuration, + }) { + final legacyPath = fileStoragePath?.path; + this.fileStorageLocation = fileStorageLocation ?? + (legacyPath is String ? Uri.tryParse(legacyPath) : null); + this.deleteFilesAfterDuration = deleteFilesAfterDuration; + } Future open() async { _collection = await BoxCollection.open( @@ -327,28 +322,6 @@ class MatrixSdkDatabase extends DatabaseApi { return; } - @override - Future deleteOldFiles(int savedAt) async { - if (_kIsWeb) return; - final path = fileStorageLocation?.toFilePath(); - final deleteFilesAfterDuration = this.deleteFilesAfterDuration; - if (!supportsFileStoring || - path == null || - deleteFilesAfterDuration == null) { - return; - } - final dir = Directory(path); - final entities = await dir.list().toList(); - for (final file in entities) { - if (file is! File) continue; - final stat = await file.stat(); - if (DateTime.now().difference(stat.modified) > deleteFilesAfterDuration) { - Logs().v('Delete old file', file.path); - await file.delete(); - } - } - } - @override Future forgetRoom(String roomId) async { await _timelineFragmentsBox.delete(TupleKey(roomId, '').toString()); @@ -471,18 +444,6 @@ class MatrixSdkDatabase extends DatabaseApi { return await _getEventsByIds(eventIds.cast(), room); }); - @override - Future getFile(Uri mxcUri) async { - if (_kIsWeb) return null; - final path = fileStorageLocation?.toFilePath(); - if (!supportsFileStoring || path == null) return null; - - final file = File('$path/${mxcUri.toString().split('/').last}'); - - if (await file.exists()) return await file.readAsBytes(); - return null; - } - @override Future getInboundGroupSession( String roomId, @@ -1156,18 +1117,6 @@ class MatrixSdkDatabase extends DatabaseApi { } } - @override - Future storeFile(Uri mxcUri, Uint8List bytes, int time) async { - if (_kIsWeb) return; - final path = fileStorageLocation?.toFilePath(); - if (!supportsFileStoring || path == null) return; - - final file = File('$path/${mxcUri.toString().split('/').last}'); - - if (await file.exists()) return; - await file.writeAsBytes(bytes); - } - @override Future storeInboundGroupSession( String roomId,