Skip to content

Commit

Permalink
THRIFT-5835 Allow exceptions to be used as regular struct datatype
Browse files Browse the repository at this point in the history
Initial feature testcase added, compiler checks disabled.
Patch: Jens Geyer
  • Loading branch information
Jens-G committed Nov 23, 2024
1 parent 58d9ff5 commit 39ce210
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 2 deletions.
6 changes: 6 additions & 0 deletions compiler/cpp/src/thrift/parse/t_function.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,23 @@ class t_function : public t_doc {

void validate() const {
get_returntype()->validate();

#ifndef ALLOW_EXCEPTIONS_AS_TYPE
if (get_returntype()->get_true_type()->is_xception()) {
failure("method %s(): exception type \"%s\" cannot be used as function return", get_name().c_str(), get_returntype()->get_name().c_str());
}
#endif

std::vector<t_field*>::const_iterator it;
std::vector<t_field*> list = get_arglist()->get_members();
for(it=list.begin(); it != list.end(); ++it) {
(*it)->get_type()->validate();

#ifndef ALLOW_EXCEPTIONS_AS_TYPE
if( (*it)->get_type()->get_true_type()->is_xception()) {
failure("method %s(): exception type \"%s\" cannot be used as function argument %s", get_name().c_str(), (*it)->get_type()->get_name().c_str(), (*it)->get_name().c_str());
}
#endif
}
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/cpp/src/thrift/parse/t_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ class t_list : public t_container {
bool is_list() const override { return true; }

void validate() const {
#ifndef ALLOW_EXCEPTIONS_AS_TYPE
if( get_elem_type()->get_true_type()->is_xception()) {
failure("exception type \"%s\" cannot be used inside a list", get_elem_type()->get_name().c_str());
}
#endif
}

private:
Expand Down
2 changes: 2 additions & 0 deletions compiler/cpp/src/thrift/parse/t_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ class t_map : public t_container {
bool is_map() const override { return true; }

void validate() const {
#ifndef ALLOW_EXCEPTIONS_AS_TYPE
if( get_key_type()->get_true_type()->is_xception()) {
failure("exception type \"%s\" cannot be used inside a map", get_key_type()->get_name().c_str());
}
if( get_val_type()->get_true_type()->is_xception()) {
failure("exception type \"%s\" cannot be used inside a map", get_val_type()->get_name().c_str());
}
#endif
}

private:
Expand Down
2 changes: 2 additions & 0 deletions compiler/cpp/src/thrift/parse/t_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ class t_set : public t_container {
bool is_set() const override { return true; }

void validate() const {
#ifndef ALLOW_EXCEPTIONS_AS_TYPE
if( get_elem_type()->get_true_type()->is_xception()) {
failure("exception type \"%s\" cannot be used inside a set", get_elem_type()->get_name().c_str());
}
#endif
}

private:
Expand Down
6 changes: 4 additions & 2 deletions compiler/cpp/src/thrift/parse/t_struct.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,8 @@ class t_struct : public t_type {
const members_type& get_sorted_members() const { return members_in_id_order_; }

bool is_struct() const override { return !is_xception_; }

bool is_xception() const override { return is_xception_; }

bool is_method_xcepts() const override { return is_method_xcepts_; }
bool is_union() const { return is_union_; }

t_field* get_field_by_name(std::string field_name) {
Expand Down Expand Up @@ -144,11 +143,14 @@ class t_struct : public t_type {
std::vector<t_field*> list = get_members();
for(it=list.begin(); it != list.end(); ++it) {
(*it)->get_type()->validate();

#ifndef ALLOW_EXCEPTIONS_AS_TYPE
if (!is_method_xcepts_) { // this is in fact the only legal usage for any exception type
if( (*it)->get_type()->get_true_type()->is_xception()) {
failure("%s %s: exception type \"%s\" cannot be used as member field type %s", what.c_str(), get_name().c_str(), (*it)->get_type()->get_name().c_str(), (*it)->get_name().c_str());
}
}
#endif
}
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/cpp/src/thrift/parse/t_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include <stdint.h>
#include <string>

#define ALLOW_EXCEPTIONS_AS_TYPE

class t_program;

/**
Expand Down Expand Up @@ -54,6 +56,7 @@ class t_type : public t_doc {
virtual bool is_enum() const { return false; }
virtual bool is_struct() const { return false; }
virtual bool is_xception() const { return false; }
virtual bool is_method_xcepts() const { return false; }
virtual bool is_container() const { return false; }
virtual bool is_list() const { return false; }
virtual bool is_set() const { return false; }
Expand Down
56 changes: 56 additions & 0 deletions test/ExceptionStruct.thrift
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

namespace * test.ExceptionStruct

enum ErrorCode {
GenericError,
ServerOverload,
InvalidData
}

struct GetRequest {
1: string id
2: binary data // some arbitrary data
}

struct GetResponse {
1: i32 job_nr
2: binary data // some arbitrary data
}

struct BatchGetRequest {
1: list<GetRequest> requests
}

struct BatchGetResponse {
1: map<string, GetRequest> responses, // key is id
2: map<string, SomeException> errors, // key is id
}

exception SomeException {
2: ErrorCode error
}

service Foo {
GetResponse get(1: GetRequest request) throws(1: SomeException error);
BatchGetResponse batchGet(1: BatchGetRequest request) throws(1: SomeException error); // may or may not be the same exception type, only throw exception when full request failed
}

# eof

0 comments on commit 39ce210

Please sign in to comment.