Skip to content

Commit 9dc9f3e

Browse files
milaGGLmilaGGL
authored
Add unchanged names to existence filter and watchFilters spec test runner (#10862)
1 parent 5252a62 commit 9dc9f3e

22 files changed

+336
-120
lines changed

‎Firestore/Example/Firestore.xcodeproj/project.pbxproj

Lines changed: 20 additions & 20 deletions
Large diffs are not rendered by default.

‎Firestore/Example/Tests/SpecTests/FSTSpecTests.mm

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
#include "Firestore/core/src/model/types.h"
5757
#include "Firestore/core/src/nanopb/message.h"
5858
#include "Firestore/core/src/nanopb/nanopb_util.h"
59+
#include "Firestore/core/src/remote/bloom_filter.h"
5960
#include "Firestore/core/src/remote/existence_filter.h"
6061
#include "Firestore/core/src/remote/serializer.h"
6162
#include "Firestore/core/src/remote/watch_change.h"
@@ -71,6 +72,7 @@
7172
#include "Firestore/core/src/util/to_string.h"
7273
#include "Firestore/core/test/unit/testutil/testutil.h"
7374
#include "absl/memory/memory.h"
75+
#include "absl/strings/escaping.h"
7476
#include "absl/types/optional.h"
7577

7678
namespace objc = firebase::firestore::objc;
@@ -98,6 +100,7 @@
98100
using firebase::firestore::nanopb::ByteString;
99101
using firebase::firestore::nanopb::MakeByteString;
100102
using firebase::firestore::nanopb::Message;
103+
using firebase::firestore::remote::BloomFilter;
101104
using firebase::firestore::remote::DocumentWatchChange;
102105
using firebase::firestore::remote::ExistenceFilter;
103106
using firebase::firestore::remote::ExistenceFilterWatchChange;
@@ -115,6 +118,7 @@
115118
using firebase::firestore::util::MakeStringPtr;
116119
using firebase::firestore::util::Path;
117120
using firebase::firestore::util::Status;
121+
using firebase::firestore::util::StatusOr;
118122
using firebase::firestore::util::TimerId;
119123
using firebase::firestore::util::ToString;
120124
using firebase::firestore::util::WrapCompare;
@@ -325,6 +329,13 @@ - (SnapshotVersion)parseVersion:(NSNumber *_Nullable)version {
325329
return Version(version.longLongValue);
326330
}
327331

332+
- (absl::optional<BloomFilter>)parseBloomFilter:(NSDictionary *_Nullable)bloomFilterProto {
333+
// TODO(Mila): None of the ported spec tests actually has the bloom filter json string, so hard
334+
// code a null value for now. Actual parsing code will be written in the next PR, where we can
335+
// validate the parsing result.
336+
return absl::nullopt;
337+
}
338+
328339
- (DocumentViewChange)parseChange:(NSDictionary *)jsonDoc ofType:(DocumentViewChange::Type)type {
329340
NSNumber *version = jsonDoc[@"version"];
330341
NSDictionary *options = jsonDoc[@"options"];
@@ -463,14 +474,17 @@ - (void)doWatchEntity:(NSDictionary *)watchEntity {
463474
}
464475
}
465476

466-
- (void)doWatchFilter:(NSArray *)watchFilter {
467-
NSArray<NSNumber *> *targets = watchFilter[0];
477+
- (void)doWatchFilter:(NSDictionary *)watchFilter {
478+
NSArray<NSNumber *> *targets = watchFilter[@"targetIds"];
468479
HARD_ASSERT(targets.count == 1, "ExistenceFilters currently support exactly one target only.");
469480

470-
int keyCount = watchFilter.count == 0 ? 0 : (int)watchFilter.count - 1;
481+
NSArray<NSNumber *> *keys = watchFilter[@"keys"];
482+
int keyCount = keys ? (int)keys.count : 0;
483+
484+
absl::optional<BloomFilter> bloomFilter = [self parseBloomFilter:watchFilter[@"bloomFilter"]];
471485

472-
ExistenceFilter filter{keyCount};
473-
ExistenceFilterWatchChange change{filter, targets[0].intValue};
486+
ExistenceFilter filter{keyCount, std::move(bloomFilter)};
487+
ExistenceFilterWatchChange change{std::move(filter), targets[0].intValue};
474488
[self.driver receiveWatchChange:change snapshotVersion:SnapshotVersion::None()];
475489
}
476490

‎Firestore/Example/Tests/SpecTests/json/existence_filter_spec_test.json

Lines changed: 67 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,14 @@
128128
]
129129
},
130130
{
131-
"watchFilter": [
132-
[
133-
2
131+
"watchFilter": {
132+
"keys": [
133+
"collection/1"
134134
],
135-
"collection/1"
136-
]
135+
"targetIds": [
136+
2
137+
]
138+
}
137139
},
138140
{
139141
"watchSnapshot": {
@@ -358,13 +360,15 @@
358360
]
359361
},
360362
{
361-
"watchFilter": [
362-
[
363-
2
363+
"watchFilter": {
364+
"keys": [
365+
"collection/1",
366+
"collection/2"
364367
],
365-
"collection/1",
366-
"collection/2"
367-
]
368+
"targetIds": [
369+
2
370+
]
371+
}
368372
},
369373
{
370374
"watchEntity": {
@@ -712,11 +716,13 @@
712716
}
713717
},
714718
{
715-
"watchFilter": [
716-
[
719+
"watchFilter": {
720+
"keys": [
721+
],
722+
"targetIds": [
717723
2
718724
]
719-
]
725+
}
720726
},
721727
{
722728
"watchRemove": {
@@ -889,12 +895,14 @@
889895
]
890896
},
891897
{
892-
"watchFilter": [
893-
[
894-
2
898+
"watchFilter": {
899+
"keys": [
900+
"collection/1"
895901
],
896-
"collection/1"
897-
]
902+
"targetIds": [
903+
2
904+
]
905+
}
898906
},
899907
{
900908
"watchSnapshot": {
@@ -1178,12 +1186,14 @@
11781186
]
11791187
},
11801188
{
1181-
"watchFilter": [
1182-
[
1183-
2
1189+
"watchFilter": {
1190+
"keys": [
1191+
"collection/1"
11841192
],
1185-
"collection/1"
1186-
]
1193+
"targetIds": [
1194+
2
1195+
]
1196+
}
11871197
},
11881198
{
11891199
"watchSnapshot": {
@@ -1287,12 +1297,14 @@
12871297
}
12881298
},
12891299
{
1290-
"watchFilter": [
1291-
[
1292-
2
1300+
"watchFilter": {
1301+
"keys": [
1302+
"collection/1"
12931303
],
1294-
"collection/1"
1295-
]
1304+
"targetIds": [
1305+
2
1306+
]
1307+
}
12961308
},
12971309
{
12981310
"watchSnapshot": {
@@ -1458,12 +1470,14 @@
14581470
]
14591471
},
14601472
{
1461-
"watchFilter": [
1462-
[
1463-
2
1473+
"watchFilter": {
1474+
"keys": [
1475+
"collection/1"
14641476
],
1465-
"collection/1"
1466-
]
1477+
"targetIds": [
1478+
2
1479+
]
1480+
}
14671481
},
14681482
{
14691483
"watchSnapshot": {
@@ -1809,12 +1823,14 @@
18091823
]
18101824
},
18111825
{
1812-
"watchFilter": [
1813-
[
1814-
2
1826+
"watchFilter": {
1827+
"keys": [
1828+
"collection/1"
18151829
],
1816-
"collection/1"
1817-
]
1830+
"targetIds": [
1831+
2
1832+
]
1833+
}
18181834
},
18191835
{
18201836
"watchSnapshot": {
@@ -2108,11 +2124,13 @@
21082124
]
21092125
},
21102126
{
2111-
"watchFilter": [
2112-
[
2127+
"watchFilter": {
2128+
"keys": [
2129+
],
2130+
"targetIds": [
21132131
2
21142132
]
2115-
]
2133+
}
21162134
},
21172135
{
21182136
"watchSnapshot": {
@@ -2223,12 +2241,14 @@
22232241
]
22242242
},
22252243
{
2226-
"watchFilter": [
2227-
[
2228-
2
2244+
"watchFilter": {
2245+
"keys": [
2246+
"collection/1"
22292247
],
2230-
"collection/1"
2231-
]
2248+
"targetIds": [
2249+
2
2250+
]
2251+
}
22322252
},
22332253
{
22342254
"watchSnapshot": {

‎Firestore/Example/Tests/SpecTests/json/limbo_spec_test.json

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3409,11 +3409,13 @@
34093409
]
34103410
},
34113411
{
3412-
"watchFilter": [
3413-
[
3412+
"watchFilter": {
3413+
"keys": [
3414+
],
3415+
"targetIds": [
34143416
1
34153417
]
3416-
]
3418+
}
34173419
},
34183420
{
34193421
"watchCurrent": [
@@ -8081,15 +8083,16 @@
80818083
}
80828084
},
80838085
{
8084-
"watchFilter": [
8085-
[
8086-
2
8086+
"watchFilter": {
8087+
"keys": [
8088+
"collection/b1",
8089+
"collection/b2",
8090+
"collection/b3"
80878091
],
8088-
"collection/b1",
8089-
"collection/b2",
8090-
"collection/b3"
8091-
]
8092-
8092+
"targetIds": [
8093+
2
8094+
]
8095+
}
80938096
},
80948097
{
80958098
"watchSnapshot": {

‎Firestore/Example/Tests/SpecTests/json/limit_spec_test.json

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5464,12 +5464,14 @@
54645464
}
54655465
},
54665466
{
5467-
"watchFilter": [
5468-
[
5469-
2
5467+
"watchFilter": {
5468+
"keys": [
5469+
"collection/b"
54705470
],
5471-
"collection/b"
5472-
]
5471+
"targetIds": [
5472+
2
5473+
]
5474+
}
54735475
},
54745476
{
54755477
"watchSnapshot": {

‎Firestore/Protos/nanopb/google/firestore/v1/bloom_filter.nanopb.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const pb_field_t google_firestore_v1_BitSequence_fields[3] = {
4444
};
4545

4646
const pb_field_t google_firestore_v1_BloomFilter_fields[3] = {
47-
PB_FIELD( 1, MESSAGE , SINGULAR, STATIC , FIRST, google_firestore_v1_BloomFilter, bits, bits, &google_firestore_v1_BitSequence_fields),
47+
PB_FIELD( 1, MESSAGE , OPTIONAL, STATIC , FIRST, google_firestore_v1_BloomFilter, bits, bits, &google_firestore_v1_BitSequence_fields),
4848
PB_FIELD( 2, INT32 , SINGULAR, STATIC , OTHER, google_firestore_v1_BloomFilter, hash_count, bits, 0),
4949
PB_LAST_FIELD
5050
};
@@ -96,7 +96,9 @@ std::string google_firestore_v1_BloomFilter::ToString(int indent) const {
9696
std::string tostring_header = PrintHeader(indent, "BloomFilter", this);
9797
std::string tostring_result;
9898

99-
tostring_result += PrintMessageField("bits ", bits, indent + 1, false);
99+
if (has_bits) {
100+
tostring_result += PrintMessageField("bits ", bits, indent + 1, true);
101+
}
100102
tostring_result += PrintPrimitiveField("hash_count: ",
101103
hash_count, indent + 1, false);
102104

‎Firestore/Protos/nanopb/google/firestore/v1/bloom_filter.nanopb.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ typedef struct _google_firestore_v1_BitSequence {
4242
} google_firestore_v1_BitSequence;
4343

4444
typedef struct _google_firestore_v1_BloomFilter {
45+
bool has_bits;
4546
google_firestore_v1_BitSequence bits;
4647
int32_t hash_count;
4748

@@ -53,9 +54,9 @@ typedef struct _google_firestore_v1_BloomFilter {
5354

5455
/* Initializer values for message structs */
5556
#define google_firestore_v1_BitSequence_init_default {NULL, 0}
56-
#define google_firestore_v1_BloomFilter_init_default {google_firestore_v1_BitSequence_init_default, 0}
57+
#define google_firestore_v1_BloomFilter_init_default {false, google_firestore_v1_BitSequence_init_default, 0}
5758
#define google_firestore_v1_BitSequence_init_zero {NULL, 0}
58-
#define google_firestore_v1_BloomFilter_init_zero {google_firestore_v1_BitSequence_init_zero, 0}
59+
#define google_firestore_v1_BloomFilter_init_zero {false, google_firestore_v1_BitSequence_init_zero, 0}
5960

6061
/* Field tags (for use in manual encoding/decoding) */
6162
#define google_firestore_v1_BitSequence_bitmap_tag 1

‎Firestore/Protos/nanopb/google/firestore/v1/write.nanopb.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ const pb_field_t google_firestore_v1_DocumentRemove_fields[4] = {
9595
const pb_field_t google_firestore_v1_ExistenceFilter_fields[4] = {
9696
PB_FIELD( 1, INT32 , SINGULAR, STATIC , FIRST, google_firestore_v1_ExistenceFilter, target_id, target_id, 0),
9797
PB_FIELD( 2, INT32 , SINGULAR, STATIC , OTHER, google_firestore_v1_ExistenceFilter, count, target_id, 0),
98-
PB_FIELD( 3, MESSAGE , SINGULAR, STATIC , OTHER, google_firestore_v1_ExistenceFilter, unchanged_names, count, &google_firestore_v1_BloomFilter_fields),
98+
PB_FIELD( 3, MESSAGE , OPTIONAL, STATIC , OTHER, google_firestore_v1_ExistenceFilter, unchanged_names, count, &google_firestore_v1_BloomFilter_fields),
9999
PB_LAST_FIELD
100100
};
101101

@@ -317,8 +317,10 @@ std::string google_firestore_v1_ExistenceFilter::ToString(int indent) const {
317317
target_id, indent + 1, false);
318318
tostring_result += PrintPrimitiveField("count: ",
319319
count, indent + 1, false);
320-
tostring_result += PrintMessageField("unchanged_names ",
321-
unchanged_names, indent + 1, false);
320+
if (has_unchanged_names) {
321+
tostring_result += PrintMessageField("unchanged_names ",
322+
unchanged_names, indent + 1, true);
323+
}
322324

323325
std::string tostring_tail = PrintTail(indent);
324326
return tostring_header + tostring_result + tostring_tail;

0 commit comments

Comments
 (0)