Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use new surface LROs in new surface clients #714

Merged
merged 17 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
},
"require-dev": {
"phpunit/phpunit": "^9.5",
"google/gax": "^1.19.1"
"google/gax": "dev-support-operations-for-compute-v2 as 1.34"
bshaffer marked this conversation as resolved.
Show resolved Hide resolved
},
"scripts": {
"update-all-tests": [
Expand Down
53 changes: 45 additions & 8 deletions src/Generation/GapicClientV2Generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
noahdietz marked this conversation as resolved.
Show resolved Hide resolved
? OperationsClient::class
: LegacyOperationsClient::class
);
$methods = Vector::new([]);

// getOperationsClient returns the operation client instance.
Expand Down Expand Up @@ -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'),
),
bshaffer marked this conversation as resolved.
Show resolved Hide resolved
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;
}

Expand Down Expand Up @@ -459,15 +496,15 @@ 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([
'restClientConfigPath' => AST::concat(AST::__DIR__, "/../resources/{$this->serviceDetails->restConfigFilename}"),
])
]);
}

if ($this->serviceDetails->hasCustomOp) {
$clientDefaultValues['operationsClientClass'] = AST::access(
$this->ctx->type($this->serviceDetails->customOperationServiceClientType),
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions src/Generation/ResourcesGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -287,7 +287,7 @@ public static function generateRestConfig(ServiceDetails $serviceDetails, Servic
);

$currentYear = (int)date("Y");

return AST::file(null)
->withApacheLicense($currentYear)
->withGeneratedCodeWarning()
Expand Down
4 changes: 3 additions & 1 deletion src/Generation/ServiceDetails.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
16 changes: 12 additions & 4 deletions src/Generation/UnitTestsV2Generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -82,8 +84,11 @@ private function __construct(SourceFileContext $ctx, ServiceDetails $serviceDeta
private function generateImpl(): PhpFile
{
// TODO(vNext): Remove the forced addition of these `use` clauses.
// $operationsClientClass = $this->serviceDetails->migrationMode === MigrationMode::NEW_SURFACE_ONLY
// ? OperationsClient::class
// : LegacyOperationsClient::class;
$this->ctx->type(Type::fromName(BidiStream::class));
$this->ctx->type(Type::fromName(\Google\ApiCore\LongRunning\OperationsClient::class));
// $this->ctx->type(Type::fromName($operationsClientClass));
$this->ctx->type(Type::fromName(ServerStream::class));
$this->ctx->type(Type::fromName(GetOperationRequest::class));
$this->ctx->type(Type::fromName(Any::class));
Expand Down Expand Up @@ -480,15 +485,18 @@ private function lroTestInit($testName)
$transport = AST::var('transport');
$client = AST::var(self::CLIENT_VARIABLE);
$incompleteOperation = AST::var('incompleteOperation');
$operationsClientClass = $this->serviceDetails->migrationMode === MigrationMode::NEW_SURFACE_ONLY
? OperationsClient::class
: LegacyOperationsClient::class;
$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($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($operationsClientClass))(AST::array([
'transport' => $transport,
'operationsClient' => $operationsClient,
]))),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions tests/Unit/autoload.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,5 @@ class_alias(FakeMessage::class, $class);

// Use a custom autoloader to produce fake proto message classes for tests.
spl_autoload_register('protosOnDemandLoader', true);

require_once __DIR__ . '/../../vendor/autoload.php';
bshaffer marked this conversation as resolved.
Show resolved Hide resolved
Loading