Skip to content

Commit

Permalink
feat(generator): merge connection options into client options (#8158)
Browse files Browse the repository at this point in the history
Store the Connection options so that they may be made available to
merge into the Client options, and then use them to implement the
four polling-policy data elements.  (This is the generator version
of #8090.)

Rules of thumb for `Options` values:
- `MergeOptions()` LHS should not include the service defaults
- `MergeOptions()` RHS should include service defaults
- An `OptionsSpan` should include the service defaults
- stored `Options` (data members) should include the service defaults

Code can then normally just look at the `CurrentOptions()`, knowing
that they will include the service defaults.  For example, the policy
options will be available.  A special exception to this, however, is
when testing the connection layer in isolation (without a client), as
no one has created an `OptionsSpan` in that case.  We could add an
`OptionsSpan` to the connection-layer calls, but it would be a no-op
in practice.  So, tests need to ensure required options are available.

Fixes #8054.
  • Loading branch information
devbww authored Jan 29, 2022
1 parent beb3852 commit 9207865
Show file tree
Hide file tree
Showing 616 changed files with 2,796 additions and 2,955 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace cloud {
namespace golden {
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

GoldenKitchenSinkClient::GoldenKitchenSinkClient(std::shared_ptr<GoldenKitchenSinkConnection> connection, Options options) : connection_(std::move(connection)), options_(golden_internal::GoldenKitchenSinkDefaultOptions(std::move(options))) {}
GoldenKitchenSinkClient::GoldenKitchenSinkClient(std::shared_ptr<GoldenKitchenSinkConnection> connection, Options options) : connection_(std::move(connection)), options_(internal::MergeOptions(std::move(options), golden_internal::GoldenKitchenSinkDefaultOptions(connection_->options()))) {}
GoldenKitchenSinkClient::~GoldenKitchenSinkClient() = default;

StatusOr<google::test::admin::database::v1::GenerateAccessTokenResponse>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ std::shared_ptr<GoldenKitchenSinkConnection> MakeGoldenKitchenSinkConnection(
auto stub = golden_internal::CreateDefaultGoldenKitchenSinkStub(
background->cq(), options);
return std::make_shared<golden_internal::GoldenKitchenSinkConnectionImpl>(
std::move(background), std::move(stub), options);
std::move(background), std::move(stub), std::move(options));
}

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand All @@ -126,9 +126,9 @@ std::shared_ptr<golden::GoldenKitchenSinkConnection>
MakeGoldenKitchenSinkConnection(
std::shared_ptr<GoldenKitchenSinkStub> stub, Options options) {
options = GoldenKitchenSinkDefaultOptions(std::move(options));
auto background = internal::MakeBackgroundThreadsFactory(options)();
return std::make_shared<golden_internal::GoldenKitchenSinkConnectionImpl>(
internal::MakeBackgroundThreadsFactory(options)(),
std::move(stub), std::move(options));
std::move(background), std::move(stub), std::move(options));
}

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class GoldenKitchenSinkConnection {
public:
virtual ~GoldenKitchenSinkConnection() = 0;

virtual Options options() { return Options{}; }

virtual StatusOr<google::test::admin::database::v1::GenerateAccessTokenResponse>
GenerateAccessToken(google::test::admin::database::v1::GenerateAccessTokenRequest const& request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace cloud {
namespace golden {
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

GoldenThingAdminClient::GoldenThingAdminClient(std::shared_ptr<GoldenThingAdminConnection> connection, Options options) : connection_(std::move(connection)), options_(golden_internal::GoldenThingAdminDefaultOptions(std::move(options))) {}
GoldenThingAdminClient::GoldenThingAdminClient(std::shared_ptr<GoldenThingAdminConnection> connection, Options options) : connection_(std::move(connection)), options_(internal::MergeOptions(std::move(options), golden_internal::GoldenThingAdminDefaultOptions(connection_->options()))) {}
GoldenThingAdminClient::~GoldenThingAdminClient() = default;

StreamRange<google::test::admin::database::v1::Database>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ std::shared_ptr<GoldenThingAdminConnection> MakeGoldenThingAdminConnection(
auto stub = golden_internal::CreateDefaultGoldenThingAdminStub(
background->cq(), options);
return std::make_shared<golden_internal::GoldenThingAdminConnectionImpl>(
std::move(background), std::move(stub), options);
std::move(background), std::move(stub), std::move(options));
}

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand All @@ -223,9 +223,9 @@ std::shared_ptr<golden::GoldenThingAdminConnection>
MakeGoldenThingAdminConnection(
std::shared_ptr<GoldenThingAdminStub> stub, Options options) {
options = GoldenThingAdminDefaultOptions(std::move(options));
auto background = internal::MakeBackgroundThreadsFactory(options)();
return std::make_shared<golden_internal::GoldenThingAdminConnectionImpl>(
internal::MakeBackgroundThreadsFactory(options)(),
std::move(stub), std::move(options));
std::move(background), std::move(stub), std::move(options));
}

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class GoldenThingAdminConnection {
public:
virtual ~GoldenThingAdminConnection() = 0;

virtual Options options() { return Options{}; }

virtual StreamRange<google::test::admin::database::v1::Database>
ListDatabases(google::test::admin::database::v1::ListDatabasesRequest request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
GoldenKitchenSinkConnectionImpl::GoldenKitchenSinkConnectionImpl(
std::unique_ptr<google::cloud::BackgroundThreads> background,
std::shared_ptr<golden_internal::GoldenKitchenSinkStub> stub,
Options const& options)
Options options)
: background_(std::move(background)), stub_(std::move(stub)),
retry_policy_prototype_(options.get<golden::GoldenKitchenSinkRetryPolicyOption>()->clone()),
backoff_policy_prototype_(options.get<golden::GoldenKitchenSinkBackoffPolicyOption>()->clone()),
idempotency_policy_(options.get<golden::GoldenKitchenSinkConnectionIdempotencyPolicyOption>()->clone()) {}
options_(internal::MergeOptions(std::move(options),
golden_internal::GoldenKitchenSinkDefaultOptions(
GoldenKitchenSinkConnection::options()))) {}

StatusOr<google::test::admin::database::v1::GenerateAccessTokenResponse>
GoldenKitchenSinkConnectionImpl::GenerateAccessToken(google::test::admin::database::v1::GenerateAccessTokenRequest const& request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ class GoldenKitchenSinkConnectionImpl
GoldenKitchenSinkConnectionImpl(
std::unique_ptr<google::cloud::BackgroundThreads> background,
std::shared_ptr<golden_internal::GoldenKitchenSinkStub> stub,
Options const& options);
Options options);

Options options() override { return options_; }

StatusOr<google::test::admin::database::v1::GenerateAccessTokenResponse>
GenerateAccessToken(google::test::admin::database::v1::GenerateAccessTokenRequest const& request) override;
Expand Down Expand Up @@ -80,30 +82,29 @@ class GoldenKitchenSinkConnectionImpl
if (options.has<golden::GoldenKitchenSinkRetryPolicyOption>()) {
return options.get<golden::GoldenKitchenSinkRetryPolicyOption>()->clone();
}
return retry_policy_prototype_->clone();
return options_.get<golden::GoldenKitchenSinkRetryPolicyOption>()->clone();
}

std::unique_ptr<BackoffPolicy> backoff_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<golden::GoldenKitchenSinkBackoffPolicyOption>()) {
return options.get<golden::GoldenKitchenSinkBackoffPolicyOption>()->clone();
}
return backoff_policy_prototype_->clone();
return options_.get<golden::GoldenKitchenSinkBackoffPolicyOption>()->clone();
}

std::unique_ptr<golden::GoldenKitchenSinkConnectionIdempotencyPolicy> idempotency_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<golden::GoldenKitchenSinkConnectionIdempotencyPolicyOption>()) {
return options.get<golden::GoldenKitchenSinkConnectionIdempotencyPolicyOption>()->clone();
}
return idempotency_policy_->clone();
return options_.get<golden::GoldenKitchenSinkConnectionIdempotencyPolicyOption>()->
clone();
}

std::unique_ptr<google::cloud::BackgroundThreads> background_;
std::shared_ptr<golden_internal::GoldenKitchenSinkStub> stub_;
std::unique_ptr<golden::GoldenKitchenSinkRetryPolicy const> retry_policy_prototype_;
std::unique_ptr<BackoffPolicy const> backoff_policy_prototype_;
std::unique_ptr<golden::GoldenKitchenSinkConnectionIdempotencyPolicy> idempotency_policy_;
Options options_;
};

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,11 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
GoldenThingAdminConnectionImpl::GoldenThingAdminConnectionImpl(
std::unique_ptr<google::cloud::BackgroundThreads> background,
std::shared_ptr<golden_internal::GoldenThingAdminStub> stub,
Options const& options)
Options options)
: background_(std::move(background)), stub_(std::move(stub)),
retry_policy_prototype_(options.get<golden::GoldenThingAdminRetryPolicyOption>()->clone()),
backoff_policy_prototype_(options.get<golden::GoldenThingAdminBackoffPolicyOption>()->clone()),
idempotency_policy_(options.get<golden::GoldenThingAdminConnectionIdempotencyPolicyOption>()->clone()),
polling_policy_prototype_(options.get<golden::GoldenThingAdminPollingPolicyOption>()->clone()) {}
options_(internal::MergeOptions(std::move(options),
golden_internal::GoldenThingAdminDefaultOptions(
GoldenThingAdminConnection::options()))) {}

StreamRange<google::test::admin::database::v1::Database>
GoldenThingAdminConnectionImpl::ListDatabases(google::test::admin::database::v1::ListDatabasesRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ class GoldenThingAdminConnectionImpl
GoldenThingAdminConnectionImpl(
std::unique_ptr<google::cloud::BackgroundThreads> background,
std::shared_ptr<golden_internal::GoldenThingAdminStub> stub,
Options const& options);
Options options);

Options options() override { return options_; }

StreamRange<google::test::admin::database::v1::Database>
ListDatabases(google::test::admin::database::v1::ListDatabasesRequest request) override;
Expand Down Expand Up @@ -116,40 +118,37 @@ class GoldenThingAdminConnectionImpl
if (options.has<golden::GoldenThingAdminRetryPolicyOption>()) {
return options.get<golden::GoldenThingAdminRetryPolicyOption>()->clone();
}
return retry_policy_prototype_->clone();
return options_.get<golden::GoldenThingAdminRetryPolicyOption>()->clone();
}

std::unique_ptr<BackoffPolicy> backoff_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<golden::GoldenThingAdminBackoffPolicyOption>()) {
return options.get<golden::GoldenThingAdminBackoffPolicyOption>()->clone();
}
return backoff_policy_prototype_->clone();
return options_.get<golden::GoldenThingAdminBackoffPolicyOption>()->clone();
}

std::unique_ptr<golden::GoldenThingAdminConnectionIdempotencyPolicy> idempotency_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<golden::GoldenThingAdminConnectionIdempotencyPolicyOption>()) {
return options.get<golden::GoldenThingAdminConnectionIdempotencyPolicyOption>()->clone();
}
return idempotency_policy_->clone();
return options_.get<golden::GoldenThingAdminConnectionIdempotencyPolicyOption>()->
clone();
}

std::unique_ptr<google::cloud::BackgroundThreads> background_;
std::shared_ptr<golden_internal::GoldenThingAdminStub> stub_;
std::unique_ptr<golden::GoldenThingAdminRetryPolicy const> retry_policy_prototype_;
std::unique_ptr<BackoffPolicy const> backoff_policy_prototype_;
std::unique_ptr<golden::GoldenThingAdminConnectionIdempotencyPolicy> idempotency_policy_;

std::unique_ptr<PollingPolicy> polling_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<golden::GoldenThingAdminPollingPolicyOption>()) {
return options.get<golden::GoldenThingAdminPollingPolicyOption>()->clone();
}
return polling_policy_prototype_->clone();
return options_.get<golden::GoldenThingAdminPollingPolicyOption>()->clone();
}

std::unique_ptr<PollingPolicy const> polling_policy_prototype_;
std::unique_ptr<google::cloud::BackgroundThreads> background_;
std::shared_ptr<golden_internal::GoldenThingAdminStub> stub_;
Options options_;
};

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

class MockGoldenKitchenSinkConnection : public golden::GoldenKitchenSinkConnection {
public:
MOCK_METHOD(Options, options, (), (override));

MOCK_METHOD(StatusOr<google::test::admin::database::v1::GenerateAccessTokenResponse>,
GenerateAccessToken,
(google::test::admin::database::v1::GenerateAccessTokenRequest const& request), (override));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

class MockGoldenThingAdminConnection : public golden::GoldenThingAdminConnection {
public:
MOCK_METHOD(Options, options, (), (override));

MOCK_METHOD(StreamRange<google::test::admin::database::v1::Database>,
ListDatabases,
(google::test::admin::database::v1::ListDatabasesRequest request), (override));
Expand Down
3 changes: 2 additions & 1 deletion generator/internal/client_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ Status ClientGenerator::GenerateCc() {
"\n"
"$client_class_name$::$client_class_name$(std::shared_ptr<$connection_class_name$> connection, Options options)"
" : connection_(std::move(connection)),"
" options_($product_internal_namespace$::$service_name$DefaultOptions(std::move(options))) {}\n");
" options_(internal::MergeOptions(std::move(options),"
" $product_internal_namespace$::$service_name$DefaultOptions(connection_->options()))) {}\n");
// clang-format on

CcPrint( // clang-format off
Expand Down
10 changes: 7 additions & 3 deletions generator/internal/connection_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ Status ConnectionGenerator::GenerateHeader() {
" virtual ~$connection_class_name$() = 0;\n");
// clang-format on

HeaderPrint(R"""(
virtual Options options() { return Options{}; }
)""");

for (auto const& method : methods()) {
if (IsBidirStreaming(method)) {
HeaderPrintMethod(method, __FILE__, __LINE__,
Expand Down Expand Up @@ -357,7 +361,7 @@ std::shared_ptr<$connection_class_name$> Make$connection_class_name$(
auto stub = $product_internal_namespace$::CreateDefault$stub_class_name$(
background->cq(), options);
return std::make_shared<$product_internal_namespace$::$connection_class_name$Impl>(
std::move(background), std::move(stub), options);
std::move(background), std::move(stub), std::move(options));
}
)""");

Expand All @@ -371,9 +375,9 @@ std::shared_ptr<$product_namespace$::$connection_class_name$>
Make$connection_class_name$(
std::shared_ptr<$stub_class_name$> stub, Options options) {
options = $service_name$DefaultOptions(std::move(options));
auto background = internal::MakeBackgroundThreadsFactory(options)();
return std::make_shared<$product_internal_namespace$::$connection_class_name$Impl>(
internal::MakeBackgroundThreadsFactory(options)(),
std::move(stub), std::move(options));
std::move(background), std::move(stub), std::move(options));
}
)""");

Expand Down
49 changes: 25 additions & 24 deletions generator/internal/connection_impl_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ class $connection_class_name$Impl
$connection_class_name$Impl(
std::unique_ptr<google::cloud::BackgroundThreads> background,
std::shared_ptr<$product_internal_namespace$::$stub_class_name$> stub,
Options const& options);
Options options);
)""");

HeaderPrint(R"""(
Options options() override { return options_; }
)""");

for (auto const& method : methods()) {
Expand All @@ -90,53 +94,56 @@ class $connection_class_name$Impl
AsyncMethodDeclaration(method));
}

// `CurrentOptions()` may not have the service default options because we
// could be running in a test that calls the ConnectionImpl layer directly,
// and it does not create an `internal::OptionsSpan` like the Client layer.
// So, we have to fallback to `options_`, which we know has the service
// default options because we added them.
HeaderPrint(R"""(
private:
std::unique_ptr<$product_namespace$::$retry_policy_name$> retry_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<$product_namespace$::$retry_policy_name$Option>()) {
return options.get<$product_namespace$::$retry_policy_name$Option>()->clone();
}
return retry_policy_prototype_->clone();
return options_.get<$product_namespace$::$retry_policy_name$Option>()->clone();
}
std::unique_ptr<BackoffPolicy> backoff_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<$product_namespace$::$service_name$BackoffPolicyOption>()) {
return options.get<$product_namespace$::$service_name$BackoffPolicyOption>()->clone();
}
return backoff_policy_prototype_->clone();
return options_.get<$product_namespace$::$service_name$BackoffPolicyOption>()->clone();
}
std::unique_ptr<$product_namespace$::$idempotency_class_name$> idempotency_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<$product_namespace$::$idempotency_class_name$Option>()) {
return options.get<$product_namespace$::$idempotency_class_name$Option>()->clone();
}
return idempotency_policy_->clone();
return options_.get<$product_namespace$::$idempotency_class_name$Option>()->
clone();
}
std::unique_ptr<google::cloud::BackgroundThreads> background_;
std::shared_ptr<$product_internal_namespace$::$stub_class_name$> stub_;
std::unique_ptr<$product_namespace$::$retry_policy_name$ const> retry_policy_prototype_;
std::unique_ptr<BackoffPolicy const> backoff_policy_prototype_;
std::unique_ptr<$product_namespace$::$idempotency_class_name$> idempotency_policy_;
)""");

if (HasLongrunningMethod()) {
HeaderPrint(R"""(
std::unique_ptr<PollingPolicy> polling_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<$product_namespace$::$service_name$PollingPolicyOption>()) {
return options.get<$product_namespace$::$service_name$PollingPolicyOption>()->clone();
}
return polling_policy_prototype_->clone();
return options_.get<$product_namespace$::$service_name$PollingPolicyOption>()->clone();
}
std::unique_ptr<PollingPolicy const> polling_policy_prototype_;
)""");
}

HeaderPrint(R"""(
std::unique_ptr<google::cloud::BackgroundThreads> background_;
std::shared_ptr<$product_internal_namespace$::$stub_class_name$> stub_;
Options options_;
)""");

// This closes the *ConnectionImpl class definition.
HeaderPrint("};\n");

Expand Down Expand Up @@ -181,17 +188,11 @@ Status ConnectionImplGenerator::GenerateCc() {
$connection_class_name$Impl::$connection_class_name$Impl(
std::unique_ptr<google::cloud::BackgroundThreads> background,
std::shared_ptr<$product_internal_namespace$::$stub_class_name$> stub,
Options const& options)
Options options)
: background_(std::move(background)), stub_(std::move(stub)),
retry_policy_prototype_(options.get<$product_namespace$::$retry_policy_name$Option>()->clone()),
backoff_policy_prototype_(options.get<$product_namespace$::$service_name$BackoffPolicyOption>()->clone()),
idempotency_policy_(options.get<$product_namespace$::$idempotency_class_name$Option>()->clone()))""");
// The constructor depends on a couple of things.
if (HasLongrunningMethod()) {
CcPrint(R"""(,
polling_policy_prototype_(options.get<$product_namespace$::$service_name$PollingPolicyOption>()->clone()))""");
}
CcPrint(R"""( {}
options_(internal::MergeOptions(std::move(options),
$product_internal_namespace$::$service_name$DefaultOptions(
$connection_class_name$::options()))) {}
)""");

for (auto const& method : methods()) {
Expand Down
8 changes: 3 additions & 5 deletions generator/internal/mock_connection_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,9 @@ Status MockConnectionGenerator::GenerateHeader() {
HeaderPrint(R"""(
class $mock_connection_class_name$ : public $product_namespace$::$connection_class_name$ {
public:)""");
// Avoid an unsightly blank line at the start of the mock class by not
// printing a newline after the `public:` access qualifier. While almost any
// class we generate will have some member functions, if there are none, we
// need that newline.
if (methods().empty() && async_methods().empty()) HeaderPrint("\n");
HeaderPrint(R"""(
MOCK_METHOD(Options, options, (), (override));
)""");

for (auto const& method : methods()) {
if (IsBidirStreaming(method)) {
Expand Down
Loading

0 comments on commit 9207865

Please sign in to comment.