Port PrimitiveValueTest test for core::Query (#1530)

(From java.)

Also fix a bug uncovered by the test. (oops)

Author: rsgowman

Firestore/core/src/firebase/firestore/core/filter.cc

@@ -29,11 +29,11 @@ using model::FieldValue;
 
 std::shared_ptr<Filter> Filter::Create(FieldPath path,
                                        Operator op,
-                                       FieldValue value) {
+                                       FieldValue value_rhs) {
   // TODO(rsgowman): Java performs a number of checks here, and then invokes the
   // ctor of the relevant Filter subclass. Port those checks here.
   return std::make_shared<RelationFilter>(std::move(path), op,
-                                          std::move(value));
+                                          std::move(value_rhs));
 }
 
 }  // namespace core

Firestore/core/src/firebase/firestore/core/filter.h

@@ -48,7 +48,7 @@ class Filter {
    */
   static std::shared_ptr<Filter> Create(model::FieldPath path,
                                         Operator op,
-                                        model::FieldValue value);
+                                        model::FieldValue value_rhs);
 
   virtual ~Filter() {
   }

Firestore/core/src/firebase/firestore/core/relation_filter.cc

@@ -27,8 +27,10 @@ namespace core {
 using model::FieldPath;
 using model::FieldValue;
 
-RelationFilter::RelationFilter(FieldPath field, Operator op, FieldValue value)
-    : field_(std::move(field)), op_(op), value_(std::move(value)) {
+RelationFilter::RelationFilter(FieldPath field,
+                               Operator op,
+                               FieldValue value_rhs)
+    : field_(std::move(field)), op_(op), value_rhs_(std::move(value_rhs)) {
 }
 
 const FieldPath& RelationFilter::field() const {
@@ -47,22 +49,22 @@ bool RelationFilter::Matches(const model::Document& doc) const {
 
 bool RelationFilter::MatchesValue(const FieldValue& other) const {
   // Only compare types with matching backend order (such as double and int).
-  return FieldValue::Comparable(value_.type(), other.type()) &&
+  return FieldValue::Comparable(other.type(), value_rhs_.type()) &&
          MatchesComparison(other);
 }
 
 bool RelationFilter::MatchesComparison(const FieldValue& other) const {
   switch (op_) {
     case Operator::LessThan:
-      return value_ < other;
+      return other < value_rhs_;
     case Operator::LessThanOrEqual:
-      return value_ <= other;
+      return other <= value_rhs_;
     case Operator::Equal:
-      return value_ == other;
+      return other == value_rhs_;
     case Operator::GreaterThan:
-      return value_ > other;
+      return other > value_rhs_;
     case Operator::GreaterThanOrEqual:
-      return value_ >= other;
+      return other >= value_rhs_;
   }
   UNREACHABLE();
 }

Firestore/core/src/firebase/firestore/core/relation_filter.h

@@ -35,7 +35,9 @@ class RelationFilter : public Filter {
    * Creates a new filter that compares fields and values. Only intended to be
    * called from Filter::Create().
    */
-  RelationFilter(model::FieldPath field, Operator op, model::FieldValue value);
+  RelationFilter(model::FieldPath field,
+                 Operator op,
+                 model::FieldValue value_rhs);
 
   const model::FieldPath& field() const override;
 
@@ -49,7 +51,7 @@ class RelationFilter : public Filter {
 
   const model::FieldPath field_;
   const Operator op_;
-  const model::FieldValue value_;
+  const model::FieldValue value_rhs_;
 };
 
 }  // namespace core

Firestore/core/test/firebase/firestore/core/query_test.cc

@@ -29,6 +29,7 @@ namespace core {
 
 using model::Document;
 using model::FieldValue;
+using model::ResourcePath;
 using testutil::Doc;
 using testutil::Filter;
 
@@ -67,6 +68,35 @@ TEST(QueryTest, EmptyFieldsAreAllowedForQueries) {
   EXPECT_FALSE(query.Matches(doc2));
 }
 
+TEST(QueryTest, PrimitiveValueFilter) {
+  Query query1 = Query::AtPath(ResourcePath::FromString("collection"))
+                     .Filter(Filter("sort", ">=", 2));
+  Query query2 = Query::AtPath(ResourcePath::FromString("collection"))
+                     .Filter(Filter("sort", "<=", 2));
+
+  Document doc1 =
+      Doc("collection/1", 0, {{"sort", FieldValue::IntegerValue(1)}});
+  Document doc2 =
+      Doc("collection/2", 0, {{"sort", FieldValue::IntegerValue(2)}});
+  Document doc3 =
+      Doc("collection/3", 0, {{"sort", FieldValue::IntegerValue(3)}});
+  Document doc4 = Doc("collection/4", 0, {{"sort", FieldValue::FalseValue()}});
+  Document doc5 =
+      Doc("collection/5", 0, {{"sort", FieldValue::StringValue("string")}});
+
+  EXPECT_FALSE(query1.Matches(doc1));
+  EXPECT_TRUE(query1.Matches(doc2));
+  EXPECT_TRUE(query1.Matches(doc3));
+  EXPECT_FALSE(query1.Matches(doc4));
+  EXPECT_FALSE(query1.Matches(doc5));
+
+  EXPECT_TRUE(query2.Matches(doc1));
+  EXPECT_TRUE(query2.Matches(doc2));
+  EXPECT_FALSE(query2.Matches(doc3));
+  EXPECT_FALSE(query2.Matches(doc4));
+  EXPECT_FALSE(query2.Matches(doc5));
+}
+
 }  // namespace core
 }  // namespace firestore
 }  // namespace firebase

Firestore/core/test/firebase/firestore/testutil/testutil.h

@@ -105,6 +105,12 @@ inline std::shared_ptr<core::Filter> Filter(absl::string_view key,
   return Filter(key, op, model::FieldValue::StringValue(value));
 }
 
+inline std::shared_ptr<core::Filter> Filter(absl::string_view key,
+                                            absl::string_view op,
+                                            int value) {
+  return Filter(key, op, model::FieldValue::IntegerValue(value));
+}
+
 // Add a non-inline function to make this library buildable.
 // TODO(zxu123): remove once there is non-inline function.
 void dummy();