Fix: abandoned child workflow start (#240)

* Fix: abandoned child workflow start * Fix: abandoned child workflow start
temporalio · Sep 2, 2022 · ac565e2 · ac565e2
1 parent f2d6dd7
commit ac565e2
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 35 deletions.
diff --git a/src/Internal/Workflow/ChildWorkflowStub.php b/src/Internal/Workflow/ChildWorkflowStub.php
@@ -32,6 +32,7 @@ final class ChildWorkflowStub implements ChildWorkflowStubInterface
     private ChildWorkflowOptions $options;
     private MarshallerInterface $marshaller;
     private ?ExecuteChildWorkflow $request = null;
+    private ?PromiseInterface $result = null;
 
     /**
      * @param MarshallerInterface $marshaller
@@ -74,27 +75,9 @@ public function start(... $args): PromiseInterface
             $this->getOptionsArray()
         );
 
-        return $this->request($this->request);
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    public function execute(array $args = [], $returnType = null): PromiseInterface
-    {
-        if ($this->request !== null) {
-            throw new \LogicException('Child workflow already has been executed');
-        }
+        $this->result = $this->request($this->request);
 
-        $this->request = new ExecuteChildWorkflow(
-            $this->workflow,
-            EncodedValues::fromValues($args),
-            $this->getOptionsArray()
-        );
-
-        $promise = $this->request($this->request);
-
-        $this->request(new GetChildWorkflowExecution($this->request))
+        $started = $this->request(new GetChildWorkflowExecution($this->request))
             ->then(
                 function (ValuesInterface $values) {
                     $execution = $values->getValue(0, WorkflowExecution::class);
@@ -104,7 +87,20 @@ function (ValuesInterface $values) {
                 }
             );
 
-        return EncodedValues::decodePromise($promise, $returnType);
+        return EncodedValues::decodePromise($started);
+    }
+
+    public function getResult($returnType = null): PromiseInterface
+    {
+        return EncodedValues::decodePromise($this->result, $returnType);
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function execute(array $args = [], $returnType = null): PromiseInterface
+    {
+        return $this->start(...$args)->then(fn() => $this->getResult($returnType));
     }
 
     /**

diff --git a/src/Workflow/ChildWorkflowStubInterface.php b/src/Workflow/ChildWorkflowStubInterface.php
@@ -46,6 +46,8 @@ public function execute(array $args = [], $returnType = null): PromiseInterface;
      */
     public function start(... $args): PromiseInterface;
 
+    public function getResult($returnType = null): PromiseInterface;
+
     /**
      * @param string $name
      * @param array $args

diff --git a/testing/src/Environment.php b/testing/src/Environment.php
@@ -82,7 +82,7 @@ public function start(string $rrCommand = null, int $commandTimeout = 10): void
             $this->output->writeln('Error starting RoadRunner: ' . $this->roadRunnerProcess->getErrorOutput());
             exit(1);
         }
-        
+
         $this->output->writeln('<info>done.</info>');
     }
 

diff --git a/tests/Fixtures/src/Workflow/AbandonedChildWithTimerWorkflow.php b/tests/Fixtures/src/Workflow/AbandonedChildWithTimerWorkflow.php
@@ -18,5 +18,6 @@ class AbandonedChildWithTimerWorkflow
     public function wait(int $timeoutInSeconds)
     {
         Workflow::timer($timeoutInSeconds);
+        return 'Hello from child';
     }
 }
diff --git a/tests/Fixtures/src/Workflow/ParentWithAbandonedChildWorkflow.php b/tests/Fixtures/src/Workflow/ParentWithAbandonedChildWorkflow.php
@@ -14,7 +14,7 @@
 class ParentWithAbandonedChildWorkflow
 {
     #[WorkflowMethod]
-    public function start(int $childTimeoutInSeconds)
+    public function start(int $childTimeoutInSeconds, bool $shouldWaitForChild)
     {
         $child = Workflow::newUntypedChildWorkflowStub(
             'abandoned_workflow',
@@ -23,6 +23,9 @@ public function start(int $childTimeoutInSeconds)
         );
 
         yield $child->start($childTimeoutInSeconds);
+        if ($shouldWaitForChild) {
+           return yield $child->getResult();
+        }
 
         return 'Welcome from parent';
     }

diff --git a/tests/Functional/Client/AbandonedChildWorkflowTestCase.php b/tests/Functional/Client/AbandonedChildWorkflowTestCase.php
@@ -4,22 +4,27 @@
 
 namespace Temporal\Tests\Functional\Client;
 
-use Temporal\Testing\WithoutTimeSkipping;
+use Carbon\Carbon;
 use Temporal\Testing\WorkflowTestCase;
 use Temporal\Tests\Workflow\ParentWithAbandonedChildWorkflow;
 
 final class AbandonedChildWorkflowTestCase extends WorkflowTestCase
 {
-    use WithoutTimeSkipping;
-
     public function testParentEndsWithoutWaitingForChild(): void
     {
-        $timeBeforeStart = $this->testingService->getCurrentTime();
-        /** @var ParentWithAbandonedChildWorkflow $parentWorkflow */
+        $timeBeforeStart = Carbon::now();
         $parentWorkflow = $this->workflowClient->newWorkflowStub(ParentWithAbandonedChildWorkflow::class);
-        $parentWorkflow->start(10);
-        $timeAfterStart = $this->testingService->getCurrentTime();
+        $run = $this->workflowClient->start($parentWorkflow, 5, false);
+        static::assertSame('Welcome from parent', $run->getResult());
+        $timeAfterStart = Carbon::now();
         static::assertTrue($timeAfterStart->diffInSeconds($timeBeforeStart) < 2);
     }
+
+    public function testParentCanWaitForChildResult(): void
+    {
+        $parentWorkflow = $this->workflowClient->newWorkflowStub(ParentWithAbandonedChildWorkflow::class);
+        $run = $this->workflowClient->start($parentWorkflow, 3, true);
+        static::assertSame('Hello from child', $run->getResult());
+    }
 }
 
diff --git a/tests/docker-compose.yaml b/tests/docker-compose.yaml
@@ -24,12 +24,12 @@ services:
       - "TEMPORAL_CLI_ADDRESS=temporal:7233"
     depends_on:
       - temporal
-  temporal-web:
-    image: temporalio/web:1.15.0
+  temporal-ui:
+    image: temporalio/ui:2.5.0
     environment:
-      - "TEMPORAL_GRPC_ENDPOINT=temporal:7233"
-      - "TEMPORAL_PERMIT_WRITE_API=true"
+      - TEMPORAL_ADDRESS=temporal:7233
+      - TEMPORAL_CORS_ORIGINS=http://localhost:3000
     ports:
-      - "8088:8088"
+      - "8088:8080"
     depends_on:
       - temporal