Port FIRSnapshotMetadata to C++ (#2580)

wilhuff · web-flow · commit fd8947ce7440 · 2019-03-19T12:55:24.000-07:00
diff --git a/.travis.yml b/.travis.yml
@@ -83,7 +83,7 @@ jobs:
       script:
         # Eliminate the one warning from BoringSSL when CocoaPods 1.6.0 is available.
         # The travis_wait is necessary because the command takes more than 10 minutes.
-        - travis_wait 25 ./scripts/if_changed.sh ./scripts/pod_lib_lint.sh FirebaseFirestore.podspec --platforms=ios --allow-warnings --no-subspecs
+        - travis_wait 30 ./scripts/if_changed.sh ./scripts/pod_lib_lint.sh FirebaseFirestore.podspec --platforms=ios --allow-warnings --no-subspecs
 
     # pod lib lint to check build and warnings for static library build - only on cron jobs
     - stage: test
diff --git a/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm b/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm
@@ -104,8 +104,8 @@ - (void)testIncludeMetadataChanges {
                             /*from_cache=*/false,
                             /*sync_state_changed=*/true,
                             /*excludes_metadata_changes=*/false};
-  FIRSnapshotMetadata *metadata = [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:NO
-                                                                               fromCache:NO];
+  FIRSnapshotMetadata *metadata = [[FIRSnapshotMetadata alloc] initWithPendingWrites:NO
+                                                                           fromCache:NO];
   FIRQuerySnapshot *snapshot = [FIRQuerySnapshot snapshotWithFirestore:firestore
                                                          originalQuery:query
                                                               snapshot:std::move(viewSnapshot)
diff --git a/Firestore/Example/Tests/API/FIRSnapshotMetadataTests.mm b/Firestore/Example/Tests/API/FIRSnapshotMetadataTests.mm
@@ -28,14 +28,11 @@ @interface FIRSnapshotMetadataTests : XCTestCase
 @implementation FIRSnapshotMetadataTests
 
 - (void)testEquals {
-  FIRSnapshotMetadata *foo = [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:YES
-                                                                          fromCache:YES];
-  FIRSnapshotMetadata *fooDup = [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:YES
-                                                                             fromCache:YES];
-  FIRSnapshotMetadata *bar = [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:YES
-                                                                          fromCache:NO];
-  FIRSnapshotMetadata *baz = [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:NO
-                                                                          fromCache:YES];
+  FIRSnapshotMetadata *foo = [[FIRSnapshotMetadata alloc] initWithPendingWrites:YES fromCache:YES];
+  FIRSnapshotMetadata *fooDup = [[FIRSnapshotMetadata alloc] initWithPendingWrites:YES
+                                                                         fromCache:YES];
+  FIRSnapshotMetadata *bar = [[FIRSnapshotMetadata alloc] initWithPendingWrites:YES fromCache:NO];
+  FIRSnapshotMetadata *baz = [[FIRSnapshotMetadata alloc] initWithPendingWrites:NO fromCache:YES];
   XCTAssertEqualObjects(foo, fooDup);
   XCTAssertNotEqualObjects(foo, bar);
   XCTAssertNotEqualObjects(foo, baz);
@@ -47,6 +44,16 @@ - (void)testEquals {
   XCTAssertNotEqual([bar hash], [baz hash]);
 }
 
+- (void)testProperties {
+  FIRSnapshotMetadata *metadata = [[FIRSnapshotMetadata alloc] initWithPendingWrites:YES
+                                                                           fromCache:NO];
+  XCTAssertTrue(metadata.hasPendingWrites);
+  XCTAssertTrue(metadata.pendingWrites);
+
+  XCTAssertFalse(metadata.isFromCache);
+  XCTAssertFalse(metadata.fromCache);
+}
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/Firestore/Example/Tests/API/FSTAPIHelpers.mm b/Firestore/Example/Tests/API/FSTAPIHelpers.mm
@@ -98,7 +98,7 @@
     bool hasPendingWrites,
     bool fromCache) {
   FIRSnapshotMetadata *metadata =
-      [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:hasPendingWrites fromCache:fromCache];
+      [[FIRSnapshotMetadata alloc] initWithPendingWrites:hasPendingWrites fromCache:fromCache];
   FSTDocumentSet *oldDocuments = FSTTestDocSet(FSTDocumentComparatorByKey, @[]);
   DocumentKeySet mutatedKeys;
   for (NSString *key in oldDocs) {
diff --git a/Firestore/Source/API/FIRDocumentSnapshot+Internal.h b/Firestore/Source/API/FIRDocumentSnapshot+Internal.h
@@ -17,23 +17,33 @@
 #import "FIRDocumentSnapshot.h"
 
 #include "Firestore/core/src/firebase/firestore/api/document_snapshot.h"
+#include "Firestore/core/src/firebase/firestore/api/snapshot_metadata.h"
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
 
 @class FIRFirestore;
 @class FSTDocument;
 
+using firebase::firestore::api::DocumentSnapshot;
+using firebase::firestore::api::Firestore;
+using firebase::firestore::api::SnapshotMetadata;
+using firebase::firestore::model::DocumentKey;
+
 NS_ASSUME_NONNULL_BEGIN
 
 @interface FIRDocumentSnapshot (/* Init */)
 
-- (instancetype)initWithSnapshot:(firebase::firestore::api::DocumentSnapshot &&)snapshot
-    NS_DESIGNATED_INITIALIZER;
+- (instancetype)initWithSnapshot:(DocumentSnapshot &&)snapshot NS_DESIGNATED_INITIALIZER;
+
+- (instancetype)initWithFirestore:(Firestore *)firestore
+                      documentKey:(DocumentKey)documentKey
+                         document:(nullable FSTDocument *)document
+                         metadata:(SnapshotMetadata)metadata;
 
-- (instancetype)initWithFirestore:(firebase::firestore::api::Firestore *)firestore
-                      documentKey:(firebase::firestore::model::DocumentKey)documentKey
+- (instancetype)initWithFirestore:(Firestore *)firestore
+                      documentKey:(DocumentKey)documentKey
                          document:(nullable FSTDocument *)document
                         fromCache:(bool)fromCache
-                 hasPendingWrites:(bool)pendingWrites;
+                 hasPendingWrites:(bool)hasPendingWrites;
 
 @end
 
diff --git a/Firestore/Source/API/FIRDocumentSnapshot.mm b/Firestore/Source/API/FIRDocumentSnapshot.mm
@@ -68,6 +68,8 @@ ServerTimestampBehavior InternalServerTimestampBehavior(FIRServerTimestampBehavi
 
 @implementation FIRDocumentSnapshot {
   DocumentSnapshot _snapshot;
+
+  FIRSnapshotMetadata *_cachedMetadata;
 }
 
 - (instancetype)initWithSnapshot:(DocumentSnapshot &&)snapshot {
@@ -80,12 +82,22 @@ - (instancetype)initWithSnapshot:(DocumentSnapshot &&)snapshot {
 - (instancetype)initWithFirestore:(Firestore *)firestore
                       documentKey:(DocumentKey)documentKey
                          document:(nullable FSTDocument *)document
-                        fromCache:(bool)fromCache
-                 hasPendingWrites:(bool)pendingWrites {
-  DocumentSnapshot wrapped{firestore, std::move(documentKey), document, fromCache, pendingWrites};
+                         metadata:(SnapshotMetadata)metadata {
+  DocumentSnapshot wrapped{firestore, std::move(documentKey), document, std::move(metadata)};
   return [self initWithSnapshot:std::move(wrapped)];
 }
 
+- (instancetype)initWithFirestore:(Firestore *)firestore
+                      documentKey:(DocumentKey)documentKey
+                         document:(nullable FSTDocument *)document
+                        fromCache:(bool)fromCache
+                 hasPendingWrites:(bool)hasPendingWrites {
+  return [self initWithFirestore:firestore
+                     documentKey:std::move(documentKey)
+                        document:document
+                        metadata:SnapshotMetadata(hasPendingWrites, fromCache)];
+}
+
 // NSObject Methods
 - (BOOL)isEqual:(nullable id)other {
   if (other == self) return YES;
@@ -120,7 +132,10 @@ - (NSString *)documentID {
 @dynamic metadata;
 
 - (FIRSnapshotMetadata *)metadata {
-  return _snapshot.GetMetadata();
+  if (!_cachedMetadata) {
+    _cachedMetadata = [[FIRSnapshotMetadata alloc] initWithMetadata:_snapshot.metadata()];
+  }
+  return _cachedMetadata;
 }
 
 - (nullable NSDictionary<NSString *, id> *)data {
diff --git a/Firestore/Source/API/FIRQuery.mm b/Firestore/Source/API/FIRQuery.mm
@@ -183,8 +183,8 @@ - (void)getDocumentsWithSource:(FIRFirestoreSource)source
     ViewSnapshot snapshot = maybe_snapshot.ValueOrDie();
 
     FIRSnapshotMetadata *metadata =
-        [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:snapshot.has_pending_writes()
-                                                     fromCache:snapshot.from_cache()];
+        [[FIRSnapshotMetadata alloc] initWithPendingWrites:snapshot.has_pending_writes()
+                                                 fromCache:snapshot.from_cache()];
 
     listener([FIRQuerySnapshot snapshotWithFirestore:firestore
                                        originalQuery:query
diff --git a/Firestore/Source/API/FIRSnapshotMetadata+Internal.h b/Firestore/Source/API/FIRSnapshotMetadata+Internal.h
@@ -18,11 +18,17 @@
 
 #import <Foundation/Foundation.h>
 
+#include "Firestore/core/src/firebase/firestore/api/snapshot_metadata.h"
+
+using firebase::firestore::api::SnapshotMetadata;
+
 NS_ASSUME_NONNULL_BEGIN
 
-@interface FIRSnapshotMetadata (Internal)
+@interface FIRSnapshotMetadata (/* Init */)
+
+- (instancetype)initWithMetadata:(SnapshotMetadata)metadata NS_DESIGNATED_INITIALIZER;
 
-+ (instancetype)snapshotMetadataWithPendingWrites:(BOOL)pendingWrites fromCache:(BOOL)fromCache;
+- (instancetype)initWithPendingWrites:(bool)pendingWrites fromCache:(bool)fromCache;
 
 @end
 
diff --git a/Firestore/Source/API/FIRSnapshotMetadata.mm b/Firestore/Source/API/FIRSnapshotMetadata.mm
@@ -16,53 +16,49 @@
 
 #import "FIRSnapshotMetadata.h"
 
-#import "Firestore/Source/API/FIRSnapshotMetadata+Internal.h"
-
-NS_ASSUME_NONNULL_BEGIN
-
-@interface FIRSnapshotMetadata ()
+#include <utility>
 
-- (instancetype)initWithPendingWrites:(BOOL)pendingWrites fromCache:(BOOL)fromCache;
+#import "Firestore/Source/API/FIRSnapshotMetadata+Internal.h"
 
-@end
+#include "Firestore/core/src/firebase/firestore/api/snapshot_metadata.h"
 
-@implementation FIRSnapshotMetadata (Internal)
+NS_ASSUME_NONNULL_BEGIN
 
-+ (instancetype)snapshotMetadataWithPendingWrites:(BOOL)pendingWrites fromCache:(BOOL)fromCache {
-  return [[FIRSnapshotMetadata alloc] initWithPendingWrites:pendingWrites fromCache:fromCache];
+@implementation FIRSnapshotMetadata {
+  SnapshotMetadata _metadata;
 }
 
-@end
-
-@implementation FIRSnapshotMetadata
-
-- (instancetype)initWithPendingWrites:(BOOL)pendingWrites fromCache:(BOOL)fromCache {
+- (instancetype)initWithMetadata:(SnapshotMetadata)metadata {
   if (self = [super init]) {
-    _pendingWrites = pendingWrites;
-    _fromCache = fromCache;
+    _metadata = std::move(metadata);
   }
   return self;
 }
 
+- (instancetype)initWithPendingWrites:(bool)pendingWrites fromCache:(bool)fromCache {
+  SnapshotMetadata wrapped(pendingWrites, fromCache);
+  return [self initWithMetadata:std::move(wrapped)];
+}
+
 // NSObject Methods
 - (BOOL)isEqual:(nullable id)other {
   if (other == self) return YES;
-  if (![[other class] isEqual:[self class]]) return NO;
+  if (![other isKindOfClass:[FIRSnapshotMetadata class]]) return NO;
 
-  return [self isEqualToMetadata:other];
+  FIRSnapshotMetadata *otherMetadata = other;
+  return _metadata == otherMetadata->_metadata;
 }
 
-- (BOOL)isEqualToMetadata:(nullable FIRSnapshotMetadata *)metadata {
-  if (self == metadata) return YES;
-  if (metadata == nil) return NO;
+- (NSUInteger)hash {
+  return _metadata.Hash();
+}
 
-  return self.pendingWrites == metadata.pendingWrites && self.fromCache == metadata.fromCache;
+- (BOOL)hasPendingWrites {
+  return _metadata.pending_writes();
 }
 
-- (NSUInteger)hash {
-  NSUInteger hash = self.pendingWrites ? 1 : 0;
-  hash = hash * 31u + (self.fromCache ? 1 : 0);
-  return hash;
+- (BOOL)isFromCache {
+  return _metadata.from_cache();
 }
 
 @end
diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm
@@ -372,8 +372,8 @@ - (void)getDocumentsFromLocalCache:(FIRQuery *)query
 
     ViewSnapshot snapshot = std::move(viewChange.snapshot).value();
     FIRSnapshotMetadata *metadata =
-        [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:snapshot.has_pending_writes()
-                                                     fromCache:snapshot.from_cache()];
+        [[FIRSnapshotMetadata alloc] initWithPendingWrites:snapshot.has_pending_writes()
+                                                 fromCache:snapshot.from_cache()];
 
     FIRQuerySnapshot *result = [FIRQuerySnapshot snapshotWithFirestore:query.firestore
                                                          originalQuery:query.query
diff --git a/Firestore/Source/Public/FIRSnapshotMetadata.h b/Firestore/Source/Public/FIRSnapshotMetadata.h
@@ -22,6 +22,7 @@ NS_ASSUME_NONNULL_BEGIN
 NS_SWIFT_NAME(SnapshotMetadata)
 @interface FIRSnapshotMetadata : NSObject
 
+/**   */
 - (instancetype)init NS_UNAVAILABLE;
 
 /**
diff --git a/Firestore/core/CMakeLists.txt b/Firestore/core/CMakeLists.txt
@@ -15,6 +15,7 @@
 add_subdirectory(include/firebase/firestore)
 
 add_subdirectory(src/firebase/firestore)
+add_subdirectory(src/firebase/firestore/api)
 add_subdirectory(src/firebase/firestore/auth)
 add_subdirectory(src/firebase/firestore/core)
 add_subdirectory(src/firebase/firestore/immutable)
diff --git a/Firestore/core/src/firebase/firestore/api/CMakeLists.txt b/Firestore/core/src/firebase/firestore/api/CMakeLists.txt
@@ -0,0 +1,23 @@
+# Copyright 2019 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.
+
+cc_library(
+  firebase_firestore_api
+  SOURCES
+    snapshot_metadata.cc
+    snapshot_metadata.h
+  DEPENDS
+    absl_meta
+    firebase_firestore_util
+)
diff --git a/Firestore/core/src/firebase/firestore/api/document_reference.mm b/Firestore/core/src/firebase/firestore/api/document_reference.mm
@@ -18,8 +18,6 @@
 
 #include <memory>
 
-#import "FIRSnapshotMetadata.h"
-
 #import "Firestore/Source/API/FIRDocumentSnapshot+Internal.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
 #import "Firestore/Source/API/FIRListenerRegistration+Internal.h"
@@ -150,7 +148,7 @@
         dispatch_semaphore_wait(registered, DISPATCH_TIME_FOREVER);
         [*listener_registration remove];
 
-        if (!snapshot.exists() && snapshot.GetMetadata().fromCache) {
+        if (!snapshot.exists() && snapshot.metadata().from_cache()) {
           // TODO(dimond): Reconsider how to raise missing documents when
           // offline. If we're online and the document doesn't exist then we
           // call the completion with a document with document.exists set to
@@ -162,7 +160,7 @@
           completion(
               Status{FirestoreErrorCode::Unavailable,
                      "Failed to get document because the client is offline."});
-        } else if (snapshot.exists() && snapshot.GetMetadata().fromCache &&
+        } else if (snapshot.exists() && snapshot.metadata().from_cache() &&
                    source == FIRFirestoreSourceServer) {
           completion(Status{FirestoreErrorCode::Unavailable,
                             "Failed to get document from server. (However, "
diff --git a/Firestore/core/src/firebase/firestore/api/document_snapshot.h b/Firestore/core/src/firebase/firestore/api/document_snapshot.h
diff --git a/Firestore/core/src/firebase/firestore/api/document_snapshot.mm b/Firestore/core/src/firebase/firestore/api/document_snapshot.mm
diff --git a/Firestore/core/src/firebase/firestore/api/snapshot_metadata.cc b/Firestore/core/src/firebase/firestore/api/snapshot_metadata.cc
diff --git a/Firestore/core/src/firebase/firestore/api/snapshot_metadata.h b/Firestore/core/src/firebase/firestore/api/snapshot_metadata.h
