From c46e7f2928f9af3ad7767d072d6c9d4d8ad81012 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 2 Apr 2018 16:01:03 -0700 Subject: Drop FSTRemoteDocumentChangeBuffer (#1013) --- .../Example/Firestore.xcodeproj/project.pbxproj | 4 - .../Local/FSTRemoteDocumentChangeBufferTests.mm | 118 --------------------- Firestore/Source/Local/FSTLocalStore.mm | 44 +++----- .../Source/Local/FSTRemoteDocumentChangeBuffer.h | 66 ------------ .../Source/Local/FSTRemoteDocumentChangeBuffer.mm | 93 ---------------- 5 files changed, 12 insertions(+), 313 deletions(-) delete mode 100644 Firestore/Example/Tests/Local/FSTRemoteDocumentChangeBufferTests.mm delete mode 100644 Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.h delete mode 100644 Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.mm (limited to 'Firestore') diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 7dbb45d..de4cf17 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -88,7 +88,6 @@ 5492E0A62021552D00B64F25 /* FSTPersistenceTestHelpers.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E08D2021552B00B64F25 /* FSTPersistenceTestHelpers.mm */; }; 5492E0A72021552D00B64F25 /* FSTLevelDBKeyTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E08E2021552B00B64F25 /* FSTLevelDBKeyTests.mm */; }; 5492E0A82021552D00B64F25 /* FSTLevelDBLocalStoreTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E08F2021552B00B64F25 /* FSTLevelDBLocalStoreTests.mm */; }; - 5492E0A92021552D00B64F25 /* FSTRemoteDocumentChangeBufferTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0902021552B00B64F25 /* FSTRemoteDocumentChangeBufferTests.mm */; }; 5492E0AA2021552D00B64F25 /* FSTLevelDBRemoteDocumentCacheTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0922021552B00B64F25 /* FSTLevelDBRemoteDocumentCacheTests.mm */; }; 5492E0AB2021552D00B64F25 /* StringViewTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0932021552B00B64F25 /* StringViewTests.mm */; }; 5492E0AC2021552D00B64F25 /* FSTMutationQueueTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0962021552C00B64F25 /* FSTMutationQueueTests.mm */; }; @@ -299,7 +298,6 @@ 5492E08D2021552B00B64F25 /* FSTPersistenceTestHelpers.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTPersistenceTestHelpers.mm; sourceTree = ""; }; 5492E08E2021552B00B64F25 /* FSTLevelDBKeyTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTLevelDBKeyTests.mm; sourceTree = ""; }; 5492E08F2021552B00B64F25 /* FSTLevelDBLocalStoreTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTLevelDBLocalStoreTests.mm; sourceTree = ""; }; - 5492E0902021552B00B64F25 /* FSTRemoteDocumentChangeBufferTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTRemoteDocumentChangeBufferTests.mm; sourceTree = ""; }; 5492E0912021552B00B64F25 /* FSTLocalStoreTests.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FSTLocalStoreTests.h; sourceTree = ""; }; 5492E0922021552B00B64F25 /* FSTLevelDBRemoteDocumentCacheTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTLevelDBRemoteDocumentCacheTests.mm; sourceTree = ""; }; 5492E0932021552B00B64F25 /* StringViewTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = StringViewTests.mm; sourceTree = ""; }; @@ -752,7 +750,6 @@ 5492E09A2021552C00B64F25 /* FSTReferenceSetTests.mm */, 5492E0852021552A00B64F25 /* FSTRemoteDocumentCacheTests.h */, 5492E09C2021552D00B64F25 /* FSTRemoteDocumentCacheTests.mm */, - 5492E0902021552B00B64F25 /* FSTRemoteDocumentChangeBufferTests.mm */, 5492E0932021552B00B64F25 /* StringViewTests.mm */, 132E36BB104830BD806351AC /* FSTLevelDBTransactionTests.mm */, ); @@ -1469,7 +1466,6 @@ 5492E03320213FFC00B64F25 /* FSTSyncEngineTestDriver.mm in Sources */, AB380CFE201A2F4500D97691 /* string_util_test.cc in Sources */, 5492E0A42021552D00B64F25 /* FSTMemoryQueryCacheTests.mm in Sources */, - 5492E0A92021552D00B64F25 /* FSTRemoteDocumentChangeBufferTests.mm in Sources */, 54C2294F1FECABAE007D065B /* log_test.cc in Sources */, B686F2B22025000D0028D6BE /* resource_path_test.cc in Sources */, 5492E0CA2021557E00B64F25 /* FSTWatchChangeTests.mm in Sources */, diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentChangeBufferTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentChangeBufferTests.mm deleted file mode 100644 index ae50e3e..0000000 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentChangeBufferTests.mm +++ /dev/null @@ -1,118 +0,0 @@ -/* - * Copyright 2017 Google - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#import "Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.h" - -#import - -#import "Firestore/Source/Local/FSTLevelDB.h" -#import "Firestore/Source/Local/FSTRemoteDocumentCache.h" -#import "Firestore/Source/Model/FSTDocument.h" - -#import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h" -#import "Firestore/Example/Tests/Util/FSTHelpers.h" - -NS_ASSUME_NONNULL_BEGIN - -@interface FSTRemoteDocumentChangeBufferTests : XCTestCase -@end - -@implementation FSTRemoteDocumentChangeBufferTests { - FSTLevelDB *_db; - id _remoteDocumentCache; - FSTRemoteDocumentChangeBuffer *_remoteDocumentBuffer; - - FSTMaybeDocument *_kInitialADoc; - FSTMaybeDocument *_kInitialBDoc; -} - -- (void)setUp { - [super setUp]; - - _db = [FSTPersistenceTestHelpers levelDBPersistence]; - _remoteDocumentCache = [_db remoteDocumentCache]; - - // Add a couple initial items to the cache. - _db.run("Test setup", [&]() { - _kInitialADoc = FSTTestDoc("coll/a", 42, @{@"test" : @"data"}, NO); - [_remoteDocumentCache addEntry:_kInitialADoc]; - - _kInitialBDoc = - [FSTDeletedDocument documentWithKey:FSTTestDocKey(@"coll/b") version:FSTTestVersion(314)]; - [_remoteDocumentCache addEntry:_kInitialBDoc]; - }); - - _remoteDocumentBuffer = - [FSTRemoteDocumentChangeBuffer changeBufferWithCache:_remoteDocumentCache]; -} - -- (void)tearDown { - _remoteDocumentBuffer = nil; - _remoteDocumentCache = nil; - _db = nil; - - [super tearDown]; -} - -- (void)testReadUnchangedEntry { - _db.run("testReadUnchangedEntry", [&]() { - XCTAssertEqualObjects([_remoteDocumentBuffer entryForKey:FSTTestDocKey(@"coll/a")], - _kInitialADoc); - }); -} - -- (void)testAddEntryAndReadItBack { - FSTMaybeDocument *newADoc = FSTTestDoc("coll/a", 43, @{@"new" : @"data"}, NO); - [_remoteDocumentBuffer addEntry:newADoc]; - XCTAssertEqualObjects([_remoteDocumentBuffer entryForKey:FSTTestDocKey(@"coll/a")], newADoc); - - // B should still be unchanged. - _db.run("testAddEntryAndReadItBack", [&]() { - XCTAssertEqualObjects([_remoteDocumentBuffer entryForKey:FSTTestDocKey(@"coll/b")], - _kInitialBDoc); - }); -} - -- (void)testApplyChanges { - FSTMaybeDocument *newADoc = FSTTestDoc("coll/a", 43, @{@"new" : @"data"}, NO); - [_remoteDocumentBuffer addEntry:newADoc]; - _db.run("testApplyChanges setup", [&]() { - XCTAssertEqualObjects([_remoteDocumentBuffer entryForKey:FSTTestDocKey(@"coll/a")], newADoc); - - // Reading directly against the cache should still yield the old result. - XCTAssertEqualObjects([_remoteDocumentCache entryForKey:FSTTestDocKey(@"coll/a")], - _kInitialADoc); - }); - - _db.run("testApplyChanges", [&]() { - [_remoteDocumentBuffer apply]; - - // Reading against the cache should now yield the new result. - XCTAssertEqualObjects([_remoteDocumentCache entryForKey:FSTTestDocKey(@"coll/a")], newADoc); - }); -} - -- (void)testMethodsThrowAfterApply { - _db.run("testMethodsThrowAfterApply", [&]() { [_remoteDocumentBuffer apply]; }); - - XCTAssertThrows([_remoteDocumentBuffer entryForKey:FSTTestDocKey(@"coll/a")]); - XCTAssertThrows([_remoteDocumentBuffer addEntry:_kInitialADoc]); - XCTAssertThrows([_remoteDocumentBuffer apply]); -} - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index 412ade2..990e332 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -30,7 +30,6 @@ #import "Firestore/Source/Local/FSTQueryData.h" #import "Firestore/Source/Local/FSTReferenceSet.h" #import "Firestore/Source/Local/FSTRemoteDocumentCache.h" -#import "Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.h" #import "Firestore/Source/Model/FSTDocument.h" #import "Firestore/Source/Model/FSTDocumentDictionary.h" #import "Firestore/Source/Model/FSTMutation.h" @@ -221,12 +220,7 @@ NS_ASSUME_NONNULL_BEGIN [self.heldBatchResults addObject:batchResult]; affected = [FSTDocumentKeySet keySet]; } else { - FSTRemoteDocumentChangeBuffer *remoteDocuments = - [FSTRemoteDocumentChangeBuffer changeBufferWithCache:self.remoteDocumentCache]; - - affected = [self releaseBatchResults:@[ batchResult ] remoteDocuments:remoteDocuments]; - - [remoteDocuments apply]; + affected = [self releaseBatchResults:@[ batchResult ]]; } [self.mutationQueue performConsistencyCheck]; @@ -268,9 +262,6 @@ NS_ASSUME_NONNULL_BEGIN return self.persistence.run("Apply remote event", [&]() -> FSTMaybeDocumentDictionary * { id queryCache = self.queryCache; - FSTRemoteDocumentChangeBuffer *remoteDocuments = - [FSTRemoteDocumentChangeBuffer changeBufferWithCache:self.remoteDocumentCache]; - [remoteEvent.targetChanges enumerateKeysAndObjectsUsingBlock:^( NSNumber *targetIDNumber, FSTTargetChange *change, BOOL *stop) { FSTTargetID targetID = targetIDNumber.intValue; @@ -315,13 +306,13 @@ NS_ASSUME_NONNULL_BEGIN const DocumentKey &key = kv.first; FSTMaybeDocument *doc = kv.second; changedDocKeys = [changedDocKeys setByAddingObject:key]; - FSTMaybeDocument *existingDoc = [remoteDocuments entryForKey:key]; + FSTMaybeDocument *existingDoc = [self.remoteDocumentCache entryForKey:key]; // Make sure we don't apply an old document version to the remote cache, though we // make an exception for [SnapshotVersion noVersion] which can happen for manufactured // events (e.g. in the case of a limbo document resolution failing). if (!existingDoc || [doc.version isEqual:[FSTSnapshotVersion noVersion]] || [doc.version compare:existingDoc.version] != NSOrderedAscending) { - [remoteDocuments addEntry:doc]; + [self.remoteDocumentCache addEntry:doc]; } else { FSTLog( @"FSTLocalStore Ignoring outdated watch update for %s. " @@ -346,10 +337,7 @@ NS_ASSUME_NONNULL_BEGIN [self.queryCache setLastRemoteSnapshotVersion:remoteVersion]; } - FSTDocumentKeySet *releasedWriteKeys = - [self releaseHeldBatchResultsWithRemoteDocuments:remoteDocuments]; - - [remoteDocuments apply]; + FSTDocumentKeySet *releasedWriteKeys = [self releaseHeldBatchResults]; // Union the two key sets. __block FSTDocumentKeySet *keysToRecalc = changedDocKeys; @@ -423,12 +411,7 @@ NS_ASSUME_NONNULL_BEGIN // If this was the last watch target, then we won't get any more watch snapshots, so we should // release any held batch results. if ([self.targetIDs count] == 0) { - FSTRemoteDocumentChangeBuffer *remoteDocuments = - [FSTRemoteDocumentChangeBuffer changeBufferWithCache:self.remoteDocumentCache]; - - [self releaseHeldBatchResultsWithRemoteDocuments:remoteDocuments]; - - [remoteDocuments apply]; + [self releaseHeldBatchResults]; } }); } @@ -464,8 +447,7 @@ NS_ASSUME_NONNULL_BEGIN * * @return the set of keys of docs that were modified by those writes. */ -- (FSTDocumentKeySet *)releaseHeldBatchResultsWithRemoteDocuments: - (FSTRemoteDocumentChangeBuffer *)remoteDocuments { +- (FSTDocumentKeySet *)releaseHeldBatchResults { NSMutableArray *toRelease = [NSMutableArray array]; for (FSTMutationBatchResult *batchResult in self.heldBatchResults) { if (![self isRemoteUpToVersion:batchResult.commitVersion]) { @@ -478,7 +460,7 @@ NS_ASSUME_NONNULL_BEGIN return [FSTDocumentKeySet keySet]; } else { [self.heldBatchResults removeObjectsInRange:NSMakeRange(0, toRelease.count)]; - return [self releaseBatchResults:toRelease remoteDocuments:remoteDocuments]; + return [self releaseBatchResults:toRelease]; } } @@ -493,11 +475,10 @@ NS_ASSUME_NONNULL_BEGIN return ![self isRemoteUpToVersion:version] || self.heldBatchResults.count > 0; } -- (FSTDocumentKeySet *)releaseBatchResults:(NSArray *)batchResults - remoteDocuments:(FSTRemoteDocumentChangeBuffer *)remoteDocuments { +- (FSTDocumentKeySet *)releaseBatchResults:(NSArray *)batchResults { NSMutableArray *batches = [NSMutableArray array]; for (FSTMutationBatchResult *batchResult in batchResults) { - [self applyBatchResult:batchResult toRemoteDocuments:remoteDocuments]; + [self applyBatchResult:batchResult]; [batches addObject:batchResult.batch]; } @@ -525,12 +506,11 @@ NS_ASSUME_NONNULL_BEGIN return affectedDocs; } -- (void)applyBatchResult:(FSTMutationBatchResult *)batchResult - toRemoteDocuments:(FSTRemoteDocumentChangeBuffer *)remoteDocuments { +- (void)applyBatchResult:(FSTMutationBatchResult *)batchResult { FSTMutationBatch *batch = batchResult.batch; FSTDocumentKeySet *docKeys = batch.keys; [docKeys enumerateObjectsUsingBlock:^(FSTDocumentKey *docKey, BOOL *stop) { - FSTMaybeDocument *_Nullable remoteDoc = [remoteDocuments entryForKey:docKey]; + FSTMaybeDocument *_Nullable remoteDoc = [self.remoteDocumentCache entryForKey:docKey]; FSTMaybeDocument *_Nullable doc = remoteDoc; FSTSnapshotVersion *ackVersion = batchResult.docVersions[docKey]; FSTAssert(ackVersion, @"docVersions should contain every doc in the write."); @@ -540,7 +520,7 @@ NS_ASSUME_NONNULL_BEGIN FSTAssert(!remoteDoc, @"Mutation batch %@ applied to document %@ resulted in nil.", batch, remoteDoc); } else { - [remoteDocuments addEntry:doc]; + [self.remoteDocumentCache addEntry:doc]; } } }]; diff --git a/Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.h b/Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.h deleted file mode 100644 index be29a89..0000000 --- a/Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.h +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright 2017 Google - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#import - -#include "Firestore/core/src/firebase/firestore/model/document_key.h" - -NS_ASSUME_NONNULL_BEGIN - -@protocol FSTRemoteDocumentCache; -@class FSTMaybeDocument; - -/** - * An in-memory buffer of entries to be written to an FSTRemoteDocumentCache. It can be used to - * batch up a set of changes to be written to the cache, but additionally supports reading entries - * back with the `entryForKey:` method, falling back to the underlying FSTRemoteDocumentCache if - * no entry is buffered. In the absence of LevelDB transactions (that would allow reading back - * uncommitted writes), this greatly simplifies the implementation of complex operations that - * may want to freely read/write entries to the FSTRemoteDocumentCache while still ensuring that - * the final writing of the buffered entries is atomic. - * - * For doing blind writes that don't depend on the current state of the FSTRemoteDocumentCache - * or for plain reads, you can/should still just use the FSTRemoteDocumentCache directly. - */ -@interface FSTRemoteDocumentChangeBuffer : NSObject - -+ (instancetype)changeBufferWithCache:(id)cache; - -- (instancetype)init __attribute__((unavailable("Use a static constructor instead"))); - -/** Buffers an `FSTRemoteDocumentCache addEntry:group:` call. */ -- (void)addEntry:(FSTMaybeDocument *)maybeDocument; - -// NOTE: removeEntryForKey: is not presently necessary and so is omitted. - -/** - * Looks up an entry in the cache. The buffered changes will first be checked, and if no - * buffered change applies, this will forward to `FSTRemoteDocumentCache entryForKey:`. - * - * @param documentKey The key of the entry to look up. - * @return The cached FSTDocument or FSTDeletedDocument entry, or nil if we have nothing cached. - */ -- (nullable FSTMaybeDocument *)entryForKey: - (const firebase::firestore::model::DocumentKey &)documentKey; - -/** - * Applies buffered changes to the underlying FSTRemoteDocumentCache - */ -- (void)apply; - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.mm b/Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.mm deleted file mode 100644 index 3812501..0000000 --- a/Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.mm +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Copyright 2017 Google - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#import "Firestore/Source/Local/FSTRemoteDocumentChangeBuffer.h" - -#include -#include - -#import "Firestore/Source/Local/FSTRemoteDocumentCache.h" -#import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Util/FSTAssert.h" - -#include "Firestore/core/src/firebase/firestore/model/document_key.h" -#include "absl/memory/memory.h" - -using firebase::firestore::model::DocumentKey; - -NS_ASSUME_NONNULL_BEGIN - -@interface FSTRemoteDocumentChangeBuffer () - -- (instancetype)initWithCache:(id)cache; - -/** The underlying cache we're buffering changes for. */ -@property(nonatomic, strong, nonnull) id remoteDocumentCache; - -@end - -@implementation FSTRemoteDocumentChangeBuffer { - /** The buffered changes, stored as a dictionary for easy lookups. */ - std::unique_ptr> _changes; -} - -+ (instancetype)changeBufferWithCache:(id)cache { - return [[FSTRemoteDocumentChangeBuffer alloc] initWithCache:cache]; -} - -- (instancetype)initWithCache:(id)cache { - if (self = [super init]) { - _remoteDocumentCache = cache; - _changes = absl::make_unique>(); - } - return self; -} - -- (void)addEntry:(FSTMaybeDocument *)maybeDocument { - [self assertValid]; - - (*_changes)[maybeDocument.key] = maybeDocument; -} - -- (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)documentKey { - [self assertValid]; - - const auto iter = _changes->find(documentKey); - if (iter == _changes->end()) { - return [self.remoteDocumentCache entryForKey:documentKey]; - } else { - return iter->second; - } -} - -- (void)apply { - [self assertValid]; - - for (const auto &kv : *_changes) { - [self.remoteDocumentCache addEntry:kv.second]; - } - - // We should not be used to buffer any more changes. - _changes.reset(); -} - -- (void)assertValid { - FSTAssert(_changes, @"Changes have already been applied."); -} - -@end - -NS_ASSUME_NONNULL_END -- cgit v1.2.3