From fcd50623fc2976b9fdd759735d94137f145b1bed Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 13 Dec 2018 14:27:28 -0800 Subject: [PATCH 01/15] Port leveldb remote document cache --- .../Local/FSTLevelDBRemoteDocumentCache.mm | 92 ++----------- .../local/leveldb_remote_document_cache.h | 70 ++++++++++ .../local/leveldb_remote_document_cache.mm | 124 ++++++++++++++++++ 3 files changed, 204 insertions(+), 82 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h create mode 100644 Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm diff --git a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm index 66f223e75f2..f4a4a3bdbf7 100644 --- a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm +++ b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm @@ -26,14 +26,17 @@ #import "Firestore/Source/Model/FSTDocumentSet.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_key.h" +#include "Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" +#include "absl/memory/memory.h" #include "leveldb/db.h" #include "leveldb/write_batch.h" NS_ASSUME_NONNULL_BEGIN +using firebase::firestore::local::LevelDbRemoteDocumentCache; using firebase::firestore::local::LevelDbRemoteDocumentKey; using firebase::firestore::local::LevelDbTransaction; using firebase::firestore::model::DocumentKey; @@ -43,110 +46,35 @@ using leveldb::DB; using leveldb::Status; -@interface FSTLevelDBRemoteDocumentCache () - -@property(nonatomic, strong, readonly) FSTLocalSerializer *serializer; - -@end - @implementation FSTLevelDBRemoteDocumentCache { - FSTLevelDB *_db; + std::unique_ptr _cache; } - (instancetype)initWithDB:(FSTLevelDB *)db serializer:(FSTLocalSerializer *)serializer { if (self = [super init]) { - _db = db; - _serializer = serializer; + _cache = absl::make_unique(db, serializer); } return self; } - (void)addEntry:(FSTMaybeDocument *)document { - std::string key = [self remoteDocumentKey:document.key]; - _db.currentTransaction->Put(key, [self.serializer encodedMaybeDocument:document]); + _cache->AddEntry(document); } - (void)removeEntryForKey:(const DocumentKey &)documentKey { - std::string key = [self remoteDocumentKey:documentKey]; - _db.currentTransaction->Delete(key); + _cache->RemoveEntry(documentKey); } - (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)documentKey { - std::string key = LevelDbRemoteDocumentKey::Key(documentKey); - std::string value; - Status status = _db.currentTransaction->Get(key, &value); - if (status.IsNotFound()) { - return nil; - } else if (status.ok()) { - return [self decodeMaybeDocument:value withKey:documentKey]; - } else { - HARD_FAIL("Fetch document for key (%s) failed with status: %s", documentKey.ToString(), - status.ToString()); - } + return _cache->Get(documentKey); } - (MaybeDocumentMap)entriesForKeys:(const DocumentKeySet &)keys { - MaybeDocumentMap results; - - LevelDbRemoteDocumentKey currentKey; - auto it = _db.currentTransaction->NewIterator(); - - for (const DocumentKey &key : keys) { - it->Seek([self remoteDocumentKey:key]); - if (!it->Valid() || !currentKey.Decode(it->key()) || currentKey.document_key() != key) { - results = results.insert(key, nil); - } else { - results = results.insert(key, [self decodeMaybeDocument:it->value() withKey:key]); - } - } - - return results; + return _cache->GetAll(keys); } - (DocumentMap)documentsMatchingQuery:(FSTQuery *)query { - DocumentMap results; - - // Documents are ordered by key, so we can use a prefix scan to narrow down - // the documents we need to match the query against. - std::string startKey = LevelDbRemoteDocumentKey::KeyPrefix(query.path); - auto it = _db.currentTransaction->NewIterator(); - it->Seek(startKey); - - LevelDbRemoteDocumentKey currentKey; - for (; it->Valid() && currentKey.Decode(it->key()); it->Next()) { - FSTMaybeDocument *maybeDoc = - [self decodeMaybeDocument:it->value() withKey:currentKey.document_key()]; - if (!query.path.IsPrefixOf(maybeDoc.key.path())) { - break; - } else if ([maybeDoc isKindOfClass:[FSTDocument class]]) { - results = results.insert(maybeDoc.key, static_cast(maybeDoc)); - } - } - - return results; -} - -- (std::string)remoteDocumentKey:(const DocumentKey &)key { - return LevelDbRemoteDocumentKey::Key(key); -} - -- (FSTMaybeDocument *)decodeMaybeDocument:(absl::string_view)encoded - withKey:(const DocumentKey &)documentKey { - NSData *data = [[NSData alloc] initWithBytesNoCopy:(void *)encoded.data() - length:encoded.size() - freeWhenDone:NO]; - - NSError *error; - FSTPBMaybeDocument *proto = [FSTPBMaybeDocument parseFromData:data error:&error]; - if (!proto) { - HARD_FAIL("FSTPBMaybeDocument failed to parse: %s", error); - } - - FSTMaybeDocument *maybeDocument = [self.serializer decodedMaybeDocument:proto]; - HARD_ASSERT(maybeDocument.key == documentKey, - "Read document has key (%s) instead of expected key (%s).", - maybeDocument.key.ToString(), documentKey.ToString()); - return maybeDocument; + return _cache->GetMatchingDocuments(query); } @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 new file mode 100644 index 00000000000..ce1ed0eda4e --- /dev/null +++ b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h @@ -0,0 +1,70 @@ +/* + * 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_LEVELDB_REMOTE_DOCUMENT_CACHE_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_LOCAL_LEVELDB_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 + +#include "absl/strings/string_view.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" +#include "Firestore/core/src/firebase/firestore/model/types.h" + +@class FSTLevelDB; +@class FSTLocalSerializer; +@class FSTMaybeDocument; +@class FSTQuery; + +NS_ASSUME_NONNULL_BEGIN + +namespace firebase { +namespace firestore { +namespace local { + +/** Cached Remote Documents backed by leveldb. */ +class LevelDbRemoteDocumentCache { + public: + LevelDbRemoteDocumentCache(FSTLevelDB *db, FSTLocalSerializer *serializer); + + void AddEntry(FSTMaybeDocument *document); + void RemoveEntry(const model::DocumentKey &key); + + FSTMaybeDocument *_Nullable Get(const model::DocumentKey &key); + model::MaybeDocumentMap GetAll(const model::DocumentKeySet &keys); + model::DocumentMap GetMatchingDocuments(FSTQuery *query); + + private: + FSTMaybeDocument *DecodeMaybeDocument(absl::string_view encoded, const model::DocumentKey& key); + + FSTLevelDB *db_; + FSTLocalSerializer *serializer_; +}; + +} // namespace local +} // namespace firestore +} // namespace firebase + +NS_ASSUME_NONNULL_END + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_LOCAL_LEVELDB_REMOTE_DOCUMENT_CACHE_H_ diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm new file mode 100644 index 00000000000..6b6aa0339c4 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm @@ -0,0 +1,124 @@ +/* + * 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. + */ + +#include "Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h" + +#include "Firestore/core/src/firebase/firestore/local/leveldb_key.h" +#include "Firestore/core/src/firebase/firestore/util/status.h" +#import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h" +#import "Firestore/Source/Core/FSTQuery.h" +#import "Firestore/Source/Local/FSTLevelDB.h" +#import "Firestore/Source/Local/FSTLocalSerializer.h" +#include "leveldb/db.h" + +using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::DocumentMap; +using firebase::firestore::model::MaybeDocumentMap; +using leveldb::Status; + +namespace firebase { +namespace firestore { +namespace local { + +LevelDbRemoteDocumentCache::LevelDbRemoteDocumentCache(FSTLevelDB *db, FSTLocalSerializer *serializer) : db_(db), serializer_(serializer) { +} + +void LevelDbRemoteDocumentCache::AddEntry(FSTMaybeDocument *document) { + std::string ldb_key = LevelDbRemoteDocumentKey::Key(document.key); + db_.currentTransaction->Put(ldb_key, [serializer_ encodedMaybeDocument:document]); +} + +void LevelDbRemoteDocumentCache::RemoveEntry(const DocumentKey &key) { + std::string ldb_key = LevelDbRemoteDocumentKey::Key(key); + db_.currentTransaction->Delete(ldb_key); +} + +FSTMaybeDocument *_Nullable LevelDbRemoteDocumentCache::Get(const DocumentKey &key) { + std::string ldb_key = LevelDbRemoteDocumentKey::Key(key); + std::string value; + Status status = db_.currentTransaction->Get(ldb_key, &value); + if (status.IsNotFound()) { + return nil; + } else if (status.ok()) { + return DecodeMaybeDocument(value, key); + } else { + HARD_FAIL("Fetch document for key (%s) failed with status: %s", key.ToString(), + status.ToString()); + } +} + +MaybeDocumentMap LevelDbRemoteDocumentCache::GetAll(const DocumentKeySet &keys) { + MaybeDocumentMap results; + + LevelDbRemoteDocumentKey currentKey; + auto it = db_.currentTransaction->NewIterator(); + + for (const DocumentKey &key : keys) { + it->Seek(LevelDbRemoteDocumentKey::Key(key)); + if (!it->Valid() || !currentKey.Decode(it->key()) || currentKey.document_key() != key) { + results = results.insert(key, nil); + } else { + results = results.insert(key, DecodeMaybeDocument(it->value(), key)); + } + } + + return results; +} + +DocumentMap LevelDbRemoteDocumentCache::GetMatchingDocuments(FSTQuery *query) { + DocumentMap results; + + // Documents are ordered by key, so we can use a prefix scan to narrow down + // the documents we need to match the query against. + std::string startKey = LevelDbRemoteDocumentKey::KeyPrefix(query.path); + auto it = db_.currentTransaction->NewIterator(); + it->Seek(startKey); + + LevelDbRemoteDocumentKey currentKey; + for (; it->Valid() && currentKey.Decode(it->key()); it->Next()) { + FSTMaybeDocument *maybeDoc = DecodeMaybeDocument(it->value(), currentKey.document_key()); + if (!query.path.IsPrefixOf(maybeDoc.key.path())) { + break; + } else if ([maybeDoc isKindOfClass:[FSTDocument class]]) { + results = results.insert(maybeDoc.key, static_cast(maybeDoc)); + } + } + + return results; +} + +FSTMaybeDocument *LevelDbRemoteDocumentCache::DecodeMaybeDocument(absl::string_view encoded, const DocumentKey &key) { + NSData *data = [[NSData alloc] initWithBytesNoCopy:(void *)encoded.data() + length:encoded.size() + freeWhenDone:NO]; + + NSError *error; + FSTPBMaybeDocument *proto = [FSTPBMaybeDocument parseFromData:data error:&error]; + if (!proto) { + HARD_FAIL("FSTPBMaybeDocument failed to parse: %s", error); + } + + FSTMaybeDocument *maybeDocument = [serializer_ decodedMaybeDocument:proto]; + HARD_ASSERT(maybeDocument.key == key, + "Read document has key (%s) instead of expected key (%s).", + maybeDocument.key.ToString(), key.ToString()); + return maybeDocument; +} + +} // namespace local +} // namespace firestore +} // namespace firebase \ No newline at end of file From 6e794b7a43ea18e3b7bb4e24dd62abac78308740 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 13 Dec 2018 15:14:52 -0800 Subject: [PATCH 02/15] Style --- .../local/leveldb_remote_document_cache.h | 5 ++- .../local/leveldb_remote_document_cache.mm | 40 ++++++++++++------- 2 files changed, 28 insertions(+), 17 deletions(-) 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 ce1ed0eda4e..627d89017bc 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 @@ -25,11 +25,11 @@ #include -#include "absl/strings/string_view.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" #include "Firestore/core/src/firebase/firestore/model/types.h" +#include "absl/strings/string_view.h" @class FSTLevelDB; @class FSTLocalSerializer; @@ -55,7 +55,8 @@ class LevelDbRemoteDocumentCache { model::DocumentMap GetMatchingDocuments(FSTQuery *query); private: - FSTMaybeDocument *DecodeMaybeDocument(absl::string_view encoded, const model::DocumentKey& key); + FSTMaybeDocument *DecodeMaybeDocument(absl::string_view encoded, + const model::DocumentKey &key); FSTLevelDB *db_; FSTLocalSerializer *serializer_; diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm index 6b6aa0339c4..5596e59cf9a 100644 --- a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm +++ b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm @@ -16,12 +16,12 @@ #include "Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h" -#include "Firestore/core/src/firebase/firestore/local/leveldb_key.h" -#include "Firestore/core/src/firebase/firestore/util/status.h" #import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h" #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTLevelDB.h" #import "Firestore/Source/Local/FSTLocalSerializer.h" +#include "Firestore/core/src/firebase/firestore/local/leveldb_key.h" +#include "Firestore/core/src/firebase/firestore/util/status.h" #include "leveldb/db.h" using firebase::firestore::model::DocumentKey; @@ -34,12 +34,15 @@ namespace firestore { namespace local { -LevelDbRemoteDocumentCache::LevelDbRemoteDocumentCache(FSTLevelDB *db, FSTLocalSerializer *serializer) : db_(db), serializer_(serializer) { +LevelDbRemoteDocumentCache::LevelDbRemoteDocumentCache( + FSTLevelDB *db, FSTLocalSerializer *serializer) + : db_(db), serializer_(serializer) { } void LevelDbRemoteDocumentCache::AddEntry(FSTMaybeDocument *document) { std::string ldb_key = LevelDbRemoteDocumentKey::Key(document.key); - db_.currentTransaction->Put(ldb_key, [serializer_ encodedMaybeDocument:document]); + db_.currentTransaction->Put(ldb_key, + [serializer_ encodedMaybeDocument:document]); } void LevelDbRemoteDocumentCache::RemoveEntry(const DocumentKey &key) { @@ -47,7 +50,8 @@ db_.currentTransaction->Delete(ldb_key); } -FSTMaybeDocument *_Nullable LevelDbRemoteDocumentCache::Get(const DocumentKey &key) { +FSTMaybeDocument *_Nullable LevelDbRemoteDocumentCache::Get( + const DocumentKey &key) { std::string ldb_key = LevelDbRemoteDocumentKey::Key(key); std::string value; Status status = db_.currentTransaction->Get(ldb_key, &value); @@ -56,12 +60,13 @@ } else if (status.ok()) { return DecodeMaybeDocument(value, key); } else { - HARD_FAIL("Fetch document for key (%s) failed with status: %s", key.ToString(), - status.ToString()); + HARD_FAIL("Fetch document for key (%s) failed with status: %s", + key.ToString(), status.ToString()); } } -MaybeDocumentMap LevelDbRemoteDocumentCache::GetAll(const DocumentKeySet &keys) { +MaybeDocumentMap LevelDbRemoteDocumentCache::GetAll( + const DocumentKeySet &keys) { MaybeDocumentMap results; LevelDbRemoteDocumentKey currentKey; @@ -69,7 +74,8 @@ for (const DocumentKey &key : keys) { it->Seek(LevelDbRemoteDocumentKey::Key(key)); - if (!it->Valid() || !currentKey.Decode(it->key()) || currentKey.document_key() != key) { + if (!it->Valid() || !currentKey.Decode(it->key()) || + currentKey.document_key() != key) { results = results.insert(key, nil); } else { results = results.insert(key, DecodeMaybeDocument(it->value(), key)); @@ -90,32 +96,36 @@ LevelDbRemoteDocumentKey currentKey; for (; it->Valid() && currentKey.Decode(it->key()); it->Next()) { - FSTMaybeDocument *maybeDoc = DecodeMaybeDocument(it->value(), currentKey.document_key()); + FSTMaybeDocument *maybeDoc = + DecodeMaybeDocument(it->value(), currentKey.document_key()); if (!query.path.IsPrefixOf(maybeDoc.key.path())) { break; } else if ([maybeDoc isKindOfClass:[FSTDocument class]]) { - results = results.insert(maybeDoc.key, static_cast(maybeDoc)); + results = + results.insert(maybeDoc.key, static_cast(maybeDoc)); } } return results; } -FSTMaybeDocument *LevelDbRemoteDocumentCache::DecodeMaybeDocument(absl::string_view encoded, const DocumentKey &key) { +FSTMaybeDocument *LevelDbRemoteDocumentCache::DecodeMaybeDocument( + absl::string_view encoded, const DocumentKey &key) { NSData *data = [[NSData alloc] initWithBytesNoCopy:(void *)encoded.data() length:encoded.size() freeWhenDone:NO]; NSError *error; - FSTPBMaybeDocument *proto = [FSTPBMaybeDocument parseFromData:data error:&error]; + FSTPBMaybeDocument *proto = + [FSTPBMaybeDocument parseFromData:data error:&error]; if (!proto) { HARD_FAIL("FSTPBMaybeDocument failed to parse: %s", error); } FSTMaybeDocument *maybeDocument = [serializer_ decodedMaybeDocument:proto]; HARD_ASSERT(maybeDocument.key == key, - "Read document has key (%s) instead of expected key (%s).", - maybeDocument.key.ToString(), key.ToString()); + "Read document has key (%s) instead of expected key (%s).", + maybeDocument.key.ToString(), key.ToString()); return maybeDocument; } From 8b6318c24f9bd68b30b9dc871f74a7d0615b2115 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 13 Dec 2018 15:39:51 -0800 Subject: [PATCH 03/15] Review feedback --- .../local/leveldb_remote_document_cache.h | 20 +++++------ .../local/leveldb_remote_document_cache.mm | 34 +++++++++---------- 2 files changed, 27 insertions(+), 27 deletions(-) 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 627d89017bc..165b7c13bec 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 @@ -45,21 +45,21 @@ namespace local { /** Cached Remote Documents backed by leveldb. */ class LevelDbRemoteDocumentCache { public: - LevelDbRemoteDocumentCache(FSTLevelDB *db, FSTLocalSerializer *serializer); + LevelDbRemoteDocumentCache(FSTLevelDB* db, FSTLocalSerializer* serializer); - void AddEntry(FSTMaybeDocument *document); - void RemoveEntry(const model::DocumentKey &key); + void AddEntry(FSTMaybeDocument* document); + void RemoveEntry(const model::DocumentKey& key); - 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); + model::MaybeDocumentMap GetAll(const model::DocumentKeySet& keys); + model::DocumentMap GetMatchingDocuments(FSTQuery* query); private: - FSTMaybeDocument *DecodeMaybeDocument(absl::string_view encoded, - const model::DocumentKey &key); + FSTMaybeDocument* DecodeMaybeDocument(absl::string_view encoded, + const model::DocumentKey& key); - FSTLevelDB *db_; - FSTLocalSerializer *serializer_; + FSTLevelDB* db_; + FSTLocalSerializer* serializer_; }; } // namespace local diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm index 5596e59cf9a..bfadd447733 100644 --- a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm +++ b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm @@ -35,23 +35,23 @@ namespace local { LevelDbRemoteDocumentCache::LevelDbRemoteDocumentCache( - FSTLevelDB *db, FSTLocalSerializer *serializer) + FSTLevelDB* db, FSTLocalSerializer* serializer) : db_(db), serializer_(serializer) { } -void LevelDbRemoteDocumentCache::AddEntry(FSTMaybeDocument *document) { +void LevelDbRemoteDocumentCache::AddEntry(FSTMaybeDocument* document) { std::string ldb_key = LevelDbRemoteDocumentKey::Key(document.key); db_.currentTransaction->Put(ldb_key, [serializer_ encodedMaybeDocument:document]); } -void LevelDbRemoteDocumentCache::RemoveEntry(const DocumentKey &key) { +void LevelDbRemoteDocumentCache::RemoveEntry(const DocumentKey& key) { std::string ldb_key = LevelDbRemoteDocumentKey::Key(key); db_.currentTransaction->Delete(ldb_key); } -FSTMaybeDocument *_Nullable LevelDbRemoteDocumentCache::Get( - const DocumentKey &key) { +FSTMaybeDocument* _Nullable LevelDbRemoteDocumentCache::Get( + const DocumentKey& key) { std::string ldb_key = LevelDbRemoteDocumentKey::Key(key); std::string value; Status status = db_.currentTransaction->Get(ldb_key, &value); @@ -66,13 +66,13 @@ } MaybeDocumentMap LevelDbRemoteDocumentCache::GetAll( - const DocumentKeySet &keys) { + const DocumentKeySet& keys) { MaybeDocumentMap results; LevelDbRemoteDocumentKey currentKey; auto it = db_.currentTransaction->NewIterator(); - for (const DocumentKey &key : keys) { + for (const DocumentKey& key : keys) { it->Seek(LevelDbRemoteDocumentKey::Key(key)); if (!it->Valid() || !currentKey.Decode(it->key()) || currentKey.document_key() != key) { @@ -85,7 +85,7 @@ return results; } -DocumentMap LevelDbRemoteDocumentCache::GetMatchingDocuments(FSTQuery *query) { +DocumentMap LevelDbRemoteDocumentCache::GetMatchingDocuments(FSTQuery* query) { DocumentMap results; // Documents are ordered by key, so we can use a prefix scan to narrow down @@ -96,33 +96,33 @@ LevelDbRemoteDocumentKey currentKey; for (; it->Valid() && currentKey.Decode(it->key()); it->Next()) { - FSTMaybeDocument *maybeDoc = + FSTMaybeDocument* maybeDoc = DecodeMaybeDocument(it->value(), currentKey.document_key()); if (!query.path.IsPrefixOf(maybeDoc.key.path())) { break; } else if ([maybeDoc isKindOfClass:[FSTDocument class]]) { results = - results.insert(maybeDoc.key, static_cast(maybeDoc)); + results.insert(maybeDoc.key, static_cast(maybeDoc)); } } return results; } -FSTMaybeDocument *LevelDbRemoteDocumentCache::DecodeMaybeDocument( - absl::string_view encoded, const DocumentKey &key) { - NSData *data = [[NSData alloc] initWithBytesNoCopy:(void *)encoded.data() +FSTMaybeDocument* LevelDbRemoteDocumentCache::DecodeMaybeDocument( + absl::string_view encoded, const DocumentKey& key) { + NSData* data = [[NSData alloc] initWithBytesNoCopy:(void*)encoded.data() length:encoded.size() freeWhenDone:NO]; - NSError *error; - FSTPBMaybeDocument *proto = + NSError* error; + FSTPBMaybeDocument* proto = [FSTPBMaybeDocument parseFromData:data error:&error]; if (!proto) { HARD_FAIL("FSTPBMaybeDocument failed to parse: %s", error); } - FSTMaybeDocument *maybeDocument = [serializer_ decodedMaybeDocument:proto]; + FSTMaybeDocument* maybeDocument = [serializer_ decodedMaybeDocument:proto]; HARD_ASSERT(maybeDocument.key == key, "Read document has key (%s) instead of expected key (%s).", maybeDocument.key.ToString(), key.ToString()); @@ -131,4 +131,4 @@ } // namespace local } // namespace firestore -} // namespace firebase \ No newline at end of file +} // namespace firebase From 78650a66b3da5bea9cb2f6747073f451146ffebc Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 13 Dec 2018 16:01:14 -0800 Subject: [PATCH 04/15] Remove foundation import --- .../firebase/firestore/local/leveldb_remote_document_cache.h | 2 -- .../src/firebase/firestore/local/memory_remote_document_cache.h | 2 -- 2 files changed, 4 deletions(-) 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 165b7c13bec..b4ba9e3d17a 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 @@ -21,8 +21,6 @@ #error "For now, this file must only be included by ObjC source files." #endif // !defined(__OBJC__) -#import - #include #include "Firestore/core/src/firebase/firestore/model/document_key.h" 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 4c23ef54103..09538a0f4fa 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 @@ -21,8 +21,6 @@ #error "For now, this file must only be included by ObjC source files." #endif // !defined(__OBJC__) -#import - #include #include "Firestore/core/src/firebase/firestore/model/document_key.h" From c6781b281f3cd112be8da19f643ed6b3e2f40863 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 13 Dec 2018 16:05:01 -0800 Subject: [PATCH 05/15] Move foundation import to implementation file --- .../firebase/firestore/local/leveldb_remote_document_cache.mm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm index bfadd447733..5b52f0e0428 100644 --- a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm +++ b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm @@ -16,6 +16,10 @@ #include "Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h" +#include + +#import + #import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h" #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTLevelDB.h" From 3389e125697206e27f1c38e9edcd0e5853d9a1eb Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 13 Dec 2018 15:28:20 -0800 Subject: [PATCH 06/15] Start work on remote document cache interface port --- .../firestore/local/remote_document_cache.h | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Firestore/core/src/firebase/firestore/local/remote_document_cache.h 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..71371840aad --- /dev/null +++ b/Firestore/core/src/firebase/firestore/local/remote_document_cache.h @@ -0,0 +1,59 @@ +/* + * 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 { + +class RemoteDocumentCache { + public: + virtual ~RemoteDocumentCache() { + } + + virtual void AddEntry(FSTMaybeDocument *document) = 0; + virtual void RemoveEntry(const model::DocumentKey &key) = 0; + + virtual FSTMaybeDocument *_Nullable Get(const model::DocumentKey &key) = 0; + virtual model::MaybeDocumentMap GetAll(const model::DocumentKeySet &keys) = 0; + 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_ From 204d8c4233dece7163753573f610d081f7a0f3c4 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 13 Dec 2018 15:52:34 -0800 Subject: [PATCH 07/15] Mark inheritance --- .../firestore/local/leveldb_remote_document_cache.h | 13 +++++++------ .../firestore/local/memory_remote_document_cache.h | 13 +++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) 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, From 6e50b121da6c14db9990add5291b6063d113d1e9 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Fri, 14 Dec 2018 16:43:33 -0800 Subject: [PATCH 08/15] Checkpoint porting FSTRemoteDocumentCache --- .../Local/FSTLRUGarbageCollectorTests.mm | 17 ++++++++-------- Firestore/Source/Local/FSTLevelDB.mm | 11 +++++++--- .../Source/Local/FSTLocalDocumentsView.h | 4 ++-- .../Source/Local/FSTLocalDocumentsView.mm | 20 +++++++++++-------- Firestore/Source/Local/FSTLocalStore.mm | 15 ++++++++------ Firestore/Source/Local/FSTPersistence.h | 5 +++-- 6 files changed, 43 insertions(+), 29 deletions(-) diff --git a/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm b/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm index 93f1968323d..f61b40017f3 100644 --- a/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm +++ b/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm @@ -26,12 +26,12 @@ #import "Firestore/Source/Local/FSTMutationQueue.h" #import "Firestore/Source/Local/FSTPersistence.h" #import "Firestore/Source/Local/FSTQueryCache.h" -#import "Firestore/Source/Local/FSTRemoteDocumentCache.h" #import "Firestore/Source/Model/FSTDocument.h" #import "Firestore/Source/Model/FSTFieldValue.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Util/FSTClasses.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" +#include "Firestore/core/src/firebase/firestore/local/remote_document_cache.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/types.h" @@ -42,6 +42,7 @@ using firebase::firestore::auth::User; using firebase::firestore::local::LruParams; using firebase::firestore::local::LruResults; +using firebase::firestore::local::RemoteDocumentCache; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeyHash; using firebase::firestore::model::DocumentKeySet; @@ -58,7 +59,7 @@ @implementation FSTLRUGarbageCollectorTests { FSTObjectValue *_bigObjectValue; id _persistence; id _queryCache; - id _documentCache; + RemoteDocumentCache* _documentCache; id _mutationQueue; id _lruDelegate; FSTLRUGarbageCollector *_gc; @@ -209,7 +210,7 @@ - (void)removeDocument:(const DocumentKey &)docKey fromTarget:(TargetId)targetId */ - (FSTDocument *)cacheADocumentInTransaction { FSTDocument *doc = [self nextTestDocument]; - [_documentCache addEntry:doc]; + _documentCache->AddEntry(doc); return doc; } @@ -461,11 +462,11 @@ - (void)testRemoveOrphanedDocuments { XCTAssertEqual(toBeRemoved.size(), removed); _persistence.run("verify", [&]() { for (const DocumentKey &key : toBeRemoved) { - XCTAssertNil([_documentCache entryForKey:key]); + XCTAssertNil(_documentCache->Get(key)); XCTAssertFalse([_queryCache containsKey:key]); } for (const DocumentKey &key : expectedRetained) { - XCTAssertNotNil([_documentCache entryForKey:key], @"Missing document %s", + XCTAssertNotNil(_documentCache->Get(key), @"Missing document %s", key.ToString().c_str()); } }); @@ -618,7 +619,7 @@ - (void)testRemoveTargetsThenGC { key:middleDocToUpdate version:testutil::Version(version) state:FSTDocumentStateSynced]; - [_documentCache addEntry:doc]; + _documentCache->AddEntry(doc); [self updateTargetInTransaction:middleTarget]; }); @@ -643,14 +644,14 @@ - (void)testRemoveTargetsThenGC { XCTAssertEqual(expectedRemoved.size(), docsRemoved); _persistence.run("verify results", [&]() { for (const DocumentKey &key : expectedRemoved) { - XCTAssertNil([_documentCache entryForKey:key], @"Did not expect to find %s in document cache", + XCTAssertNil(_documentCache->Get(key), @"Did not expect to find %s in document cache", key.ToString().c_str()); XCTAssertFalse([_queryCache containsKey:key], @"Did not expect to find %s in queryCache", key.ToString().c_str()); [self expectSentinelRemoved:key]; } for (const DocumentKey &key : expectedRetained) { - XCTAssertNotNil([_documentCache entryForKey:key], @"Expected to find %s in document cache", + XCTAssertNotNil(_documentCache->Get(key), @"Expected to find %s in document cache", key.ToString().c_str()); } }); diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm index 116bb65b903..7edcaea7fa2 100644 --- a/Firestore/Source/Local/FSTLevelDB.mm +++ b/Firestore/Source/Local/FSTLevelDB.mm @@ -33,6 +33,7 @@ #include "Firestore/core/src/firebase/firestore/core/database_info.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_key.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_migrations.h" +#include "Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_util.h" #include "Firestore/core/src/firebase/firestore/model/database_id.h" @@ -60,6 +61,7 @@ using firebase::firestore::local::LevelDbDocumentTargetKey; using firebase::firestore::local::LevelDbMigrations; using firebase::firestore::local::LevelDbMutationKey; +using firebase::firestore::local::LevelDbRemoteDocumentCache; using firebase::firestore::local::LevelDbTransaction; using firebase::firestore::local::LruParams; using firebase::firestore::model::DatabaseId; @@ -211,7 +213,7 @@ - (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)upperB if (sequenceNumber <= upperBound) { if (![self isPinned:docKey]) { count++; - [self->_db.remoteDocumentCache removeEntryForKey:docKey]; + self->_db.remoteDocumentCache->RemoveEntry(docKey); [self removeSentinel:docKey]; } } @@ -267,6 +269,7 @@ @implementation FSTLevelDB { Path _directory; std::unique_ptr _transaction; std::unique_ptr _ptr; + std::unique_ptr _documentCache; FSTTransactionRunner _transactionRunner; FSTLevelDBLRUDelegate *_referenceDelegate; FSTLevelDBQueryCache *_queryCache; @@ -337,6 +340,7 @@ - (instancetype)initWithLevelDB:(std::unique_ptr)db _directory = std::move(directory); _serializer = serializer; _queryCache = [[FSTLevelDBQueryCache alloc] initWithDB:self serializer:self.serializer]; + _documentCache = absl::make_unique(self, _serializer); _referenceDelegate = [[FSTLevelDBLRUDelegate alloc] initWithPersistence:self lruParams:lruParams]; _transactionRunner.SetBackingPersistence(self); @@ -461,8 +465,9 @@ - (LevelDbTransaction *)currentTransaction { return _queryCache; } -- (id)remoteDocumentCache { - return [[FSTLevelDBRemoteDocumentCache alloc] initWithDB:self serializer:self.serializer]; +- (LevelDbRemoteDocumentCache *)remoteDocumentCache { + //return [[FSTLevelDBRemoteDocumentCache alloc] initWithDB:self serializer:self.serializer]; + return _documentCache.get(); } - (void)startTransaction:(absl::string_view)label { diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.h b/Firestore/Source/Local/FSTLocalDocumentsView.h index 6ec6570e548..33d045b8344 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.h +++ b/Firestore/Source/Local/FSTLocalDocumentsView.h @@ -16,6 +16,7 @@ #import +#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" @@ -23,7 +24,6 @@ @class FSTMaybeDocument; @class FSTQuery; @protocol FSTMutationQueue; -@protocol FSTRemoteDocumentCache; NS_ASSUME_NONNULL_BEGIN @@ -34,7 +34,7 @@ NS_ASSUME_NONNULL_BEGIN */ @interface FSTLocalDocumentsView : NSObject -+ (instancetype)viewWithRemoteDocumentCache:(id)remoteDocumentCache ++ (instancetype)viewWithRemoteDocumentCache:(firebase::firestore::local::RemoteDocumentCache *)remoteDocumentCache mutationQueue:(id)mutationQueue; - (instancetype)init __attribute__((unavailable("Use a static constructor"))); diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index e1820df16e5..41edeebdb6a 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -23,12 +23,14 @@ #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Model/FSTMutationBatch.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_map.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" +using firebase::firestore::local::RemoteDocumentCache; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::DocumentMap; @@ -39,22 +41,24 @@ NS_ASSUME_NONNULL_BEGIN @interface FSTLocalDocumentsView () -- (instancetype)initWithRemoteDocumentCache:(id)remoteDocumentCache +- (instancetype)initWithRemoteDocumentCache:(RemoteDocumentCache *)remoteDocumentCache mutationQueue:(id)mutationQueue NS_DESIGNATED_INITIALIZER; -@property(nonatomic, strong, readonly) id remoteDocumentCache; +//@property(nonatomic, strong, readonly) id remoteDocumentCache; @property(nonatomic, strong, readonly) id mutationQueue; @end -@implementation FSTLocalDocumentsView +@implementation FSTLocalDocumentsView { + RemoteDocumentCache* _remoteDocumentCache; +} -+ (instancetype)viewWithRemoteDocumentCache:(id)remoteDocumentCache ++ (instancetype)viewWithRemoteDocumentCache:(RemoteDocumentCache *)remoteDocumentCache mutationQueue:(id)mutationQueue { return [[FSTLocalDocumentsView alloc] initWithRemoteDocumentCache:remoteDocumentCache mutationQueue:mutationQueue]; } -- (instancetype)initWithRemoteDocumentCache:(id)remoteDocumentCache +- (instancetype)initWithRemoteDocumentCache:(RemoteDocumentCache *)remoteDocumentCache mutationQueue:(id)mutationQueue { if (self = [super init]) { _remoteDocumentCache = remoteDocumentCache; @@ -72,7 +76,7 @@ - (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key { // Internal version of documentForKey: which allows reusing `batches`. - (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key inBatches:(NSArray *)batches { - FSTMaybeDocument *_Nullable document = [self.remoteDocumentCache entryForKey:key]; + FSTMaybeDocument *_Nullable document = _remoteDocumentCache->Get(key); for (FSTMutationBatch *batch in batches) { document = [batch applyToLocalDocument:document documentKey:key]; } @@ -98,7 +102,7 @@ - (MaybeDocumentMap)applyLocalMutationsToDocuments:(const MaybeDocumentMap &)doc } - (MaybeDocumentMap)documentsForKeys:(const DocumentKeySet &)keys { - MaybeDocumentMap docs = [self.remoteDocumentCache entriesForKeys:keys]; + MaybeDocumentMap docs = _remoteDocumentCache->GetAll(keys); return [self localViewsForDocuments:docs]; } @@ -153,7 +157,7 @@ - (DocumentMap)documentsMatchingDocumentQuery:(const ResourcePath &)docPath { } - (DocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { - DocumentMap results = [self.remoteDocumentCache documentsMatchingQuery:query]; + DocumentMap results = _remoteDocumentCache->GetMatchingDocuments(query); // Get locally persisted mutation batches. NSArray *matchingBatches = [self.mutationQueue allMutationBatchesAffectingQuery:query]; diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index 7d04c080f4a..fb0e2689bcb 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -40,6 +40,7 @@ #include "Firestore/core/src/firebase/firestore/auth/user.h" #include "Firestore/core/src/firebase/firestore/core/target_id_generator.h" #include "Firestore/core/src/firebase/firestore/immutable/sorted_set.h" +#include "Firestore/core/src/firebase/firestore/local/remote_document_cache.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "Firestore/core/src/firebase/firestore/util/log.h" @@ -47,6 +48,7 @@ using firebase::firestore::auth::User; using firebase::firestore::core::TargetIdGenerator; using firebase::firestore::local::LruResults; +using firebase::firestore::local::RemoteDocumentCache; using firebase::firestore::model::BatchId; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; @@ -76,7 +78,7 @@ @interface FSTLocalStore () @property(nonatomic, strong) id mutationQueue; /** The set of all cached remote documents. */ -@property(nonatomic, strong) id remoteDocumentCache; +//@property(nonatomic) RemoteDocumentCache* remoteDocumentCache; /** The "local" view of all documents (layering mutationQueue on top of remoteDocumentCache). */ @property(nonatomic, strong) FSTLocalDocumentsView *localDocuments; @@ -95,6 +97,7 @@ @interface FSTLocalStore () @implementation FSTLocalStore { /** Used to generate targetIDs for queries tracked locally. */ TargetIdGenerator _targetIDGenerator; + RemoteDocumentCache* _remoteDocumentCache; } - (instancetype)initWithPersistence:(id)persistence @@ -141,7 +144,7 @@ - (MaybeDocumentMap)userDidChange:(const User &)user { // Recreate our LocalDocumentsView using the new MutationQueue. self.localDocuments = - [FSTLocalDocumentsView viewWithRemoteDocumentCache:self.remoteDocumentCache + [FSTLocalDocumentsView viewWithRemoteDocumentCache:_remoteDocumentCache mutationQueue:self.mutationQueue]; // Union the old/new changed keys. @@ -272,7 +275,7 @@ - (MaybeDocumentMap)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { } // Each loop iteration only affects its "own" doc, so it's safe to get all the remote // documents in advance in a single call. - MaybeDocumentMap existingDocs = [self.remoteDocumentCache entriesForKeys:updatedKeys]; + MaybeDocumentMap existingDocs = _remoteDocumentCache->GetAll(updatedKeys); for (const auto &kv : remoteEvent.documentUpdates) { const DocumentKey &key = kv.first; @@ -289,7 +292,7 @@ - (MaybeDocumentMap)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { if (!existingDoc || doc.version == SnapshotVersion::None() || (authoritativeUpdates.contains(doc.key) && !existingDoc.hasPendingWrites) || doc.version >= existingDoc.version) { - [self.remoteDocumentCache addEntry:doc]; + _remoteDocumentCache->AddEntry(doc); changedDocs = changedDocs.insert(key, doc); } else { LOG_DEBUG( @@ -453,7 +456,7 @@ - (void)applyBatchResult:(FSTMutationBatchResult *)batchResult { DocumentKeySet docKeys = batch.keys; const DocumentVersionMap &versions = batchResult.docVersions; for (const DocumentKey &docKey : docKeys) { - FSTMaybeDocument *_Nullable remoteDoc = [self.remoteDocumentCache entryForKey:docKey]; + FSTMaybeDocument *_Nullable remoteDoc = _remoteDocumentCache->Get(docKey); FSTMaybeDocument *_Nullable doc = remoteDoc; auto ackVersionIter = versions.find(docKey); @@ -466,7 +469,7 @@ - (void)applyBatchResult:(FSTMutationBatchResult *)batchResult { HARD_ASSERT(!remoteDoc, "Mutation batch %s applied to document %s resulted in nil.", batch, remoteDoc); } else { - [self.remoteDocumentCache addEntry:doc]; + _remoteDocumentCache->AddEntry(doc); } } } diff --git a/Firestore/Source/Local/FSTPersistence.h b/Firestore/Source/Local/FSTPersistence.h index 4b106af5080..b909f2b724e 100644 --- a/Firestore/Source/Local/FSTPersistence.h +++ b/Firestore/Source/Local/FSTPersistence.h @@ -17,6 +17,7 @@ #import #include "Firestore/core/src/firebase/firestore/auth/user.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/types.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" @@ -27,7 +28,6 @@ @protocol FSTMutationQueue; @protocol FSTQueryCache; @protocol FSTReferenceDelegate; -@protocol FSTRemoteDocumentCache; struct FSTTransactionRunner; @@ -82,7 +82,8 @@ NS_ASSUME_NONNULL_BEGIN - (id)queryCache; /** Creates an FSTRemoteDocumentCache representing the persisted cache of remote documents. */ -- (id)remoteDocumentCache; +//- (id)remoteDocumentCache; +- (firebase::firestore::local::RemoteDocumentCache *)remoteDocumentCache; @property(nonatomic, readonly, assign) const FSTTransactionRunner &run; From 45a1e2f9bff0dc91721e5f0e8ea2c2d042efeb5c Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 17 Dec 2018 10:39:17 -0800 Subject: [PATCH 09/15] 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_ From 43e86097420cf38d530b690212ab4f57b110bed5 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 17 Dec 2018 13:47:09 -0800 Subject: [PATCH 10/15] Remove FSTRemoteDocumentCache --- .../Tests/Local/FSTRemoteDocumentCacheTests.h | 2 - Firestore/Source/Local/FSTLevelDB.mm | 1 - .../Local/FSTLevelDBRemoteDocumentCache.h | 43 ---------- .../Local/FSTLevelDBRemoteDocumentCache.mm | 82 ------------------ .../Source/Local/FSTLocalDocumentsView.mm | 1 - Firestore/Source/Local/FSTLocalStore.mm | 1 - .../Source/Local/FSTMemoryPersistence.mm | 21 +++-- .../Local/FSTMemoryRemoteDocumentCache.h | 41 --------- .../Local/FSTMemoryRemoteDocumentCache.mm | 74 ---------------- Firestore/Source/Local/FSTPersistence.h | 1 - .../Source/Local/FSTRemoteDocumentCache.h | 84 ------------------- 11 files changed, 10 insertions(+), 341 deletions(-) delete mode 100644 Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.h delete mode 100644 Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm delete mode 100644 Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h delete mode 100644 Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm delete mode 100644 Firestore/Source/Local/FSTRemoteDocumentCache.h diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h index 71e0073f79a..8b660eae0ef 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h @@ -14,8 +14,6 @@ * limitations under the License. */ -#import "Firestore/Source/Local/FSTRemoteDocumentCache.h" - #import #include "Firestore/core/src/firebase/firestore/local/remote_document_cache.h" diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm index 887c3df22fd..23af686c5b0 100644 --- a/Firestore/Source/Local/FSTLevelDB.mm +++ b/Firestore/Source/Local/FSTLevelDB.mm @@ -24,7 +24,6 @@ #import "Firestore/Source/Local/FSTLRUGarbageCollector.h" #import "Firestore/Source/Local/FSTLevelDBMutationQueue.h" #import "Firestore/Source/Local/FSTLevelDBQueryCache.h" -#import "Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.h" #import "Firestore/Source/Local/FSTReferenceSet.h" #import "Firestore/Source/Remote/FSTSerializerBeta.h" diff --git a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.h b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.h deleted file mode 100644 index 381d308d203..00000000000 --- a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.h +++ /dev/null @@ -1,43 +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 - -#import "Firestore/Source/Local/FSTRemoteDocumentCache.h" - -@class FSTLevelDB; -@class FSTLocalSerializer; - -NS_ASSUME_NONNULL_BEGIN - -/** Cached Remote Documents backed by leveldb. */ -@interface FSTLevelDBRemoteDocumentCache : NSObject - -- (instancetype)init NS_UNAVAILABLE; - -/** - * Creates a new remote documents cache in the given leveldb. - * - * @param db The leveldb in which to create the cache. - */ -- (instancetype)initWithDB:(FSTLevelDB *)db - serializer:(FSTLocalSerializer *)serializer NS_DESIGNATED_INITIALIZER; - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm deleted file mode 100644 index f4a4a3bdbf7..00000000000 --- a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm +++ /dev/null @@ -1,82 +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/FSTLevelDBRemoteDocumentCache.h" - -#include - -#import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h" -#import "Firestore/Source/Core/FSTQuery.h" -#import "Firestore/Source/Local/FSTLevelDB.h" -#import "Firestore/Source/Local/FSTLocalSerializer.h" -#import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentSet.h" - -#include "Firestore/core/src/firebase/firestore/local/leveldb_key.h" -#include "Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h" -#include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h" -#include "Firestore/core/src/firebase/firestore/model/document_key.h" -#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" -#include "absl/memory/memory.h" -#include "leveldb/db.h" -#include "leveldb/write_batch.h" - -NS_ASSUME_NONNULL_BEGIN - -using firebase::firestore::local::LevelDbRemoteDocumentCache; -using firebase::firestore::local::LevelDbRemoteDocumentKey; -using firebase::firestore::local::LevelDbTransaction; -using firebase::firestore::model::DocumentKey; -using firebase::firestore::model::DocumentKeySet; -using firebase::firestore::model::DocumentMap; -using firebase::firestore::model::MaybeDocumentMap; -using leveldb::DB; -using leveldb::Status; - -@implementation FSTLevelDBRemoteDocumentCache { - std::unique_ptr _cache; -} - -- (instancetype)initWithDB:(FSTLevelDB *)db serializer:(FSTLocalSerializer *)serializer { - if (self = [super init]) { - _cache = absl::make_unique(db, serializer); - } - return self; -} - -- (void)addEntry:(FSTMaybeDocument *)document { - _cache->AddEntry(document); -} - -- (void)removeEntryForKey:(const DocumentKey &)documentKey { - _cache->RemoveEntry(documentKey); -} - -- (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)documentKey { - return _cache->Get(documentKey); -} - -- (MaybeDocumentMap)entriesForKeys:(const DocumentKeySet &)keys { - return _cache->GetAll(keys); -} - -- (DocumentMap)documentsMatchingQuery:(FSTQuery *)query { - return _cache->GetMatchingDocuments(query); -} - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 41edeebdb6a..00599785466 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -18,7 +18,6 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTMutationQueue.h" -#import "Firestore/Source/Local/FSTRemoteDocumentCache.h" #import "Firestore/Source/Model/FSTDocument.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Model/FSTMutationBatch.h" diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index fb0e2689bcb..72e70028fe1 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -31,7 +31,6 @@ #import "Firestore/Source/Local/FSTQueryCache.h" #import "Firestore/Source/Local/FSTQueryData.h" #import "Firestore/Source/Local/FSTReferenceSet.h" -#import "Firestore/Source/Local/FSTRemoteDocumentCache.h" #import "Firestore/Source/Model/FSTDocument.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Model/FSTMutationBatch.h" diff --git a/Firestore/Source/Local/FSTMemoryPersistence.mm b/Firestore/Source/Local/FSTMemoryPersistence.mm index 6aa8d6738e8..f0687b9e30a 100644 --- a/Firestore/Source/Local/FSTMemoryPersistence.mm +++ b/Firestore/Source/Local/FSTMemoryPersistence.mm @@ -24,17 +24,18 @@ #import "Firestore/Source/Core/FSTListenSequence.h" #import "Firestore/Source/Local/FSTMemoryMutationQueue.h" #import "Firestore/Source/Local/FSTMemoryQueryCache.h" -#import "Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h" #import "Firestore/Source/Local/FSTReferenceSet.h" #include "absl/memory/memory.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" +#include "Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" using firebase::firestore::auth::HashUser; using firebase::firestore::auth::User; using firebase::firestore::local::LruParams; +using firebase::firestore::local::MemoryRemoteDocumentCache; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeyHash; using firebase::firestore::model::ListenSequenceNumber; @@ -48,7 +49,7 @@ @interface FSTMemoryPersistence () - (FSTMemoryQueryCache *)queryCache; -- (FSTMemoryRemoteDocumentCache *)remoteDocumentCache; +- (MemoryRemoteDocumentCache *)remoteDocumentCache; @property(nonatomic, readonly) MutationQueues &mutationQueues; @@ -71,7 +72,7 @@ @implementation FSTMemoryPersistence { FSTMemoryQueryCache *_queryCache; /** The FSTRemoteDocumentCache representing the persisted cache of remote documents. */ - FSTMemoryRemoteDocumentCache *_remoteDocumentCache; + MemoryRemoteDocumentCache _remoteDocumentCache; FSTTransactionRunner _transactionRunner; @@ -98,7 +99,7 @@ + (instancetype)persistenceWithLruParams:(firebase::firestore::local::LruParams) - (instancetype)init { if (self = [super init]) { _queryCache = [[FSTMemoryQueryCache alloc] initWithPersistence:self]; - _remoteDocumentCache = [[FSTMemoryRemoteDocumentCache alloc] init]; + //_remoteDocumentCache = [[FSTMemoryRemoteDocumentCache alloc] init]; self.started = YES; } return self; @@ -143,8 +144,8 @@ - (FSTMemoryQueryCache *)queryCache { return _queryCache; } -- (id)remoteDocumentCache { - return _remoteDocumentCache; +- (MemoryRemoteDocumentCache *)remoteDocumentCache { + return &_remoteDocumentCache; } @end @@ -249,9 +250,7 @@ - (int32_t)sequenceNumberCount { - (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)upperBound { std::vector removed = - [(FSTMemoryRemoteDocumentCache *)_persistence.remoteDocumentCache - removeOrphanedDocuments:self - throughSequenceNumber:upperBound]; + static_cast(_persistence.remoteDocumentCache)->RemoveOrphanedDocuments(self, upperBound); for (const auto &key : removed) { _sequenceNumbers.erase(key); } @@ -304,7 +303,7 @@ - (size_t)byteSize { // and count bytes) is inefficient and inexact, but won't run in production. size_t count = 0; count += [_persistence.queryCache byteSizeWithSerializer:_serializer]; - count += [_persistence.remoteDocumentCache byteSizeWithSerializer:_serializer]; + count += _persistence.remoteDocumentCache->CalculateByteSize(_serializer); const MutationQueues &queues = [_persistence mutationQueues]; for (const auto &entry : queues) { count += [entry.second byteSizeWithSerializer:_serializer]; @@ -397,7 +396,7 @@ - (BOOL)mutationQueuesContainKey:(const DocumentKey &)key { - (void)commitTransaction { for (const auto &key : *_orphaned) { if (![self isReferenced:key]) { - [[_persistence remoteDocumentCache] removeEntryForKey:key]; + _persistence.remoteDocumentCache->RemoveEntry(key); } } _orphaned.reset(); diff --git a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h deleted file mode 100644 index 1f0884fe33a..00000000000 --- a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h +++ /dev/null @@ -1,41 +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 - -#import "Firestore/Source/Local/FSTRemoteDocumentCache.h" - -#include "Firestore/core/src/firebase/firestore/model/document_key.h" -#include "Firestore/core/src/firebase/firestore/model/types.h" - -NS_ASSUME_NONNULL_BEGIN - -@class FSTLocalSerializer; -@class FSTMemoryLRUReferenceDelegate; - -@interface FSTMemoryRemoteDocumentCache : NSObject - -- (std::vector) - removeOrphanedDocuments:(FSTMemoryLRUReferenceDelegate *)referenceDelegate - throughSequenceNumber:(firebase::firestore::model::ListenSequenceNumber)upperBound; - -- (size_t)byteSizeWithSerializer:(FSTLocalSerializer *)serializer; - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm deleted file mode 100644 index 4f4070141aa..00000000000 --- a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm +++ /dev/null @@ -1,74 +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/FSTMemoryRemoteDocumentCache.h" - -#import -#import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h" -#import "Firestore/Source/Core/FSTQuery.h" -#import "Firestore/Source/Local/FSTMemoryPersistence.h" -#import "Firestore/Source/Model/FSTDocument.h" - -#include "Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.h" -#include "Firestore/core/src/firebase/firestore/model/document_key.h" -#include "Firestore/core/src/firebase/firestore/model/document_map.h" - -using firebase::firestore::local::MemoryRemoteDocumentCache; -using firebase::firestore::model::DocumentKey; -using firebase::firestore::model::DocumentKeySet; -using firebase::firestore::model::ListenSequenceNumber; -using firebase::firestore::model::DocumentMap; -using firebase::firestore::model::MaybeDocumentMap; - -NS_ASSUME_NONNULL_BEGIN - -@implementation FSTMemoryRemoteDocumentCache { - MemoryRemoteDocumentCache _cache; -} - -- (void)addEntry:(FSTMaybeDocument *)document { - _cache.AddEntry(document); -} - -- (void)removeEntryForKey:(const DocumentKey &)key { - _cache.RemoveEntry(key); -} - -- (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)key { - return _cache.Get(key); -} - -- (MaybeDocumentMap)entriesForKeys:(const DocumentKeySet &)keys { - return _cache.GetAll(keys); -} - -- (DocumentMap)documentsMatchingQuery:(FSTQuery *)query { - return _cache.GetMatchingDocuments(query); -} - -- (std::vector)removeOrphanedDocuments: - (FSTMemoryLRUReferenceDelegate *)referenceDelegate - throughSequenceNumber:(ListenSequenceNumber)upperBound { - return _cache.RemoveOrphanedDocuments(referenceDelegate, upperBound); -} - -- (size_t)byteSizeWithSerializer:(FSTLocalSerializer *)serializer { - return _cache.CalculateByteSize(serializer); -} - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Local/FSTPersistence.h b/Firestore/Source/Local/FSTPersistence.h index b909f2b724e..8c9b7a62136 100644 --- a/Firestore/Source/Local/FSTPersistence.h +++ b/Firestore/Source/Local/FSTPersistence.h @@ -82,7 +82,6 @@ NS_ASSUME_NONNULL_BEGIN - (id)queryCache; /** Creates an FSTRemoteDocumentCache representing the persisted cache of remote documents. */ -//- (id)remoteDocumentCache; - (firebase::firestore::local::RemoteDocumentCache *)remoteDocumentCache; @property(nonatomic, readonly, assign) const FSTTransactionRunner &run; diff --git a/Firestore/Source/Local/FSTRemoteDocumentCache.h b/Firestore/Source/Local/FSTRemoteDocumentCache.h deleted file mode 100644 index 51a6cf1b72e..00000000000 --- a/Firestore/Source/Local/FSTRemoteDocumentCache.h +++ /dev/null @@ -1,84 +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" -#include "Firestore/core/src/firebase/firestore/model/document_key_set.h" -#include "Firestore/core/src/firebase/firestore/model/document_map.h" - -@class FSTMaybeDocument; -@class FSTQuery; - -NS_ASSUME_NONNULL_BEGIN - -/** - * 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). - */ -@protocol FSTRemoteDocumentCache - -/** - * Adds or replaces an entry in the cache. - * - * The cache key is extracted from `maybeDocument.key`. If there is already a cache entry for - * the key, it will be replaced. - * - * @param maybeDocument A FSTDocument or FSTDeletedDocument to put in the cache. - */ -- (void)addEntry:(FSTMaybeDocument *)maybeDocument; - -/** Removes the cached entry for the given key (no-op if no entry exists). */ -- (void)removeEntryForKey:(const firebase::firestore::model::DocumentKey &)documentKey; - -/** - * 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. - */ -- (nullable FSTMaybeDocument *)entryForKey: - (const firebase::firestore::model::DocumentKey &)documentKey; - -/** - * Looks up a set of entries in the cache. - * - * @param documentKeys 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. - */ -- (firebase::firestore::model::MaybeDocumentMap)entriesForKeys: - (const firebase::firestore::model::DocumentKeySet &)documentKeys; - -/** - * 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. - */ -- (firebase::firestore::model::DocumentMap)documentsMatchingQuery:(FSTQuery *)query; - -@end - -NS_ASSUME_NONNULL_END From 02b2b58ad2504c89d78a20d6001f728e5f8d0301 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 17 Dec 2018 13:47:52 -0800 Subject: [PATCH 11/15] style --- .../Example/Tests/Local/FSTLRUGarbageCollectorTests.mm | 5 ++--- Firestore/Source/Local/FSTLevelDB.mm | 2 +- Firestore/Source/Local/FSTLocalDocumentsView.h | 3 ++- Firestore/Source/Local/FSTLocalDocumentsView.mm | 2 +- Firestore/Source/Local/FSTLocalStore.mm | 7 +++---- Firestore/Source/Local/FSTMemoryPersistence.mm | 3 ++- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm b/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm index f61b40017f3..fed6d1b6378 100644 --- a/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm +++ b/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm @@ -59,7 +59,7 @@ @implementation FSTLRUGarbageCollectorTests { FSTObjectValue *_bigObjectValue; id _persistence; id _queryCache; - RemoteDocumentCache* _documentCache; + RemoteDocumentCache *_documentCache; id _mutationQueue; id _lruDelegate; FSTLRUGarbageCollector *_gc; @@ -466,8 +466,7 @@ - (void)testRemoveOrphanedDocuments { XCTAssertFalse([_queryCache containsKey:key]); } for (const DocumentKey &key : expectedRetained) { - XCTAssertNotNil(_documentCache->Get(key), @"Missing document %s", - key.ToString().c_str()); + XCTAssertNotNil(_documentCache->Get(key), @"Missing document %s", key.ToString().c_str()); } }); [_persistence shutdown]; diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm index 23af686c5b0..d947fb02cab 100644 --- a/Firestore/Source/Local/FSTLevelDB.mm +++ b/Firestore/Source/Local/FSTLevelDB.mm @@ -464,7 +464,7 @@ - (LevelDbTransaction *)currentTransaction { } - (LevelDbRemoteDocumentCache *)remoteDocumentCache { - //return [[FSTLevelDBRemoteDocumentCache alloc] initWithDB:self serializer:self.serializer]; + // return [[FSTLevelDBRemoteDocumentCache alloc] initWithDB:self serializer:self.serializer]; return _documentCache.get(); } diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.h b/Firestore/Source/Local/FSTLocalDocumentsView.h index 33d045b8344..a559b7c2136 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.h +++ b/Firestore/Source/Local/FSTLocalDocumentsView.h @@ -34,7 +34,8 @@ NS_ASSUME_NONNULL_BEGIN */ @interface FSTLocalDocumentsView : NSObject -+ (instancetype)viewWithRemoteDocumentCache:(firebase::firestore::local::RemoteDocumentCache *)remoteDocumentCache ++ (instancetype)viewWithRemoteDocumentCache: + (firebase::firestore::local::RemoteDocumentCache *)remoteDocumentCache mutationQueue:(id)mutationQueue; - (instancetype)init __attribute__((unavailable("Use a static constructor"))); diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 00599785466..8d493b2d1f5 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -48,7 +48,7 @@ - (instancetype)initWithRemoteDocumentCache:(RemoteDocumentCache *)remoteDocumen @end @implementation FSTLocalDocumentsView { - RemoteDocumentCache* _remoteDocumentCache; + RemoteDocumentCache *_remoteDocumentCache; } + (instancetype)viewWithRemoteDocumentCache:(RemoteDocumentCache *)remoteDocumentCache diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index 72e70028fe1..1dc4a2fe16e 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -96,7 +96,7 @@ @interface FSTLocalStore () @implementation FSTLocalStore { /** Used to generate targetIDs for queries tracked locally. */ TargetIdGenerator _targetIDGenerator; - RemoteDocumentCache* _remoteDocumentCache; + RemoteDocumentCache *_remoteDocumentCache; } - (instancetype)initWithPersistence:(id)persistence @@ -142,9 +142,8 @@ - (MaybeDocumentMap)userDidChange:(const User &)user { NSArray *newBatches = [self.mutationQueue allMutationBatches]; // Recreate our LocalDocumentsView using the new MutationQueue. - self.localDocuments = - [FSTLocalDocumentsView viewWithRemoteDocumentCache:_remoteDocumentCache - mutationQueue:self.mutationQueue]; + self.localDocuments = [FSTLocalDocumentsView viewWithRemoteDocumentCache:_remoteDocumentCache + mutationQueue:self.mutationQueue]; // Union the old/new changed keys. DocumentKeySet changedKeys; diff --git a/Firestore/Source/Local/FSTMemoryPersistence.mm b/Firestore/Source/Local/FSTMemoryPersistence.mm index f0687b9e30a..cd7295ceae4 100644 --- a/Firestore/Source/Local/FSTMemoryPersistence.mm +++ b/Firestore/Source/Local/FSTMemoryPersistence.mm @@ -250,7 +250,8 @@ - (int32_t)sequenceNumberCount { - (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)upperBound { std::vector removed = - static_cast(_persistence.remoteDocumentCache)->RemoveOrphanedDocuments(self, upperBound); + static_cast(_persistence.remoteDocumentCache) + ->RemoveOrphanedDocuments(self, upperBound); for (const auto &key : removed) { _sequenceNumbers.erase(key); } From f2f267230f5e9d5ab4fe338484078d4a410ba9da Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 17 Dec 2018 14:14:58 -0800 Subject: [PATCH 12/15] Fix warnings --- .../Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm | 4 +++- .../Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm | 4 +++- Firestore/Source/Local/FSTLevelDB.h | 2 +- Firestore/Source/Local/FSTLevelDB.mm | 5 +++-- .../src/firebase/firestore/local/remote_document_cache.h | 2 +- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm index 78e105c9dc7..ac106caad0d 100644 --- a/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm @@ -21,6 +21,7 @@ #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/local/remote_document_cache.h" #include "Firestore/core/src/firebase/firestore/util/ordered_code.h" #include "absl/memory/memory.h" @@ -30,6 +31,7 @@ using leveldb::WriteOptions; using firebase::firestore::local::LevelDbRemoteDocumentCache; +using firebase::firestore::local::RemoteDocumentCache; using firebase::firestore::util::OrderedCode; // A dummy document value, useful for testing code that's known to examine only document keys. @@ -61,7 +63,7 @@ - (void)setUp { [self writeDummyRowWithSegments:@[ @"remote_documentsa", @"foo", @"bar" ]]; } -- (LevelDbRemoteDocumentCache *_Nullable)remoteDocumentCache { +- (RemoteDocumentCache *_Nullable)remoteDocumentCache { return _cache.get(); } diff --git a/Firestore/Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm index c33ce4b63ee..c39a15f1fbf 100644 --- a/Firestore/Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTMemoryRemoteDocumentCacheTests.mm @@ -18,12 +18,14 @@ #import "Firestore/Source/Local/FSTMemoryPersistence.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 "absl/memory/memory.h" #import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h" #import "Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h" using firebase::firestore::local::MemoryRemoteDocumentCache; +using firebase::firestore::local::RemoteDocumentCache; @interface FSTMemoryRemoteDocumentCacheTests : FSTRemoteDocumentCacheTests @end @@ -45,7 +47,7 @@ - (void)setUp { _cache = absl::make_unique(); } -- (MemoryRemoteDocumentCache *)remoteDocumentCache { +- (RemoteDocumentCache *)remoteDocumentCache { return _cache.get(); } diff --git a/Firestore/Source/Local/FSTLevelDB.h b/Firestore/Source/Local/FSTLevelDB.h index de78f724629..5f888096a00 100644 --- a/Firestore/Source/Local/FSTLevelDB.h +++ b/Firestore/Source/Local/FSTLevelDB.h @@ -50,7 +50,7 @@ NS_ASSUME_NONNULL_BEGIN serializer:(FSTLocalSerializer *)serializer lruParams: (firebase::firestore::local::LruParams)lruParams - ptr:(FSTLevelDB **)ptr; + ptr:(FSTLevelDB *_Nullable*_Nonnull)ptr; - (instancetype)init NS_UNAVAILABLE; diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm index d947fb02cab..e9b96b835cf 100644 --- a/Firestore/Source/Local/FSTLevelDB.mm +++ b/Firestore/Source/Local/FSTLevelDB.mm @@ -35,6 +35,7 @@ #include "Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_util.h" +#include "Firestore/core/src/firebase/firestore/local/remote_document_cache.h" #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" @@ -63,6 +64,7 @@ using firebase::firestore::local::LevelDbRemoteDocumentCache; using firebase::firestore::local::LevelDbTransaction; using firebase::firestore::local::LruParams; +using firebase::firestore::local::RemoteDocumentCache; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::ListenSequenceNumber; @@ -463,8 +465,7 @@ - (LevelDbTransaction *)currentTransaction { return _queryCache; } -- (LevelDbRemoteDocumentCache *)remoteDocumentCache { - // return [[FSTLevelDBRemoteDocumentCache alloc] initWithDB:self serializer:self.serializer]; +- (RemoteDocumentCache *)remoteDocumentCache { return _documentCache.get(); } diff --git a/Firestore/core/src/firebase/firestore/local/remote_document_cache.h b/Firestore/core/src/firebase/firestore/local/remote_document_cache.h index b9721281a35..01e1109bf0f 100644 --- a/Firestore/core/src/firebase/firestore/local/remote_document_cache.h +++ b/Firestore/core/src/firebase/firestore/local/remote_document_cache.h @@ -66,7 +66,7 @@ class RemoteDocumentCache { /** * Looks up an entry in the cache. * - * @param documentKey The key of the entry to look up. + * @param key The key of the entry to look up. * @return The cached FSTDocument or FSTDeletedDocument entry, or nil if we * have nothing cached. */ From 7bc391ecde910c71c513fa5d179793bd80454219 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 17 Dec 2018 14:22:49 -0800 Subject: [PATCH 13/15] Style and remove cast --- Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm | 3 --- Firestore/Source/Local/FSTLevelDB.h | 2 +- Firestore/Source/Local/FSTMemoryPersistence.mm | 3 +-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm index e94224cdbdb..f33bf1545ff 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm @@ -21,7 +21,6 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTPersistence.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentSet.h" #import "Firestore/Example/Tests/Util/FSTHelpers.h" @@ -188,8 +187,6 @@ - (void)testDocumentsMatchingQuery { } #pragma mark - Helpers -// TODO(gsoltis): reevaluate if any of these helpers are still needed - - (FSTDocument *)setTestDocumentAtPath:(const absl::string_view)path { FSTDocument *doc = FSTTestDoc(path, kVersion, _kDocData, FSTDocumentStateSynced); self.remoteDocumentCache->AddEntry(doc); diff --git a/Firestore/Source/Local/FSTLevelDB.h b/Firestore/Source/Local/FSTLevelDB.h index 5f888096a00..2ef3f6eb47e 100644 --- a/Firestore/Source/Local/FSTLevelDB.h +++ b/Firestore/Source/Local/FSTLevelDB.h @@ -50,7 +50,7 @@ NS_ASSUME_NONNULL_BEGIN serializer:(FSTLocalSerializer *)serializer lruParams: (firebase::firestore::local::LruParams)lruParams - ptr:(FSTLevelDB *_Nullable*_Nonnull)ptr; + ptr:(FSTLevelDB *_Nullable *_Nonnull)ptr; - (instancetype)init NS_UNAVAILABLE; diff --git a/Firestore/Source/Local/FSTMemoryPersistence.mm b/Firestore/Source/Local/FSTMemoryPersistence.mm index cd7295ceae4..3c8dac2f316 100644 --- a/Firestore/Source/Local/FSTMemoryPersistence.mm +++ b/Firestore/Source/Local/FSTMemoryPersistence.mm @@ -250,8 +250,7 @@ - (int32_t)sequenceNumberCount { - (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)upperBound { std::vector removed = - static_cast(_persistence.remoteDocumentCache) - ->RemoveOrphanedDocuments(self, upperBound); + _persistence.remoteDocumentCache->RemoveOrphanedDocuments(self, upperBound); for (const auto &key : removed) { _sequenceNumbers.erase(key); } From 5bdf42bc0df510b2b8760f260905dbb631a1bc5d Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 17 Dec 2018 14:32:39 -0800 Subject: [PATCH 14/15] Drop commented-out code --- Firestore/Source/Local/FSTLocalDocumentsView.mm | 2 +- Firestore/Source/Local/FSTLocalStore.mm | 4 +--- Firestore/Source/Local/FSTMemoryPersistence.mm | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 8d493b2d1f5..4d1362e1f9e 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -43,7 +43,7 @@ @interface FSTLocalDocumentsView () - (instancetype)initWithRemoteDocumentCache:(RemoteDocumentCache *)remoteDocumentCache mutationQueue:(id)mutationQueue NS_DESIGNATED_INITIALIZER; -//@property(nonatomic, strong, readonly) id remoteDocumentCache; + @property(nonatomic, strong, readonly) id mutationQueue; @end diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index 1dc4a2fe16e..95397907b23 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -76,9 +76,6 @@ @interface FSTLocalStore () /** The set of all mutations that have been sent but not yet been applied to the backend. */ @property(nonatomic, strong) id mutationQueue; -/** The set of all cached remote documents. */ -//@property(nonatomic) RemoteDocumentCache* remoteDocumentCache; - /** The "local" view of all documents (layering mutationQueue on top of remoteDocumentCache). */ @property(nonatomic, strong) FSTLocalDocumentsView *localDocuments; @@ -96,6 +93,7 @@ @interface FSTLocalStore () @implementation FSTLocalStore { /** Used to generate targetIDs for queries tracked locally. */ TargetIdGenerator _targetIDGenerator; + /** The set of all cached remote documents. */ RemoteDocumentCache *_remoteDocumentCache; } diff --git a/Firestore/Source/Local/FSTMemoryPersistence.mm b/Firestore/Source/Local/FSTMemoryPersistence.mm index 3c8dac2f316..c34eeef2bef 100644 --- a/Firestore/Source/Local/FSTMemoryPersistence.mm +++ b/Firestore/Source/Local/FSTMemoryPersistence.mm @@ -71,7 +71,7 @@ @implementation FSTMemoryPersistence { */ FSTMemoryQueryCache *_queryCache; - /** The FSTRemoteDocumentCache representing the persisted cache of remote documents. */ + /** The RemoteDocumentCache representing the persisted cache of remote documents. */ MemoryRemoteDocumentCache _remoteDocumentCache; FSTTransactionRunner _transactionRunner; @@ -99,7 +99,6 @@ + (instancetype)persistenceWithLruParams:(firebase::firestore::local::LruParams) - (instancetype)init { if (self = [super init]) { _queryCache = [[FSTMemoryQueryCache alloc] initWithPersistence:self]; - //_remoteDocumentCache = [[FSTMemoryRemoteDocumentCache alloc] init]; self.started = YES; } return self; From cd97a9eb802f3c9af5e07c6d230a436e9a531252 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 18 Dec 2018 12:24:44 -0800 Subject: [PATCH 15/15] Review feedback --- .../Local/FSTLRUGarbageCollectorTests.mm | 4 +-- .../Local/FSTRemoteDocumentCacheTests.mm | 12 +++---- Firestore/Source/Local/FSTLevelDB.mm | 2 +- .../Source/Local/FSTLocalDocumentsView.mm | 2 +- Firestore/Source/Local/FSTLocalStore.mm | 4 +-- .../Source/Local/FSTMemoryPersistence.mm | 2 +- .../local/leveldb_remote_document_cache.h | 6 ++-- .../local/leveldb_remote_document_cache.mm | 6 ++-- .../local/memory_remote_document_cache.h | 6 ++-- .../local/memory_remote_document_cache.mm | 34 +++++++++---------- .../firestore/local/remote_document_cache.h | 6 ++-- 11 files changed, 42 insertions(+), 42 deletions(-) diff --git a/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm b/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm index fed6d1b6378..296791b1ce9 100644 --- a/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm +++ b/Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm @@ -210,7 +210,7 @@ - (void)removeDocument:(const DocumentKey &)docKey fromTarget:(TargetId)targetId */ - (FSTDocument *)cacheADocumentInTransaction { FSTDocument *doc = [self nextTestDocument]; - _documentCache->AddEntry(doc); + _documentCache->Add(doc); return doc; } @@ -618,7 +618,7 @@ - (void)testRemoveTargetsThenGC { key:middleDocToUpdate version:testutil::Version(version) state:FSTDocumentStateSynced]; - _documentCache->AddEntry(doc); + _documentCache->Add(doc); [self updateTargetInTransaction:middleTarget]; }); diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm index f33bf1545ff..c13d967bf89 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm @@ -126,7 +126,7 @@ - (void)testSetAndReadDeletedDocument { self.persistence.run("testSetAndReadDeletedDocument", [&]() { FSTDeletedDocument *deletedDoc = FSTTestDeletedDoc(kDocPath, kVersion, NO); - self.remoteDocumentCache->AddEntry(deletedDoc); + self.remoteDocumentCache->Add(deletedDoc); XCTAssertEqualObjects(self.remoteDocumentCache->Get(testutil::Key(kDocPath)), deletedDoc); }); @@ -138,7 +138,7 @@ - (void)testSetDocumentToNewValue { self.persistence.run("testSetDocumentToNewValue", [&]() { [self setTestDocumentAtPath:kDocPath]; FSTDocument *newDoc = FSTTestDoc(kDocPath, kVersion, @{@"data" : @2}, FSTDocumentStateSynced); - self.remoteDocumentCache->AddEntry(newDoc); + self.remoteDocumentCache->Add(newDoc); XCTAssertEqualObjects(self.remoteDocumentCache->Get(testutil::Key(kDocPath)), newDoc); }); } @@ -148,7 +148,7 @@ - (void)testRemoveDocument { self.persistence.run("testRemoveDocument", [&]() { [self setTestDocumentAtPath:kDocPath]; - self.remoteDocumentCache->RemoveEntry(testutil::Key(kDocPath)); + self.remoteDocumentCache->Remove(testutil::Key(kDocPath)); XCTAssertNil(self.remoteDocumentCache->Get(testutil::Key(kDocPath))); }); @@ -159,7 +159,7 @@ - (void)testRemoveNonExistentDocument { self.persistence.run("testRemoveNonExistentDocument", [&]() { // no-op, but make sure it doesn't throw. - XCTAssertNoThrow(self.remoteDocumentCache->RemoveEntry(testutil::Key(kDocPath))); + XCTAssertNoThrow(self.remoteDocumentCache->Remove(testutil::Key(kDocPath))); }); } @@ -176,7 +176,7 @@ - (void)testDocumentsMatchingQuery { [self setTestDocumentAtPath:"c/1"]; FSTQuery *query = FSTTestQuery("b"); - DocumentMap results = self.remoteDocumentCache->GetMatchingDocuments(query); + DocumentMap results = self.remoteDocumentCache->GetMatching(query); [self expectMap:results.underlying_map() hasDocsInArray:@[ FSTTestDoc("b/1", kVersion, _kDocData, FSTDocumentStateSynced), @@ -189,7 +189,7 @@ - (void)testDocumentsMatchingQuery { #pragma mark - Helpers - (FSTDocument *)setTestDocumentAtPath:(const absl::string_view)path { FSTDocument *doc = FSTTestDoc(path, kVersion, _kDocData, FSTDocumentStateSynced); - self.remoteDocumentCache->AddEntry(doc); + self.remoteDocumentCache->Add(doc); return doc; } diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm index e9b96b835cf..04caa3830d3 100644 --- a/Firestore/Source/Local/FSTLevelDB.mm +++ b/Firestore/Source/Local/FSTLevelDB.mm @@ -213,7 +213,7 @@ - (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)upperB if (sequenceNumber <= upperBound) { if (![self isPinned:docKey]) { count++; - self->_db.remoteDocumentCache->RemoveEntry(docKey); + self->_db.remoteDocumentCache->Remove(docKey); [self removeSentinel:docKey]; } } diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 4d1362e1f9e..bdc92a8dc08 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -156,7 +156,7 @@ - (DocumentMap)documentsMatchingDocumentQuery:(const ResourcePath &)docPath { } - (DocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { - DocumentMap results = _remoteDocumentCache->GetMatchingDocuments(query); + DocumentMap results = _remoteDocumentCache->GetMatching(query); // Get locally persisted mutation batches. NSArray *matchingBatches = [self.mutationQueue allMutationBatchesAffectingQuery:query]; diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index 95397907b23..684ce15543b 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -288,7 +288,7 @@ - (MaybeDocumentMap)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { if (!existingDoc || doc.version == SnapshotVersion::None() || (authoritativeUpdates.contains(doc.key) && !existingDoc.hasPendingWrites) || doc.version >= existingDoc.version) { - _remoteDocumentCache->AddEntry(doc); + _remoteDocumentCache->Add(doc); changedDocs = changedDocs.insert(key, doc); } else { LOG_DEBUG( @@ -465,7 +465,7 @@ - (void)applyBatchResult:(FSTMutationBatchResult *)batchResult { HARD_ASSERT(!remoteDoc, "Mutation batch %s applied to document %s resulted in nil.", batch, remoteDoc); } else { - _remoteDocumentCache->AddEntry(doc); + _remoteDocumentCache->Add(doc); } } } diff --git a/Firestore/Source/Local/FSTMemoryPersistence.mm b/Firestore/Source/Local/FSTMemoryPersistence.mm index c34eeef2bef..d246f2a9c60 100644 --- a/Firestore/Source/Local/FSTMemoryPersistence.mm +++ b/Firestore/Source/Local/FSTMemoryPersistence.mm @@ -395,7 +395,7 @@ - (BOOL)mutationQueuesContainKey:(const DocumentKey &)key { - (void)commitTransaction { for (const auto &key : *_orphaned) { if (![self isReferenced:key]) { - _persistence.remoteDocumentCache->RemoveEntry(key); + _persistence.remoteDocumentCache->Remove(key); } } _orphaned.reset(); 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 ef8a83f429c..536d74c1d33 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 @@ -46,12 +46,12 @@ class LevelDbRemoteDocumentCache : public RemoteDocumentCache { public: LevelDbRemoteDocumentCache(FSTLevelDB* db, FSTLocalSerializer* serializer); - void AddEntry(FSTMaybeDocument* document) override; - void RemoveEntry(const model::DocumentKey& key) override; + void Add(FSTMaybeDocument* document) override; + void Remove(const model::DocumentKey& key) override; FSTMaybeDocument* _Nullable Get(const model::DocumentKey& key) override; model::MaybeDocumentMap GetAll(const model::DocumentKeySet& keys) override; - model::DocumentMap GetMatchingDocuments(FSTQuery* query) override; + model::DocumentMap GetMatching(FSTQuery* query) override; private: FSTMaybeDocument* DecodeMaybeDocument(absl::string_view encoded, diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm index 341ce12fb09..1a8bbeaecc6 100644 --- a/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm +++ b/Firestore/core/src/firebase/firestore/local/leveldb_remote_document_cache.mm @@ -43,13 +43,13 @@ : db_(db), serializer_(serializer) { } -void LevelDbRemoteDocumentCache::AddEntry(FSTMaybeDocument* document) { +void LevelDbRemoteDocumentCache::Add(FSTMaybeDocument* document) { std::string ldb_key = LevelDbRemoteDocumentKey::Key(document.key); db_.currentTransaction->Put(ldb_key, [serializer_ encodedMaybeDocument:document]); } -void LevelDbRemoteDocumentCache::RemoveEntry(const DocumentKey& key) { +void LevelDbRemoteDocumentCache::Remove(const DocumentKey& key) { std::string ldb_key = LevelDbRemoteDocumentKey::Key(key); db_.currentTransaction->Delete(ldb_key); } @@ -89,7 +89,7 @@ return results; } -DocumentMap LevelDbRemoteDocumentCache::GetMatchingDocuments(FSTQuery* query) { +DocumentMap LevelDbRemoteDocumentCache::GetMatching(FSTQuery* query) { DocumentMap results; // Documents are ordered by key, so we can use a prefix scan to narrow down 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 ed9175bbd93..d1eebd08e99 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 @@ -42,12 +42,12 @@ namespace local { class MemoryRemoteDocumentCache : public RemoteDocumentCache { public: - void AddEntry(FSTMaybeDocument *document) override; - void RemoveEntry(const model::DocumentKey &key) override; + void Add(FSTMaybeDocument *document) override; + void Remove(const model::DocumentKey &key) override; FSTMaybeDocument *_Nullable Get(const model::DocumentKey &key) override; model::MaybeDocumentMap GetAll(const model::DocumentKeySet &keys) override; - model::DocumentMap GetMatchingDocuments(FSTQuery *query) override; + model::DocumentMap GetMatching(FSTQuery *query) override; std::vector RemoveOrphanedDocuments( FSTMemoryLRUReferenceDelegate *reference_delegate, diff --git a/Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.mm b/Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.mm index b7535309e83..2514eaaa16d 100644 --- a/Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.mm +++ b/Firestore/core/src/firebase/firestore/local/memory_remote_document_cache.mm @@ -36,32 +36,32 @@ * document key in memory. This is only an estimate and includes the size * of the segments of the path, but not any object overhead or path separators. */ -size_t DocumentKeyByteSize(const DocumentKey &key) { +size_t DocumentKeyByteSize(const DocumentKey& key) { size_t count = 0; - for (const auto &segment : key.path()) { + for (const auto& segment : key.path()) { count += segment.size(); } return count; } } // namespace -void MemoryRemoteDocumentCache::AddEntry(FSTMaybeDocument *document) { +void MemoryRemoteDocumentCache::Add(FSTMaybeDocument* document) { docs_ = docs_.insert(document.key, document); } -void MemoryRemoteDocumentCache::RemoveEntry(const DocumentKey &key) { +void MemoryRemoteDocumentCache::Remove(const DocumentKey& key) { docs_ = docs_.erase(key); } -FSTMaybeDocument *_Nullable MemoryRemoteDocumentCache::Get( - const DocumentKey &key) { +FSTMaybeDocument* _Nullable MemoryRemoteDocumentCache::Get( + const DocumentKey& key) { auto found = docs_.find(key); return found != docs_.end() ? found->second : nil; } -MaybeDocumentMap MemoryRemoteDocumentCache::GetAll(const DocumentKeySet &keys) { +MaybeDocumentMap MemoryRemoteDocumentCache::GetAll(const DocumentKeySet& keys) { MaybeDocumentMap results; - for (const DocumentKey &key : keys) { + for (const DocumentKey& key : keys) { // Make sure each key has a corresponding entry, which is null in case the // document is not found. // TODO(http://b/32275378): Don't conflate missing / deleted. @@ -70,22 +70,22 @@ size_t DocumentKeyByteSize(const DocumentKey &key) { return results; } -DocumentMap MemoryRemoteDocumentCache::GetMatchingDocuments(FSTQuery *query) { +DocumentMap MemoryRemoteDocumentCache::GetMatching(FSTQuery* query) { DocumentMap results; // Documents are ordered by key, so we can use a prefix scan to narrow down // the documents we need to match the query against. DocumentKey prefix{query.path.Append("")}; for (auto it = docs_.lower_bound(prefix); it != docs_.end(); ++it) { - const DocumentKey &key = it->first; + const DocumentKey& key = it->first; if (!query.path.IsPrefixOf(key.path())) { break; } - FSTMaybeDocument *maybeDoc = it->second; + FSTMaybeDocument* maybeDoc = it->second; if (![maybeDoc isKindOfClass:[FSTDocument class]]) { continue; } - FSTDocument *doc = static_cast(maybeDoc); + FSTDocument* doc = static_cast(maybeDoc); if ([query matchesDocument:doc]) { results = results.insert(key, doc); } @@ -94,12 +94,12 @@ size_t DocumentKeyByteSize(const DocumentKey &key) { } std::vector MemoryRemoteDocumentCache::RemoveOrphanedDocuments( - FSTMemoryLRUReferenceDelegate *reference_delegate, + FSTMemoryLRUReferenceDelegate* reference_delegate, ListenSequenceNumber upper_bound) { std::vector removed; MaybeDocumentMap updated_docs = docs_; - for (const auto &kv : docs_) { - const DocumentKey &key = kv.first; + for (const auto& kv : docs_) { + const DocumentKey& key = kv.first; if (![reference_delegate isPinnedAtSequenceNumber:upper_bound document:key]) { updated_docs = updated_docs.erase(key); @@ -111,9 +111,9 @@ size_t DocumentKeyByteSize(const DocumentKey &key) { } size_t MemoryRemoteDocumentCache::CalculateByteSize( - FSTLocalSerializer *serializer) { + FSTLocalSerializer* serializer) { size_t count = 0; - for (const auto &kv : docs_) { + for (const auto& kv : docs_) { count += DocumentKeyByteSize(kv.first); count += [[serializer encodedMaybeDocument:kv.second] serializedSize]; } diff --git a/Firestore/core/src/firebase/firestore/local/remote_document_cache.h b/Firestore/core/src/firebase/firestore/local/remote_document_cache.h index 01e1109bf0f..f1ff54c354f 100644 --- a/Firestore/core/src/firebase/firestore/local/remote_document_cache.h +++ b/Firestore/core/src/firebase/firestore/local/remote_document_cache.h @@ -58,10 +58,10 @@ class RemoteDocumentCache { * * @param document A FSTDocument or FSTDeletedDocument to put in the cache. */ - virtual void AddEntry(FSTMaybeDocument* document) = 0; + virtual void Add(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; + virtual void Remove(const model::DocumentKey& key) = 0; /** * Looks up an entry in the cache. @@ -92,7 +92,7 @@ class RemoteDocumentCache { * @param query The query to match documents against. * @return The set of matching documents. */ - virtual model::DocumentMap GetMatchingDocuments(FSTQuery* query) = 0; + virtual model::DocumentMap GetMatching(FSTQuery* query) = 0; }; } // namespace local