Migrate FSTMutationBatch to C++ MutationBatch (#3673)

Author: wilhuff

Firestore/Example/Tests/Integration/FSTDatastoreTests.mm

@@ -24,7 +24,6 @@
 
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
 #import "Firestore/Source/API/FSTUserDataConverter.h"
-#import "Firestore/Source/Model/FSTMutationBatch.h"
 
 #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
 
@@ -57,6 +56,8 @@
 using firebase::firestore::model::DocumentKey;
 using firebase::firestore::model::DocumentKeySet;
 using firebase::firestore::model::FieldValue;
+using firebase::firestore::model::MutationBatch;
+using firebase::firestore::model::MutationBatchResult;
 using firebase::firestore::model::Precondition;
 using firebase::firestore::model::OnlineState;
 using firebase::firestore::model::TargetId;
@@ -84,18 +85,17 @@ - (void)expectWriteEventWithDescription:(NSString *)description;
 - (void)expectListenEventWithDescription:(NSString *)description;
 
 @property(nonatomic, weak, nullable) XCTestCase *testCase;
-@property(nonatomic, strong) NSMutableArray<NSObject *> *writeEvents;
 @property(nonatomic, strong) NSMutableArray<XCTestExpectation *> *writeEventExpectations;
 @property(nonatomic, strong) NSMutableArray<XCTestExpectation *> *listenEventExpectations;
 @end
 
 @implementation FSTRemoteStoreEventCapture {
   std::vector<RemoteEvent> _listenEvents;
+  std::vector<MutationBatchResult> _writeEvents;
 }
 
 - (instancetype)initWithTestCase:(XCTestCase *_Nullable)testCase {
   if (self = [super init]) {
-    _writeEvents = [NSMutableArray array];
     _testCase = testCase;
     _writeEventExpectations = [NSMutableArray array];
     _listenEventExpectations = [NSMutableArray array];
@@ -125,8 +125,8 @@ - (void)expectListenEventWithDescription:(NSString *)description {
                                                                     description]]];
 }
 
-- (void)applySuccessfulWriteWithResult:(FSTMutationBatchResult *)batchResult {
-  [self.writeEvents addObject:batchResult];
+- (void)applySuccessfulWriteWithResult:(const MutationBatchResult &)batchResult {
+  _writeEvents.push_back(batchResult);
   XCTestExpectation *expectation = [self.writeEventExpectations objectAtIndex:0];
   [self.writeEventExpectations removeObjectAtIndex:0];
   [expectation fulfill];
@@ -223,10 +223,7 @@ - (void)testStreamingWrite {
   _remoteStore->set_sync_engine(capture);
 
   auto mutation = testutil::SetMutation("rooms/eros", Map("name", "Eros"));
-  FSTMutationBatch *batch = [[FSTMutationBatch alloc] initWithBatchID:23
-                                                       localWriteTime:Timestamp::Now()
-                                                        baseMutations:{}
-                                                            mutations:{mutation}];
+  MutationBatch batch = MutationBatch(23, Timestamp::Now(), {}, {mutation});
   _testWorkerQueue->Enqueue([=] {
     _remoteStore->AddToWritePipeline(batch);
     // The added batch won't be written immediately because write stream wasn't yet open --

Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm

@@ -31,7 +31,6 @@
 #import "Firestore/Protos/objc/google/firestore/v1/Query.pbobjc.h"
 #import "Firestore/Protos/objc/google/firestore/v1/Write.pbobjc.h"
 #import "Firestore/Protos/objc/google/type/Latlng.pbobjc.h"
-#import "Firestore/Source/Model/FSTMutationBatch.h"
 #import "Firestore/Source/Remote/FSTSerializerBeta.h"
 
 #import "Firestore/Example/Tests/Util/FSTHelpers.h"
@@ -58,6 +57,7 @@
 using firebase::firestore::model::FieldMask;
 using firebase::firestore::model::MaybeDocument;
 using firebase::firestore::model::Mutation;
+using firebase::firestore::model::MutationBatch;
 using firebase::firestore::model::NoDocument;
 using firebase::firestore::model::PatchMutation;
 using firebase::firestore::model::Precondition;
@@ -103,10 +103,7 @@ - (void)testEncodesMutationBatch {
   Mutation del = testutil::DeleteMutation("baz/quux");
 
   Timestamp writeTime = Timestamp::Now();
-  FSTMutationBatch *model = [[FSTMutationBatch alloc] initWithBatchID:42
-                                                       localWriteTime:writeTime
-                                                        baseMutations:{base}
-                                                            mutations:{set, patch, del}];
+  MutationBatch model = MutationBatch(42, writeTime, {base}, {set, patch, del});
 
   GCFSWrite *baseProto = [GCFSWrite message];
   baseProto.update.name = @"projects/p/databases/d/documents/bar/baz";
@@ -146,12 +143,12 @@ - (void)testEncodesMutationBatch {
   batchProto.localWriteTime = writeTimeProto;
 
   XCTAssertEqualObjects([self.serializer encodedMutationBatch:model], batchProto);
-  FSTMutationBatch *decoded = [self.serializer decodedMutationBatch:batchProto];
-  XCTAssertEqual(decoded.batchID, model.batchID);
-  XCTAssertEqual(decoded.localWriteTime, model.localWriteTime);
-  XCTAssertEqual(decoded.baseMutations, model.baseMutations);
-  XCTAssertEqual(decoded.mutations, model.mutations);
-  XCTAssertEqual([decoded keys], [model keys]);
+  MutationBatch decoded = [self.serializer decodedMutationBatch:batchProto];
+  XCTAssertEqual(decoded.batch_id(), model.batch_id());
+  XCTAssertEqual(decoded.local_write_time(), model.local_write_time());
+  XCTAssertEqual(decoded.base_mutations(), model.base_mutations());
+  XCTAssertEqual(decoded.mutations(), model.mutations());
+  XCTAssertEqual(decoded.keys(), model.keys());
 }
 
 - (void)testEncodesDocumentAsMaybeDocument {

Firestore/Example/Tests/Local/FSTLocalStoreTests.mm

@@ -25,7 +25,6 @@
 
 #import "Firestore/Source/API/FIRFieldValue+Internal.h"
 #import "Firestore/Source/Local/FSTPersistence.h"
-#import "Firestore/Source/Model/FSTMutationBatch.h"
 #import "Firestore/Source/Util/FSTClasses.h"
 
 #import "Firestore/Example/Tests/Local/FSTLocalStoreTests.h"
@@ -38,6 +37,7 @@
 #include "Firestore/core/src/firebase/firestore/local/query_data.h"
 #include "Firestore/core/src/firebase/firestore/model/document_map.h"
 #include "Firestore/core/src/firebase/firestore/model/document_set.h"
+#include "Firestore/core/src/firebase/firestore/model/mutation_batch_result.h"
 #include "Firestore/core/src/firebase/firestore/remote/remote_event.h"
 #include "Firestore/core/src/firebase/firestore/remote/watch_change.h"
 #include "Firestore/core/src/firebase/firestore/util/status.h"
@@ -57,6 +57,8 @@
 using firebase::firestore::model::ListenSequenceNumber;
 using firebase::firestore::model::MaybeDocument;
 using firebase::firestore::model::Mutation;
+using firebase::firestore::model::MutationBatch;
+using firebase::firestore::model::MutationBatchResult;
 using firebase::firestore::model::MutationResult;
 using firebase::firestore::model::DocumentMap;
 using firebase::firestore::model::MaybeDocumentMap;
@@ -106,12 +108,12 @@ @interface FSTLocalStoreTests ()
 @property(nonatomic, strong, readwrite) id<FSTPersistence> localStorePersistence;
 @property(nonatomic, strong, readwrite) FSTLocalStore *localStore;
 
-@property(nonatomic, strong, readonly) NSMutableArray<FSTMutationBatch *> *batches;
 @property(nonatomic, assign, readwrite) TargetId lastTargetID;
 
 @end
 
 @implementation FSTLocalStoreTests {
+  std::vector<MutationBatch> _batches;
   MaybeDocumentMap _lastChanges;
 }
 
@@ -128,7 +130,6 @@ - (void)setUp {
                                                    initialUser:User::Unauthenticated()];
   [self.localStore start];
 
-  _batches = [NSMutableArray array];
   _lastTargetID = 0;
 }
 
@@ -156,16 +157,14 @@ - (BOOL)isTestBaseClass {
 }
 
 - (void)writeMutation:(Mutation)mutation {
-  [self writeMutations:{mutation}];
+  [self writeMutations:{std::move(mutation)}];
 }
 
 - (void)writeMutations:(std::vector<Mutation> &&)mutations {
   auto mutationsCopy = mutations;
   LocalWriteResult result = [self.localStore locallyWriteMutations:std::move(mutationsCopy)];
-  [self.batches addObject:[[FSTMutationBatch alloc] initWithBatchID:result.batch_id()
-                                                     localWriteTime:Timestamp::Now()
-                                                      baseMutations:{}
-                                                          mutations:std::move(mutations)]];
+  _batches.emplace_back(result.batch_id(), Timestamp::Now(), std::vector<Mutation>{},
+                        std::move(mutations));
   _lastChanges = result.changes();
 }
 
@@ -179,9 +178,12 @@ - (void)notifyLocalViewChanges:(LocalViewChanges)changes {
 
 - (void)acknowledgeMutationWithVersion:(FSTTestSnapshotVersion)documentVersion
                        transformResult:(id _Nullable)transformResult {
-  FSTMutationBatch *batch = [self.batches firstObject];
-  [self.batches removeObjectAtIndex:0];
-  XCTAssertEqual(batch.mutations.size(), 1, @"Acknowledging more than one mutation not supported.");
+  XCTAssertGreaterThan(_batches.size(), 0, @"Missing batch to acknowledge.");
+  MutationBatch batch = _batches.front();
+  _batches.erase(_batches.begin());
+
+  XCTAssertEqual(batch.mutations().size(), 1,
+                 @"Acknowledging more than one mutation not supported.");
   SnapshotVersion version = testutil::Version(documentVersion);
 
   absl::optional<std::vector<FieldValue>> mutationTransformResult;
@@ -190,10 +192,7 @@ - (void)acknowledgeMutationWithVersion:(FSTTestSnapshotVersion)documentVersion
   }
 
   MutationResult mutationResult(version, mutationTransformResult);
-  FSTMutationBatchResult *result = [FSTMutationBatchResult resultWithBatch:batch
-                                                             commitVersion:version
-                                                           mutationResults:{mutationResult}
-                                                               streamToken:nil];
+  MutationBatchResult result(batch, version, {mutationResult}, {});
   _lastChanges = [self.localStore acknowledgeBatchWithResult:result];
 }
 
@@ -202,9 +201,9 @@ - (void)acknowledgeMutationWithVersion:(FSTTestSnapshotVersion)documentVersion {
 }
 
 - (void)rejectMutation {
-  FSTMutationBatch *batch = [self.batches firstObject];
-  [self.batches removeObjectAtIndex:0];
-  _lastChanges = [self.localStore rejectBatchID:batch.batchID];
+  MutationBatch batch = _batches.front();
+  _batches.erase(_batches.begin());
+  _lastChanges = [self.localStore rejectBatchID:batch.batch_id()];
 }
 
 - (TargetId)allocateQuery:(core::Query)query {
@@ -268,11 +267,8 @@ - (void)testMutationBatchKeys {
   Mutation base = FSTTestSetMutation(@"foo/ignore", @{@"foo" : @"bar"});
   Mutation set1 = FSTTestSetMutation(@"foo/bar", @{@"foo" : @"bar"});
   Mutation set2 = FSTTestSetMutation(@"bar/baz", @{@"bar" : @"baz"});
-  FSTMutationBatch *batch = [[FSTMutationBatch alloc] initWithBatchID:1
-                                                       localWriteTime:Timestamp::Now()
-                                                        baseMutations:{base}
-                                                            mutations:{set1, set2}];
-  DocumentKeySet keys = [batch keys];
+  MutationBatch batch = MutationBatch(1, Timestamp::Now(), {base}, {set1, set2});
+  DocumentKeySet keys = batch.keys();
   XCTAssertEqual(keys.size(), 2u);
 }