From 331b3323927e65e796f8bc59d838d8f72c40609c Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 6 Apr 2020 21:49:16 -0700 Subject: [PATCH] Try fixing interop tests --- .../message_decompress/message_decompress_filter.cc | 10 ++++++---- src/core/lib/transport/byte_stream.h | 9 +++++++-- test/cpp/interop/client_helper.h | 8 ++++++-- test/cpp/interop/interop_client.cc | 9 ++++----- test/cpp/interop/interop_server.cc | 5 ++--- test/cpp/interop/server_helper.cc | 8 ++++++-- test/cpp/interop/server_helper.h | 2 +- 7 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/core/ext/filters/http/message_decompress/message_decompress_filter.cc b/src/core/ext/filters/http/message_decompress/message_decompress_filter.cc index c30285408f6..0a756449573 100644 --- a/src/core/ext/filters/http/message_decompress/message_decompress_filter.cc +++ b/src/core/ext/filters/http/message_decompress/message_decompress_filter.cc @@ -35,11 +35,8 @@ #include "src/core/lib/compression/message_compress.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/manual_constructor.h" -#include "src/core/lib/profiling/timers.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" -#include "src/core/lib/surface/call.h" -#include "src/core/lib/transport/static_metadata.h" namespace { @@ -229,15 +226,20 @@ void CallData::FinishRecvMessage() { GPR_DEBUG_ASSERT(error_ == GRPC_ERROR_NONE); error_ = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg); gpr_free(msg); + grpc_slice_buffer_destroy_internal(&decompressed_slices); } else { uint32_t recv_flags = - (*recv_message_)->flags() & (~GRPC_WRITE_INTERNAL_COMPRESS); + ((*recv_message_)->flags() & (~GRPC_WRITE_INTERNAL_COMPRESS)) | + GRPC_WRITE_INTERNAL_TEST_ONLY_WAS_COMPRESSED; // Swap out the original receive byte stream with our new one and send the // batch down. + // Initializing recv_replacement_stream_ with decompressed_slices removes + // all the slices from decompressed_slices leaving it empty. recv_replacement_stream_.Init(&decompressed_slices, recv_flags); recv_message_->reset(recv_replacement_stream_.get()); recv_message_ = nullptr; } + grpc_slice_buffer_destroy_internal(&recv_slices_); ContinueRecvMessageReadyCallback(GRPC_ERROR_REF(error_)); } diff --git a/src/core/lib/transport/byte_stream.h b/src/core/lib/transport/byte_stream.h index 6988ab843b2..ecb605ad9bb 100644 --- a/src/core/lib/transport/byte_stream.h +++ b/src/core/lib/transport/byte_stream.h @@ -26,10 +26,15 @@ #include "src/core/lib/iomgr/closure.h" /** Internal bit flag for grpc_begin_message's \a flags signaling the use of - * compression for the message */ + * compression for the message. (Does not apply for stream compression.) */ #define GRPC_WRITE_INTERNAL_COMPRESS (0x80000000u) +/** Internal bit flag for determining whether the message was compressed and had + * to be decompressed by the message_decompress filter. (Does not apply for + * stream compression.) */ +#define GRPC_WRITE_INTERNAL_TEST_ONLY_WAS_COMPRESSED (0x40000000u) /** Mask of all valid internal flags. */ -#define GRPC_WRITE_INTERNAL_USED_MASK (GRPC_WRITE_INTERNAL_COMPRESS) +#define GRPC_WRITE_INTERNAL_USED_MASK \ + (GRPC_WRITE_INTERNAL_COMPRESS | GRPC_WRITE_INTERNAL_TEST_ONLY_WAS_COMPRESSED) namespace grpc_core { diff --git a/test/cpp/interop/client_helper.h b/test/cpp/interop/client_helper.h index e0bb6eacd90..e97274177ab 100644 --- a/test/cpp/interop/client_helper.h +++ b/test/cpp/interop/client_helper.h @@ -27,6 +27,7 @@ #include #include "src/core/lib/surface/call_test_only.h" +#include "src/core/lib/transport/byte_stream.h" namespace grpc { namespace testing { @@ -54,8 +55,11 @@ class InteropClientContextInspector { return grpc_call_test_only_get_compression_algorithm(context_.call_); } - uint32_t GetMessageFlags() const { - return grpc_call_test_only_get_message_flags(context_.call_); + bool WasCompressed() const { + return (grpc_call_test_only_get_message_flags(context_.call_) & + GRPC_WRITE_INTERNAL_COMPRESS) || + (grpc_call_test_only_get_message_flags(context_.call_) & + GRPC_WRITE_INTERNAL_TEST_ONLY_WAS_COMPRESSED); } private: diff --git a/test/cpp/interop/interop_client.cc b/test/cpp/interop/interop_client.cc index b05e1de82ef..71ee79a6e96 100644 --- a/test/cpp/interop/interop_client.cc +++ b/test/cpp/interop/interop_client.cc @@ -30,7 +30,6 @@ #include #include -#include "src/core/lib/transport/byte_stream.h" #include "src/proto/grpc/testing/empty.pb.h" #include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" @@ -67,10 +66,10 @@ void UnaryCompressionChecks(const InteropClientContextInspector& inspector, "from server."); abort(); } - GPR_ASSERT(inspector.GetMessageFlags() & GRPC_WRITE_INTERNAL_COMPRESS); + GPR_ASSERT(inspector.WasCompressed()); } else { // Didn't request compression -> make sure the response is uncompressed - GPR_ASSERT(!(inspector.GetMessageFlags() & GRPC_WRITE_INTERNAL_COMPRESS)); + GPR_ASSERT(!(inspector.WasCompressed())); } } } // namespace @@ -577,10 +576,10 @@ bool InteropClient::DoServerCompressedStreaming() { GPR_ASSERT(request.response_parameters(k).has_compressed()); if (request.response_parameters(k).compressed().value()) { GPR_ASSERT(inspector.GetCallCompressionAlgorithm() > GRPC_COMPRESS_NONE); - GPR_ASSERT(inspector.GetMessageFlags() & GRPC_WRITE_INTERNAL_COMPRESS); + GPR_ASSERT(inspector.WasCompressed()); } else { // requested *no* compression. - GPR_ASSERT(!(inspector.GetMessageFlags() & GRPC_WRITE_INTERNAL_COMPRESS)); + GPR_ASSERT(!(inspector.WasCompressed())); } ++k; } diff --git a/test/cpp/interop/interop_server.cc b/test/cpp/interop/interop_server.cc index 5482e4f6759..ca3a03883df 100644 --- a/test/cpp/interop/interop_server.cc +++ b/test/cpp/interop/interop_server.cc @@ -31,7 +31,6 @@ #include #include "src/core/lib/gpr/string.h" -#include "src/core/lib/transport/byte_stream.h" #include "src/proto/grpc/testing/empty.pb.h" #include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" @@ -118,7 +117,7 @@ bool CheckExpectedCompression(const ServerContext& context, "Expected compression but got uncompressed request from client."); return false; } - if (!(inspector.GetMessageFlags() & GRPC_WRITE_INTERNAL_COMPRESS)) { + if (!(inspector.WasCompressed())) { gpr_log(GPR_ERROR, "Failure: Requested compression in a compressable request, but " "compression bit in message flags not set."); @@ -126,7 +125,7 @@ bool CheckExpectedCompression(const ServerContext& context, } } else { // Didn't expect compression -> make sure the request is uncompressed - if (inspector.GetMessageFlags() & GRPC_WRITE_INTERNAL_COMPRESS) { + if (inspector.WasCompressed()) { gpr_log(GPR_ERROR, "Failure: Didn't requested compression, but compression bit in " "message flags set."); diff --git a/test/cpp/interop/server_helper.cc b/test/cpp/interop/server_helper.cc index 21945219982..6a3637f6bfa 100644 --- a/test/cpp/interop/server_helper.cc +++ b/test/cpp/interop/server_helper.cc @@ -24,6 +24,7 @@ #include #include "src/core/lib/surface/call_test_only.h" +#include "src/core/lib/transport/byte_stream.h" #include "test/cpp/util/test_credentials_provider.h" DECLARE_bool(use_alts); @@ -60,8 +61,11 @@ uint32_t InteropServerContextInspector::GetEncodingsAcceptedByClient() const { return grpc_call_test_only_get_encodings_accepted_by_peer(context_.call_); } -uint32_t InteropServerContextInspector::GetMessageFlags() const { - return grpc_call_test_only_get_message_flags(context_.call_); +bool InteropServerContextInspector::WasCompressed() const { + return (grpc_call_test_only_get_message_flags(context_.call_) & + GRPC_WRITE_INTERNAL_COMPRESS) || + (grpc_call_test_only_get_message_flags(context_.call_) & + GRPC_WRITE_INTERNAL_TEST_ONLY_WAS_COMPRESSED); } std::shared_ptr diff --git a/test/cpp/interop/server_helper.h b/test/cpp/interop/server_helper.h index 1bfbf8e474d..237c33cd0cd 100644 --- a/test/cpp/interop/server_helper.h +++ b/test/cpp/interop/server_helper.h @@ -44,7 +44,7 @@ class InteropServerContextInspector { bool IsCancelled() const; grpc_compression_algorithm GetCallCompressionAlgorithm() const; uint32_t GetEncodingsAcceptedByClient() const; - uint32_t GetMessageFlags() const; + bool WasCompressed() const; private: const ::grpc::ServerContext& context_;