Browse Source

Do not consider MutableDocument's state for DocumentSnapshot comparison (#8647)

* `operator==` (isEqual) behavior for DocumentSnapshot.

* Do not consider MutableDocument's state for DocumentSnapshot comparison.

* Address comments.

* Update changelog.

* Don't compare the key twice.
Ehsan 4 years ago
parent
commit
d191d0e1dd

+ 2 - 0
Firestore/CHANGELOG.md

@@ -1,5 +1,7 @@
 # v8.6.0
 - [changed] Internal refactor to improve serialization performance.
+- [changed] `DocumentSnapshot` objects consider the document's key and data for
+  equality comparison, but ignore the internal state and internal version.
 
 # v8.4.0
 - [fixed] Fixed handling of Unicode characters in log and assertion messages

+ 30 - 0
Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm

@@ -60,6 +60,36 @@ using firebase::firestore::util::TimerId;
   XCTAssertEqualObjects(result.data, finalData);
 }
 
+- (void)testEqualityComparison {
+  FIRDocumentReference *doc = [self.db documentWithPath:@"rooms/eros"];
+  NSDictionary<NSString *, id> *initialData =
+      @{@"desc" : @"Description", @"owner" : @{@"name" : @"Jonny", @"email" : @"abc@xyz.com"}};
+
+  [self writeDocumentRef:doc data:initialData];
+
+  FIRDocumentSnapshot *snap1 = [self readDocumentForRef:doc];
+  FIRDocumentSnapshot *snap2 = [self readDocumentForRef:doc];
+  FIRDocumentSnapshot *snap3 = [self readDocumentForRef:doc];
+
+  XCTAssertTrue([snap1.metadata isEqual:snap2.metadata]);
+  XCTAssertTrue([snap2.metadata isEqual:snap3.metadata]);
+
+  XCTAssertTrue([snap1.documentID isEqual:snap2.documentID]);
+  XCTAssertTrue([snap2.documentID isEqual:snap3.documentID]);
+
+  XCTAssertTrue(snap1.exists == snap2.exists);
+  XCTAssertTrue(snap2.exists == snap3.exists);
+
+  XCTAssertTrue([snap1.reference isEqual:snap2.reference]);
+  XCTAssertTrue([snap2.reference isEqual:snap3.reference]);
+
+  XCTAssertTrue([[snap1 data] isEqual:[snap2 data]]);
+  XCTAssertTrue([[snap2 data] isEqual:[snap3 data]]);
+
+  XCTAssertTrue([snap1 isEqual:snap2]);
+  XCTAssertTrue([snap2 isEqual:snap3]);
+}
+
 - (void)testCanUpdateAnUnknownDocument {
   [self readerAndWriterOnDocumentRef:^(FIRDocumentReference *readerRef,
                                        FIRDocumentReference *writerRef) {

+ 4 - 1
Firestore/core/src/api/document_snapshot.cc

@@ -88,7 +88,10 @@ absl::optional<google_firestore_v1_Value> DocumentSnapshot::GetValue(
 bool operator==(const DocumentSnapshot& lhs, const DocumentSnapshot& rhs) {
   return lhs.firestore_ == rhs.firestore_ &&
          lhs.internal_key_ == rhs.internal_key_ &&
-         lhs.internal_document_ == rhs.internal_document_ &&
+         lhs.exists() == rhs.exists() &&
+         (lhs.exists() ? lhs.internal_document_->get().data() ==
+                             rhs.internal_document_->get().data()
+                       : true) &&
          lhs.metadata_ == rhs.metadata_;
 }
 

+ 1 - 1
Firestore/core/src/model/mutable_document.h

@@ -167,7 +167,7 @@ class MutableDocument {
     return value_->Get();
   }
 
-  ObjectValue& data() {
+  ObjectValue& data() const {
     return *value_;
   }