diff --git a/firestore/CMakeLists.txt b/firestore/CMakeLists.txt index afe6d7db72..c6732b4ef1 100644 --- a/firestore/CMakeLists.txt +++ b/firestore/CMakeLists.txt @@ -143,6 +143,7 @@ set(android_SRCS src/jni/class.h src/jni/collection.cc src/jni/collection.h + src/jni/compare.h src/jni/declaration.h src/jni/double.cc src/jni/double.h diff --git a/firestore/integration_test_internal/src/document_snapshot_test.cc b/firestore/integration_test_internal/src/document_snapshot_test.cc index 70e0f90a5c..be2e78a319 100644 --- a/firestore/integration_test_internal/src/document_snapshot_test.cc +++ b/firestore/integration_test_internal/src/document_snapshot_test.cc @@ -3,6 +3,8 @@ #include #include "firebase/firestore.h" +#include "firestore_integration_test.h" + #if defined(__ANDROID__) #include "firestore/src/android/document_snapshot_android.h" #include "firestore/src/common/wrapper_assertions.h" @@ -14,9 +16,9 @@ namespace firebase { namespace firestore { -#if defined(__ANDROID__) +using DocumentSnapshotTest = FirestoreIntegrationTest; -using DocumentSnapshotTest = testing::Test; +#if defined(__ANDROID__) TEST_F(DocumentSnapshotTest, Construction) { testutil::AssertWrapperConstructionContract diff --git a/firestore/src/android/query_android.cc b/firestore/src/android/query_android.cc index 845bfcc4e8..57bbb663b2 100644 --- a/firestore/src/android/query_android.cc +++ b/firestore/src/android/query_android.cc @@ -18,6 +18,7 @@ #include "firestore/src/include/firebase/firestore.h" #include "firestore/src/jni/array.h" #include "firestore/src/jni/array_list.h" +#include "firestore/src/jni/compare.h" #include "firestore/src/jni/env.h" #include "firestore/src/jni/loader.h" #include "firestore/src/jni/task.h" @@ -332,8 +333,7 @@ Local> QueryInternal::ConvertFieldValues( } bool operator==(const QueryInternal& lhs, const QueryInternal& rhs) { - Env env = FirestoreInternal::GetEnv(); - return Object::Equals(env, lhs.ToJava(), rhs.ToJava()); + return jni::EqualityCompareJni(lhs, rhs); } } // namespace firestore diff --git a/firestore/src/android/query_snapshot_android.cc b/firestore/src/android/query_snapshot_android.cc index 6765f2538c..4f9fef1f4e 100644 --- a/firestore/src/android/query_snapshot_android.cc +++ b/firestore/src/android/query_snapshot_android.cc @@ -9,6 +9,7 @@ #include "firestore/src/android/query_android.h" #include "firestore/src/android/snapshot_metadata_android.h" #include "firestore/src/android/util_android.h" +#include "firestore/src/jni/compare.h" #include "firestore/src/jni/env.h" #include "firestore/src/jni/list.h" #include "firestore/src/jni/loader.h" @@ -74,8 +75,7 @@ std::size_t QuerySnapshotInternal::size() const { bool operator==(const QuerySnapshotInternal& lhs, const QuerySnapshotInternal& rhs) { - Env env = FirestoreInternal::GetEnv(); - return Object::Equals(env, lhs.ToJava(), rhs.ToJava()); + return jni::EqualityCompareJni(lhs, rhs); } } // namespace firestore diff --git a/firestore/src/common/document_snapshot.cc b/firestore/src/common/document_snapshot.cc index a82e4d8040..4377833cf1 100644 --- a/firestore/src/common/document_snapshot.cc +++ b/firestore/src/common/document_snapshot.cc @@ -141,5 +141,9 @@ std::ostream& operator<<(std::ostream& out, const DocumentSnapshot& document) { return out << document.ToString(); } +bool operator==(const DocumentSnapshot& lhs, const DocumentSnapshot& rhs) { + return EqualityCompare(lhs.internal_, rhs.internal_); +} + } // namespace firestore } // namespace firebase diff --git a/firestore/src/common/field_value.cc b/firestore/src/common/field_value.cc index 6f915c13d4..fa87d82cfe 100644 --- a/firestore/src/common/field_value.cc +++ b/firestore/src/common/field_value.cc @@ -11,6 +11,7 @@ #include "firebase/firestore/timestamp.h" #include "firestore/src/common/hard_assert_common.h" #include "firestore/src/common/to_string.h" +#include "firestore/src/common/util.h" #include "firestore/src/include/firebase/firestore/document_reference.h" #if defined(__ANDROID__) #include "firestore/src/android/field_value_android.h" @@ -327,9 +328,7 @@ std::string FieldValue::ToString() const { } bool operator==(const FieldValue& lhs, const FieldValue& rhs) { - return lhs.internal_ == rhs.internal_ || - (lhs.internal_ != nullptr && rhs.internal_ != nullptr && - *lhs.internal_ == *rhs.internal_); + return EqualityCompare(lhs.internal_, rhs.internal_); } std::ostream& operator<<(std::ostream& out, const FieldValue& value) { diff --git a/firestore/src/common/query.cc b/firestore/src/common/query.cc index 0b352d646f..17a7836430 100644 --- a/firestore/src/common/query.cc +++ b/firestore/src/common/query.cc @@ -8,6 +8,7 @@ #include "firestore/src/common/event_listener.h" #include "firestore/src/common/futures.h" #include "firestore/src/common/hard_assert_common.h" +#include "firestore/src/common/util.h" #include "firestore/src/include/firebase/firestore/document_snapshot.h" #include "firestore/src/include/firebase/firestore/field_path.h" #include "firestore/src/include/firebase/firestore/field_value.h" @@ -288,11 +289,7 @@ ListenerRegistration Query::AddSnapshotListener( } bool operator==(const Query& lhs, const Query& rhs) { - if (!lhs.internal_ || !rhs.internal_) { - return lhs.internal_ == rhs.internal_; - } - - return lhs.internal_ == rhs.internal_ || *lhs.internal_ == *rhs.internal_; + return EqualityCompare(lhs.internal_, rhs.internal_); } } // namespace firestore diff --git a/firestore/src/common/query_snapshot.cc b/firestore/src/common/query_snapshot.cc index 5201cca28c..338f01cd31 100644 --- a/firestore/src/common/query_snapshot.cc +++ b/firestore/src/common/query_snapshot.cc @@ -6,6 +6,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_change.h" #include "firestore/src/include/firebase/firestore/document_snapshot.h" #include "firestore/src/include/firebase/firestore/query.h" @@ -104,11 +105,7 @@ std::size_t QuerySnapshot::size() const { } bool operator==(const QuerySnapshot& lhs, const QuerySnapshot& rhs) { - if (!lhs.internal_ || !rhs.internal_) { - return lhs.internal_ == rhs.internal_; - } - - return lhs.internal_ == rhs.internal_ || *lhs.internal_ == *rhs.internal_; + return EqualityCompare(lhs.internal_, rhs.internal_); } } // namespace firestore diff --git a/firestore/src/common/util.h b/firestore/src/common/util.h index 1f63ab39da..287c9087eb 100644 --- a/firestore/src/common/util.h +++ b/firestore/src/common/util.h @@ -13,6 +13,13 @@ namespace firestore { // a string to refer to. const std::string& EmptyString(); +// Returns true if the two given pointers are equal or the underlying objects +// that they point to are equal. Returns false otherwise. +template +bool EqualityCompare(T* lhs, T* rhs) { + return lhs == rhs || (lhs != nullptr && rhs != nullptr && *lhs == *rhs); +} + } // namespace firestore } // namespace firebase diff --git a/firestore/src/include/firebase/firestore/document_snapshot.h b/firestore/src/include/firebase/firestore/document_snapshot.h index 98286660f8..ca02a696d0 100644 --- a/firestore/src/include/firebase/firestore/document_snapshot.h +++ b/firestore/src/include/firebase/firestore/document_snapshot.h @@ -261,6 +261,9 @@ class DocumentSnapshot { const DocumentSnapshot& document); private: + friend bool operator==(const DocumentSnapshot& lhs, + const DocumentSnapshot& rhs); + friend class DocumentChangeInternal; friend class EventListenerInternal; friend class FirestoreInternal; @@ -276,6 +279,15 @@ class DocumentSnapshot { mutable DocumentSnapshotInternal* internal_ = nullptr; }; +/** Checks `lhs` and `rhs` for equality. */ +bool operator==(const DocumentSnapshot& lhs, const DocumentSnapshot& rhs); + +/** Checks `lhs` and `rhs` for inequality. */ +inline bool operator!=(const DocumentSnapshot& lhs, + const DocumentSnapshot& rhs) { + return !(lhs == rhs); +} + } // namespace firestore } // namespace firebase diff --git a/firestore/src/jni/compare.h b/firestore/src/jni/compare.h new file mode 100644 index 0000000000..d8a9dd55bd --- /dev/null +++ b/firestore/src/jni/compare.h @@ -0,0 +1,39 @@ +/* + * Copyright 2021 Google LLC + * + * 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/src/android/exception_android.h" +#include "firestore/src/android/wrapper.h" +#include "firestore/src/jni/env.h" +#include "firestore/src/jni/object.h" + +namespace firebase { +namespace firestore { +namespace jni { + +/** + * Returns the result of `Object::Equals` for the given `lhs` and `rhs` in the + * current Firestore `jni::Env`. + */ +template +bool EqualityCompareJni(const T& lhs, const T& rhs) { + Env env; + env.SetUnhandledExceptionHandler(GlobalUnhandledExceptionHandler, nullptr); + return Object::Equals(env, lhs.ToJava(), rhs.ToJava()); +} + +} // namespace jni +} // namespace firestore +} // namespace firebase diff --git a/firestore/src/main/document_snapshot_main.cc b/firestore/src/main/document_snapshot_main.cc index d8deb97727..617a4463fa 100644 --- a/firestore/src/main/document_snapshot_main.cc +++ b/firestore/src/main/document_snapshot_main.cc @@ -177,5 +177,10 @@ FieldValue DocumentSnapshotInternal::ConvertServerTimestamp( FIRESTORE_UNREACHABLE(); } +bool operator==(const DocumentSnapshotInternal& lhs, + const DocumentSnapshotInternal& rhs) { + return lhs.snapshot_ == rhs.snapshot_; +} + } // namespace firestore } // namespace firebase diff --git a/firestore/src/main/document_snapshot_main.h b/firestore/src/main/document_snapshot_main.h index 04936bfe57..915065e8fe 100644 --- a/firestore/src/main/document_snapshot_main.h +++ b/firestore/src/main/document_snapshot_main.h @@ -49,6 +49,9 @@ class DocumentSnapshotInternal { DocumentReference reference() const; + friend bool operator==(const DocumentSnapshotInternal& lhs, + const DocumentSnapshotInternal& rhs); + private: using ServerTimestampBehavior = DocumentSnapshot::ServerTimestampBehavior; @@ -74,6 +77,11 @@ class DocumentSnapshotInternal { api::DocumentSnapshot snapshot_; }; +inline bool operator!=(const DocumentSnapshotInternal& lhs, + const DocumentSnapshotInternal& rhs) { + return !(lhs == rhs); +} + } // namespace firestore } // namespace firebase diff --git a/release_build_files/readme.md b/release_build_files/readme.md index c908a3af1e..8f5475b8c2 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -570,7 +570,7 @@ code. ### 8.4.0 - Changes - Firestore: Added `operator==` and `operator!=` for `SnapshotMetadata`, - `Settings`, and `QuerySnapshot`. + `Settings`, `QuerySnapshot`, and `DocumentSnapshot`. ### 8.3.0 - Changes