Skip to content

Pass FSTMutations* using a vector #2357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Firestore/Example/Tests/Integration/FSTDatastoreTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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];
});
Expand All @@ -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 --
Expand Down
10 changes: 6 additions & 4 deletions Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

#include <unordered_map>
#include <unordered_set>
#include <utility>
#include <vector>

#import "FIRTimestamp.h"
#import "Firestore/Example/Tests/Util/FSTHelpers.h"
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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]);
}

Expand Down
52 changes: 27 additions & 25 deletions Firestore/Example/Tests/Local/FSTLocalStoreTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#import <FirebaseFirestore/FIRTimestamp.h>
#import <XCTest/XCTest.h>

#include <utility>
#include <vector>

#import "Firestore/Source/Core/FSTQuery.h"
Expand Down Expand Up @@ -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;
}

Expand All @@ -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];
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) ]);
Expand All @@ -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),
Expand Down Expand Up @@ -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"},
Expand All @@ -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(
Expand All @@ -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), (@[
Expand Down Expand Up @@ -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")};
Expand Down
55 changes: 24 additions & 31 deletions Firestore/Example/Tests/Local/FSTMutationQueueTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#import <FirebaseFirestore/FIRTimestamp.h>

#include <set>
#include <utility>
#include <vector>

#import "Firestore/Source/Core/FSTQuery.h"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
});
}

Expand All @@ -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];
}

Expand All @@ -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"),
Expand All @@ -281,7 +274,7 @@ - (void)testAllMutationBatchesAffectingDocumentKeys_handlesOverlap {
std::vector<FSTMutationBatch *> matches =
self.mutationQueue->AllMutationBatchesAffectingDocumentKeys(keys);

[self assertVector:matches matchesExpected:expected];
FSTAssertEqualVectors(matches, expected);
});
}

Expand All @@ -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];
}

Expand All @@ -309,7 +302,7 @@ - (void)testAllMutationBatchesAffectingQuery {
std::vector<FSTMutationBatch *> matches =
self.mutationQueue->AllMutationBatchesAffectingQuery(query);

[self assertVector:matches matchesExpected:expected];
FSTAssertEqualVectors(matches, expected);
});
}

Expand All @@ -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]);
Expand All @@ -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]);
Expand All @@ -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());

Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion Firestore/Example/Tests/SpecTests/FSTMockDatastore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading