Skip to content

Commit

Permalink
Revert D68911786
Browse files Browse the repository at this point in the history
Summary:
This diff reverts D68911786
API breakage in SyntaxGraph & schema.thrift consumers

Reviewed By: thedavekwon

Differential Revision: D69137050

fbshipit-source-id: c78146d022a710e7995ae68874583998aeb0cb70
  • Loading branch information
generatedunixname89002005232357 authored and facebook-github-bot committed Feb 5, 2025
1 parent e5ea616 commit 6219d89
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 189 deletions.
38 changes: 17 additions & 21 deletions thrift/compiler/sema/schematizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,32 +52,31 @@ std::unique_ptr<t_const_value> val(Enm val) {
std::unique_ptr<t_const_value> val(std::string_view s) {
return val(std::string{s});
}

std::string uri_or_name(const t_named& node) {
if (!node.uri().empty()) {
return node.uri();
}
if (node.program()) {
return node.program()->scope_name(node);
}
return node.name();
}
} // namespace

t_type_ref schematizer::std_type(std::string_view uri) {
return t_type_ref::from_req_ptr(
static_cast<const t_type*>(scope_.find_by_uri(uri)));
}

// Returns the `TypeUri` type & the corresponding Uri value for the given node
schematizer::resolved_uri schematizer::calculate_uri(const t_named& node) {
if (opts_.use_hash) {
return {"definitionKey", identify_definition(node)};
}
if (!node.uri().empty()) {
return {"uri", node.uri()};
}
if (node.program()) {
return {"scopedName", node.program()->scope_name(node)};
}
return {"scopedName", node.name()};
}

std::unique_ptr<t_const_value> schematizer::type_uri(const t_type& type) {
auto ret = t_const_value::make_map();
auto uri = calculate_uri(type);
ret->add_map(val(uri.uri_type), val(std::move(uri.value)));
if (opts_.use_hash) {
ret->add_map(val("definitionKey"), val(identify_definition(type)));
} else if (!type.uri().empty()) {
ret->add_map(val("uri"), val(type.uri()));
} else {
ret->add_map(val("scopedName"), val(type.get_scoped_name()));
}
ret->set_ttype(std_type("facebook.com/thrift/type/TypeUri"));
return ret;
}
Expand Down Expand Up @@ -133,10 +132,7 @@ void schematizer::add_definition(
}

annot->set_ttype(std_type("facebook.com/thrift/type/Annotation"));
auto uri = calculate_uri(*item.type());
// We're ignoring the UriType here, as annotations are stored as
// map<string, Annotation>.
annots->add_map(val(std::move(uri.value)), std::move(annot));
annots->add_map(val(uri_or_name(*item.type())), std::move(annot));
}

// Double write to deprecated externed path (T161963504).
Expand Down
7 changes: 0 additions & 7 deletions thrift/compiler/sema/schematizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,6 @@ class schematizer {
intern_func& intern_value);

std::string_view program_checksum(const t_program& program);

struct resolved_uri {
std::string_view uri_type;
std::string value;
};

resolved_uri calculate_uri(const t_named& node);
};

// Tag for obtaining a compact-encoded schema for the root program via a
Expand Down
174 changes: 13 additions & 161 deletions thrift/compiler/test/schematizer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,19 @@

#include <folly/portability/GTest.h>

using namespace apache::thrift::compiler;
using apache::thrift::compiler::t_const_value;
using apache::thrift::compiler::t_enum;
using apache::thrift::compiler::t_enum_value;
using apache::thrift::compiler::t_field;
using apache::thrift::compiler::t_list;
using apache::thrift::compiler::t_map;
using apache::thrift::compiler::t_primitive_type;
using apache::thrift::compiler::t_program;
using apache::thrift::compiler::t_set;
using apache::thrift::compiler::t_struct;
using apache::thrift::compiler::t_type_ref;
using apache::thrift::compiler::t_typedef;
using apache::thrift::compiler::detail::protocol_value_builder;
using apache::thrift::compiler::detail::schematizer;

template <typename... Args>
std::unique_ptr<t_const_value> val(Args&&... args) {
Expand Down Expand Up @@ -123,17 +133,12 @@ std::unique_ptr<t_struct> make_foo_bar(const t_program* program) {
return struct_ty;
}

std::unique_ptr<t_const_value> make_foo_bar_value() {
TEST(SchematizerTest, wrap_with_protocol_with_struct_ty) {
auto strct = t_const_value::make_map();
strct->add_map(val("foo"), val(1));
strct->add_map(val("bar"), val(2));
strct->add_map(val("baz"), val(3.14));
strct->add_map(val("qux"), val(4.2));
return strct;
}

TEST(SchematizerTest, wrap_with_protocol_with_struct_ty) {
auto strct = make_foo_bar_value();

// With the type mapping, integers will be represented by their types in
// FooBar.
Expand Down Expand Up @@ -347,156 +352,3 @@ TEST(SchematizerTest, wrap_with_protocol_map) {
expectWrappedValue(
*inner_map_val2, t_const_value::CV_INTEGER, "i32Value", 888);
}

constexpr auto UTILITY_CLASSES = {
std::pair(
"StructuredAnnotation",
"facebook.com/thrift/type/StructuredAnnotation"),
std::pair("Annotation", "facebook.com/thrift/type/Annotation"),
std::pair("ProtocolValue", "facebook.com/thrift/protocol/Value"),
std::pair{"TypeUri", "facebook.com/thrift/type/TypeUri"},
};

template <typename F>
void with_schematizer(schematizer::options opts, F&& f) {
source_manager sm;
sm.add_virtual_file("my_prog", "abcd");

t_scope scope{};
auto program = std::make_unique<t_program>("my_prog", "my_prog");

// Add some well-known types to the scope.
auto protocol_value_ty = std::make_unique<t_struct>(
program.get(), "ProtocolValue"); // Can be empty
protocol_value_ty->set_uri("facebook.com/thrift/protocol/Value");
std::vector<std::unique_ptr<t_struct>> utility_types;
for (const auto& [name, uri] : UTILITY_CLASSES) {
utility_types.push_back(std::make_unique<t_struct>(program.get(), name));
utility_types.back()->set_uri(uri);
scope.add_def(*utility_types.back().get());
}

schematizer schematizer{scope, sm, std::move(opts)};
f(*program.get(), schematizer);
}

template <typename F, typename G>
void with_structured_annotation_uris(
const t_const_value& struct_schema,
const std::string_view struct_name,
F&& on_annotation,
G&& on_structured_annotation) {
const auto [attrs_key, attrs] = struct_schema.get_map().at(0);
EXPECT_EQ(attrs_key->get_string(), "attrs");
const auto& definition_attrs = attrs->get_map();

const auto [name_key, name] = definition_attrs.at(0);
EXPECT_EQ(name_key->get_string(), "name");
EXPECT_EQ(name->get_string(), struct_name);

const auto [structured_annotations_key, structured_annotation_ids_set] =
definition_attrs.at(1);
EXPECT_EQ(structured_annotations_key->get_string(), "structuredAnnotations");
const auto& structured_annotations =
structured_annotation_ids_set->get_list();

for (size_t i = 0; i < structured_annotations.size(); ++i) {
const auto annotation_id = structured_annotations.at(i)->get_integer();
on_structured_annotation(i, annotation_id);
}

const auto [annotations_key, annotations_map] = definition_attrs.at(2);
EXPECT_EQ(annotations_key->get_string(), "annotations");
const auto& annotations = annotations_map->get_map();

for (size_t i = 0; i < annotations.size(); ++i) {
const auto [annotation_name, _] = annotations.at(i);
on_annotation(i, annotation_name->get_string());
}
}

template <typename F>
void with_foo_bar_plus_structured_annotations(t_program& program, F f) {
auto foo_bar_with_annotations_ty =
std::make_unique<t_struct>(&program, "FooBarWithStructuredAnnotations");
auto foo_bar_ty = make_foo_bar(&program);
foo_bar_with_annotations_ty->add_structured_annotation(
std::make_unique<t_const>(
&program,
t_type_ref{*foo_bar_ty},
"FooBarWithStructuredAnnotations",
make_foo_bar_value()));

f(*foo_bar_with_annotations_ty, *foo_bar_ty);
}

TEST(SchematizerTest, use_hash_for_structured_annotations) {
for (const bool use_hash : {true, false}) {
schematizer::options opts;
opts.use_hash = use_hash;
size_t id = 0;
std::vector<std::unique_ptr<t_const_value>> interned_values;
opts.intern_value = [&](std::unique_ptr<t_const_value> val, t_program*) {
interned_values.push_back(std::move(val));
return static_cast<schematizer::value_id>(id++);
};

with_schematizer(opts, [&](t_program& program, schematizer& schematizer) {
with_foo_bar_plus_structured_annotations(
program,
[&](const auto& foo_bar_with_annotations_ty, const auto& foo_bar_ty) {
// Generate a schema.thrift representation of
// `FooBarWithStructuredAnnotations`
const std::unique_ptr<t_const_value> struct_schema =
schematizer.gen_schema(foo_bar_with_annotations_ty);

// Verify that the URIs of the structured annotations are
// hashed
with_structured_annotation_uris(
*struct_schema,
"FooBarWithStructuredAnnotations",
[&](size_t annotation_idx, const std::string& annotation_name) {
ASSERT_EQ(annotation_idx, 0); // There's only one annotation

if (use_hash) {
EXPECT_TRUE(
annotation_name.find("FooBar") == std::string::npos);
EXPECT_EQ(
annotation_name,
schematizer.identify_definition(foo_bar_ty));
} else {
EXPECT_EQ(
annotation_name,
"my_prog.FooBar"); // This should not be hashed
}
},
[&](size_t annotation_idx, size_t id) {
ASSERT_EQ(
annotation_idx,
0); // There's only one structured annotation

const auto& interned_annotation =
interned_values.at(id)->get_map();
const auto [type_key, type_uri] = interned_annotation.at(1);
ASSERT_EQ(type_key->get_string(), "type");

// TypeUri should be a union, so check for the appropriate
// field
const auto [type_uri_key, type_uri_value] =
type_uri->get_map().at(0);
if (use_hash) {
EXPECT_EQ(type_uri_key->get_string(), "definitionKey");
EXPECT_EQ(
type_uri_value->get_string(),
schematizer.identify_definition(foo_bar_ty));
} else {
EXPECT_EQ(type_uri_key->get_string(), "scopedName");
EXPECT_EQ(
type_uri_value->get_string(),
"my_prog.FooBar"); // This should not be hashed
}
});
});
});
}
}

0 comments on commit 6219d89

Please sign in to comment.