Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mobile: Remove the deprecated Stream::sendHeaders and Stream::sendTrailers #33933

Merged
merged 1 commit into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions mobile/examples/cc/fetch_client/fetch_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "source/exe/platform_impl.h"

#include "library/common/data/utility.h"
#include "library/common/http/header_utility.h"

namespace Envoy {

Expand Down Expand Up @@ -94,11 +95,13 @@ void Fetch::sendRequest(const absl::string_view url_string) {

Platform::StreamSharedPtr stream = stream_prototype->start(/*explicit_flow_control=*/false);

Platform::RequestHeadersBuilder builder(Platform::RequestMethod::GET, std::string(url.scheme()),
std::string(url.hostAndPort()),
std::string(url.pathAndQueryParams()));
auto headers = Http::Utility::createRequestHeaderMapPtr();
headers->addCopy(Http::LowerCaseString(":method"), "GET");
headers->addCopy(Http::LowerCaseString(":scheme"), "https");
headers->addCopy(Http::LowerCaseString(":authority"), url.hostAndPort());
headers->addCopy(Http::LowerCaseString(":path"), url.pathAndQueryParams());
stream->sendHeaders(std::move(headers), true);

stream->sendHeaders(std::make_shared<Platform::RequestHeaders>(builder.build()), true);
request_finished.WaitForNotification();
}

Expand Down
27 changes: 0 additions & 27 deletions mobile/library/cc/stream.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#include "stream.h"

#include "library/cc/bridge_utility.h"
#include "library/common/data/utility.h"
#include "library/common/http/header_utility.h"
#include "library/common/internal_engine.h"
Expand All @@ -11,22 +10,6 @@ namespace Platform {

Stream::Stream(InternalEngine* engine, envoy_stream_t handle) : engine_(engine), handle_(handle) {}

Stream& Stream::sendHeaders(RequestHeadersSharedPtr headers, bool end_stream) {
auto request_header_map = Http::Utility::createRequestHeaderMapPtr();
for (const auto& [key, values] : headers->allHeaders()) {
if (request_header_map->formatter().has_value()) {
Http::StatefulHeaderKeyFormatter& formatter = request_header_map->formatter().value();
// Make sure the formatter knows the original case.
formatter.processKey(key);
}
for (const auto& value : values) {
request_header_map->addCopy(Http::LowerCaseString(key), value);
}
}
engine_->sendHeaders(handle_, std::move(request_header_map), end_stream);
return *this;
}

Stream& Stream::sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream) {
engine_->sendHeaders(handle_, std::move(headers), end_stream);
return *this;
Expand All @@ -47,16 +30,6 @@ Stream& Stream::readData(size_t bytes_to_read) {
return *this;
}

void Stream::close(RequestTrailersSharedPtr trailers) {
auto request_trailer_map = Http::Utility::createRequestTrailerMapPtr();
for (const auto& [key, values] : trailers->allHeaders()) {
for (const auto& value : values) {
request_trailer_map->addCopy(Http::LowerCaseString(key), value);
}
}
engine_->sendTrailers(handle_, std::move(request_trailer_map));
}

void Stream::close(Http::RequestTrailerMapPtr trailers) {
engine_->sendTrailers(handle_, std::move(trailers));
}
Expand Down
6 changes: 0 additions & 6 deletions mobile/library/cc/stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
#include "envoy/buffer/buffer.h"
#include "envoy/http/header_map.h"

#include "library/cc/request_headers.h"
#include "library/cc/request_trailers.h"
#include "library/common/types/c_types.h"

namespace Envoy {
Expand All @@ -17,8 +15,6 @@ class Stream {
public:
Stream(InternalEngine* engine, envoy_stream_t handle);

[[deprecated]] Stream& sendHeaders(RequestHeadersSharedPtr headers, bool end_stream);

/**
* Send the headers over an open HTTP stream. This function can be invoked
* once and needs to be called before `sendData`.
Expand All @@ -39,8 +35,6 @@ class Stream {

Stream& readData(size_t bytes_to_read);

[[deprecated]] void close(RequestTrailersSharedPtr trailers);

/**
* Send trailers over an open HTTP stream. This method can only be invoked once per stream.
* Note that this method implicitly closes the stream locally.
Expand Down
25 changes: 15 additions & 10 deletions mobile/test/cc/engine_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "absl/synchronization/notification.h"
#include "gtest/gtest.h"
#include "library/cc/engine_builder.h"
#include "library/common/http/header_utility.h"

namespace Envoy {

Expand Down Expand Up @@ -49,11 +50,13 @@ TEST(EngineTest, SetLogger) {
})
.start();

auto request_headers =
Platform::RequestHeadersBuilder(Platform::RequestMethod::GET, "https",
engine_with_test_server.testServer().getAddress(), "/")
.build();
stream->sendHeaders(std::make_shared<Platform::RequestHeaders>(request_headers), true);
auto headers = Http::Utility::createRequestHeaderMapPtr();
headers->addCopy(Http::LowerCaseString(":method"), "GET");
headers->addCopy(Http::LowerCaseString(":scheme"), "https");
headers->addCopy(Http::LowerCaseString(":authority"),
engine_with_test_server.testServer().getAddress());
headers->addCopy(Http::LowerCaseString(":path"), "/");
stream->sendHeaders(std::move(headers), true);
stream_complete.WaitForNotification();

EXPECT_EQ(actual_status_code, 200);
Expand Down Expand Up @@ -96,11 +99,13 @@ TEST(EngineTest, SetEngineCallbacks) {
})
.start();

auto request_headers =
Platform::RequestHeadersBuilder(Platform::RequestMethod::GET, "https",
engine_with_test_server.testServer().getAddress(), "/")
.build();
stream->sendHeaders(std::make_shared<Platform::RequestHeaders>(request_headers), true);
auto headers = Http::Utility::createRequestHeaderMapPtr();
headers->addCopy(Http::LowerCaseString(":method"), "GET");
headers->addCopy(Http::LowerCaseString(":scheme"), "https");
headers->addCopy(Http::LowerCaseString(":authority"),
engine_with_test_server.testServer().getAddress());
headers->addCopy(Http::LowerCaseString(":path"), "/");
stream->sendHeaders(std::move(headers), true);
stream_complete.WaitForNotification();

EXPECT_EQ(actual_status_code, 200);
Expand Down
1 change: 1 addition & 0 deletions mobile/test/cc/integration/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ envoy_cc_test(
repository = "@envoy",
deps = [
"//library/cc:engine_builder_lib",
"//library/common/http:header_utility_lib",
"//test/common/integration:engine_with_test_server",
"//test/common/integration:test_server_lib",
"@envoy_build_config//:test_extensions",
Expand Down
16 changes: 9 additions & 7 deletions mobile/test/cc/integration/lifetimes_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#include "gtest/gtest.h"
#include "library/cc/engine_builder.h"
#include "library/cc/envoy_error.h"
#include "library/cc/request_headers_builder.h"
#include "library/cc/request_method.h"
#include "library/cc/request_headers.h"
#include "library/common/http/header_utility.h"

namespace Envoy {
namespace {
Expand Down Expand Up @@ -41,11 +41,13 @@ void sendRequest() {
})
.start();

auto request_headers =
Platform::RequestHeadersBuilder(Platform::RequestMethod::GET, "https",
engine_with_test_server.testServer().getAddress(), "/")
.build();
stream->sendHeaders(std::make_shared<Platform::RequestHeaders>(request_headers), true);
auto headers = Http::Utility::createRequestHeaderMapPtr();
headers->addCopy(Http::LowerCaseString(":method"), "GET");
headers->addCopy(Http::LowerCaseString(":scheme"), "https");
headers->addCopy(Http::LowerCaseString(":authority"),
engine_with_test_server.testServer().getAddress());
headers->addCopy(Http::LowerCaseString(":path"), "/");
stream->sendHeaders(std::move(headers), true);
stream_complete.WaitForNotification();

EXPECT_EQ(actual_status_code, 200);
Expand Down
48 changes: 0 additions & 48 deletions mobile/test/cc/integration/send_headers_test.cc
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
#include "test/common/integration/engine_with_test_server.h"
#include "test/common/integration/test_server.h"

#include "absl/strings/str_format.h"
#include "absl/synchronization/notification.h"
#include "gtest/gtest.h"
#include "library/cc/engine_builder.h"
#include "library/cc/envoy_error.h"
#include "library/cc/request_headers_builder.h"
#include "library/cc/request_method.h"
#include "library/common/http/header_utility.h"

namespace Envoy {
Expand Down Expand Up @@ -72,49 +69,4 @@ TEST(SendHeadersTest, Success) {
EXPECT_TRUE(actual_end_stream);
}

TEST(SendHeadersTest, SuccessWithDeprecatedFunction) {
absl::Notification engine_running;
Platform::EngineBuilder engine_builder;
engine_builder.enforceTrustChainVerification(false)
.setLogLevel(Logger::Logger::debug)
.setOnEngineRunning([&]() { engine_running.Notify(); });
EngineWithTestServer engine_with_test_server(engine_builder, TestServerType::HTTP2_WITH_TLS);
engine_running.WaitForNotification();

int actual_status_code;
bool actual_end_stream;
absl::Notification stream_complete;
auto stream_prototype = engine_with_test_server.engine()->streamClient()->newStreamPrototype();
Platform::StreamSharedPtr stream =
(*stream_prototype)
.setOnHeaders(
[&](Platform::ResponseHeadersSharedPtr headers, bool end_stream, envoy_stream_intel) {
actual_status_code = headers->httpStatus();
actual_end_stream = end_stream;
})
.setOnData([&](envoy_data data, bool end_stream) {
actual_end_stream = end_stream;
data.release(data.context);
})
.setOnComplete(
[&](envoy_stream_intel, envoy_final_stream_intel) { stream_complete.Notify(); })
.setOnError([&](Platform::EnvoyErrorSharedPtr, envoy_stream_intel,
envoy_final_stream_intel) { stream_complete.Notify(); })
.setOnCancel(
[&](envoy_stream_intel, envoy_final_stream_intel) { stream_complete.Notify(); })
.start();

Platform::RequestHeadersBuilder request_headers_builder(
Platform::RequestMethod::GET, "https", engine_with_test_server.testServer().getAddress(),
"/");
auto request_headers = request_headers_builder.build();
auto request_headers_ptr =
Platform::RequestHeadersSharedPtr(new Platform::RequestHeaders(request_headers));
stream->sendHeaders(request_headers_ptr, true);
stream_complete.WaitForNotification();

EXPECT_EQ(actual_status_code, 200);
EXPECT_TRUE(actual_end_stream);
}

} // namespace Envoy
26 changes: 0 additions & 26 deletions mobile/test/common/integration/base_client_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,32 +121,6 @@ void BaseClientIntegrationTest::initialize() {
default_request_headers_.setHost(fake_upstreams_[0]->localAddress()->asStringView());
}

std::shared_ptr<Platform::RequestHeaders> BaseClientIntegrationTest::envoyToMobileHeaders(
const Http::TestRequestHeaderMapImpl& request_headers) {

Platform::RequestHeadersBuilder builder(
Platform::RequestMethod::GET,
std::string(default_request_headers_.Scheme()->value().getStringView()),
std::string(default_request_headers_.Host()->value().getStringView()),
std::string(default_request_headers_.Path()->value().getStringView()));

request_headers.iterate(
[&request_headers, &builder](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate {
std::string key = std::string(header.key().getStringView());
if (request_headers.formatter().has_value()) {
const Envoy::Http::StatefulHeaderKeyFormatter& formatter =
request_headers.formatter().value();
key = formatter.format(key);
}
auto value = std::vector<std::string>();
value.push_back(std::string(header.value().getStringView()));
builder.set(key, value);
return Http::HeaderMap::Iterate::Continue;
});

return std::make_shared<Platform::RequestHeaders>(builder.build());
}

void BaseClientIntegrationTest::threadRoutine(absl::Notification& engine_running) {
builder_.setOnEngineRunning([&]() { engine_running.Notify(); });
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ class BaseClientIntegrationTest : public BaseIntegrationTest {
void createEnvoy() override;
void threadRoutine(absl::Notification& engine_running);

// Converts TestRequestHeaderMapImpl to Envoy::Platform::RequestHeadersSharedPtr
Envoy::Platform::RequestHeadersSharedPtr
envoyToMobileHeaders(const Http::TestRequestHeaderMapImpl& request_headers);

// Get the value of a Counter in the Envoy instance.
uint64_t getCounterValue(const std::string& name);
// Wait until the Counter specified by `name` is >= `value`.
Expand Down
Loading