Merge branch 'main' into renovate/symplify-coding-standard-9.x

googleapis · May 25, 2023 · 5cec88a · 5cec88a
2 parents af76b6c + 5a7f5c4
commit 5cec88a
Show file tree

Hide file tree

Showing 126 changed files with 866 additions and 193 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,47 @@
 # Changelog
 
+## [1.7.5](https://github.com/googleapis/gapic-generator-php/compare/v1.7.4...v1.7.5) (2023-05-10)
+
+
+### Bug Fixes
+
+* Remove oneof wrappers in v2 ([#629](https://github.com/googleapis/gapic-generator-php/issues/629)) ([f69fcfb](https://github.com/googleapis/gapic-generator-php/commit/f69fcfb19121d1bb4f4c6282be3cced945e6accc))
+
+## [1.7.4](https://github.com/googleapis/gapic-generator-php/compare/v1.7.3...v1.7.4) (2023-05-09)
+
+
+### Bug Fixes
+
+* Do not generate fragments for requests in other packages ([#626](https://github.com/googleapis/gapic-generator-php/issues/626)) ([d5be067](https://github.com/googleapis/gapic-generator-php/commit/d5be067dce236efb8f66737e80074277dc5dfd86))
+
+## [1.7.3](https://github.com/googleapis/gapic-generator-php/compare/v1.7.2...v1.7.3) (2023-05-03)
+
+
+### Bug Fixes
+
+* Reduce noise by reverting sample comment ([#622](https://github.com/googleapis/gapic-generator-php/issues/622)) ([d2e87ab](https://github.com/googleapis/gapic-generator-php/commit/d2e87ab51912c4f1e3e09e85f11330a1d3ef7970))
+
+## [1.7.2](https://github.com/googleapis/gapic-generator-php/compare/v1.7.1...v1.7.2) (2023-05-02)
+
+
+### Bug Fixes
+
+* **bazel:** Ignore empty zip files during packaging ([#619](https://github.com/googleapis/gapic-generator-php/issues/619)) ([0db730f](https://github.com/googleapis/gapic-generator-php/commit/0db730faadf82ffc1430ef2cc42c3919e48fa91d))
+
+## [1.7.1](https://github.com/googleapis/gapic-generator-php/compare/v1.7.0...v1.7.1) (2023-05-02)
+
+
+### Bug Fixes
+
+* Lower hashing constant to avoid overflow ([#617](https://github.com/googleapis/gapic-generator-php/issues/617)) ([b05d16f](https://github.com/googleapis/gapic-generator-php/commit/b05d16fe1650cd92a97eb2d872dc0561462924c8))
+
+## [1.7.0](https://github.com/googleapis/gapic-generator-php/compare/v1.6.8...v1.7.0) (2023-05-01)
+
+
+### Features
+
+* Introduce next generation PHP client surface ([#612](https://github.com/googleapis/gapic-generator-php/issues/612)) ([c675c3d](https://github.com/googleapis/gapic-generator-php/commit/c675c3d6a98e37c4cb79bbcb187a481d3cbceea7))
+
 ## [1.6.8](https://github.com/googleapis/gapic-generator-php/compare/v1.6.7...v1.6.8) (2023-04-26)
 
 

diff --git a/composer.json b/composer.json
@@ -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",

diff --git a/rules_php_gapic/php_gapic_pkg.bzl b/rules_php_gapic/php_gapic_pkg.bzl
@@ -55,11 +55,19 @@ def _php_gapic_src_pkg_impl(ctx):
     set -e
     for proto_grpc_src in {proto_grpc_srcs}; do
         mkdir -p {package_dir_path}/proto/src
-        unzip -q -o $proto_grpc_src -d {package_dir_path}/proto/src
+        if zipinfo -t $proto_grpc_src | grep -q "Empty zipfile."; then
+            echo "Ignoring empty proto/grpc zip file $proto_grpc_src";
+        else
+            unzip -q -o $proto_grpc_src -d {package_dir_path}/proto/src
+        fi
     done
     for gapic_src in {gapic_srcs}; do
-        mkdir -p {package_dir_path}/proto
-        unzip -q -o $gapic_src -d {package_dir_path}
+        if zipinfo -t $gapic_src | grep -q "Empty zipfile."; then
+            echo "Ignoring empty gapic zip file $gapic_src";
+        else
+            mkdir -p {package_dir_path}/proto
+            unzip -q -o $gapic_src -d {package_dir_path}
+        fi
     done
     {post_processor} --input {package_dir_path}
     cd {package_dir_path}/{tar_cd_suffix}

diff --git a/src/CodeGenerator.php b/src/CodeGenerator.php
@@ -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);
@@ -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);
@@ -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

diff --git a/src/Collections/Vector.php b/src/Collections/Vector.php
@@ -144,7 +144,7 @@ public function getHash(): int
     {
         $hash = 1;
         foreach ($this->data as $item) {
-            $hash *= 23;
+            $hash *= 13;
             $hash ^= static::hash($item);
         }
         return $hash;

diff --git a/src/Generation/BuildMethodFragmentGenerator.php b/src/Generation/BuildMethodFragmentGenerator.php
@@ -56,6 +56,10 @@ private function generateImpl(): Map
         $buildMethodSnippets = Map::new([]);
         foreach ($this->serviceDetails->methods as $methodDetails) {
             if ($methodDetails->methodSignature && !$methodDetails->isMixin()) {
+                // Do not generate build method fragments for request messages outside this package
+                if (0 !== strpos($methodDetails->inputMsg->desc->getFullName(), $this->serviceDetails->package)) {
+                    continue;
+                }
                 $buildMethods = Vector::new();
                 foreach ($methodDetails->methodSignature as $i => $methodSignature) {
                     if (empty($methodSignature)) {

diff --git a/src/Generation/SnippetGenerator.php b/src/Generation/SnippetGenerator.php
@@ -378,7 +378,11 @@ private function buildSnippetFunctions(SnippetDetails $snippetDetails, array $tr
         $shouldGenerateDocBlock = $docLineCount > 0
             || count($snippetDetails->phpDocParams) > 0
             || !$hasSampleParams;
-        $clientType = $this->serviceDetails->migrationMode == MigrationMode::MIGRATION_MODE_UNSPECIFIED || $this->serviceDetails->migrationMode == MigrationMode::PRE_MIGRATION_SURFACE_ONLY ?
+        $preMigrationSurface = in_array(
+            $this->serviceDetails->migrationMode,
+            [MigrationMode::MIGRATION_MODE_UNSPECIFIED, MigrationMode::PRE_MIGRATION_SURFACE_ONLY]
+        );
+        $clientType = $preMigrationSurface ?
             $this->serviceDetails->emptyClientType :
             $this->serviceDetails->emptyClientV2Type;
 
@@ -393,7 +397,11 @@ private function buildSnippetFunctions(SnippetDetails $snippetDetails, array $tr
                         AST::new($snippetDetails->context->type($clientType))()
                     ),
                     $hasSampleAssignments ? PHP_EOL : null,
-                    $hasSampleAssignments ? '// Prepare the request message.' : null,
+                    $hasSampleAssignments ? (
+                        $preMigrationSurface
+                            ? '// Prepare any non-scalar elements to be passed along with the request.'
+                            : '// Prepare the request message.'
+                        ) : null,
                     $snippetDetails->sampleAssignments,
                     PHP_EOL,
                     '// Call the API and handle any network failures.',

diff --git a/src/Generation/TestNameValueProducer.php b/src/Generation/TestNameValueProducer.php
@@ -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
     {
@@ -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();

diff --git a/src/Generation/UnitTestsV2Generator.php b/src/Generation/UnitTestsV2Generator.php
@@ -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');
@@ -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(
@@ -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'))()),
             ))
@@ -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');
@@ -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');
@@ -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);
@@ -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');
@@ -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)));
@@ -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)));
@@ -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');
@@ -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');
@@ -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);

diff --git a/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/analyze_iam_policy.php b/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/analyze_iam_policy.php
@@ -50,7 +50,7 @@ function analyze_iam_policy_sample(string $analysisQueryScope): void
     // Create a client.
     $assetServiceClient = new AssetServiceClient();
 
-    // Prepare the request message.
+    // Prepare any non-scalar elements to be passed along with the request.
     $analysisQuery = (new IamPolicyAnalysisQuery())
         ->setScope($analysisQueryScope);
 

diff --git a/...ntegration/goldens/asset/samples/V1/AssetServiceClient/analyze_iam_policy_longrunning.php b/...ntegration/goldens/asset/samples/V1/AssetServiceClient/analyze_iam_policy_longrunning.php
@@ -60,7 +60,7 @@ function analyze_iam_policy_longrunning_sample(string $analysisQueryScope): void
     // Create a client.
     $assetServiceClient = new AssetServiceClient();
 
-    // Prepare the request message.
+    // Prepare any non-scalar elements to be passed along with the request.
     $analysisQuery = (new IamPolicyAnalysisQuery())
         ->setScope($analysisQueryScope);
     $outputConfig = new IamPolicyAnalysisOutputConfig();

diff --git a/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/batch_get_assets_history.php b/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/batch_get_assets_history.php
@@ -48,7 +48,7 @@ function batch_get_assets_history_sample(string $parent, int $contentType): void
     // Create a client.
     $assetServiceClient = new AssetServiceClient();
 
-    // Prepare the request message.
+    // Prepare any non-scalar elements to be passed along with the request.
     $readTimeWindow = new TimeWindow();
 
     // Call the API and handle any network failures.

diff --git a/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/create_feed.php b/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/create_feed.php
@@ -52,7 +52,7 @@ function create_feed_sample(string $parent, string $feedId, string $feedName): v
     // Create a client.
     $assetServiceClient = new AssetServiceClient();
 
-    // Prepare the request message.
+    // Prepare any non-scalar elements to be passed along with the request.
     $feedFeedOutputConfig = new FeedOutputConfig();
     $feed = (new Feed())
         ->setName($feedName)

diff --git a/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/export_assets.php b/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/export_assets.php
@@ -52,7 +52,7 @@ function export_assets_sample(string $parent): void
     // Create a client.
     $assetServiceClient = new AssetServiceClient();
 
-    // Prepare the request message.
+    // Prepare any non-scalar elements to be passed along with the request.
     $outputConfig = new OutputConfig();
 
     // Call the API and handle any network failures.

diff --git a/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/update_feed.php b/tests/Integration/goldens/asset/samples/V1/AssetServiceClient/update_feed.php
@@ -45,7 +45,7 @@ function update_feed_sample(string $feedName): void
     // Create a client.
     $assetServiceClient = new AssetServiceClient();
 
-    // Prepare the request message.
+    // Prepare any non-scalar elements to be passed along with the request.
     $feedFeedOutputConfig = new FeedOutputConfig();
     $feed = (new Feed())
         ->setName($feedName)

diff --git a/tests/Integration/goldens/compute_small/samples/V1/AddressesClient/insert.php b/tests/Integration/goldens/compute_small/samples/V1/AddressesClient/insert.php
@@ -40,7 +40,7 @@ function insert_sample(string $project, string $region): void
     // Create a client.
     $addressesClient = new AddressesClient();
 
-    // Prepare the request message.
+    // Prepare any non-scalar elements to be passed along with the request.
     $addressResource = new Address();
 
     // Call the API and handle any network failures.