firebase · gsoltis · Dec 17, 2018 · Dec 17, 2018
diff --git a/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTLevelDBRemoteDocumentCacheTests.mm
@@ -14,18 +14,22 @@
  * limitations under the License.
  */
 
+#include <memory>
 #include <string>
 
 #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,24 +45,31 @@ @interface FSTLevelDBRemoteDocumentCacheTests : FSTRemoteDocumentCacheTests
 
 @implementation FSTLevelDBRemoteDocumentCacheTests {
   FSTLevelDB *_db;
+  std::unique_ptr<LevelDbRemoteDocumentCache> _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<LevelDbRemoteDocumentCache>(_db, _db.serializer);
 
   // Write a couple dummy rows that should appear before/after the remote_documents table to make
   // sure the tests are unaffected.
   [self writeDummyRowWithSegments:@[ @"remote_documentr", @"foo", @"bar" ]];
   [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
@@ -14,13 +14,17 @@
  * limitations under the License.
  */
 
-#import "Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h"
+#include <memory>
 
 #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<MemoryRemoteDocumentCache> _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>();
+}
+
+- (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
@@ -18,6 +18,8 @@
 
 #import <XCTest/XCTest.h>
 
+#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<FSTRemoteDocumentCache> remoteDocumentCache;
+@property(nonatomic, nullable) firebase::firestore::local::RemoteDocumentCache* remoteDocumentCache;
 @property(nonatomic, strong, nullable) id<FSTPersistence> persistence;
 @end
 

diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm
@@ -16,13 +16,17 @@
 
 #import "Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.h"
 
+#include <memory>
+
 #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"
 
+#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,15 +67,15 @@ - (void)testReadDocumentNotInCache {
   if (!self.remoteDocumentCache) return;
 
   self.persistence.run("testReadDocumentNotInCache", [&]() {
-    XCTAssertNil([self.remoteDocumentCache entryForKey:testutil::Key(kDocPath)]);
+    XCTAssertNil(self.remoteDocumentCache->Get(testutil::Key(kDocPath)));
   });
 }
 
 // Helper for next two tests.
 - (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<FSTDocument *> *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<FSTDocument *> *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
@@ -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
@@ -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
@@ -23,6 +23,7 @@
 
 #include <vector>
 
+#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
@@ -23,6 +23,7 @@
 
 #include <vector>
 
+#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<model::DocumentKey> RemoveOrphanedDocuments(
       FSTMemoryLRUReferenceDelegate *reference_delegate,
-Original file line number
+Diff line change
@@ Expand Up / @@ -83,7 +83,6 @@ @interface FSTLevelDB () @@
     - (size_t)byteSize;
     @property(nonatomic, assign, getter=isStarted) BOOL started;
-    @property(nonatomic, strong, readonly) FSTLocalSerializer *serializer;
     @end
@@ Expand Down @@