From 4789448592e3afebae47bf50e9b2e3e5262c78c7 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Wed, 17 May 2017 22:01:13 -0700 Subject: [PATCH 1/3] transcoding: Refactor integration test --- src/envoy/transcoding/filter_test.cc | 2 +- src/envoy/transcoding/integration_test.cc | 162 +++++++++++++++------- 2 files changed, 115 insertions(+), 49 deletions(-) diff --git a/src/envoy/transcoding/filter_test.cc b/src/envoy/transcoding/filter_test.cc index cb39100d7c2..95b31e88dbd 100644 --- a/src/envoy/transcoding/filter_test.cc +++ b/src/envoy/transcoding/filter_test.cc @@ -53,7 +53,7 @@ class GrpcHttpJsonTranscodingFilterTest : public testing::Test { filter_.setEncoderFilterCallbacks(encoder_callbacks_); } - const Json::ObjectPtr bookstoreJson() { + const Json::ObjectSharedPtr bookstoreJson() { std::string descriptor_path = TestEnvironment::runfilesPath( "src/envoy/transcoding/test/bookstore.descriptor"); std::string json_string = "{\"proto_descriptor\": \"" + descriptor_path + diff --git a/src/envoy/transcoding/integration_test.cc b/src/envoy/transcoding/integration_test.cc index 9e6d2b47a97..f079908997a 100644 --- a/src/envoy/transcoding/integration_test.cc +++ b/src/envoy/transcoding/integration_test.cc @@ -13,13 +13,22 @@ * limitations under the License. */ -#include "test/integration/integration.h" #include "common/grpc/codec.h" #include "common/grpc/common.h" +#include "common/http/message_impl.h" #include "src/envoy/transcoding/test/bookstore.pb.h" +#include "google/protobuf/stubs/status.h" +#include "google/protobuf/util/message_differencer.h" #include "gtest/gtest.h" +#include "test/integration/integration.h" +#include "test/mocks/http/mocks.h" + +using google::protobuf::util::MessageDifferencer; +using google::protobuf::util::Status; +using google::protobuf::Message; + namespace Envoy { namespace { @@ -44,64 +53,121 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, test_server_.reset(); fake_upstreams_.clear(); } + + protected: + template + void testTranscoding(Http::Message& request, + const std::vector& grpc_request_messages, + const std::vector& grpc_response_messages, + const Status& grpc_status, + Http::Message& expected_response) { + IntegrationCodecClientPtr codec_client; + FakeHttpConnectionPtr fake_upstream_connection; + IntegrationStreamDecoderPtr response( + new IntegrationStreamDecoder(*dispatcher_)); + FakeStreamPtr request_stream; + + codec_client = + makeHttpConnection(lookupPort("http"), Http::CodecClient::Type::HTTP1); + + if (request.body()) { + Http::StreamEncoder& encoder = + codec_client->startRequest(request.headers(), *response); + codec_client->sendData(encoder, *request.body(), true); + } else { + codec_client->makeHeaderOnlyRequest(request.headers(), *response); + } + + if (!grpc_request_messages.empty()) { + fake_upstream_connection = + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); + request_stream = fake_upstream_connection->waitForNewStream(); + request_stream->waitForEndStream(*dispatcher_); + + Grpc::Decoder grpc_decoder; + std::vector frames; + grpc_decoder.decode(request_stream->body(), frames); + + EXPECT_EQ(grpc_request_messages.size(), frames.size()); + + for (size_t i = 0; i < grpc_request_messages.size(); ++i) { + RequestType actual_message; + actual_message.ParseFromArray( + frames[i].data_->linearize(frames[i].length_), frames[i].length_); + + EXPECT_TRUE(MessageDifferencer::Equals(grpc_request_messages[i], + actual_message)); + } + } + + if (request_stream) { + Http::TestHeaderMapImpl response_headers; + response_headers.insertStatus().value(200); + response_headers.insertContentType().value( + std::string("application/grpc")); + if (grpc_response_messages.empty()) { + response_headers.insertGrpcStatus().value(grpc_status.error_code()); + response_headers.insertGrpcMessage().value(grpc_status.error_message()); + request_stream->encodeHeaders(response_headers, true); + } else { + request_stream->encodeHeaders(response_headers, false); + for (const auto& response_message : grpc_response_messages) { + auto buffer = Grpc::Common::serializeBody(response_message); + request_stream->encodeData(*buffer, false); + } + Http::TestHeaderMapImpl response_trailers; + response_trailers.insertGrpcStatus().value(grpc_status.error_code()); + response_trailers.insertGrpcMessage().value( + grpc_status.error_message()); + request_stream->encodeTrailers(response_trailers); + } + EXPECT_TRUE(request_stream->complete()); + } + + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + expected_response.headers().iterate( + [](const Http::HeaderEntry& entry, void* context) -> void { + IntegrationStreamDecoder* response = + static_cast(context); + Http::LowerCaseString lower_key{entry.key().c_str()}; + EXPECT_STREQ(entry.value().c_str(), + response->headers().get(lower_key)->value().c_str()); + }, + response.get()); + if (expected_response.body()) { + EXPECT_EQ(expected_response.bodyAsString(), response->body()); + } + + codec_client->close(); + fake_upstream_connection->close(); + fake_upstream_connection->waitForDisconnect(); + } }; TEST_F(TranscodingIntegrationTest, BasicUnary) { - IntegrationCodecClientPtr codec_client; - FakeHttpConnectionPtr fake_upstream_connection; - IntegrationStreamDecoderPtr response( - new IntegrationStreamDecoder(*dispatcher_)); - - codec_client = - makeHttpConnection(lookupPort("http"), Http::CodecClient::Type::HTTP1); - Http::StreamEncoder& encoder = codec_client->startRequest( - Http::TestHeaderMapImpl{{":method", "POST"}, - {":path", "/shelf"}, - {":authority", "host"}, - {"content-type", "application/json"}}, - *response); - Buffer::OwnedImpl request_data{"{\"theme\": \"Children\"}"}; - codec_client->sendData(encoder, request_data, true); - - fake_upstream_connection = - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); - FakeStreamPtr request_stream = fake_upstream_connection->waitForNewStream(); - - request_stream->waitForEndStream(*dispatcher_); - - Grpc::Decoder grpc_decoder; - std::vector frames; - grpc_decoder.decode(request_stream->body(), frames); - EXPECT_EQ(1, frames.size()); - + Http::MessagePtr request(new Http::RequestMessageImpl(Http::HeaderMapPtr{ + new Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/shelf"}, + {":authority", "host"}, + {"content-type", "application/json"}}})); + request->body().reset(new Buffer::OwnedImpl{"{\"theme\": \"Children\"}"}); bookstore::CreateShelfRequest csr; - csr.ParseFromArray(frames[0].data_->linearize(frames[0].length_), - frames[0].length_); - EXPECT_EQ("Children", csr.shelf().theme()); - - request_stream->encodeHeaders( - Http::TestHeaderMapImpl{{"content-type", "application/grpc"}, - {":status", "200"}}, - false); + csr.mutable_shelf()->set_theme("Children"); bookstore::Shelf response_pb; response_pb.set_id(20); response_pb.set_theme("Children"); - auto response_data = Grpc::Common::serializeBody(response_pb); - request_stream->encodeData(*response_data, false); - - request_stream->encodeTrailers( - Http::TestHeaderMapImpl{{"grpc-status", "0"}, {"grpc-message", ""}}); + Http::MessagePtr response(new Http::ResponseMessageImpl( + Http::HeaderMapPtr{new Http::TestHeaderMapImpl{ + {":status", "200"}, {"content-type", "application/json"}}})); - response->waitForEndStream(); - EXPECT_TRUE(request_stream->complete()); - EXPECT_TRUE(response->complete()); - EXPECT_EQ("{\"id\":\"20\",\"theme\":\"Children\"}", response->body()); + response->body().reset( + new Buffer::OwnedImpl{"{\"id\":\"20\",\"theme\":\"Children\"}"}); - codec_client->close(); - fake_upstream_connection->close(); - fake_upstream_connection->waitForDisconnect(); + testTranscoding( + *request, {csr}, {response_pb}, Status::OK, *response); } } // namespace From 57d739d64374faa1380762fb16c879459add35ad Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Thu, 18 May 2017 00:44:39 -0700 Subject: [PATCH 2/3] fix --- src/envoy/transcoding/integration_test.cc | 76 +++++++++++----------- src/envoy/transcoding/test/bookstore.proto | 3 + 2 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/envoy/transcoding/integration_test.cc b/src/envoy/transcoding/integration_test.cc index f079908997a..3e958d33116 100644 --- a/src/envoy/transcoding/integration_test.cc +++ b/src/envoy/transcoding/integration_test.cc @@ -19,6 +19,7 @@ #include "src/envoy/transcoding/test/bookstore.pb.h" #include "google/protobuf/stubs/status.h" +#include "google/protobuf/text_format.h" #include "google/protobuf/util/message_differencer.h" #include "gtest/gtest.h" @@ -28,6 +29,7 @@ using google::protobuf::util::MessageDifferencer; using google::protobuf::util::Status; using google::protobuf::Message; +using google::protobuf::TextFormat; namespace Envoy { namespace { @@ -56,11 +58,13 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, protected: template - void testTranscoding(Http::Message& request, - const std::vector& grpc_request_messages, - const std::vector& grpc_response_messages, + void testTranscoding(Http::HeaderMap&& request_headers, + const std::string& request_body, + const std::vector& grpc_request_messages, + const std::vector& grpc_response_messages, const Status& grpc_status, - Http::Message& expected_response) { + Http::HeaderMap&& response_headers, + const std::string& response_body) { IntegrationCodecClientPtr codec_client; FakeHttpConnectionPtr fake_upstream_connection; IntegrationStreamDecoderPtr response( @@ -70,12 +74,13 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, codec_client = makeHttpConnection(lookupPort("http"), Http::CodecClient::Type::HTTP1); - if (request.body()) { + if (!request_body.empty()) { Http::StreamEncoder& encoder = - codec_client->startRequest(request.headers(), *response); - codec_client->sendData(encoder, *request.body(), true); + codec_client->startRequest(request_headers, *response); + Buffer::OwnedImpl body(request_body); + codec_client->sendData(encoder, body, true); } else { - codec_client->makeHeaderOnlyRequest(request.headers(), *response); + codec_client->makeHeaderOnlyRequest(request_headers, *response); } if (!grpc_request_messages.empty()) { @@ -92,11 +97,15 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, for (size_t i = 0; i < grpc_request_messages.size(); ++i) { RequestType actual_message; - actual_message.ParseFromArray( - frames[i].data_->linearize(frames[i].length_), frames[i].length_); + EXPECT_TRUE(actual_message.ParseFromArray( + frames[i].data_->linearize(frames[i].length_), frames[i].length_)); - EXPECT_TRUE(MessageDifferencer::Equals(grpc_request_messages[i], - actual_message)); + RequestType expected_message; + EXPECT_TRUE(TextFormat::ParseFromString(grpc_request_messages[i], + &expected_message)); + + EXPECT_TRUE( + MessageDifferencer::Equivalent(expected_message, actual_message)); } } @@ -111,7 +120,10 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, request_stream->encodeHeaders(response_headers, true); } else { request_stream->encodeHeaders(response_headers, false); - for (const auto& response_message : grpc_response_messages) { + for (const auto& response_message_str : grpc_response_messages) { + ResponseType response_message; + EXPECT_TRUE(TextFormat::ParseFromString(response_message_str, + &response_message)); auto buffer = Grpc::Common::serializeBody(response_message); request_stream->encodeData(*buffer, false); } @@ -126,7 +138,7 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, response->waitForEndStream(); EXPECT_TRUE(response->complete()); - expected_response.headers().iterate( + response_headers.iterate( [](const Http::HeaderEntry& entry, void* context) -> void { IntegrationStreamDecoder* response = static_cast(context); @@ -135,8 +147,8 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, response->headers().get(lower_key)->value().c_str()); }, response.get()); - if (expected_response.body()) { - EXPECT_EQ(expected_response.bodyAsString(), response->body()); + if (!response_body.empty()) { + EXPECT_EQ(response_body, response->body()); } codec_client->close(); @@ -145,29 +157,17 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, } }; -TEST_F(TranscodingIntegrationTest, BasicUnary) { - Http::MessagePtr request(new Http::RequestMessageImpl(Http::HeaderMapPtr{ - new Http::TestHeaderMapImpl{{":method", "POST"}, - {":path", "/shelf"}, - {":authority", "host"}, - {"content-type", "application/json"}}})); - request->body().reset(new Buffer::OwnedImpl{"{\"theme\": \"Children\"}"}); - bookstore::CreateShelfRequest csr; - csr.mutable_shelf()->set_theme("Children"); - - bookstore::Shelf response_pb; - response_pb.set_id(20); - response_pb.set_theme("Children"); - - Http::MessagePtr response(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{ - {":status", "200"}, {"content-type", "application/json"}}})); - - response->body().reset( - new Buffer::OwnedImpl{"{\"id\":\"20\",\"theme\":\"Children\"}"}); - +TEST_F(TranscodingIntegrationTest, UnaryPost) { testTranscoding( - *request, {csr}, {response_pb}, Status::OK, *response); + Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/shelf"}, + {":authority", "host"}, + {"content-type", "application/json"}}, + R"({"theme": "Children"})", {R"(shelf { theme: "Children" })"}, + {R"(id: 20 theme: "Children" )"}, Status::OK, + Http::TestHeaderMapImpl{{":status", "200"}, + {"content-type", "application/json"}}, + R"({"id":"20","theme":"Children"})"); } } // namespace diff --git a/src/envoy/transcoding/test/bookstore.proto b/src/envoy/transcoding/test/bookstore.proto index da088d7d5bb..853567383be 100644 --- a/src/envoy/transcoding/test/bookstore.proto +++ b/src/envoy/transcoding/test/bookstore.proto @@ -27,6 +27,9 @@ import "google/protobuf/empty.proto"; service Bookstore { // Returns a list of all shelves in the bookstore. rpc ListShelves(google.protobuf.Empty) returns (ListShelvesResponse) { + option (google.api.http) = { + get: "/shelves" + }; } // Creates a new shelf in the bookstore. rpc CreateShelf(CreateShelfRequest) returns (Shelf) { From 76a3d1fcc358183f28756e936535196ecc69d643 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Thu, 18 May 2017 02:52:16 -0700 Subject: [PATCH 3/3] More tests --- WORKSPACE | 2 +- src/envoy/transcoding/integration_test.cc | 134 +++++++++++++++++++-- src/envoy/transcoding/test/bookstore.proto | 16 ++- 3 files changed, 143 insertions(+), 9 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 2fb3d785a2a..5be9cbd5954 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -71,7 +71,7 @@ bind( git_repository( name = "envoy", remote = "https://github.com/lyft/envoy.git", - commit = "f10afbaaa6445862bc7db27f9c0d6c2914e8cfd7", + commit = "d0ff5a35e4ad844cd21446dab9701843490c5f01", ) load("@envoy//bazel:repositories.bzl", "envoy_dependencies") diff --git a/src/envoy/transcoding/integration_test.cc b/src/envoy/transcoding/integration_test.cc index 3e958d33116..96080415005 100644 --- a/src/envoy/transcoding/integration_test.cc +++ b/src/envoy/transcoding/integration_test.cc @@ -28,6 +28,7 @@ using google::protobuf::util::MessageDifferencer; using google::protobuf::util::Status; +using google::protobuf::Empty; using google::protobuf::Message; using google::protobuf::TextFormat; @@ -83,23 +84,24 @@ class TranscodingIntegrationTest : public BaseIntegrationTest, codec_client->makeHeaderOnlyRequest(request_headers, *response); } + fake_upstream_connection = + fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); if (!grpc_request_messages.empty()) { - fake_upstream_connection = - fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); request_stream = fake_upstream_connection->waitForNewStream(); request_stream->waitForEndStream(*dispatcher_); Grpc::Decoder grpc_decoder; std::vector frames; - grpc_decoder.decode(request_stream->body(), frames); - + EXPECT_TRUE(grpc_decoder.decode(request_stream->body(), frames)); EXPECT_EQ(grpc_request_messages.size(), frames.size()); for (size_t i = 0; i < grpc_request_messages.size(); ++i) { RequestType actual_message; - EXPECT_TRUE(actual_message.ParseFromArray( - frames[i].data_->linearize(frames[i].length_), frames[i].length_)); - + if (frames[i].length_ > 0) { + EXPECT_TRUE(actual_message.ParseFromArray( + frames[i].data_->linearize(frames[i].length_), + frames[i].length_)); + } RequestType expected_message; EXPECT_TRUE(TextFormat::ParseFromString(grpc_request_messages[i], &expected_message)); @@ -170,5 +172,123 @@ TEST_F(TranscodingIntegrationTest, UnaryPost) { R"({"id":"20","theme":"Children"})"); } +TEST_F(TranscodingIntegrationTest, UnaryGet) { + testTranscoding( + Http::TestHeaderMapImpl{ + {":method", "GET"}, {":path", "/shelves"}, {":authority", "host"}}, + "", {""}, {R"(shelves { id: 20 theme: "Children" } + shelves { id: 1 theme: "Foo" } )"}, + Status::OK, Http::TestHeaderMapImpl{{":status", "200"}, + {"content-type", "application/json"}}, + R"({"shelves":[{"id":"20","theme":"Children"},{"id":"1","theme":"Foo"}]})"); +} + +TEST_F(TranscodingIntegrationTest, UnaryDelete) { + testTranscoding( + Http::TestHeaderMapImpl{{":method", "DELETE"}, + {":path", "/shelves/456/books/123"}, + {":authority", "host"}}, + "", {"shelf: 456 book: 123"}, {""}, Status::OK, + Http::TestHeaderMapImpl{{":status", "200"}, + {"content-type", "application/json"}}, + "{}"); +} + +TEST_F(TranscodingIntegrationTest, BindingAndBody) { + testTranscoding( + Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/shelves/1/books"}, + {":authority", "host"}}, + R"({"author" : "Leo Tolstoy", "title" : "War and Peace"})", + {R"(shelf: 1 book { author: "Leo Tolstoy" title: "War and Peace" })"}, + { + R"(id: 3 author: "Leo Tolstoy" title: "War and Peace")", + }, + Status::OK, Http::TestHeaderMapImpl{{":status", "200"}, + {"content-type", "application/json"}}, + R"({"id":"3","author":"Leo Tolstoy","title":"War and Peace"})"); +} + +TEST_F(TranscodingIntegrationTest, ServerStreamingGet) { + testTranscoding( + Http::TestHeaderMapImpl{{":method", "GET"}, + {":path", "/shelves/1/books"}, + {":authority", "host"}}, + "", {"shelf: 1"}, + {R"(id: 1 author: "Neal Stephenson" title: "Readme")", + R"(id: 2 author: "George R.R. Martin" title: "A Game of Thrones")"}, + Status::OK, Http::TestHeaderMapImpl{{":status", "200"}, + {"content-type", "application/json"}}, + R"([{"id":"1","author":"Neal Stephenson","title":"Readme"})" + R"(,{"id":"2","author":"George R.R. Martin","title":"A Game of Thrones"}])"); +} + +TEST_F(TranscodingIntegrationTest, StreamingPost) { + testTranscoding( + Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/bulk/shelves"}, + {":authority", "host"}}, + R"([ + { "theme" : "Classics" }, + { "theme" : "Satire" }, + { "theme" : "Russian" }, + { "theme" : "Children" }, + { "theme" : "Documentary" }, + { "theme" : "Mystery" }, + ])", + {R"(shelf { theme: "Classics" })", + R"(shelf { theme: "Satire" })", + R"(shelf { theme: "Russian" })", + R"(shelf { theme: "Children" })", + R"(shelf { theme: "Documentary" })", + R"(shelf { theme: "Mystery" })"}, + {R"(id: 3 theme: "Classics")", + R"(id: 4 theme: "Satire")", + R"(id: 5 theme: "Russian")", + R"(id: 6 theme: "Children")", + R"(id: 7 theme: "Documentary")", + R"(id: 8 theme: "Mystery")"}, + Status::OK, Http::TestHeaderMapImpl{{":status", "200"}, + {"content-type", "application/json"}}, + R"([{"id":"3","theme":"Classics"})" + R"(,{"id":"4","theme":"Satire"})" + R"(,{"id":"5","theme":"Russian"})" + R"(,{"id":"6","theme":"Children"})" + R"(,{"id":"7","theme":"Documentary"})" + R"(,{"id":"8","theme":"Mystery"}])"); +} + +TEST_F(TranscodingIntegrationTest, InvalidJson) { + testTranscoding( + Http::TestHeaderMapImpl{ + {":method", "POST"}, {":path", "/shelf"}, {":authority", "host"}}, + R"(INVALID_JSON)", {}, {}, Status::OK, + Http::TestHeaderMapImpl{{":status", "400"}, + {"content-type", "text/plain"}}, + "Unexpected token.\n" + "INVALID_JSON\n" + "^"); + + testTranscoding( + Http::TestHeaderMapImpl{ + {":method", "POST"}, {":path", "/shelf"}, {":authority", "host"}}, + R"({ "theme" : "Children")", {}, {}, Status::OK, + Http::TestHeaderMapImpl{{":status", "400"}, + {"content-type", "text/plain"}}, + "Unexpected end of string. Expected , or } after key:value pair.\n" + "\n" + "^"); + + testTranscoding( + Http::TestHeaderMapImpl{ + {":method", "POST"}, {":path", "/shelf"}, {":authority", "host"}}, + R"({ "theme" "Children" })", {}, {}, Status::OK, + Http::TestHeaderMapImpl{{":status", "400"}, + {"content-type", "text/plain"}}, + "Expected : between key:value pair.\n" + "{ \"theme\" \"Children\" }\n" + " ^"); +} + } // namespace } // namespace Envoy diff --git a/src/envoy/transcoding/test/bookstore.proto b/src/envoy/transcoding/test/bookstore.proto index 853567383be..2f90b106fa7 100644 --- a/src/envoy/transcoding/test/bookstore.proto +++ b/src/envoy/transcoding/test/bookstore.proto @@ -40,6 +40,10 @@ service Bookstore { } // Creates multiple shelves with one streaming call rpc BulkCreateShelf(stream CreateShelfRequest) returns (stream Shelf) { + option (google.api.http) = { + post: "/bulk/shelves" + body: "shelf" + }; } // Returns a specific bookstore shelf. rpc GetShelf(GetShelfRequest) returns (Shelf) { @@ -49,15 +53,25 @@ service Bookstore { } // Returns a list of books on a shelf. rpc ListBooks(ListBooksRequest) returns (stream Book) { + option (google.api.http) = { + get: "/shelves/{shelf}/books" + }; } // Creates a new book. rpc CreateBook(CreateBookRequest) returns (Book) { + option (google.api.http) = { + post: "/shelves/{shelf}/books" + body: "book" + }; } // Returns a specific book. rpc GetBook(GetBookRequest) returns (Book) { } // Deletes a book from a shelf. - rpc DeleteBook(stream DeleteBookRequest) returns (google.protobuf.Empty) { + rpc DeleteBook(DeleteBookRequest) returns (google.protobuf.Empty) { + option (google.api.http) = { + delete: "/shelves/{shelf}/books/{book}" + }; } }