Add clear support to BoolPatch

Summary: Which will unset an optional value, or clear a non-optional valuie.

Differential Revision: D39161781

fbshipit-source-id: 140a6c23443b88577390a9586f50b0d38b936f7c

Author: Alfred Fuller

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

@@ -200,12 +200,13 @@ class BaseAssignPatch : public BasePatch<Patch, Derived> {
   using Base::resetAnd;
   ~BaseAssignPatch() = default; // abstract base class
 
+  FOLLY_NODISCARD bool hasAssign() const { return hasValue(data_.assign()); }
   FOLLY_NODISCARD value_type& assignOr(value_type& value) noexcept {
-    return hasValue(data_.assign()) ? *data_.assign() : value;
+    return hasAssign() ? *data_.assign() : value;
   }
 
   bool applyAssign(value_type& val) const {
-    if (hasValue(data_.assign())) {
+    if (hasAssign()) {
       val = *data_.assign();
       return true;
     }
@@ -218,7 +219,7 @@ class BaseAssignPatch : public BasePatch<Patch, Derived> {
       data_ = std::forward<U>(next).toThrift();
       return true;
     }
-    if (hasValue(data_.assign())) {
+    if (hasAssign()) {
       next.apply(*data_.assign());
       return true;
     }
@@ -239,15 +240,29 @@ class BaseClearPatch : public BaseAssignPatch<Patch, Derived> {
  public:
   using Base::Base;
   using Base::operator=;
+  using Base::apply;
 
   FOLLY_NODISCARD static Derived createClear() {
     Derived patch;
     patch.clear();
     return patch;
   }
 
+  // Clear resets optional fields.
+  template <typename U>
+  if_opt_type<folly::remove_cvref_t<U>> apply(U&& field) const {
+    if (data_.clear() == true && !hasAssign()) {
+      field.reset();
+    } else if (field.has_value()) {
+      derived().apply(*std::forward<U>(field));
+    }
+  }
+
  protected:
+  using Base::applyAssign;
   using Base::data_;
+  using Base::derived;
+  using Base::hasAssign;
   using Base::mergeAssign;
   using Base::resetAnd;
   ~BaseClearPatch() = default;
@@ -266,6 +281,17 @@ class BaseClearPatch : public BaseAssignPatch<Patch, Derived> {
     return mergeAssign(std::forward<U>(next));
   }
 
+  template <typename Tag>
+  bool applyAssignAndClear(T& val) const {
+    if (applyAssign(val)) {
+      return true;
+    }
+    if (data_.clear() == true) {
+      op::clear<Tag>(val);
+    }
+    return false;
+  }
+
   void clear() { resetAnd().clear() = true; }
   FOLLY_NODISCARD T& clearAnd() { return (clear(), data_); }
 };

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

@@ -53,10 +53,11 @@ class AssignPatch : public BaseAssignPatch<Patch, AssignPatch<Patch>> {
 
 // Patch must have the following fields:
 //   optional T assign;
+//   bool clear;
 //   bool invert;
 template <typename Patch>
-class BoolPatch : public BaseAssignPatch<Patch, BoolPatch<Patch>> {
-  using Base = BaseAssignPatch<Patch, BoolPatch>;
+class BoolPatch : public BaseClearPatch<Patch, BoolPatch<Patch>> {
+  using Base = BaseClearPatch<Patch, BoolPatch>;
   using T = typename Base::value_type;
 
  public:
@@ -72,23 +73,23 @@ class BoolPatch : public BaseAssignPatch<Patch, BoolPatch<Patch>> {
   }
 
   void apply(T& val) const {
-    if (!applyAssign(val) && *data_.invert()) {
+    if (!Base::template applyAssignAndClear<type::bool_t>(val) &&
+        *data_.invert()) {
       val = !val;
     }
   }
 
   template <typename U>
   void merge(U&& next) {
-    if (!mergeAssign(std::forward<U>(next))) {
+    if (!mergeAssignAndClear(std::forward<U>(next))) {
       *data_.invert() ^= *next.toThrift().invert();
     }
   }
 
  private:
-  using Base::applyAssign;
   using Base::assignOr;
   using Base::data_;
-  using Base::mergeAssign;
+  using Base::mergeAssignAndClear;
 
   friend BoolPatch operator!(BoolPatch val) { return (val.invert(), val); }
 };

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

@@ -155,10 +155,17 @@ void ApplyPatch::operator()(const Object& patch, protocol::Value& value) const {
 }
 
 void ApplyPatch::operator()(const Object& patch, bool& value) const {
-  checkOps(patch, Value::Type::boolValue, {PatchOp::Assign, PatchOp::Put});
+  checkOps(
+      patch,
+      Value::Type::boolValue,
+      {PatchOp::Assign, PatchOp::Put, PatchOp::Clear});
   if (applyAssign<type::bool_t>(patch, value)) {
     return; // Ignore all other ops.
   }
+  if (auto* clear = findOp(patch, PatchOp::Clear);
+      clear != nullptr && *clear->boolValue_ref()) {
+    value = false;
+  }
   if (auto* invert = findOp(patch, PatchOp::Put)) { // Put is Invert for bool.
     if (argAs<type::bool_t>(*invert)) {
       value = !value;

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

@@ -58,6 +58,9 @@ struct BoolPatch {
    */
   1: optional bool assign;
 
+  /** Clear any set value. */
+  2: bool clear;
+
   /** If the bool value should be inverted. */
   9: bool invert;
 }

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

@@ -21,6 +21,7 @@
 #include <thrift/lib/cpp2/op/Testing.h>
 #include <thrift/lib/cpp2/op/detail/StructPatch.h>
 #include <thrift/lib/cpp2/type/Testing.h>
+#include <thrift/lib/thrift/gen-cpp2/patch_types.h>
 #include <thrift/test/gen-cpp2/StructPatchTest_fatal_types.h>
 #include <thrift/test/gen-cpp2/StructPatchTest_types.h>
 
@@ -115,6 +116,18 @@ TEST(StructPatchTest, Clear) {
   test::expectPatch(op::StringPatch::createClear(), {"hi"}, "");
 }
 
+TEST(StructPatchTest, ClearField_Bool) {
+  MyStruct actual;
+
+  actual.boolVal() = true;
+  op::BoolPatch::createClear().apply(actual.boolVal());
+  EXPECT_FALSE(*actual.boolVal());
+
+  actual.optBoolVal() = true;
+  op::BoolPatch::createClear().apply(actual.optBoolVal());
+  EXPECT_FALSE(actual.optBoolVal().has_value());
+}
+
 TEST(StructPatchTest, Patch) {
   MyStruct val;
   val.stringVal() = "hi";