Skip to content

Commit

Permalink
Check for @python.Name for exception messages in thrift py3/python
Browse files Browse the repository at this point in the history
Summary: In both thrift python and thrift py3 exception messages were just using the field name directly without running it through `py3::get_py3_name` which could result in grabbing the wrong field name (if it had a python.Name annotation). Fixing that here.

Reviewed By: yoney

Differential Revision: D60245118

fbshipit-source-id: a68107fad4363c01863b9e461386a7a40d6d9bf6
  • Loading branch information
Satish Kumar authored and facebook-github-bot committed Jul 26, 2024
1 parent 772a848 commit f5a5f5a
Show file tree
Hide file tree
Showing 34 changed files with 3,476 additions and 27 deletions.
2 changes: 1 addition & 1 deletion thrift/compiler/generate/t_mstch_py3_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ class py3_mstch_struct : public mstch_struct {
mstch::node exceptionMessage() {
const auto* message_field =
dynamic_cast<const t_exception&>(*struct_).get_message_field();
return message_field ? message_field->name() : "";
return message_field ? py3::get_py3_name(*message_field) : "";
}

mstch::node py3_fields() { return make_mstch_fields(py3_fields_); }
Expand Down
2 changes: 1 addition & 1 deletion thrift/compiler/generate/t_mstch_python_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ class python_mstch_struct : public mstch_struct {
mstch::node exception_message() {
const auto* message_field =
dynamic_cast<const t_exception&>(*struct_).get_message_field();
return message_field ? message_field->name() : "";
return message_field ? py3::get_py3_name(*message_field) : "";
}

mstch::node adapter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ StructMetadata<::cpp2::Serious>::gen(ThriftMetadata& metadata) {
module_Serious.is_union() = false;
static const auto* const
module_Serious_fields = new std::array<EncodedThriftField, 1>{ {
{ 1, "sonnet", true, std::make_unique<Primitive>(ThriftPrimitiveType::THRIFT_STRING_TYPE), std::vector<ThriftConstStruct>{ *cvStruct("thrift.ExceptionMessage", { }).cv_struct_ref(), }}, }};
{ 1, "sonnet", true, std::make_unique<Primitive>(ThriftPrimitiveType::THRIFT_STRING_TYPE), std::vector<ThriftConstStruct>{ *cvStruct("thrift.ExceptionMessage", { }).cv_struct_ref(), *cvStruct("python.Name", { {"name", cvString("not_sonnet") } }).cv_struct_ref(), }}, }};
for (const auto& f : *module_Serious_fields) {
::apache::thrift::metadata::ThriftField field;
field.id() = f.id;
Expand Down Expand Up @@ -204,7 +204,7 @@ void ExceptionMetadata<::cpp2::Serious>::gen(ThriftMetadata& metadata) {
module_Serious.name() = "module.Serious";
static const auto* const
module_Serious_fields = new std::array<EncodedThriftField, 1>{ {
{ 1, "sonnet", true, std::make_unique<Primitive>(ThriftPrimitiveType::THRIFT_STRING_TYPE), std::vector<ThriftConstStruct>{ *cvStruct("thrift.ExceptionMessage", { }).cv_struct_ref(), }}, }};
{ 1, "sonnet", true, std::make_unique<Primitive>(ThriftPrimitiveType::THRIFT_STRING_TYPE), std::vector<ThriftConstStruct>{ *cvStruct("thrift.ExceptionMessage", { }).cv_struct_ref(), *cvStruct("python.Name", { {"name", cvString("not_sonnet") } }).cv_struct_ref(), }}, }};
for (const auto& f : *module_Serious_fields) {
::apache::thrift::metadata::ThriftField field;
field.id() = f.id;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Autogenerated by Thrift for thrift/annotation/python.thrift
//
// DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING
// @generated

package python

import (
thrift "github.com/facebook/fbthrift/thrift/lib/go/thrift"
)

// (needed to ensure safety because of naive import list construction)
var _ = thrift.ZERO

var GoUnusedProtection__ int

Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
// Autogenerated by Thrift for thrift/annotation/python.thrift
//
// DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING
// @generated

package python

import (
thrift "github.com/facebook/fbthrift/thrift/lib/go/thrift"
metadata "github.com/facebook/fbthrift/thrift/lib/thrift/metadata"
)

// mapsCopy is a copy of maps.Copy from Go 1.21
// TODO: remove mapsCopy once we can safely upgrade to Go 1.21 without requiring any rollback.
func mapsCopy[M1 ~map[K]V, M2 ~map[K]V, K comparable, V any](dst M1, src M2) {
for k, v := range src {
dst[k] = v
}
}

// (needed to ensure safety because of naive import list construction)
var _ = thrift.ZERO
// TODO: uncomment when can safely upgrade to Go 1.21 without requiring any rollback.
// var _ = maps.Copy[map[int]int, map[int]int]
var _ = metadata.GoUnusedProtection__

// Premade Thrift types
var (
premadeThriftType_string = metadata.NewThriftType().SetTPrimitive(
metadata.ThriftPrimitiveType_THRIFT_STRING_TYPE.Ptr(),
)
premadeThriftType_bool = metadata.NewThriftType().SetTPrimitive(
metadata.ThriftPrimitiveType_THRIFT_BOOL_TYPE.Ptr(),
)
)

var structMetadatas = []*metadata.ThriftStruct{
metadata.NewThriftStruct().
SetName("python.Py3Hidden").
SetIsUnion(false),
metadata.NewThriftStruct().
SetName("python.Flags").
SetIsUnion(false),
metadata.NewThriftStruct().
SetName("python.Name").
SetIsUnion(false).
SetFields(
[]*metadata.ThriftField{
metadata.NewThriftField().
SetId(1).
SetName("name").
SetIsOptional(false).
SetType(premadeThriftType_string),
},
),
metadata.NewThriftStruct().
SetName("python.Adapter").
SetIsUnion(false).
SetFields(
[]*metadata.ThriftField{
metadata.NewThriftField().
SetId(1).
SetName("name").
SetIsOptional(false).
SetType(premadeThriftType_string),
metadata.NewThriftField().
SetId(2).
SetName("typeHint").
SetIsOptional(false).
SetType(premadeThriftType_string),
},
),
metadata.NewThriftStruct().
SetName("python.UseCAPI").
SetIsUnion(false).
SetFields(
[]*metadata.ThriftField{
metadata.NewThriftField().
SetId(1).
SetName("serialize").
SetIsOptional(false).
SetType(premadeThriftType_bool),
},
),
}

var exceptionMetadatas = []*metadata.ThriftException{
}

var enumMetadatas = []*metadata.ThriftEnum{
}

var serviceMetadatas = []*metadata.ThriftService{
}

// GetThriftMetadata returns complete Thrift metadata for current and imported packages.
func GetThriftMetadata() *metadata.ThriftMetadata {
allEnums := GetEnumsMetadata()
allStructs := GetStructsMetadata()
allExceptions := GetExceptionsMetadata()
allServices := GetServicesMetadata()

return metadata.NewThriftMetadata().
SetEnums(allEnums).
SetStructs(allStructs).
SetExceptions(allExceptions).
SetServices(allServices)
}

// GetEnumsMetadata returns Thrift metadata for enums in the current and recursively included packages.
func GetEnumsMetadata() map[string]*metadata.ThriftEnum {
allEnumsMap := make(map[string]*metadata.ThriftEnum)

// Add enum metadatas from the current program...
for _, enumMetadata := range enumMetadatas {
allEnumsMap[enumMetadata.GetName()] = enumMetadata
}

// ...now add enum metadatas from recursively included programs.

return allEnumsMap
}

// GetStructsMetadata returns Thrift metadata for structs in the current and recursively included packages.
func GetStructsMetadata() map[string]*metadata.ThriftStruct {
allStructsMap := make(map[string]*metadata.ThriftStruct)

// Add struct metadatas from the current program...
for _, structMetadata := range structMetadatas {
allStructsMap[structMetadata.GetName()] = structMetadata
}

// ...now add struct metadatas from recursively included programs.

return allStructsMap
}

// GetExceptionsMetadata returns Thrift metadata for exceptions in the current and recursively included packages.
func GetExceptionsMetadata() map[string]*metadata.ThriftException {
allExceptionsMap := make(map[string]*metadata.ThriftException)

// Add exception metadatas from the current program...
for _, exceptionMetadata := range exceptionMetadatas {
allExceptionsMap[exceptionMetadata.GetName()] = exceptionMetadata
}

// ...now add exception metadatas from recursively included programs.

return allExceptionsMap
}

// GetServicesMetadata returns Thrift metadata for services in the current and recursively included packages.
func GetServicesMetadata() map[string]*metadata.ThriftService {
allServicesMap := make(map[string]*metadata.ThriftService)

// Add service metadatas from the current program...
for _, serviceMetadata := range serviceMetadatas {
allServicesMap[serviceMetadata.GetName()] = serviceMetadata
}

// ...now add service metadatas from recursively included programs.

return allServicesMap
}

// GetThriftMetadataForService returns Thrift metadata for the given service.
func GetThriftMetadataForService(scopedServiceName string) *metadata.ThriftMetadata {
thriftMetadata := GetThriftMetadata()

allServicesMap := thriftMetadata.GetServices()
relevantServicesMap := make(map[string]*metadata.ThriftService)

serviceMetadata := allServicesMap[scopedServiceName]
// Visit and record all recursive parents of the target service.
for serviceMetadata != nil {
relevantServicesMap[serviceMetadata.GetName()] = serviceMetadata
if serviceMetadata.IsSetParent() {
serviceMetadata = allServicesMap[serviceMetadata.GetParent()]
} else {
serviceMetadata = nil
}
}

thriftMetadata.SetServices(relevantServicesMap)

return thriftMetadata
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Autogenerated by Thrift for thrift/annotation/python.thrift
//
// DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING
// @generated

package python

Loading

0 comments on commit f5a5f5a

Please sign in to comment.