firebase · gsoltis · Dec 18, 2018 · Dec 13, 2018 · Dec 13, 2018 · Dec 13, 2018
diff --git 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<FSTPersistence> _persistence;
   id<FSTQueryCache> _queryCache;
-  id<FSTRemoteDocumentCache> _documentCache;
+  RemoteDocumentCache *_documentCache;
   id<FSTMutationQueue> _mutationQueue;
   id<FSTLRUDelegate> _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->Add(doc);
   return doc;
 }
 
@@ -461,12 +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",
-                      key.ToString().c_str());
+      XCTAssertNotNil(_documentCache->Get(key), @"Missing document %s", key.ToString().c_str());
     }
   });
   [_persistence shutdown];
@@ -618,7 +618,7 @@ - (void)testRemoveTargetsThenGC {
                                                  key:middleDocToUpdate
                                              version:testutil::Version(version)
                                                state:FSTDocumentStateSynced];
-    [_documentCache addEntry:doc];
+    _documentCache->Add(doc);
     [self updateTargetInTransaction:middleTarget];
   });
 
@@ -643,14 +643,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/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
@@ -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>();
 }
 
-- (MemoryRemoteDocumentCache *)remoteDocumentCache {
+- (RemoteDocumentCache *)remoteDocumentCache {
   return _cache.get();
 }
 

diff --git 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 <XCTest/XCTest.h>
 
 #include "Firestore/core/src/firebase/firestore/local/remote_document_cache.h"

diff --git 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"
 
@@ -127,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);
   });
@@ -139,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);
   });
 }
@@ -149,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)));
   });
@@ -160,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)));
   });
 }
 
@@ -177,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),
@@ -188,11 +187,9 @@ - (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);
+  self.remoteDocumentCache->Add(doc);
   return doc;
 }
 

diff --git 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
@@ -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"
 
@@ -33,8 +32,10 @@
 #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/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"
@@ -60,8 +61,10 @@
 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::local::RemoteDocumentCache;
 using firebase::firestore::model::DatabaseId;
 using firebase::firestore::model::DocumentKey;
 using firebase::firestore::model::ListenSequenceNumber;
@@ -210,7 +213,7 @@ - (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)upperB
     if (sequenceNumber <= upperBound) {
       if (![self isPinned:docKey]) {
         count++;
-        [self->_db.remoteDocumentCache removeEntryForKey:docKey];
+        self->_db.remoteDocumentCache->Remove(docKey);
         [self removeSentinel:docKey];
       }
     }
@@ -266,6 +269,7 @@ @implementation FSTLevelDB {
   Path _directory;
   std::unique_ptr<LevelDbTransaction> _transaction;
   std::unique_ptr<leveldb::DB> _ptr;
+  std::unique_ptr<LevelDbRemoteDocumentCache> _documentCache;
   FSTTransactionRunner _transactionRunner;
   FSTLevelDBLRUDelegate *_referenceDelegate;
   FSTLevelDBQueryCache *_queryCache;
@@ -336,6 +340,7 @@ - (instancetype)initWithLevelDB:(std::unique_ptr<leveldb::DB>)db
     _directory = std::move(directory);
     _serializer = serializer;
     _queryCache = [[FSTLevelDBQueryCache alloc] initWithDB:self serializer:self.serializer];
+    _documentCache = absl::make_unique<LevelDbRemoteDocumentCache>(self, _serializer);
     _referenceDelegate =
         [[FSTLevelDBLRUDelegate alloc] initWithPersistence:self lruParams:lruParams];
     _transactionRunner.SetBackingPersistence(self);
@@ -460,8 +465,8 @@ - (LevelDbTransaction *)currentTransaction {
   return _queryCache;
 }
 
-- (id<FSTRemoteDocumentCache>)remoteDocumentCache {
-  return [[FSTLevelDBRemoteDocumentCache alloc] initWithDB:self serializer:self.serializer];
+- (RemoteDocumentCache *)remoteDocumentCache {
+  return _documentCache.get();
 }
 
 - (void)startTransaction:(absl::string_view)label {

diff --git a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.h b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.h