From 677d1670aa2fe9760e7c51e20efe6513f4690645 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Mon, 16 Aug 2021 15:42:37 -0400 Subject: [PATCH] thrift-proxy: cleanup ConnectionManager::ActiveRpc Reduce nested code in applyDecoderFilters(). Signed-off-by: Raul Gutierrez Segales --- .../network/thrift_proxy/conn_manager.cc | 73 +++++++++---------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/source/extensions/filters/network/thrift_proxy/conn_manager.cc b/source/extensions/filters/network/thrift_proxy/conn_manager.cc index c3de018f2dfda..1e100247e59cc 100644 --- a/source/extensions/filters/network/thrift_proxy/conn_manager.cc +++ b/source/extensions/filters/network/thrift_proxy/conn_manager.cc @@ -319,46 +319,45 @@ void ConnectionManager::ActiveRpcDecoderFilter::continueDecoding() { FilterStatus ConnectionManager::ActiveRpc::applyDecoderFilters(ActiveRpcDecoderFilter* filter) { ASSERT(filter_action_ != nullptr); - if (!local_response_sent_) { - if (upgrade_handler_) { - // Divert events to the current protocol upgrade handler. - const FilterStatus status = filter_action_(upgrade_handler_.get()); - filter_context_.reset(); - return status; - } + if (local_response_sent_) { + filter_action_ = nullptr; + filter_context_.reset(); + return FilterStatus::Continue; + } - std::list::iterator entry; - if (!filter) { - entry = decoder_filters_.begin(); - } else { - entry = std::next(filter->entry()); - } + if (upgrade_handler_) { + // Divert events to the current protocol upgrade handler. + const FilterStatus status = filter_action_(upgrade_handler_.get()); + filter_context_.reset(); + return status; + } - for (; entry != decoder_filters_.end(); entry++) { - const FilterStatus status = filter_action_((*entry)->handle_.get()); - if (local_response_sent_) { - // The filter called sendLocalReply but _did not_ close the connection. - // We return FilterStatus::Continue irrespective of the current result, - // which is fine because subsequent calls to this method will skip - // filters anyway. - // - // Note: we need to return FilterStatus::Continue here, in order for decoding - // to proceed. This is important because as noted above, the connection remains - // open so we need to consume the remaining bytes. - break; - } + std::list::iterator entry = + !filter ? decoder_filters_.begin() : std::next(filter->entry()); + for (; entry != decoder_filters_.end(); entry++) { + const FilterStatus status = filter_action_((*entry)->handle_.get()); + if (local_response_sent_) { + // The filter called sendLocalReply but _did not_ close the connection. + // We return FilterStatus::Continue irrespective of the current result, + // which is fine because subsequent calls to this method will skip + // filters anyway. + // + // Note: we need to return FilterStatus::Continue here, in order for decoding + // to proceed. This is important because as noted above, the connection remains + // open so we need to consume the remaining bytes. + break; + } - if (status != FilterStatus::Continue) { - // If we got FilterStatus::StopIteration and a local reply happened but - // local_response_sent_ was not set, the connection was closed. - // - // In this case, either resetAllRpcs() gets called via onEvent(LocalClose) or - // dispatch() stops the processing. - // - // In other words, after a local reply closes the connection and StopIteration - // is returned we are done. - return status; - } + if (status != FilterStatus::Continue) { + // If we got FilterStatus::StopIteration and a local reply happened but + // local_response_sent_ was not set, the connection was closed. + // + // In this case, either resetAllRpcs() gets called via onEvent(LocalClose) or + // dispatch() stops the processing. + // + // In other words, after a local reply closes the connection and StopIteration + // is returned we are done. + return status; } }