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: Add support for grpc only gapic #707

Merged
merged 6 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"Testing\\BasicBidiStreaming\\": "tests/Unit/ProtoTests/BasicBidiStreaming/out/src",
"Testing\\BasicClientStreaming\\": "tests/Unit/ProtoTests/BasicClientStreaming/out/src",
"Testing\\BasicDiregapic\\": "tests/Unit/ProtoTests/BasicDiregapic/out/src",
"Testing\\BasicGrpcOnly\\": "tests/Unit/ProtoTests/BasicGrpcOnly/out/src",
"Testing\\BasicLro\\": "tests/Unit/ProtoTests/BasicLro/out/src",
"Testing\\BasicOneof\\": "tests/Unit/ProtoTests/BasicOneof/out/src",
"Testing\\BasicOneofNew\\": "tests/Unit/ProtoTests/BasicOneofNew/out/src",
Expand Down
6 changes: 2 additions & 4 deletions rules_php_gapic/php_gapic.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,8 @@ def php_gapic_srcjar(
# Transport.
if transport == None:
transport = "grpc+rest"
if transport == "grpc":
fail("Error: gRPC-only PHP GAPIC libraries are not yet supported")
if transport != "grpc+rest" and transport != "rest":
fail("Error: Only 'grpc+rest' or 'rest' transports are supported")
if transport != "grpc+rest" and transport != "rest" and transport != "grpc":
fail("Error: Only 'grpc+rest', 'rest' or `grpc` transports are supported")

# Set plugin arguments.
plugin_args = ["metadata"] # Generate the gapic_metadata.json file.
Expand Down
2 changes: 1 addition & 1 deletion src/Generation/GapicClientGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ private function getClientDefaults(): PhpClassMember

// TODO: Consolidate setting all the known array values together.
// We do this here to maintain the existing sensible ordering.
if ($this->serviceDetails->transportType === Transport::GRPC_REST) {
if ($this->serviceDetails->transportType !== Transport::REST) {
Hectorhammett marked this conversation as resolved.
Show resolved Hide resolved
$clientDefaultValues['gcpApiConfigPath'] =
AST::concat(AST::__DIR__, "/../resources/{$this->serviceDetails->grpcConfigFilename}");
}
Expand Down
75 changes: 50 additions & 25 deletions src/Generation/GapicClientV2Generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -478,15 +478,23 @@ private function defaultTransport()

private function supportedTransports()
{
if ($this->serviceDetails->transportType !== Transport::REST) {
Hectorhammett marked this conversation as resolved.
Show resolved Hide resolved
return null;
if ($this->serviceDetails->transportType === Transport::REST) {
return AST::method('supportedTransports')
->withPhpDocText('Implements ClientOptionsTrait::supportedTransports.')
->withAccess(Access::PRIVATE, Access::STATIC)
->withBody(AST::block(
AST::return(AST::array(['rest']))
));
}

if ($this->serviceDetails->transportType === Transport::GRPC) {
return AST::method('supportedTransports')
->withPhpDocText('Implements ClientOptionsTrait::supportedTransports.')
->withAccess(Access::PRIVATE, Access::STATIC)
->withBody(AST::block(
AST::return(AST::array(['grpc', 'grpc-fallback']))
));
}
return AST::method('supportedTransports')
->withPhpDocText('Implements GapicClientTrait::supportedTransports.')
->withAccess(Access::PRIVATE, Access::STATIC)
->withBody(AST::block(
AST::return(AST::array(['rest']))
));
}

private function construct(): PhpClassMember
Expand All @@ -498,30 +506,30 @@ private function construct(): PhpClassMember
$options = AST::var('options');
$optionsParam = AST::param(ResolvedType::array(), $options, AST::array([]));
$clientOptions = AST::var('clientOptions');
$transportType = $this->serviceDetails->transportType;

// Assumes there are only two transport types.
$isGrpcRest = $this->serviceDetails->transportType === Transport::GRPC_REST;

$restTransportDocText = 'At the moment, supports only `rest`.';
$grpcTransportDocText = 'May be either the string `rest` or `grpc`. Defaults to `grpc` if gRPC support is detected on the system.';
$transportDocText =
PhpDoc::text(
'The transport used for executing network requests. ',
$isGrpcRest ? $grpcTransportDocText : $restTransportDocText,
$this->transportDocText($transportType),
'*Advanced usage*: Additionally, it is possible to pass in an already instantiated',
// TODO(vNext): Don't use a fully-qualified type here.
$ctx->type(Type::fromName(TransportInterface::class), true),
'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()
];

$transportConfigSampleValues = [];
if ($isGrpcRest) {
$transportConfigSampleValues['grpc'] = AST::arrayEllipsis();
if (Transport::isGrpcOnly($transportType)) {
unset($transportConfigSampleValues['rest']);
} elseif (Transport::isRestOnly($transportType)) {
unset($transportConfigSampleValues['grpc']);
}
// Set this value here, don't initialize it, so we can maintain alphabetical order
// for the resulting printed doc.
$transportConfigSampleValues['rest'] = AST::arrayEllipsis();

$transportConfigDocText =
PhpDoc::text(
'Configuration options that will be used to construct the transport. Options for',
Expand All @@ -539,22 +547,26 @@ private function construct(): PhpClassMember
'See the',
AST::call(
$ctx->type(
Type::fromName($isGrpcRest ? GrpcTransport::class : RestTransport::class),
Type::fromName(
Transport::isRestOnly($transportType) ?
RestTransport::class :
GrpcTransport::class
),
true
),
AST::method('build')
)(),
$isGrpcRest ? 'and' : '',
$isGrpcRest
? AST::call(
Transport::isGRPCRest($transportType) ? 'and' : '',
Transport::isGRPCRest($transportType) ?
AST::call(
$ctx->type(
Type::fromName(RestTransport::class),
true
),
AST::method('build')
)()
: '',
$isGrpcRest ? 'methods ' : 'method ',
Transport::isGRPCRest($transportType) ? 'methods ' : 'method ',
'for the supported options.'
);
return AST::method('__construct')
Expand Down Expand Up @@ -661,6 +673,19 @@ private function construct(): PhpClassMember
));
}

private function transportDocText(int $transportType): string
{
if (Transport::isRestOnly($transportType)) {
return 'At the moment, supports only `rest`.';
}

if (Transport::isGrpcOnly($transportType)) {
return 'At the moment, supports only `grpc`.';
}

return 'May be either the string `rest` or `grpc`. Defaults to `grpc` if gRPC support is detected on the system.';;
bshaffer marked this conversation as resolved.
Show resolved Hide resolved
}

private function rpcMethod(MethodDetails $method): PhpClassMember
{
$request = AST::var('request');
Expand Down
23 changes: 22 additions & 1 deletion src/Utils/Transport.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class Transport
// supported in the future (e.g. gRPC only).
public const GRPC_REST = 1;
public const REST = 2;
public const GRPC = 3;

/**
* Returns true if the given transport string indicates that grpc+rest transports
Expand All @@ -38,9 +39,29 @@ public static function parseTransport(?string $transport): int
return static::REST;
}
if ($transport === "grpc") {
throw new \Exception("gRPC-only PHP clients are not supported at this time");
return static::GRPC;
}

throw new \Exception("Transport $transport not supported");
}

public static function isRestOnly(int $transport): bool
{
return Transport::compareTransports(Transport::REST, $transport);
}

public static function isGrpcOnly(int $transport): bool
{
return Transport::compareTransports(Transport::GRPC, $transport);
}

public static function isGRPCRest(int $transport): bool
{
return Transport::compareTransports(Transport::GRPC_REST, $transport);
}

private static function compareTransports(int $transportA, int $transportB): bool
{
return $transportA === $transportB;
}
bshaffer marked this conversation as resolved.
Show resolved Hide resolved
}
1 change: 1 addition & 0 deletions tests/Integration/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ php_gapic_library(
grpc_service_config = "@com_google_googleapis//google/cloud/redis/v1:redis_grpc_service_config.json",
service_yaml = "@com_google_googleapis//google/cloud/redis/v1:redis_v1.yaml",
migration_mode = "MIGRATION_MODE_UNSPECIFIED",
transport = "grpc",
deps = [
":redis_php_grpc",
":redis_php_proto",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ private static function getClientDefaults()
'apiEndpoint' => self::SERVICE_ADDRESS . ':' . self::DEFAULT_SERVICE_PORT,
'clientConfig' => __DIR__ . '/../resources/cloud_redis_client_config.json',
'descriptorsConfigPath' => __DIR__ . '/../resources/cloud_redis_descriptor_config.php',
'gcpApiConfigPath' => __DIR__ . '/../resources/cloud_redis_grpc_config.json',
Hectorhammett marked this conversation as resolved.
Show resolved Hide resolved
'credentialsConfig' => [
'defaultScopes' => self::$serviceScopes,
],
Expand All @@ -143,6 +142,15 @@ private static function getClientDefaults()
];
}

/** Implements ClientOptionsTrait::supportedTransports. */
private static function supportedTransports()
{
return [
'grpc',
'grpc-fallback',
];
}

/**
* Return an OperationsClient object with the same endpoint as $this.
*
Expand Down Expand Up @@ -263,9 +271,8 @@ public static function parseName(string $formattedName, string $template = null)
* default this settings points to the default client config file, which is
* provided in the resources folder.
* @type string|TransportInterface $transport
* The transport used for executing network requests. May be either the string
* `rest` or `grpc`. Defaults to `grpc` if gRPC support is detected on the system.
* *Advanced usage*: Additionally, it is possible to pass in an already
* The transport used for executing network requests. At the moment, supports only
* `grpc`. *Advanced usage*: Additionally, it is possible to pass in an already
* instantiated {@see \Google\ApiCore\Transport\TransportInterface} object. Note
* that when this object is provided, any settings in $transportConfig, and any
* $apiEndpoint setting, will be ignored.
Expand All @@ -275,10 +282,8 @@ public static function parseName(string $formattedName, string $template = null)
* example:
* $transportConfig = [
* 'grpc' => [...],
* 'rest' => [...],
* ];
* See the {@see \Google\ApiCore\Transport\GrpcTransport::build()} and
* {@see \Google\ApiCore\Transport\RestTransport::build()} methods for the
* See the {@see \Google\ApiCore\Transport\GrpcTransport::build()} method for the
* supported options.
* @type callable $clientCertSource
* A callable which returns the client cert as a string. This can be used to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,8 @@ public function resumeOperation($operationName, $methodName = null)
* default this settings points to the default client config file, which is
* provided in the resources folder.
* @type string|TransportInterface $transport
* The transport used for executing network requests. May be either the string
* `rest` or `grpc`. Defaults to `grpc` if gRPC support is detected on the system.
* *Advanced usage*: Additionally, it is possible to pass in an already
* The transport used for executing network requests. At the moment, supports only
* `rest`. *Advanced usage*: Additionally, it is possible to pass in an already
Hectorhammett marked this conversation as resolved.
Show resolved Hide resolved
* instantiated {@see \Google\ApiCore\Transport\TransportInterface} object. Note
* that when this object is provided, any settings in $transportConfig, and any
* $apiEndpoint setting, will be ignored.
Expand All @@ -358,11 +357,9 @@ public function resumeOperation($operationName, $methodName = null)
* each supported transport type should be passed in a key for that transport. For
* example:
* $transportConfig = [
* 'grpc' => [...],
* 'rest' => [...],
* ];
* See the {@see \Google\ApiCore\Transport\GrpcTransport::build()} and
* {@see \Google\ApiCore\Transport\RestTransport::build()} methods for the
* See the {@see \Google\ApiCore\Transport\RestTransport::build()} method for the
* supported options.
* @type callable $clientCertSource
* A callable which returns the client cert as a string. This can be used to
Expand Down
14 changes: 14 additions & 0 deletions tests/Unit/ProtoTests/BasicGrpcOnly/basic-grpc-only.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
syntax = "proto3";

package testing.basicgrpconly;

// php_namespace option not included; to test generating namespace from proto package.

import "google/api/annotations.proto";
import "google/api/client.proto";
import "google/api/field_behavior.proto";

// This is a basicGrpcOnly service.
service BasicGrpcOnly {
option (google.api.default_host) = "basicGrpcOnly.example.com";
bshaffer marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: google.api.Service
config_version: 3

authentication:
rules:
- selector: 'testing.basicGrpcOnly.BasicGrpcOnly.*'