Migrate null, numbers, and GeoPoints to FSTDelegateValue (#3044)

* Migrate FSTNullValue to FSTDelegateValue
* Migrate FSTIntegerValue to FSTDelegateValue
* Migrate FSTDoubleValue to FSTDelegateValue
* Migrate FSTGeoPointValue to FSTDelegateValue

Author: wilhuff

Firestore/Example/Tests/API/FSTUserDataConverterTests.mm

@@ -41,7 +41,7 @@ - (void)testConvertsIntegers {
   ];
   for (id value in values) {
     FSTFieldValue *wrapped = FSTTestFieldValue(value);
-    XCTAssertEqualObjects([wrapped class], [FSTIntegerValue class]);
+    XCTAssertEqualObjects([wrapped class], [FSTDelegateValue class]);
     XCTAssertEqualObjects([wrapped value], @([value longLongValue]));
     XCTAssertEqual(wrapped.type, FieldValue::Type::Integer);
   }
@@ -56,17 +56,17 @@ - (void)testConvertsDoubles {
   ];
   for (id value in values) {
     FSTFieldValue *wrapped = FSTTestFieldValue(value);
-    XCTAssertEqualObjects([wrapped class], [FSTDoubleValue class]);
+    XCTAssertEqualObjects([wrapped class], [FSTDelegateValue class]);
     XCTAssertEqualObjects([wrapped value], value);
     XCTAssertEqual(wrapped.type, FieldValue::Type::Double);
   }
 }
 
 - (void)testConvertsNilAndNSNull {
-  FSTNullValue *nullValue = [FSTNullValue nullValue];
-  XCTAssertEqual(FSTTestFieldValue(nil), nullValue);
-  XCTAssertEqual(FSTTestFieldValue([NSNull null]), nullValue);
-  XCTAssertEqual([nullValue value], [NSNull null]);
+  FSTFieldValue *nullValue = FieldValue::Null().Wrap();
+  XCTAssertEqualObjects(FSTTestFieldValue(nil), nullValue);
+  XCTAssertEqualObjects(FSTTestFieldValue([NSNull null]), nullValue);
+  XCTAssertEqualObjects([nullValue value], [NSNull null]);
   XCTAssertEqual(nullValue.type, FieldValue::Type::Null);
 }
 
@@ -87,7 +87,7 @@ - (void)testConvertsUnsignedCharToInteger {
   // choose, and it's more useful to support shorts as Integers than it is to treat unsigned char as
   // Boolean.
   FSTFieldValue *wrapped = FSTTestFieldValue([NSNumber numberWithUnsignedChar:1]);
-  XCTAssertEqualObjects(wrapped, [FSTIntegerValue integerValue:1]);
+  XCTAssertEqualObjects(wrapped, FieldValue::FromInteger(1).Wrap());
 }
 
 union DoubleBits {
@@ -121,7 +121,7 @@ - (void)testConvertsGeoPoints {
 
   for (id value in values) {
     FSTFieldValue *wrapped = FSTTestFieldValue(value);
-    XCTAssertEqualObjects([wrapped class], [FSTGeoPointValue class]);
+    XCTAssertEqualObjects([wrapped class], [FSTDelegateValue class]);
     XCTAssertEqualObjects([wrapped value], value);
     XCTAssertEqual(wrapped.type, FieldValue::Type::GeoPoint);
   }
@@ -161,9 +161,9 @@ - (void)testConvertsSimpleObjects {
       FSTTestObjectValue(@{@"a" : @"foo", @"b" : @(1L), @"c" : @YES, @"d" : [NSNull null]});
   FSTObjectValue *expected = [[FSTObjectValue alloc] initWithDictionary:@{
     @"a" : FieldValue::FromString("foo").Wrap(),
-    @"b" : [FSTIntegerValue integerValue:1LL],
+    @"b" : FieldValue::FromInteger(1).Wrap(),
     @"c" : FieldValue::True().Wrap(),
-    @"d" : [FSTNullValue nullValue]
+    @"d" : FieldValue::Null().Wrap()
   }];
   XCTAssertEqualObjects(actual, expected);
   XCTAssertEqual(actual.type, FieldValue::Type::Object);

Firestore/Example/Tests/Core/FSTViewTests.mm

@@ -44,6 +44,7 @@
 using firebase::firestore::model::ResourcePath;
 using firebase::firestore::model::DocumentKeySet;
 using firebase::firestore::model::DocumentSet;
+using firebase::firestore::model::FieldValue;
 using testing::ElementsAre;
 
 NS_ASSUME_NONNULL_BEGIN
@@ -173,7 +174,7 @@ - (void)testFiltersDocumentsBasedOnQueryWithFilter {
   FSTQuery *query = [self queryForMessages];
   FSTRelationFilter *filter = [FSTRelationFilter filterWithField:testutil::Field("sort")
                                                   filterOperator:Filter::Operator::LessThanOrEqual
-                                                           value:[FSTDoubleValue doubleValue:2]];
+                                                           value:FieldValue::FromDouble(2).Wrap()];
   query = [query queryByAddingFilter:filter];
 
   FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
@@ -211,7 +212,7 @@ - (void)testUpdatesDocumentsBasedOnQueryWithFilter {
   FSTQuery *query = [self queryForMessages];
   FSTRelationFilter *filter = [FSTRelationFilter filterWithField:testutil::Field("sort")
                                                   filterOperator:Filter::Operator::LessThanOrEqual
-                                                           value:[FSTDoubleValue doubleValue:2]];
+                                                           value:FieldValue::FromDouble(2).Wrap()];
   query = [query queryByAddingFilter:filter];
 
   FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];

Firestore/Example/Tests/Model/FSTFieldValueTests.mm

@@ -26,13 +26,15 @@
 #import "Firestore/Example/Tests/API/FSTAPIHelpers.h"
 #import "Firestore/Example/Tests/Util/FSTHelpers.h"
 
+#include "Firestore/core/include/firebase/firestore/geo_point.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/model/field_value.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 #include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
 
 namespace testutil = firebase::firestore::testutil;
 namespace util = firebase::firestore::util;
+using firebase::firestore::GeoPoint;
 using firebase::firestore::model::DatabaseId;
 using firebase::firestore::model::FieldValue;
 
@@ -123,22 +125,22 @@ - (void)testNormalizesNaNs {
   // Ensure we get the same normalization behavior (currently implemented explicitly by checking
   // for isnan() and then explicitly assigning NAN).
   union DoubleBits result;
-  result.d = [[FSTDoubleValue doubleValue:canonical.d] internalValue];
+  result.d = FieldValue::FromDouble(canonical.d).double_value();
   XCTAssertEqual(result.bits, canonical.bits);
 
-  result.d = [[FSTDoubleValue doubleValue:alternate.d] internalValue];
+  result.d = FieldValue::FromDouble(alternate.d).double_value();
   XCTAssertEqual(result.bits, canonical.bits);
 
   // A NaN that's canonical except it has the sign bit set (would be negative if signs mattered)
   union DoubleBits negative = {.bits = 0xfff8000000000000ULL};
-  result.d = [[FSTDoubleValue doubleValue:negative.d] internalValue];
+  result.d = FieldValue::FromDouble(negative.d).double_value();
   XCTAssertTrue(isnan(negative.d));
   XCTAssertEqual(result.bits, canonical.bits);
 
   // A signaling NaN with significand where MSB is 0, and some non-MSB bit is one.
   union DoubleBits signaling = {.bits = 0xfff4000000000000ULL};
   XCTAssertTrue(isnan(signaling.d));
-  result.d = [[FSTDoubleValue doubleValue:signaling.d] internalValue];
+  result.d = FieldValue::FromDouble(signaling.d).double_value();
   XCTAssertEqual(result.bits, canonical.bits);
 }
 
@@ -157,15 +159,15 @@ - (void)testZeros {
   XCTAssertEqual(normalized.bits, negativeZero.bits);
   XCTAssertEqualObjects([NSNumber numberWithDouble:0.0], [NSNumber numberWithDouble:-0.0]);
 
-  // FSTDoubleValue preserves positive/negative zero
+  // FieldValue::FromDouble preserves positive/negative zero
   union DoubleBits result;
-  result.d = [[[FSTDoubleValue doubleValue:zero.d] value] doubleValue];
+  result.d = FieldValue::FromDouble(zero.d).double_value();
   XCTAssertEqual(result.bits, zero.bits);
-  result.d = [[[FSTDoubleValue doubleValue:negativeZero.d] value] doubleValue];
+  result.d = FieldValue::FromDouble(negativeZero.d).double_value();
   XCTAssertEqual(result.bits, negativeZero.bits);
 
   // ... but compares positive/negative zero as unequal, compatibly with Firestore.
-  XCTAssertNotEqualObjects([FSTDoubleValue doubleValue:0.0], [FSTDoubleValue doubleValue:-0.0]);
+  XCTAssertNotEqual(FieldValue::FromDouble(0.0), FieldValue::FromDouble(-0.0));
 }
 
 - (void)testExtractsFields {
@@ -288,14 +290,14 @@ - (void)testValueEquality {
   NSArray *groups = @[
     @[ FSTTestFieldValue(@YES), FieldValue::True().Wrap() ],
     @[ FSTTestFieldValue(@NO), FieldValue::False().Wrap() ],
-    @[ FSTTestFieldValue([NSNull null]), [FSTNullValue nullValue] ],
-    @[ FSTTestFieldValue(@(0.0 / 0.0)), FSTTestFieldValue(@(NAN)), [FSTDoubleValue nanValue] ],
+    @[ FSTTestFieldValue([NSNull null]), FieldValue::Null().Wrap() ],
+    @[ FSTTestFieldValue(@(0.0 / 0.0)), FSTTestFieldValue(@(NAN)), FieldValue::Nan().Wrap() ],
     // -0.0 and 0.0 compare: the same (but are not isEqual:)
     @[ FSTTestFieldValue(@(-0.0)) ], @[ FSTTestFieldValue(@0.0) ],
-    @[ FSTTestFieldValue(@1), FSTTestFieldValue(@1LL), [FSTIntegerValue integerValue:1LL] ],
+    @[ FSTTestFieldValue(@1), FSTTestFieldValue(@1LL), FieldValue::FromInteger(1LL).Wrap() ],
     // double and unit64_t values can compare: the same (but won't be isEqual:)
-    @[ FSTTestFieldValue(@1.0), [FSTDoubleValue doubleValue:1.0] ],
-    @[ FSTTestFieldValue(@1.1), [FSTDoubleValue doubleValue:1.1] ],
+    @[ FSTTestFieldValue(@1.0), FieldValue::FromDouble(1.0).Wrap() ],
+    @[ FSTTestFieldValue(@1.1), FieldValue::FromDouble(1.1).Wrap() ],
     @[
       FSTTestFieldValue(FSTTestData(0, 1, 2, -1)), [FSTBlobValue blobValue:FSTTestData(0, 1, 2, -1)]
     ],
@@ -323,7 +325,7 @@ - (void)testValueEquality {
                                  previousValue:nil] ],
     @[
       FSTTestFieldValue(FSTTestGeoPoint(0, 1)),
-      [FSTGeoPointValue geoPointValue:FSTTestGeoPoint(0, 1)]
+      FieldValue::FromGeoPoint(GeoPoint(0, 1)).Wrap()
     ],
     @[ FSTTestFieldValue(FSTTestGeoPoint(1, 0)) ],
     @[

Firestore/Example/Tests/Model/FSTMutationTests.mm

@@ -31,6 +31,7 @@
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
 #include "Firestore/core/src/firebase/firestore/model/field_mask.h"
 #include "Firestore/core/src/firebase/firestore/model/field_transform.h"
+#include "Firestore/core/src/firebase/firestore/model/field_value.h"
 #include "Firestore/core/src/firebase/firestore/model/precondition.h"
 #include "Firestore/core/src/firebase/firestore/model/transform_operations.h"
 #include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
@@ -41,6 +42,7 @@
 using firebase::firestore::model::FieldMask;
 using firebase::firestore::model::FieldPath;
 using firebase::firestore::model::FieldTransform;
+using firebase::firestore::model::FieldValue;
 using firebase::firestore::model::Precondition;
 using firebase::firestore::model::TransformOperation;
 
@@ -420,7 +422,7 @@ - (void)testAppliesServerAckedIncrementTransformToDocuments {
 
   FSTMutationResult *mutationResult =
       [[FSTMutationResult alloc] initWithVersion:testutil::Version(1)
-                                transformResults:@[ [FSTIntegerValue integerValue:3] ]];
+                                transformResults:@[ FieldValue::FromInteger(3).Wrap() ]];
 
   FSTMaybeDocument *transformedDoc = [transform applyToRemoteDocument:baseDoc
                                                        mutationResult:mutationResult];
@@ -461,7 +463,7 @@ - (void)testAppliesServerAckedArrayTransformsToDocuments {
   // Server just sends null transform results for array operations.
   FSTMutationResult *mutationResult = [[FSTMutationResult alloc]
        initWithVersion:testutil::Version(1)
-      transformResults:@[ [FSTNullValue nullValue], [FSTNullValue nullValue] ]];
+      transformResults:@[ FieldValue::Null().Wrap(), FieldValue::Null().Wrap() ]];
 
   FSTMaybeDocument *transformedDoc = [transform applyToRemoteDocument:baseDoc
                                                        mutationResult:mutationResult];

Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm

@@ -145,7 +145,7 @@ - (void)setUp {
 }
 
 - (void)testEncodesNull {
-  FSTFieldValue *model = [FSTNullValue nullValue];
+  FSTFieldValue *model = FieldValue::Null().Wrap();
 
   GCFSValue *proto = [GCFSValue message];
   proto.nullValue = GPBNullValue_NullValue;

Firestore/Source/API/FIRGeoPoint+Internal.h

@@ -18,9 +18,21 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+namespace firebase {
+namespace firestore {
+class GeoPoint;
+}  // namespace firestore
+}  // namespace firebase
+
+namespace firestore = firebase::firestore;
+
 /** Internal FIRGeoPoint API we don't want exposed in our public header files. */
 @interface FIRGeoPoint (Internal)
+
 - (NSComparisonResult)compare:(FIRGeoPoint *)other;
+
+- (firestore::GeoPoint)toGeoPoint;
+
 @end
 
 NS_ASSUME_NONNULL_END

Firestore/Source/API/FIRGeoPoint.mm

@@ -16,6 +16,7 @@
 
 #import "Firestore/Source/API/FIRGeoPoint+Internal.h"
 
+#include "Firestore/core/include/firebase/firestore/geo_point.h"
 #include "Firestore/core/src/firebase/firestore/api/input_validation.h"
 #include "Firestore/core/src/firebase/firestore/util/comparison.h"
 
@@ -85,4 +86,12 @@ - (id)copyWithZone:(NSZone *_Nullable)zone {
 
 @end
 
+@implementation FIRGeoPoint (Internal)
+
+- (firestore::GeoPoint)toGeoPoint {
+  return firestore::GeoPoint(self.latitude, self.longitude);
+}
+
+@end
+
 NS_ASSUME_NONNULL_END

Firestore/Source/API/FSTUserDataConverter.mm

@@ -29,6 +29,7 @@
 #import "Firestore/Source/API/FIRFieldPath+Internal.h"
 #import "Firestore/Source/API/FIRFieldValue+Internal.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
+#import "Firestore/Source/API/FIRGeoPoint+Internal.h"
 #import "Firestore/Source/Model/FSTFieldValue.h"
 #import "Firestore/Source/Model/FSTMutation.h"
 
@@ -289,7 +290,7 @@ - (FSTFieldValue *)parseArray:(NSArray *)array context:(ParseContext &&)context
     FSTFieldValue *_Nullable parsedEntry = [self parseData:entry context:context.ChildContext(idx)];
     if (!parsedEntry) {
       // Just include nulls in the array for fields being replaced with a sentinel.
-      parsedEntry = [FSTNullValue nullValue];
+      parsedEntry = FieldValue::Null().Wrap();
     }
     [result addObject:parsedEntry];
   }];
@@ -350,8 +351,7 @@ - (void)parseSentinelFieldValue:(FIRFieldValue *)fieldValue context:(ParseContex
   } else if ([fieldValue isKindOfClass:[FSTNumericIncrementFieldValue class]]) {
     FSTNumericIncrementFieldValue *numericIncrementFieldValue =
         (FSTNumericIncrementFieldValue *)fieldValue;
-    FSTNumberValue *operand =
-        (FSTNumberValue *)[self parsedQueryValue:numericIncrementFieldValue.operand];
+    FSTFieldValue *operand = [self parsedQueryValue:numericIncrementFieldValue.operand];
     auto numeric_increment = absl::make_unique<NumericIncrementTransform>(operand);
 
     context.AddToFieldTransforms(*context.path(), std::move(numeric_increment));
@@ -373,7 +373,7 @@ - (void)parseSentinelFieldValue:(FIRFieldValue *)fieldValue context:(ParseContex
  */
 - (nullable FSTFieldValue *)parseScalarValue:(nullable id)input context:(ParseContext &&)context {
   if (!input || [input isMemberOfClass:[NSNull class]]) {
-    return [FSTNullValue nullValue];
+    return FieldValue::Null().Wrap();
 
   } else if ([input isKindOfClass:[NSNumber class]]) {
     // Recover the underlying type of the number, using the method described here:
@@ -385,7 +385,7 @@ - (nullable FSTFieldValue *)parseScalarValue:(nullable id)input context:(ParseCo
     // Articles/ocrtTypeEncodings.html
     switch (cType[0]) {
       case 'q':
-        return [FSTIntegerValue integerValue:[input longLongValue]];
+        return FieldValue::FromInteger([input longLongValue]).Wrap();
 
       case 'i':  // Falls through.
       case 's':  // Falls through.
@@ -394,7 +394,7 @@ - (nullable FSTFieldValue *)parseScalarValue:(nullable id)input context:(ParseCo
       case 'S':
         // Coerce integer values that aren't long long. Allow unsigned integer types that are
         // guaranteed small enough to skip a length check.
-        return [FSTIntegerValue integerValue:[input longLongValue]];
+        return FieldValue::FromInteger([input longLongValue]).Wrap();
 
       case 'L':  // Falls through.
       case 'Q':
@@ -408,19 +408,19 @@ - (nullable FSTFieldValue *)parseScalarValue:(nullable id)input context:(ParseCo
                                  context.FieldDescription());
 
           } else {
-            return [FSTIntegerValue integerValue:(int64_t)extended];
+            return FieldValue::FromInteger(static_cast<int64_t>(extended)).Wrap();
           }
         }
 
       case 'f':
-        return [FSTDoubleValue doubleValue:[input doubleValue]];
+        return FieldValue::FromDouble([input doubleValue]).Wrap();
 
       case 'd':
         // Double values are already the right type, so just reuse the existing boxed double.
         //
         // Note that NSNumber already performs NaN normalization to a single shared instance
         // so there's no need to treat NaN specially here.
-        return [FSTDoubleValue doubleValue:[input doubleValue]];
+        return FieldValue::FromDouble([input doubleValue]).Wrap();
 
       case 'B':  // Falls through.
       case 'c':  // Falls through.
@@ -453,7 +453,8 @@ - (nullable FSTFieldValue *)parseScalarValue:(nullable id)input context:(ParseCo
     return [FSTTimestampValue timestampValue:truncatedTimestamp];
 
   } else if ([input isKindOfClass:[FIRGeoPoint class]]) {
-    return [FSTGeoPointValue geoPointValue:input];
+    FIRGeoPoint *geoPoint = input;
+    return FieldValue::FromGeoPoint([geoPoint toGeoPoint]).Wrap();
 
   } else if ([input isKindOfClass:[NSData class]]) {
     return [FSTBlobValue blobValue:input];

Firestore/Source/Core/FSTQuery.h

@@ -36,9 +36,9 @@ NS_ASSUME_NONNULL_BEGIN
 /**
  * Creates a filter for the provided path, operator, and value.
  *
- * Note that if the relational operator is Filter::Operator::Equal and
- * the value is [FSTNullValue nullValue] or [FSTDoubleValue nanValue], this
- * will return the appropriate FSTNullFilter or FSTNanFilter class instead of a
+ * Note that if the relational operator is Filter::Operator::Equal and the
+ * value is FieldValue::Null() or FieldValue::Nan(), this will return the
+ * appropriate FSTNullFilter or FSTNanFilter class instead of a
  * FSTRelationFilter.
  */
 + (instancetype)filterWithField:(const model::FieldPath &)field

Firestore/Source/Core/FSTQuery.mm

@@ -76,12 +76,12 @@ @implementation FSTFilter
 + (instancetype)filterWithField:(const FieldPath &)field
                  filterOperator:(Filter::Operator)op
                           value:(FSTFieldValue *)value {
-  if ([value isEqual:[FSTNullValue nullValue]]) {
+  if (value.type == FieldValue::Type::Null) {
     if (op != Filter::Operator::Equal) {
       ThrowInvalidArgument("Invalid Query. Nil and NSNull only support equality comparisons.");
     }
     return [[FSTNullFilter alloc] initWithField:field];
-  } else if ([value isEqual:[FSTDoubleValue nanValue]]) {
+  } else if (value.isNAN) {
     if (op != Filter::Operator::Equal) {
       ThrowInvalidArgument("Invalid Query. NaN only supports equality comparisons.");
     }
@@ -268,7 +268,7 @@ - (instancetype)initWithField:(FieldPath)field {
 
 - (BOOL)matchesDocument:(FSTDocument *)document {
   FSTFieldValue *fieldValue = [document fieldForPath:self.field];
-  return fieldValue != nil && [fieldValue isEqual:[FSTNullValue nullValue]];
+  return fieldValue != nil && fieldValue.type == FieldValue::Type::Null;
 }
 
 - (NSString *)canonicalID {
@@ -314,7 +314,7 @@ - (instancetype)initWithField:(FieldPath)field {
 
 - (BOOL)matchesDocument:(FSTDocument *)document {
   FSTFieldValue *fieldValue = [document fieldForPath:self.field];
-  return fieldValue != nil && [fieldValue isEqual:[FSTDoubleValue nanValue]];
+  return fieldValue != nil && fieldValue.isNAN;
 }
 
 - (NSString *)canonicalID {