diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index 1c81bade95f..787768352ef 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,4 +1,5 @@ # 12.13.0 +- [feature] Added support for `minimum` and `maximum` FieldValue operations. - [feature] Added search stage support for `languageCode`, `offset`, `limit`, and `retrievalDepth`. - [feature] Added support for Pipeline expressions `arraySlice`, `arrayFilter`, `arrayTransform` and `arrayTransformWithIndex`. (#16001) - [fixed] Add missing `noexcept` specifiers to move, hash, swap operations [#16117]. diff --git a/Firestore/Example/Tests/Integration/API/FIRNumericTransformTests.mm b/Firestore/Example/Tests/Integration/API/FIRNumericTransformTests.mm index b6939e81801..5f552254e7c 100644 --- a/Firestore/Example/Tests/Integration/API/FIRNumericTransformTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRNumericTransformTests.mm @@ -18,6 +18,8 @@ #import +#import + #import "Firestore/Source/API/FIRFieldValue+Internal.h" #import "Firestore/Example/Tests/Util/FSTEventAccumulator.h" @@ -215,4 +217,125 @@ - (void)testMultipleDoubleIncrements { XCTAssertEqualWithAccuracy(0.111, [snap[@"sum"] doubleValue], DOUBLE_EPSILON); } +- (void)testCreateDocumentWithMinimum { + [self writeDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMinimum:1337]}]; + [self expectLocalAndRemoteValue:1337]; +} + +- (void)testCreateDocumentWithMaximum { + [self writeDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMaximum:1337]}]; + [self expectLocalAndRemoteValue:1337]; +} + +- (void)testMinimumWithExistingInteger { + [self writeInitialData:@{@"sum" : @10}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMinimum:5]}]; + [self expectLocalAndRemoteValue:5]; + + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMinimum:20]}]; + [self expectLocalAndRemoteValue:5]; +} + +- (void)testMaximumWithExistingInteger { + [self writeInitialData:@{@"sum" : @10}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMaximum:5]}]; + [self expectLocalAndRemoteValue:10]; + + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMaximum:20]}]; + [self expectLocalAndRemoteValue:20]; +} + +- (void)testMinimumWithExistingDouble { + [self writeInitialData:@{@"sum" : @10.5}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMinimum:5.5]}]; + [self expectApproximateLocalAndRemoteValue:5.5]; + + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMinimum:20.5]}]; + [self expectApproximateLocalAndRemoteValue:5.5]; +} + +- (void)testMaximumWithExistingDouble { + [self writeInitialData:@{@"sum" : @10.5}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMaximum:5.5]}]; + [self expectApproximateLocalAndRemoteValue:10.5]; + + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMaximum:20.5]}]; + [self expectApproximateLocalAndRemoteValue:20.5]; +} + +- (void)testMixedTypesPreserveOperandTypeForMinimum { + // field and input value of mixed types: field takes on type of smaller operand + [self writeInitialData:@{@"sum" : @10}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMinimum:5.5]}]; + [self expectApproximateLocalAndRemoteValue:5.5]; + + [self writeInitialData:@{@"sum" : @10.5}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMinimum:5]}]; + [self expectLocalAndRemoteValue:5]; +} + +- (void)testMixedTypesPreserveOperandTypeForMaximum { + // field and input value of mixed types: field takes on type of larger operand + [self writeInitialData:@{@"sum" : @10}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMaximum:20.5]}]; + [self expectApproximateLocalAndRemoteValue:20.5]; + + [self writeInitialData:@{@"sum" : @10.5}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMaximum:20]}]; + [self expectLocalAndRemoteValue:20]; +} + +- (void)testEquivalentValuesDoNotChangeTypeForMinimum { + // equivalent (e.g. 3 and 3.0), field does not change type + [self writeInitialData:@{@"sum" : @3}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMinimum:3.0]}]; + [self expectLocalAndRemoteValue:3]; + + [self writeInitialData:@{@"sum" : @3.0}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMinimum:3]}]; + [self expectApproximateLocalAndRemoteValue:3.0]; +} + +- (void)testEquivalentValuesDoNotChangeTypeForMaximum { + // equivalent (e.g. 3 and 3.0), field does not change type + [self writeInitialData:@{@"sum" : @3}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMaximum:3.0]}]; + [self expectLocalAndRemoteValue:3]; + + [self writeInitialData:@{@"sum" : @3.0}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMaximum:3]}]; + [self expectApproximateLocalAndRemoteValue:3.0]; +} + +- (void)expectLocalAndRemoteNaN { + FIRDocumentSnapshot *snap = [_accumulator awaitLocalEvent]; + XCTAssertTrue([snap[@"sum"] isKindOfClass:[NSNumber class]]); + XCTAssertTrue(isnan([snap[@"sum"] doubleValue])); + snap = [_accumulator awaitRemoteEvent]; + XCTAssertTrue([snap[@"sum"] isKindOfClass:[NSNumber class]]); + XCTAssertTrue(isnan([snap[@"sum"] doubleValue])); +} + +- (void)testMinimumWithNaN { + // If one of the values is NaN, minimum is NaN + [self writeInitialData:@{@"sum" : @(NAN)}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMinimum:5]}]; + [self expectLocalAndRemoteNaN]; + + [self writeInitialData:@{@"sum" : @5}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMinimum:NAN]}]; + [self expectLocalAndRemoteNaN]; +} + +- (void)testMaximumWithNaN { + // If one of the values is NaN, maximum is NaN + [self writeInitialData:@{@"sum" : @(NAN)}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForIntegerMaximum:5]}]; + [self expectLocalAndRemoteNaN]; + + [self writeInitialData:@{@"sum" : @5}]; + [self updateDocumentRef:_docRef data:@{@"sum" : [FIRFieldValue fieldValueForDoubleMaximum:NAN]}]; + [self expectLocalAndRemoteNaN]; +} + @end diff --git a/Firestore/Source/API/FIRFieldValue+Internal.h b/Firestore/Source/API/FIRFieldValue+Internal.h index 7e0193db39c..630df29c404 100644 --- a/Firestore/Source/API/FIRFieldValue+Internal.h +++ b/Firestore/Source/API/FIRFieldValue+Internal.h @@ -60,4 +60,16 @@ NS_ASSUME_NONNULL_BEGIN @property(strong, nonatomic, readonly) NSNumber *operand; @end +/** FIRFieldValue class for number minimum transforms. */ +@interface FSTNumericMinimumFieldValue : FIRFieldValue +- (instancetype)init NS_UNAVAILABLE; +@property(strong, nonatomic, readonly) NSNumber *operand; +@end + +/** FIRFieldValue class for number maximum transforms. */ +@interface FSTNumericMaximumFieldValue : FIRFieldValue +- (instancetype)init NS_UNAVAILABLE; +@property(strong, nonatomic, readonly) NSNumber *operand; +@end + NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/API/FIRFieldValue.mm b/Firestore/Source/API/FIRFieldValue.mm index 23c5060a8ee..c264a4c5356 100644 --- a/Firestore/Source/API/FIRFieldValue.mm +++ b/Firestore/Source/API/FIRFieldValue.mm @@ -144,6 +144,48 @@ - (NSString *)methodName { @end +#pragma mark - FSTNumericMinimumFieldValue + +/* FieldValue class for minimum() transforms. */ +@interface FSTNumericMinimumFieldValue () +- (instancetype)initWithOperand:(NSNumber *)operand; +@end + +@implementation FSTNumericMinimumFieldValue +- (instancetype)initWithOperand:(NSNumber *)operand { + if (self = [super initPrivate]) { + _operand = operand; + } + return self; +} + +- (NSString *)methodName { + return @"FieldValue.minimum()"; +} + +@end + +#pragma mark - FSTNumericMaximumFieldValue + +/* FieldValue class for maximum() transforms. */ +@interface FSTNumericMaximumFieldValue () +- (instancetype)initWithOperand:(NSNumber *)operand; +@end + +@implementation FSTNumericMaximumFieldValue +- (instancetype)initWithOperand:(NSNumber *)operand { + if (self = [super initPrivate]) { + _operand = operand; + } + return self; +} + +- (NSString *)methodName { + return @"FieldValue.maximum()"; +} + +@end + #pragma mark - FIRFieldValue @implementation FIRFieldValue @@ -177,6 +219,22 @@ + (instancetype)fieldValueForIntegerIncrement:(int64_t)l { return [[FSTNumericIncrementFieldValue alloc] initWithOperand:@(l)]; } ++ (instancetype)fieldValueForDoubleMinimum:(double)d { + return [[FSTNumericMinimumFieldValue alloc] initWithOperand:@(d)]; +} + ++ (instancetype)fieldValueForIntegerMinimum:(int64_t)l { + return [[FSTNumericMinimumFieldValue alloc] initWithOperand:@(l)]; +} + ++ (instancetype)fieldValueForDoubleMaximum:(double)d { + return [[FSTNumericMaximumFieldValue alloc] initWithOperand:@(d)]; +} + ++ (instancetype)fieldValueForIntegerMaximum:(int64_t)l { + return [[FSTNumericMaximumFieldValue alloc] initWithOperand:@(l)]; +} + + (nonnull FIRVectorValue *)vectorWithArray:(nonnull NSArray *)array { return [[FIRVectorValue alloc] initWithArray:array]; } diff --git a/Firestore/Source/API/FSTUserDataReader.mm b/Firestore/Source/API/FSTUserDataReader.mm index 4720c78d493..d2d494c1790 100644 --- a/Firestore/Source/API/FSTUserDataReader.mm +++ b/Firestore/Source/API/FSTUserDataReader.mm @@ -84,6 +84,8 @@ using firebase::firestore::model::FieldTransform; using firebase::firestore::model::NullValue; using firebase::firestore::model::NumericIncrementTransform; +using firebase::firestore::model::NumericMaximumTransform; +using firebase::firestore::model::NumericMinimumTransform; using firebase::firestore::model::ObjectValue; using firebase::firestore::model::ResourcePath; using firebase::firestore::model::ServerTimestampTransform; @@ -453,6 +455,20 @@ - (void)parseSentinelFieldValue:(FIRFieldValue *)fieldValue context:(ParseContex context.AddToFieldTransforms(*context.path(), std::move(numeric_increment)); + } else if ([fieldValue isKindOfClass:[FSTNumericMinimumFieldValue class]]) { + auto *numericMinimumFieldValue = (FSTNumericMinimumFieldValue *)fieldValue; + auto operand = [self parsedQueryValue:numericMinimumFieldValue.operand]; + NumericMinimumTransform numeric_minimum(std::move(operand)); + + context.AddToFieldTransforms(*context.path(), std::move(numeric_minimum)); + + } else if ([fieldValue isKindOfClass:[FSTNumericMaximumFieldValue class]]) { + auto *numericMaximumFieldValue = (FSTNumericMaximumFieldValue *)fieldValue; + auto operand = [self parsedQueryValue:numericMaximumFieldValue.operand]; + NumericMaximumTransform numeric_maximum(std::move(operand)); + + context.AddToFieldTransforms(*context.path(), std::move(numeric_maximum)); + } else { HARD_FAIL("Unknown FIRFieldValue type: %s", NSStringFromClass([fieldValue class])); } diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRFieldValue.h b/Firestore/Source/Public/FirebaseFirestore/FIRFieldValue.h index 9defa3a0569..b984537eb00 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRFieldValue.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRFieldValue.h @@ -92,6 +92,54 @@ NS_SWIFT_NAME(FieldValue) */ + (instancetype)fieldValueForIntegerIncrement:(int64_t)l NS_SWIFT_NAME(increment(_:)); +/** + * Returns a special value that can be used with `setData()` or `updateData()` that tells the server + * to set the field to the minimum of its current value and the given value. + * + * If the current field value is not an integer or double, or if the field does not yet exist, + * the transformation will set the field to the given value. + * + * @param d The double value to compare. + * @return The `FieldValue` sentinel for use in a call to `setData()` or `updateData()`. + */ ++ (instancetype)fieldValueForDoubleMinimum:(double)d NS_SWIFT_NAME(minimum(_:)); + +/** + * Returns a special value that can be used with `setData()` or `updateData()` that tells the server + * to set the field to the minimum of its current value and the given value. + * + * If the current field value is not an integer or double, or if the field does not yet exist, + * the transformation will set the field to the given value. + * + * @param l The integer value to compare. + * @return The `FieldValue` sentinel for use in a call to `setData()` or `updateData()`. + */ ++ (instancetype)fieldValueForIntegerMinimum:(int64_t)l NS_SWIFT_NAME(minimum(_:)); + +/** + * Returns a special value that can be used with `setData()` or `updateData()` that tells the server + * to set the field to the maximum of its current value and the given value. + * + * If the current field value is not an integer or double, or if the field does not yet exist, + * the transformation will set the field to the given value. + * + * @param d The double value to compare. + * @return The `FieldValue` sentinel for use in a call to `setData()` or `updateData()`. + */ ++ (instancetype)fieldValueForDoubleMaximum:(double)d NS_SWIFT_NAME(maximum(_:)); + +/** + * Returns a special value that can be used with `setData()` or `updateData()` that tells the server + * to set the field to the maximum of its current value and the given value. + * + * If the current field value is not an integer or double, or if the field does not yet exist, + * the transformation will set the field to the given value. + * + * @param l The integer value to compare. + * @return The `FieldValue` sentinel for use in a call to `setData()` or `updateData()`. + */ ++ (instancetype)fieldValueForIntegerMaximum:(int64_t)l NS_SWIFT_NAME(maximum(_:)); + /** * Creates a new `VectorValue` constructed with a copy of the given array of NSNumbers. * diff --git a/Firestore/core/src/model/transform_operation.cc b/Firestore/core/src/model/transform_operation.cc index 1f016fc0f7c..ec91ff552ce 100644 --- a/Firestore/core/src/model/transform_operation.cc +++ b/Firestore/core/src/model/transform_operation.cc @@ -16,6 +16,7 @@ #include "Firestore/core/src/model/transform_operation.h" +#include #include #include #include @@ -290,25 +291,17 @@ Message ArrayTransform::Rep::Apply( return result; } -// MARK: - NumericIncrementTransform +// MARK: - NumericTransform -static_assert(sizeof(TransformOperation) == sizeof(NumericIncrementTransform), +static_assert(sizeof(TransformOperation) == sizeof(NumericTransform), "No additional members allowed (everything must go in Rep)"); -class NumericIncrementTransform::Rep : public TransformOperation::Rep { +class NumericTransform::Rep : public TransformOperation::Rep { public: explicit Rep(Message operand) : operand_(std::move(operand)) { } - Type type() const override { - return Type::Increment; - } - - Message ApplyToLocalView( - const absl::optional& previous_value, - const Timestamp& local_write_time) const override; - Message ApplyToRemoteDocument( const absl::optional&, Message transform_result) const override { @@ -316,42 +309,172 @@ class NumericIncrementTransform::Rep : public TransformOperation::Rep { } absl::optional> ComputeBaseValue( - const absl::optional& previous_value) - const override; + const absl::optional&) const override { + return absl::nullopt; + } - double OperandAsDouble() const; + double OperandAsDouble() const { + if (IsDouble(*operand_)) { + return operand_->double_value; + } else if (IsInteger(*operand_)) { + return static_cast(operand_->integer_value); + } else { + HARD_FAIL( + "Expected 'operand' to be of numeric type, but was %s (type %s)", + CanonicalId(*operand_), GetTypeOrder(*operand_)); + } + } - bool Equals(const TransformOperation::Rep& other) const override; + bool Equals(const TransformOperation::Rep& other) const override { + if (other.type() != type()) { + return false; + } + return *operand_ == + static_cast(other).operand(); + } size_t Hash() const override { return std::hash()(CanonicalId(*operand_)); } - std::string ToString() const override { - return absl::StrCat("NumericIncrement(", operand_->ToString(), ")"); + const google_firestore_v1_Value& operand() const { + return *operand_; } - private: - friend class NumericIncrementTransform; - + protected: Message operand_{}; }; +NumericTransform::NumericTransform( + std::shared_ptr rep) + : TransformOperation(std::move(rep)) { +} + +NumericTransform::NumericTransform(const TransformOperation& op) + : TransformOperation(op) { + HARD_ASSERT(op.type() == Type::Increment || op.type() == Type::Minimum || + op.type() == Type::Maximum, + "Expected numeric transform type; got %s", op.type()); +} + +const google_firestore_v1_Value& NumericTransform::operand() const { + return static_cast(rep()).operand(); +} + +// MARK: - NumericIncrementTransform + +static_assert(sizeof(NumericTransform) == sizeof(NumericIncrementTransform), + "No additional members allowed (everything must go in Rep)"); + +class NumericIncrementTransform::Rep : public NumericTransform::Rep { + public: + using NumericTransform::Rep::Rep; + + Type type() const override { + return Type::Increment; + } + + Message ApplyToLocalView( + const absl::optional& previous_value, + const Timestamp& local_write_time) const override; + + absl::optional> ComputeBaseValue( + const absl::optional& previous_value) + const override { + if (IsNumber(previous_value)) { + return DeepClone(*previous_value); + } + + Message zero_value; + zero_value->which_value_type = google_firestore_v1_Value_integer_value_tag; + zero_value->integer_value = 0; + return zero_value; + } + + std::string ToString() const override { + return absl::StrCat("NumericIncrement(", operand_->ToString(), ")"); + } +}; + NumericIncrementTransform::NumericIncrementTransform( Message operand) - : TransformOperation(std::make_shared(std::move(operand))) { + : NumericTransform(std::make_shared(std::move(operand))) { HARD_ASSERT(IsNumber(this->operand())); } NumericIncrementTransform::NumericIncrementTransform( const TransformOperation& op) - : TransformOperation(op) { + : NumericTransform(op) { HARD_ASSERT(op.type() == Type::Increment, "Expected increment type; got %s", op.type()); } -const google_firestore_v1_Value& NumericIncrementTransform::operand() const { - return *static_cast(rep()).operand_; +// MARK: - NumericMinimumTransform + +static_assert(sizeof(NumericTransform) == sizeof(NumericMinimumTransform), + "No additional members allowed (everything must go in Rep)"); + +class NumericMinimumTransform::Rep : public NumericTransform::Rep { + public: + using NumericTransform::Rep::Rep; + + Type type() const override { + return Type::Minimum; + } + + Message ApplyToLocalView( + const absl::optional& previous_value, + const Timestamp& local_write_time) const override; + + std::string ToString() const override { + return absl::StrCat("NumericMinimum(", operand_->ToString(), ")"); + } +}; + +NumericMinimumTransform::NumericMinimumTransform( + Message operand) + : NumericTransform(std::make_shared(std::move(operand))) { + HARD_ASSERT(IsNumber(this->operand())); +} + +NumericMinimumTransform::NumericMinimumTransform(const TransformOperation& op) + : NumericTransform(op) { + HARD_ASSERT(op.type() == Type::Minimum, "Expected minimum type; got %s", + op.type()); +} + +// MARK: - NumericMaximumTransform + +static_assert(sizeof(NumericTransform) == sizeof(NumericMaximumTransform), + "No additional members allowed (everything must go in Rep)"); + +class NumericMaximumTransform::Rep : public NumericTransform::Rep { + public: + using NumericTransform::Rep::Rep; + + Type type() const override { + return Type::Maximum; + } + + Message ApplyToLocalView( + const absl::optional& previous_value, + const Timestamp& local_write_time) const override; + + std::string ToString() const override { + return absl::StrCat("NumericMaximum(", operand_->ToString(), ")"); + } +}; + +NumericMaximumTransform::NumericMaximumTransform( + Message operand) + : NumericTransform(std::make_shared(std::move(operand))) { + HARD_ASSERT(IsNumber(this->operand())); +} + +NumericMaximumTransform::NumericMaximumTransform(const TransformOperation& op) + : NumericTransform(op) { + HARD_ASSERT(op.type() == Type::Maximum, "Expected maximum type; got %s", + op.type()); } namespace { @@ -374,17 +497,6 @@ int64_t SafeIncrement(int64_t x, int64_t y) { } // namespace -double NumericIncrementTransform::Rep::OperandAsDouble() const { - if (IsDouble(*operand_)) { - return operand_->double_value; - } else if (IsInteger(*operand_)) { - return static_cast(operand_->integer_value); - } else { - HARD_FAIL("Expected 'operand' to be of numeric type, but was %s (type %s)", - CanonicalId(*operand_), GetTypeOrder(*operand_)); - } -} - Message NumericIncrementTransform::Rep::ApplyToLocalView( const absl::optional& previous_value, @@ -410,27 +522,84 @@ NumericIncrementTransform::Rep::ApplyToLocalView( return result; } -absl::optional> -NumericIncrementTransform::Rep::ComputeBaseValue( - const absl::optional& previous_value) const { - if (IsNumber(previous_value)) { - return DeepClone(*previous_value); +Message +NumericMinimumTransform::Rep::ApplyToLocalView( + const absl::optional& previous_value, + const Timestamp& /* local_write_time */) const { + if (!IsNumber(previous_value)) { + return DeepClone(*operand_); + } + + auto base_value = *previous_value; + Message result; + + // Return an integer value only if both the previous value and the operand are + // integers. + if (IsInteger(base_value) && IsInteger(*operand_)) { + result->which_value_type = google_firestore_v1_Value_integer_value_tag; + result->integer_value = + std::min(base_value.integer_value, operand_->integer_value); + return result; + } + + double prev_double = IsInteger(base_value) + ? static_cast(base_value.integer_value) + : base_value.double_value; + double oper_double = OperandAsDouble(); + + if (std::isnan(prev_double)) { + return DeepClone(base_value); + } + if (std::isnan(oper_double)) { + return DeepClone(*operand_); + } + + if (prev_double == oper_double) { + return DeepClone(base_value); } - Message zero_value; - zero_value->which_value_type = google_firestore_v1_Value_integer_value_tag; - zero_value->integer_value = 0; - return zero_value; + bool choose_previous = prev_double < oper_double; + return choose_previous ? DeepClone(base_value) : DeepClone(*operand_); } -bool NumericIncrementTransform::Rep::Equals( - const TransformOperation::Rep& other) const { - if (other.type() != type()) { - return false; +Message +NumericMaximumTransform::Rep::ApplyToLocalView( + const absl::optional& previous_value, + const Timestamp& /* local_write_time */) const { + if (!IsNumber(previous_value)) { + return DeepClone(*operand_); + } + + auto base_value = *previous_value; + Message result; + + // Return an integer value only if both the previous value and the operand are + // integers. + if (IsInteger(base_value) && IsInteger(*operand_)) { + result->which_value_type = google_firestore_v1_Value_integer_value_tag; + result->integer_value = + std::max(base_value.integer_value, operand_->integer_value); + return result; + } + + double prev_double = IsInteger(base_value) + ? static_cast(base_value.integer_value) + : base_value.double_value; + double oper_double = OperandAsDouble(); + + if (std::isnan(prev_double)) { + return DeepClone(base_value); + } + if (std::isnan(oper_double)) { + return DeepClone(*operand_); + } + + if (prev_double == oper_double) { + return DeepClone(base_value); } - return *operand_ == - *static_cast(other).operand_; + bool choose_previous = prev_double > oper_double; + return choose_previous ? DeepClone(base_value) : DeepClone(*operand_); } } // namespace model diff --git a/Firestore/core/src/model/transform_operation.h b/Firestore/core/src/model/transform_operation.h index 80411440b9b..663c09beff6 100644 --- a/Firestore/core/src/model/transform_operation.h +++ b/Firestore/core/src/model/transform_operation.h @@ -59,6 +59,8 @@ class TransformOperation { ArrayUnion, ArrayRemove, Increment, + Minimum, + Maximum, }; TransformOperation() = default; @@ -199,12 +201,27 @@ class ArrayTransform : public TransformOperation { const Rep& array_rep() const; }; +/** + * A parent class for numeric transforms (increment, minimum, and maximum). + */ +class NumericTransform : public TransformOperation { + public: + explicit NumericTransform(const TransformOperation& op); + + const google_firestore_v1_Value& operand() const; + + class Rep; + + protected: + explicit NumericTransform(std::shared_ptr rep); +}; + /** * Implements the backend semantics for locally computed NUMERIC_ADD (increment) * transforms. Converts all field values to longs or doubles and resolves * overflows to LONG_MAX/LONG_MIN. */ -class NumericIncrementTransform : public TransformOperation { +class NumericIncrementTransform : public NumericTransform { public: explicit NumericIncrementTransform( nanopb::Message operand); @@ -216,7 +233,45 @@ class NumericIncrementTransform : public TransformOperation { */ explicit NumericIncrementTransform(const TransformOperation& op); - const google_firestore_v1_Value& operand() const; + private: + class Rep; +}; + +/** + * Implements the backend semantics for locally computed NUMERIC_MIN (minimum) + * transforms. + */ +class NumericMinimumTransform : public NumericTransform { + public: + explicit NumericMinimumTransform( + nanopb::Message operand); + + /** + * Casts a TransformOperation to a NumericMinimumTransform. This is a + * checked operation that will assert if the type of the TransformOperation + * isn't actually Type::Minimum. + */ + explicit NumericMinimumTransform(const TransformOperation& op); + + private: + class Rep; +}; + +/** + * Implements the backend semantics for locally computed NUMERIC_MAX (maximum) + * transforms. + */ +class NumericMaximumTransform : public NumericTransform { + public: + explicit NumericMaximumTransform( + nanopb::Message operand); + + /** + * Casts a TransformOperation to a NumericMaximumTransform. This is a + * checked operation that will assert if the type of the TransformOperation + * isn't actually Type::Maximum. + */ + explicit NumericMaximumTransform(const TransformOperation& op); private: class Rep; diff --git a/Firestore/core/src/remote/serializer.cc b/Firestore/core/src/remote/serializer.cc index 932064a01a9..ddd785d83c5 100644 --- a/Firestore/core/src/remote/serializer.cc +++ b/Firestore/core/src/remote/serializer.cc @@ -92,6 +92,8 @@ using model::MutationResult; using model::NaNValue; using model::NullValue; using model::NumericIncrementTransform; +using model::NumericMaximumTransform; +using model::NumericMinimumTransform; using model::ObjectValue; using model::PatchMutation; using model::Precondition; @@ -579,6 +581,24 @@ Serializer::EncodeFieldTransform(const FieldTransform& field_transform) const { proto.increment = increment.operand(); return proto; } + + case Type::Minimum: { + proto.which_transform_type = + google_firestore_v1_DocumentTransform_FieldTransform_minimum_tag; + const auto& minimum = static_cast( + field_transform.transformation()); + proto.minimum = minimum.operand(); + return proto; + } + + case Type::Maximum: { + proto.which_transform_type = + google_firestore_v1_DocumentTransform_FieldTransform_maximum_tag; + const auto& maximum = static_cast( + field_transform.transformation()); + proto.maximum = maximum.operand(); + return proto; + } } UNREACHABLE(); @@ -626,6 +646,22 @@ FieldTransform Serializer::DecodeFieldTransform( std::move(field), NumericIncrementTransform(MakeMessage(proto.increment))); } + + case google_firestore_v1_DocumentTransform_FieldTransform_minimum_tag: { + FieldTransform field_transform( + std::move(field), + NumericMinimumTransform(MakeMessage(proto.minimum))); + proto.minimum = {}; + return field_transform; + } + + case google_firestore_v1_DocumentTransform_FieldTransform_maximum_tag: { + FieldTransform field_transform( + std::move(field), + NumericMaximumTransform(MakeMessage(proto.maximum))); + proto.maximum = {}; + return field_transform; + } } UNREACHABLE(); diff --git a/Firestore/core/test/unit/model/mutation_test.cc b/Firestore/core/test/unit/model/mutation_test.cc index cf4d1df7cc6..5c85ed1b18c 100644 --- a/Firestore/core/test/unit/model/mutation_test.cc +++ b/Firestore/core/test/unit/model/mutation_test.cc @@ -295,6 +295,16 @@ auto Increment(T value) -> decltype(NumericIncrementTransform(Value(value))) { return NumericIncrementTransform(Value(value)); } +template +auto Minimum(T value) -> decltype(NumericMinimumTransform(Value(value))) { + return NumericMinimumTransform(Value(value)); +} + +template +auto Maximum(T value) -> decltype(NumericMaximumTransform(Value(value))) { + return NumericMaximumTransform(Value(value)); +} + template TransformOperation ArrayUnion(Args... args) { return ArrayTransform(TransformOperation::Type::ArrayUnion, @@ -871,6 +881,120 @@ TEST(MutationTest, OverlayByCombinationsAndPermutations_Increments) { EXPECT_EQ(5871, test_cases); } +TEST(MutationTest, AppliesMinimumTransformToDocument) { + auto base_data = + Map("longMinLong", 5, "longMinDouble", 5, "doubleMinLong", 5.5, + "doubleMinDouble", 5.5, "longMinNan", 5, "doubleMinNan", 5.5); + TransformPairs transforms = { + {"longMinLong", Minimum(2)}, {"longMinDouble", Minimum(2.2)}, + {"doubleMinLong", Minimum(2)}, {"doubleMinDouble", Minimum(2.2)}, + {"longMinNan", Minimum(NAN)}, {"doubleMinNan", Minimum(NAN)}, + }; + auto expected = + Map("longMinLong", 2L, "longMinDouble", 2.2, "doubleMinLong", 2L, + "doubleMinDouble", 2.2, "longMinNan", NAN, "doubleMinNan", NAN); + TransformBaseDoc(std::move(base_data), transforms, std::move(expected)); +} + +TEST(MutationTest, AppliesMaximumTransformToDocument) { + auto base_data = + Map("longMaxLong", 5, "longMaxDouble", 5, "doubleMaxLong", 5.5, + "doubleMaxDouble", 5.5, "longMaxNan", 5, "doubleMaxNan", 5.5); + TransformPairs transforms = { + {"longMaxLong", Maximum(8)}, {"longMaxDouble", Maximum(8.8)}, + {"doubleMaxLong", Maximum(8)}, {"doubleMaxDouble", Maximum(8.8)}, + {"longMaxNan", Maximum(NAN)}, {"doubleMaxNan", Maximum(NAN)}, + }; + auto expected = + Map("longMaxLong", 8L, "longMaxDouble", 8.8, "doubleMaxLong", 8L, + "doubleMaxDouble", 8.8, "longMaxNan", NAN, "doubleMaxNan", NAN); + TransformBaseDoc(std::move(base_data), transforms, std::move(expected)); +} + +TEST(MutationTest, AppliesMinimumTransformToUnexpectedType) { + auto base_data = Map("string", "value"); + TransformPairs transforms = { + {"string", Minimum(1)}, + }; + auto expected = Map("string", 1); + TransformBaseDoc(std::move(base_data), transforms, std::move(expected)); +} + +TEST(MutationTest, AppliesMaximumTransformToUnexpectedType) { + auto base_data = Map("string", "value"); + TransformPairs transforms = { + {"string", Maximum(1)}, + }; + auto expected = Map("string", 1); + TransformBaseDoc(std::move(base_data), transforms, std::move(expected)); +} + +TEST(MutationTest, AppliesMinimumTransformToMissingField) { + auto base_data = Map(); + TransformPairs transforms = { + {"missing", Minimum(1)}, + }; + auto expected = Map("missing", 1); + TransformBaseDoc(std::move(base_data), transforms, std::move(expected)); +} + +TEST(MutationTest, AppliesMaximumTransformToMissingField) { + auto base_data = Map(); + TransformPairs transforms = { + {"missing", Maximum(1)}, + }; + auto expected = Map("missing", 1); + TransformBaseDoc(std::move(base_data), transforms, std::move(expected)); +} + +TEST(MutationTest, NumericMinimumBaseValue) { + auto nested_values = Map("ignore", "foo", "double", 42.0, "long", 42, + "string", "foo", "map", Map()); + auto all_values = + Map("ignore", "foo", "double", 42.0, "long", 42, "string", "foo", "map", + Map(), "nested", std::move(nested_values)); + MutableDocument base_doc = Doc("collection/key", 1, std::move(all_values)); + + Mutation mutation = PatchMutation("collection/key", Map(), {}, + {{"double", Minimum(1)}, + {"long", Minimum(1)}, + {"string", Minimum(1)}, + {"map", Minimum(1)}, + {"missing", Minimum(1)}, + {"nested.double", Minimum(1)}, + {"nested.long", Minimum(1)}, + {"nested.string", Minimum(1)}, + {"nested.map", Minimum(1)}, + {"nested.missing", Minimum(1)}}); + absl::optional base_value = + mutation.ExtractTransformBaseValue(base_doc); + EXPECT_FALSE(base_value.has_value()); +} + +TEST(MutationTest, NumericMaximumBaseValue) { + auto nested_values = Map("ignore", "foo", "double", 42.0, "long", 42, + "string", "foo", "map", Map()); + auto all_values = + Map("ignore", "foo", "double", 42.0, "long", 42, "string", "foo", "map", + Map(), "nested", std::move(nested_values)); + MutableDocument base_doc = Doc("collection/key", 1, std::move(all_values)); + + Mutation mutation = PatchMutation("collection/key", Map(), {}, + {{"double", Maximum(1)}, + {"long", Maximum(1)}, + {"string", Maximum(1)}, + {"map", Maximum(1)}, + {"missing", Maximum(1)}, + {"nested.double", Maximum(1)}, + {"nested.long", Maximum(1)}, + {"nested.string", Maximum(1)}, + {"nested.map", Maximum(1)}, + {"nested.missing", Maximum(1)}}); + absl::optional base_value = + mutation.ExtractTransformBaseValue(base_doc); + EXPECT_FALSE(base_value.has_value()); +} + } // namespace } // namespace model } // namespace firestore diff --git a/Firestore/core/test/unit/model/transform_operation_test.cc b/Firestore/core/test/unit/model/transform_operation_test.cc index 892714dd403..7f379215aac 100644 --- a/Firestore/core/test/unit/model/transform_operation_test.cc +++ b/Firestore/core/test/unit/model/transform_operation_test.cc @@ -37,6 +37,30 @@ TEST(TransformOperationsTest, ServerTimestamp) { EXPECT_NE(transform, other); } +TEST(TransformOperationsTest, Minimum) { + NumericMinimumTransform transform(Value(1)); + EXPECT_EQ(Type::Minimum, transform.type()); + + NumericMinimumTransform another(Value(1)); + NumericMinimumTransform different(Value(2)); + NumericIncrementTransform other(Value(1)); + EXPECT_EQ(transform, another); + EXPECT_NE(transform, different); + EXPECT_NE(transform, other); +} + +TEST(TransformOperationsTest, Maximum) { + NumericMaximumTransform transform(Value(1)); + EXPECT_EQ(Type::Maximum, transform.type()); + + NumericMaximumTransform another(Value(1)); + NumericMaximumTransform different(Value(2)); + NumericMinimumTransform other(Value(1)); + EXPECT_EQ(transform, another); + EXPECT_NE(transform, different); + EXPECT_NE(transform, other); +} + // TODO(mikelehen): Add ArrayTransform test once it no longer depends on // FSTFieldValue and can be exposed to C++ code. diff --git a/Firestore/core/test/unit/testutil/testutil.cc b/Firestore/core/test/unit/testutil/testutil.cc index e59c42e36fc..027784da942 100644 --- a/Firestore/core/test/unit/testutil/testutil.cc +++ b/Firestore/core/test/unit/testutil/testutil.cc @@ -577,6 +577,22 @@ std::pair Increment( std::move(transform)); } +std::pair Minimum( + std::string field, Message operand) { + model::NumericMinimumTransform transform(std::move(operand)); + + return std::pair(std::move(field), + std::move(transform)); +} + +std::pair Maximum( + std::string field, Message operand) { + model::NumericMaximumTransform transform(std::move(operand)); + + return std::pair(std::move(field), + std::move(transform)); +} + std::pair ArrayUnion( std::string field, const std::vector>& operands) { diff --git a/Firestore/core/test/unit/testutil/testutil.h b/Firestore/core/test/unit/testutil/testutil.h index 5af75e4a8cf..be04a1f41dc 100644 --- a/Firestore/core/test/unit/testutil/testutil.h +++ b/Firestore/core/test/unit/testutil/testutil.h @@ -466,6 +466,22 @@ std::pair ServerTimestamp( std::pair Increment( std::string field, nanopb::Message operand); +/** + * Creates a pair of field name, TransformOperation that represents a numeric + * minimum on the given field, suitable for passing to TransformMutation, + * above. + */ +std::pair Minimum( + std::string field, nanopb::Message operand); + +/** + * Creates a pair of field name, TransformOperation that represents a numeric + * maximum on the given field, suitable for passing to TransformMutation, + * above. + */ +std::pair Maximum( + std::string field, nanopb::Message operand); + /** * Creates a pair of field name, TransformOperation that represents an array * union on the given field, suitable for passing to TransformMutation,