From 908d92f3ef8c0f26cffc65e8b5eb9987eba813a8 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 29 May 2024 17:49:29 -0700 Subject: [PATCH] feat: new surface LROs for new surface clients (#714) --- composer.json | 2 +- src/Generation/GapicClientV2Generator.php | 53 ++++++++++++++++--- src/Generation/ResourcesGenerator.php | 4 +- src/Generation/ServiceDetails.php | 4 +- src/Generation/UnitTestsV2Generator.php | 14 +++-- .../V1/Client/CloudFunctionsServiceClient.php | 21 +++++++- .../CloudFunctionsServiceClientTest.php | 2 +- .../redis/src/V1/Client/CloudRedisClient.php | 21 +++++++- .../Unit/V1/Client/CloudRedisClientTest.php | 2 +- 9 files changed, 103 insertions(+), 20 deletions(-) diff --git a/composer.json b/composer.json index d93f2662..ad33f94b 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,7 @@ }, "require-dev": { "phpunit/phpunit": "^9.5", - "google/gax": "^1.19.1" + "google/gax": "^1.34" }, "scripts": { "update-all-tests": [ diff --git a/src/Generation/GapicClientV2Generator.php b/src/Generation/GapicClientV2Generator.php index 55593269..718405d6 100644 --- a/src/Generation/GapicClientV2Generator.php +++ b/src/Generation/GapicClientV2Generator.php @@ -20,7 +20,7 @@ use Google\ApiCore\ApiException; use Google\ApiCore\CredentialsWrapper; -use Google\ApiCore\LongRunning\OperationsClient; +use Google\ApiCore\LongRunning\OperationsClient as LegacyOperationsClient; use Google\ApiCore\OperationResponse; use Google\ApiCore\PagedListResponse; use Google\ApiCore\RequestParamsHeaderDescriptor; @@ -44,6 +44,7 @@ use Google\Generator\Utils\ResolvedType; use Google\Generator\Utils\Transport; use Google\Generator\Utils\Type; +use Google\LongRunning\Client\OperationsClient; use GuzzleHttp\Promise\PromiseInterface; class GapicClientV2Generator @@ -285,10 +286,12 @@ private function operationMethods(): Vector if (!$this->serviceDetails->hasLro && !$this->serviceDetails->hasCustomOp) { return Vector::new([]); } - - $ctype = $this->serviceDetails->hasCustomOp ? - $this->serviceDetails->customOperationServiceClientType : - Type::fromName(OperationsClient::class); + $ctype = $this->serviceDetails->hasCustomOp + ? $this->serviceDetails->customOperationServiceClientType + : Type::fromName($this->serviceDetails->migrationMode === MigrationMode::NEW_SURFACE_ONLY + ? OperationsClient::class + : LegacyOperationsClient::class + ); $methods = Vector::new([]); // getOperationsClient returns the operation client instance. @@ -368,6 +371,40 @@ private function operationMethods(): Vector )); $methods = $methods->append($resumeOperation); + if ($this->serviceDetails->migrationMode === MigrationMode::NEW_SURFACE_ONLY) { + // write createOperationsClient method for new surface clients + $operationsClientType = $this->serviceDetails->hasCustomOp + ? $this->ctx->type($this->serviceDetails->customOperationServiceClientType) + : $this->ctx->type(Type::fromName(OperationsClient::class)); + $createOperationsClient = AST::method('createOperationsClient') + ->withAccess(Access::PRIVATE) + ->withParams(AST::param(ResolvedType::array(), $options)) + ->withBody(AST::block( + '// Unset client-specific configuration options', + AST::call(AST::method('unset'))( + AST::index($options, 'serviceName'), + AST::index($options, 'clientConfig'), + AST::index($options, 'descriptorsConfigPath'), + ), + PHP_EOL, + AST::if(AST::call(AST::method('isset'))(AST::index($options, 'operationsClient')) + )->then(AST::return(AST::index($options, 'operationsClient'))), + PHP_EOL, + AST::return(AST::new($operationsClientType)($options)) + )) + ->withPhpDoc(PhpDoc::block( + PhpDoc::text( + 'Create the default operation client for the service.', + ), + PhpDoc::param( + AST::param(ResolvedType::array(), $options), + PhpDoc::text('ClientOptions for the client.') + ), + PhpDoc::return($operationsClientType) + )); + $methods = $methods->append($createOperationsClient); + } + return $methods; } @@ -459,7 +496,7 @@ private function getClientDefaults(): PhpClassMember $credentialsConfig['useJwtAccessWithScope'] = false; } $clientDefaultValues['credentialsConfig'] = AST::array($credentialsConfig); - + if ($this->serviceDetails->transportType !== Transport::GRPC) { $clientDefaultValues['transportConfig'] = AST::array([ 'rest' => AST::array([ @@ -467,7 +504,7 @@ private function getClientDefaults(): PhpClassMember ]) ]); } - + if ($this->serviceDetails->hasCustomOp) { $clientDefaultValues['operationsClientClass'] = AST::access( $this->ctx->type($this->serviceDetails->customOperationServiceClientType), @@ -537,7 +574,7 @@ private function construct(): PhpClassMember 'object. Note that when this object is provided, any settings in $transportConfig, and any $apiEndpoint', 'setting, will be ignored.' ); - + $transportConfigSampleValues = [ 'grpc' => AST::arrayEllipsis(), 'rest' => AST::arrayEllipsis() diff --git a/src/Generation/ResourcesGenerator.php b/src/Generation/ResourcesGenerator.php index cf8aa11b..8286cde2 100644 --- a/src/Generation/ResourcesGenerator.php +++ b/src/Generation/ResourcesGenerator.php @@ -186,7 +186,7 @@ public static function generateDescriptorConfig(ServiceDetails $serviceDetails, ->withApacheLicense($currentYear) ->withGeneratedCodeWarning() ->withBlock($codeBlock) - ->toCode() . ";"; + ->toCode() . ";"; } public static function customOperationDescriptor(ServiceDetails $serviceDetails, MethodDetails $method) @@ -287,7 +287,7 @@ public static function generateRestConfig(ServiceDetails $serviceDetails, Servic ); $currentYear = (int)date("Y"); - + return AST::file(null) ->withApacheLicense($currentYear) ->withGeneratedCodeWarning() diff --git a/src/Generation/ServiceDetails.php b/src/Generation/ServiceDetails.php index 9dd1e538..23d26c68 100644 --- a/src/Generation/ServiceDetails.php +++ b/src/Generation/ServiceDetails.php @@ -215,7 +215,9 @@ public function __construct( // Assuming the custom operations service client is in the same namespace as the client to generate. $cname = $this->customOperationService->getName() . 'Client'; - $this->customOperationServiceClientType = Type::fromName("{$this->namespace}\\{$cname}"); + $this->customOperationServiceClientType = $this->migrationMode === MigrationMode::NEW_SURFACE_ONLY + ? Type::fromName("{$this->namespace}\\Client\\{$cname}") + : Type::fromName("{$this->namespace}\\{$cname}"); } if ($desc->hasOptions() && $desc->getOptions()->hasDeprecated()) { $this->isDeprecated = $desc->getOptions()->getDeprecated(); diff --git a/src/Generation/UnitTestsV2Generator.php b/src/Generation/UnitTestsV2Generator.php index caa1b44d..5f96f74e 100644 --- a/src/Generation/UnitTestsV2Generator.php +++ b/src/Generation/UnitTestsV2Generator.php @@ -22,7 +22,7 @@ use Google\ApiCore\BidiStream; use Google\ApiCore\CredentialsWrapper; use Google\ApiCore\ServerStream; -use Google\ApiCore\LongRunning\OperationsClient; +use Google\ApiCore\LongRunning\OperationsClient as LegacyOperationsClient; use Google\ApiCore\Testing\GeneratedTest; use Google\ApiCore\Testing\MockTransport; use Google\ApiCore\Transport\TransportInterface; @@ -37,7 +37,9 @@ use Google\Generator\Collections\Vector; use Google\Generator\Utils\Helpers; use Google\Generator\Utils\ProtoHelpers; +use Google\Generator\Utils\MigrationMode; use Google\Generator\Utils\Type; +use Google\LongRunning\Client\OperationsClient; use Google\LongRunning\GetOperationRequest; use Google\LongRunning\Operation; use Google\Protobuf\Any; @@ -63,6 +65,7 @@ public static function generate(SourceFileContext $ctx, ServiceDetails $serviceD private $assertInstanceOf; private $assertArrayHasKey; private $fail; + private $operationsClientClass; private function __construct(SourceFileContext $ctx, ServiceDetails $serviceDetails) { @@ -77,13 +80,16 @@ private function __construct(SourceFileContext $ctx, ServiceDetails $serviceDeta $this->assertInstanceOf = AST::call(AST::THIS, AST::method('assertInstanceOf')); $this->assertArrayHasKey = AST::call(AST::THIS, AST::method('assertArrayHasKey')); $this->fail = AST::call(AST::THIS, AST::method('fail')); + $this->operationsClientClass = $this->serviceDetails->migrationMode === MigrationMode::NEW_SURFACE_ONLY + ? OperationsClient::class + : LegacyOperationsClient::class; } private function generateImpl(): PhpFile { // TODO(vNext): Remove the forced addition of these `use` clauses. $this->ctx->type(Type::fromName(BidiStream::class)); - $this->ctx->type(Type::fromName(\Google\ApiCore\LongRunning\OperationsClient::class)); + $this->ctx->type(Type::fromName($this->operationsClientClass)); $this->ctx->type(Type::fromName(ServerStream::class)); $this->ctx->type(Type::fromName(GetOperationRequest::class)); $this->ctx->type(Type::fromName(Any::class)); @@ -482,13 +488,13 @@ private function lroTestInit($testName) $incompleteOperation = AST::var('incompleteOperation'); $initCode = Vector::new([ AST::assign($operationsTransport, AST::call(AST::THIS, $this->createTransport())()), - AST::assign($operationsClient, AST::new($this->ctx->type(Type::fromName(OperationsClient::class)))(AST::array([ + AST::assign($operationsClient, AST::new($this->ctx->type(Type::fromName($this->operationsClientClass)))(AST::array([ 'apiEndpoint' => '', 'transport' => $operationsTransport, 'credentials' => AST::call(AST::THIS, $this->createCredentials())(), ]))), AST::assign($transport, AST::call(AST::THIS, $this->createTransport())()), - AST::assign($client, AST::call(AST::THIS, $this->createClient())(AST::array([ + AST::assign($client, AST::call(AST::THIS, $this->createClient($this->operationsClientClass))(AST::array([ 'transport' => $transport, 'operationsClient' => $operationsClient, ]))), diff --git a/tests/Integration/goldens/functions/src/V1/Client/CloudFunctionsServiceClient.php b/tests/Integration/goldens/functions/src/V1/Client/CloudFunctionsServiceClient.php index 50865ef5..e5c9475f 100644 --- a/tests/Integration/goldens/functions/src/V1/Client/CloudFunctionsServiceClient.php +++ b/tests/Integration/goldens/functions/src/V1/Client/CloudFunctionsServiceClient.php @@ -27,7 +27,6 @@ use Google\ApiCore\ApiException; use Google\ApiCore\CredentialsWrapper; use Google\ApiCore\GapicClientTrait; -use Google\ApiCore\LongRunning\OperationsClient; use Google\ApiCore\OperationResponse; use Google\ApiCore\PagedListResponse; use Google\ApiCore\ResourceHelperTrait; @@ -52,6 +51,7 @@ use Google\Cloud\Iam\V1\SetIamPolicyRequest; use Google\Cloud\Iam\V1\TestIamPermissionsRequest; use Google\Cloud\Iam\V1\TestIamPermissionsResponse; +use Google\LongRunning\Client\OperationsClient; use Google\LongRunning\Operation; use GuzzleHttp\Promise\PromiseInterface; @@ -157,6 +157,25 @@ public function resumeOperation($operationName, $methodName = null) return $operation; } + /** + * Create the default operation client for the service. + * + * @param array $options ClientOptions for the client. + * + * @return OperationsClient + */ + private function createOperationsClient(array $options) + { + // Unset client-specific configuration options + unset($options['serviceName'], $options['clientConfig'], $options['descriptorsConfigPath']); + + if (isset($options['operationsClient'])) { + return $options['operationsClient']; + } + + return new OperationsClient($options); + } + /** * Formats a string containing the fully-qualified path to represent a * cloud_function resource. diff --git a/tests/Integration/goldens/functions/tests/Unit/V1/Client/CloudFunctionsServiceClientTest.php b/tests/Integration/goldens/functions/tests/Unit/V1/Client/CloudFunctionsServiceClientTest.php index 089257b4..17d71704 100644 --- a/tests/Integration/goldens/functions/tests/Unit/V1/Client/CloudFunctionsServiceClientTest.php +++ b/tests/Integration/goldens/functions/tests/Unit/V1/Client/CloudFunctionsServiceClientTest.php @@ -24,7 +24,6 @@ use Google\ApiCore\ApiException; use Google\ApiCore\CredentialsWrapper; -use Google\ApiCore\LongRunning\OperationsClient; use Google\ApiCore\Testing\GeneratedTest; use Google\ApiCore\Testing\MockTransport; use Google\Cloud\Functions\V1\CallFunctionRequest; @@ -46,6 +45,7 @@ use Google\Cloud\Iam\V1\SetIamPolicyRequest; use Google\Cloud\Iam\V1\TestIamPermissionsRequest; use Google\Cloud\Iam\V1\TestIamPermissionsResponse; +use Google\LongRunning\Client\OperationsClient; use Google\LongRunning\GetOperationRequest; use Google\LongRunning\Operation; use Google\Protobuf\Any; diff --git a/tests/Integration/goldens/redis/src/V1/Client/CloudRedisClient.php b/tests/Integration/goldens/redis/src/V1/Client/CloudRedisClient.php index f1c47626..018cde32 100644 --- a/tests/Integration/goldens/redis/src/V1/Client/CloudRedisClient.php +++ b/tests/Integration/goldens/redis/src/V1/Client/CloudRedisClient.php @@ -27,7 +27,6 @@ use Google\ApiCore\ApiException; use Google\ApiCore\CredentialsWrapper; use Google\ApiCore\GapicClientTrait; -use Google\ApiCore\LongRunning\OperationsClient; use Google\ApiCore\OperationResponse; use Google\ApiCore\PagedListResponse; use Google\ApiCore\ResourceHelperTrait; @@ -51,6 +50,7 @@ use Google\Cloud\Redis\V1\RescheduleMaintenanceRequest; use Google\Cloud\Redis\V1\UpdateInstanceRequest; use Google\Cloud\Redis\V1\UpgradeInstanceRequest; +use Google\LongRunning\Client\OperationsClient; use Google\LongRunning\Operation; use GuzzleHttp\Promise\PromiseInterface; @@ -176,6 +176,25 @@ public function resumeOperation($operationName, $methodName = null) return $operation; } + /** + * Create the default operation client for the service. + * + * @param array $options ClientOptions for the client. + * + * @return OperationsClient + */ + private function createOperationsClient(array $options) + { + // Unset client-specific configuration options + unset($options['serviceName'], $options['clientConfig'], $options['descriptorsConfigPath']); + + if (isset($options['operationsClient'])) { + return $options['operationsClient']; + } + + return new OperationsClient($options); + } + /** * Formats a string containing the fully-qualified path to represent a instance * resource. diff --git a/tests/Integration/goldens/redis/tests/Unit/V1/Client/CloudRedisClientTest.php b/tests/Integration/goldens/redis/tests/Unit/V1/Client/CloudRedisClientTest.php index be089e34..7f0c89d1 100644 --- a/tests/Integration/goldens/redis/tests/Unit/V1/Client/CloudRedisClientTest.php +++ b/tests/Integration/goldens/redis/tests/Unit/V1/Client/CloudRedisClientTest.php @@ -24,7 +24,6 @@ use Google\ApiCore\ApiException; use Google\ApiCore\CredentialsWrapper; -use Google\ApiCore\LongRunning\OperationsClient; use Google\ApiCore\Testing\GeneratedTest; use Google\ApiCore\Testing\MockTransport; use Google\Cloud\Location\GetLocationRequest; @@ -50,6 +49,7 @@ use Google\Cloud\Redis\V1\RescheduleMaintenanceRequest\RescheduleType; use Google\Cloud\Redis\V1\UpdateInstanceRequest; use Google\Cloud\Redis\V1\UpgradeInstanceRequest; +use Google\LongRunning\Client\OperationsClient; use Google\LongRunning\GetOperationRequest; use Google\LongRunning\Operation; use Google\Protobuf\Any;