Skip to content

operator== and operator!= for DocumentSnapshot. #602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions firestore/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 26 additions & 2 deletions firestore/integration_test_internal/src/document_snapshot_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <utility>

#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"
Expand All @@ -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<DocumentSnapshot,
Expand All @@ -30,5 +32,27 @@ TEST_F(DocumentSnapshotTest, Assignment) {

#endif

TEST_F(DocumentSnapshotTest, Equality) {
DocumentReference doc1 = Collection("col1").Document();
DocumentReference doc2 = Collection("col1").Document();
DocumentSnapshot doc1_snap1 = ReadDocument(doc1);
DocumentSnapshot doc1_snap2 = ReadDocument(doc1);
DocumentSnapshot doc2_snap1 = ReadDocument(doc2);
DocumentSnapshot snap3 = DocumentSnapshot();
DocumentSnapshot snap4 = DocumentSnapshot();

EXPECT_TRUE(doc1_snap1 == doc1_snap1);
EXPECT_TRUE(doc1_snap1 == doc1_snap2);
EXPECT_TRUE(doc1_snap1 != doc2_snap1);
EXPECT_TRUE(doc1_snap1 != snap3);
EXPECT_TRUE(snap3 == snap4);

EXPECT_FALSE(doc1_snap1 != doc1_snap1);
EXPECT_FALSE(doc1_snap1 != doc1_snap2);
EXPECT_FALSE(doc1_snap1 == doc2_snap1);
EXPECT_FALSE(doc1_snap1 == snap3);
EXPECT_FALSE(snap3 != snap4);
}

} // namespace firestore
} // namespace firebase
6 changes: 6 additions & 0 deletions firestore/src/android/document_snapshot_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "firestore/src/android/server_timestamp_behavior_android.h"
#include "firestore/src/android/snapshot_metadata_android.h"
#include "firestore/src/include/firebase/firestore.h"
#include "firestore/src/jni/compare.h"
#include "firestore/src/jni/env.h"
#include "firestore/src/jni/loader.h"

Expand Down Expand Up @@ -114,5 +115,10 @@ FieldValue DocumentSnapshotInternal::Get(const FieldPath& field,
return FieldValueInternal::Create(env, field_value);
}

bool operator==(const DocumentSnapshotInternal& lhs,
const DocumentSnapshotInternal& rhs) {
return jni::EqualityCompareJni(lhs, rhs);
}

} // namespace firestore
} // namespace firebase
8 changes: 8 additions & 0 deletions firestore/src/android/document_snapshot_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ class DocumentSnapshotInternal : public Wrapper {
mutable std::string cached_id_;
};

bool operator==(const DocumentSnapshotInternal& lhs,
const DocumentSnapshotInternal& rhs);

inline bool operator!=(const DocumentSnapshotInternal& lhs,
const DocumentSnapshotInternal& rhs) {
return !(lhs == rhs);
}

} // namespace firestore
} // namespace firebase
#endif // FIREBASE_FIRESTORE_SRC_ANDROID_DOCUMENT_SNAPSHOT_ANDROID_H_
4 changes: 2 additions & 2 deletions firestore/src/android/field_value_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "firestore/src/jni/array_list.h"
#include "firestore/src/jni/boolean.h"
#include "firestore/src/jni/class.h"
#include "firestore/src/jni/compare.h"
#include "firestore/src/jni/double.h"
#include "firestore/src/jni/env.h"
#include "firestore/src/jni/hash_map.h"
Expand Down Expand Up @@ -370,8 +371,7 @@ FieldValue FieldValueInternal::DoubleIncrement(double by_value) {
}

bool operator==(const FieldValueInternal& lhs, const FieldValueInternal& rhs) {
Env env = FieldValueInternal::GetEnv();
return Object::Equals(env, lhs.object_, rhs.object_);
return jni::EqualityCompareJni(lhs, rhs);
}

template <typename T>
Expand Down
4 changes: 2 additions & 2 deletions firestore/src/android/query_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -332,8 +333,7 @@ Local<Array<Object>> 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
Expand Down
4 changes: 2 additions & 2 deletions firestore/src/android/query_snapshot_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions firestore/src/common/document_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 2 additions & 3 deletions firestore/src/common/field_value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 2 additions & 5 deletions firestore/src/common/query.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions firestore/src/common/query_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions firestore/src/common/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T>
bool EqualityCompare(T* lhs, T* rhs) {
return lhs == rhs || (lhs != nullptr && rhs != nullptr && *lhs == *rhs);
}

} // namespace firestore
} // namespace firebase

Expand Down
12 changes: 12 additions & 0 deletions firestore/src/include/firebase/firestore/document_snapshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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

Expand Down
39 changes: 39 additions & 0 deletions firestore/src/jni/compare.h
Original file line number Diff line number Diff line change
@@ -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 <typename T>
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
5 changes: 5 additions & 0 deletions firestore/src/main/document_snapshot_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 8 additions & 0 deletions firestore/src/main/document_snapshot_main.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class DocumentSnapshotInternal {

DocumentReference reference() const;

friend bool operator==(const DocumentSnapshotInternal& lhs,
const DocumentSnapshotInternal& rhs);

private:
using ServerTimestampBehavior = DocumentSnapshot::ServerTimestampBehavior;

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion release_build_files/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down