diff --git a/fboss/thrift_cow/nodes/Serializer.h b/fboss/thrift_cow/nodes/Serializer.h index efce0fc12172a..138d9ca8240e1 100644 --- a/fboss/thrift_cow/nodes/Serializer.h +++ b/fboss/thrift_cow/nodes/Serializer.h @@ -236,9 +236,12 @@ struct Serializable { virtual folly::dynamic toFollyDynamic() const = 0; }; +struct ThriftObject {}; + template class SerializableWrapper : public Serializable { public: + using CowType = ThriftObject; explicit SerializableWrapper(TType& node) : node_(node) {} folly::IOBuf encodeBuf(fsdb::OperProtocol proto) const override { diff --git a/fboss/thrift_cow/nodes/ThriftStructNode-inl.h b/fboss/thrift_cow/nodes/ThriftStructNode-inl.h index 53ebc5ba47e43..9162efd38fc07 100644 --- a/fboss/thrift_cow/nodes/ThriftStructNode-inl.h +++ b/fboss/thrift_cow/nodes/ThriftStructNode-inl.h @@ -600,13 +600,11 @@ class ThriftStructNode : public NodeBaseT< return; } auto tok = *begin; - if constexpr (thrift_cow::is_cow_type_v) { - if constexpr (std::is_same_v< - typename folly::remove_cvref_t< - decltype(node)>::CowType, - NodeType>) { - node.modify(tok); - } + if constexpr (std::is_same_v< + typename folly::remove_cvref_t< + decltype(node)>::CowType, + NodeType>) { + node.modify(tok); } }); diff --git a/fboss/thrift_cow/nodes/tests/ThriftStructNodeTests.cpp b/fboss/thrift_cow/nodes/tests/ThriftStructNodeTests.cpp index 10f5ef3bfeb20..4c0a12a6b0d94 100644 --- a/fboss/thrift_cow/nodes/tests/ThriftStructNodeTests.cpp +++ b/fboss/thrift_cow/nodes/tests/ThriftStructNodeTests.cpp @@ -510,14 +510,7 @@ TYPED_TEST(ThriftStructNodeTestSuite, ThriftStructNodeModifyPathTest) { folly::dynamic dyn; auto processPath = pvlambda([&dyn](auto& node, auto begin, auto end) { EXPECT_EQ(begin, end); - if constexpr (std::is_base_of_v< - Serializable, - std::remove_cvref_t>) { - dyn = node.toFollyDynamic(); - } else { - facebook::thrift::to_dynamic( - dyn, node, facebook::thrift::dynamic_format::JSON_1); - } + dyn = node.toFollyDynamic(); }); // non-existent node auto visitResult = RootPathVisitor::visit( diff --git a/fboss/thrift_cow/visitors/PathVisitor.h b/fboss/thrift_cow/visitors/PathVisitor.h index 264a9443f289e..24d517ae35a5e 100644 --- a/fboss/thrift_cow/visitors/PathVisitor.h +++ b/fboss/thrift_cow/visitors/PathVisitor.h @@ -188,10 +188,22 @@ struct LambdaPathVisitorOperator { template inline auto - visitTyped(Node& node, pv_detail::PathIter begin, pv_detail::PathIter end) { + visitTyped(Node& node, pv_detail::PathIter begin, pv_detail::PathIter end) + requires(is_cow_type_v) + { return f_(node, begin, end); } + template + inline auto + visitTyped(Node& node, pv_detail::PathIter begin, pv_detail::PathIter end) + requires(!is_cow_type_v) + { + // Node is not a Serializable, dispatch with wrapper + SerializableWrapper wrapper(node); + return f_(wrapper, begin, end); + } + private: Func f_; }; diff --git a/fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp b/fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp index 382e990dfc5f8..c4eae6b27e7de 100644 --- a/fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp +++ b/fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp @@ -70,16 +70,9 @@ TYPED_TEST(PathVisitorTests, AccessField) { auto nodeA = this->initNode(structA); folly::dynamic dyn; - auto processPath = pvlambda([&dyn](auto& node, auto begin, auto end) { + auto processPath = pvlambda([&dyn](const auto& node, auto begin, auto end) { EXPECT_EQ(begin, end); - if constexpr (std::is_base_of_v< - Serializable, - std::remove_cvref_t>) { - dyn = node.toFollyDynamic(); - } else { - facebook::thrift::to_dynamic( - dyn, node, facebook::thrift::dynamic_format::JSON_1); - } + dyn = node.toFollyDynamic(); }); std::vector path{"inlineInt"}; auto result = RootPathVisitor::visit( @@ -145,16 +138,9 @@ TYPED_TEST(PathVisitorTests, AccessAtHybridNodeTest) { auto nodeA = this->initNode(root); folly::dynamic dyn; - auto processPath = pvlambda([&dyn](auto& node, auto begin, auto end) { + auto processPath = pvlambda([&dyn](Serializable& node, auto begin, auto end) { EXPECT_EQ(begin, end); - if constexpr (std::is_base_of_v< - Serializable, - std::remove_cvref_t>) { - dyn = node.toFollyDynamic(); - } else { - facebook::thrift::to_dynamic( - dyn, node, facebook::thrift::dynamic_format::JSON_1); - } + dyn = node.toFollyDynamic(); }); // Thrift path terminating at HybridNode - Access @@ -210,14 +196,7 @@ TYPED_TEST(PathVisitorTests, AccessAtHybridThriftContainerTest) { folly::dynamic dyn; auto processPath = pvlambda([&dyn](auto& node, auto begin, auto end) { EXPECT_EQ(begin, end); - if constexpr (std::is_base_of_v< - Serializable, - std::remove_cvref_t>) { - dyn = node.toFollyDynamic(); - } else { - facebook::thrift::to_dynamic( - dyn, node, facebook::thrift::dynamic_format::JSON_1); - } + dyn = node.toFollyDynamic(); }); // Thrift path at thrift container under HybridNode - Access @@ -277,14 +256,7 @@ TYPED_TEST(PathVisitorTests, AccessAtHybridThriftContainerKeyTest) { folly::dynamic dyn; auto processPath = pvlambda([&dyn](auto& node, auto begin, auto end) { EXPECT_EQ(begin, end); - if constexpr (std::is_base_of_v< - Serializable, - std::remove_cvref_t>) { - dyn = node.toFollyDynamic(); - } else { - facebook::thrift::to_dynamic( - dyn, node, facebook::thrift::dynamic_format::JSON_1); - } + dyn = node.toFollyDynamic(); }); // Thrift path at thrift container key under HybridNode - Access @@ -339,14 +311,7 @@ TYPED_TEST(PathVisitorTests, AccessFieldInContainer) { folly::dynamic dyn; auto processPath = pvlambda([&dyn](auto& node, auto begin, auto end) { EXPECT_EQ(begin, end); - if constexpr (std::is_base_of_v< - Serializable, - std::remove_cvref_t>) { - dyn = node.toFollyDynamic(); - } else { - facebook::thrift::to_dynamic( - dyn, node, facebook::thrift::dynamic_format::JSON_1); - } + dyn = node.toFollyDynamic(); }); std::vector path{"mapOfEnumToStruct", "3"}; auto result = RootPathVisitor::visit( @@ -406,13 +371,7 @@ TEST(PathVisitorTests, AccessOptional) { std::string got; auto processPath = pvlambda([&got](auto& node, auto begin, auto end) { EXPECT_EQ(begin, end); - if constexpr (std::is_base_of_v< - Serializable, - std::remove_cvref_t>) { - got = node.toFollyDynamic().asString(); - } else { - FAIL() << "unexpected non-cow visit"; - } + got = node.toFollyDynamic().asString(); }); std::vector path{"optionalString"}; auto result = RootPathVisitor::visit(