THRIFT-5044 Improve serialization support for TApplicationExceptions and custom exceptions
Client: Delphi
Patch: Jens Geyer

This closes #1960
diff --git a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc
index 4a2ebda..cffe305 100644
--- a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc
@@ -152,11 +152,13 @@
   void generate_delphi_struct_writer_impl(ostream& out,
                                           std::string cls_prefix,
                                           t_struct* tstruct,
-                                          bool is_exception);
+                                          bool is_exception,
+                                          bool is_x_factory);
   void generate_delphi_struct_result_writer_impl(ostream& out,
                                                  std::string cls_prefix,
                                                  t_struct* tstruct,
-                                                 bool is_exception);
+                                                 bool is_exception,
+                                                 bool is_x_factory);
 
   void generate_delphi_struct_tostring_impl(ostream& out,
                                             std::string cls_prefix,
@@ -169,7 +171,8 @@
   void generate_delphi_struct_reader_impl(ostream& out,
                                           std::string cls_prefix,
                                           t_struct* tstruct,
-                                          bool is_exception);
+                                          bool is_exception,
+                                          bool is_x_factory);
   void generate_delphi_create_exception_impl(ostream& out,
                                              string cls_prefix,
                                              t_struct* tstruct,
@@ -1532,11 +1535,30 @@
     indent_impl(out) << "begin" << endl;
     indent_up_impl();
     indent_impl(out) << "if F" << exception_factory_name << " = nil" << endl;
-    indent_impl(out) << "then F" << exception_factory_name << " := T" << exception_factory_name << "Impl.Create;" << endl;
-    indent_impl(out) << endl;
+    indent_impl(out) << "then F" << exception_factory_name << " := T" << exception_factory_name << "Impl.Create;" << endl << endl;
     indent_impl(out) << "result := F" << exception_factory_name << ";" << endl;
     indent_down_impl();
     indent_impl(out) << "end;" << endl << endl;
+    indent_impl(out) << "function " << cls_prefix << cls_nm << ".QueryInterface(const IID: TGUID; out Obj): HRESULT;" << endl;
+    indent_impl(out) << "begin" << endl;
+    indent_up_impl();
+    indent_impl(out) << "if GetInterface(IID, Obj)" << endl;
+    indent_impl(out) << "then result := S_OK" << endl;
+    indent_impl(out) << "else result := E_NOINTERFACE;" << endl;
+    indent_down_impl();
+    indent_impl(out) << "end;" << endl << endl;
+    indent_impl(out) << "function " << cls_prefix << cls_nm << "._AddRef: Integer;" << endl;
+    indent_impl(out) << "begin" << endl;
+    indent_up_impl();
+    indent_impl(out) << "result := -1;    // not refcounted" << endl;
+    indent_down_impl();
+    indent_impl(out) << "end;" << endl << endl;
+    indent_impl(out) << "function " << cls_prefix << cls_nm << "._Release: Integer;" << endl;
+    indent_impl(out) << "begin" << endl;
+    indent_up_impl();
+    indent_impl(out) << "result := -1;    // not refcounted" << endl;
+    indent_down_impl();
+    indent_impl(out) << "end;" << endl << endl;
   }
 
   if (tstruct->is_union()) {
@@ -1586,13 +1608,11 @@
     }
   }
 
-  if ((!is_exception) || is_x_factory) {
-    generate_delphi_struct_reader_impl(out, cls_prefix, tstruct, is_exception);
-    if (is_result) {
-      generate_delphi_struct_result_writer_impl(out, cls_prefix, tstruct, is_exception);
-    } else {
-      generate_delphi_struct_writer_impl(out, cls_prefix, tstruct, is_exception);
-    }
+  generate_delphi_struct_reader_impl(out, cls_prefix, tstruct, is_exception, is_x_factory);
+  if (is_result) {
+    generate_delphi_struct_result_writer_impl(out, cls_prefix, tstruct, is_exception, is_x_factory);
+  } else {
+    generate_delphi_struct_writer_impl(out, cls_prefix, tstruct, is_exception, is_x_factory);
   }
   generate_delphi_struct_tostring_impl(out, cls_prefix, tstruct, is_exception, is_x_factory);
 
@@ -1741,7 +1761,7 @@
   }
   out << "class(";
   if (is_exception && (!is_x_factory)) {
-    out << "TException";
+    out << "TException, IInterface, IBase, ISupportsToString";
   } else {
     out << "TInterfacedObject, IBase, ISupportsToString, " << struct_intf_name;
   }
@@ -1801,8 +1821,18 @@
     }
   }
 
-  indent_down();
+  if (is_exception && (!is_x_factory)) {
+    out << endl;
+    indent_down();
+    indent(out) << "strict protected" << endl;
+    indent_up();  
+    indent(out) << "function QueryInterface(const IID: TGUID; out Obj): HRESULT; stdcall;" << endl;
+    indent(out) << "function _AddRef: Integer; stdcall;" << endl;
+    indent(out) << "function _Release: Integer; stdcall;" << endl;
+    out << endl;
+  }
 
+  indent_down();
   indent(out) << "public" << endl;
   indent_up();
 
@@ -1825,12 +1855,10 @@
     indent(out) << "function " << exception_factory_name << ": " << struct_intf_name << ";" << endl;
   }
 
-  if ((!is_exception) || is_x_factory) {
-    out << endl;
-    indent(out) << "// IBase" << endl;
-    indent(out) << "procedure Read( const iprot: IProtocol);" << endl;
-    indent(out) << "procedure Write( const oprot: IProtocol);" << endl;
-  }
+  out << endl;
+  indent(out) << "// IBase" << endl;
+  indent(out) << "procedure Read( const iprot: IProtocol);" << endl;
+  indent(out) << "procedure Write( const oprot: IProtocol);" << endl;
 
   if (is_exception && is_x_factory) {
     out << endl;
@@ -2163,9 +2191,7 @@
       indent_impl(s_service_impl) << "begin" << endl;
       indent_up_impl();
       indent_impl(s_service_impl) << msgvar << " := iprot_.ReadMessageBegin();" << endl;
-      indent_impl(s_service_impl) << "if (" << msgvar << ".Type_ = TMessageType.Exception) then"
-                                  << endl;
-      indent_impl(s_service_impl) << "begin" << endl;
+      indent_impl(s_service_impl) << "if (" << msgvar << ".Type_ = TMessageType.Exception) then begin" << endl;
       indent_up_impl();
       indent_impl(s_service_impl) << appexvar << " := TApplicationException.Read(iprot_);" << endl;
       indent_impl(s_service_impl) << "iprot_.ReadMessageEnd();" << endl;
@@ -2178,8 +2204,7 @@
       indent_impl(s_service_impl) << "iprot_.ReadMessageEnd();" << endl;
 
       if (!(*f_iter)->get_returntype()->is_void()) {
-        indent_impl(s_service_impl) << "if (" << retvar << ".__isset_success) then" << endl;
-        indent_impl(s_service_impl) << "begin" << endl;
+        indent_impl(s_service_impl) << "if (" << retvar << ".__isset_success) then begin" << endl;
         indent_up_impl();
         indent_impl(s_service_impl) << "Result := " << retvar << ".Success;" << endl;
         t_type* type = (*f_iter)->get_returntype();
@@ -2195,8 +2220,7 @@
       vector<t_field*>::const_iterator x_iter;
       for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
         indent_impl(s_service_impl) << "if (" << retvar << ".__isset_" << prop_name(*x_iter)
-                                    << ") then" << endl;
-        indent_impl(s_service_impl) << "begin" << endl;
+                                    << ") then begin" << endl;
         indent_up_impl();
         indent_impl(s_service_impl) << exceptvar << " := " << retvar << "." << prop_name(*x_iter)
                                     << ".CreateException;" << endl;
@@ -2324,8 +2348,7 @@
   indent_impl(s_service_impl) << "msg := iprot.ReadMessageBegin();" << endl;
   indent_impl(s_service_impl) << "fn := nil;" << endl;
   indent_impl(s_service_impl) << "if not processMap_.TryGetValue(msg.Name, fn)" << endl;
-  indent_impl(s_service_impl) << "or not Assigned(fn) then" << endl;
-  indent_impl(s_service_impl) << "begin" << endl;
+  indent_impl(s_service_impl) << "or not Assigned(fn) then begin" << endl;
   indent_up_impl();
   indent_impl(s_service_impl) << "TProtocolUtil.Skip(iprot, TType.Struct);" << endl;
   indent_impl(s_service_impl) << "iprot.ReadMessageEnd();" << endl;
@@ -2716,8 +2739,7 @@
     indent_impl(out) << obj << " := iprot.ReadListBegin();" << endl;
   }
 
-  indent_impl(out) << "for " << counter << " := 0 to " << obj << ".Count - 1 do" << endl;
-  indent_impl(out) << "begin" << endl;
+  indent_impl(out) << "for " << counter << " := 0 to " << obj << ".Count - 1 do begin" << endl;
   indent_up_impl();
   if (ttype->is_map()) {
     generate_deserialize_map_element(out, is_xception, (t_map*)ttype, name, local_vars);
@@ -2904,20 +2926,17 @@
   string iter = tmp("_iter");
   if (ttype->is_map()) {
     local_vars << "  " << iter << ": " << type_name(((t_map*)ttype)->get_key_type()) << ";" << endl;
-    indent_impl(out) << "for " << iter << " in " << prefix << ".Keys do" << endl;
-    indent_impl(out) << "begin" << endl;
+    indent_impl(out) << "for " << iter << " in " << prefix << ".Keys do begin" << endl;
     indent_up_impl();
   } else if (ttype->is_set()) {
     local_vars << "  " << iter << ": " << type_name(((t_set*)ttype)->get_elem_type()) << ";"
                << endl;
-    indent_impl(out) << "for " << iter << " in " << prefix << " do" << endl;
-    indent_impl(out) << "begin" << endl;
+    indent_impl(out) << "for " << iter << " in " << prefix << " do begin" << endl;
     indent_up_impl();
   } else if (ttype->is_list()) {
     local_vars << "  " << iter << ": " << type_name(((t_list*)ttype)->get_elem_type()) << ";"
                << endl;
-    indent_impl(out) << "for " << iter << " in " << prefix << " do" << endl;
-    indent_impl(out) << "begin" << endl;
+    indent_impl(out) << "for " << iter << " in " << prefix << " do begin" << endl;
     indent_up_impl();
   }
 
@@ -3575,7 +3594,8 @@
 void t_delphi_generator::generate_delphi_struct_reader_impl(ostream& out,
                                                             string cls_prefix,
                                                             t_struct* tstruct,
-                                                            bool is_exception) {
+                                                            bool is_exception,
+                                                            bool is_x_factory) {
 
   ostringstream local_vars;
   ostringstream code_block;
@@ -3604,32 +3624,28 @@
   indent_impl(code_block) << "try" << endl;
   indent_up_impl();
 
-  indent_impl(code_block) << "while (true) do" << endl;
-  indent_impl(code_block) << "begin" << endl;
+  indent_impl(code_block) << "while (true) do begin" << endl;
   indent_up_impl();
 
   indent_impl(code_block) << "field_ := iprot.ReadFieldBegin();" << endl;
 
-  indent_impl(code_block) << "if (field_.Type_ = TType.Stop) then" << endl;
-  indent_impl(code_block) << "begin" << endl;
-  indent_up_impl();
-  indent_impl(code_block) << "Break;" << endl;
-  indent_down_impl();
-  indent_impl(code_block) << "end;" << endl;
+  indent_impl(code_block) << "if (field_.Type_ = TType.Stop) then Break;" << endl;
 
   bool first = true;
 
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
 
     if (first) {
+      code_block << endl;
       indent_impl(code_block) << "case field_.ID of" << endl;
       indent_up_impl();
     }
 
     first = false;
     if (f_iter != fields.begin()) {
-      code_block << ";" << endl;
+      code_block << endl;
     }
+
     indent_impl(code_block) << (*f_iter)->get_key() << ": begin" << endl;
     indent_up_impl();
     indent_impl(code_block) << "if (field_.Type_ = " << type_to_enum((*f_iter)->get_type())
@@ -3652,12 +3668,13 @@
     indent_down_impl();
     indent_impl(code_block) << "end;" << endl;
     indent_down_impl();
-    indent_impl(code_block) << "end";
+    indent_impl(code_block) << "end;";
   }
 
   if (!first) {
     code_block << endl;
-    indent_impl(code_block) << "else begin" << endl;
+    indent_down_impl();
+    indent_impl(code_block) << "else" << endl;
     indent_up_impl();
   }
 
@@ -3666,8 +3683,6 @@
   if (!first) {
     indent_down_impl();
     indent_impl(code_block) << "end;" << endl;
-    indent_down_impl();
-    indent_impl(code_block) << "end;" << endl;
   }
 
   indent_impl(code_block) << "iprot.ReadFieldEnd;" << endl;
@@ -3684,8 +3699,13 @@
   indent_impl(code_block) << "end;" << endl;
 
   // all required fields have been read?
+  first = true;
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     if ((*f_iter)->get_req() == t_field::T_REQUIRED) {
+      if(first) {
+        code_block << endl;
+        first = false;
+      }
       indent_impl(code_block) << "if not _req_isset_" << prop_name(*f_iter, is_exception) << endl;
       indent_impl(code_block)
           << "then raise TProtocolExceptionInvalidData.Create("
@@ -3693,13 +3713,17 @@
           << endl;
     }
   }
-
+  
+  if( is_exception && (!is_x_factory)) {
+    code_block << endl;
+    indent_impl(code_block) << "UpdateMessageProperty;" << endl;
+  }
   indent_down_impl();
   indent_impl(code_block) << "end;" << endl << endl;
 
   string cls_nm;
 
-  cls_nm = type_name(tstruct, true, false, is_exception, is_exception);
+  cls_nm = type_name(tstruct, true, is_exception && (!is_x_factory), is_x_factory, is_x_factory);
 
   indent_impl(out) << "procedure " << cls_prefix << cls_nm << ".Read( const iprot: IProtocol);"
                    << endl;
@@ -3715,7 +3739,8 @@
 void t_delphi_generator::generate_delphi_struct_result_writer_impl(ostream& out,
                                                                    string cls_prefix,
                                                                    t_struct* tstruct,
-                                                                   bool is_exception) {
+                                                                   bool is_exception,
+                                                                   bool is_x_factory) {
 
   ostringstream local_vars;
   ostringstream code_block;
@@ -3759,7 +3784,7 @@
 
   string cls_nm;
 
-  cls_nm = type_name(tstruct, true, false, is_exception, is_exception);
+  cls_nm = type_name(tstruct, true, is_exception && (!is_x_factory), is_x_factory, is_x_factory);
 
   indent_impl(out) << "procedure " << cls_prefix << cls_nm << ".Write( const oprot: IProtocol);"
                    << endl;
@@ -3779,7 +3804,8 @@
 void t_delphi_generator::generate_delphi_struct_writer_impl(ostream& out,
                                                             string cls_prefix,
                                                             t_struct* tstruct,
-                                                            bool is_exception) {
+                                                            bool is_exception,
+                                                            bool is_x_factory) {
 
   ostringstream local_vars;
   ostringstream code_block;
@@ -3847,7 +3873,7 @@
 
   string cls_nm;
 
-  cls_nm = type_name(tstruct, true, false, is_exception, is_exception);
+  cls_nm = type_name(tstruct, true, is_exception && (!is_x_factory), is_x_factory, is_x_factory);
 
   indent_impl(out) << "procedure " << cls_prefix << cls_nm << ".Write( const oprot: IProtocol);"
                    << endl;
diff --git a/lib/delphi/src/Thrift.Exception.pas b/lib/delphi/src/Thrift.Exception.pas
index 5d15c36..88b1cfe 100644
--- a/lib/delphi/src/Thrift.Exception.pas
+++ b/lib/delphi/src/Thrift.Exception.pas
@@ -29,6 +29,8 @@
 type
   // base class for all Thrift exceptions
   TException = class( SysUtils.Exception)
+  strict private
+    function GetMessageText : string;
   public
     function Message : string;        // hide inherited property: allow read, but prevent accidental writes
     procedure UpdateMessageProperty;  // update inherited message property with toString()
@@ -45,17 +47,25 @@
 // allow read (exception summary), but prevent accidental writes
 // read will return the exception summary
 begin
-  result := Self.ToString;
+  result := Self.GetMessageText;
 end;
 
+
 procedure TException.UpdateMessageProperty;
 // Update the inherited Message property to better conform to standard behaviour.
 // Nice benefit: The IDE is now able to show the exception message again.
 begin
-  inherited Message := Self.ToString;  // produces a summary text
+  inherited Message := Self.GetMessageText;
 end;
 
 
+function TException.GetMessageText : string;
+// produces a summary text
+begin
+  result := Self.ToString;
+  if (result <> '') and (result[1] = '(')
+  then result := Copy(result,2,Length(result)-2);
+end;
 
 
 end.
diff --git a/lib/delphi/src/Thrift.pas b/lib/delphi/src/Thrift.pas
index 716e4d2..1926b11 100644
--- a/lib/delphi/src/Thrift.pas
+++ b/lib/delphi/src/Thrift.pas
@@ -23,6 +23,7 @@
 
 uses
   SysUtils,
+  Thrift.Utils,
   Thrift.Exception,
   Thrift.Protocol;
 
@@ -34,7 +35,7 @@
 
   TApplicationExceptionSpecializedClass = class of TApplicationExceptionSpecialized;
 
-  TApplicationException = class abstract( TException)
+  TApplicationException = class( TException, IBase, ISupportsToString)
   public
     type
 {$SCOPEDENUMS ON}
@@ -52,10 +53,18 @@
         UnsupportedClientType
       );
 {$SCOPEDENUMS OFF}
+  strict private
+    FExceptionType : TExceptionType;
+
+  strict protected
+    function QueryInterface(const IID: TGUID; out Obj): HResult; stdcall;
+    function _AddRef: Integer; stdcall;
+    function _Release: Integer; stdcall;
+
   strict protected
     constructor HiddenCreate(const Msg: string);
-    class function GetType: TExceptionType;  virtual; abstract;
     class function GetSpecializedExceptionType(AType: TExceptionType): TApplicationExceptionSpecializedClass;
+
   public
     // purposefully hide inherited constructor
     class function Create(const Msg: string): TApplicationException; overload; deprecated 'Use specialized TApplicationException types (or regenerate from IDL)';
@@ -63,7 +72,10 @@
     class function Create( AType: TExceptionType): TApplicationException; overload; deprecated 'Use specialized TApplicationException types (or regenerate from IDL)';
     class function Create( AType: TExceptionType; const msg: string): TApplicationException; overload; deprecated 'Use specialized TApplicationException types (or regenerate from IDL)';
 
-    property Type_: TExceptionType read GetType;
+    function Type_: TExceptionType; virtual;
+
+    procedure IBase_Read( const iprot: IProtocol);
+    procedure IBase.Read = IBase_Read;
 
     class function Read( const iprot: IProtocol): TApplicationException;
     procedure Write( const oprot: IProtocol );
@@ -71,8 +83,11 @@
 
   // Needed to remove deprecation warning
   TApplicationExceptionSpecialized = class abstract (TApplicationException)
+  strict protected
+    class function GetType: TApplicationException.TExceptionType;  virtual; abstract;
   public
     constructor Create(const Msg: string);
+    function Type_: TApplicationException.TExceptionType; override;
   end;
 
   TApplicationExceptionUnknown = class (TApplicationExceptionSpecialized)
@@ -163,6 +178,31 @@
   Result := GetSpecializedExceptionType(AType).Create(msg);
 end;
 
+
+function TApplicationException.QueryInterface(const IID: TGUID; out Obj): HResult;
+begin
+  if GetInterface(IID, Obj)
+  then result := S_OK
+  else result := E_NOINTERFACE;
+end;
+
+function TApplicationException._AddRef: Integer;
+begin
+  result := -1;    // not refcounted
+end;
+
+function TApplicationException._Release: Integer;
+begin
+  result := -1;    // not refcounted
+end;
+
+
+function TApplicationException.Type_: TExceptionType;
+begin
+  result := FExceptionType;
+end;
+
+
 class function TApplicationException.GetSpecializedExceptionType(AType: TExceptionType): TApplicationExceptionSpecializedClass;
 begin
   case AType of
@@ -183,52 +223,60 @@
 end;
 
 
-class function TApplicationException.Read( const iprot: IProtocol): TApplicationException;
+procedure TApplicationException.IBase_Read( const iprot: IProtocol);
 var
   field : TThriftField;
-  msg : string;
-  typ : TExceptionType;
   struc : TThriftStruct;
 begin
-  msg := '';
-  typ := TExceptionType.Unknown;
   struc := iprot.ReadStructBegin;
   while ( True ) do
   begin
     field := iprot.ReadFieldBegin;
-    if ( field.Type_ = TType.Stop) then
-    begin
+    if ( field.Type_ = TType.Stop) then begin
       Break;
     end;
 
     case field.Id of
       1 : begin
-        if ( field.Type_ = TType.String_) then
-        begin
-          msg := iprot.ReadString;
-        end else
-        begin
+        if ( field.Type_ = TType.String_) then begin
+          Exception(Self).Message := iprot.ReadString;
+        end else begin
           TProtocolUtil.Skip( iprot, field.Type_ );
         end;
       end;
 
       2 : begin
-        if ( field.Type_ = TType.I32) then
-        begin
-          typ := TExceptionType( iprot.ReadI32 );
-        end else
-        begin
+        if ( field.Type_ = TType.I32) then begin
+          FExceptionType := TExceptionType( iprot.ReadI32 );
+        end else begin
           TProtocolUtil.Skip( iprot, field.Type_ );
         end;
-      end else
-      begin
+      end else begin
         TProtocolUtil.Skip( iprot, field.Type_);
       end;
     end;
     iprot.ReadFieldEnd;
   end;
   iprot.ReadStructEnd;
-  Result := GetSpecializedExceptionType(typ).Create(msg);
+end;
+
+
+class function TApplicationException.Read( const iprot: IProtocol): TApplicationException;
+var instance : TApplicationException;
+    base : IBase;
+begin
+  instance := TApplicationException.CreateFmt('',[]);
+  try
+    if Supports( instance, IBase, base) then try
+      base.Read(iprot);
+    finally
+      base := nil;  // clear ref before free
+    end;
+
+    result := GetSpecializedExceptionType(instance.Type_).Create( Exception(instance).Message);
+  finally
+    instance.Free;
+  end;
 end;
 
 procedure TApplicationException.Write( const oprot: IProtocol);
@@ -240,8 +288,7 @@
   Init(field);
 
   oprot.WriteStructBegin( struc );
-  if Message <> '' then
-  begin
+  if Message <> '' then begin
     field.Name := 'message';
     field.Type_ := TType.String_;
     field.Id := 1;
@@ -254,7 +301,7 @@
   field.Type_ := TType.I32;
   field.Id := 2;
   oprot.WriteFieldBegin(field);
-  oprot.WriteI32(Integer(GetType));
+  oprot.WriteI32(Integer(Type_));
   oprot.WriteFieldEnd();
   oprot.WriteFieldStop();
   oprot.WriteStructEnd();
@@ -267,6 +314,12 @@
   inherited HiddenCreate(Msg);
 end;
 
+function TApplicationExceptionSpecialized.Type_: TApplicationException.TExceptionType;
+begin
+  result := GetType;
+end;
+
+
 { specialized TApplicationExceptions }
 
 class function TApplicationExceptionUnknownMethod.GetType : TApplicationException.TExceptionType;