Skip to content

Commit

Permalink
feat grpc: resolve a dependency cycle in server middlewares
Browse files Browse the repository at this point in the history
`ugrpc::server::ServerComponent` previously called `FindComponent` for all the specified `service-defaults.middlewares`. It could lead to a dependency cycle:

```
grpc-server -> middleware -> something -> grpc-server
```

This PR breaks the `grpc-server -> middleware` dependency. The new components dependency structure looks like this:

```
service-component -> grpc-server
       v
    middleware -> something -> grpc-server
```

Fixes: userver-framework#432
Closes: userver-framework#435
  • Loading branch information
Anton3 committed Nov 16, 2023
1 parent 1a77eb9 commit 991cf89
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 19 deletions.
42 changes: 27 additions & 15 deletions grpc/src/ugrpc/server/impl/parse_config.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#include <ugrpc/server/impl/parse_config.hpp>

#include <boost/range/adaptor/transformed.hpp>

#include <userver/logging/component.hpp>
#include <userver/logging/level_serialization.hpp>
#include <userver/logging/null_logger.hpp>
#include <userver/utils/algo.hpp>

#include <userver/ugrpc/server/middlewares/base.hpp>

Expand Down Expand Up @@ -44,16 +47,19 @@ engine::TaskProcessor& ParseTaskProcessor(
return context.GetTaskProcessor(field.As<std::string>());
}

Middlewares ParseMiddlewares(const yaml_config::YamlConfig& field,
const components::ComponentContext& context) {
Middlewares middlewares;
middlewares.reserve(field.GetSize());
for (const auto& name_yaml : field) {
const auto name = name_yaml.As<std::string>();
auto& component = context.FindComponent<MiddlewareComponentBase>(name);
middlewares.push_back(component.GetMiddleware());
}
return middlewares;
std::vector<std::string> ParseMiddlewares(
const yaml_config::YamlConfig& field,
const components::ComponentContext& /*context*/) {
return field.As<std::vector<std::string>>();
}

Middlewares FindMiddlewares(const std::vector<std::string>& names,
const components::ComponentContext& context) {
return utils::AsContainer<Middlewares>(
names | boost::adaptors::transformed([&](const std::string& name) {
return context.FindComponent<MiddlewareComponentBase>(name)
.GetMiddleware();
}));
}

} // namespace
Expand All @@ -62,7 +68,9 @@ ServiceDefaults ParseServiceDefaults(
const yaml_config::YamlConfig& value,
const components::ComponentContext& context) {
return ServiceDefaults{
ParseOptional(value[kTaskProcessorKey], context, ParseTaskProcessor),
/*task_processor=*/ParseOptional(value[kTaskProcessorKey], context,
ParseTaskProcessor),
/*middleware_names=*/
ParseOptional(value[kMiddlewaresKey], context, ParseMiddlewares),
};
}
Expand All @@ -72,10 +80,14 @@ server::ServiceConfig ParseServiceConfig(
const components::ComponentContext& context,
const ServiceDefaults& defaults) {
return server::ServiceConfig{
MergeField(value[kTaskProcessorKey], defaults.task_processor, context,
ParseTaskProcessor),
MergeField(value[kMiddlewaresKey], defaults.middlewares, context,
ParseMiddlewares),
/*task_processor=*/MergeField(value[kTaskProcessorKey],
defaults.task_processor, context,
ParseTaskProcessor),
/*middlewares=*/
FindMiddlewares(
MergeField(value[kMiddlewaresKey], defaults.middleware_names, context,
ParseMiddlewares),
context),
};
}

Expand Down
7 changes: 3 additions & 4 deletions grpc/src/ugrpc/server/impl/service_defaults.hpp
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
#pragma once

#include <optional>
#include <string>
#include <vector>

#include <boost/optional/optional.hpp>

#include <userver/engine/task/task_processor_fwd.hpp>

#include <userver/ugrpc/server/middlewares/fwd.hpp>

USERVER_NAMESPACE_BEGIN

namespace ugrpc::server::impl {

struct ServiceDefaults final {
// using boost::optional to easily generalize to references
boost::optional<engine::TaskProcessor&> task_processor;
boost::optional<Middlewares> middlewares;
boost::optional<std::vector<std::string>> middleware_names;
};

} // namespace ugrpc::server::impl
Expand Down

0 comments on commit 991cf89

Please sign in to comment.