diff --git a/fboss/thrift_cow/visitors/PathVisitor.h b/fboss/thrift_cow/visitors/PathVisitor.h index 05f6221cbf1f1..48ed8c968debd 100644 --- a/fboss/thrift_cow/visitors/PathVisitor.h +++ b/fboss/thrift_cow/visitors/PathVisitor.h @@ -31,10 +31,8 @@ using PathIter = typename std::vector::const_iterator; // instantiations class BasePathVisitorOperator { public: - template + template class SerializableWrapper : public Serializable { - using TC = apache::thrift::type_class::structure; - public: explicit SerializableWrapper(TType& node) : node_(node) {} @@ -68,7 +66,7 @@ class BasePathVisitorOperator { virtual ~BasePathVisitorOperator() = default; - template + template inline void visitTyped(Node& node, pv_detail::PathIter begin, pv_detail::PathIter end) requires(is_cow_type_v) @@ -82,13 +80,13 @@ class BasePathVisitorOperator { } } - template + template inline void 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); + SerializableWrapper wrapper(node); if constexpr (std::is_const_v) { cvisit(wrapper, begin, end); cvisit(wrapper); @@ -221,7 +219,7 @@ template struct LambdaPathVisitorOperator { explicit LambdaPathVisitorOperator(Func&& f) : f_(std::forward(f)) {} - template + template inline auto visitTyped(Node& node, pv_detail::PathIter begin, pv_detail::PathIter end) -> std::invoke_result_t< @@ -232,7 +230,7 @@ struct LambdaPathVisitorOperator { return f_(node, begin, end); } - template + template inline auto visitTyped( Node& node, pv_detail::PathIter /* begin */, @@ -252,7 +250,7 @@ visitNode(Node& node, const VisitImplParams& params, PathIter cursor) { if (params.mode == PathVisitMode::FULL || cursor == params.end) { try { - params.op.visitTyped(node, cursor, params.end); + params.op.template visitTyped(node, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -295,7 +293,7 @@ struct PathVisitorImpl> { { try { if (params.mode == PathVisitMode::FULL || cursor == params.end) { - params.op.visitTyped(tObj, cursor, params.end); + params.op.template visitTyped(tObj, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -334,7 +332,7 @@ struct PathVisitorImpl> { { try { if (params.mode == PathVisitMode::FULL || cursor == params.end) { - params.op.visitTyped(node, cursor, params.end); + params.op.template visitTyped(node, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -414,7 +412,7 @@ struct PathVisitorImpl> { { try { if (params.mode == PathVisitMode::FULL || cursor == params.end) { - params.op.visitTyped(tObj, cursor, params.end); + params.op.template visitTyped(tObj, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -443,7 +441,7 @@ struct PathVisitorImpl> { { try { if (params.mode == PathVisitMode::FULL || cursor == params.end) { - params.op.visitTyped(node, cursor, params.end); + params.op.template visitTyped(node, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -515,7 +513,7 @@ struct PathVisitorImpl< { try { if (params.mode == PathVisitMode::FULL || cursor == params.end) { - params.op.visitTyped(tObj, cursor, params.end); + params.op.template visitTyped(tObj, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -547,7 +545,7 @@ struct PathVisitorImpl< { try { if (params.mode == PathVisitMode::FULL || cursor == params.end) { - params.op.visitTyped(node, cursor, params.end); + params.op.template visitTyped(node, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -698,7 +696,7 @@ struct PathVisitorImpl { { try { if (params.mode == PathVisitMode::FULL || cursor == params.end) { - params.op.visitTyped(node, cursor, params.end); + params.op.template visitTyped(node, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -737,7 +735,7 @@ struct PathVisitorImpl { { try { if (params.mode == PathVisitMode::FULL || cursor == params.end) { - params.op.visitTyped(tObj, cursor, params.end); + params.op.template visitTyped(tObj, cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; } @@ -835,7 +833,9 @@ struct PathVisitorImpl { // take a const param. Here we cast away the const and rely on // primitive node's functions throwing an exception if the node is // immutable. - params.op.visitTyped( + params.op.template visitTyped< + std::remove_const_t, + std::remove_const_t>( *const_cast*>(&node), cursor, params.end); if (cursor == params.end) { return ThriftTraverseResult::OK; diff --git a/fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp b/fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp index ec90d0b549e76..c7f3af8de70de 100644 --- a/fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp +++ b/fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp @@ -199,6 +199,73 @@ TYPED_TEST(PathVisitorTests, AccessAtHybridNodeTest) { EXPECT_EQ(getOp.val, newVal); } +TYPED_TEST(PathVisitorTests, AccessAtHybridThriftContainerTest) { + RootTestStruct root; + ParentTestStruct parent; + auto testStruct = createSimpleTestStruct(); + parent.mapOfI32ToMapOfStruct() = {{3, {{"4", std::move(testStruct)}}}}; + root.mapOfI32ToMapOfStruct() = {{1, {{"2", std::move(parent)}}}}; + + auto nodeA = this->initNode(root); + 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); + } + }); + + // Thrift path at thrift container under HybridNode - Access + std::vector path{ + "mapOfI32ToMapOfStruct", + "1", + "2", + "mapOfI32ToMapOfStruct", + "3", + "4", + "hybridMapOfI32ToStruct", + "20", + "structMap"}; + auto result = RootPathVisitor::visit( + *nodeA, path.begin(), path.end(), PathVisitMode::LEAF, processPath); + EXPECT_EQ(result, ThriftTraverseResult::OK); + EXPECT_NE(dyn.find("30"), dyn.items().end()); + cfg::L4PortRange got = facebook::thrift::from_dynamic( + dyn["30"], facebook::thrift::dynamic_format::JSON_1); + EXPECT_EQ(*got.min(), 100); + EXPECT_EQ(*got.max(), 200); + + // Thrift path at thrift container under HybridNode - Set + using TC = apache::thrift::type_class::map< + apache::thrift::type_class::string, + apache::thrift::type_class::structure>; + + std::map newMap; + cfg::L4PortRange newRange; + newRange.min() = 3000; + newRange.max() = 4000; + newMap.emplace("200", std::move(newRange)); + folly::fbstring newVal = + serialize(fsdb::OperProtocol::SIMPLE_JSON, newMap); + SetEncodedPathVisitorOperator setOp(fsdb::OperProtocol::SIMPLE_JSON, newVal); + + result = RootPathVisitor::visit( + *nodeA, path.begin(), path.end(), PathVisitMode::LEAF, setOp); + EXPECT_EQ(result, ThriftTraverseResult::OK); + + // Thrift path at thrift container under HybridNode - Get + GetEncodedPathVisitorOperator getOp(fsdb::OperProtocol::SIMPLE_JSON); + result = RootPathVisitor::visit( + *nodeA, path.begin(), path.end(), PathVisitMode::LEAF, getOp); + EXPECT_EQ(result, ThriftTraverseResult::OK); + EXPECT_EQ(getOp.val, newVal); +} + TYPED_TEST(PathVisitorTests, AccessFieldInContainer) { auto structA = createSimpleTestStruct(); auto nodeA = std::make_shared