[jira] [Comment Edited] (THRIFT-1474) Not throw user defined exception when a defined method returns boolean result

["Adrian Muraru (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-1474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13417087#comment-13417087 ] 

Adrian Muraru edited comment on THRIFT-1474 at 7/18/12 2:08 PM:
----------------------------------------------------------------

This worked in previous versions of thrift (not sure when it's been introduced), however what about inspecting the result.x exception before checking the return value in recv_* functions:

{code:title=t_java_generator.cc|borderStyle=solid}
      t_struct* xs = (*f_iter)->get_xceptions();
      const std::vector<t_field*>& xceptions = xs->get_members();
      vector<t_field*>::const_iterator x_iter;
      for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
        f_service_ <<
          indent() << "if (result." << (*x_iter)->get_name() << " != null) {" << endl <<
          indent() << "  throw result." << (*x_iter)->get_name() << ";" << endl <<
          indent() << "}" << endl;
      }

      // Careful, only return _result if not a void function
      if (!(*f_iter)->get_returntype()->is_void()) {
        f_service_ <<
          indent() << "if (result." << generate_isset_check("success") << ") {" << endl <<
          indent() << "  return result.success;" << endl <<
          indent() << "}" << endl;
      }
{code}

Currently all generators look for return.success first and then for result.x. 
I'd suggest to have the exceptions treated with higher priority.

                
      was (Author: amuraru):
    This worked in previous versions of thrift (not sure when it's been introduced), however what about inspecting the result.x exception before checking the return value in recv_* functions:

{code:title=t_java_generator.cc|borderStyle=solid}
      t_struct* xs = (*f_iter)->get_xceptions();
      const std::vector<t_field*>& xceptions = xs->get_members();
      vector<t_field*>::const_iterator x_iter;
      for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
        f_service_ <<
          indent() << "if (result." << (*x_iter)->get_name() << " != null) {" << endl <<
          indent() << "  throw result." << (*x_iter)->get_name() << ";" << endl <<
          indent() << "}" << endl;
      }

      // Careful, only return _result if not a void function
      if (!(*f_iter)->get_returntype()->is_void()) {
        f_service_ <<
          indent() << "if (result." << generate_isset_check("success") << ") {" << endl <<
          indent() << "  return result.success;" << endl <<
          indent() << "}" << endl;
      }
{code}

Currently all generators look for return.success first and than for result.x. 
I'd suggest to have the exceptions treated with higher priority.

                  
> Not throw user defined exception when a defined method returns boolean result
> -----------------------------------------------------------------------------
>
>                 Key: THRIFT-1474
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1474
>             Project: Thrift
>          Issue Type: Bug
>          Components: Java - Compiler
>    Affects Versions: 0.8
>            Reporter: Koji Hisano
>            Assignee: Bryan Duxbury
>         Attachments: codegen.diff, primitive_result_fix.patch
>
>
> A boolean result method always returns result value and exception value when a user defined exception is thrown.
> So, client handles the result as having a result value and ignore the exception.
> {code:title=Sample thrift definition|borderStyle=solid}
> bool createAccount(1:string userName, 2:string password, 3:string mailAddress) throws (1: Exception.ServiceException serviceException);
> {code}
> {code:title=Current generated code in createAccount_resultStandardScheme class|borderStyle=solid}
> public void write(org.apache.thrift.protocol.TProtocol oprot, createAccount_result struct) throws org.apache.thrift.TException {
> 	struct.validate();
> 	oprot.writeStructBegin(STRUCT_DESC);
> 	oprot.writeFieldBegin(SUCCESS_FIELD_DESC); // <- Problem because the value existence is not checked
> 	oprot.writeBool(struct.success);
> 	oprot.writeFieldEnd();
> 	if (struct.serviceException != null) { // <- I think it is better to use #isSetServiceException
> 		oprot.writeFieldBegin(SERVICE_EXCEPTION_FIELD_DESC);
> 		struct.serviceException.write(oprot);
> 		oprot.writeFieldEnd();
> 	}
> 	oprot.writeFieldStop();
> 	oprot.writeStructEnd();
> }
> {code}
> {code:title=Correct generated code (Maybe)|borderStyle=solid}
> public void write(org.apache.thrift.protocol.TProtocol oprot, createAccount_result struct) throws org.apache.thrift.TException {
> 	struct.validate();
> 	oprot.writeStructBegin(STRUCT_DESC);
> 	if (struct.isSetSuccess()) {
> 		oprot.writeFieldBegin(SUCCESS_FIELD_DESC);
> 		oprot.writeBool(struct.success);
> 		oprot.writeFieldEnd();
> 	}
> 	if (struct.isSetServiceException()) {
> 		oprot.writeFieldBegin(SERVICE_EXCEPTION_FIELD_DESC);
> 		struct.serviceException.write(oprot);
> 		oprot.writeFieldEnd();
> 	}
> 	oprot.writeFieldStop();
> 	oprot.writeStructEnd();
> }
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira