split native and web tests. Add zone gaurds to web connections

powersync-ja · Feb 1, 2024 · 1243a5d · 1243a5d
1 parent 2892d21
commit 1243a5d
Show file tree

Hide file tree

Showing 8 changed files with 303 additions and 127 deletions.
diff --git a/lib/src/web/database/web_sqlite_connection_impl.dart b/lib/src/web/database/web_sqlite_connection_impl.dart
@@ -1,5 +1,6 @@
 import 'dart:async';
 import 'package:meta/meta.dart';
+import 'package:sqlite_async/src/common/abstract_mutex.dart';
 import 'package:sqlite_async/src/common/abstract_open_factory.dart';
 
 import 'package:sqlite_async/src/sqlite_connection.dart';
@@ -51,14 +52,35 @@ class WebSqliteConnectionImpl with SqliteQueries implements SqliteConnection {
   Future<T> readLock<T>(Future<T> Function(SqliteReadContext tx) callback,
       {Duration? lockTimeout, String? debugContext}) async {
     await isInitialized;
-    return mutex.lock(() => callback(WebReadContext(executor!)));
+    return _runZoned(
+        () => mutex.lock(() => callback(WebReadContext(executor!)),
+            timeout: lockTimeout),
+        debugContext: debugContext ?? 'execute()');
   }
 
   @override
   Future<T> writeLock<T>(Future<T> Function(SqliteWriteContext tx) callback,
       {Duration? lockTimeout, String? debugContext}) async {
     await isInitialized;
-    return mutex.lock(() => callback(WebWriteContext(executor!)));
+    return _runZoned(
+        () => mutex.lock(() => callback(WebWriteContext(executor!)),
+            timeout: lockTimeout),
+        debugContext: debugContext ?? 'execute()');
+  }
+
+  /// The [Mutex] on individual connections do already error in recursive locks.
+  ///
+  /// We duplicate the same check here, to:
+  /// 1. Also error when the recursive transaction is handled by a different
+  ///    connection (with a different lock).
+  /// 2. Give a more specific error message when it happens.
+  T _runZoned<T>(T Function() callback, {required String debugContext}) {
+    if (Zone.current[this] != null) {
+      throw LockError(
+          'Recursive lock is not allowed. Use `tx.$debugContext` instead of `db.$debugContext`.');
+    }
+    var zone = Zone.current.fork(zoneValues: {this: true});
+    return zone.run(callback);
   }
 
   @override

diff --git a/test/basic_test.dart → test/basic_native_test.dart b/test/basic_test.dart → test/basic_native_test.dart
@@ -1,3 +1,4 @@
+@TestOn('!browser')
 import 'dart:async';
 import 'dart:math';
 
@@ -16,7 +17,6 @@ void main() {
 
     setUp(() async {
       path = testUtils.dbPath();
-      await testUtils.init();
       await testUtils.cleanDb(path: path);
     });
 
@@ -53,7 +53,8 @@ void main() {
 
     // Manually verified
     test('Concurrency', () async {
-      final db = SqliteDatabase.withFactory(testUtils.testFactory(path: path),
+      final db = SqliteDatabase.withFactory(
+          await testUtils.testFactory(path: path),
           maxReaders: 3);
       await db.initialize();
       await createTables(db);

diff --git a/test/basic_shared_test.dart b/test/basic_shared_test.dart
@@ -0,0 +1,83 @@
+import 'dart:async';
+import 'package:sqlite_async/mutex.dart';
+import 'package:sqlite_async/sqlite_async.dart';
+import 'package:test/test.dart';
+
+import 'utils/test_utils_impl.dart';
+
+final testUtils = TestUtils();
+
+void main() {
+  group('Shared Basic Tests', () {
+    late String path;
+
+    setUp(() async {
+      path = testUtils.dbPath();
+      await testUtils.cleanDb(path: path);
+    });
+
+    tearDown(() async {
+      await testUtils.cleanDb(path: path);
+    });
+
+    createTables(SqliteDatabase db) async {
+      await db.writeTransaction((tx) async {
+        await tx.execute(
+            'CREATE TABLE test_data(id INTEGER PRIMARY KEY AUTOINCREMENT, description TEXT)');
+      });
+    }
+
+    test('should not allow direct db calls within a transaction callback',
+        () async {
+      final db = await testUtils.setupDatabase(path: path);
+      await createTables(db);
+
+      await db.writeTransaction((tx) async {
+        await expectLater(() async {
+          await db.execute(
+              'INSERT INTO test_data(description) VALUES(?)', ['test']);
+        }, throwsA((e) => e is LockError && e.message.contains('tx.execute')));
+      });
+    });
+
+    test('should allow PRAMGAs', () async {
+      final db = await testUtils.setupDatabase(path: path);
+      await createTables(db);
+      // Not allowed in transactions, but does work as a direct statement.
+      await db.execute('PRAGMA wal_checkpoint(TRUNCATE)');
+      await db.execute('VACUUM');
+    });
+
+    test('should allow ignoring errors', () async {
+      final db = await testUtils.setupDatabase(path: path);
+      await createTables(db);
+
+      ignore(db.execute(
+          'INSERT INTO test_data(description) VALUES(json(?))', ['test3']));
+    });
+
+    test('should handle normal errors', () async {
+      final db = await testUtils.setupDatabase(path: path);
+      await createTables(db);
+      Error? caughtError;
+      final syntheticError = ArgumentError('foobar');
+      await db.writeLock<void>((db) async {
+        throw syntheticError;
+      }).catchError((error) {
+        caughtError = error;
+      });
+      expect(caughtError.toString(), equals(syntheticError.toString()));
+
+      // Check that we can still continue afterwards
+      final computed = await db.writeLock((db) async {
+        return 5;
+      });
+      expect(computed, equals(5));
+    });
+  });
+}
+
+// For some reason, future.ignore() doesn't actually ignore errors in these tests.
+void ignore(Future future) {
+  future.then((_) {}, onError: (_) {});
+}
diff --git a/test/utils/abstract_test_utils.dart b/test/utils/abstract_test_utils.dart
@@ -19,23 +19,21 @@ abstract class AbstractTestUtils {
   }
 
   /// Generates a test open factory
-  TestDefaultSqliteOpenFactory testFactory(
+  Future<TestDefaultSqliteOpenFactory> testFactory(
       {String? path,
       String sqlitePath = '',
-      SqliteOptions options = const SqliteOptions.defaults()}) {
+      SqliteOptions options = const SqliteOptions.defaults()}) async {
     return TestDefaultSqliteOpenFactory(
         path: path ?? dbPath(), sqliteOptions: options);
   }
 
   /// Creates a SqliteDatabaseConnection
   Future<SqliteDatabase> setupDatabase({String? path}) async {
-    final db = SqliteDatabase.withFactory(testFactory(path: path));
+    final db = SqliteDatabase.withFactory(await testFactory(path: path));
     await db.initialize();
     return db;
   }
 
-  Future<void> init();
-
   /// Deletes any DB data
   Future<void> cleanDb({required String path});
 

diff --git a/test/utils/native_test_utils.dart b/test/utils/native_test_utils.dart
@@ -49,9 +49,6 @@ class TestSqliteOpenFactory extends TestDefaultSqliteOpenFactory {
 }
 
 class TestUtils extends AbstractTestUtils {
-  @override
-  Future<void> init() async {}
-
   @override
   String dbPath() {
     Directory("test-db").createSync(recursive: false);
@@ -88,10 +85,10 @@ class TestUtils extends AbstractTestUtils {
   }
 
   @override
-  TestDefaultSqliteOpenFactory testFactory(
+  Future<TestDefaultSqliteOpenFactory> testFactory(
       {String? path,
       String sqlitePath = defaultSqlitePath,
-      SqliteOptions options = const SqliteOptions.defaults()}) {
+      SqliteOptions options = const SqliteOptions.defaults()}) async {
     return TestSqliteOpenFactory(
         path: path ?? dbPath(), sqlitePath: sqlitePath, sqliteOptions: options);
   }

diff --git a/test/utils/web_test_utils.dart b/test/utils/web_test_utils.dart
@@ -1,43 +1,51 @@
 import 'dart:async';
+import 'dart:html';
 
+import 'package:js/js.dart';
 import 'package:sqlite_async/sqlite_async.dart';
 import 'package:test/test.dart';
 import 'abstract_test_utils.dart';
 
+@JS('URL.createObjectURL')
+external String _createObjectURL(Blob blob);
+
 class TestUtils extends AbstractTestUtils {
-  late final Future<void> _isInitialized;
-  late SqliteOptions? webOptions;
+  late Future<void> _isInitialized;
+  late final SqliteOptions webOptions;
 
   TestUtils() {
-    _isInitialized = init();
+    _isInitialized = _init();
   }
 
-  @override
-  Future<void> init() async {
-    if (webOptions != null) {
-      return;
-    }
-
+  Future<void> _init() async {
     final channel = spawnHybridUri('/test/server/asset_server.dart');
     final port = await channel.stream.first as int;
 
-    final sqliteWasm = Uri.parse('http://localhost:$port/sqlite3.wasm');
-    final sqliteDrift = Uri.parse('http://localhost:$port/drift_worker.js');
+    final sqliteWasmUri = Uri.parse('http://localhost:$port/sqlite3.wasm');
+
+    // Cross origin workers are not supported, but we can supply a Blob
+    var sqliteDriftUri =
+        Uri.parse('http://localhost:$port/drift_worker.js').toString();
+    final blob = Blob(<String>['importScripts("$sqliteDriftUri");'],
+        'application/javascript');
+    sqliteDriftUri = _createObjectURL(blob);
 
     webOptions = SqliteOptions(
         webSqliteOptions: WebSqliteOptions(
-            wasmUri: sqliteWasm.toString(), workerUri: sqliteDrift.toString()));
+            wasmUri: sqliteWasmUri.toString(),
+            workerUri: sqliteDriftUri.toString()));
   }
 
   @override
   Future<void> cleanDb({required String path}) async {}
 
   @override
-  TestDefaultSqliteOpenFactory testFactory(
+  Future<TestDefaultSqliteOpenFactory> testFactory(
       {String? path,
       String? sqlitePath,
-      SqliteOptions options = const SqliteOptions.defaults()}) {
-    return super.testFactory(path: path, options: webOptions!);
+      SqliteOptions options = const SqliteOptions.defaults()}) async {
+    await _isInitialized;
+    return super.testFactory(path: path, options: webOptions);
   }
 
   @override