Allow converting individual nodes to TypeSystem

Summary:
Avoids the need to roundtrip from a definition node to its URI to convert between systems, which also adds support for nodes without URIs.
Accessing the URI of a node that does not have one is changed to throw, which will ensure we don't accidentally serialize it as part of the type system, use it to create an Any, or otherwise leak the partial nodes.

Reviewed By: pranavtbhat

Differential Revision: D75178791

fbshipit-source-id: c09b43ec6c2a190c5749fb2c22e866e51f152c0b

Author: iahs

third-party/thrift/src/thrift/lib/cpp2/dynamic/TypeSystem.cpp

@@ -319,7 +319,14 @@ std::string_view kindToString(TypeRef::Kind k) noexcept {
   folly::assume_unreachable();
 }
 
-const Uri& DefinitionRef::uri() const noexcept {
+const Uri& DefinitionNode::uri() const {
+  if (uri_.empty()) {
+    folly::throw_exception<InvalidTypeError>("Type does not have URI set.");
+  }
+  return uri_;
+}
+
+const Uri& DefinitionRef::uri() const {
   return visit(std::mem_fn(&DefinitionNode::uri));
 }
 

third-party/thrift/src/thrift/lib/cpp2/dynamic/TypeSystem.h

@@ -567,7 +567,11 @@ struct FastFieldHandle {
 
 class DefinitionNode {
  public:
-  const Uri& uri() const noexcept { return uri_; }
+  /**
+   * Returns the URI associated with this definition.
+   * Throws `InvalidTypeError` if the type does not have a URI.
+   */
+  const Uri& uri() const;
 
  protected:
   Uri uri_;
@@ -790,10 +794,10 @@ class DefinitionRef final {
   }
 
   /**
-   * Returns the URI associated with this definition. Since all user-defined
-   * types must have URIs, this function is infallible.
+   * Returns the URI associated with this definition.
+   * Throws `InvalidTypeError` if the type does not have a URI.
    */
-  const Uri& uri() const noexcept;
+  const Uri& uri() const;
 
   /**
    * An `std::visit`-like API for pattern-matching on the active variant

third-party/thrift/src/thrift/lib/cpp2/schema/SyntaxGraph.cpp

@@ -712,7 +712,6 @@ class TypeSystemFacade final : public type_system::TypeSystem {
     return {};
   }
 
- private:
   // Convert the definition to TypeSystem's representation.
   // The approach is:
   // 1. Traverse the root definition node's fields to gather the set of
@@ -855,6 +854,7 @@ class TypeSystemFacade final : public type_system::TypeSystem {
     });
   }
 
+ private:
   type_system::TypeRef convertType(const TypeRef& type) {
     return type.trueType().visit(
         [](const Primitive& primitive) {
@@ -933,6 +933,24 @@ type_system::TypeSystem& SyntaxGraph::asTypeSystem() {
       "SyntaxGraph instance does not support URI-based lookup");
 }
 
+type_system::DefinitionRef SyntaxGraph::asTypeSystemDefinitionRef(
+    const DefinitionNode& node) {
+  auto& facade = static_cast<TypeSystemFacade&>(asTypeSystem());
+  return facade.convertUserDefinedType(&node);
+}
+const type_system::StructNode& SyntaxGraph::asTypeSystemStructNode(
+    const StructNode& node) {
+  return asTypeSystemDefinitionRef(node.definition()).asStruct();
+}
+const type_system::UnionNode& SyntaxGraph::asTypeSystemUnionNode(
+    const UnionNode& node) {
+  return asTypeSystemDefinitionRef(node.definition()).asUnion();
+}
+const type_system::EnumNode& SyntaxGraph::asTypeSystemEnumNode(
+    const EnumNode& node) {
+  return asTypeSystemDefinitionRef(node.definition()).asEnum();
+}
+
 } // namespace apache::thrift::syntax_graph
 
 #endif // THRIFT_SCHEMA_AVAILABLE

third-party/thrift/src/thrift/lib/cpp2/schema/SyntaxGraph.h

@@ -45,12 +45,9 @@
 #include <vector>
 
 #include <thrift/common/tree_printer.h>
+#include <thrift/lib/cpp2/dynamic/TypeSystem.h>
 #include <thrift/lib/cpp2/schema/gen-cpp2/syntax_graph_types.h>
 
-namespace apache::thrift::type_system {
-class TypeSystem;
-}
-
 namespace apache::thrift::syntax_graph {
 
 /**
@@ -1575,6 +1572,17 @@ class SyntaxGraph final : public detail::WithDebugPrinting<SyntaxGraph> {
    */
   type_system::TypeSystem& asTypeSystem();
 
+  /**
+   * Allows converting a SyntaxGraph node into its corresponding TypeSystem
+   * node. Equivalent to `asTypeSystem().getUserDefinedType(node.uri())`, but
+   * more efficient and supports nodes without URIs.
+   */
+  type_system::DefinitionRef asTypeSystemDefinitionRef(
+      const DefinitionNode& node);
+  const type_system::StructNode& asTypeSystemStructNode(const StructNode& node);
+  const type_system::UnionNode& asTypeSystemUnionNode(const UnionNode& node);
+  const type_system::EnumNode& asTypeSystemEnumNode(const EnumNode& node);
+
   explicit SyntaxGraph(std::unique_ptr<detail::Resolver> resolver);
 
   void printTo(

third-party/thrift/src/thrift/lib/cpp2/schema/test/SyntaxGraphTest.cpp

@@ -711,6 +711,7 @@ TEST_F(ServiceSchemaTest, asTypeSystem) {
   EXPECT_EQ(structNode.fields()[0].identity().id(), FieldId{1});
   EXPECT_EQ(structNode.fields()[0].type().asStruct().uri(), uri);
   EXPECT_EQ(&structNode.fields()[0].type().asStruct(), &structNode);
+  EXPECT_EQ(&syntaxGraph.asTypeSystemStructNode(def->asStruct()), &structNode);
 
   uri = "meta.com/thrift_test/TestEnum";
   const type_system::EnumNode& enumNode =