From 656502603dab4228c7e8df7e81d2e3a7f382e2f5 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 15 Jun 2018 23:57:07 +0100 Subject: [PATCH] Refactor generated protocol classes Simplify hierarchy slightly, document it in comments, and fix the bug where mLastError* were shadowed in Wrapper classes, causing reply error checking to fail. --- lib/server/makeprotocol.pl.in | 277 ++++++++++++++++++++++++++++------ 1 file changed, 231 insertions(+), 46 deletions(-) diff --git a/lib/server/makeprotocol.pl.in b/lib/server/makeprotocol.pl.in index 6ad21f1f8..0118e3beb 100755 --- a/lib/server/makeprotocol.pl.in +++ b/lib/server/makeprotocol.pl.in @@ -213,15 +213,43 @@ my $request_base_class = "${protocol_name}ProtocolRequest"; my $reply_base_class = "${protocol_name}ProtocolReply"; # the abstract protocol interface my $custom_protocol_subclass = $protocol_name."Protocol"; -my $client_server_base_class = $protocol_name."ProtocolClientServer"; +my $comms_base_class = $protocol_name."ProtocolComms"; +my $comms_impl_class = $protocol_name."ProtocolCommsImpl"; my $replyable_base_class = $protocol_name."ProtocolReplyable"; my $callable_base_class = $protocol_name."ProtocolCallable"; -my $send_receive_class = $protocol_name."ProtocolSendReceive"; +my $callable_impl_class = $protocol_name."ProtocolCallableImpl"; print H <<__E; +/* + Class hierarchy: + + $request_base_class: empty, base of all Command message classes (along with $message_base_class) + $reply_base_class: empty, base of all Reply message classes (along with $message_base_class) + + Protocol: Handshake, SendInternal, ReceiveInternal, SendStream, ReceiveStream, Read(*), Write(*), + | abstract GetProtocolIdentString and MakeMessage, abstract logging API + '- $custom_protocol_subclass: GetProtocolIdentString and MakeMessage impl + + Message: GetType, IsError, ToString, logging helpers + '- $message_base_class: DoCommand(*), HasStreamWithCommand + + $comms_base_class: Send, Receive, ReceiveStream, Get/SetLastError (all abstract) + +- $comms_impl_class: Get/SetLastError (implementations) + | '- $replyable_base_class: SendStreamAfterCommand, HandleException + | '- ${protocol_name}ProtocolServer: Send, Receive, DoServer + '- $callable_base_class: Query(*), Query*, abstract logging API + +- ${protocol_name}ProtocolWrapper: Query(*), Query*, concrete logging API + | (all forwarded to wrapped $callable_base_class) + '- $callable_impl_class: Query(*), Query* + +- ${protocol_name}ProtocolClient: Query(*) (to Send), Query*, concrete logging API + | $replyable_base_class + '--+- ${protocol_name}ProtocolLocal: Query(*) (to Send, then DoCommand), Query*, concrete logging API +*/ + class $custom_protocol_subclass; -class $client_server_base_class; +class $comms_base_class; class $callable_base_class; +class $callable_impl_class; class $replyable_base_class; class $message_base_class : public Message @@ -242,13 +270,6 @@ class $request_base_class { }; -class $send_receive_class { -public: - virtual ~$send_receive_class() { } - virtual void Send(const $message_base_class &rObject) = 0; - virtual std::auto_ptr<$message_base_class> Receive() = 0; -}; - class $custom_protocol_subclass : public Protocol { public: @@ -588,12 +609,26 @@ my $error_class = $protocol_name."ProtocolError"; # the abstract protocol interface print H <<__E; -class $client_server_base_class +class $comms_base_class { public: - $client_server_base_class(); - virtual ~$client_server_base_class(); + $comms_base_class() { } + virtual ~$comms_base_class() { } virtual std::auto_ptr ReceiveStream() = 0; + virtual bool GetLastError(int &rTypeOut, int &rSubTypeOut) = 0; + virtual int GetLastErrorType() = 0; + virtual const std::string& GetLastErrorMessage() = 0; + +protected: + friend class ${protocol_name}ProtocolWrapper; + virtual void SetLastError(int Type, int SubType, const std::string& Message) = 0; +}; + +class $comms_impl_class : public $comms_base_class +{ +public: + $comms_impl_class(); + virtual ~$comms_impl_class(); bool GetLastError(int &rTypeOut, int &rSubTypeOut); int GetLastErrorType() { return mLastErrorSubType; } const std::string& GetLastErrorMessage() { return mLastErrorMessage; } @@ -605,17 +640,15 @@ protected: mLastErrorSubType = SubType; mLastErrorMessage = Message; } - std::string mPreviousCommand; - std::string mPreviousReply; private: - $client_server_base_class(const $client_server_base_class &rToCopy); /* do not call */ + $comms_impl_class(const $comms_impl_class &rToCopy); /* do not call */ int mLastErrorType; int mLastErrorSubType; std::string mLastErrorMessage; }; -class $replyable_base_class : public virtual $client_server_base_class +class $replyable_base_class : public virtual $comms_impl_class { public: $replyable_base_class() { } @@ -636,13 +669,13 @@ private: __E print CPP <<__E; -$client_server_base_class\::$client_server_base_class() +$comms_impl_class\::$comms_impl_class() : mLastErrorType(Protocol::NoError), mLastErrorSubType(Protocol::NoError), mLastErrorMessage("no messages received") { } -$client_server_base_class\::~$client_server_base_class() +$comms_impl_class\::~$comms_impl_class() { } const char *$custom_protocol_subclass\::GetProtocolIdentString() @@ -695,14 +728,14 @@ void $replyable_base_class\::DeleteStreamsToSend() mStreamsToSend.clear(); } -void $callable_base_class\::CheckReply(const std::string& requestCommandName, +void $callable_impl_class\::CheckReply(const std::string& requestCommandName, const $message_base_class &rCommand, const $message_base_class &rReply, int expectedType) { if(rReply.GetType() == expectedType) { // Correct response, do nothing - SetLastError(Protocol::NoError, Protocol::NoError, "no error"); + $comms_impl_class\::SetLastError(Protocol::NoError, Protocol::NoError, "no error"); } else { @@ -711,7 +744,7 @@ void $callable_base_class\::CheckReply(const std::string& requestCommandName, if(rReply.IsError(type, subType)) { - SetLastError(type, subType, (($error_class&)rReply).GetMessage()); + $comms_impl_class\::SetLastError(type, subType, (($error_class&)rReply).GetMessage()); THROW_EXCEPTION_MESSAGE(ConnectionException, Protocol_UnexpectedReply, requestCommandName << " command failed: " @@ -720,7 +753,7 @@ void $callable_base_class\::CheckReply(const std::string& requestCommandName, } else { - SetLastError(Protocol::UnknownError, Protocol::UnknownError, + $comms_impl_class\::SetLastError(Protocol::UnknownError, Protocol::UnknownError, rReply.ToString()); THROW_EXCEPTION_MESSAGE(ConnectionException, Protocol_UnexpectedReply, @@ -745,7 +778,7 @@ void $callable_base_class\::CheckReply(const std::string& requestCommandName, // Created: 2003/08/19 // // -------------------------------------------------------------------------- -bool $client_server_base_class\::GetLastError(int &rTypeOut, int &rSubTypeOut) +bool $comms_impl_class\::GetLastError(int &rTypeOut, int &rSubTypeOut) { if(mLastErrorType == Protocol::NoError) { @@ -769,28 +802,88 @@ __E # the callable protocol interface (implemented by Client and Local classes) # with Query methods that don't take a context parameter print H <<__E; -class $callable_base_class : public virtual $client_server_base_class, - public $send_receive_class +class $callable_base_class : public virtual $comms_base_class { public: virtual int GetTimeout() = 0; - // Duplicate of the Protocol logging API: + virtual void Send(const $message_base_class &rObject) = 0; + virtual std::auto_ptr<$message_base_class> Receive() = 0; + + // Duplicate of the Protocol logging API, still abstract: virtual bool GetLogToSysLog() = 0; virtual FILE *GetLogToFile() = 0; virtual void SetLogToSysLog(bool Log = false) = 0; virtual void SetLogToFile(FILE *File = 0) = 0; +protected: + virtual void CheckReply(const std::string& requestCommandName, + const $message_base_class &rCommand, + const $message_base_class &rReply, int expectedType) = 0; + friend class ${protocol_name}ProtocolWrapper; + +public: +__E + +# add plain object taking query functions (virtual) +my $with_params; +for my $cmd (@cmd_list) +{ + if(obj_is_type($cmd,'Command')) + { + my $has_stream = obj_is_type($cmd,'StreamWithCommand'); + my $argextra = $has_stream?', std::auto_ptr apStream':''; + my $queryextra = $has_stream?', apStream':''; + my $request_class = $cmd_classes{$cmd}; + my $reply_class = $cmd_classes{obj_get_type_params($cmd,'Command')}; + + print H "\tvirtual std::auto_ptr<$reply_class> Query(const $request_class &rQuery$argextra) = 0;\n"; + my @a; + my @na; + for(my $x = 0; $x < $#{$cmd_contents{$cmd}}; $x+=2) + { + my ($ty,$nm) = (${$cmd_contents{$cmd}}[$x], ${$cmd_contents{$cmd}}[$x+1]); + push @a,translate_type_to_arg_type($ty)." $nm"; + push @na,"$nm"; + } + my $ar = join(', ',@a); + my $nar = join(', ',@na); + $nar = "($nar)" if $nar ne ''; + + $with_params .= <<__E; + virtual inline std::auto_ptr<$reply_class> Query$cmd($ar$argextra) = 0; +__E + } +} + +# quick hack to correct bad argument lists for commands with zero parameters but with streams +$with_params =~ s/\(, /(/g; + +print H <<__E; + +$with_params +}; +__E + +# the callable protocol interface (implemented by Client and Local classes) +# with Query methods that don't take a context parameter +print H <<__E; +class $callable_impl_class : public $callable_base_class, + public virtual $comms_impl_class // for Get/SetLastError +{ protected: void CheckReply(const std::string& requestCommandName, const $message_base_class &rCommand, const $message_base_class &rReply, int expectedType); + std::string mPreviousCommand; + std::string mPreviousReply; public: __E -# add plain object taking query functions -my $with_params; +$with_params = ""; + +# add plain object taking query functions (implementations) for my $cmd (@cmd_list) { if(obj_is_type($cmd,'Command')) @@ -844,17 +937,21 @@ foreach my $type ('Client', 'Server', 'Local', 'Wrapper') my $server_or_client_class = $protocol_name."Protocol".$type; my @base_classes; - if (not $writing_client and not $writing_wrapper) + if ($writing_client or $writing_local) { - push @base_classes, $replyable_base_class; + push @base_classes, $callable_impl_class; } - - if (not $writing_server) + elsif ($writing_wrapper) { push @base_classes, $callable_base_class; } - if (not $writing_local and not $writing_wrapper) + if ($writing_server or $writing_local) + { + push @base_classes, $replyable_base_class; + } + + if ($writing_server or $writing_client) { push @base_classes, $custom_protocol_subclass; } @@ -868,6 +965,57 @@ public: virtual ~$server_or_client_class(); __E + if($writing_client or $writing_local) + { + print H <<__E; +protected: + virtual void SetLastError(int Type, int SubType, const std::string& Message) + { + $comms_impl_class\::SetLastError(Type, SubType, Message); + } + +public: + virtual bool GetLastError(int &rTypeOut, int &rSubTypeOut) + { + return $comms_impl_class\::GetLastError(rTypeOut, rSubTypeOut); + } + virtual int GetLastErrorType() + { + return $comms_impl_class\::GetLastErrorType(); + } + virtual const std::string& GetLastErrorMessage() + { + return $comms_impl_class\::GetLastErrorMessage(); + } + +__E + } + elsif($writing_wrapper) + { + print H <<__E; +protected: + virtual void SetLastError(int Type, int SubType, const std::string& Message) + { + mapImpl->SetLastError(Type, SubType, Message); + } + +public: + virtual bool GetLastError(int &rTypeOut, int &rSubTypeOut) + { + return mapImpl->GetLastError(rTypeOut, rSubTypeOut); + } + virtual int GetLastErrorType() + { + return mapImpl->GetLastErrorType(); + } + virtual const std::string& GetLastErrorMessage() + { + return mapImpl->GetLastErrorMessage(); + } + +__E + } + if($writing_local) { print H <<__E; @@ -885,6 +1033,7 @@ __E { mapImpl = apImpl; } + // Warning: SetImplementation takes ownership of the supplied pointer! void SetImplementation($callable_base_class* pImpl) { mapImpl.reset(pImpl); @@ -928,7 +1077,35 @@ __E my $queryextra = $has_stream?', apStream':''; my $request_class = $cmd_classes{$cmd}; my $reply_class = $cmd_classes{obj_get_type_params($cmd,'Command')}; - print H "\tstd::auto_ptr<$reply_class> Query(const $request_class &rQuery$argextra);\n"; + + if($writing_client or $writing_local) + { + print H "\tstd::auto_ptr<$reply_class> Query(const $request_class &rQuery$argextra);\n"; + } + else # $writing_wrapper + { + # add query forwarding functions + my @a; + my @na; + for(my $x = 0; $x < $#{$cmd_contents{$cmd}}; $x+=2) + { + my ($ty,$nm) = (${$cmd_contents{$cmd}}[$x], ${$cmd_contents{$cmd}}[$x+1]); + push @a,translate_type_to_arg_type($ty)." $nm"; + push @na,"$nm"; + } + my $ar = join(', ',@a); + my $nar = join(', ',@na); + print H <<__E; + virtual inline std::auto_ptr<$reply_class> Query(const $request_class &rQuery$argextra) + { + return mapImpl->Query(rQuery$queryextra); + } + virtual inline std::auto_ptr<$reply_class> Query$cmd($ar$argextra) + { + return mapImpl->Query$cmd($nar$queryextra); + } +__E + } } } } @@ -987,6 +1164,12 @@ public: FILE *GetLogToFile() { return mapImpl->GetLogToFile(); } void SetLogToSysLog(bool Log = false) { mapImpl->SetLogToSysLog(Log); } void SetLogToFile(FILE *File = 0) { mapImpl->SetLogToFile(File); } + virtual void CheckReply(const std::string& requestCommandName, + const $message_base_class &rCommand, + const $message_base_class &rReply, int expectedType) + { + mapImpl->CheckReply(requestCommandName, rCommand, rReply, expectedType); + } __E } else @@ -1045,7 +1228,7 @@ __E } __E } - else + else # server or client { print H <<__E; virtual int GetTimeout() @@ -1053,7 +1236,16 @@ __E return $custom_protocol_subclass\::GetTimeout(); } __E - } + } + + if($writing_server) + { + print H <<__E; +protected: + std::string mPreviousCommand; + std::string mPreviousReply; +__E + } print H <<__E; @@ -1291,7 +1483,7 @@ __E } # write client Query functions? - if($writing_client or $writing_local or $writing_wrapper) + if($writing_client or $writing_local) { for my $cmd (@cmd_list) { @@ -1310,14 +1502,7 @@ std::auto_ptr<$reply_class> $server_or_client_class\::Query(const $request_class { __E - if($writing_wrapper) - { - my $call_extra = $has_stream ? ', apDataStream' : ''; - print CPP <<__E; - return mapImpl->Query(rQuery$call_extra); -__E - } - elsif($writing_client) + if($writing_client) { if($has_stream) {