add type and protocol fields to debug string

Summary: Internal : - Motivation : - [User ask](https://fb.workplace.com/groups/1276923829803334/permalink/1406651230163926/) - ICSP has been using JSON in their Any objects just to be able to log/debug the contents of `AnyStruct`s reasonably. Rather than this being a forcing function to support multiple protocols in Any or for users to implement temporary workarounds like this CLI-based deserialization in D44525331, we want to fix the root cause here of poor debuggability when using Thrift Any. - Details : https://docs.google.com/document/d/18senTbsbyQBYHvLnl4c0WZgsmsdFzMsiZbNIeh8Pn3A/edit#heading=h.tevuif6iiq0 Pretty print `type` and `protocol` fields Reviewed By: thedavekwon Differential Revision: D52314629 fbshipit-source-id: 685ae789f04c8cc31ea9a87d9913373eaaff7b8e
facebook · Jan 6, 2024 · 1f7224d · 1f7224d
1 parent 45de99a
commit 1f7224d
Show file tree

Hide file tree

Showing 3 changed files with 159 additions and 1 deletion.
diff --git a/thrift/lib/cpp2/type/AnyDebugWriter.cpp b/thrift/lib/cpp2/type/AnyDebugWriter.cpp
@@ -35,14 +35,113 @@ std::string anyDebugString(
 }
 
 namespace detail {
+namespace {
+
+std::string_view appendTypeUri(const type::TypeUri& uri) {
+  switch (uri.getType()) {
+    case apache::thrift::type::TypeUri::uri:
+      return *uri.uri_ref();
+    case apache::thrift::type::TypeUri::typeHashPrefixSha2_256:
+      return uri.typeHashPrefixSha2_256_ref()->c_str();
+    case apache::thrift::type::TypeUri::scopedName:
+    case apache::thrift::type::TypeUri::__EMPTY__:
+      return "(unspecified)";
+  }
+}
+
+void appendType(const type::TypeStruct& type, fmt::memory_buffer& buf);
+
+void appendTypeParams(
+    const std::vector<type::TypeStruct>& types, fmt::memory_buffer& buf) {
+  bool first = true;
+  for (const auto& t : types) {
+    if (!std::exchange(first, false)) {
+      buf.push_back(',');
+    }
+    appendType(t, buf);
+  }
+}
+
+std::string getTypeName(const type::TypeStruct& type) {
+  switch (type.name()->getType()) {
+    case type::TypeName::boolType:
+      return "bool";
+    case type::TypeName::byteType:
+      return "byte";
+    case type::TypeName::i16Type:
+      return "i16";
+    case type::TypeName::i32Type:
+      return "i32";
+    case type::TypeName::i64Type:
+      return "i64";
+    case type::TypeName::floatType:
+      return "float";
+    case type::TypeName::doubleType:
+      return "double";
+    case type::TypeName::stringType:
+      return "string";
+    case type::TypeName::binaryType:
+      return "binary";
+    case type::TypeName::enumType:
+      return fmt::format(
+          "enum<{}>", appendTypeUri(*type.name()->enumType_ref()));
+    case type::TypeName::typedefType:
+      return fmt::format(
+          "typedef<{}>", appendTypeUri(*type.name()->typedefType_ref()));
+    case type::TypeName::structType:
+      return fmt::format(
+          "struct<{}>", appendTypeUri(*type.name()->structType_ref()));
+    case type::TypeName::unionType:
+      return fmt::format(
+          "union<{}>", appendTypeUri(*type.name()->unionType_ref()));
+    case type::TypeName::exceptionType:
+      return fmt::format(
+          "exception<{}>", appendTypeUri(*type.name()->exceptionType_ref()));
+    case type::TypeName::listType:
+      return "list";
+    case type::TypeName::setType:
+      return "set";
+    case type::TypeName::mapType:
+      return "map";
+    case type::TypeName::__EMPTY__:
+      return "void";
+  }
+}
+
+void appendType(const type::TypeStruct& type, fmt::memory_buffer& buf) {
+  fmt::format_to(std::back_inserter(buf), "{}", getTypeName(type));
+  if (!type.get_params().empty()) {
+    buf.push_back('<');
+    appendTypeParams(type.get_params(), buf);
+    buf.push_back('>');
+  }
+}
+} // namespace
 
 uint32_t AnyDebugWriter::write(const type::AnyStruct& any) {
   uint32_t s = 0;
   s += writeStructBegin("AnyStruct");
   const type::Type& type = *any.type();
   type::BaseType baseType = type.baseType();
 
-  // TODO(rashmim): Add type and protocol fields
+  s += writeFieldBegin(
+      "type",
+      TType::T_STRUCT,
+      folly::to_underlying(
+          op::get_field_id<type::AnyStruct, apache::thrift::ident::type>::
+              value));
+  s += write(type);
+  s += writeFieldEnd();
+
+  s += writeFieldBegin(
+      "protocol",
+      TType::T_STRUCT,
+      folly::to_underlying(
+          op::get_field_id<type::AnyStruct, apache::thrift::ident::protocol>::
+              value));
+  s += writeString(any.protocol()->name());
+  s += writeFieldEnd();
+
   s += writeFieldBegin(
       "data",
       type::toTType(baseType),
@@ -210,5 +309,11 @@ uint32_t AnyDebugWriter::writeUnregisteredAnyImpl(
     }
   }
 }
+
+uint32_t AnyDebugWriter::write(const type::Type& type) {
+  fmt::memory_buffer buf;
+  appendType(type.toThrift(), buf);
+  return writeString(fmt::to_string(buf));
+}
 } // namespace detail
 } // namespace apache::thrift
diff --git a/thrift/lib/cpp2/type/AnyDebugWriter.h b/thrift/lib/cpp2/type/AnyDebugWriter.h
@@ -31,6 +31,8 @@ class AnyDebugWriter : public DebugProtocolWriter {
 
   uint32_t write(const type::AnyData& any);
 
+  uint32_t write(const type::Type& type);
+
  private:
   uint32_t writeUnregisteredAny(const type::AnyStruct& any);
 

diff --git a/thrift/lib/cpp2/type/AnyDebugWriterTest.cpp b/thrift/lib/cpp2/type/AnyDebugWriterTest.cpp
@@ -22,14 +22,59 @@ namespace apache::thrift {
 
 using apache::thrift::test::NestedAny;
 
+std::string anyTypeDebugString(const type::Type& obj) {
+  folly::IOBufQueue queue;
+  detail::AnyDebugWriter proto(false);
+  proto.setOutput(&queue);
+  proto.write(obj);
+  std::unique_ptr<folly::IOBuf> buf = queue.move();
+  folly::ByteRange br = buf->coalesce();
+  return std::string(reinterpret_cast<const char*>(br.data()), br.size());
+}
+
+template <typename Tag>
+std::string tagToTypeString() {
+  return anyTypeDebugString(type::Type::get<Tag>());
+}
+
 TEST(AnyTest, any_struct_fields) {
   auto any = type::toAnyData<type::i16_t>();
   auto ret = anyDebugString(any);
+  EXPECT_NE(ret.find("1: type (struct) = \"i16\""), ret.npos) << ret;
+  EXPECT_NE(ret.find("2: protocol (struct) = \"Compact\""), ret.npos) << ret;
   EXPECT_NE(
       ret.find(fmt::format("3: data (i16) = {}", type::kMagicString)), ret.npos)
       << ret;
 }
 
+TEST(AnyTest, type_str) {
+  // valid TypeStruct created from Tags
+  EXPECT_EQ(tagToTypeString<type::i16_t>(), "\"i16\"");
+  EXPECT_EQ(tagToTypeString<type::i32_t>(), "\"i32\"");
+  EXPECT_EQ(tagToTypeString<type::bool_t>(), "\"bool\"");
+  EXPECT_EQ(tagToTypeString<type::byte_t>(), "\"byte\"");
+  EXPECT_EQ(tagToTypeString<type::string_t>(), "\"string\"");
+  EXPECT_EQ(tagToTypeString<type::binary_t>(), "\"binary\"");
+  EXPECT_EQ(tagToTypeString<type::list<type::i32_t>>(), "\"list<i32>\"");
+  EXPECT_EQ(tagToTypeString<type::set<type::i32_t>>(), "\"set<i32>\"");
+  EXPECT_EQ(
+      (tagToTypeString<type::map<type::i32_t, type::float_t>>()),
+      "\"map<i32,float>\"");
+  EXPECT_EQ(
+      tagToTypeString<type::struct_t<test::AnyTestStruct>>(),
+      fmt::format("\"struct<{}>\"", thrift::uri<test::AnyTestStruct>()));
+  EXPECT_EQ(
+      tagToTypeString<type::exception_t<test::AnyTestException>>(),
+      fmt::format("\"exception<{}>\"", thrift::uri<test::AnyTestException>()));
+
+  // invalid TypeStruct
+  auto map_type = type::Type::get<type::map<type::i32_t, type::float_t>>();
+  auto map_type_struct = map_type.toThrift();
+  map_type_struct.name().ensure().boolType_ref() = type::Void::Unused;
+  EXPECT_EQ(
+      anyTypeDebugString(type::Type(map_type_struct)), "\"bool<i32,float>\"");
+}
+
 template <typename>
 class AnyTestFixture : public ::testing::Test {};
 
@@ -48,6 +93,8 @@ void verifyDebugString(const type::AnyData& any) {
   } else {
     check(type::kMagicString);
   }
+  check(anyTypeDebugString(any.type()));
+  check(any.protocol().name());
 }
 
 TYPED_TEST(AnyTestFixture, unregistered_compact) {
@@ -93,6 +140,10 @@ TYPED_TEST(AnyTestFixture, unregistered_json) {
   auto encoded_str = folly::cEscape<std::string>(
       std::string(reinterpret_cast<const char*>(br.data()), br.size()));
   EXPECT_NE(ret.find(encoded_str.data()), ret.npos) << ret << encoded_str;
+  auto type_str = anyTypeDebugString(*any.type());
+  EXPECT_NE(ret.find(type_str), ret.npos) << ret << type_str;
+  auto protocol_str = any.protocol()->name();
+  EXPECT_NE(ret.find(protocol_str), ret.npos) << ret << protocol_str;
 }
 
 } // namespace apache::thrift