Skip to content

Commit d43a0ef

Browse files
tom-andersentom-andersen
committed
Add tests. Revealed bugs to be fixed.
1 parent 5105939 commit d43a0ef

File tree

5 files changed

+155
-16
lines changed

5 files changed

+155
-16
lines changed

‎firestore/integration_test_internal/src/filter_test.cc

Lines changed: 140 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,109 @@ namespace {
2323

2424
using FilterTest = FirestoreIntegrationTest;
2525

26-
TEST_F(FilterTest, IdenticalShouldBeEqual) {
26+
TEST_F(FilterTest, CopyConstructorReturnsEqualObject) {
27+
const Filter filter1a = Filter::EqualTo("foo", FieldValue::Integer(42));
28+
const Filter filter2a = Filter::ArrayContainsAny(
29+
"bar", {FieldValue::Integer(4), FieldValue::Integer(2)});
30+
const Filter filter3a = Filter::And(filter1a, filter2a);
31+
32+
const Filter filter1b(filter1a);
33+
const Filter filter2b(filter2a);
34+
const Filter filter3b(filter3a);
35+
36+
EXPECT_EQ(filter1a, filter1b);
37+
EXPECT_EQ(filter2a, filter2b);
38+
EXPECT_EQ(filter3a, filter3b);
39+
}
40+
41+
TEST_F(FilterTest, CopyAssignementReturnsEqualObject) {
42+
const Filter filter1 = Filter::EqualTo("foo", FieldValue::Integer(42));
43+
const Filter filter2 = Filter::ArrayContainsAny(
44+
"bar", {FieldValue::Integer(4), FieldValue::Integer(2)});
45+
const Filter filter3 = Filter::And(filter1, filter2);
46+
47+
Filter filter = Filter::And();
48+
49+
EXPECT_NE(filter, filter1);
50+
EXPECT_NE(filter, filter2);
51+
EXPECT_NE(filter, filter3);
52+
53+
filter = filter1;
54+
55+
EXPECT_EQ(filter, filter1);
56+
EXPECT_NE(filter, filter2);
57+
EXPECT_NE(filter, filter3);
58+
59+
filter = filter2;
60+
61+
EXPECT_NE(filter, filter1);
62+
EXPECT_EQ(filter, filter2);
63+
EXPECT_NE(filter, filter3);
64+
65+
filter = filter3;
66+
67+
EXPECT_NE(filter, filter1);
68+
EXPECT_NE(filter, filter2);
69+
EXPECT_EQ(filter, filter3);
70+
}
71+
72+
TEST_F(FilterTest, MoveConstructorReturnsEqualObject) {
73+
Filter filter1a = Filter::EqualTo("foo", FieldValue::Integer(42));
74+
Filter filter2a = Filter::ArrayContainsAny(
75+
"bar", {FieldValue::Integer(4), FieldValue::Integer(2)});
76+
Filter filter3a = Filter::And(filter1a, filter2a);
77+
78+
Filter filter1b(std::move(filter1a));
79+
EXPECT_EQ(filter1b, Filter::EqualTo("foo", FieldValue::Integer(42)));
80+
81+
Filter filter2b(std::move(filter2a));
82+
EXPECT_EQ(filter2b, Filter::ArrayContainsAny("bar", {FieldValue::Integer(4), FieldValue::Integer(2)}));
83+
84+
Filter filter3b(std::move(filter3a));
85+
EXPECT_EQ(filter3b, Filter::And(filter1b, filter2b));
86+
}
87+
88+
TEST_F(FilterTest, MoveAssignmentReturnsEqualObject) {
89+
Filter filter1a = Filter::EqualTo("foo", FieldValue::Integer(42));
90+
Filter filter2a = Filter::ArrayContainsAny(
91+
"bar", {FieldValue::Integer(4), FieldValue::Integer(2)});
92+
Filter filter3a = Filter::And(filter1a, filter2a);
93+
94+
Filter filter1b = std::move(filter1a);
95+
EXPECT_EQ(filter1b, Filter::EqualTo("foo", FieldValue::Integer(42)));
96+
97+
Filter filter2b = std::move(filter2a);
98+
EXPECT_EQ(filter2b, Filter::ArrayContainsAny("bar", {FieldValue::Integer(4), FieldValue::Integer(2)}));
99+
100+
Filter filter3b = std::move(filter3a);
101+
EXPECT_EQ(filter3b, Filter::And(filter1b, filter2b));
102+
}
103+
104+
TEST_F(FilterTest, MoveAssignmentAppliedToSelfReturnsEqualObject) {
105+
Filter filter1 = Filter::EqualTo("foo", FieldValue::Integer(42));
106+
Filter filter2 = Filter::ArrayContainsAny(
107+
"bar", {FieldValue::Integer(4), FieldValue::Integer(2)});
108+
Filter filter3 = Filter::And(filter1, filter2);
109+
110+
filter1 = std::move(filter1);
111+
EXPECT_EQ(filter1, Filter::EqualTo("foo", FieldValue::Integer(42)));
112+
113+
filter2 = std::move(filter2);
114+
EXPECT_EQ(filter2, Filter::ArrayContainsAny("bar", {FieldValue::Integer(4), FieldValue::Integer(2)}));
115+
116+
filter3 = std::move(filter3);
117+
EXPECT_EQ(filter3, Filter::And(filter1, filter2));
118+
}
119+
120+
TEST_F(FilterTest, IdenticalFilterShouldBeEqual) {
27121
FieldPath foo_path{std::vector<std::string>{"foo"}};
28122

29123
Filter filter1a = Filter::ArrayContains("foo", FieldValue::Integer(42));
30124
Filter filter1b = Filter::ArrayContains(foo_path, FieldValue::Integer(42));
31125

32-
Filter filter2a = Filter::ArrayContainsAny(
33-
"foo", std::vector<FieldValue>{FieldValue::Integer(42)});
34-
Filter filter2b = Filter::ArrayContainsAny(
35-
foo_path, std::vector<FieldValue>{FieldValue::Integer(42)});
126+
Filter filter2a = Filter::ArrayContainsAny("foo", {FieldValue::Integer(42)});
127+
Filter filter2b =
128+
Filter::ArrayContainsAny(foo_path, {FieldValue::Integer(42)});
36129

37130
Filter filter3a = Filter::EqualTo("foo", FieldValue::Integer(42));
38131
Filter filter3b = Filter::EqualTo(foo_path, FieldValue::Integer(42));
@@ -55,15 +148,17 @@ TEST_F(FilterTest, IdenticalShouldBeEqual) {
55148
Filter filter8b =
56149
Filter::LessThanOrEqualTo(foo_path, FieldValue::Integer(42));
57150

58-
Filter filter9a =
59-
Filter::In("foo", std::vector<FieldValue>{FieldValue::Integer(42)});
60-
Filter filter9b =
61-
Filter::In(foo_path, std::vector<FieldValue>{FieldValue::Integer(42)});
151+
Filter filter9a = Filter::In("foo", {FieldValue::Integer(42)});
152+
Filter filter9b = Filter::In(foo_path, {FieldValue::Integer(42)});
153+
154+
Filter filter10a = Filter::NotIn("foo", {FieldValue::Integer(42)});
155+
Filter filter10b = Filter::NotIn(foo_path, {FieldValue::Integer(42)});
62156

63-
Filter filter10a =
64-
Filter::NotIn("foo", std::vector<FieldValue>{FieldValue::Integer(42)});
65-
Filter filter10b =
66-
Filter::NotIn(foo_path, std::vector<FieldValue>{FieldValue::Integer(42)});
157+
Filter filter11a = Filter::And(filter1a, filter2a);
158+
Filter filter11b = Filter::And(filter1b, filter2b);
159+
160+
Filter filter12a = Filter::Or(filter3a, filter4a, filter5a, filter6a, filter7a);
161+
Filter filter12b = Filter::Or(filter3b, filter4b, filter5b, filter6b, filter7b);
67162

68163
EXPECT_TRUE(filter1a == filter1a);
69164
EXPECT_TRUE(filter2a == filter2a);
@@ -75,6 +170,8 @@ TEST_F(FilterTest, IdenticalShouldBeEqual) {
75170
EXPECT_TRUE(filter8a == filter8a);
76171
EXPECT_TRUE(filter9a == filter9a);
77172
EXPECT_TRUE(filter10a == filter10a);
173+
EXPECT_TRUE(filter11a == filter11a);
174+
EXPECT_TRUE(filter12a == filter12a);
78175

79176
EXPECT_TRUE(filter1a == filter1b);
80177
EXPECT_TRUE(filter2a == filter2b);
@@ -86,6 +183,8 @@ TEST_F(FilterTest, IdenticalShouldBeEqual) {
86183
EXPECT_TRUE(filter8a == filter8b);
87184
EXPECT_TRUE(filter9a == filter9b);
88185
EXPECT_TRUE(filter10a == filter10b);
186+
EXPECT_TRUE(filter11a == filter11b);
187+
EXPECT_TRUE(filter12a == filter12b);
89188

90189
EXPECT_FALSE(filter1a != filter1a);
91190
EXPECT_FALSE(filter2a != filter2a);
@@ -97,6 +196,8 @@ TEST_F(FilterTest, IdenticalShouldBeEqual) {
97196
EXPECT_FALSE(filter8a != filter8a);
98197
EXPECT_FALSE(filter9a != filter9a);
99198
EXPECT_FALSE(filter10a != filter10a);
199+
EXPECT_FALSE(filter11a != filter11a);
200+
EXPECT_FALSE(filter12a != filter12a);
100201

101202
EXPECT_FALSE(filter1a != filter1b);
102203
EXPECT_FALSE(filter2a != filter2b);
@@ -108,6 +209,8 @@ TEST_F(FilterTest, IdenticalShouldBeEqual) {
108209
EXPECT_FALSE(filter8a != filter8b);
109210
EXPECT_FALSE(filter9a != filter9b);
110211
EXPECT_FALSE(filter10a != filter10b);
212+
EXPECT_FALSE(filter11a != filter11b);
213+
EXPECT_FALSE(filter12a != filter12b);
111214

112215
EXPECT_TRUE(filter1a != filter2a);
113216
EXPECT_TRUE(filter1a != filter3a);
@@ -118,6 +221,8 @@ TEST_F(FilterTest, IdenticalShouldBeEqual) {
118221
EXPECT_TRUE(filter1a != filter8a);
119222
EXPECT_TRUE(filter1a != filter9a);
120223
EXPECT_TRUE(filter1a != filter10a);
224+
EXPECT_TRUE(filter1a != filter11a);
225+
EXPECT_TRUE(filter1a != filter12a);
121226
EXPECT_TRUE(filter2a != filter3a);
122227
EXPECT_TRUE(filter2a != filter4a);
123228
EXPECT_TRUE(filter2a != filter5a);
@@ -126,34 +231,53 @@ TEST_F(FilterTest, IdenticalShouldBeEqual) {
126231
EXPECT_TRUE(filter2a != filter8a);
127232
EXPECT_TRUE(filter2a != filter9a);
128233
EXPECT_TRUE(filter2a != filter10a);
234+
EXPECT_TRUE(filter2a != filter11a);
235+
EXPECT_TRUE(filter2a != filter12a);
129236
EXPECT_TRUE(filter3a != filter4a);
130237
EXPECT_TRUE(filter3a != filter5a);
131238
EXPECT_TRUE(filter3a != filter6a);
132239
EXPECT_TRUE(filter3a != filter7a);
133240
EXPECT_TRUE(filter3a != filter8a);
134241
EXPECT_TRUE(filter3a != filter9a);
135242
EXPECT_TRUE(filter3a != filter10a);
243+
EXPECT_TRUE(filter3a != filter11a);
244+
EXPECT_TRUE(filter3a != filter12a);
136245
EXPECT_TRUE(filter4a != filter5a);
137246
EXPECT_TRUE(filter4a != filter6a);
138247
EXPECT_TRUE(filter4a != filter7a);
139248
EXPECT_TRUE(filter4a != filter8a);
140249
EXPECT_TRUE(filter4a != filter9a);
141250
EXPECT_TRUE(filter4a != filter10a);
251+
EXPECT_TRUE(filter4a != filter11a);
252+
EXPECT_TRUE(filter4a != filter12a);
142253
EXPECT_TRUE(filter5a != filter6a);
143254
EXPECT_TRUE(filter5a != filter7a);
144255
EXPECT_TRUE(filter5a != filter8a);
145256
EXPECT_TRUE(filter5a != filter9a);
146257
EXPECT_TRUE(filter5a != filter10a);
258+
EXPECT_TRUE(filter5a != filter11a);
259+
EXPECT_TRUE(filter5a != filter12a);
147260
EXPECT_TRUE(filter6a != filter7a);
148261
EXPECT_TRUE(filter6a != filter8a);
149262
EXPECT_TRUE(filter6a != filter9a);
150263
EXPECT_TRUE(filter6a != filter10a);
264+
EXPECT_TRUE(filter6a != filter11a);
265+
EXPECT_TRUE(filter6a != filter12a);
151266
EXPECT_TRUE(filter7a != filter8a);
152267
EXPECT_TRUE(filter7a != filter9a);
153268
EXPECT_TRUE(filter7a != filter10a);
269+
EXPECT_TRUE(filter7a != filter11a);
270+
EXPECT_TRUE(filter7a != filter12a);
154271
EXPECT_TRUE(filter8a != filter9a);
155272
EXPECT_TRUE(filter8a != filter10a);
273+
EXPECT_TRUE(filter8a != filter11a);
274+
EXPECT_TRUE(filter8a != filter12a);
156275
EXPECT_TRUE(filter9a != filter10a);
276+
EXPECT_TRUE(filter9a != filter11a);
277+
EXPECT_TRUE(filter9a != filter12a);
278+
EXPECT_TRUE(filter10a != filter11a);
279+
EXPECT_TRUE(filter10a != filter12a);
280+
EXPECT_TRUE(filter11a != filter12a);
157281
}
158282

159283
TEST_F(FilterTest, DifferentValuesAreNotEqual) {
@@ -231,7 +355,7 @@ TEST_F(FilterTest, CompositesWithOneFilterAreTheSameAsFilter) {
231355
EXPECT_FALSE(filter1 != filter3);
232356
}
233357

234-
TEST_F(FilterTest, EmptyCompositeIsIgnored) {
358+
TEST_F(FilterTest, EmptyCompositeIsIgnoredByCompositesAndQueries) {
235359
Filter filter1 = Filter::And();
236360
Filter filter2 = Filter::And(Filter::And(), Filter::And());
237361
Filter filter3 = Filter::And(Filter::Or(), Filter::Or());
@@ -287,7 +411,9 @@ TEST_F(FilterTest, CompositeComparison) {
287411
EXPECT_EQ(or3, or3);
288412
EXPECT_EQ(or4, or4);
289413

414+
// Is equal because single filter composite is same as filter itself.
290415
EXPECT_EQ(and1, or1);
416+
291417
EXPECT_NE(and2, or2);
292418
EXPECT_NE(and3, or3);
293419
EXPECT_NE(and4, or4);

‎firestore/src/include/firebase/firestore/filter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ class Filter {
362362
bool IsEmpty() const;
363363

364364
explicit Filter(FilterInternal* internal);
365-
FilterInternal* internal_;
365+
FilterInternal* internal_ = nullptr;
366366
};
367367

368368
/** Checks `lhs` and `rhs` for equality. */

‎firestore/src/main/composite_filter_main.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "Firestore/core/src/core/composite_filter.h"
2525
#include "firestore/src/main/composite_filter_main.h"
2626
#include "firestore/src/main/converter_main.h"
27+
#include "absl/algorithm/container.h"
2728

2829
namespace firebase {
2930
namespace firestore {
@@ -54,7 +55,15 @@ core::Filter CompositeFilterInternal::ToCoreFilter(
5455

5556
bool operator==(const CompositeFilterInternal& lhs,
5657
const CompositeFilterInternal& rhs) {
57-
return lhs.op_ == rhs.op_ && lhs.filters_ == rhs.filters_;
58+
if (lhs.op_ != rhs.op_) return false;
59+
const std::vector<std::shared_ptr<FilterInternal>>& lhs_filters = lhs.filters_;
60+
const std::vector<std::shared_ptr<FilterInternal>>& rhs_filters = rhs.filters_;
61+
const unsigned long size = lhs_filters.size();
62+
if (size != rhs_filters.size()) return false;
63+
for (int i = 0; i < size; i++) {
64+
if (lhs_filters[i] != rhs_filters[i] && *lhs_filters[i] != *rhs_filters[i]) return false;
65+
}
66+
return true;
5867
}
5968

6069
} // namespace firestore

‎firestore/src/main/composite_filter_main.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ class CompositeFilterInternal : public FilterInternal {
3535
CompositeFilterInternal(core::CompositeFilter::Operator op,
3636
std::vector<FilterInternal*>& filters);
3737

38+
~CompositeFilterInternal() override = default;
39+
3840
core::Filter ToCoreFilter(const api::Query& query,
3941
const firebase::firestore::UserDataConverter&
4042
user_data_converter) const override;

‎firestore/src/main/unary_filter_main.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ class UnaryFilterInternal final : public FilterInternal {
3838
core::FieldFilter::Operator op,
3939
const std::vector<FieldValue>& values);
4040

41+
~UnaryFilterInternal() override = default;
42+
4143
core::Filter ToCoreFilter(const api::Query& query,
4244
const firebase::firestore::UserDataConverter&
4345
user_data_converter) const override;

0 commit comments

Comments
 (0)