operator== and operator!= for DocumentChange (#607)

ehsannas · web-flow · commit c85d0ed92349 · 2021-08-23T20:11:15.000-05:00
* operator== and operator!= for DocumentChange.

* delegate to underlying impl.

* Address comments.

* Update readme.

* Use ASSERT_EQ in a few more instances.

* rename third_change to change3.
diff --git a/firestore/integration_test_internal/src/document_change_test.cc b/firestore/integration_test_internal/src/document_change_test.cc
@@ -27,10 +27,10 @@
 namespace firebase {
 namespace firestore {
 
-#if defined(__ANDROID__)
-
 using DocumentChangeTest = FirestoreIntegrationTest;
 
+#if defined(__ANDROID__)
+
 TEST_F(DocumentChangeTest, Construction) {
   testutil::AssertWrapperConstructionContract<DocumentChange,
                                               DocumentChangeInternal>();
@@ -59,7 +59,7 @@ TEST_F(DocumentChangeTest, TestDocumentChanges) {
   snapshot = listener.last_result();
 
   std::vector<DocumentChange> changes = snapshot.DocumentChanges();
-  EXPECT_EQ(changes.size(), 1);
+  ASSERT_EQ(changes.size(), 1);
 
   EXPECT_EQ(changes[0].type(), DocumentChange::Type::kAdded);
   EXPECT_EQ(changes[0].document().id(), doc1.id());
@@ -71,7 +71,7 @@ TEST_F(DocumentChangeTest, TestDocumentChanges) {
   snapshot = listener.last_result();
 
   changes = snapshot.DocumentChanges();
-  EXPECT_EQ(changes.size(), 1);
+  ASSERT_EQ(changes.size(), 1);
   EXPECT_EQ(changes[0].type(), DocumentChange::Type::kAdded);
   EXPECT_EQ(changes[0].document().id(), doc2.id());
   EXPECT_EQ(changes[0].old_index(), DocumentChange::npos);
@@ -83,7 +83,7 @@ TEST_F(DocumentChangeTest, TestDocumentChanges) {
   snapshot = listener.last_result();
 
   changes = snapshot.DocumentChanges();
-  EXPECT_EQ(changes.size(), 1);
+  ASSERT_EQ(changes.size(), 1);
   EXPECT_EQ(changes[0].type(), DocumentChange::Type::kModified);
   EXPECT_EQ(changes[0].document().id(), doc2.id());
   EXPECT_EQ(changes[0].old_index(), 1);
@@ -92,5 +92,67 @@ TEST_F(DocumentChangeTest, TestDocumentChanges) {
 
 #endif  // defined(__ANDROID__)
 
+TEST_F(DocumentChangeTest, Equality) {
+  DocumentChange invalid_change_1 = DocumentChange();
+  DocumentChange invalid_change_2 = DocumentChange();
+
+  EXPECT_TRUE(invalid_change_1 == invalid_change_2);
+  EXPECT_FALSE(invalid_change_1 != invalid_change_2);
+
+  CollectionReference collection = Collection();
+  Query query = collection.OrderBy("a");
+
+  DocumentReference doc1 = collection.Document("1");
+  DocumentReference doc2 = collection.Document("2");
+
+  TestEventListener<QuerySnapshot> listener("TestDocumentChanges");
+  ListenerRegistration registration = listener.AttachTo(&query);
+  Await(listener);
+  QuerySnapshot snapshot = listener.last_result();
+  EXPECT_EQ(snapshot.size(), 0);
+
+  WriteDocument(doc1, MapFieldValue{{"a", FieldValue::Integer(1)}});
+  Await(listener);
+  snapshot = listener.last_result();
+
+  std::vector<DocumentChange> changes = snapshot.DocumentChanges();
+  ASSERT_EQ(changes.size(), 1);
+  // First change: Added doc1.
+  auto change1 = changes[0];
+  EXPECT_TRUE(change1 == change1);
+  EXPECT_TRUE(change1 != invalid_change_1);
+  EXPECT_FALSE(change1 != change1);
+  EXPECT_FALSE(change1 == invalid_change_1);
+
+  WriteDocument(doc2, MapFieldValue{{"a", FieldValue::Integer(2)}});
+  Await(listener, 2);
+  snapshot = listener.last_result();
+
+  changes = snapshot.DocumentChanges();
+  ASSERT_EQ(changes.size(), 1);
+  // Second change: Added doc2.
+  auto change2 = changes[0];
+  EXPECT_TRUE(change2 != change1);
+  EXPECT_TRUE(change2 != invalid_change_1);
+  EXPECT_FALSE(change2 == change1);
+  EXPECT_FALSE(change2 == invalid_change_1);
+
+  // Make doc2 ordered before doc1.
+  WriteDocument(doc2, MapFieldValue{{"a", FieldValue::Integer(0)}});
+  Await(listener, 3);
+  snapshot = listener.last_result();
+
+  changes = snapshot.DocumentChanges();
+  ASSERT_EQ(changes.size(), 1);
+  // Third change: Modified doc2.
+  auto change3 = changes[0];
+  EXPECT_TRUE(change3 != change1);
+  EXPECT_TRUE(change3 != change2);
+  EXPECT_TRUE(change3 != invalid_change_1);
+  EXPECT_FALSE(change3 == change1);
+  EXPECT_FALSE(change3 == change2);
+  EXPECT_FALSE(change3 == invalid_change_1);
+}
+
 }  // namespace firestore
 }  // namespace firebase
diff --git a/firestore/src/android/document_change_android.cc b/firestore/src/android/document_change_android.cc
@@ -18,6 +18,7 @@
 
 #include "firestore/src/android/document_change_type_android.h"
 #include "firestore/src/android/document_snapshot_android.h"
+#include "firestore/src/jni/compare.h"
 #include "firestore/src/jni/env.h"
 #include "firestore/src/jni/loader.h"
 
@@ -69,5 +70,10 @@ std::size_t DocumentChangeInternal::new_index() const {
   return env.Call(obj_, kNewIndex);
 }
 
+bool operator==(const DocumentChangeInternal& lhs,
+                const DocumentChangeInternal& rhs) {
+  return jni::EqualityCompareJni(lhs, rhs);
+}
+
 }  // namespace firestore
 }  // namespace firebase
diff --git a/firestore/src/android/document_change_android.h b/firestore/src/android/document_change_android.h
@@ -38,6 +38,14 @@ class DocumentChangeInternal : public Wrapper {
   std::size_t new_index() const;
 };
 
+bool operator==(const DocumentChangeInternal& lhs,
+                const DocumentChangeInternal& rhs);
+
+inline bool operator!=(const DocumentChangeInternal& lhs,
+                       const DocumentChangeInternal& rhs) {
+  return !(lhs == rhs);
+}
+
 }  // namespace firestore
 }  // namespace firebase
 
diff --git a/firestore/src/common/document_change.cc b/firestore/src/common/document_change.cc
@@ -20,6 +20,7 @@
 
 #include "firestore/src/common/cleanup.h"
 #include "firestore/src/common/hard_assert_common.h"
+#include "firestore/src/common/util.h"
 #include "firestore/src/include/firebase/firestore/document_snapshot.h"
 #if defined(__ANDROID__)
 #include "firestore/src/android/document_change_android.h"
@@ -118,5 +119,9 @@ std::size_t DocumentChange::new_index() const {
   return internal_->new_index();
 }
 
+bool operator==(const DocumentChange& lhs, const DocumentChange& rhs) {
+  return EqualityCompare(lhs.internal_, rhs.internal_);
+}
+
 }  // namespace firestore
 }  // namespace firebase
diff --git a/firestore/src/include/firebase/firestore/document_change.h b/firestore/src/include/firebase/firestore/document_change.h
@@ -173,6 +173,8 @@ class DocumentChange {
   bool is_valid() const { return internal_ != nullptr; }
 
  private:
+  friend bool operator==(const DocumentChange& lhs, const DocumentChange& rhs);
+
   friend class FirestoreInternal;
   friend class Wrapper;
   friend struct ConverterImpl;
@@ -184,6 +186,14 @@ class DocumentChange {
   mutable DocumentChangeInternal* internal_ = nullptr;
 };
 
+/** Checks `lhs` and `rhs` for equality. */
+bool operator==(const DocumentChange& lhs, const DocumentChange& rhs);
+
+/** Checks `lhs` and `rhs` for inequality. */
+inline bool operator!=(const DocumentChange& lhs, const DocumentChange& rhs) {
+  return !(lhs == rhs);
+}
+
 }  // namespace firestore
 }  // namespace firebase
 
diff --git a/firestore/src/main/document_change_main.cc b/firestore/src/main/document_change_main.cc
@@ -60,5 +60,10 @@ std::size_t DocumentChangeInternal::new_index() const {
   return change_.new_index();
 }
 
+bool operator==(const DocumentChangeInternal& lhs,
+                const DocumentChangeInternal& rhs) {
+  return lhs.change_ == rhs.change_;
+}
+
 }  // namespace firestore
 }  // namespace firebase
diff --git a/firestore/src/main/document_change_main.h b/firestore/src/main/document_change_main.h
@@ -41,10 +41,18 @@ class DocumentChangeInternal {
   std::size_t old_index() const;
   std::size_t new_index() const;
 
+  friend bool operator==(const DocumentChangeInternal& lhs,
+                         const DocumentChangeInternal& rhs);
+
  private:
   api::DocumentChange change_;
 };
 
+inline bool operator!=(const DocumentChangeInternal& lhs,
+                       const DocumentChangeInternal& rhs) {
+  return !(lhs == rhs);
+}
+
 }  // namespace firestore
 }  // namespace firebase
 
diff --git a/release_build_files/readme.md b/release_build_files/readme.md
@@ -571,7 +571,8 @@ code.
 -   Changes
     -   General: Updating Android and iOS dependencies to the latest.
     -   Firestore: Added `operator==` and `operator!=` for `SnapshotMetadata`,
-        `Settings`, `QuerySnapshot`, `DocumentSnapshot`, and `SetOptions`.
+        `Settings`, `QuerySnapshot`, `DocumentSnapshot`, `SetOptions`, and
+        `DocumentChange`.
 
 ### 8.3.0
 -   Changes
