Running a transaction (#969)

* Start work on leveldb transactions

* Style

* Working API. Not plumbed in yet

* Move files into correct place

* Wrangling file locations and associations

* Tests pass

* Add some comments

* style

* Fix copyright

* Rewrite iterator internals to handle deletion-while-iterating. Also add tests for same

* Switch to strings instead of slices

* Style

* More style fixes

* Start switching writegroup over

* Swap out write group tracking for transaction usage

* Style

* Response to feedback before updating docs

* Style

* Add comment

* Initialize version_

* Satisfy the linter

* Start switching writegroup over

* Swap out write group tracking for transaction usage

* Style

* Checkpoint before implementing BatchDescription

* Style

* Initial plumbing for leveldb local parts

* Add model::BatchId

* Port leveldb_key.{h,cc}

* Add string StartsWith

* Add leveldb_key_test.cc to the project

* Revert back to using leveldb::Slice for read/describe

These operations universally operate on keys obtained from leveldb so
it's actually unhelpful to force all the callers to make
absl::string_views from them.

* Everything passing

* Drop unused function

* Style

* STart work on reads

* Swap reads in queryCache to use transactions

* Fix up tests of querycache

* Drop commented out code

* Cleanup

* Style

* Fix up for passing tests

* style

* Renaming

* Style

* Start work on ToString for transactions

* Add ToString() method to LevelDbTransaction

* Style

* lint

* Fix includes, drop runTransaction

* current_transaction -> currentTransaction

* LevelDbTransaction::NewIterator now returns a unique_ptr

* Style

* Revert addition of util::StartsWith

* Add log line

* Style

* Add log line

* Style

* Add debug log line for commits, drop unused BatchDescription

* STart work on reads

* Swap reads in queryCache to use transactions

* Start on remote documents

* Transition mutation queue and remote documents to use transactions

* Style

* Make everything pass

* Make everything pass

* Make it compile

* Style

* Style

* Revert name change, use DefaultReadOptions()

* Style

* Example of running a transaction with a lambda

* Drop errant typo

* Drop duplicate method declarations

* replace usage of auto w/ decltype

* Drop an unnecessary _Nullable. Add some nullability warning suppression

* use absl::make_unique, handle void return type

* Style

* Wrap backing persistence and expectation of backing persistence for transaction runner into a method

* More comments, trigger CI

6 files changed, 107 insertions, 6 deletions

diff --git a/Firestore/Source/Local/FSTLevelDB.h b/Firestore/Source/Local/FSTLevelDB.h
index b03b65f..95b80a6 100644
--- a/Firestore/Source/Local/FSTLevelDB.h
+++ b/Firestore/Source/Local/FSTLevelDB.h
@@ -29,7 +29,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 /** A LevelDB-backed instance of FSTPersistence. */
 // TODO(mikelehen): Rename to FSTLevelDBPersistence.
-@interface FSTLevelDB : NSObject <FSTPersistence>
+@interface FSTLevelDB : NSObject <FSTPersistence, FSTTransactional>
 
 /**
  * Initializes the LevelDB in the given directory. Note that all expensive startup work including
diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm
index 2edccb4..922c5b4 100644
--- a/Firestore/Source/Local/FSTLevelDB.mm
+++ b/Firestore/Source/Local/FSTLevelDB.mm
@@ -28,6 +28,7 @@
 #import "Firestore/Source/Remote/FSTSerializerBeta.h"
 #import "Firestore/Source/Util/FSTAssert.h"
 #import "Firestore/Source/Util/FSTLogger.h"
+#include "absl/memory/memory.h"
 
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
 #include "Firestore/core/src/firebase/firestore/core/database_info.h"
@@ -62,6 +63,7 @@ using leveldb::WriteOptions;
 
 @implementation FSTLevelDB {
   std::unique_ptr<LevelDbTransaction> _transaction;
+  FSTTransactionRunner _transactionRunner;
 }
 
 /**
@@ -79,10 +81,15 @@ using leveldb::WriteOptions;
     _directory = [directory copy];
     _writeGroupTracker = [FSTWriteGroupTracker tracker];
     _serializer = serializer;
+    _transactionRunner.SetBackingPersistence(self);
   }
   return self;
 }
 
+- (const FSTTransactionRunner &)run {
+  return _transactionRunner;
+}
+
 + (NSString *)documentsDirectory {
 #if TARGET_OS_IPHONE
   NSArray<NSString *> *directories =
@@ -222,9 +229,20 @@ using leveldb::WriteOptions;
   return [[FSTLevelDBRemoteDocumentCache alloc] initWithDB:self serializer:self.serializer];
 }
 
+- (void)startTransaction {
+  FSTAssert(_transaction == nullptr, @"Starting a transaction while one is already outstanding");
+  _transaction = absl::make_unique<LevelDbTransaction>(_ptr.get());
+}
+
+- (void)commitTransaction {
+  FSTAssert(_transaction != nullptr, @"Committing a transaction before one is started");
+  _transaction->Commit();
+  _transaction.reset();
+}
+
 - (FSTWriteGroup *)startGroupWithAction:(NSString *)action {
   FSTAssert(_transaction == nullptr, @"Starting a transaction while one is already outstanding");
-  _transaction = std::make_unique<LevelDbTransaction>(_ptr.get());
+  _transaction = absl::make_unique<LevelDbTransaction>(_ptr.get());
   return [self.writeGroupTracker startGroupWithAction:action transaction:_transaction.get()];
 }
 
diff --git a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm
index 4ddd29f..1f2a97d 100644
--- a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm
+++ b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm
@@ -31,7 +31,6 @@
 #import "Firestore/Source/Model/FSTDocumentSet.h"
 #import "Firestore/Source/Util/FSTAssert.h"
 #include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h"
-
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
 
 NS_ASSUME_NONNULL_BEGIN
diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm
index 2ea3328..7508358 100644
--- a/Firestore/Source/Local/FSTLocalStore.mm
+++ b/Firestore/Source/Local/FSTLocalStore.mm
@@ -393,9 +393,9 @@ NS_ASSUME_NONNULL_BEGIN
 }
 
 - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID {
-  FSTWriteGroup *group = [self.persistence startGroupWithAction:@"nextMutationBatchAfterBatchID"];
-  FSTMutationBatch *result = [self.mutationQueue nextMutationBatchAfterBatchID:batchID];
-  [self.persistence commitGroup:group];
+  FSTMutationBatch *result = self.persistence.run([&]() -> FSTMutationBatch * {
+    return [self.mutationQueue nextMutationBatchAfterBatchID:batchID];
+  });
   return result;
 }
 
diff --git a/Firestore/Source/Local/FSTMemoryPersistence.mm b/Firestore/Source/Local/FSTMemoryPersistence.mm
index ba71f9c..f1f9885 100644
--- a/Firestore/Source/Local/FSTMemoryPersistence.mm
+++ b/Firestore/Source/Local/FSTMemoryPersistence.mm
@@ -52,6 +52,8 @@ NS_ASSUME_NONNULL_BEGIN
   FSTMemoryRemoteDocumentCache *_remoteDocumentCache;
 
   std::unordered_map<User, id<FSTMutationQueue>, HashUser> _mutationQueues;
+
+  FSTTransactionRunner _transactionRunner;
 }
 
 + (instancetype)persistence {
@@ -80,6 +82,10 @@ NS_ASSUME_NONNULL_BEGIN
   self.started = NO;
 }
 
+- (const FSTTransactionRunner &)run {
+  return _transactionRunner;
+}
+
 - (id<FSTMutationQueue>)mutationQueueForUser:(const User &)user {
   id<FSTMutationQueue> queue = _mutationQueues[user];
   if (!queue) {
diff --git a/Firestore/Source/Local/FSTPersistence.h b/Firestore/Source/Local/FSTPersistence.h
index 30eb211..6f75d73 100644
--- a/Firestore/Source/Local/FSTPersistence.h
+++ b/Firestore/Source/Local/FSTPersistence.h
@@ -16,6 +16,7 @@
 
 #import <Foundation/Foundation.h>
 
+#import "Firestore/Source/Util/FSTAssert.h"
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
 
 @class FSTWriteGroup;
@@ -55,6 +56,7 @@ NS_ASSUME_NONNULL_BEGIN
  * FSTPersistence. The cost is that the FSTLocalStore needs to be slightly careful about the order
  * of its reads and writes in order to avoid relying on being able to read back uncommitted writes.
  */
+struct FSTTransactionRunner;
 @protocol FSTPersistence <NSObject>
 
 /**
@@ -99,6 +101,82 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)commitGroup:(FSTWriteGroup *)group;
 
+@property(nonatomic, readonly, assign) const FSTTransactionRunner &run;
+
+@end
+
+@protocol FSTTransactional
+
+- (void)startTransaction;
+
+- (void)commitTransaction;
+
 @end
 
+struct FSTTransactionRunner {
+// Intentionally disable nullability checking for this function. We cannot properly annotate
+// the function because this function can handle both pointer and non-pointer types. It is an error
+// to annotate non-pointer types with a nullability annotation.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wnullability-completeness"
+
+  /**
+   * The following two functions handle accepting callables and optionally running them within a
+   * transaction. Persistence layers that conform to the FSTTransactional protocol can set
+   * themselves as the backing persistence for a transaction runner, in which case a transaction
+   * will be started before a block is run, and committed after the block has executed. If there is
+   * no backing instance of FSTTransactional, the block will be run directly.
+   *
+   * There are two instances of operator() to handle the case where the block returns void, rather
+   * than a type.
+   *
+   * The transaction runner keeps a weak reference to the backing persistence so as not to cause a
+   * retain cycle. The reference is upgraded to strong (with a fatal error if it has disappeared)
+   * for the duration of running a transaction.
+   */
+
+  template <typename F>
+  auto operator()(F block) ->
+      typename std::enable_if<std::is_void<decltype(block())>::value, void>::type {
+    __strong id<FSTTransactional> strongDb = _db;
+    if (!strongDb && _expect_db) {
+      FSTCFail(@"Transaction runner accessed without underlying db when it expected one");
+    }
+    if (strongDb) {
+      [strongDb startTransaction];
+    }
+    block();
+    if (strongDb) {
+      [strongDb commitTransaction];
+    }
+  }
+
+  template <typename F>
+  auto operator()(F block) const ->
+      typename std::enable_if<!std::is_void<decltype(block())>::value, decltype(block())>::type {
+    using ReturnT = decltype(block());
+    __strong id<FSTTransactional> strongDb = _db;
+    if (!strongDb && _expect_db) {
+      FSTCFail(@"Transaction runner accessed without underlying db when it expected one");
+    }
+    if (strongDb) {
+      [strongDb startTransaction];
+    }
+    ReturnT result = block();
+    if (strongDb) {
+      [strongDb commitTransaction];
+    }
+    return result;
+  }
+#pragma clang diagnostic pop
+  void SetBackingPersistence(id<FSTTransactional> db) {
+    _db = db;
+    _expect_db = true;
+  }
+
+ private:
+  __weak id<FSTTransactional> _db;
+  bool _expect_db = false;
+};
+
 NS_ASSUME_NONNULL_END