Skip to content

Commit fe45beb

Browse files
Update Query code to use Protobuf (#8070)
1 parent 20e3266 commit fe45beb

File tree

6 files changed

+61
-54
lines changed

6 files changed

+61
-54
lines changed

Firestore/core/src/core/bound.cc

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,43 +21,51 @@
2121
#include "Firestore/core/src/core/order_by.h"
2222
#include "Firestore/core/src/immutable/append_only_list.h"
2323
#include "Firestore/core/src/model/document.h"
24+
#include "Firestore/core/src/model/document_key.h"
25+
#include "Firestore/core/src/model/value_util.h"
26+
#include "Firestore/core/src/nanopb/nanopb_util.h"
2427
#include "Firestore/core/src/util/hashing.h"
2528
#include "Firestore/core/src/util/to_string.h"
2629

2730
namespace firebase {
2831
namespace firestore {
2932
namespace core {
3033

34+
using model::CanonicalId;
35+
using model::Compare;
36+
using model::DocumentKey;
3137
using model::FieldPath;
32-
using model::FieldValue;
38+
using model::GetTypeOrder;
39+
using model::TypeOrder;
3340
using util::ComparisonResult;
3441

3542
bool Bound::SortsBeforeDocument(const OrderByList& order_by,
3643
const model::Document& document) const {
37-
HARD_ASSERT(position_.size() <= order_by.size(),
44+
HARD_ASSERT(position_->values_count <= order_by.size(),
3845
"Bound has more components than the provided order by.");
3946

4047
ComparisonResult result = ComparisonResult::Same;
41-
for (size_t idx = 0; idx < position_.size(); ++idx) {
42-
const FieldValue& field_value = position_[idx];
48+
for (size_t idx = 0; idx < position_->values_count; ++idx) {
49+
const google_firestore_v1_Value& field_value = position_->values[idx];
4350
const OrderBy& ordering_component = order_by[idx];
4451

4552
ComparisonResult comparison;
4653
if (ordering_component.field() == FieldPath::KeyFieldPath()) {
4754
HARD_ASSERT(
48-
field_value.type() == FieldValue::Type::Reference,
55+
GetTypeOrder(field_value) == TypeOrder ::kReference,
4956
"Bound has a non-key value where the key path is being used %s",
5057
field_value.ToString());
51-
const auto& ref = field_value.reference_value();
52-
comparison = ref.key().CompareTo(document.key());
58+
auto key = DocumentKey::FromName(
59+
nanopb::MakeString(field_value.reference_value));
60+
comparison = key.CompareTo(document->key());
5361

5462
} else {
55-
absl::optional<FieldValue> doc_value =
56-
document.field(ordering_component.field());
63+
absl::optional<google_firestore_v1_Value> doc_value =
64+
document->field(ordering_component.field());
5765
HARD_ASSERT(
5866
doc_value.has_value(),
5967
"Field should exist since document matched the orderBy already.");
60-
comparison = field_value.CompareTo(*doc_value);
68+
comparison = Compare(field_value, *doc_value);
6169
}
6270

6371
comparison = ordering_component.direction().ApplyTo(comparison);
@@ -73,15 +81,15 @@ bool Bound::SortsBeforeDocument(const OrderByList& order_by,
7381

7482
std::string Bound::CanonicalId() const {
7583
std::string result = before_ ? "b:" : "a:";
76-
for (const FieldValue& component : position_) {
77-
result.append(component.ToString());
84+
for (pb_size_t i = 0; i < position_->values_count; ++i) {
85+
result.append(CanonicalId(position_->values[i]));
7886
}
7987
return result;
8088
}
8189

8290
std::string Bound::ToString() const {
8391
return util::StringFormat("Bound(position=%s, before=%s)",
84-
util::ToString(position_), util::ToString(before_));
92+
CanonicalId(*position_), util::ToString(before_));
8593
}
8694

8795
std::ostream& operator<<(std::ostream& os, const Bound& bound) {
@@ -93,7 +101,7 @@ bool operator==(const Bound& lhs, const Bound& rhs) {
93101
}
94102

95103
size_t Bound::Hash() const {
96-
return util::Hash(position_, before_);
104+
return util::Hash(model::CanonicalId(*position_), before_);
97105
}
98106

99107
} // namespace core

Firestore/core/src/core/bound.h

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@
1818
#define FIRESTORE_CORE_SRC_CORE_BOUND_H_
1919

2020
#include <iosfwd>
21+
#include <memory>
2122
#include <string>
2223
#include <utility>
23-
#include <vector>
2424

25+
#include "Firestore/Protos/nanopb/google/firestore/v1/document.nanopb.h"
2526
#include "Firestore/core/src/core/core_fwd.h"
26-
#include "Firestore/core/src/model/field_value.h"
2727
#include "Firestore/core/src/model/model_fwd.h"
28+
#include "Firestore/core/src/nanopb/message.h"
2829

2930
namespace firebase {
3031
namespace firestore {
@@ -53,15 +54,15 @@ class Bound {
5354
* @param is_before Whether this bound is just before or just after the
5455
* position.
5556
*/
56-
Bound(std::vector<model::FieldValue> position, bool is_before)
57-
: position_(std::move(position)), before_(is_before) {
57+
Bound(google_firestore_v1_ArrayValue position, bool is_before)
58+
: position_{position}, before_(is_before) {
5859
}
5960

6061
/**
6162
* The index position of this bound represented as an array of field values.
6263
*/
63-
const std::vector<model::FieldValue>& position() const {
64-
return position_;
64+
const google_firestore_v1_ArrayValue& position() const {
65+
return *position_;
6566
}
6667

6768
/** Whether this bound is just before or just after the provided position */
@@ -83,7 +84,7 @@ class Bound {
8384
size_t Hash() const;
8485

8586
private:
86-
std::vector<model::FieldValue> position_;
87+
nanopb::SharedMessage<google_firestore_v1_ArrayValue> position_;
8788
bool before_;
8889
};
8990

Firestore/core/src/core/order_by.cc

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
#include <ostream>
2020

2121
#include "Firestore/core/src/model/document.h"
22-
#include "Firestore/core/src/model/field_value.h"
22+
#include "Firestore/core/src/model/value_util.h"
2323
#include "Firestore/core/src/util/string_format.h"
2424
#include "absl/strings/str_cat.h"
2525

@@ -29,20 +29,19 @@ namespace core {
2929

3030
using model::Document;
3131
using model::FieldPath;
32-
using model::FieldValue;
3332
using util::ComparisonResult;
3433

3534
ComparisonResult OrderBy::Compare(const Document& lhs,
3635
const Document& rhs) const {
3736
ComparisonResult result;
3837
if (field_ == FieldPath::KeyFieldPath()) {
39-
result = lhs.key().CompareTo(rhs.key());
38+
result = lhs->key().CompareTo(rhs->key());
4039
} else {
41-
absl::optional<FieldValue> value1 = lhs.field(field_);
42-
absl::optional<FieldValue> value2 = rhs.field(field_);
40+
absl::optional<google_firestore_v1_Value> value1 = lhs->field(field_);
41+
absl::optional<google_firestore_v1_Value> value2 = rhs->field(field_);
4342
HARD_ASSERT(value1.has_value() && value2.has_value(),
4443
"Trying to compare documents on fields that don't exist.");
45-
result = value1->CompareTo(*value2);
44+
result = model::Compare(*value1, *value2);
4645
}
4746

4847
return direction_.ApplyTo(result);

Firestore/core/src/core/query.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,16 +221,16 @@ Query Query::AsCollectionQueryAtPath(ResourcePath path) const {
221221
// MARK: - Matching
222222

223223
bool Query::Matches(const Document& doc) const {
224-
return MatchesPathAndCollectionGroup(doc) && MatchesOrderBy(doc) &&
225-
MatchesFilters(doc) && MatchesBounds(doc);
224+
return doc->is_found_document() && MatchesPathAndCollectionGroup(doc) &&
225+
MatchesOrderBy(doc) && MatchesFilters(doc) && MatchesBounds(doc);
226226
}
227227

228228
bool Query::MatchesPathAndCollectionGroup(const Document& doc) const {
229-
const ResourcePath& doc_path = doc.key().path();
229+
const ResourcePath& doc_path = doc->key().path();
230230
if (collection_group_) {
231231
// NOTE: path_ is currently always empty since we don't expose Collection
232232
// Group queries rooted at a document path yet.
233-
return doc.key().HasCollectionId(*collection_group_) &&
233+
return doc->key().HasCollectionId(*collection_group_) &&
234234
path_.IsPrefixOf(doc_path);
235235
} else if (DocumentKey::IsDocumentKey(path_)) {
236236
// Exact match for document queries.
@@ -253,7 +253,7 @@ bool Query::MatchesOrderBy(const Document& doc) const {
253253
const FieldPath& field_path = order_by.field();
254254
// order by key always matches
255255
if (field_path != FieldPath::KeyFieldPath() &&
256-
doc.field(field_path) == absl::nullopt) {
256+
doc->field(field_path) == absl::nullopt) {
257257
return false;
258258
}
259259
}

Firestore/core/test/unit/core/query_test.cc

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@
2020

2121
#include "Firestore/core/src/core/bound.h"
2222
#include "Firestore/core/src/core/field_filter.h"
23-
#include "Firestore/core/src/model/document.h"
2423
#include "Firestore/core/src/model/document_set.h"
2524
#include "Firestore/core/src/model/field_path.h"
26-
#include "Firestore/core/src/model/field_value.h"
25+
#include "Firestore/core/src/model/mutable_document.h"
2726
#include "Firestore/core/src/model/resource_path.h"
2827
#include "Firestore/core/test/unit/testutil/testutil.h"
2928
#include "gmock/gmock.h"
@@ -34,10 +33,9 @@ namespace firestore {
3433
namespace core {
3534

3635
using firebase::firestore::util::ComparisonResult;
37-
using model::Document;
3836
using model::DocumentComparator;
3937
using model::FieldPath;
40-
using model::FieldValue;
38+
using model::MutableDocument;
4139
using model::ResourcePath;
4240

4341
using testing::AssertionResult;
@@ -465,12 +463,13 @@ TEST(QueryTest, FiltersBasedOnObjectValue) {
465463
* Checks that an ordered array of elements yields the correct pair-wise
466464
* comparison result for the supplied comparator.
467465
*/
468-
testing::AssertionResult CorrectComparisons(const std::vector<Document>& vector,
469-
const DocumentComparator& comp) {
466+
testing::AssertionResult CorrectComparisons(
467+
const std::vector<MutableDocument>& vector,
468+
const DocumentComparator& comp) {
470469
for (size_t i = 0; i < vector.size(); i++) {
471470
for (size_t j = 0; j < vector.size(); j++) {
472-
const Document& i_doc = vector[i];
473-
const Document& j_doc = vector[j];
471+
const MutableDocument& i_doc = vector[i];
472+
const MutableDocument& j_doc = vector[j];
474473
ComparisonResult expected = util::Compare(i, j);
475474
ComparisonResult actual = comp.Compare(i_doc, j_doc);
476475
if (actual != expected) {
@@ -487,7 +486,7 @@ TEST(QueryTest, SortsDocumentsInTheCorrectOrder) {
487486
auto query = testutil::Query("collection").AddingOrderBy(OrderBy("sort"));
488487

489488
// clang-format off
490-
std::vector<Document> docs = {
489+
std::vector<MutableDocument> docs = {
491490
Doc("collection/1", 0, Map("sort", nullptr)),
492491
Doc("collection/1", 0, Map("sort", false)),
493492
Doc("collection/1", 0, Map("sort", true)),
@@ -514,7 +513,7 @@ TEST(QueryTest, SortsDocumentsUsingMultipleFields) {
514513
.AddingOrderBy(OrderBy("sort2"));
515514

516515
// clang-format off
517-
std::vector<Document> docs = {
516+
std::vector<MutableDocument> docs = {
518517
Doc("collection/1", 0, Map("sort1", 1, "sort2", 1)),
519518
Doc("collection/1", 0, Map("sort1", 1, "sort2", 2)),
520519
Doc("collection/2", 0, Map("sort1", 1, "sort2", 2)), // by key
@@ -537,7 +536,7 @@ TEST(QueryTest, SortsDocumentsWithDescendingToo) {
537536
.AddingOrderBy(OrderBy("sort2", "desc"));
538537

539538
// clang-format off
540-
std::vector<Document> docs = {
539+
std::vector<MutableDocument> docs = {
541540
Doc("collection/1", 0, Map("sort1", 2, "sort2", 3)),
542541
Doc("collection/3", 0, Map("sort1", 2, "sort2", 2)),
543542
Doc("collection/2", 0, Map("sort1", 2, "sort2", 2)), // by key
@@ -770,7 +769,7 @@ TEST(QueryTest, CanonicalIDs) {
770769
filters = testutil::Query("coll").AddingFilter(
771770
Filter("a", "not-in", Array(1, 2, 3)));
772771
EXPECT_THAT(filters,
773-
HasCanonicalId("coll|f:anot-in[1, 2, 3]|ob:aasc__name__asc"));
772+
HasCanonicalId("coll|f:anot-in[1,2,3]|ob:aasc__name__asc"));
774773

775774
auto order_bys = testutil::Query("coll").AddingOrderBy(OrderBy("up", "asc"));
776775
EXPECT_THAT(order_bys, HasCanonicalId("coll|f:|ob:upasc__name__asc"));
@@ -783,12 +782,13 @@ TEST(QueryTest, CanonicalIDs) {
783782
auto limit = testutil::Query("coll").WithLimitToFirst(25);
784783
EXPECT_THAT(limit, HasCanonicalId("coll|f:|ob:__name__asc|l:25|lt:f"));
785784

786-
auto bounds =
787-
testutil::Query("airports")
788-
.AddingOrderBy(OrderBy("name", "asc"))
789-
.AddingOrderBy(OrderBy("score", "desc"))
790-
.StartingAt(Bound({Value("OAK"), Value(1000)}, /* is_before= */ true))
791-
.EndingAt(Bound({Value("SFO"), Value(2000)}, /* is_before= */ false));
785+
auto bounds = testutil::Query("airports")
786+
.AddingOrderBy(OrderBy("name", "asc"))
787+
.AddingOrderBy(OrderBy("score", "desc"))
788+
.StartingAt(Bound({Array("OAK", 1000)},
789+
/* is_before= */ true))
790+
.EndingAt(Bound({Array("SFO", 2000)},
791+
/* is_before= */ false));
792792
EXPECT_THAT(bounds, HasCanonicalId("airports|f:|ob:nameascscoredesc__name__"
793793
"desc|lb:b:OAK1000|ub:a:SFO2000"));
794794
}
@@ -809,10 +809,10 @@ TEST(QueryTest, MatchesAllDocuments) {
809809
query = base_query.WithLimitToFirst(1);
810810
EXPECT_FALSE(query.MatchesAllDocuments());
811811

812-
query = base_query.StartingAt(Bound({Value("SFO")}, true));
812+
query = base_query.StartingAt(Bound({Array("SFO")}, true));
813813
EXPECT_FALSE(query.MatchesAllDocuments());
814814

815-
query = base_query.StartingAt(Bound({Value("OAK")}, true));
815+
query = base_query.StartingAt(Bound({Array("OAK")}, true));
816816
EXPECT_FALSE(query.MatchesAllDocuments());
817817
}
818818

Firestore/core/test/unit/model/mutation_test.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,7 @@ TEST(MutationTest,
330330
TransformBaseDoc(base_data, transforms, expected);
331331
}
332332

333-
TEST(MutationTest,
334-
AppliesLocalArrayUnionTransformWithExistingElementsInOrder) {
333+
TEST(MutationTest, AppliesLocalArrayUnionTransformWithExistingElementsInOrder) {
335334
// New elements should be appended in order.
336335
auto base_data = Map("array", Array(1, 3));
337336
TransformPairs transforms = {{"array", ArrayUnion(1, 2, 3, 4, 5)}};

0 commit comments

Comments
 (0)