Add clear support to NumberPatch

Differential Revision: D39161903

fbshipit-source-id: 1c7e2aa33a600b4bcdb48ce7b544c61e183beb59

Author: Alfred Fuller

third-party/thrift/src/thrift/lib/cpp2/op/detail/ValuePatch.h

@@ -96,11 +96,13 @@ class BoolPatch : public BaseClearPatch<Patch, BoolPatch<Patch>> {
 
 // Patch must have the following fields:
 //   optional T assign;
+//   bool clear;
 //   T add;
 template <typename Patch>
-class NumberPatch : public BaseAssignPatch<Patch, NumberPatch<Patch>> {
-  using Base = BaseAssignPatch<Patch, NumberPatch>;
+class NumberPatch : public BaseClearPatch<Patch, NumberPatch<Patch>> {
+  using Base = BaseClearPatch<Patch, NumberPatch>;
   using T = typename Base::value_type;
+  using Tag = type::infer_tag<T>;
 
  public:
   using Base::apply;
@@ -131,14 +133,14 @@ class NumberPatch : public BaseAssignPatch<Patch, NumberPatch<Patch>> {
   }
 
   void apply(T& val) const {
-    if (!applyAssign(val)) {
+    if (!Base::template applyAssignAndClear<Tag>(val)) {
       val += *data_.add();
     }
   }
 
   template <typename U>
   void merge(U&& next) {
-    if (!mergeAssign(std::forward<U>(next))) {
+    if (!mergeAssignAndClear(std::forward<U>(next))) {
       *data_.add() += *next.toThrift().add();
     }
   }
@@ -156,10 +158,9 @@ class NumberPatch : public BaseAssignPatch<Patch, NumberPatch<Patch>> {
   }
 
  private:
-  using Base::applyAssign;
   using Base::assignOr;
   using Base::data_;
-  using Base::mergeAssign;
+  using Base::mergeAssignAndClear;
 
   template <typename U>
   friend NumberPatch operator+(NumberPatch lhs, U&& rhs) {

third-party/thrift/src/thrift/lib/cpp2/protocol/Patch.cpp

@@ -104,10 +104,16 @@ bool applyAssign(const Object& patch, value_native_type<Tag>& value) {
 template <typename Tag, typename T>
 void applyNumericPatch(const Object& patch, T& value) {
   constexpr auto valueType = static_cast<Value::Type>(type::base_type_v<Tag>);
-  checkOps(patch, valueType, {PatchOp::Assign, PatchOp::Add});
+  checkOps(patch, valueType, {PatchOp::Assign, PatchOp::Clear, PatchOp::Add});
   if (applyAssign<Tag>(patch, value)) {
     return; // Ignore all other ops.
   }
+  if (auto* clear = findOp(patch, PatchOp::Clear)) {
+    if (argAs<type::bool_t>(*clear)) {
+      value = {};
+    }
+  }
+
   if (auto* arg = findOp(patch, PatchOp::Add)) {
     value += argAs<Tag>(*arg);
   }

third-party/thrift/src/thrift/lib/thrift/patch.thrift

@@ -80,6 +80,9 @@ struct BytePatch {
    */
   1: optional byte assign;
 
+  /** Clear any set value. */
+  2: bool clear;
+
   /** Add to a given value. */
   8: byte add;
 }
@@ -99,6 +102,9 @@ struct I16Patch {
    */
   1: optional i16 assign;
 
+  /** Clear any set value. */
+  2: bool clear;
+
   /** Add to a given value. */
   8: i16 add;
 }
@@ -118,6 +124,9 @@ struct I32Patch {
    */
   1: optional i32 assign;
 
+  /** Clear any set value. */
+  2: bool clear;
+
   /** Add to a given value. */
   8: i32 add;
 }
@@ -137,6 +146,9 @@ struct I64Patch {
    */
   1: optional i64 assign;
 
+  /** Clear any set value. */
+  2: bool clear;
+
   /** Add to a given value. */
   8: i64 add;
 }
@@ -156,6 +168,9 @@ struct FloatPatch {
    */
   1: optional float assign;
 
+  /** Clear any set value. */
+  2: bool clear;
+
   /** Add to a given value. */
   8: float add;
 }
@@ -175,6 +190,9 @@ struct DoublePatch {
    */
   1: optional double assign;
 
+  /** Clear any set value. */
+  2: bool clear;
+
   /** Add to a given value. */
   8: double add;
 }

third-party/thrift/src/thrift/test/StructPatchTest.cpp

@@ -116,7 +116,7 @@ TEST(StructPatchTest, Clear) {
   test::expectPatch(op::StringPatch::createClear(), {"hi"}, "");
 }
 
-TEST(StructPatchTest, ClearField_Bool) {
+TEST(StructPatchTest, ClearField_BoolPatch) {
   MyStruct actual;
 
   actual.boolVal() = true;
@@ -128,6 +128,30 @@ TEST(StructPatchTest, ClearField_Bool) {
   EXPECT_FALSE(actual.optBoolVal().has_value());
 }
 
+TEST(StructPatchTest, ClearField_NumberPatch) {
+  MyStruct actual;
+
+  actual.i16Val() = 1;
+  op::I16Patch::createClear().apply(actual.i16Val());
+  EXPECT_EQ(*actual.boolVal(), 0);
+
+  actual.optDoubleVal() = 1;
+  op::DoublePatch::createClear().apply(actual.optDoubleVal());
+  EXPECT_FALSE(actual.optDoubleVal().has_value());
+}
+
+TEST(StructPatchTest, ClearField_StringPatch) {
+  MyStruct actual;
+
+  actual.stringVal() = "hi";
+  op::StringPatch::createClear().apply(actual.stringVal());
+  EXPECT_EQ(*actual.stringVal(), "");
+
+  actual.optBinaryVal().ensure();
+  op::BinaryPatch::createClear().apply(actual.optBinaryVal());
+  EXPECT_FALSE(actual.optBinaryVal().has_value());
+}
+
 TEST(StructPatchTest, Patch) {
   MyStruct val;
   val.stringVal() = "hi";