Skip to content

Commit

Permalink
Cleanup: cleaning up .first/.second [source/common/config] (#12468)
Browse files Browse the repository at this point in the history
This is the second PR to this #12354. This time is in the common/config submodule.

Signed-off-by: Yifan Yang <needyyang@google.com>
  • Loading branch information
aimless404 authored Aug 10, 2020
1 parent d651d2e commit 3cb95f6
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 23 deletions.
8 changes: 4 additions & 4 deletions source/common/config/delta_subscription_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,16 +124,16 @@ DeltaSubscriptionState::getNextRequestAckless() {
// initial_resource_versions "must be populated for first request in a stream".
// Also, since this might be a new server, we must explicitly state *all* of our subscription
// interest.
for (auto const& resource : resource_versions_) {
for (auto const& [resource_name, resource_version] : resource_versions_) {
// Populate initial_resource_versions with the resource versions we currently have.
// Resources we are interested in, but are still waiting to get any version of from the
// server, do not belong in initial_resource_versions. (But do belong in new subscriptions!)
if (!resource.second.waitingForServer()) {
(*request.mutable_initial_resource_versions())[resource.first] = resource.second.version();
if (!resource_version.waitingForServer()) {
(*request.mutable_initial_resource_versions())[resource_name] = resource_version.version();
}
// As mentioned above, fill resource_names_subscribe with everything, including names we
// have yet to receive any resource for.
names_added_.insert(resource.first);
names_added_.insert(resource_name);
}
names_removed_.clear();
}
Expand Down
7 changes: 4 additions & 3 deletions source/common/config/metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,11 @@ template <typename factoryClass> class TypedMetadataImpl : public TypedMetadata
*/
void populateFrom(const envoy::config::core::v3::Metadata& metadata) {
auto& data_by_key = metadata.filter_metadata();
for (const auto& it : Registry::FactoryRegistry<factoryClass>::factories()) {
const auto& meta_iter = data_by_key.find(it.first);
for (const auto& [factory_name, factory] :
Registry::FactoryRegistry<factoryClass>::factories()) {
const auto& meta_iter = data_by_key.find(factory_name);
if (meta_iter != data_by_key.end()) {
data_[it.second->name()] = it.second->parse(meta_iter->second);
data_[factory->name()] = factory->parse(meta_iter->second);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions source/common/config/new_grpc_mux_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ void NewGrpcMuxImpl::onDiscoveryResponse(
}

void NewGrpcMuxImpl::onStreamEstablished() {
for (auto& sub : subscriptions_) {
sub.second->sub_state_.markStreamFresh();
for (auto& [type_url, subscription] : subscriptions_) {
subscription->sub_state_.markStreamFresh();
}
trySendDiscoveryRequests();
}
Expand All @@ -88,8 +88,8 @@ void NewGrpcMuxImpl::onEstablishmentFailure() {
absl::flat_hash_map<std::string, DeltaSubscriptionState*> all_subscribed;
absl::flat_hash_map<std::string, DeltaSubscriptionState*> already_called;
do {
for (auto& sub : subscriptions_) {
all_subscribed[sub.first] = &sub.second->sub_state_;
for (auto& [type_url, subscription] : subscriptions_) {
all_subscribed[type_url] = &subscription->sub_state_;
}
for (auto& sub : all_subscribed) {
if (already_called.insert(sub).second) { // insert succeeded ==> not already called
Expand Down
8 changes: 3 additions & 5 deletions source/common/config/type_to_endpoint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,11 @@ TypeUrlToVersionedServiceMap* buildTypeUrlToServiceMap() {
}},
}},
}) {
for (const auto& registered_service : registered) {
const TypeUrl resource_type_url = getResourceTypeUrl(registered_service.first);
for (const auto& [registered_service_name, registered_service_info] : registered) {
const TypeUrl resource_type_url = getResourceTypeUrl(registered_service_name);
VersionedService& service = (*type_url_to_versioned_service_map)[resource_type_url];

for (const auto& versioned_service_name : registered_service.second.names_) {
const ServiceName& service_name = versioned_service_name.second;
for (const auto& [transport_api_version, service_name] : registered_service_info.names_) {
const auto* service_desc =
Protobuf::DescriptorPool::generated_pool()->FindServiceByName(service_name);
ASSERT(service_desc != nullptr, fmt::format("{} missing", service_name));
Expand All @@ -200,7 +199,6 @@ TypeUrlToVersionedServiceMap* buildTypeUrlToServiceMap() {
// services don't implement all, e.g. VHDS doesn't support SotW or REST.
for (int method_index = 0; method_index < service_desc->method_count(); ++method_index) {
const auto& method_desc = *service_desc->method(method_index);
const auto transport_api_version = versioned_service_name.first;
if (absl::StartsWith(method_desc.name(), "Stream")) {
service.sotw_grpc_.methods_[transport_api_version] = method_desc.full_name();
} else if (absl::StartsWith(method_desc.name(), "Delta")) {
Expand Down
13 changes: 6 additions & 7 deletions source/common/config/watch_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -181,28 +181,27 @@ void WatchMap::onConfigUpdate(
}

// We just bundled up the updates into nice per-watch packages. Now, deliver them.
for (const auto& added : per_watch_added) {
const Watch* cur_watch = added.first;
for (const auto& [cur_watch, resource_to_add] : per_watch_added) {
if (deferred_removed_during_update_->count(cur_watch) > 0) {
continue;
}
const auto removed = per_watch_removed.find(cur_watch);
if (removed == per_watch_removed.end()) {
// additions only, no removals
cur_watch->callbacks_.onConfigUpdate(added.second, {}, system_version_info);
cur_watch->callbacks_.onConfigUpdate(resource_to_add, {}, system_version_info);
} else {
// both additions and removals
cur_watch->callbacks_.onConfigUpdate(added.second, removed->second, system_version_info);
cur_watch->callbacks_.onConfigUpdate(resource_to_add, removed->second, system_version_info);
// Drop the removals now, so the final removals-only pass won't use them.
per_watch_removed.erase(removed);
}
}
// Any removals-only updates will not have been picked up in the per_watch_added loop.
for (auto& removed : per_watch_removed) {
if (deferred_removed_during_update_->count(removed.first) > 0) {
for (auto& [cur_watch, resource_to_remove] : per_watch_removed) {
if (deferred_removed_during_update_->count(cur_watch) > 0) {
continue;
}
removed.first->callbacks_.onConfigUpdate({}, removed.second, system_version_info);
cur_watch->callbacks_.onConfigUpdate({}, resource_to_remove, system_version_info);
}
}

Expand Down

0 comments on commit 3cb95f6

Please sign in to comment.