diff --git a/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm b/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm index ec43e8ba7eb..243febbb8ab 100644 --- a/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm +++ b/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 -- diff --git a/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm b/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm index 3a1f5b78876..c691c675f9b 100644 --- a/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm +++ b/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm @@ -20,6 +20,8 @@ #include #include +#include +#include #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 expectedRetained; // we add two mutations later, for now track them in an array. - NSMutableArray *mutations = [NSMutableArray arrayWithCapacity:2]; + std::vector 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,7 +442,7 @@ - (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); }); @@ -448,7 +450,7 @@ - (void)testRemoveOrphanedDocuments { // 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. diff --git a/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm b/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm index 538f6c61e86..32a0a55a6f9 100644 --- a/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm @@ -19,6 +19,9 @@ #import #import +#include +#include + #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]); } diff --git a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm index 026db12ad05..da8a8b0b4c9 100644 --- a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm @@ -19,6 +19,7 @@ #import #import +#include #include #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 *)mutations { - FSTLocalWriteResult *result = [self.localStore locallyWriteMutations:mutations]; +- (void)writeMutations:(std::vector &&)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")}; diff --git a/Firestore/Example/Tests/Local/FSTMutationQueueTests.mm b/Firestore/Example/Tests/Local/FSTMutationQueueTests.mm index c975bf70cd8..c3fcc6f6131 100644 --- a/Firestore/Example/Tests/Local/FSTMutationQueueTests.mm +++ b/Firestore/Example/Tests/Local/FSTMutationQueueTests.mm @@ -19,6 +19,7 @@ #import #include +#include #include #import "Firestore/Source/Core/FSTQuery.h" @@ -50,14 +51,6 @@ - (void)tearDown { [super tearDown]; } -- (void)assertVector:(const std::vector &)actual - matchesExpected:(const std::vector &)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,7 +201,7 @@ - (void)testAllMutationBatchesAffectingDocumentKey { NSMutableArray *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]; } @@ -216,7 +209,7 @@ - (void)testAllMutationBatchesAffectingDocumentKey { std::vector matches = self.mutationQueue->AllMutationBatchesAffectingDocumentKey(testutil::Key("foo/bar")); - [self assertVector:matches matchesExpected:expected]; + FSTAssertEqualVectors(matches, expected); }); } @@ -235,7 +228,7 @@ - (void)testAllMutationBatchesAffectingDocumentKeys { NSMutableArray *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,7 +241,7 @@ - (void)testAllMutationBatchesAffectingDocumentKeys { std::vector matches = self.mutationQueue->AllMutationBatchesAffectingDocumentKeys(keys); - [self assertVector:matches matchesExpected:expected]; + FSTAssertEqualVectors(matches, expected); }); } @@ -256,21 +249,21 @@ - (void)testAllMutationBatchesAffectingDocumentKeys_handlesOverlap { if ([self isTestBaseClass]) return; self.persistence.run("testAllMutationBatchesAffectingDocumentKeys_handlesOverlap", [&]() { - NSArray *group1 = @[ - FSTTestSetMutation(@"foo/bar", @{@"a" : @1}), - FSTTestSetMutation(@"foo/baz", @{@"a" : @1}), - ]; + std::vector 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 *group2 = @[ FSTTestSetMutation(@"food/bar", @{@"a" : @1}) ]; - self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], group2); + std::vector group2 = {FSTTestSetMutation(@"food/bar", @{@"a" : @1})}; + self.mutationQueue->AddMutationBatch([FIRTimestamp timestamp], std::move(group2)); - NSArray *group3 = @[ - FSTTestSetMutation(@"foo/bar", @{@"b" : @1}), - ]; + std::vector 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 matches = self.mutationQueue->AllMutationBatchesAffectingDocumentKeys(keys); - [self assertVector:matches matchesExpected:expected]; + FSTAssertEqualVectors(matches, expected); }); } @@ -300,7 +293,7 @@ - (void)testAllMutationBatchesAffectingQuery { NSMutableArray *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 matches = self.mutationQueue->AllMutationBatchesAffectingQuery(query); - [self assertVector:matches matchesExpected:expected]; + FSTAssertEqualVectors(matches, expected); }); } @@ -327,7 +320,7 @@ - (void)testRemoveMutationBatches { std::vector found; found = self.mutationQueue->AllMutationBatches(); - [self assertVector:found matchesExpected:batches]; + FSTAssertEqualVectors(found, batches); XCTAssertEqual(found.size(), 9); self.mutationQueue->RemoveMutationBatch(batches[0]); @@ -337,7 +330,7 @@ - (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]); @@ -345,7 +338,7 @@ - (void)testRemoveMutationBatches { 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; } diff --git a/Firestore/Example/Tests/SpecTests/FSTMockDatastore.h b/Firestore/Example/Tests/SpecTests/FSTMockDatastore.h index c9555cd9455..cc1e4335d59 100644 --- a/Firestore/Example/Tests/SpecTests/FSTMockDatastore.h +++ b/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* NextSentWrite(); + std::vector NextSentWrite(); /** Returns the number of writes that have been sent to the backend but not waited on yet. */ int WritesSent() const; diff --git a/Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm b/Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm index 49bf0b71375..ab25c01e567 100644 --- a/Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm +++ b/Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm @@ -196,7 +196,7 @@ void WriteHandshake() override { callback_->OnWriteStreamHandshakeComplete(); } - void WriteMutations(NSArray* mutations) override { + void WriteMutations(const std::vector& mutations) override { datastore_->IncrementWriteStreamRequests(); sent_mutations_.push(mutations); } @@ -215,10 +215,10 @@ void FailStream(const Status& error) { /** * Returns the next write that was "sent to the backend", failing if there are no queued sent */ - NSArray* NextSentWrite() { + std::vector NextSentWrite() { HARD_ASSERT(!sent_mutations_.empty(), "Writes need to happen before you can call NextSentWrite."); - NSArray* result = std::move(sent_mutations_.front()); + std::vector result = std::move(sent_mutations_.front()); sent_mutations_.pop(); return result; } @@ -233,7 +233,7 @@ int sent_mutations_count() const { private: bool open_ = false; - std::queue*> sent_mutations_; + std::queue> sent_mutations_; MockDatastore* datastore_ = nullptr; WriteStreamCallback* callback_ = nullptr; }; @@ -281,7 +281,7 @@ int sent_mutations_count() const { return watch_stream_->IsOpen(); } -NSArray* MockDatastore::NextSentWrite() { +std::vector MockDatastore::NextSentWrite() { return write_stream_->NextSentWrite(); } diff --git a/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm b/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm index 4f196b15033..2d54a833ca3 100644 --- a/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm +++ b/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm @@ -22,6 +22,7 @@ #include #include #include +#include #import "Firestore/Source/Core/FSTEventManager.h" #import "Firestore/Source/Core/FSTQuery.h" @@ -227,9 +228,9 @@ - (void)shutdown { } - (void)validateNextWriteSent:(FSTMutation *)expectedWrite { - NSArray *request = _datastore->NextSentWrite(); + std::vector request = _datastore->NextSentWrite(); // Make sure the write went through the pipe like we expected it to. - HARD_ASSERT(request.count == 1, "Only single mutation requests are supported at the moment"); + HARD_ASSERT(request.size() == 1, "Only single mutation requests are supported at the moment"); FSTMutation *actualWrite = request[0]; HARD_ASSERT([actualWrite isEqual:expectedWrite], "Mock datastore received write %s but first outstanding mutation was %s", actualWrite, @@ -356,7 +357,7 @@ - (void)writeUserMutation:(FSTMutation *)mutation { [[self currentOutstandingWrites] addObject:write]; LOG_DEBUG("sending a user write."); _workerQueue->EnqueueBlocking([=] { - [self.syncEngine writeMutations:@[ mutation ] + [self.syncEngine writeMutations:{mutation} completion:^(NSError *_Nullable error) { LOG_DEBUG("A callback was called with error: %s", error); write.done = YES; diff --git a/Firestore/Example/Tests/Util/FSTHelpers.h b/Firestore/Example/Tests/Util/FSTHelpers.h index 6e6f57158ab..8f9f4fff6fb 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.h +++ b/Firestore/Example/Tests/Util/FSTHelpers.h @@ -141,6 +141,15 @@ inline NSString *FSTRemoveExceptionPrefix(NSString *exception) { XCTAssertTrue(didThrow, ##__VA_ARGS__); \ } while (0) +// Helper to compare vectors containing Objective-C objects. +#define FSTAssertEqualVectors(v1, v2) \ + do { \ + XCTAssertEqual(v1.size(), v2.size(), @"Vector length mismatch"); \ + for (size_t i = 0; i < v1.size(); i++) { \ + XCTAssertEqualObjects(v1[i], v2[i]); \ + } \ + } while (0) + /** * An implementation of `TargetMetadataProvider` that provides controlled access to the * `TargetMetadataProvider` callbacks. Any target accessed via these callbacks must be diff --git a/Firestore/Source/API/FIRDocumentReference.mm b/Firestore/Source/API/FIRDocumentReference.mm index 0222843c718..82f915f2d71 100644 --- a/Firestore/Source/API/FIRDocumentReference.mm +++ b/Firestore/Source/API/FIRDocumentReference.mm @@ -174,7 +174,7 @@ - (void)deleteDocument { - (void)deleteDocumentWithCompletion:(nullable void (^)(NSError *_Nullable error))completion { FSTDeleteMutation *mutation = [[FSTDeleteMutation alloc] initWithKey:self.key precondition:Precondition::None()]; - return [self.firestore.client writeMutations:@[ mutation ] completion:completion]; + return [self.firestore.client writeMutations:{mutation} completion:completion]; } - (void)getDocumentWithCompletion:(void (^)(FIRDocumentSnapshot *_Nullable document, diff --git a/Firestore/Source/API/FIRWriteBatch.mm b/Firestore/Source/API/FIRWriteBatch.mm index 8ce58d3077e..484b8b0f4e8 100644 --- a/Firestore/Source/API/FIRWriteBatch.mm +++ b/Firestore/Source/API/FIRWriteBatch.mm @@ -16,7 +16,9 @@ #import "FIRWriteBatch.h" +#include #include +#include #import "Firestore/Source/API/FIRDocumentReference+Internal.h" #import "Firestore/Source/API/FIRFirestore+Internal.h" @@ -41,7 +43,6 @@ @interface FIRWriteBatch () - (instancetype)initWithFirestore:(FIRFirestore *)firestore NS_DESIGNATED_INITIALIZER; @property(nonatomic, strong, readonly) FIRFirestore *firestore; -@property(nonatomic, strong, readonly) NSMutableArray *mutations; @property(nonatomic, assign) BOOL committed; @end @@ -54,13 +55,14 @@ + (instancetype)writeBatchWithFirestore:(FIRFirestore *)firestore { @end -@implementation FIRWriteBatch +@implementation FIRWriteBatch { + std::vector _mutations; +} - (instancetype)initWithFirestore:(FIRFirestore *)firestore { self = [super init]; if (self) { _firestore = firestore; - _mutations = [NSMutableArray array]; } return self; } @@ -75,10 +77,13 @@ - (FIRWriteBatch *)setData:(NSDictionary *)data merge:(BOOL)merge { [self verifyNotCommitted]; [self validateReference:document]; + ParsedSetData parsed = merge ? [self.firestore.dataConverter parsedMergeData:data fieldMask:nil] : [self.firestore.dataConverter parsedSetData:data]; - [self.mutations - addObjectsFromArray:std::move(parsed).ToMutations(document.key, Precondition::None())]; + std::vector append_mutations = + std::move(parsed).ToMutations(document.key, Precondition::None()); + std::move(append_mutations.begin(), append_mutations.end(), std::back_inserter(_mutations)); + return self; } @@ -87,9 +92,12 @@ - (FIRWriteBatch *)setData:(NSDictionary *)data mergeFields:(NSArray *)mergeFields { [self verifyNotCommitted]; [self validateReference:document]; + ParsedSetData parsed = [self.firestore.dataConverter parsedMergeData:data fieldMask:mergeFields]; - [self.mutations - addObjectsFromArray:std::move(parsed).ToMutations(document.key, Precondition::None())]; + std::vector append_mutations = + std::move(parsed).ToMutations(document.key, Precondition::None()); + std::move(append_mutations.begin(), append_mutations.end(), std::back_inserter(_mutations)); + return self; } @@ -97,17 +105,22 @@ - (FIRWriteBatch *)updateData:(NSDictionary *)fields forDocument:(FIRDocumentReference *)document { [self verifyNotCommitted]; [self validateReference:document]; + ParsedUpdateData parsed = [self.firestore.dataConverter parsedUpdateData:fields]; - [self.mutations - addObjectsFromArray:std::move(parsed).ToMutations(document.key, Precondition::Exists(true))]; + std::vector append_mutations = + std::move(parsed).ToMutations(document.key, Precondition::Exists(true)); + std::move(append_mutations.begin(), append_mutations.end(), std::back_inserter(_mutations)); + return self; } - (FIRWriteBatch *)deleteDocument:(FIRDocumentReference *)document { [self verifyNotCommitted]; [self validateReference:document]; - [self.mutations addObject:[[FSTDeleteMutation alloc] initWithKey:document.key - precondition:Precondition::None()]]; + + _mutations.push_back([[FSTDeleteMutation alloc] initWithKey:document.key + precondition:Precondition::None()]); + ; return self; } @@ -118,7 +131,7 @@ - (void)commit { - (void)commitWithCompletion:(nullable void (^)(NSError *_Nullable error))completion { [self verifyNotCommitted]; self.committed = TRUE; - [self.firestore.client writeMutations:self.mutations completion:completion]; + [self.firestore.client writeMutations:std::move(_mutations) completion:completion]; } - (void)verifyNotCommitted { diff --git a/Firestore/Source/Core/FSTFirestoreClient.h b/Firestore/Source/Core/FSTFirestoreClient.h index 1d2e3abe66c..233e7c2d027 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.h +++ b/Firestore/Source/Core/FSTFirestoreClient.h @@ -17,6 +17,7 @@ #import #include +#include #import "Firestore/Source/Core/FSTTypes.h" #import "Firestore/Source/Core/FSTViewSnapshot.h" @@ -100,7 +101,7 @@ NS_ASSUME_NONNULL_BEGIN NSError *_Nullable error))completion; /** Write mutations. completion will be notified when it's written to the backend. */ -- (void)writeMutations:(NSArray *)mutations +- (void)writeMutations:(std::vector &&)mutations completion:(nullable FSTVoidErrorBlock)completion; /** Tries to execute the transaction in updateBlock up to retries times. */ diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm index 4de9a6a39f2..5e720f7c90b 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.mm +++ b/Firestore/Source/Core/FSTFirestoreClient.mm @@ -386,15 +386,16 @@ - (void)getDocumentsFromLocalCache:(FIRQuery *)query }); } -- (void)writeMutations:(NSArray *)mutations +- (void)writeMutations:(std::vector &&)mutations completion:(nullable FSTVoidErrorBlock)completion { - _workerQueue->Enqueue([self, mutations, completion] { - if (mutations.count == 0) { + // TODO(c++14): move `mutations` into lambda (C++14). + _workerQueue->Enqueue([self, mutations, completion]() mutable { + if (mutations.empty()) { if (completion) { self->_userExecutor->Execute([=] { completion(nil); }); } } else { - [self.syncEngine writeMutations:mutations + [self.syncEngine writeMutations:std::move(mutations) completion:^(NSError *error) { // Dispatch the result back onto the user dispatch queue. if (completion) { diff --git a/Firestore/Source/Core/FSTSyncEngine.h b/Firestore/Source/Core/FSTSyncEngine.h index c99b06616f4..85e56b2b02b 100644 --- a/Firestore/Source/Core/FSTSyncEngine.h +++ b/Firestore/Source/Core/FSTSyncEngine.h @@ -16,6 +16,8 @@ #import +#include + #import "Firestore/Source/Core/FSTTypes.h" #import "Firestore/Source/Remote/FSTRemoteStore.h" @@ -90,7 +92,8 @@ NS_ASSUME_NONNULL_BEGIN * write caused. The provided completion block will be called once the write has been acked or * rejected by the backend (or failed locally for any other reason). */ -- (void)writeMutations:(NSArray *)mutations completion:(FSTVoidErrorBlock)completion; +- (void)writeMutations:(std::vector &&)mutations + completion:(FSTVoidErrorBlock)completion; /** * Runs the given transaction block up to retries times and then calls completion. diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index 86794d0fec5..576b2b1c710 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -247,11 +247,11 @@ - (void)stopListeningToQuery:(FSTQuery *)query { [self removeAndCleanupQuery:queryView]; } -- (void)writeMutations:(NSArray *)mutations +- (void)writeMutations:(std::vector &&)mutations completion:(FSTVoidErrorBlock)completion { [self assertDelegateExistsForSelector:_cmd]; - FSTLocalWriteResult *result = [self.localStore locallyWriteMutations:mutations]; + FSTLocalWriteResult *result = [self.localStore locallyWriteMutations:std::move(mutations)]; [self addMutationCompletionBlock:completion batchID:result.batchID]; [self emitNewSnapshotsAndNotifyLocalStoreWithChanges:result.changes remoteEvent:absl::nullopt]; diff --git a/Firestore/Source/Core/FSTTransaction.h b/Firestore/Source/Core/FSTTransaction.h index 352fa7a4f55..64e10607776 100644 --- a/Firestore/Source/Core/FSTTransaction.h +++ b/Firestore/Source/Core/FSTTransaction.h @@ -18,16 +18,10 @@ #include -#import "Firestore/Source/Core/FSTTypes.h" - #include "Firestore/core/src/firebase/firestore/core/user_data.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/remote/datastore.h" -@class FSTMaybeDocument; -@class FSTObjectValue; -@class FSTParsedUpdateData; - NS_ASSUME_NONNULL_BEGIN #pragma mark - FSTTransaction diff --git a/Firestore/Source/Core/FSTTransaction.mm b/Firestore/Source/Core/FSTTransaction.mm index 2ee220705cf..34a6c531f3f 100644 --- a/Firestore/Source/Core/FSTTransaction.mm +++ b/Firestore/Source/Core/FSTTransaction.mm @@ -16,17 +16,16 @@ #import "Firestore/Source/Core/FSTTransaction.h" +#include #include #include #include #import "FIRFirestoreErrors.h" -#import "Firestore/Source/API/FSTUserDataConverter.h" #import "Firestore/Source/Model/FSTDocument.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Util/FSTUsageValidation.h" -#include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/document_key_set.h" #include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" @@ -46,7 +45,6 @@ #pragma mark - FSTTransaction @interface FSTTransaction () -@property(nonatomic, strong, readonly) NSMutableArray *mutations; @property(nonatomic, assign) BOOL commitCalled; /** * An error that may have occurred as a consequence of a write. If set, needs to be raised in the @@ -57,6 +55,7 @@ @interface FSTTransaction () @implementation FSTTransaction { Datastore *_datastore; + std::vector _mutations; std::map _readVersions; } @@ -68,7 +67,6 @@ - (instancetype)initWithDatastore:(Datastore *)datastore { self = [super init]; if (self) { _datastore = datastore; - _mutations = [NSMutableArray array]; _commitCalled = NO; } return self; @@ -113,7 +111,7 @@ - (BOOL)recordVersionForDocument:(FSTMaybeDocument *)doc error:(NSError **)error - (void)lookupDocumentsForKeys:(const std::vector &)keys completion:(FSTVoidMaybeDocumentArrayErrorBlock)completion { [self ensureCommitNotCalled]; - if (self.mutations.count) { + if (!_mutations.empty()) { FSTThrowInvalidUsage(@"FIRIllegalStateException", @"All reads in a transaction must be done before any writes."); } @@ -135,9 +133,9 @@ - (void)lookupDocumentsForKeys:(const std::vector &)keys } /** Stores mutations to be written when commitWithCompletion is called. */ -- (void)writeMutations:(NSArray *)mutations { +- (void)writeMutations:(const std::vector &)mutations { [self ensureCommitNotCalled]; - [self.mutations addObjectsFromArray:mutations]; + std::move(mutations.begin(), mutations.end(), std::back_inserter(_mutations)); } /** @@ -201,9 +199,9 @@ - (void)updateData:(ParsedUpdateData &&)data forDocument:(const DocumentKey &)ke } - (void)deleteDocument:(const DocumentKey &)key { - [self writeMutations:@[ [[FSTDeleteMutation alloc] + [self writeMutations:{[[FSTDeleteMutation alloc] initWithKey:key - precondition:[self preconditionForDocumentKey:key]] ]]; + precondition:[self preconditionForDocumentKey:key]]}]; // Since the delete will be applied before all following writes, we need to ensure that the // precondition for the next write will be exists without timestamp. _readVersions[key] = SnapshotVersion::None(); @@ -226,7 +224,7 @@ - (void)commitWithCompletion:(FSTVoidErrorBlock)completion { unwritten = unwritten.insert(kv.first); }; // For each mutation, note that the doc was written. - for (FSTMutation *mutation in self.mutations) { + for (FSTMutation *mutation : _mutations) { unwritten = unwritten.erase(mutation.key); } if (!unwritten.empty()) { @@ -239,7 +237,7 @@ - (void)commitWithCompletion:(FSTVoidErrorBlock)completion { @"written in that transaction." }]); } else { - _datastore->CommitMutations(self.mutations, ^(NSError *_Nullable error) { + _datastore->CommitMutations(_mutations, ^(NSError *_Nullable error) { if (error) { completion(error); } else { diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index eb11d3ca0ab..a357f5b5f4f 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -166,7 +166,7 @@ - (DocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { _mutationQueue->AllMutationBatchesAffectingQuery(query); for (FSTMutationBatch *batch : matchingBatches) { - for (FSTMutation *mutation in batch.mutations) { + for (FSTMutation *mutation : [batch mutations]) { // Only process documents belonging to the collection. if (!query.path.IsImmediateParentOf(mutation.key.path())) { continue; diff --git a/Firestore/Source/Local/FSTLocalSerializer.mm b/Firestore/Source/Local/FSTLocalSerializer.mm index 5ad70e43c47..528af368266 100644 --- a/Firestore/Source/Local/FSTLocalSerializer.mm +++ b/Firestore/Source/Local/FSTLocalSerializer.mm @@ -17,6 +17,8 @@ #import "Firestore/Source/Local/FSTLocalSerializer.h" #include +#include +#include #import "FIRTimestamp.h" #import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h" @@ -182,7 +184,7 @@ - (FSTPBWriteBatch *)encodedMutationBatch:(FSTMutationBatch *)batch { encodedTimestamp:Timestamp{batch.localWriteTime.seconds, batch.localWriteTime.nanoseconds}]; NSMutableArray *writes = proto.writesArray; - for (FSTMutation *mutation in batch.mutations) { + for (FSTMutation *mutation : [batch mutations]) { [writes addObject:[remoteSerializer encodedMutation:mutation]]; } return proto; @@ -192,9 +194,9 @@ - (FSTMutationBatch *)decodedMutationBatch:(FSTPBWriteBatch *)batch { FSTSerializerBeta *remoteSerializer = self.remoteSerializer; int batchID = batch.batchId; - NSMutableArray *mutations = [NSMutableArray array]; + std::vector mutations; for (GCFSWrite *write in batch.writesArray) { - [mutations addObject:[remoteSerializer decodedMutation:write]]; + mutations.push_back([remoteSerializer decodedMutation:write]); } Timestamp localWriteTime = [remoteSerializer decodedTimestamp:batch.localWriteTime]; @@ -203,7 +205,7 @@ - (FSTMutationBatch *)decodedMutationBatch:(FSTPBWriteBatch *)batch { initWithBatchID:batchID localWriteTime:[FIRTimestamp timestampWithSeconds:localWriteTime.seconds() nanoseconds:localWriteTime.nanoseconds()] - mutations:mutations]; + mutations:std::move(mutations)]; } - (FSTPBTarget *)encodedQueryData:(FSTQueryData *)queryData { diff --git a/Firestore/Source/Local/FSTLocalStore.h b/Firestore/Source/Local/FSTLocalStore.h index 21f05b68e4c..8d696fe9c95 100644 --- a/Firestore/Source/Local/FSTLocalStore.h +++ b/Firestore/Source/Local/FSTLocalStore.h @@ -16,6 +16,8 @@ #import +#include + #import "Firestore/Source/Local/FSTLRUGarbageCollector.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" @@ -103,7 +105,7 @@ NS_ASSUME_NONNULL_BEGIN (const firebase::firestore::auth::User &)user; /** Accepts locally generated Mutations and commits them to storage. */ -- (FSTLocalWriteResult *)locallyWriteMutations:(NSArray *)mutations; +- (FSTLocalWriteResult *)locallyWriteMutations:(std::vector &&)mutations; /** Returns the current value of a document with a given key, or nil if not found. */ - (nullable FSTMaybeDocument *)readDocument:(const firebase::firestore::model::DocumentKey &)key; diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index 22d9aa283e0..eabe3c58080 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -152,7 +152,7 @@ - (MaybeDocumentMap)userDidChange:(const User &)user { DocumentKeySet changedKeys; for (const std::vector &batches : {oldBatches, newBatches}) { for (FSTMutationBatch *batch : batches) { - for (FSTMutation *mutation in batch.mutations) { + for (FSTMutation *mutation : [batch mutations]) { changedKeys = changedKeys.insert(mutation.key); } } @@ -163,10 +163,11 @@ - (MaybeDocumentMap)userDidChange:(const User &)user { }); } -- (FSTLocalWriteResult *)locallyWriteMutations:(NSArray *)mutations { +- (FSTLocalWriteResult *)locallyWriteMutations:(std::vector &&)mutations { return self.persistence.run("Locally write mutations", [&]() -> FSTLocalWriteResult * { FIRTimestamp *localWriteTime = [FIRTimestamp timestamp]; - FSTMutationBatch *batch = _mutationQueue->AddMutationBatch(localWriteTime, mutations); + FSTMutationBatch *batch = + _mutationQueue->AddMutationBatch(localWriteTime, std::move(mutations)); DocumentKeySet keys = [batch keys]; MaybeDocumentMap changedDocuments = [self.localDocuments documentsForKeys:keys]; return [FSTLocalWriteResult resultForBatchID:batch.batchID changes:std::move(changedDocuments)]; diff --git a/Firestore/Source/Model/FSTMutationBatch.h b/Firestore/Source/Model/FSTMutationBatch.h index ac8212db47a..64c55dafe96 100644 --- a/Firestore/Source/Model/FSTMutationBatch.h +++ b/Firestore/Source/Model/FSTMutationBatch.h @@ -53,7 +53,7 @@ NS_ASSUME_NONNULL_BEGIN /** Initializes a mutation batch with the given batchID, localWriteTime, and mutations. */ - (instancetype)initWithBatchID:(firebase::firestore::model::BatchId)batchID localWriteTime:(FIRTimestamp *)localWriteTime - mutations:(NSArray *)mutations NS_DESIGNATED_INITIALIZER; + mutations:(std::vector &&)mutations NS_DESIGNATED_INITIALIZER; - (id)init NS_UNAVAILABLE; @@ -83,9 +83,10 @@ NS_ASSUME_NONNULL_BEGIN /** Returns the set of unique keys referenced by all mutations in the batch. */ - (firebase::firestore::model::DocumentKeySet)keys; +- (const std::vector &)mutations; + @property(nonatomic, assign, readonly) firebase::firestore::model::BatchId batchID; @property(nonatomic, strong, readonly) FIRTimestamp *localWriteTime; -@property(nonatomic, strong, readonly) NSArray *mutations; @end diff --git a/Firestore/Source/Model/FSTMutationBatch.mm b/Firestore/Source/Model/FSTMutationBatch.mm index c76c5e3fd2f..2074a79030e 100644 --- a/Firestore/Source/Model/FSTMutationBatch.mm +++ b/Firestore/Source/Model/FSTMutationBatch.mm @@ -16,6 +16,7 @@ #import "Firestore/Source/Model/FSTMutationBatch.h" +#include #include #import "FIRTimestamp.h" @@ -24,6 +25,7 @@ #import "Firestore/Source/Model/FSTMutation.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" +#include "Firestore/core/src/firebase/firestore/util/hashing.h" using firebase::firestore::model::BatchId; using firebase::firestore::model::DocumentKey; @@ -31,24 +33,31 @@ using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::DocumentVersionMap; using firebase::firestore::model::SnapshotVersion; +using firebase::firestore::util::Hash; NS_ASSUME_NONNULL_BEGIN -@implementation FSTMutationBatch +@implementation FSTMutationBatch { + std::vector _mutations; +} - (instancetype)initWithBatchID:(BatchId)batchID localWriteTime:(FIRTimestamp *)localWriteTime - mutations:(NSArray *)mutations { - HARD_ASSERT(mutations.count != 0, "Cannot create an empty mutation batch"); + mutations:(std::vector &&)mutations { + HARD_ASSERT(!mutations.empty(), "Cannot create an empty mutation batch"); self = [super init]; if (self) { _batchID = batchID; _localWriteTime = localWriteTime; - _mutations = mutations; + _mutations = std::move(mutations); } return self; } +- (const std::vector &)mutations { + return _mutations; +} + - (BOOL)isEqual:(id)other { if (self == other) { return YES; @@ -59,19 +68,27 @@ - (BOOL)isEqual:(id)other { FSTMutationBatch *otherBatch = (FSTMutationBatch *)other; return self.batchID == otherBatch.batchID && [self.localWriteTime isEqual:otherBatch.localWriteTime] && - [self.mutations isEqual:otherBatch.mutations]; + std::equal(_mutations.begin(), _mutations.end(), otherBatch.mutations.begin(), + [](FSTMutation *lhs, FSTMutation *rhs) { return [lhs isEqual:rhs]; }); } - (NSUInteger)hash { NSUInteger result = (NSUInteger)self.batchID; result = result * 31 + self.localWriteTime.hash; - result = result * 31 + self.mutations.hash; + for (FSTMutation *mutation : _mutations) { + result = result * 31 + [mutation hash]; + } return result; } - (NSString *)description { + // TODO(varconst): quick-and-dirty-way to create a readable description. + NSMutableArray *mutationsCopy = [NSMutableArray array]; + for (FSTMutation *mutation : _mutations) { + [mutationsCopy addObject:mutation]; + } return [NSString stringWithFormat:@"", - self.batchID, self.localWriteTime, self.mutations]; + self.batchID, self.localWriteTime, mutationsCopy]; } - (FSTMaybeDocument *_Nullable)applyToRemoteDocument:(FSTMaybeDocument *_Nullable)maybeDoc @@ -82,12 +99,12 @@ - (FSTMaybeDocument *_Nullable)applyToRemoteDocument:(FSTMaybeDocument *_Nullabl "applyTo: key %s doesn't match maybeDoc key %s", documentKey.ToString(), maybeDoc.key.ToString()); - HARD_ASSERT(mutationBatchResult.mutationResults.size() == self.mutations.count, - "Mismatch between mutations length (%s) and results length (%s)", - self.mutations.count, mutationBatchResult.mutationResults.size()); + HARD_ASSERT(mutationBatchResult.mutationResults.size() == _mutations.size(), + "Mismatch between mutations length (%s) and results length (%s)", _mutations.size(), + mutationBatchResult.mutationResults.size()); - for (NSUInteger i = 0; i < self.mutations.count; i++) { - FSTMutation *mutation = self.mutations[i]; + for (size_t i = 0; i < _mutations.size(); i++) { + FSTMutation *mutation = _mutations[i]; FSTMutationResult *mutationResult = mutationBatchResult.mutationResults[i]; if (mutation.key == documentKey) { maybeDoc = [mutation applyToRemoteDocument:maybeDoc mutationResult:mutationResult]; @@ -103,8 +120,7 @@ - (FSTMaybeDocument *_Nullable)applyToLocalDocument:(FSTMaybeDocument *_Nullable maybeDoc.key.ToString()); FSTMaybeDocument *baseDoc = maybeDoc; - for (NSUInteger i = 0; i < self.mutations.count; i++) { - FSTMutation *mutation = self.mutations[i]; + for (FSTMutation *mutation : _mutations) { if (mutation.key == documentKey) { maybeDoc = [mutation applyToLocalDocument:maybeDoc baseDocument:baseDoc @@ -114,10 +130,9 @@ - (FSTMaybeDocument *_Nullable)applyToLocalDocument:(FSTMaybeDocument *_Nullable return maybeDoc; } -// TODO(klimt): This could use NSMutableDictionary instead. - (DocumentKeySet)keys { DocumentKeySet set; - for (FSTMutation *mutation in self.mutations) { + for (FSTMutation *mutation : _mutations) { set = set.insert(mutation.key); } return set; @@ -172,13 +187,13 @@ + (instancetype)resultWithBatch:(FSTMutationBatch *)batch commitVersion:(SnapshotVersion)commitVersion mutationResults:(std::vector)mutationResults streamToken:(nullable NSData *)streamToken { - HARD_ASSERT(batch.mutations.count == mutationResults.size(), - "Mutations sent %s must equal results received %s", batch.mutations.count, + HARD_ASSERT(batch.mutations.size() == mutationResults.size(), + "Mutations sent %s must equal results received %s", batch.mutations.size(), mutationResults.size()); DocumentVersionMap docVersions; - NSArray *mutations = batch.mutations; - for (NSUInteger i = 0; i < mutations.count; i++) { + std::vector mutations = batch.mutations; + for (size_t i = 0; i < mutations.size(); i++) { absl::optional version = mutationResults[i].version; if (!version) { // deletes don't have a version, so we substitute the commitVersion diff --git a/Firestore/core/src/firebase/firestore/core/user_data.h b/Firestore/core/src/firebase/firestore/core/user_data.h index cdb059af5c0..d3c3bf94b1e 100644 --- a/Firestore/core/src/firebase/firestore/core/user_data.h +++ b/Firestore/core/src/firebase/firestore/core/user_data.h @@ -266,7 +266,7 @@ class ParsedSetData { * * This method consumes the values stored in the ParsedSetData */ - NSArray* ToMutations( + std::vector ToMutations( const model::DocumentKey& key, const model::Precondition& precondition) &&; @@ -299,7 +299,7 @@ class ParsedUpdateData { * * This method consumes the values stored in the ParsedUpdateData */ - NSArray* ToMutations( + std::vector ToMutations( const model::DocumentKey& key, const model::Precondition& precondition) &&; diff --git a/Firestore/core/src/firebase/firestore/core/user_data.mm b/Firestore/core/src/firebase/firestore/core/user_data.mm index 525c0ee212d..237e49f234c 100644 --- a/Firestore/core/src/firebase/firestore/core/user_data.mm +++ b/Firestore/core/src/firebase/firestore/core/user_data.mm @@ -210,25 +210,30 @@ patch_{true} { } -NSArray* ParsedSetData::ToMutations( +std::vector ParsedSetData::ToMutations( const DocumentKey& key, const Precondition& precondition) && { - NSMutableArray* mutations = [NSMutableArray array]; + std::vector mutations; if (patch_) { - [mutations - addObject:[[FSTPatchMutation alloc] initWithKey:key - fieldMask:std::move(field_mask_) - value:data_ - precondition:precondition]]; + FSTMutation* mutation = + [[FSTPatchMutation alloc] initWithKey:key + fieldMask:std::move(field_mask_) + value:data_ + precondition:precondition]; + mutations.push_back(mutation); } else { - [mutations addObject:[[FSTSetMutation alloc] initWithKey:key - value:data_ - precondition:precondition]]; + FSTMutation* mutation = [[FSTSetMutation alloc] initWithKey:key + value:data_ + precondition:precondition]; + mutations.push_back(mutation); } + if (!field_transforms_.empty()) { - [mutations - addObject:[[FSTTransformMutation alloc] initWithKey:key - fieldTransforms:field_transforms_]]; + FSTMutation* mutation = + [[FSTTransformMutation alloc] initWithKey:key + fieldTransforms:field_transforms_]; + mutations.push_back(mutation); } + return mutations; } @@ -243,19 +248,24 @@ field_transforms_{std::move(field_transforms)} { } -NSArray* ParsedUpdateData::ToMutations( +std::vector ParsedUpdateData::ToMutations( const DocumentKey& key, const Precondition& precondition) && { - NSMutableArray* mutations = [NSMutableArray array]; - [mutations - addObject:[[FSTPatchMutation alloc] initWithKey:key - fieldMask:std::move(field_mask_) - value:data_ - precondition:precondition]]; + std::vector mutations; + + FSTMutation* mutation = + [[FSTPatchMutation alloc] initWithKey:key + fieldMask:std::move(field_mask_) + value:data_ + precondition:precondition]; + mutations.push_back(mutation); + if (!field_transforms_.empty()) { - [mutations addObject:[[FSTTransformMutation alloc] - initWithKey:key - fieldTransforms:std::move(field_transforms_)]]; + FSTMutation* mutation = + [[FSTTransformMutation alloc] initWithKey:key + fieldTransforms:std::move(field_transforms_)]; + mutations.push_back(mutation); } + return mutations; } diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.h b/Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.h index ef205ef276b..c8b00e572ed 100644 --- a/Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.h +++ b/Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.h @@ -70,8 +70,9 @@ class LevelDbMutationQueue : public MutationQueue { void AcknowledgeBatch(FSTMutationBatch* batch, NSData* _Nullable stream_token) override; - FSTMutationBatch* AddMutationBatch(FIRTimestamp* local_write_time, - NSArray* mutations) override; + FSTMutationBatch* AddMutationBatch( + FIRTimestamp* local_write_time, + std::vector&& mutations) override; void RemoveMutationBatch(FSTMutationBatch* batch) override; diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.mm b/Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.mm index b020c563dab..9592cd724c6 100644 --- a/Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.mm +++ b/Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.mm @@ -17,6 +17,7 @@ #include "Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.h" #include +#include #import "Firestore/Protos/objc/firestore/local/Mutation.pbobjc.h" #import "Firestore/Source/Core/FSTQuery.h" @@ -154,14 +155,14 @@ BatchId LoadNextBatchIdFromDb(DB* db) { } FSTMutationBatch* LevelDbMutationQueue::AddMutationBatch( - FIRTimestamp* local_write_time, NSArray* mutations) { + FIRTimestamp* local_write_time, std::vector&& mutations) { BatchId batch_id = next_batch_id_; next_batch_id_++; FSTMutationBatch* batch = [[FSTMutationBatch alloc] initWithBatchID:batch_id localWriteTime:local_write_time - mutations:mutations]; + mutations:std::move(mutations)]; std::string key = mutation_batch_key(batch_id); db_.currentTransaction->Put(key, [serializer_ encodedMutationBatch:batch]); @@ -171,7 +172,7 @@ BatchId LoadNextBatchIdFromDb(DB* db) { // buffer (and the parser will see all default values). std::string empty_buffer; - for (FSTMutation* mutation in mutations) { + for (FSTMutation* mutation : [batch mutations]) { key = LevelDbDocumentMutationKey::Key(user_id_, mutation.key, batch_id); db_.currentTransaction->Put(key, empty_buffer); } @@ -197,7 +198,7 @@ BatchId LoadNextBatchIdFromDb(DB* db) { db_.currentTransaction->Delete(key); - for (FSTMutation* mutation in batch.mutations) { + for (FSTMutation* mutation : [batch mutations]) { key = LevelDbDocumentMutationKey::Key(user_id_, mutation.key, batch_id); db_.currentTransaction->Delete(key); [db_.referenceDelegate removeMutationReference:mutation.key]; @@ -468,4 +469,4 @@ BatchId LoadNextBatchIdFromDb(DB* db) { } // namespace firestore } // namespace firebase -NS_ASSUME_NONNULL_END \ No newline at end of file +NS_ASSUME_NONNULL_END diff --git a/Firestore/core/src/firebase/firestore/local/memory_mutation_queue.h b/Firestore/core/src/firebase/firestore/local/memory_mutation_queue.h index a47a2899576..cf863d23092 100644 --- a/Firestore/core/src/firebase/firestore/local/memory_mutation_queue.h +++ b/Firestore/core/src/firebase/firestore/local/memory_mutation_queue.h @@ -58,8 +58,9 @@ class MemoryMutationQueue : public MutationQueue { void AcknowledgeBatch(FSTMutationBatch* batch, NSData* _Nullable stream_token) override; - FSTMutationBatch* AddMutationBatch(FIRTimestamp* local_write_time, - NSArray* mutations) override; + FSTMutationBatch* AddMutationBatch( + FIRTimestamp* local_write_time, + std::vector&& mutations) override; void RemoveMutationBatch(FSTMutationBatch* batch) override; diff --git a/Firestore/core/src/firebase/firestore/local/memory_mutation_queue.mm b/Firestore/core/src/firebase/firestore/local/memory_mutation_queue.mm index 5979976d4fb..9f5d9bd89f2 100644 --- a/Firestore/core/src/firebase/firestore/local/memory_mutation_queue.mm +++ b/Firestore/core/src/firebase/firestore/local/memory_mutation_queue.mm @@ -16,6 +16,8 @@ #include "Firestore/core/src/firebase/firestore/local/memory_mutation_queue.h" +#include + #import "Firestore/Protos/objc/firestore/local/Mutation.pbobjc.h" #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTLocalSerializer.h" @@ -72,8 +74,8 @@ } FSTMutationBatch* MemoryMutationQueue::AddMutationBatch( - FIRTimestamp* local_write_time, NSArray* mutations) { - HARD_ASSERT(mutations.count > 0, "Mutation batches should not be empty"); + FIRTimestamp* local_write_time, std::vector&& mutations) { + HARD_ASSERT(!mutations.empty(), "Mutation batches should not be empty"); BatchId batch_id = next_batch_id_; next_batch_id_++; @@ -87,11 +89,11 @@ FSTMutationBatch* batch = [[FSTMutationBatch alloc] initWithBatchID:batch_id localWriteTime:local_write_time - mutations:mutations]; + mutations:std::move(mutations)]; queue_.push_back(batch); // Track references by document key. - for (FSTMutation* mutation in batch.mutations) { + for (FSTMutation* mutation : [batch mutations]) { batches_by_document_key_ = batches_by_document_key_.insert( DocumentReference{mutation.key, batch_id}); } @@ -109,7 +111,7 @@ queue_.erase(queue_.begin()); // Remove entries from the index too. - for (FSTMutation* mutation in batch.mutations) { + for (FSTMutation* mutation : [batch mutations]) { const DocumentKey& key = mutation.key; [persistence_.referenceDelegate removeMutationReference:key]; diff --git a/Firestore/core/src/firebase/firestore/local/mutation_queue.h b/Firestore/core/src/firebase/firestore/local/mutation_queue.h index 784a7e55f56..3e66a7c082d 100644 --- a/Firestore/core/src/firebase/firestore/local/mutation_queue.h +++ b/Firestore/core/src/firebase/firestore/local/mutation_queue.h @@ -61,7 +61,8 @@ class MutationQueue { /** Creates a new mutation batch and adds it to this mutation queue. */ virtual FSTMutationBatch* AddMutationBatch( - FIRTimestamp* local_write_time, NSArray* mutations) = 0; + FIRTimestamp* local_write_time, + std::vector&& mutations) = 0; /** * Removes the given mutation batch from the queue. This is useful in two diff --git a/Firestore/core/src/firebase/firestore/remote/datastore.h b/Firestore/core/src/firebase/firestore/remote/datastore.h index 1c9c592977a..35938eff336 100644 --- a/Firestore/core/src/firebase/firestore/remote/datastore.h +++ b/Firestore/core/src/firebase/firestore/remote/datastore.h @@ -92,7 +92,7 @@ class Datastore : public std::enable_shared_from_this { virtual std::shared_ptr CreateWriteStream( WriteStreamCallback* callback); - void CommitMutations(NSArray* mutations, + void CommitMutations(const std::vector& mutations, FSTVoidErrorBlock completion); void LookupDocuments(const std::vector& keys, FSTVoidMaybeDocumentArrayErrorBlock completion); @@ -155,9 +155,10 @@ class Datastore : public std::enable_shared_from_this { private: void PollGrpcQueue(); - void CommitMutationsWithCredentials(const auth::Token& token, - NSArray* mutations, - FSTVoidErrorBlock completion); + void CommitMutationsWithCredentials( + const auth::Token& token, + const std::vector& mutations, + FSTVoidErrorBlock completion); void OnCommitMutationsResponse(const util::StatusOr& result, FSTVoidErrorBlock completion); diff --git a/Firestore/core/src/firebase/firestore/remote/datastore.mm b/Firestore/core/src/firebase/firestore/remote/datastore.mm index d7f955abed8..1037a075011 100644 --- a/Firestore/core/src/firebase/firestore/remote/datastore.mm +++ b/Firestore/core/src/firebase/firestore/remote/datastore.mm @@ -164,7 +164,7 @@ void LogGrpcCallFinished(absl::string_view rpc_name, &grpc_connection_, callback); } -void Datastore::CommitMutations(NSArray* mutations, +void Datastore::CommitMutations(const std::vector& mutations, FSTVoidErrorBlock completion) { ResumeRpcWithCredentials( [this, mutations, completion](const StatusOr& maybe_credentials) { @@ -177,9 +177,10 @@ void LogGrpcCallFinished(absl::string_view rpc_name, }); } -void Datastore::CommitMutationsWithCredentials(const Token& token, - NSArray* mutations, - FSTVoidErrorBlock completion) { +void Datastore::CommitMutationsWithCredentials( + const Token& token, + const std::vector& mutations, + FSTVoidErrorBlock completion) { grpc::ByteBuffer message = serializer_bridge_.ToByteBuffer( serializer_bridge_.CreateCommitRequest(mutations)); diff --git a/Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.h b/Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.h index 63f23e03781..7330dbe04c0 100644 --- a/Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.h +++ b/Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.h @@ -110,9 +110,9 @@ class WriteStreamSerializer { GCFSWriteRequest* CreateHandshake() const; GCFSWriteRequest* CreateWriteMutationsRequest( - NSArray* mutations) const; + const std::vector& mutations) const; GCFSWriteRequest* CreateEmptyMutationsList() { - return CreateWriteMutationsRequest(@[]); + return CreateWriteMutationsRequest({}); } static grpc::ByteBuffer ToByteBuffer(GCFSWriteRequest* request); @@ -146,7 +146,7 @@ class DatastoreSerializer { explicit DatastoreSerializer(const core::DatabaseInfo& database_info); GCFSCommitRequest* CreateCommitRequest( - NSArray* mutations) const; + const std::vector& mutations) const; static grpc::ByteBuffer ToByteBuffer(GCFSCommitRequest* request); GCFSBatchGetDocumentsRequest* CreateLookupRequest( diff --git a/Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.mm b/Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.mm index c2f0882b807..169d135b148 100644 --- a/Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.mm +++ b/Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.mm @@ -179,10 +179,10 @@ bool IsLoggingEnabled() { } GCFSWriteRequest* WriteStreamSerializer::CreateWriteMutationsRequest( - NSArray* mutations) const { + const std::vector& mutations) const { NSMutableArray* protos = - [NSMutableArray arrayWithCapacity:mutations.count]; - for (FSTMutation* mutation in mutations) { + [NSMutableArray arrayWithCapacity:mutations.size()]; + for (FSTMutation* mutation : mutations) { [protos addObject:[serializer_ encodedMutation:mutation]]; }; @@ -238,12 +238,12 @@ bool IsLoggingEnabled() { } GCFSCommitRequest* DatastoreSerializer::CreateCommitRequest( - NSArray* mutations) const { + const std::vector& mutations) const { GCFSCommitRequest* request = [GCFSCommitRequest message]; request.database = [serializer_ encodedDatabaseID]; NSMutableArray* mutationProtos = [NSMutableArray array]; - for (FSTMutation* mutation in mutations) { + for (FSTMutation* mutation : mutations) { [mutationProtos addObject:[serializer_ encodedMutation:mutation]]; } request.writesArray = mutationProtos; diff --git a/Firestore/core/src/firebase/firestore/remote/write_stream.h b/Firestore/core/src/firebase/firestore/remote/write_stream.h index 8e85bcf1693..bfa75304ab5 100644 --- a/Firestore/core/src/firebase/firestore/remote/write_stream.h +++ b/Firestore/core/src/firebase/firestore/remote/write_stream.h @@ -131,7 +131,7 @@ class WriteStream : public Stream { virtual void WriteHandshake(); /** Sends a group of mutations to the Firestore backend to apply. */ - virtual void WriteMutations(NSArray* mutations); + virtual void WriteMutations(const std::vector& mutations); protected: // For tests only diff --git a/Firestore/core/src/firebase/firestore/remote/write_stream.mm b/Firestore/core/src/firebase/firestore/remote/write_stream.mm index db778509fda..7e0513a5b47 100644 --- a/Firestore/core/src/firebase/firestore/remote/write_stream.mm +++ b/Firestore/core/src/firebase/firestore/remote/write_stream.mm @@ -65,7 +65,7 @@ // stream token on the handshake, ignoring any stream token we might have. } -void WriteStream::WriteMutations(NSArray* mutations) { +void WriteStream::WriteMutations(const std::vector& mutations) { EnsureOnQueue(); HARD_ASSERT(IsOpen(), "Writing mutations requires an opened stream"); HARD_ASSERT(handshake_complete(), diff --git a/Firestore/core/test/firebase/firestore/remote/datastore_test.mm b/Firestore/core/test/firebase/firestore/remote/datastore_test.mm index b8d376959c2..847865f49f5 100644 --- a/Firestore/core/test/firebase/firestore/remote/datastore_test.mm +++ b/Firestore/core/test/firebase/firestore/remote/datastore_test.mm @@ -174,7 +174,7 @@ void ForceFinishAnyTypeOrder( TEST_F(DatastoreTest, CommitMutationsSuccess) { __block bool done = false; __block NSError* resulting_error = nullptr; - datastore->CommitMutations(@[], ^(NSError* _Nullable error) { + datastore->CommitMutations({}, ^(NSError* _Nullable error) { done = true; resulting_error = error; }); @@ -246,7 +246,7 @@ void ForceFinishAnyTypeOrder( TEST_F(DatastoreTest, CommitMutationsError) { __block bool done = false; __block NSError* resulting_error = nullptr; - datastore->CommitMutations(@[], ^(NSError* _Nullable error) { + datastore->CommitMutations({}, ^(NSError* _Nullable error) { done = true; resulting_error = error; }); @@ -307,7 +307,7 @@ void ForceFinishAnyTypeOrder( credentials.FailGetToken(); __block NSError* resulting_error = nullptr; - datastore->CommitMutations(@[], ^(NSError* _Nullable error) { + datastore->CommitMutations({}, ^(NSError* _Nullable error) { resulting_error = error; }); worker_queue.EnqueueBlocking([] {}); @@ -330,7 +330,7 @@ void ForceFinishAnyTypeOrder( credentials.DelayGetToken(); worker_queue.EnqueueBlocking([&] { - datastore->CommitMutations(@[], ^(NSError* _Nullable error) { + datastore->CommitMutations({}, ^(NSError* _Nullable error) { FAIL() << "Callback shouldn't be invoked"; }); }); @@ -343,7 +343,7 @@ void ForceFinishAnyTypeOrder( credentials.DelayGetToken(); worker_queue.EnqueueBlocking([&] { - datastore->CommitMutations(@[], ^(NSError* _Nullable error) { + datastore->CommitMutations({}, ^(NSError* _Nullable error) { FAIL() << "Callback shouldn't be invoked"; }); });