Skip to content

Commit

Permalink
code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
markdroth committed Jun 27, 2024
1 parent fd19770 commit 1f56d20
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/core/client_channel/lb_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ LbMetadata::TestOnlyCopyToVector() const {
void MetadataMutationHandler::Apply(
LoadBalancingPolicy::MetadataMutations& metadata_mutations,
grpc_metadata_batch* metadata) {
for (auto& p : metadata_mutations.additions_) {
for (auto& p : metadata_mutations.metadata_) {
absl::string_view key = p.first;
Slice& value =
grpc_event_engine::experimental::internal::SliceCast<Slice>(p.second);
Expand Down
4 changes: 2 additions & 2 deletions src/core/load_balancing/grpclb/grpclb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ GrpcLb::PickResult GrpcLb::Picker::Pick(PickArgs args) {
// a string and rely on the client_load_reporting filter to know
// how to interpret it.
// NOLINTBEGIN(bugprone-string-constructor)
complete_pick->metadata_mutations.Add(
complete_pick->metadata_mutations.Set(
GrpcLbClientStatsMetadata::key(),
grpc_event_engine::experimental::Slice(grpc_slice_from_static_buffer(
reinterpret_cast<const char*>(client_stats), 0)));
Expand All @@ -791,7 +791,7 @@ GrpcLb::PickResult GrpcLb::Picker::Pick(PickArgs args) {
// may get refreshed between when we return this pick and when the
// initial metadata goes out on the wire.
if (!subchannel_wrapper->lb_token().empty()) {
complete_pick->metadata_mutations.Add(
complete_pick->metadata_mutations.Set(
LbTokenMetadata::key(), subchannel_wrapper->lb_token().Ref());
}
// Unwrap subchannel to pass up to the channel.
Expand Down
12 changes: 6 additions & 6 deletions src/core/load_balancing/lb_policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
/// A list of metadata mutations to be returned along with a PickResult.
class MetadataMutations {
public:
/// Adds a key/value pair. If the key is already present, it will
/// Sets a key/value pair. If the key is already present, it will
/// be replaced with the new value.
void Add(absl::string_view key, absl::string_view value) {
Add(key, grpc_event_engine::experimental::Slice::FromCopiedString(value));
void Set(absl::string_view key, absl::string_view value) {
Set(key, grpc_event_engine::experimental::Slice::FromCopiedString(value));
}
void Add(absl::string_view key,
void Set(absl::string_view key,
grpc_event_engine::experimental::Slice value) {
additions_.push_back({key, std::move(value)});
metadata_.push_back({key, std::move(value)});
}

private:
Expand All @@ -143,7 +143,7 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
// cases with more than 3 additions.
absl::InlinedVector<
std::pair<absl::string_view, grpc_event_engine::experimental::Slice>, 3>
additions_;
metadata_;
};

/// Arguments used when picking a subchannel for a call.
Expand Down
2 changes: 1 addition & 1 deletion src/core/load_balancing/rls/rls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,7 @@ LoadBalancingPolicy::PickResult RlsLb::Cache::Entry::Pick(PickArgs args) {
auto* complete_pick =
absl::get_if<PickResult::Complete>(&pick_result.result);
if (complete_pick != nullptr) {
complete_pick->metadata_mutations.Add(kRlsHeaderKey, header_data_.Ref());
complete_pick->metadata_mutations.Set(kRlsHeaderKey, header_data_.Ref());
}
}
return pick_result;
Expand Down
16 changes: 8 additions & 8 deletions test/core/client_channel/lb_metadata_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,25 @@ namespace grpc_core {
namespace testing {
namespace {

TEST(LbMetadataMutation, AddsUnknownHeader) {
TEST(LbMetadataMutation, SetsUnknownHeader) {
grpc_metadata_batch metadata;
LoadBalancingPolicy::MetadataMutations mutations;
mutations.Add("key", "value");
mutations.Set("key", "value");
MetadataMutationHandler::Apply(mutations, &metadata);
std::string buffer;
EXPECT_EQ(metadata.GetStringValue("key", &buffer), "value");
}

TEST(LbMetadataMutation, AddsTraitHeader) {
TEST(LbMetadataMutation, SetsTraitHeader) {
grpc_metadata_batch metadata;
LoadBalancingPolicy::MetadataMutations mutations;
mutations.Add("user-agent", "value");
mutations.Set("user-agent", "value");
MetadataMutationHandler::Apply(mutations, &metadata);
std::string buffer;
EXPECT_EQ(metadata.GetStringValue("user-agent", &buffer), "value");
}

TEST(LbMetadataMutation, OverwritesExistingKeys) {
TEST(LbMetadataMutation, OverwritesExistingHeader) {
grpc_metadata_batch metadata;
metadata.Append("key", Slice::FromCopiedString("value1"),
[&](absl::string_view error, const Slice& value) {
Expand All @@ -60,12 +60,12 @@ TEST(LbMetadataMutation, OverwritesExistingKeys) {
std::string buffer;
EXPECT_EQ(metadata.GetStringValue("key", &buffer), "value1,value2");
LoadBalancingPolicy::MetadataMutations mutations;
mutations.Add("key", "value3");
mutations.Set("key", "value3");
MetadataMutationHandler::Apply(mutations, &metadata);
EXPECT_EQ(metadata.GetStringValue("key", &buffer), "value3");
}

TEST(LbMetadataMutation, OverwritesTraitKeys) {
TEST(LbMetadataMutation, OverwritesTraitHeader) {
grpc_metadata_batch metadata;
metadata.Append("user-agent", Slice::FromCopiedString("value1"),
[&](absl::string_view error, const Slice& value) {
Expand All @@ -74,7 +74,7 @@ TEST(LbMetadataMutation, OverwritesTraitKeys) {
std::string buffer;
EXPECT_EQ(metadata.GetStringValue("user-agent", &buffer), "value1");
LoadBalancingPolicy::MetadataMutations mutations;
mutations.Add("user-agent", "value2");
mutations.Set("user-agent", "value2");
MetadataMutationHandler::Apply(mutations, &metadata);
EXPECT_EQ(metadata.GetStringValue("user-agent", &buffer), "value2");
}
Expand Down
2 changes: 1 addition & 1 deletion test/cpp/interop/rpc_behavior_lb_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class RpcBehaviorLbPolicy : public LoadBalancingPolicy {
auto* complete_pick =
absl::get_if<PickResult::Complete>(&pick_result.result);
if (complete_pick != nullptr) {
complete_pick->metadata_mutations.Add(kRpcBehaviorMetadataKey,
complete_pick->metadata_mutations.Set(kRpcBehaviorMetadataKey,
rpc_behavior_);
}
// Return result.
Expand Down

0 comments on commit 1f56d20

Please sign in to comment.