Skip to content

Commit

Permalink
PathVisitor (Set) implementation 1/n
Browse files Browse the repository at this point in the history
Summary: data type Set, similar as other types framework

Differential Revision:
D64510649

Privacy Context Container: L1125642

fbshipit-source-id: f58b7f65a9047fe457b24fa26bff9a297f755b8b
  • Loading branch information
Wei-Cheng Lin authored and facebook-github-bot committed Nov 16, 2024
1 parent fe39636 commit 50d09a8
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 13 deletions.
4 changes: 3 additions & 1 deletion fboss/thrift_cow/nodes/tests/test.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ struct TestStruct {
allow_skip_thrift_cow = true,
);
33: map<i32, map<i32, i32>> hybridMapOfMap (allow_skip_thrift_cow = true);
// 34: map<i32, set<string>> mapOfI32ToSetOfString; // (allow_skip_thrift_cow = true);
34: map<i32, set<string>> mapOfI32ToSetOfString (
allow_skip_thrift_cow = true,
);
}

struct ParentTestStruct {
Expand Down
77 changes: 70 additions & 7 deletions fboss/thrift_cow/visitors/PathVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,17 +288,80 @@ struct PathVisitorImpl<apache::thrift::type_class::set<ValueTypeClass>> {
return pv_detail::visitNode<TC>(node, params, cursor);
}

template <typename Obj, typename Op>
static inline ThriftTraverseResult
visit(Obj& tObj, const VisitImplParams<Op>& params, PathIter cursor)
requires(!is_cow_type_v<Obj> && !is_field_type_v<Obj>)
{
try {
if (params.mode == PathVisitMode::FULL || cursor == params.end) {
params.op.visitTyped(tObj, cursor, params.end);
if (cursor == params.end) {
return ThriftTraverseResult::OK;
}
}
} catch (const std::exception& ex) {
XLOG(ERR) << "Exception while traversing path: "
<< folly::join("/", params.begin, params.end)
<< " at: " << (cursor == params.end ? "(end)" : *cursor)
<< ", exception: " << ex.what();
return ThriftTraverseResult::VISITOR_EXCEPTION;
}
using ValueTType = typename Obj::value_type;

// Get value
auto token = *cursor++;

if (auto value = tryParseKey<ValueTType, ValueTypeClass>(token)) {
if (auto it = tObj.find(*value); it != tObj.end()) {
// Recurse further
return PathVisitorImpl<ValueTypeClass>::visit(*it, params, cursor);
} else {
return ThriftTraverseResult::NON_EXISTENT_NODE;
}
}
// if we get here, we must have a malformed value
return ThriftTraverseResult::INVALID_SET_MEMBER;
}

template <typename Node, typename Op>
static ThriftTraverseResult
visit(Node& /* node */, const VisitImplParams<Op>& params, PathIter cursor)
static ThriftTraverseResult visit(
Node& node /* node */,
const VisitImplParams<Op>& params,
PathIter cursor)
// only enable for HybridNode types
requires(std::is_same_v<typename Node::CowType, HybridNodeType>)
{
// TODO: implement specialization for hybrid nodes
XLOG(ERR) << "Unimplemented visitation for hybrid node: path: "
<< folly::join("/", params.begin, params.end)
<< " at: " << (cursor == params.end ? "(end)" : *cursor);
return ThriftTraverseResult::VISITOR_EXCEPTION;
try {
if (params.mode == PathVisitMode::FULL || cursor == params.end) {
params.op.visitTyped(node, cursor, params.end);
if (cursor == params.end) {
return ThriftTraverseResult::OK;
}
}
} catch (const std::exception& ex) {
XLOG(ERR) << "Exception while traversing path: "
<< folly::join("/", params.begin, params.end)
<< " at: " << (cursor == params.end ? "(end)" : *cursor)
<< ", exception: " << ex.what();
return ThriftTraverseResult::VISITOR_EXCEPTION;
}
auto& tObj = node.ref();
using ValueTType = typename Node::ThriftType::value_type;

// Get value
auto token = *cursor++;

if (auto value = tryParseKey<ValueTType, ValueTypeClass>(token)) {
if (auto it = tObj.find(*value); it != tObj.end()) {
// Recurse further
return PathVisitorImpl<ValueTypeClass>::visit(*it, params, cursor);
} else {
return ThriftTraverseResult::NON_EXISTENT_NODE;
}
}
// if we get here, we must have a malformed value
return ThriftTraverseResult::INVALID_SET_MEMBER;
}

template <typename Fields, typename Op>
Expand Down
35 changes: 34 additions & 1 deletion fboss/thrift_cow/visitors/tests/PathVisitorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,19 @@ TYPED_TEST(PathVisitorTests, AccessField) {
*nodeA, path.begin(), path.end(), PathVisitMode::LEAF, processPath);
EXPECT_EQ(result, ThriftTraverseResult::OK);
EXPECT_EQ(100, dyn.asInt());

// mapOfI32ToSetOfString
path = {"mapOfI32ToSetOfString", "20", "test1"};
result = RootPathVisitor::visit(
*nodeA, path.begin(), path.end(), PathVisitMode::LEAF, processPath);
EXPECT_EQ(result, ThriftTraverseResult::OK);
EXPECT_EQ("test1", dyn.asString());

// mapOfI32ToSetOfString invalid
path = {"mapOfI32ToSetOfString", "20", "invalid_entry"};
result = RootPathVisitor::visit(
*nodeA, path.begin(), path.end(), PathVisitMode::LEAF, processPath);
EXPECT_EQ(result, ThriftTraverseResult::NON_EXISTENT_NODE);
}

TEST(PathVisitorTests, HybridMapPrimitiveAccess) {
Expand Down Expand Up @@ -336,7 +349,10 @@ TEST(PathVisitorTests, HybridMapOfMapAccess) {

TYPED_TEST(PathVisitorTests, AccessFieldInContainer) {
auto structA = createSimpleTestStruct();
auto nodeA = std::make_shared<ThriftStructNode<TestStruct>>(structA);
auto nodeA = std::make_shared<ThriftStructNode<
TestStruct,
ThriftStructResolver<TestStruct, true>,
true>>(structA);

folly::dynamic dyn;
auto processPath = pvlambda([&dyn](auto& node, auto begin, auto end) {
Expand Down Expand Up @@ -541,6 +557,23 @@ TYPED_TEST(PathVisitorTests, VisitWithOperators) {
EXPECT_EQ(getStruct.min(), 666);
EXPECT_EQ(getStruct.max(), 999);
}

{
using TC = apache::thrift::type_class::structure;
std::vector<std::string> path{"mapOfI32ToSetOfString", "20", "test1"};

GetEncodedPathVisitorOperator getOp(fsdb::OperProtocol::SIMPLE_JSON);
auto result = RootPathVisitor::visit(
*nodeA, path.begin(), path.end(), PathVisitMode::LEAF, getOp);
EXPECT_EQ(result, ThriftTraverseResult::OK);

auto encoded = *getOp.val;
auto buf =
folly::IOBuf::wrapBufferAsValue(encoded.data(), encoded.length());
auto getStruct = deserializeBuf<TC, std::string>(
fsdb::OperProtocol::SIMPLE_JSON, std::move(buf));
EXPECT_EQ(getStruct, "test1");
}
}

} // namespace facebook::fboss::thrift_cow::test
7 changes: 4 additions & 3 deletions fboss/thrift_cow/visitors/tests/RecurseVisitorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ folly::dynamic createTestDynamic() {
"listOfPrimitives", dynamic::array())("listOfStructs", dynamic::array())(
"mapOfEnumToI32", dynamic::object())("mapOfI32ToI32", dynamic::object())(
"mapOfI32ToListOfStructs", dynamic::object())(
"mapOfI32ToSetOfString", dynamic::object())(
"mapOfI32ToStruct", dynamic::object())(
"mapOfStringToI32", dynamic::object())(
"mapOfStringToStruct", dynamic::object())("setOfEnum", dynamic::array())(
Expand Down Expand Up @@ -126,6 +127,7 @@ TYPED_TEST(RecurseVisitorTests, TestFullRecurse) {
{{"mapOfEnumToI32"}, dynamic::object()},
{{"mapOfI32ToI32"}, dynamic::object()},
{{"mapOfI32ToListOfStructs"}, dynamic::object()},
{{"mapOfI32ToSetOfString"}, dynamic::object()},
{{"mapOfI32ToStruct"}, dynamic::object()},
{{"mapOfStringToI32"}, dynamic::object()},
{{"mapOfStringToStruct"}, dynamic::object()},
Expand Down Expand Up @@ -211,7 +213,7 @@ TYPED_TEST(RecurseVisitorTests, TestLeafRecurse) {
{{"mapOfEnumToI32"}, testDyn["mapOfEnumToI32"]},
{{"mapOfI32ToI32"}, testDyn["mapOfI32ToI32"]},
{{"mapOfI32ToListOfStructs"}, testDyn["mapOfI32ToListOfStructs"]},
// {{"mapOfI32ToSetOfString"}, testDyn["mapOfI32ToSetOfString"]},
{{"mapOfI32ToSetOfString"}, testDyn["mapOfI32ToSetOfString"]},
{{"mapOfEnumToStruct", "3"}, testDyn["mapOfEnumToStruct"][3]},
{{"hybridMap"}, testDyn["hybridMap"]},
{{"hybridMapOfI32ToStruct"}, testDyn["hybridMapOfI32ToStruct"]},
Expand Down Expand Up @@ -274,8 +276,7 @@ TYPED_TEST(RecurseVisitorTests, TestLeafRecurse) {
{{"31"}, testDyn["hybridStruct"]},
{{"32"}, testDyn["hybridMapOfI32ToStruct"]},
{{"33"}, testDyn["hybridMapOfMap"]},
// {{"34"}, testDyn["mapOfI32ToSetOfString"]}
};
{{"34"}, testDyn["mapOfI32ToSetOfString"]}};

if (this->isHybridStorage()) {
for (const auto& entry : hybridNodes) {
Expand Down
4 changes: 3 additions & 1 deletion fboss/thrift_cow/visitors/tests/VisitorTestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ TestStruct createSimpleTestStruct() {
dynamic::object(20, dynamic::object("min", 400)("max", 600)))(
"mapOfI32ToListOfStructs",
dynamic::object(
20, dynamic::array(dynamic::object("min", 100)("max", 200))));
20, dynamic::array(dynamic::object("min", 100)("max", 200))))(
"mapOfI32ToSetOfString",
dynamic::object(20, dynamic::array("test1", "test2")));

return facebook::thrift::from_dynamic<TestStruct>(
testDyn, facebook::thrift::dynamic_format::JSON_1);
Expand Down

0 comments on commit 50d09a8

Please sign in to comment.