Pass FSTMutations using a vector (#2357)

Author: var-const

Firestore/Example/Tests/Integration/FSTDatastoreTests.mm

@@ -207,7 +207,7 @@ - (void)tearDown {
 - (void)testCommit {
   XCTestExpectation *expectation = [self expectationWithDescription:@"commitWithCompletion"];
 
-  _datastore->CommitMutations(@[], ^(NSError *_Nullable error) {
+  _datastore->CommitMutations({}, ^(NSError *_Nullable error) {
     XCTAssertNil(error, @"Failed to commit");
     [expectation fulfill];
   });
@@ -224,7 +224,7 @@ - (void)testStreamingWrite {
   FSTSetMutation *mutation = [self setMutation];
   FSTMutationBatch *batch = [[FSTMutationBatch alloc] initWithBatchID:23
                                                        localWriteTime:[FIRTimestamp timestamp]
-                                                            mutations:@[ mutation ]];
+                                                            mutations:{mutation}];
   _testWorkerQueue->Enqueue([=] {
     [_remoteStore addBatchToWritePipeline:batch];
     // The added batch won't be written immediately because write stream wasn't yet open --

Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm

@@ -20,6 +20,8 @@
 
 #include <unordered_map>
 #include <unordered_set>
+#include <utility>
+#include <vector>
 
 #import "FIRTimestamp.h"
 #import "Firestore/Example/Tests/Util/FSTHelpers.h"
@@ -412,7 +414,7 @@ - (void)testRemoveOrphanedDocuments {
   // as any documents with pending mutations.
   std::unordered_set<DocumentKey, DocumentKeyHash> expectedRetained;
   // we add two mutations later, for now track them in an array.
-  NSMutableArray *mutations = [NSMutableArray arrayWithCapacity:2];
+  std::vector<FSTMutation *> mutations;
 
   // Add a target and add two documents to it. The documents are expected to be
   // retained, since their membership in the target keeps them alive.
@@ -426,7 +428,7 @@ - (void)testRemoveOrphanedDocuments {
     FSTDocument *doc2 = [self cacheADocumentInTransaction];
     [self addDocument:doc2.key toTarget:queryData.targetID];
     expectedRetained.insert(doc2.key);
-    [mutations addObject:[self mutationForDocument:doc2.key]];
+    mutations.push_back([self mutationForDocument:doc2.key]);
   });
 
   // Add a second query and register a third document on it
@@ -440,15 +442,15 @@ - (void)testRemoveOrphanedDocuments {
   // cache another document and prepare a mutation on it.
   _persistence.run("queue a mutation", [&]() {
     FSTDocument *doc4 = [self cacheADocumentInTransaction];
-    [mutations addObject:[self mutationForDocument:doc4.key]];
+    mutations.push_back([self mutationForDocument:doc4.key]);
     expectedRetained.insert(doc4.key);
   });
 
   // Insert the mutations. These operations don't have a sequence number, they just
   // serve to keep the mutated documents from being GC'd while the mutations are outstanding.
   _persistence.run("actually register the mutations", [&]() {
     FIRTimestamp *writeTime = [FIRTimestamp timestamp];
-    _mutationQueue->AddMutationBatch(writeTime, mutations);
+    _mutationQueue->AddMutationBatch(writeTime, std::move(mutations));
   });
 
   // Mark 5 documents eligible for GC. This simulates documents that were mutated then ack'd.

Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm

@@ -19,6 +19,9 @@
 #import <FirebaseFirestore/FIRTimestamp.h>
 #import <XCTest/XCTest.h>
 
+#include <utility>
+#include <vector>
+
 #import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h"
 #import "Firestore/Protos/objc/firestore/local/Mutation.pbobjc.h"
 #import "Firestore/Protos/objc/firestore/local/Target.pbobjc.h"
@@ -89,7 +92,7 @@ - (void)testEncodesMutationBatch {
   FIRTimestamp *writeTime = [FIRTimestamp timestamp];
   FSTMutationBatch *model = [[FSTMutationBatch alloc] initWithBatchID:42
                                                        localWriteTime:writeTime
-                                                            mutations:@[ set, patch, del ]];
+                                                            mutations:{set, patch, del}];
 
   GCFSWrite *setProto = [GCFSWrite message];
   setProto.update.name = @"projects/p/databases/d/documents/foo/bar";
@@ -123,7 +126,7 @@ - (void)testEncodesMutationBatch {
   FSTMutationBatch *decoded = [self.serializer decodedMutationBatch:batchProto];
   XCTAssertEqual(decoded.batchID, model.batchID);
   XCTAssertEqualObjects(decoded.localWriteTime, model.localWriteTime);
-  XCTAssertEqualObjects(decoded.mutations, model.mutations);
+  FSTAssertEqualVectors(decoded.mutations, model.mutations);
   XCTAssertEqual([decoded keys], [model keys]);
 }
 

Firestore/Example/Tests/Local/FSTLocalStoreTests.mm

@@ -19,6 +19,7 @@
 #import <FirebaseFirestore/FIRTimestamp.h>
 #import <XCTest/XCTest.h>
 
+#include <utility>
 #include <vector>
 
 #import "Firestore/Source/Core/FSTQuery.h"
@@ -124,15 +125,16 @@ - (BOOL)isTestBaseClass {
 }
 
 - (void)writeMutation:(FSTMutation *)mutation {
-  [self writeMutations:@[ mutation ]];
+  [self writeMutations:{mutation}];
 }
 
-- (void)writeMutations:(NSArray<FSTMutation *> *)mutations {
-  FSTLocalWriteResult *result = [self.localStore locallyWriteMutations:mutations];
+- (void)writeMutations:(std::vector<FSTMutation *> &&)mutations {
+  auto mutationsCopy = mutations;
+  FSTLocalWriteResult *result = [self.localStore locallyWriteMutations:std::move(mutationsCopy)];
   XCTAssertNotNil(result);
   [self.batches addObject:[[FSTMutationBatch alloc] initWithBatchID:result.batchID
                                                      localWriteTime:[FIRTimestamp timestamp]
-                                                          mutations:mutations]];
+                                                          mutations:std::move(mutations)]];
   _lastChanges = result.changes;
 }
 
@@ -147,7 +149,7 @@ - (void)notifyLocalViewChanges:(FSTLocalViewChanges *)changes {
 - (void)acknowledgeMutationWithVersion:(FSTTestSnapshotVersion)documentVersion {
   FSTMutationBatch *batch = [self.batches firstObject];
   [self.batches removeObjectAtIndex:0];
-  XCTAssertEqual(batch.mutations.count, 1, @"Acknowledging more than one mutation not supported.");
+  XCTAssertEqual(batch.mutations.size(), 1, @"Acknowledging more than one mutation not supported.");
   SnapshotVersion version = testutil::Version(documentVersion);
   FSTMutationResult *mutationResult = [[FSTMutationResult alloc] initWithVersion:version
                                                                 transformResults:nil];
@@ -227,7 +229,7 @@ - (void)testMutationBatchKeys {
   FSTMutation *set2 = FSTTestSetMutation(@"bar/baz", @{@"bar" : @"baz"});
   FSTMutationBatch *batch = [[FSTMutationBatch alloc] initWithBatchID:1
                                                        localWriteTime:[FIRTimestamp timestamp]
-                                                            mutations:@[ set1, set2 ]];
+                                                            mutations:{set1, set2}];
   DocumentKeySet keys = [batch keys];
   XCTAssertEqual(keys.size(), 2u);
 }
@@ -619,10 +621,10 @@ - (void)testHandlesSetMutationThenPatchMutationThenDocumentThenAckThenAck {
 - (void)testHandlesSetMutationAndPatchMutationTogether {
   if ([self isTestBaseClass]) return;
 
-  [self writeMutations:@[
+  [self writeMutations:{
     FSTTestSetMutation(@"foo/bar", @{@"foo" : @"old"}),
-    FSTTestPatchMutation("foo/bar", @{@"foo" : @"bar"}, {})
-  ]];
+        FSTTestPatchMutation("foo/bar", @{@"foo" : @"bar"}, {})
+  }];
 
   FSTAssertChanged(
       @[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, FSTDocumentStateLocalMutations) ]);
@@ -649,11 +651,11 @@ - (void)testHandlesSetMutationThenPatchMutationThenReject {
 - (void)testHandlesSetMutationsAndPatchMutationOfJustOneTogether {
   if ([self isTestBaseClass]) return;
 
-  [self writeMutations:@[
+  [self writeMutations:{
     FSTTestSetMutation(@"foo/bar", @{@"foo" : @"old"}),
-    FSTTestSetMutation(@"bar/baz", @{@"bar" : @"baz"}),
-    FSTTestPatchMutation("foo/bar", @{@"foo" : @"bar"}, {})
-  ]];
+        FSTTestSetMutation(@"bar/baz", @{@"bar" : @"baz"}),
+        FSTTestPatchMutation("foo/bar", @{@"foo" : @"bar"}, {})
+  }];
 
   FSTAssertChanged((@[
     FSTTestDoc("bar/baz", 0, @{@"bar" : @"baz"}, FSTDocumentStateLocalMutations),
@@ -843,11 +845,11 @@ - (void)testThrowsAwayDocumentsWithUnknownTargetIDsImmediately {
 - (void)testCanExecuteDocumentQueries {
   if ([self isTestBaseClass]) return;
 
-  [self.localStore locallyWriteMutations:@[
+  [self.localStore locallyWriteMutations:{
     FSTTestSetMutation(@"foo/bar", @{@"foo" : @"bar"}),
-    FSTTestSetMutation(@"foo/baz", @{@"foo" : @"baz"}),
-    FSTTestSetMutation(@"foo/bar/Foo/Bar", @{@"Foo" : @"Bar"})
-  ]];
+        FSTTestSetMutation(@"foo/baz", @{@"foo" : @"baz"}),
+        FSTTestSetMutation(@"foo/bar/Foo/Bar", @{@"Foo" : @"Bar"})
+  }];
   FSTQuery *query = FSTTestQuery("foo/bar");
   DocumentMap docs = [self.localStore executeQuery:query];
   XCTAssertEqualObjects(docMapToArray(docs), @[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"},
@@ -857,13 +859,13 @@ - (void)testCanExecuteDocumentQueries {
 - (void)testCanExecuteCollectionQueries {
   if ([self isTestBaseClass]) return;
 
-  [self.localStore locallyWriteMutations:@[
+  [self.localStore locallyWriteMutations:{
     FSTTestSetMutation(@"fo/bar", @{@"fo" : @"bar"}),
-    FSTTestSetMutation(@"foo/bar", @{@"foo" : @"bar"}),
-    FSTTestSetMutation(@"foo/baz", @{@"foo" : @"baz"}),
-    FSTTestSetMutation(@"foo/bar/Foo/Bar", @{@"Foo" : @"Bar"}),
-    FSTTestSetMutation(@"fooo/blah", @{@"fooo" : @"blah"})
-  ]];
+        FSTTestSetMutation(@"foo/bar", @{@"foo" : @"bar"}),
+        FSTTestSetMutation(@"foo/baz", @{@"foo" : @"baz"}),
+        FSTTestSetMutation(@"foo/bar/Foo/Bar", @{@"Foo" : @"Bar"}),
+        FSTTestSetMutation(@"fooo/blah", @{@"fooo" : @"blah"})
+  }];
   FSTQuery *query = FSTTestQuery("foo");
   DocumentMap docs = [self.localStore executeQuery:query];
   XCTAssertEqualObjects(
@@ -887,7 +889,7 @@ - (void)testCanExecuteMixedCollectionQueries {
                              FSTTestDoc("foo/bar", 20, @{@"a" : @"b"}, FSTDocumentStateSynced), {2},
                              {})];
 
-  [self.localStore locallyWriteMutations:@[ FSTTestSetMutation(@"foo/bonk", @{@"a" : @"b"}) ]];
+  [self.localStore locallyWriteMutations:{ FSTTestSetMutation(@"foo/bonk", @{@"a" : @"b"}) }];
 
   DocumentMap docs = [self.localStore executeQuery:query];
   XCTAssertEqualObjects(docMapToArray(docs), (@[
@@ -942,7 +944,7 @@ - (void)testRemoteDocumentKeysForTarget {
       applyRemoteEvent:FSTTestAddedRemoteEvent(
                            FSTTestDoc("foo/bar", 20, @{@"a" : @"b"}, FSTDocumentStateSynced), {2})];
 
-  [self.localStore locallyWriteMutations:@[ FSTTestSetMutation(@"foo/bonk", @{@"a" : @"b"}) ]];
+  [self.localStore locallyWriteMutations:{ FSTTestSetMutation(@"foo/bonk", @{@"a" : @"b"}) }];
 
   DocumentKeySet keys = [self.localStore remoteDocumentKeysForTarget:2];
   DocumentKeySet expected{testutil::Key("foo/bar"), testutil::Key("foo/baz")};

Firestore/Example/Tests/Local/FSTMutationQueueTests.mm

@@ -19,6 +19,7 @@
 #import <FirebaseFirestore/FIRTimestamp.h>
 
 #include <set>
+#include <utility>
 #include <vector>
 
 #import "Firestore/Source/Core/FSTQuery.h"
@@ -50,14 +51,6 @@ - (void)tearDown {
   [super tearDown];
 }
 
-- (void)assertVector:(const std::vector<FSTMutationBatch *> &)actual
-     matchesExpected:(const std::vector<FSTMutationBatch *> &)expected {
-  XCTAssertEqual(actual.size(), expected.size(), @"Vector length mismatch");
-  for (int i = 0; i < expected.size(); i++) {
-    XCTAssertEqualObjects(actual[i], expected[i]);
-  }
-}
-
 /**
  * Xcode will run tests from any class that extends XCTestCase, but this doesn't work for
  * FSTMutationQueueTests since it is incomplete without the implementations supplied by its
@@ -208,15 +201,15 @@ - (void)testAllMutationBatchesAffectingDocumentKey {
     NSMutableArray<FSTMutationBatch *> *batches = [NSMutableArray array];
     for (FSTMutation *mutation in mutations) {
       FSTMutationBatch *batch =
-          self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], @[ mutation ]);
+          self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], { mutation });
       [batches addObject:batch];
     }
 
     std::vector<FSTMutationBatch *> expected{batches[1], batches[2]};
     std::vector<FSTMutationBatch *> matches =
         self.mutationQueue->AllMutationBatchesAffectingDocumentKey(testutil::Key("foo/bar"));
 
-    [self assertVector:matches matchesExpected:expected];
+    FSTAssertEqualVectors(matches, expected);
   });
 }
 
@@ -235,7 +228,7 @@ - (void)testAllMutationBatchesAffectingDocumentKeys {
     NSMutableArray<FSTMutationBatch *> *batches = [NSMutableArray array];
     for (FSTMutation *mutation in mutations) {
       FSTMutationBatch *batch =
-          self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], @[ mutation ]);
+          self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], { mutation });
       [batches addObject:batch];
     }
 
@@ -248,29 +241,29 @@ - (void)testAllMutationBatchesAffectingDocumentKeys {
     std::vector<FSTMutationBatch *> matches =
         self.mutationQueue->AllMutationBatchesAffectingDocumentKeys(keys);
 
-    [self assertVector:matches matchesExpected:expected];
+    FSTAssertEqualVectors(matches, expected);
   });
 }
 
 - (void)testAllMutationBatchesAffectingDocumentKeys_handlesOverlap {
   if ([self isTestBaseClass]) return;
 
   self.persistence.run("testAllMutationBatchesAffectingDocumentKeys_handlesOverlap", [&]() {
-    NSArray<FSTMutation *> *group1 = @[
-      FSTTestSetMutation(@"foo/bar", @{@"a" : @1}),
-      FSTTestSetMutation(@"foo/baz", @{@"a" : @1}),
-    ];
+    std::vector<FSTMutation *> group1 = {
+        FSTTestSetMutation(@"foo/bar", @{@"a" : @1}),
+        FSTTestSetMutation(@"foo/baz", @{@"a" : @1}),
+    };
     FSTMutationBatch *batch1 =
-        self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], group1);
+        self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], std::move(group1));
 
-    NSArray<FSTMutation *> *group2 = @[ FSTTestSetMutation(@"food/bar", @{@"a" : @1}) ];
-    self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], group2);
+    std::vector<FSTMutation *> group2 = {FSTTestSetMutation(@"food/bar", @{@"a" : @1})};
+    self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], std::move(group2));
 
-    NSArray<FSTMutation *> *group3 = @[
-      FSTTestSetMutation(@"foo/bar", @{@"b" : @1}),
-    ];
+    std::vector<FSTMutation *> group3 = {
+        FSTTestSetMutation(@"foo/bar", @{@"b" : @1}),
+    };
     FSTMutationBatch *batch3 =
-        self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], group3);
+        self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], std::move(group3));
 
     DocumentKeySet keys{
         Key("foo/bar"),
@@ -281,7 +274,7 @@ - (void)testAllMutationBatchesAffectingDocumentKeys_handlesOverlap {
     std::vector<FSTMutationBatch *> matches =
         self.mutationQueue->AllMutationBatchesAffectingDocumentKeys(keys);
 
-    [self assertVector:matches matchesExpected:expected];
+    FSTAssertEqualVectors(matches, expected);
   });
 }
 
@@ -300,7 +293,7 @@ - (void)testAllMutationBatchesAffectingQuery {
     NSMutableArray<FSTMutationBatch *> *batches = [NSMutableArray array];
     for (FSTMutation *mutation in mutations) {
       FSTMutationBatch *batch =
-          self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], @[ mutation ]);
+          self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], { mutation });
       [batches addObject:batch];
     }
 
@@ -309,7 +302,7 @@ - (void)testAllMutationBatchesAffectingQuery {
     std::vector<FSTMutationBatch *> matches =
         self.mutationQueue->AllMutationBatchesAffectingQuery(query);
 
-    [self assertVector:matches matchesExpected:expected];
+    FSTAssertEqualVectors(matches, expected);
   });
 }
 
@@ -327,7 +320,7 @@ - (void)testRemoveMutationBatches {
     std::vector<FSTMutationBatch *> found;
 
     found = self.mutationQueue->AllMutationBatches();
-    [self assertVector:found matchesExpected:batches];
+    FSTAssertEqualVectors(found, batches);
     XCTAssertEqual(found.size(), 9);
 
     self.mutationQueue->RemoveMutationBatch(batches[0]);
@@ -337,15 +330,15 @@ - (void)testRemoveMutationBatches {
     XCTAssertEqual([self batchCount], 6);
 
     found = self.mutationQueue->AllMutationBatches();
-    [self assertVector:found matchesExpected:batches];
+    FSTAssertEqualVectors(found, batches);
     XCTAssertEqual(found.size(), 6);
 
     self.mutationQueue->RemoveMutationBatch(batches[0]);
     batches.erase(batches.begin());
     XCTAssertEqual([self batchCount], 5);
 
     found = self.mutationQueue->AllMutationBatches();
-    [self assertVector:found matchesExpected:batches];
+    FSTAssertEqualVectors(found, batches);
     XCTAssertEqual(found.size(), 5);
 
     self.mutationQueue->RemoveMutationBatch(batches[0]);
@@ -357,7 +350,7 @@ - (void)testRemoveMutationBatches {
     XCTAssertEqual([self batchCount], 3);
 
     found = self.mutationQueue->AllMutationBatches();
-    [self assertVector:found matchesExpected:batches];
+    FSTAssertEqualVectors(found, batches);
     XCTAssertEqual(found.size(), 3);
     XCTAssertFalse(self.mutationQueue->IsEmpty());
 
@@ -404,7 +397,7 @@ - (FSTMutationBatch *)addMutationBatchWithKey:(NSString *)key {
   FSTSetMutation *mutation = FSTTestSetMutation(key, @{@"a" : @1});
 
   FSTMutationBatch *batch =
-      self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], @[ mutation ]);
+      self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], { mutation });
   return batch;
 }
 

Firestore/Example/Tests/SpecTests/FSTMockDatastore.h

@@ -78,7 +78,7 @@ class MockDatastore : public Datastore {
   /**
    * Returns the next write that was "sent to the backend", failing if there are no queued sent
    */
-  NSArray<FSTMutation*>* NextSentWrite();
+  std::vector<FSTMutation*> NextSentWrite();
   /** Returns the number of writes that have been sent to the backend but not waited on yet. */
   int WritesSent() const;