diff --git a/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm b/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm index 9a45e8c388c..10384de4022 100644 --- a/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm +++ b/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm @@ -42,16 +42,6 @@ NS_ASSUME_NONNULL_BEGIN -@interface FIRDocumentChange () - -// Expose initializer for testing. -- (instancetype)initWithType:(FIRDocumentChangeType)type - document:(FIRQueryDocumentSnapshot *)document - oldIndex:(NSUInteger)oldIndex - newIndex:(NSUInteger)newIndex; - -@end - @interface FIRQuerySnapshotTests : XCTestCase @end @@ -91,56 +81,40 @@ - (void)testIncludeMetadataChanges { DocumentSet oldDocuments = FSTTestDocSet(FSTDocumentComparatorByKey, @[ doc1Old, doc2Old ]); DocumentSet newDocuments = FSTTestDocSet(FSTDocumentComparatorByKey, @[ doc2New, doc2New ]); std::vector documentChanges{ - DocumentViewChange{doc1New, DocumentViewChange::Type::kMetadata}, - DocumentViewChange{doc2New, DocumentViewChange::Type::kModified}, + DocumentViewChange(doc1New, DocumentViewChange::Type::kMetadata), + DocumentViewChange(doc2New, DocumentViewChange::Type::kModified), }; Firestore *firestore = FSTTestFirestore().wrapped; FSTQuery *query = FSTTestQuery("foo"); - ViewSnapshot viewSnapshot{query, - newDocuments, - oldDocuments, - std::move(documentChanges), - /*mutated_keys=*/DocumentKeySet{}, + ViewSnapshot viewSnapshot(query, newDocuments, oldDocuments, std::move(documentChanges), + /*mutated_keys=*/DocumentKeySet(), /*from_cache=*/false, /*sync_state_changed=*/true, - /*excludes_metadata_changes=*/false}; + /*excludes_metadata_changes=*/false); SnapshotMetadata metadata(/*pending_writes=*/false, /*from_cache=*/false); FIRQuerySnapshot *snapshot = [[FIRQuerySnapshot alloc] initWithFirestore:firestore originalQuery:query snapshot:std::move(viewSnapshot) metadata:std::move(metadata)]; - FIRQueryDocumentSnapshot *doc1Snap = - [[FIRQueryDocumentSnapshot alloc] initWithFirestore:firestore - documentKey:doc1New.key - document:doc1New - fromCache:false - hasPendingWrites:false]; - FIRQueryDocumentSnapshot *doc2Snap = - [[FIRQueryDocumentSnapshot alloc] initWithFirestore:firestore - documentKey:doc2New.key - document:doc2New - fromCache:false - hasPendingWrites:false]; + DocumentSnapshot doc1Snap(firestore, doc1New.key, doc1New, SnapshotMetadata()); + DocumentSnapshot doc2Snap(firestore, doc2New.key, doc2New, SnapshotMetadata()); NSArray *changesWithoutMetadata = @[ - [[FIRDocumentChange alloc] initWithType:FIRDocumentChangeTypeModified - document:doc2Snap - oldIndex:1 - newIndex:1], + [[FIRDocumentChange alloc] + initWithDocumentChange:DocumentChange(DocumentChange::Type::Modified, doc2Snap, + /*old_index=*/1, /*new_index=*/1)], ]; XCTAssertEqualObjects(snapshot.documentChanges, changesWithoutMetadata); NSArray *changesWithMetadata = @[ - [[FIRDocumentChange alloc] initWithType:FIRDocumentChangeTypeModified - document:doc1Snap - oldIndex:0 - newIndex:0], - [[FIRDocumentChange alloc] initWithType:FIRDocumentChangeTypeModified - document:doc2Snap - oldIndex:1 - newIndex:1], + [[FIRDocumentChange alloc] + initWithDocumentChange:DocumentChange(DocumentChange::Type::Modified, doc1Snap, + /*old_index=*/0, /*new_index=*/0)], + [[FIRDocumentChange alloc] + initWithDocumentChange:DocumentChange(DocumentChange::Type::Modified, doc2Snap, + /*old_index=*/1, /*new_index=*/1)], ]; XCTAssertEqualObjects([snapshot documentChangesWithIncludeMetadataChanges:YES], changesWithMetadata); diff --git a/Firestore/Source/API/FIRDocumentChange+Internal.h b/Firestore/Source/API/FIRDocumentChange+Internal.h index c1c26904db8..6c79f543ec1 100644 --- a/Firestore/Source/API/FIRDocumentChange+Internal.h +++ b/Firestore/Source/API/FIRDocumentChange+Internal.h @@ -16,21 +16,17 @@ #import "FIRDocumentChange.h" -#include "Firestore/core/src/firebase/firestore/api/firestore.h" -#include "Firestore/core/src/firebase/firestore/core/view_snapshot.h" +#import -@class FIRFirestore; +#include "Firestore/core/src/firebase/firestore/api/document_change.h" + +using firebase::firestore::api::DocumentChange; NS_ASSUME_NONNULL_BEGIN -/** Internal FIRDocumentChange API we don't want exposed in our public header files. */ -@interface FIRDocumentChange (Internal) +@interface FIRDocumentChange (/* Init */) -/** Calculates the array of FIRDocumentChange's based on the given FSTViewSnapshot. */ -+ (NSArray *) - documentChangesForSnapshot:(const firebase::firestore::core::ViewSnapshot &)snapshot - includeMetadataChanges:(bool)includeMetadataChanges - firestore:(firebase::firestore::api::Firestore *)firestore; +- (instancetype)initWithDocumentChange:(DocumentChange &&)documentChange NS_DESIGNATED_INITIALIZER; @end diff --git a/Firestore/Source/API/FIRDocumentChange.mm b/Firestore/Source/API/FIRDocumentChange.mm index 1a5bccd20bb..7a561c44dd3 100644 --- a/Firestore/Source/API/FIRDocumentChange.mm +++ b/Firestore/Source/API/FIRDocumentChange.mm @@ -14,128 +14,24 @@ * limitations under the License. */ -#import "FIRDocumentChange.h" +#import "Firestore/Source/API/FIRDocumentChange+Internal.h" #import "Firestore/Source/API/FIRDocumentSnapshot+Internal.h" -#import "Firestore/Source/API/FIRFirestore+Internal.h" -#import "Firestore/Source/Core/FSTQuery.h" -#import "Firestore/Source/Model/FSTDocument.h" -#include "Firestore/core/src/firebase/firestore/core/view_snapshot.h" -#include "Firestore/core/src/firebase/firestore/model/document_set.h" +#include "Firestore/core/src/firebase/firestore/api/document_change.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" -using firebase::firestore::api::Firestore; -using firebase::firestore::core::DocumentViewChange; -using firebase::firestore::core::ViewSnapshot; -using firebase::firestore::model::DocumentSet; +using firebase::firestore::api::DocumentChange; NS_ASSUME_NONNULL_BEGIN -@interface FIRDocumentChange () - -- (instancetype)initWithType:(FIRDocumentChangeType)type - document:(FIRDocumentSnapshot *)document - oldIndex:(NSUInteger)oldIndex - newIndex:(NSUInteger)newIndex NS_DESIGNATED_INITIALIZER; - -@end - -@implementation FIRDocumentChange (Internal) - -+ (FIRDocumentChangeType)documentChangeTypeForChange:(const DocumentViewChange &)change { - switch (change.type()) { - case DocumentViewChange::Type::kAdded: - return FIRDocumentChangeTypeAdded; - case DocumentViewChange::Type::kModified: - case DocumentViewChange::Type::kMetadata: - return FIRDocumentChangeTypeModified; - case DocumentViewChange::Type::kRemoved: - return FIRDocumentChangeTypeRemoved; - } - - HARD_FAIL("Unknown DocumentViewChange::Type: %s", change.type()); +@implementation FIRDocumentChange { + DocumentChange _documentChange; } -+ (NSArray *)documentChangesForSnapshot:(const ViewSnapshot &)snapshot - includeMetadataChanges:(bool)includeMetadataChanges - firestore:(Firestore *)firestore { - if (snapshot.old_documents().empty()) { - // Special case the first snapshot because index calculation is easy and fast. Also all changes - // on the first snapshot are adds so there are also no metadata-only changes to filter out. - FSTDocument *_Nullable lastDocument = nil; - NSUInteger index = 0; - NSMutableArray *changes = [NSMutableArray array]; - for (const DocumentViewChange &change : snapshot.document_changes()) { - FIRQueryDocumentSnapshot *document = [[FIRQueryDocumentSnapshot alloc] - initWithFirestore:firestore - documentKey:change.document().key - document:change.document() - fromCache:snapshot.from_cache() - hasPendingWrites:snapshot.mutated_keys().contains(change.document().key)]; - HARD_ASSERT(change.type() == DocumentViewChange::Type::kAdded, - "Invalid event type for first snapshot"); - HARD_ASSERT(!lastDocument || snapshot.query().comparator(lastDocument, change.document()) == - NSOrderedAscending, - "Got added events in wrong order"); - [changes addObject:[[FIRDocumentChange alloc] initWithType:FIRDocumentChangeTypeAdded - document:document - oldIndex:NSNotFound - newIndex:index++]]; - } - return changes; - } else { - // A DocumentSet that is updated incrementally as changes are applied to use to lookup the index - // of a document. - DocumentSet indexTracker = snapshot.old_documents(); - NSMutableArray *changes = [NSMutableArray array]; - for (const DocumentViewChange &change : snapshot.document_changes()) { - if (!includeMetadataChanges && change.type() == DocumentViewChange::Type::kMetadata) { - continue; - } - - FIRQueryDocumentSnapshot *document = [[FIRQueryDocumentSnapshot alloc] - initWithFirestore:firestore - documentKey:change.document().key - document:change.document() - fromCache:snapshot.from_cache() - hasPendingWrites:snapshot.mutated_keys().contains(change.document().key)]; - - size_t oldIndex = DocumentSet::npos; - size_t newIndex = DocumentSet::npos; - if (change.type() != DocumentViewChange::Type::kAdded) { - oldIndex = indexTracker.IndexOf(change.document().key); - HARD_ASSERT(oldIndex != DocumentSet::npos, "Index for document not found"); - indexTracker = indexTracker.erase(change.document().key); - } - if (change.type() != DocumentViewChange::Type::kRemoved) { - indexTracker = indexTracker.insert(change.document()); - newIndex = indexTracker.IndexOf(change.document().key); - } - [FIRDocumentChange documentChangeTypeForChange:change]; - FIRDocumentChangeType type = [FIRDocumentChange documentChangeTypeForChange:change]; - [changes addObject:[[FIRDocumentChange alloc] initWithType:type - document:document - oldIndex:oldIndex - newIndex:newIndex]]; - } - return changes; - } -} - -@end - -@implementation FIRDocumentChange - -- (instancetype)initWithType:(FIRDocumentChangeType)type - document:(FIRQueryDocumentSnapshot *)document - oldIndex:(NSUInteger)oldIndex - newIndex:(NSUInteger)newIndex { +- (instancetype)initWithDocumentChange:(DocumentChange &&)documentChange { if (self = [super init]) { - _type = type; - _document = document; - _oldIndex = oldIndex; - _newIndex = newIndex; + _documentChange = std::move(documentChange); } return self; } @@ -145,16 +41,36 @@ - (BOOL)isEqual:(nullable id)other { if (![other isKindOfClass:[FIRDocumentChange class]]) return NO; FIRDocumentChange *change = (FIRDocumentChange *)other; - return self.type == change.type && [self.document isEqual:change.document] && - self.oldIndex == change.oldIndex && self.newIndex == change.newIndex; + return _documentChange == change->_documentChange; } - (NSUInteger)hash { - NSUInteger result = (NSUInteger)self.type; - result = result * 31u + [self.document hash]; - result = result * 31u + (NSUInteger)self.oldIndex; - result = result * 31u + (NSUInteger)self.newIndex; - return result; + return _documentChange.Hash(); +} + +- (FIRDocumentChangeType)type { + switch (_documentChange.type()) { + case DocumentChange::Type::Added: + return FIRDocumentChangeTypeAdded; + case DocumentChange::Type::Modified: + return FIRDocumentChangeTypeModified; + case DocumentChange::Type::Removed: + return FIRDocumentChangeTypeRemoved; + } + + HARD_FAIL("Unknown DocumentChange::Type: %s", _documentChange.type()); +} + +- (FIRQueryDocumentSnapshot *)document { + return [[FIRQueryDocumentSnapshot alloc] initWithSnapshot:_documentChange.document()]; +} + +- (NSUInteger)oldIndex { + return _documentChange.old_index(); +} + +- (NSUInteger)newIndex { + return _documentChange.new_index(); } @end diff --git a/Firestore/Source/API/FIRQuerySnapshot.mm b/Firestore/Source/API/FIRQuerySnapshot.mm index 05a9ad0c29f..7ff14860f3c 100644 --- a/Firestore/Source/API/FIRQuerySnapshot.mm +++ b/Firestore/Source/API/FIRQuerySnapshot.mm @@ -123,15 +123,15 @@ - (NSInteger)count { - (NSArray *)documentChangesWithIncludeMetadataChanges: (BOOL)includeMetadataChanges { - if (includeMetadataChanges && _snapshot->view_snapshot().excludes_metadata_changes()) { - ThrowInvalidArgument("To include metadata changes with your document changes, you must call " - "addSnapshotListener(includeMetadataChanges: true)."); - } - if (!_documentChanges || _documentChangesIncludeMetadataChanges != includeMetadataChanges) { - _documentChanges = [FIRDocumentChange documentChangesForSnapshot:_snapshot->view_snapshot() - includeMetadataChanges:includeMetadataChanges - firestore:_snapshot->firestore()]; + NSMutableArray *documentChanges = [NSMutableArray array]; + _snapshot->ForEachChange( + static_cast(includeMetadataChanges), [&documentChanges](DocumentChange change) { + [documentChanges + addObject:[[FIRDocumentChange alloc] initWithDocumentChange:std::move(change)]]; + }); + + _documentChanges = documentChanges; _documentChangesIncludeMetadataChanges = includeMetadataChanges; } return _documentChanges; diff --git a/Firestore/core/src/firebase/firestore/api/document_change.h b/Firestore/core/src/firebase/firestore/api/document_change.h new file mode 100644 index 00000000000..abc1d0d54a1 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/api/document_change.h @@ -0,0 +1,81 @@ +/* + * 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. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_API_DOCUMENT_CHANGE_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_API_DOCUMENT_CHANGE_H_ + +#include + +#include "Firestore/core/src/firebase/firestore/api/document_snapshot.h" + +namespace firebase { +namespace firestore { +namespace api { + +class DocumentChange { + public: + enum class Type { Added, Modified, Removed }; + + DocumentChange() = default; + DocumentChange(Type type, + DocumentSnapshot document, + size_t old_index, + size_t new_index) + : type_(type), + document_(std::move(document)), + old_index_(old_index), + new_index_(new_index) { + } + + size_t Hash() const; + + Type type() const { + return type_; + } + + DocumentSnapshot document() const { + return document_; + } + + size_t old_index() const { + return old_index_; + } + + size_t new_index() const { + return new_index_; + } + + /** + * A sentinel return value for old_index() and new_index() indicating that + * there's no relevant index to return because the document was newly added + * or removed respectively. + */ + static constexpr size_t npos = static_cast(-1); + + private: + Type type_; + DocumentSnapshot document_; + size_t old_index_; + size_t new_index_; +}; + +bool operator==(const DocumentChange& lhs, const DocumentChange& rhs); + +} // namespace api +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_API_DOCUMENT_CHANGE_H_ diff --git a/Firestore/core/src/firebase/firestore/api/document_change.mm b/Firestore/core/src/firebase/firestore/api/document_change.mm new file mode 100644 index 00000000000..e6c2c485041 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/api/document_change.mm @@ -0,0 +1,37 @@ +/* + * 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. + */ + +#include "Firestore/core/src/firebase/firestore/api/document_change.h" + +#include "Firestore/core/src/firebase/firestore/util/hashing.h" + +namespace firebase { +namespace firestore { +namespace api { + +size_t DocumentChange::Hash() const { + return util::Hash(static_cast(type_), document_, old_index_, new_index_); +} + +bool operator==(const DocumentChange& lhs, const DocumentChange& rhs) { + return lhs.type() == rhs.type() && lhs.document() == rhs.document() && + lhs.old_index() == rhs.old_index() && + lhs.new_index() == rhs.new_index(); +} + +} // namespace api +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/api/document_reference.h b/Firestore/core/src/firebase/firestore/api/document_reference.h index d107d452315..f9da6004cf6 100644 --- a/Firestore/core/src/firebase/firestore/api/document_reference.h +++ b/Firestore/core/src/firebase/firestore/api/document_reference.h @@ -27,7 +27,6 @@ #include #include -#import "FIRDocumentReference.h" #import "FIRFirestoreSource.h" #include "Firestore/core/src/firebase/firestore/api/document_snapshot.h" @@ -39,7 +38,6 @@ NS_ASSUME_NONNULL_BEGIN -@class FIRFirestore; @class FSTMutation; namespace firebase { diff --git a/Firestore/core/src/firebase/firestore/api/query_snapshot.h b/Firestore/core/src/firebase/firestore/api/query_snapshot.h index 7c5f49dde79..7495636c45c 100644 --- a/Firestore/core/src/firebase/firestore/api/query_snapshot.h +++ b/Firestore/core/src/firebase/firestore/api/query_snapshot.h @@ -26,6 +26,7 @@ #include #include +#include "Firestore/core/src/firebase/firestore/api/document_change.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/core/view_snapshot.h" @@ -76,10 +77,6 @@ class QuerySnapshot { return internal_query_; } - const core::ViewSnapshot& view_snapshot() const { - return snapshot_; - } - /** * Metadata about this snapshot, concerning its source and if it has local * modifications. @@ -92,6 +89,13 @@ class QuerySnapshot { void ForEachDocument( const std::function& callback) const; + /** + * Iterates over the `DocumentChanges` representing the changes between + * the prior snapshot and this one. + */ + void ForEachChange(bool include_metadata_changes, + const std::function& callback) const; + friend bool operator==(const QuerySnapshot& lhs, const QuerySnapshot& rhs); private: diff --git a/Firestore/core/src/firebase/firestore/api/query_snapshot.mm b/Firestore/core/src/firebase/firestore/api/query_snapshot.mm index c0269809a59..f14cbb148c0 100644 --- a/Firestore/core/src/firebase/firestore/api/query_snapshot.mm +++ b/Firestore/core/src/firebase/firestore/api/query_snapshot.mm @@ -25,9 +25,12 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Model/FSTDocument.h" +#include "Firestore/core/src/firebase/firestore/api/input_validation.h" #include "Firestore/core/src/firebase/firestore/core/view_snapshot.h" #include "Firestore/core/src/firebase/firestore/model/document_set.h" +#include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "Firestore/core/src/firebase/firestore/util/objc_compatibility.h" +#include "absl/types/optional.h" NS_ASSUME_NONNULL_BEGIN @@ -37,6 +40,7 @@ namespace objc = util::objc; using api::Firestore; +using core::DocumentViewChange; using core::ViewSnapshot; using model::DocumentSet; @@ -63,6 +67,89 @@ DocumentSnapshot snap(firestore_, document.key, document, from_cache, } } +static DocumentChange::Type DocumentChangeTypeForChange( + const DocumentViewChange& change) { + switch (change.type()) { + case DocumentViewChange::Type::kAdded: + return DocumentChange::Type::Added; + case DocumentViewChange::Type::kModified: + case DocumentViewChange::Type::kMetadata: + return DocumentChange::Type::Modified; + case DocumentViewChange::Type::kRemoved: + return DocumentChange::Type::Removed; + } + + HARD_FAIL("Unknown DocumentViewChange::Type: %s", change.type()); +} + +void QuerySnapshot::ForEachChange( + bool include_metadata_changes, + const std::function& callback) const { + if (include_metadata_changes && snapshot_.excludes_metadata_changes()) { + ThrowInvalidArgument("To include metadata changes with your document " + "changes, you must call " + "addSnapshotListener(includeMetadataChanges:true)."); + } + + if (snapshot_.old_documents().empty()) { + // Special case the first snapshot because index calculation is easy and + // fast. Also all changes on the first snapshot are adds so there are also + // no metadata-only changes to filter out. + FSTDocument* last_document = nil; + size_t index = 0; + for (const DocumentViewChange& change : snapshot_.document_changes()) { + FSTDocument* doc = change.document(); + SnapshotMetadata metadata( + /*pending_writes=*/snapshot_.mutated_keys().contains(doc.key), + /*from_cache=*/snapshot_.from_cache()); + DocumentSnapshot document(firestore_, doc.key, doc, metadata); + + HARD_ASSERT(change.type() == DocumentViewChange::Type::kAdded, + "Invalid event type for first snapshot"); + HARD_ASSERT(!last_document || snapshot_.query().comparator( + last_document, change.document()) == + NSOrderedAscending, + "Got added events in wrong order"); + + callback(DocumentChange(DocumentChange::Type::Added, std::move(document), + DocumentChange::npos, index++)); + } + + } else { + // A DocumentSet that is updated incrementally as changes are applied to use + // to lookup the index of a document. + DocumentSet index_tracker = snapshot_.old_documents(); + for (const DocumentViewChange& change : snapshot_.document_changes()) { + if (!include_metadata_changes && + change.type() == DocumentViewChange::Type::kMetadata) { + continue; + } + + FSTDocument* doc = change.document(); + SnapshotMetadata metadata( + /*pending_writes=*/snapshot_.mutated_keys().contains(doc.key), + /*from_cache=*/snapshot_.from_cache()); + DocumentSnapshot document(firestore_, doc.key, doc, metadata); + + size_t old_index = DocumentChange::npos; + size_t new_index = DocumentChange::npos; + if (change.type() != DocumentViewChange::Type::kAdded) { + old_index = index_tracker.IndexOf(change.document().key); + HARD_ASSERT(old_index != DocumentSet::npos, + "Index for document not found"); + index_tracker = index_tracker.erase(change.document().key); + } + if (change.type() != DocumentViewChange::Type::kRemoved) { + index_tracker = index_tracker.insert(change.document()); + new_index = index_tracker.IndexOf(change.document().key); + } + + DocumentChange::Type type = DocumentChangeTypeForChange(change); + callback(DocumentChange(type, std::move(document), old_index, new_index)); + } + } +} + } // namespace api } // namespace firestore } // namespace firebase