Skip to content

Commit 2bb5233

Browse files
scotthartscotthart
authored
impl(rest): improve http header representation (#16058)
1 parent ae60274 commit 2bb5233

26 files changed

+263
-153
lines changed

‎generator/integration_tests/tests/golden_thing_admin_rest_metadata_decorator_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ TEST(ThingAdminRestMetadataDecoratorTest, DropDatabaseExplicitRoutingMatch) {
312312
EXPECT_THAT(context.GetHeader("x-goog-quota-user"), IsEmpty());
313313
EXPECT_THAT(context.GetHeader("x-server-timeout"), IsEmpty());
314314
EXPECT_THAT(
315-
context.GetHeader("x-goog-request-params")[0],
315+
context.GetHeader("x-goog-request-params").values().front(),
316316
AllOf(
317317
HasSubstr(std::string("project=projects%2Fmy_project")),
318318
HasSubstr(std::string("instance=instances%2Fmy_instance")),

‎google/cloud/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ cc_library(
256256
visibility = ["//:__subpackages__"],
257257
deps = [
258258
":google_cloud_cpp_common",
259+
"@abseil-cpp//absl/container:flat_hash_map",
259260
"@abseil-cpp//absl/functional:function_ref",
260261
"@abseil-cpp//absl/types:span",
261262
"@curl",

‎google/cloud/bigquery/v2/minimal/internal/log_wrapper.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ Result LogWrapper(Functor&& functor, rest_internal::RestContext& context,
3838
TracingOptions const& options) {
3939
auto formatter = [options](std::string* out, auto const& header) {
4040
auto const* delim = options.single_line_mode() ? "&" : "\n";
41-
absl::StrAppend(
42-
out, " { name: \"", header.first, "\" value: \"",
43-
internal::DebugString(absl::StrJoin(header.second, delim), options),
44-
"\" }");
41+
absl::StrAppend(out, " { name: \"", std::string_view{header.first},
42+
"\" value: \"",
43+
internal::DebugString(
44+
absl::StrJoin(header.second.values(), delim), options),
45+
"\" }");
4546
};
4647
GCP_LOG(DEBUG) << where << "() << "
4748
<< request.DebugString(request_name, options) << ", Context {"

‎google/cloud/bigquery/v2/minimal/internal/rest_stub_utils.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ StatusOr<rest_internal::RestRequest> PrepareRestRequest(
4848
if (!rest_context.headers().empty()) {
4949
for (auto const& h : rest_context.headers()) {
5050
if (!h.second.empty()) {
51-
rest_request->AddHeader(h.first, absl::StrJoin(h.second, "&"));
51+
rest_request->AddHeader(h.first, absl::StrJoin(h.second.values(), "&"));
5252
}
5353
}
5454
}

‎google/cloud/bigquery/v2/minimal/testing/metadata_test_utils.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@ static auto const kUserProject = "test-only-project";
3030
static auto const kQuotaUser = "test-quota-user";
3131

3232
void VerifyMetadataContext(rest_internal::RestContext& context) {
33-
EXPECT_THAT(context.GetHeader("x-goog-api-client"),
33+
EXPECT_THAT(context.GetHeader("x-goog-api-client").values(),
3434
ElementsAre(internal::HandCraftedLibClientHeader()));
35-
EXPECT_THAT(context.GetHeader("x-goog-request-params"), IsEmpty());
36-
EXPECT_THAT(context.GetHeader("x-goog-user-project"),
35+
EXPECT_THAT(context.GetHeader("x-goog-request-params").values(), IsEmpty());
36+
EXPECT_THAT(context.GetHeader("x-goog-user-project").values(),
3737
ElementsAre(kUserProject));
38-
EXPECT_THAT(context.GetHeader("x-goog-quota-user"), ElementsAre(kQuotaUser));
39-
EXPECT_THAT(context.GetHeader("x-server-timeout"), ElementsAre("3.141"));
38+
EXPECT_THAT(context.GetHeader("x-goog-quota-user").values(),
39+
ElementsAre(kQuotaUser));
40+
EXPECT_THAT(context.GetHeader("x-server-timeout").values(),
41+
ElementsAre("3.141"));
4042
}
4143

4244
Options GetMetadataOptions() {

‎google/cloud/google_cloud_cpp_rest_internal.cmake‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ add_library(
137137
target_link_libraries(
138138
google_cloud_cpp_rest_internal
139139
PUBLIC absl::span google-cloud-cpp::common CURL::libcurl
140-
nlohmann_json::nlohmann_json)
140+
absl::flat_hash_map nlohmann_json::nlohmann_json)
141141
if (WIN32)
142142
target_compile_definitions(google_cloud_cpp_rest_internal
143143
PRIVATE WIN32_LEAN_AND_MEAN)
@@ -197,6 +197,7 @@ google_cloud_cpp_add_pkgconfig(
197197
"Provides REST Transport for the Google Cloud C++ Client Library."
198198
"google_cloud_cpp_common"
199199
"libcurl"
200+
"absl_flat_hash_map"
200201
NON_WIN32_REQUIRES
201202
openssl
202203
WIN32_LIBS

‎google/cloud/internal/curl_impl.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,9 @@ void CurlImpl::MergeAndWriteHeaders(
297297
}
298298
}
299299

300-
void CurlImpl::SetHeaders(
301-
std::unordered_map<std::string, std::vector<std::string>> const& headers) {
300+
void CurlImpl::SetHeaders(HttpHeaders const& headers) {
302301
for (auto const& header : headers) {
303-
SetHeader(HttpHeader(header.first, header.second));
302+
SetHeader(header.second);
304303
}
305304
}
306305

‎google/cloud/internal/curl_impl.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
#include <map>
3636
#include <memory>
3737
#include <string>
38-
#include <utility>
3938
#include <vector>
4039

4140
namespace google {
@@ -84,8 +83,7 @@ class CurlImpl {
8483
CurlImpl& operator=(CurlImpl&&) = default;
8584

8685
void SetHeader(HttpHeader header);
87-
void SetHeaders(
88-
std::unordered_map<std::string, std::vector<std::string>> const& headers);
86+
void SetHeaders(HttpHeaders const& headers);
8987

9088
std::string MakeEscapedString(std::string const& s);
9189

‎google/cloud/internal/curl_rest_client_test.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,21 @@ TEST(CurlRestClientStandaloneFunctions, HostHeader) {
2929
std::string expected;
3030
} cases[] = {
3131
{"https://storage.googleapis.com", "storage.googleapis.com",
32-
"Host: storage.googleapis.com"},
33-
{"https://storage.googleapis.com", "", "Host: storage.googleapis.com"},
34-
{"https://storage.googleapis.com", "auth", "Host: auth"},
32+
"host: storage.googleapis.com"},
33+
{"https://storage.googleapis.com", "", "host: storage.googleapis.com"},
34+
{"https://storage.googleapis.com", "auth", "host: auth"},
3535
{"https://storage.googleapis.com:443", "storage.googleapis.com",
36-
"Host: storage.googleapis.com"},
36+
"host: storage.googleapis.com"},
3737
{"https://restricted.googleapis.com", "storage.googleapis.com",
38-
"Host: storage.googleapis.com"},
38+
"host: storage.googleapis.com"},
3939
{"https://private.googleapis.com", "storage.googleapis.com",
40-
"Host: storage.googleapis.com"},
40+
"host: storage.googleapis.com"},
4141
{"https://restricted.googleapis.com", "iamcredentials.googleapis.com",
42-
"Host: iamcredentials.googleapis.com"},
42+
"host: iamcredentials.googleapis.com"},
4343
{"https://private.googleapis.com", "iamcredentials.googleapis.com",
44-
"Host: iamcredentials.googleapis.com"},
44+
"host: iamcredentials.googleapis.com"},
4545
{"http://localhost:8080", "", ""},
46-
{"http://localhost:8080", "auth", "Host: auth"},
46+
{"http://localhost:8080", "auth", "host: auth"},
4747
{"http://[::1]", "", ""},
4848
{"http://[::1]/", "", ""},
4949
{"http://[::1]/foo/bar", "", ""},

‎google/cloud/internal/http_header.cc‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,48 +22,50 @@ namespace cloud {
2222
namespace rest_internal {
2323
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
2424

25-
HttpHeader::HttpHeader(std::string key) : key_(std::move(key)) {}
25+
HttpHeader::HttpHeader(HttpHeaderName key) : name_(std::move(key)) {}
2626

27-
HttpHeader::HttpHeader(std::string key, std::string value)
28-
: key_(std::move(key)), values_({std::move(value)}) {}
27+
HttpHeader::HttpHeader(std::pair<std::string, std::string> header)
28+
: HttpHeader(std::move(header.first), std::move(header.second)) {}
2929

30-
HttpHeader::HttpHeader(std::string key, std::vector<std::string> values)
31-
: key_(std::move(key)), values_(std::move(values)) {}
30+
HttpHeader::HttpHeader(HttpHeaderName key, std::string value)
31+
: name_(std::move(key)), values_({std::move(value)}) {}
3232

33-
HttpHeader::HttpHeader(std::string key,
33+
HttpHeader::HttpHeader(HttpHeaderName key, std::vector<std::string> values)
34+
: name_(std::move(key)), values_(std::move(values)) {}
35+
36+
HttpHeader::HttpHeader(HttpHeaderName key,
3437
std::initializer_list<char const*> values)
35-
: key_(std::move(key)) {
38+
: name_(std::move(key)) {
3639
for (auto&& v : values) values_.emplace_back(v);
3740
}
3841

3942
bool operator==(HttpHeader const& lhs, HttpHeader const& rhs) {
40-
return absl::AsciiStrToLower(lhs.key_) == absl::AsciiStrToLower(rhs.key_) &&
41-
lhs.values_ == rhs.values_;
43+
return lhs.name_ == rhs.name_ && lhs.values_ == rhs.values_;
4244
}
4345

4446
bool operator<(HttpHeader const& lhs, HttpHeader const& rhs) {
45-
return absl::AsciiStrToLower(lhs.key_) < absl::AsciiStrToLower(rhs.key_);
47+
return lhs.name_ < rhs.name_;
4648
}
4749

48-
bool HttpHeader::IsSameKey(std::string const& key) const {
49-
return absl::AsciiStrToLower(key_) == absl::AsciiStrToLower(key);
50+
bool HttpHeader::IsSameKey(HttpHeaderName const& name) const {
51+
return name_ == name.name();
5052
}
5153

5254
bool HttpHeader::IsSameKey(HttpHeader const& other) const {
53-
return IsSameKey(other.key_);
55+
return name_ == other.name_;
5456
}
5557

5658
HttpHeader::operator std::string() const {
57-
if (key_.empty()) return {};
58-
if (values_.empty()) return absl::StrCat(key_, ":");
59-
return absl::StrCat(key_, ": ", absl::StrJoin(values_, ","));
59+
if (name_.empty()) return {};
60+
if (values_.empty()) return absl::StrCat(name_.name(), ":");
61+
return absl::StrCat(name_.name(), ": ", absl::StrJoin(values_, ","));
6062
}
6163

6264
std::string HttpHeader::DebugString() const {
63-
if (key_.empty()) return {};
64-
if (values_.empty()) return absl::StrCat(key_, ":");
65+
if (name_.empty()) return {};
66+
if (values_.empty()) return absl::StrCat(name_.name(), ":");
6567
return absl::StrCat(
66-
key_, ": ",
68+
name_.name(), ": ",
6769
absl::StrJoin(values_, ",", [](std::string* out, std::string const& v) {
6870
absl::StrAppend(out, v.substr(0, 10));
6971
}));

0 commit comments

Comments
 (0)