Make FieldFilter constructor protected

Author: wilhuff

Firestore/Example/Tests/Core/FSTViewTests.mm

@@ -50,6 +50,7 @@
 
 using testing::ElementsAre;
 using testutil::Field;
+using testutil::Filter;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -176,9 +177,7 @@ - (void)testDoesNotReturnNilForFirstChanges {
 
 - (void)testFiltersDocumentsBasedOnQueryWithFilter {
   FSTQuery *query = [self queryForMessages];
-  auto filter = std::make_shared<FieldFilter>(Field("sort"), Filter::Operator::LessThanOrEqual,
-                                              FieldValue::FromDouble(2));
-  query = [query queryByAddingFilter:filter];
+  query = [query queryByAddingFilter:Filter("sort", "<=", 2)];
 
   FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
   FSTDocument *doc1 =
@@ -213,9 +212,7 @@ - (void)testFiltersDocumentsBasedOnQueryWithFilter {
 
 - (void)testUpdatesDocumentsBasedOnQueryWithFilter {
   FSTQuery *query = [self queryForMessages];
-  auto filter = std::make_shared<FieldFilter>(Field("sort"), Filter::Operator::LessThanOrEqual,
-                                              FieldValue::FromDouble(2));
-  query = [query queryByAddingFilter:filter];
+  query = [query queryByAddingFilter:Filter("sort", "<=", 2)];
 
   FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
   FSTDocument *doc1 =

Firestore/Source/Remote/FSTSerializerBeta.mm

@@ -951,7 +951,7 @@ - (GCFSStructuredQuery_Filter *)encodedFieldFilter:(const FieldFilter &)filter {
   FieldPath fieldPath = FieldPath::FromServerFormat(util::MakeString(proto.field.fieldPath));
   Filter::Operator op = [self decodedFieldFilterOperator:proto.op];
   FieldValue value = [self decodedFieldValue:proto.value];
-  return std::make_shared<FieldFilter>(std::move(fieldPath), op, std::move(value));
+  return FieldFilter::Create(std::move(fieldPath), op, std::move(value));
 }
 
 - (GCFSStructuredQuery_Filter *)encodedUnaryFilter:(const Filter &)filter {

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

@@ -44,18 +44,6 @@ class FieldFilter : public Filter {
 
   FieldFilter() = default;
 
-  /**
-   * Creates a new filter that compares fields and values. Only intended to be
-   * called from Filter::Create().
-   *
-   * @param field A path to a field in the document to filter on. The LHS of the
-   *     expression.
-   * @param op The binary operator to apply.
-   * @param value_rhs A constant value to compare @a field to. The RHS of the
-   *     expression.
-   */
-  FieldFilter(model::FieldPath field, Operator op, model::FieldValue value_rhs);
-
   Type type() const override {
     return Type::kFieldFilter;
   }
@@ -79,11 +67,25 @@ class FieldFilter : public Filter {
 
   bool IsInequality() const override;
 
+ protected:
+  /**
+   * Creates a new filter that compares fields and values. Only intended to be
+   * called from Filter::Create().
+   *
+   * @param field A path to a field in the document to filter on. The LHS of the
+   *     expression.
+   * @param op The binary operator to apply.
+   * @param value_rhs A constant value to compare @a field to. The RHS of the
+   *     expression.
+   */
+  FieldFilter(model::FieldPath field, Operator op, model::FieldValue value_rhs);
+
+  bool MatchesComparison(util::ComparisonResult result) const;
+
  private:
   bool Equals(const Filter& other) const override;
 
   bool MatchesValue(const model::FieldValue& lhs) const;
-  bool MatchesComparison(util::ComparisonResult result) const;
 
   /** The left hand side of the relation. A path into a document field. */
   model::FieldPath field_;

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

@@ -34,21 +34,21 @@ using testutil::Value;
 using Operator = Filter::Operator;
 
 TEST(FilterTest, Equality) {
-  FieldFilter filter(Field("f"), Operator::Equal, Value(1));
-  EXPECT_EQ(filter, FieldFilter(Field("f"), Operator::Equal, Value(1)));
-  EXPECT_NE(filter, FieldFilter(Field("g"), Operator::Equal, Value(1)));
-  EXPECT_NE(filter, FieldFilter(Field("f"), Operator::GreaterThan, Value(1)));
-  EXPECT_NE(filter, FieldFilter(Field("f"), Operator::Equal, Value(2)));
-  EXPECT_NE(filter, NanFilter(Field("f")));
-  EXPECT_NE(filter, NullFilter(Field("f")));
-
-  NullFilter null_filter(Field("g"));
-  EXPECT_EQ(null_filter, NullFilter(Field("g")));
-  EXPECT_NE(null_filter, NullFilter(Field("h")));
-
-  NanFilter nan_filter(Field("g"));
-  EXPECT_EQ(nan_filter, NanFilter(Field("g")));
-  EXPECT_NE(nan_filter, NanFilter(Field("h")));
+  auto filter = Filter("f", "==", 1);
+  EXPECT_EQ(*filter, *Filter("f", "==", 1));
+  EXPECT_NE(*filter, *Filter("g", "==", 1));
+  EXPECT_NE(*filter, *Filter("f", ">", 1));
+  EXPECT_NE(*filter, *Filter("f", "==", 2));
+  EXPECT_NE(*filter, *Filter("f", "==", NAN));
+  EXPECT_NE(*filter, *Filter("f", "==", nullptr));
+
+  auto null_filter = Filter("g", "==", nullptr);
+  EXPECT_EQ(*null_filter, *Filter("g", "==", nullptr));
+  EXPECT_NE(*null_filter, *Filter("h", "==", nullptr));
+
+  auto nan_filter = Filter("g", "==", NAN);
+  EXPECT_EQ(*nan_filter, *Filter("g", "==", NAN));
+  EXPECT_NE(*nan_filter, *Filter("h", "==", NAN));
 }
 
 }  // namespace core