Refactor APIs to lock() and transaction().

Author: rkistner

lib/src/connection_pool.dart

@@ -132,6 +132,7 @@ class SqliteConnectionPool with SqliteQueries implements SqliteConnection {
         readOnly: false,
         openFactory: _factory);
     return _runZoned(() {
+      // ignore: deprecated_member_use_from_same_package
       return _writeConnection!.writeLock(callback,
           lockTimeout: lockTimeout, debugContext: debugContext);
     }, debugContext: debugContext ?? 'execute()');

lib/src/sqlite_connection.dart

@@ -72,11 +72,26 @@ abstract class SqliteWriteContext extends SqliteReadContext {
 
 /// Abstract class representing a connection to the SQLite database.
 abstract class SqliteConnection extends SqliteWriteContext {
+  /// Open a transaction.
+  ///
+  /// Opens a read-write transaction by default. Set [readOnly] to open a
+  /// read-only transaction.
+  ///
+  /// Only one read-write transaction can execute against the database at a time.
+  ///
+  /// Statements within the transaction must be done on the provided
+  /// [SqliteWriteContext] - attempting statements on the [SqliteConnection]
+  /// instance will error.
+  Future<T> transaction<T>(Future<T> Function(SqliteWriteContext tx) callback,
+      {bool? readOnly});
+
   /// Open a read-only transaction.
   ///
   /// Statements within the transaction must be done on the provided
   /// [SqliteReadContext] - attempting statements on the [SqliteConnection]
   /// instance will error.
+  @Deprecated(
+      'Use [transaction(callback)] instead, and set lockTimeout on [SqliteOpenOptions].')
   Future<T> readTransaction<T>(
       Future<T> Function(SqliteReadContext tx) callback,
       {Duration? lockTimeout});
@@ -89,6 +104,8 @@ abstract class SqliteConnection extends SqliteWriteContext {
   /// Statements within the transaction must be done on the provided
   /// [SqliteWriteContext] - attempting statements on the [SqliteConnection]
   /// instance will error.
+  @Deprecated(
+      'Use [transaction(callback)] instead, and set lockTimeout on [SqliteOpenOptions].')
   Future<T> writeTransaction<T>(
       Future<T> Function(SqliteWriteContext tx) callback,
       {Duration? lockTimeout});
@@ -102,18 +119,31 @@ abstract class SqliteConnection extends SqliteWriteContext {
       {List<Object?> parameters = const [],
       Duration throttle = const Duration(milliseconds: 30)});
 
-  /// Takes a read lock, without starting a transaction.
+  /// Takes a read lock on this connection, without starting a transaction.
   ///
-  /// In most cases, [readTransaction] should be used instead.
+  /// This is a low-level API. In most cases, [transaction] should be used instead.
+  @Deprecated('Use [lock] instead, and set lockTimeout on [SqliteOpenOptions].')
   Future<T> readLock<T>(Future<T> Function(SqliteReadContext tx) callback,
       {Duration? lockTimeout, String? debugContext});
 
-  /// Takes a global lock, without starting a transaction.
+  /// Takes a global write lock, without starting a transaction.
   ///
-  /// In most cases, [writeTransaction] should be used instead.
+  /// This is a low-level API. In most cases, [transaction] should be used instead.
+  @Deprecated('Use [lock] instead, and set lockTimeout on [SqliteOpenOptions].')
   Future<T> writeLock<T>(Future<T> Function(SqliteWriteContext tx) callback,
       {Duration? lockTimeout, String? debugContext});
 
+  /// Lock a connection for exclusive usage.
+  ///
+  /// When using a connection pool, use [readOnly] to select a read connection.
+  /// In other contexts, [readOnly] has no effect.
+  ///
+  /// This is a low-level API. In most cases, [transaction] should be used instead.
+  ///
+  /// Any direct query methods such as [getAll] or [execute] uses [lock] internally.
+  Future<T> lock<T>(Future<T> Function(SqliteWriteContext tx) callback,
+      {bool? readOnly, String? debugContext});
+
   Future<void> close();
 
   /// Returns true if the connection is closed

lib/src/sqlite_migrations.dart

@@ -90,7 +90,7 @@ class SqliteMigrations {
   Future<void> migrate(SqliteConnection db) async {
     _validateCreateDatabase();
 
-    await db.writeTransaction((tx) async {
+    await db.transaction((tx) async {
       await tx.execute(
           'CREATE TABLE IF NOT EXISTS $migrationTable(id INTEGER PRIMARY KEY, down_migrations TEXT)');
 

lib/src/sqlite_queries.dart

@@ -6,41 +6,40 @@ import 'update_notification.dart';
 
 /// Mixin to provide default query functionality.
 ///
-/// Classes using this need to implement [SqliteConnection.readLock]
-/// and [SqliteConnection.writeLock].
+/// Classes using this need to implement [SqliteConnection.lock].
 mixin SqliteQueries implements SqliteWriteContext, SqliteConnection {
   /// Broadcast stream that is notified of any table updates
   Stream<UpdateNotification>? get updates;
 
   @override
   Future<sqlite.ResultSet> execute(String sql,
       [List<Object?> parameters = const []]) async {
-    return writeLock((ctx) async {
+    return lock((ctx) async {
       return ctx.execute(sql, parameters);
     }, debugContext: 'execute()');
   }
 
   @override
   Future<sqlite.ResultSet> getAll(String sql,
       [List<Object?> parameters = const []]) {
-    return readLock((ctx) async {
+    return lock((ctx) async {
       return ctx.getAll(sql, parameters);
-    }, debugContext: 'getAll()');
+    }, readOnly: true, debugContext: 'getAll()');
   }
 
   @override
   Future<sqlite.Row> get(String sql, [List<Object?> parameters = const []]) {
-    return readLock((ctx) async {
+    return lock((ctx) async {
       return ctx.get(sql, parameters);
-    }, debugContext: 'get()');
+    }, readOnly: true, debugContext: 'get()');
   }
 
   @override
   Future<sqlite.Row?> getOptional(String sql,
       [List<Object?> parameters = const []]) {
-    return readLock((ctx) async {
+    return lock((ctx) async {
       return ctx.getOptional(sql, parameters);
-    }, debugContext: 'getOptional()');
+    }, readOnly: true, debugContext: 'getOptional()');
   }
 
   @override
@@ -102,6 +101,7 @@ mixin SqliteQueries implements SqliteWriteContext, SqliteConnection {
   Future<T> readTransaction<T>(
       Future<T> Function(SqliteReadContext tx) callback,
       {Duration? lockTimeout}) async {
+    // ignore: deprecated_member_use_from_same_package
     return readLock((ctx) async {
       return await internalReadTransaction(ctx, callback);
     }, lockTimeout: lockTimeout, debugContext: 'readTransaction()');
@@ -111,6 +111,7 @@ mixin SqliteQueries implements SqliteWriteContext, SqliteConnection {
   Future<T> writeTransaction<T>(
       Future<T> Function(SqliteWriteContext tx) callback,
       {Duration? lockTimeout}) async {
+    // ignore: deprecated_member_use_from_same_package
     return writeLock((ctx) async {
       return await internalWriteTransaction(ctx, callback);
     }, lockTimeout: lockTimeout, debugContext: 'writeTransaction()');
@@ -140,4 +141,29 @@ mixin SqliteQueries implements SqliteWriteContext, SqliteConnection {
       return tx.executeBatch(sql, parameterSets);
     });
   }
+
+  @override
+  Future<T> lock<T>(Future<T> Function(SqliteWriteContext tx) callback,
+      {bool? readOnly, String? debugContext}) {
+    if (readOnly == true) {
+      // ignore: deprecated_member_use_from_same_package
+      return readLock((ctx) => callback(ctx as SqliteWriteContext),
+          debugContext: debugContext);
+    } else {
+      // ignore: deprecated_member_use_from_same_package
+      return writeLock(callback, debugContext: debugContext);
+    }
+  }
+
+  @override
+  Future<T> transaction<T>(Future<T> Function(SqliteWriteContext tx) callback,
+      {bool? readOnly}) {
+    if (readOnly == true) {
+      // ignore: deprecated_member_use_from_same_package
+      return readTransaction((ctx) => callback(ctx as SqliteWriteContext));
+    } else {
+      // ignore: deprecated_member_use_from_same_package
+      return writeTransaction(callback);
+    }
+  }
 }

scripts/benchmark.dart

@@ -46,14 +46,14 @@ List<SqliteBenchmark> benchmarks = [
       }
     });
   }, maxBatchSize: 200000, enabled: false),
-  SqliteBenchmark('writeLock in isolate',
+  SqliteBenchmark('lock in isolate',
       (SqliteDatabase db, List<List<String>> parameters) async {
     var factory = db.isolateConnectionFactory();
     var len = parameters.length;
     await Isolate.run(() async {
       final db = factory.open();
       for (var i = 0; i < len; i++) {
-        await db.writeLock((tx) async {});
+        await db.lock((tx) async {});
       }
       await db.close();
     });

test/util.dart

@@ -9,8 +9,8 @@ import 'package:sqlite3/sqlite3.dart' as sqlite;
 import 'package:sqlite_async/sqlite_async.dart';
 import 'package:test_api/src/backend/invoker.dart';
 
-const defaultSqlitePath = 'libsqlite3.so.0';
-// const defaultSqlitePath = './sqlite-autoconf-3410100/.libs/libsqlite3.so.0';
+// const defaultSqlitePath = 'libsqlite3.so.0';
+const defaultSqlitePath = './sqlite-autoconf-3410100/.libs/libsqlite3.so.0';
 
 class TestSqliteOpenFactory extends DefaultSqliteOpenFactory {
   String sqlitePath;