Skip to content

Commit

Permalink
fix: remove oneof wrappers in v2 (#629)
Browse files Browse the repository at this point in the history
  • Loading branch information
bshaffer committed May 10, 2023
1 parent cf0abeb commit f69fcfb
Show file tree
Hide file tree
Showing 16 changed files with 696 additions and 33 deletions.
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"Testing\\BasicDiregapic\\": "tests/Unit/ProtoTests/BasicDiregapic/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",
"Testing\\BasicPaginated\\": "tests/Unit/ProtoTests/BasicPaginated/out/src",
"Testing\\BasicServerStreaming\\": "tests/Unit/ProtoTests/BasicServerStreaming/out/src",
"Testing\\CustomLro\\": "tests/Unit/ProtoTests/CustomLro/out/src",
Expand Down
24 changes: 13 additions & 11 deletions src/CodeGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ private static function generateServices(
$code = $file->toCode();
$code = Formatter::format($code);
yield ["src/{$version}{$service->emptyClientType->name}.php", $code];

// Unit tests.
$ctx = new SourceFileContext($service->unitTestsType->getNamespace(), $licenseYear);
$file = UnitTestsGenerator::generate($ctx, $service);
Expand Down Expand Up @@ -363,7 +363,7 @@ private static function generateServices(
$code = $file->toCode();
$code = Formatter::format($code);
yield ["tests/Unit/{$version}Client/{$service->unitTestsV2Type->name}.php", $code];


// Resource: build_method.txt
$ctx = new SourceFileContext($service->gapicClientType->getNamespace(), $licenseYear);
Expand All @@ -380,15 +380,17 @@ private static function generateServices(
// [Start surface version-agnostic code generation]

// Oneof wrapper classes.
$ctx = new SourceFileContext($service->gapicClientType->getNamespace(), $licenseYear);
$oneofWrapperFiles = OneofWrapperGenerator::generate($ctx, $service);
foreach ($oneofWrapperFiles as $oneofWrapperFile) {
$oneofClassNameComponents = explode('\\', $oneofWrapperFile->class->type->getFullname(/* omitLeadingBackslash = */ true));
$oneofContainingMessageName = $oneofClassNameComponents[sizeof($oneofClassNameComponents) - 2];
$oneofClassName = $oneofClassNameComponents[sizeof($oneofClassNameComponents) - 1];
$oneofCode = $oneofWrapperFile->toCode();
$oneofCode = Formatter::format($oneofCode);
yield ["src/{$version}$oneofContainingMessageName/$oneofClassName.php", $oneofCode];
if ($migrationMode != MigrationMode::NEW_SURFACE_ONLY) {
$ctx = new SourceFileContext($service->gapicClientType->getNamespace(), $licenseYear);
$oneofWrapperFiles = OneofWrapperGenerator::generate($ctx, $service);
foreach ($oneofWrapperFiles as $oneofWrapperFile) {
$oneofClassNameComponents = explode('\\', $oneofWrapperFile->class->type->getFullname(/* omitLeadingBackslash = */ true));
$oneofContainingMessageName = $oneofClassNameComponents[sizeof($oneofClassNameComponents) - 2];
$oneofClassName = $oneofClassNameComponents[sizeof($oneofClassNameComponents) - 1];
$oneofCode = $oneofWrapperFile->toCode();
$oneofCode = Formatter::format($oneofCode);
yield ["src/{$version}$oneofContainingMessageName/$oneofClassName.php", $oneofCode];
}
}

// Resource: descriptor_config.php
Expand Down
7 changes: 5 additions & 2 deletions src/Generation/TestNameValueProducer.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,22 @@

class TestNameValueProducer
{
public function __construct(ProtoCatalog $catalog, SourceFileContext $ctx)
public function __construct(ProtoCatalog $catalog, SourceFileContext $ctx, bool $isV2Test = false)
{
$this->catalog = $catalog;
$this->ctx = $ctx;
$this->names = Set::new();
$this->values = Set::new();
$this->valuesByName = Map::new();
$this->isV2Test = $isV2Test;
}

private ProtoCatalog $catalog;
private SourceFileContext $ctx;
private Set $names;
private Set $values;
private Map $valuesByName;
private bool $isV2Test;

public function name(string $name): string
{
Expand Down Expand Up @@ -145,7 +147,8 @@ public function fieldInit(MethodDetails $method, FieldDetails $field, &$fieldVar

// This should only use oneof wrapper types if the oneof is on the top level request message.
$inTopLevel = $method->inputMsg === $field->containingMessage;
if ($field->isOneOf && $inTopLevel) {
// This should only be used in V1 tests, as V2 does not need Oneof wrappers.
if ($field->isOneOf && $inTopLevel && !$this->isV2Test) {
$oneofWrapperType = $field->toOneofWrapperType($method->serviceDetails->namespace);
// Initialize the oneof, e.g.
// $supplementaryData = new SupplementaryDataOneof();
Expand Down
32 changes: 12 additions & 20 deletions src/Generation/UnitTestsV2Generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ private function testAsync(MethodDetails $method)

private function testSuccessCaseNormal(MethodDetails $method, $async = false): PhpMethod
{
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$transport = AST::var('transport');
$client = AST::var(self::CLIENT_VARIABLE);
$expectedResponse = AST::var('expectedResponse');
Expand All @@ -259,7 +259,7 @@ private function testSuccessCaseNormal(MethodDetails $method, $async = false): P
$invocation = $async
? AST::call($client->instanceCall(AST::method($methodName))($requestAssignment->to), AST::method('wait'))()
: $client->instanceCall(AST::method($methodName))($requestAssignment->to);

return AST::method($async ? $method->testAsyncMethodName : $method->testSuccessMethodName)
->withAccess(Access::PUBLIC)
->withBody(AST::block(
Expand Down Expand Up @@ -288,15 +288,7 @@ private function testSuccessCaseNormal(MethodDetails $method, $async = false): P
($this->assertSame)("/$rpcHostServiceName/{$method->name}", $actualFuncCall),
$requestPerField->map(fn ($x) => Vector::new([
AST::assign($actualValue, $actualRequestObject->instanceCall($x->field->getter)()),
!$x->field->isOneOf
? ($this->assertProtobufEquals)($x->var, $actualValue)
: ($this->assertTrue)(
// Variable of type oneof wrapper, e.g. supplementaryData.
AST::call(
AST::var(Helpers::toCamelCase($x->field->getOneofDesc()->getName())),
AST::method("is" . Helpers::toUpperCamelCase($x->field->name))
)()
)
($this->assertProtobufEquals)($x->var, $actualValue)
])),
($this->assertTrue)(AST::call($transport, AST::method('isExhausted'))()),
))
Expand All @@ -305,7 +297,7 @@ private function testSuccessCaseNormal(MethodDetails $method, $async = false): P

private function testExceptionalCaseNormal(MethodDetails $method): PhpMethod
{
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$transport = AST::var('transport');
$client = AST::var(self::CLIENT_VARIABLE);
$status = AST::var('status');
Expand Down Expand Up @@ -350,7 +342,7 @@ private function testExceptionalCaseNormal(MethodDetails $method): PhpMethod

private function testSuccessCaseLro(MethodDetails $method, $async = false): PhpMethod
{
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$expectedResponse = AST::var('expectedResponse');
$anyResponse = AST::var('anyResponse');
$completeOperation = AST::var('completeOperation');
Expand Down Expand Up @@ -430,7 +422,7 @@ private function testSuccessCaseLro(MethodDetails $method, $async = false): PhpM

private function testExceptionalCaseLro(MethodDetails $method): PhpMethod
{
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$status = AST::var('status');
$expectedExceptionMessage = AST::var('expectedExceptionMessage');
[$requestPerField, $requestCallArgs] = $prod->perFieldRequest($method);
Expand Down Expand Up @@ -513,7 +505,7 @@ private function lroTestInit($testName)

private function testSuccessCasePaginated(MethodDetails $method, $async = false): PhpMethod
{
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$transport = AST::var('transport');
$client = AST::var(self::CLIENT_VARIABLE);
$expectedResponse = AST::var('expectedResponse');
Expand Down Expand Up @@ -597,7 +589,7 @@ private function testSuccessCasePaginated(MethodDetails $method, $async = false)

private function testSuccessCaseBidiStreaming(MethodDetails $method): PhpMethod
{
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$transport = AST::var('transport');
$client = AST::var(self::CLIENT_VARIABLE);
$expectedResponseList = Vector::range(1, 3)->map(fn ($i) => AST::var('expectedResponse' . ($i === 1 ? '' : $i)));
Expand Down Expand Up @@ -713,7 +705,7 @@ private function testExceptionalCaseBidiStreaming(MethodDetails $method): PhpMet
private function testSuccessCaseServerStreaming(MethodDetails $method): PhpMethod
{
// TODO: Support resource-names in request args.
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$transport = AST::var('transport');
$client = AST::var(self::CLIENT_VARIABLE);
$expectedResponseList = Vector::range(1, 3)->map(fn ($i) => AST::var('expectedResponse' . ($i === 1 ? '' : $i)));
Expand Down Expand Up @@ -776,7 +768,7 @@ private function testSuccessCaseServerStreaming(MethodDetails $method): PhpMetho

private function testExceptionalCaseServerStreaming(MethodDetails $method): PhpMethod
{
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$transport = AST::var('transport');
$client = AST::var(self::CLIENT_VARIABLE);
$status = AST::var('status');
Expand Down Expand Up @@ -825,7 +817,7 @@ private function testExceptionalCaseServerStreaming(MethodDetails $method): PhpM
private function testSuccessCaseCustomOp(MethodDetails $method, $async = false): PhpMethod
{
$testMethodName = $async ? $method->testAsyncMethodName : $method->testSuccessMethodName;
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$completeOperation = AST::var('completeOperation');
[$requestPerField, $requestCallArgs] = $prod->perFieldRequest($method);
$response = AST::var('response');
Expand Down Expand Up @@ -903,7 +895,7 @@ private function testSuccessCaseCustomOp(MethodDetails $method, $async = false):

private function testExceptionalCaseCustomOp(MethodDetails $method): PhpMethod
{
$prod = new TestNameValueProducer($method->catalog, $this->ctx);
$prod = new TestNameValueProducer($method->catalog, $this->ctx, true);
$status = AST::var('status');
$expectedExceptionMessage = AST::var('expectedExceptionMessage');
[$requestPerField, $requestCallArgs] = $prod->perFieldRequest($method);
Expand Down
77 changes: 77 additions & 0 deletions tests/Unit/ProtoTests/BasicOneofNew/basic-oneof-new.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
syntax = "proto3";

package testing.basiconeofnew;

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

option php_namespace = "Testing\\BasicOneofNew";

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

// This is a basic service.
service BasicOneofNew {
option (google.api.default_host) = "basic.example.com";
option (google.api.oauth_scopes) = "scope1,scope2";

// Test including method args with required oneofs.
rpc AMethod(Request) returns (Response) {
option (google.api.http) = {
post: "/path:aMethod"
body: "*"
};
}
}

message PartOfRequestA {}

message Request {
int32 an_int = 1;

oneof supplementary_data {
// Supplemental request description.
string extra_description = 2 [(google.api.field_behavior) = REQUIRED];

// Supplemental request summary.
string extra_summary = 3 [(google.api.field_behavior) = REQUIRED];

// An extra request.
PartOfRequestA extra_request = 4 [(google.api.field_behavior) = REQUIRED];

// An extra index.
int32 extra_index = 5 [(google.api.field_behavior) = REQUIRED];

// An extra double.
double extra_double = 6 [(google.api.field_behavior) = REQUIRED];

// An extra float.
float extra_float = 7 [(google.api.field_behavior) = REQUIRED];

// An extra bool.
bool extra_bool = 8 [(google.api.field_behavior) = REQUIRED];
}

oneof optional_data {
// An optional payload.
string optional_payload = 10;

// An optional count.
int32 optional_count = 11;
}

message Other {
oneof other_of {
string first = 1 [(google.api.field_behavior) = REQUIRED];

string second = 2 [(google.api.field_behavior) = REQUIRED];
}
}

Other other = 12 [(google.api.field_behavior) = REQUIRED];

optional string required_optional = 13 [(google.api.field_behavior) = REQUIRED];
}

message Response {}
10 changes: 10 additions & 0 deletions tests/Unit/ProtoTests/BasicOneofNew/basic-oneof-new_gapic.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
type: google.api.Service
config_version: 3

authentication:
rules:
- selector: 'testing.basic.BasicOneof.*'
oauth:
canonical_scopes: |-
scope1,
scope2
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/*
* GENERATED CODE WARNING
* This file was automatically generated - do not edit!
*/

require_once __DIR__ . '/../../../vendor/autoload.php';

// [START basic_generated_BasicOneofNew_AMethod_sync]
use Google\ApiCore\ApiException;
use Testing\BasicOneofNew\Client\BasicOneofNewClient;
use Testing\BasicOneofNew\Request;
use Testing\BasicOneofNew\Request\Other;
use Testing\BasicOneofNew\Response;

/**
* Test including method args with required oneofs.
*
* @param string $extraDescription Supplemental request description.
* @param string $otherFirst
* @param string $requiredOptional
*/
function a_method_sample(
string $extraDescription,
string $otherFirst,
string $requiredOptional
): void {
// Create a client.
$basicOneofNewClient = new BasicOneofNewClient();

// Prepare the request message.
$other = (new Other())
->setFirst($otherFirst);
$request = (new Request())
->setExtraDescription($extraDescription)
->setOther($other)
->setRequiredOptional($requiredOptional);

// Call the API and handle any network failures.
try {
/** @var Response $response */
$response = $basicOneofNewClient->aMethod($request);
printf('Response data: %s' . PHP_EOL, $response->serializeToJsonString());
} catch (ApiException $ex) {
printf('Call failed with message: %s' . PHP_EOL, $ex->getMessage());
}
}

/**
* Helper to execute the sample.
*
* This sample has been automatically generated and should be regarded as a code
* template only. It will require modifications to work:
* - It may require correct/in-range values for request initialization.
* - It may require specifying regional endpoints when creating the service client,
* please see the apiEndpoint client configuration option for more details.
*/
function callSample(): void
{
$extraDescription = '[EXTRA_DESCRIPTION]';
$otherFirst = '[FIRST]';
$requiredOptional = '[REQUIRED_OPTIONAL]';

a_method_sample($extraDescription, $otherFirst, $requiredOptional);
}
// [END basic_generated_BasicOneofNew_AMethod_sync]
Loading

0 comments on commit f69fcfb

Please sign in to comment.