mobile: Remove the deprecated Stream::sendHeaders and Stream::sendTra…

…ilers (envoyproxy#33933) Remove deprecated Stream::sendHeaders and Stream::sendTrailers Signed-off-by: Fredy Wijaya <[email protected]>
martinduke · May 2, 2024 · 6091f47 · 6091f47
1 parent 099d859
commit 6091f47
Show file tree

Hide file tree

Showing 11 changed files with 98 additions and 181 deletions.
diff --git a/mobile/examples/cc/fetch_client/fetch_client.cc b/mobile/examples/cc/fetch_client/fetch_client.cc
@@ -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 {
 
@@ -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();
 }
 

diff --git a/mobile/library/cc/stream.cc b/mobile/library/cc/stream.cc
@@ -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"
@@ -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;
@@ -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));
 }

diff --git a/mobile/library/cc/stream.h b/mobile/library/cc/stream.h
@@ -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 {
@@ -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`.
@@ -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.

diff --git a/mobile/test/cc/engine_test.cc b/mobile/test/cc/engine_test.cc
@@ -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 {
 
@@ -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);
@@ -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);

diff --git a/mobile/test/cc/integration/BUILD b/mobile/test/cc/integration/BUILD
@@ -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",

diff --git a/mobile/test/cc/integration/lifetimes_test.cc b/mobile/test/cc/integration/lifetimes_test.cc
@@ -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 {
@@ -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);

diff --git a/mobile/test/cc/integration/send_headers_test.cc b/mobile/test/cc/integration/send_headers_test.cc
@@ -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 {
@@ -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
diff --git a/mobile/test/common/integration/base_client_integration_test.cc b/mobile/test/common/integration/base_client_integration_test.cc
@@ -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(); });
   {

diff --git a/mobile/test/common/integration/base_client_integration_test.h b/mobile/test/common/integration/base_client_integration_test.h
@@ -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`.