fix: ensure compute request build parameters have the operation ID last (#625)

Author: bshaffer

src/OperationResponse.php

@@ -473,16 +473,28 @@ public function getMetadata()
      */
     private function operationsCall(string $method, ?string $requestClass)
     {
-        $args = array_merge([$this->getName()], array_values($this->additionalArgs));
-        if ($requestClass) {
-            if (!method_exists($requestClass, 'build')) {
-                throw new LogicException('Request class must support the static build method');
+        // V1 GAPIC clients have an empty $requestClass
+        if (empty($requestClass)) {
+            if ($this->additionalArgs) {
+                return $this->operationsClient->$method(
+                    $this->getName(),
+                    ...array_values($this->additionalArgs)
+                );
             }
-            $request = call_user_func_array($requestClass . '::build', $args);
-            $args = [$request];
+            return $this->operationsClient->$method($this->getName());
         }
 
-        return call_user_func_array([$this->operationsClient, $method], $args);
+        if (!method_exists($requestClass, 'build')) {
+            throw new LogicException('Request class must support the static build method');
+        }
+        // In V2 of Compute, the Request "build" methods contain the operation ID last instead
+        // of first. Compute is the only API which uses $additionalArgs, so switching the order
+        // will not break anything.
+        $request = $requestClass::build(...array_merge(
+            array_values($this->additionalArgs),
+            [$this->getName()]
+        ));
+        return $this->operationsClient->$method($request);
     }
 
     private function canHaveResult()

tests/Unit/OperationResponseTest.php

@@ -274,7 +274,14 @@ public function testNewSurfaceCustomOperation()
                 $phpunit->assertEquals('test-123', $request->name);
                 $phpunit->assertEquals('arg2', $request->arg2);
                 $phpunit->assertEquals('arg3', $request->arg3);
-                return new \stdClass();
+                return new class {
+                    public function getDone() {
+                        return true;
+                    }
+                    public function getError() {
+                        return false;
+                    }
+                };
             });
         $operationClient->cancelNewSurfaceOperation(Argument::type(Client\CancelOperationRequest::class))
             ->shouldBeCalledOnce()
@@ -311,6 +318,11 @@ public function testNewSurfaceCustomOperation()
         // Test getOperationMethod
         $operationResponse->reload();
 
+        // Test operationStatusMethod and operationStatusDoneValue
+        $this->assertTrue($operationResponse->isDone());
+
+        $this->assertTrue($operationResponse->operationSucceeded());
+
         // test cancelOperationMethod
         $operationResponse->cancel();
 

tests/Unit/testdata/src/CustomOperationClient.php

@@ -27,7 +27,7 @@ abstract class BaseOperationRequest
     public string $arg2;
     public string $arg3;
 
-    public static function build(string $name, string $arg2, string $arg3): static
+    public static function build(string $arg2, string $arg3, string $name): static
     {
         $request = new static();
         $request->name = $name;