From 45a1e2f9bff0dc91721e5f0e8ea2c2d042efeb5c Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 17 Dec 2018 10:39:17 -0800 Subject: [PATCH] Port FSTRemoteDocumentCacheTest to use C++ interface --- .../FSTLevelDBRemoteDocumentCacheTests.mm | 13 ++- .../FSTMemoryRemoteDocumentCacheTests.mm | 19 +++- .../Tests/Local/FSTRemoteDocumentCacheTests.h | 4 +- .../Local/FSTRemoteDocumentCacheTests.mm | 43 ++++---- Firestore/Source/Local/FSTLevelDB.h | 2 + Firestore/Source/Local/FSTLevelDB.mm | 1 - .../local/leveldb_remote_document_cache.h | 13 ++- .../local/memory_remote_document_cache.h | 13 ++- .../firestore/local/remote_document_cache.h | 104 ++++++++++++++++++ 9 files changed, 173 insertions(+), 39 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/local/remote_document_cache.h diff --git a/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm index fc44a5a71b4..78e105c9dc7 100644 --- a/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm @@ -14,18 +14,22 @@ * limitations under the License. */ +#include #include #import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h" #import "Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h" #import "Firestore/Source/Local/FSTLevelDB.h" +#include "Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h" #include "Firestore/core/src/firebase/firestore/util/ordered_code.h" +#include "absl/memory/memory.h" #include "leveldb/db.h" NS_ASSUME_NONNULL_BEGIN using leveldb::WriteOptions; +using firebase::firestore::local::LevelDbRemoteDocumentCache; using firebase::firestore::util::OrderedCode; // A dummy document value, useful for testing code that's known to examine only document keys. @@ -41,13 +45,15 @@ @interface FSTLevelDBRemoteDocumentCacheTests : FSTRemoteDocumentCacheTests @implementation FSTLevelDBRemoteDocumentCacheTests { FSTLevelDB *_db; + std::unique_ptr _cache; } - (void)setUp { [super setUp]; _db = [FSTPersistenceTestHelpers levelDBPersistence]; self.persistence = _db; - self.remoteDocumentCache = [self.persistence remoteDocumentCache]; + HARD_ASSERT(!_cache, "Previous cache not torn down"); + _cache = absl::make_unique(_db, _db.serializer); // Write a couple dummy rows that should appear before/after the remote_documents table to make // sure the tests are unaffected. @@ -55,10 +61,15 @@ - (void)setUp { [self writeDummyRowWithSegments:@[ @"remote_documentsa", @"foo", @"bar" ]]; } +- (LevelDbRemoteDocumentCache *_Nullable)remoteDocumentCache { + return _cache.get(); +} + - (void)tearDown { [super tearDown]; self.remoteDocumentCache = nil; self.persistence = nil; + _cache.reset(); _db = nil; } diff --git a/Firestore/Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm index 3cbc3203496..c33ce4b63ee 100644 --- a/Firestore/Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm @@ -14,13 +14,17 @@ * limitations under the License. */ -#import "Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h" +#include #import "Firestore/Source/Local/FSTMemoryPersistence.h" +#include "Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.h" +#include "absl/memory/memory.h" #import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h" #import "Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h" +using firebase::firestore::local::MemoryRemoteDocumentCache; + @interface FSTMemoryRemoteDocumentCacheTests : FSTRemoteDocumentCacheTests @end @@ -29,18 +33,25 @@ @interface FSTMemoryRemoteDocumentCacheTests : FSTRemoteDocumentCacheTests * protocol in FSTRemoteDocumentCacheTests. This class is merely responsible for setting up and * tearing down the @a remoteDocumentCache. */ -@implementation FSTMemoryRemoteDocumentCacheTests +@implementation FSTMemoryRemoteDocumentCacheTests { + std::unique_ptr _cache; +} - (void)setUp { [super setUp]; self.persistence = [FSTPersistenceTestHelpers eagerGCMemoryPersistence]; - self.remoteDocumentCache = [self.persistence remoteDocumentCache]; + HARD_ASSERT(!_cache, "Previous cache not torn down"); + _cache = absl::make_unique(); +} + +- (MemoryRemoteDocumentCache *)remoteDocumentCache { + return _cache.get(); } - (void)tearDown { + _cache.reset(); self.persistence = nil; - self.remoteDocumentCache = nil; [super tearDown]; } diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h index 29e7b86aa4f..71e0073f79a 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h @@ -18,6 +18,8 @@ #import +#include "Firestore/core/src/firebase/firestore/local/remote_document_cache.h" + @protocol FSTPersistence; NS_ASSUME_NONNULL_BEGIN @@ -32,7 +34,7 @@ NS_ASSUME_NONNULL_BEGIN * + override -tearDown, cleaning up remoteDocumentCache and persistence */ @interface FSTRemoteDocumentCacheTests : XCTestCase -@property(nonatomic, strong, nullable) id remoteDocumentCache; +@property(nonatomic, nullable) firebase::firestore::local::RemoteDocumentCache* remoteDocumentCache; @property(nonatomic, strong, nullable) id persistence; @end diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm index cd15d98b94f..e94224cdbdb 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm @@ -16,6 +16,8 @@ #import "Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h" +#include + #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTPersistence.h" #import "Firestore/Source/Model/FSTDocument.h" @@ -23,6 +25,8 @@ #import "Firestore/Example/Tests/Util/FSTHelpers.h" +#include "Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.h" +#include "Firestore/core/src/firebase/firestore/local/remote_document_cache.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/document_map.h" @@ -32,6 +36,7 @@ namespace testutil = firebase::firestore::testutil; namespace util = firebase::firestore::util; +using firebase::firestore::local::RemoteDocumentCache; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::DocumentMap; @@ -62,7 +67,7 @@ - (void)testReadDocumentNotInCache { if (!self.remoteDocumentCache) return; self.persistence.run("testReadDocumentNotInCache", [&]() { - XCTAssertNil([self.remoteDocumentCache entryForKey:testutil::Key(kDocPath)]); + XCTAssertNil(self.remoteDocumentCache->Get(testutil::Key(kDocPath))); }); } @@ -70,7 +75,7 @@ - (void)testReadDocumentNotInCache { - (void)setAndReadADocumentAtPath:(const absl::string_view)path { self.persistence.run("setAndReadADocumentAtPath", [&]() { FSTDocument *written = [self setTestDocumentAtPath:path]; - FSTMaybeDocument *read = [self.remoteDocumentCache entryForKey:testutil::Key(path)]; + FSTMaybeDocument *read = self.remoteDocumentCache->Get(testutil::Key(path)); XCTAssertEqualObjects(read, written); }); } @@ -87,8 +92,8 @@ - (void)testSetAndReadSeveralDocuments { self.persistence.run("testSetAndReadSeveralDocuments", [=]() { NSArray *written = @[ [self setTestDocumentAtPath:kDocPath], [self setTestDocumentAtPath:kLongDocPath] ]; - MaybeDocumentMap read = [self.remoteDocumentCache - entriesForKeys:DocumentKeySet{testutil::Key(kDocPath), testutil::Key(kLongDocPath)}]; + MaybeDocumentMap read = self.remoteDocumentCache->GetAll( + DocumentKeySet{testutil::Key(kDocPath), testutil::Key(kLongDocPath)}); [self expectMap:read hasDocsInArray:written exactly:YES]; }); } @@ -99,12 +104,11 @@ - (void)testSetAndReadSeveralDocumentsIncludingMissingDocument { self.persistence.run("testSetAndReadSeveralDocumentsIncludingMissingDocument", [=]() { NSArray *written = @[ [self setTestDocumentAtPath:kDocPath], [self setTestDocumentAtPath:kLongDocPath] ]; - MaybeDocumentMap read = - [self.remoteDocumentCache entriesForKeys:DocumentKeySet{ - testutil::Key(kDocPath), - testutil::Key(kLongDocPath), - testutil::Key("foo/nonexistent"), - }]; + MaybeDocumentMap read = self.remoteDocumentCache->GetAll(DocumentKeySet{ + testutil::Key(kDocPath), + testutil::Key(kLongDocPath), + testutil::Key("foo/nonexistent"), + }); [self expectMap:read hasDocsInArray:written exactly:NO]; auto found = read.find(DocumentKey::FromPathString("foo/nonexistent")); XCTAssertTrue(found != read.end()); @@ -123,10 +127,9 @@ - (void)testSetAndReadDeletedDocument { self.persistence.run("testSetAndReadDeletedDocument", [&]() { FSTDeletedDocument *deletedDoc = FSTTestDeletedDoc(kDocPath, kVersion, NO); - [self.remoteDocumentCache addEntry:deletedDoc]; + self.remoteDocumentCache->AddEntry(deletedDoc); - XCTAssertEqualObjects([self.remoteDocumentCache entryForKey:testutil::Key(kDocPath)], - deletedDoc); + XCTAssertEqualObjects(self.remoteDocumentCache->Get(testutil::Key(kDocPath)), deletedDoc); }); } @@ -136,8 +139,8 @@ - (void)testSetDocumentToNewValue { self.persistence.run("testSetDocumentToNewValue", [&]() { [self setTestDocumentAtPath:kDocPath]; FSTDocument *newDoc = FSTTestDoc(kDocPath, kVersion, @{@"data" : @2}, FSTDocumentStateSynced); - [self.remoteDocumentCache addEntry:newDoc]; - XCTAssertEqualObjects([self.remoteDocumentCache entryForKey:testutil::Key(kDocPath)], newDoc); + self.remoteDocumentCache->AddEntry(newDoc); + XCTAssertEqualObjects(self.remoteDocumentCache->Get(testutil::Key(kDocPath)), newDoc); }); } @@ -146,9 +149,9 @@ - (void)testRemoveDocument { self.persistence.run("testRemoveDocument", [&]() { [self setTestDocumentAtPath:kDocPath]; - [self.remoteDocumentCache removeEntryForKey:testutil::Key(kDocPath)]; + self.remoteDocumentCache->RemoveEntry(testutil::Key(kDocPath)); - XCTAssertNil([self.remoteDocumentCache entryForKey:testutil::Key(kDocPath)]); + XCTAssertNil(self.remoteDocumentCache->Get(testutil::Key(kDocPath))); }); } @@ -157,7 +160,7 @@ - (void)testRemoveNonExistentDocument { self.persistence.run("testRemoveNonExistentDocument", [&]() { // no-op, but make sure it doesn't throw. - XCTAssertNoThrow([self.remoteDocumentCache removeEntryForKey:testutil::Key(kDocPath)]); + XCTAssertNoThrow(self.remoteDocumentCache->RemoveEntry(testutil::Key(kDocPath))); }); } @@ -174,7 +177,7 @@ - (void)testDocumentsMatchingQuery { [self setTestDocumentAtPath:"c/1"]; FSTQuery *query = FSTTestQuery("b"); - DocumentMap results = [self.remoteDocumentCache documentsMatchingQuery:query]; + DocumentMap results = self.remoteDocumentCache->GetMatchingDocuments(query); [self expectMap:results.underlying_map() hasDocsInArray:@[ FSTTestDoc("b/1", kVersion, _kDocData, FSTDocumentStateSynced), @@ -189,7 +192,7 @@ - (void)testDocumentsMatchingQuery { - (FSTDocument *)setTestDocumentAtPath:(const absl::string_view)path { FSTDocument *doc = FSTTestDoc(path, kVersion, _kDocData, FSTDocumentStateSynced); - [self.remoteDocumentCache addEntry:doc]; + self.remoteDocumentCache->AddEntry(doc); return doc; } diff --git a/Firestore/Source/Local/FSTLevelDB.h b/Firestore/Source/Local/FSTLevelDB.h index b629d4861f1..de78f724629 100644 --- a/Firestore/Source/Local/FSTLevelDB.h +++ b/Firestore/Source/Local/FSTLevelDB.h @@ -83,6 +83,8 @@ NS_ASSUME_NONNULL_BEGIN @property(nonatomic, readonly, strong) FSTLevelDBLRUDelegate *referenceDelegate; +@property(nonatomic, readonly, strong) FSTLocalSerializer *serializer; + @end NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm index 116bb65b903..cce7d313b35 100644 --- a/Firestore/Source/Local/FSTLevelDB.mm +++ b/Firestore/Source/Local/FSTLevelDB.mm @@ -83,7 +83,6 @@ @interface FSTLevelDB () - (size_t)byteSize; @property(nonatomic, assign, getter=isStarted) BOOL started; -@property(nonatomic, strong, readonly) FSTLocalSerializer *serializer; @end diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h index b4ba9e3d17a..ef8a83f429c 100644 --- a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h +++ b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h @@ -23,6 +23,7 @@ #include +#include "Firestore/core/src/firebase/firestore/local/remote_document_cache.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/document_map.h" @@ -41,16 +42,16 @@ namespace firestore { namespace local { /** Cached Remote Documents backed by leveldb. */ -class LevelDbRemoteDocumentCache { +class LevelDbRemoteDocumentCache : public RemoteDocumentCache { public: LevelDbRemoteDocumentCache(FSTLevelDB* db, FSTLocalSerializer* serializer); - void AddEntry(FSTMaybeDocument* document); - void RemoveEntry(const model::DocumentKey& key); + void AddEntry(FSTMaybeDocument* document) override; + void RemoveEntry(const model::DocumentKey& key) override; - FSTMaybeDocument* _Nullable Get(const model::DocumentKey& key); - model::MaybeDocumentMap GetAll(const model::DocumentKeySet& keys); - model::DocumentMap GetMatchingDocuments(FSTQuery* query); + FSTMaybeDocument* _Nullable Get(const model::DocumentKey& key) override; + model::MaybeDocumentMap GetAll(const model::DocumentKeySet& keys) override; + model::DocumentMap GetMatchingDocuments(FSTQuery* query) override; private: FSTMaybeDocument* DecodeMaybeDocument(absl::string_view encoded, diff --git a/Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.h b/Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.h index 09538a0f4fa..ed9175bbd93 100644 --- a/Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.h +++ b/Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.h @@ -23,6 +23,7 @@ #include +#include "Firestore/core/src/firebase/firestore/local/remote_document_cache.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/document_map.h" @@ -39,14 +40,14 @@ namespace firebase { namespace firestore { namespace local { -class MemoryRemoteDocumentCache { +class MemoryRemoteDocumentCache : public RemoteDocumentCache { public: - void AddEntry(FSTMaybeDocument *document); - void RemoveEntry(const model::DocumentKey &key); + void AddEntry(FSTMaybeDocument *document) override; + void RemoveEntry(const model::DocumentKey &key) override; - FSTMaybeDocument *_Nullable Get(const model::DocumentKey &key); - model::MaybeDocumentMap GetAll(const model::DocumentKeySet &keys); - model::DocumentMap GetMatchingDocuments(FSTQuery *query); + FSTMaybeDocument *_Nullable Get(const model::DocumentKey &key) override; + model::MaybeDocumentMap GetAll(const model::DocumentKeySet &keys) override; + model::DocumentMap GetMatchingDocuments(FSTQuery *query) override; std::vector RemoveOrphanedDocuments( FSTMemoryLRUReferenceDelegate *reference_delegate, diff --git a/Firestore/core/src/firebase/firestore/local/remote_document_cache.h b/Firestore/core/src/firebase/firestore/local/remote_document_cache.h new file mode 100644 index 00000000000..b9721281a35 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/local/remote_document_cache.h @@ -0,0 +1,104 @@ +/* + * Copyright 2018 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. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_LOCAL_REMOTE_DOCUMENT_CACHE_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_LOCAL_REMOTE_DOCUMENT_CACHE_H_ + +#if !defined(__OBJC__) +#error "For now, this file must only be included by ObjC source files." +#endif // !defined(__OBJC__) + +#import + +#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/document_map.h" +#include "Firestore/core/src/firebase/firestore/model/types.h" + +@class FSTMaybeDocument; +@class FSTQuery; + +NS_ASSUME_NONNULL_BEGIN + +namespace firebase { +namespace firestore { +namespace local { + +/** + * Represents cached documents received from the remote backend. + * + * The cache is keyed by DocumentKey and entries in the cache are + * FSTMaybeDocument instances, meaning we can cache both FSTDocument instances + * (an actual document with data) as well as FSTDeletedDocument instances + * (indicating that the document is known to not exist). + */ +class RemoteDocumentCache { + public: + virtual ~RemoteDocumentCache() { + } + + /** + * Adds or replaces an entry in the cache. + * + * The cache key is extracted from `document.key`. If there is already a cache + * entry for the key, it will be replaced. + * + * @param document A FSTDocument or FSTDeletedDocument to put in the cache. + */ + virtual void AddEntry(FSTMaybeDocument* document) = 0; + + /** Removes the cached entry for the given key (no-op if no entry exists). */ + virtual void RemoveEntry(const model::DocumentKey& key) = 0; + + /** + * Looks up an entry in the cache. + * + * @param documentKey The key of the entry to look up. + * @return The cached FSTDocument or FSTDeletedDocument entry, or nil if we + * have nothing cached. + */ + virtual FSTMaybeDocument* _Nullable Get(const model::DocumentKey& key) = 0; + + /** + * Looks up a set of entries in the cache. + * + * @param keys The keys of the entries to look up. + * @return The cached Document or NoDocument entries indexed by key. If an + * entry is not cached, the corresponding key will be mapped to a null value. + */ + virtual model::MaybeDocumentMap GetAll(const model::DocumentKeySet& keys) = 0; + + /** + * Executes a query against the cached FSTDocument entries + * + * Implementations may return extra documents if convenient. The results + * should be re-filtered by the consumer before presenting them to the user. + * + * Cached FSTDeletedDocument entries have no bearing on query results. + * + * @param query The query to match documents against. + * @return The set of matching documents. + */ + virtual model::DocumentMap GetMatchingDocuments(FSTQuery* query) = 0; +}; + +} // namespace local +} // namespace firestore +} // namespace firebase + +NS_ASSUME_NONNULL_END + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_LOCAL_REMOTE_DOCUMENT_CACHE_H_