Merge pull request #469: Fix empty scheduled workflow id

temporalio · Jul 2, 2024 · 4c119c0 · 4c119c0
2 parents 6f70059 + 57bc802
commit 4c119c0
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 7 deletions.
diff --git a/src/Client/Schedule/Action/StartWorkflowAction.php b/src/Client/Schedule/Action/StartWorkflowAction.php
@@ -11,6 +11,7 @@
 use Temporal\Common\IdReusePolicy;
 use Temporal\Common\RetryOptions;
 use Temporal\Common\TaskQueue\TaskQueue;
+use Temporal\Common\Uuid;
 use Temporal\DataConverter\EncodedCollection;
 use Temporal\DataConverter\EncodedValues;
 use Temporal\DataConverter\ValuesInterface;
@@ -107,7 +108,7 @@ final class StartWorkflowAction extends ScheduleAction
 
     private function __construct(WorkflowType $workflowType)
     {
-        $this->workflowId = '';
+        $this->workflowId = Uuid::v4();
         $this->workflowType = $workflowType;
         $this->taskQueue = TaskQueue::new('default');
         $this->input = EncodedValues::empty();
@@ -130,6 +131,8 @@ public static function new(string|WorkflowType $workflowType): self
 
     public function withWorkflowId(string $workflowId): self
     {
+        $workflowId !== '' or throw new \InvalidArgumentException('Workflow ID cannot be empty.');
+
         /** @see self::$workflowId */
         return $this->with('workflowId', $workflowId);
     }

diff --git a/tests/Unit/Schedule/Action/StartWorkflowActionTestCase.php b/tests/Unit/Schedule/Action/StartWorkflowActionTestCase.php
@@ -43,22 +43,21 @@ public function testWithWorkflowTypeObject(): void
     public function testWithWorkflowId(): void
     {
         $init = StartWorkflowAction::new('TestWorkflow');
+        $initId = $init->workflowId;
         $new = $init->withWorkflowId('workflow-id');
 
         $this->assertNotSame($init, $new, 'immutable method clones object');
-        $this->assertSame('', $init->workflowId, 'init value was not changed');
+        $this->assertSame($initId, $init->workflowId, 'init value was not changed');
         $this->assertSame('workflow-id', $new->workflowId);
     }
 
     public function testWithEmptyWorkflowId(): void
     {
         $init = StartWorkflowAction::new('TestWorkflow')->withWorkflowId('test-id');
 
-        $new = $init->withWorkflowId('');
+        $this->expectException(\InvalidArgumentException::class);
 
-        $this->assertNotSame($init, $new, 'immutable method clones object');
-        $this->assertSame('test-id', $init->workflowId, 'init value was not changed');
-        $this->assertSame('', $new->workflowId);
+        $init->withWorkflowId('');
     }
 
     public function testWithTaskQueue(): void

diff --git a/tests/Unit/Schedule/Mapper/WorkflowExecutionInfoMapperTestCase.php b/tests/Unit/Schedule/Mapper/WorkflowExecutionInfoMapperTestCase.php
@@ -206,7 +206,7 @@ public function testToMessageEmptyValues(): void
         $this->assertSame(0, $startWorkflow->getHeader()->getFields()->count());
         $this->assertSame(0, $startWorkflow->getMemo()->getFields()->count());
         $this->assertSame(0, $startWorkflow->getSearchAttributes()->getIndexedFields()->count());
-        $this->assertEmpty($startWorkflow->getWorkflowId());
+        $this->assertNotEmpty($startWorkflow->getWorkflowId());
         $this->assertSame(IdReusePolicy::Unspecified->value, $startWorkflow->getWorkflowIdReusePolicy());
 
         // Test Spec

diff --git a/tests/Unit/Schedule/ScheduleClientTestCase.php b/tests/Unit/Schedule/ScheduleClientTestCase.php
@@ -57,6 +57,35 @@ public function testCreateSchedule(): void
         $this->assertSame('default', $testContext->request->getNamespace());
         $this->assertSame('test-id', $testContext->request->getScheduleId());
         $this->assertSame('test-id', $result->getID());
+        $this->assertSame(
+            'workflow-id',
+            $testContext->request->getSchedule()->getAction()->getStartWorkflow()->getWorkflowId(),
+        );
+    }
+
+    public function testCreateScheduleWithoutWorkflowId(): void
+    {
+        $testContext = new class {
+            public CreateScheduleRequest $request;
+        };
+        // Prepare mocks
+        $clientMock = $this->createMock(ServiceClientInterface::class);
+        $clientMock->expects($this->once())
+            ->method('CreateSchedule')
+            ->with($this->callback(fn(CreateScheduleRequest $request) => $testContext->request = $request or true))
+            ->willReturn((new CreateScheduleResponse())->setConflictToken('test-conflict-token'));
+        $scheduleClient = $this->createScheduleClient(
+            client: $clientMock,
+        );
+        $result = $scheduleClient->createSchedule(
+            Schedule::new()->withAction(
+                StartWorkflowAction::new('PingSite')
+            ),
+            scheduleId: 'test-id',
+        );
+
+        $this->assertTrue(isset($testContext->request));
+        $this->assertNotEmpty($testContext->request->getSchedule()->getAction()->getStartWorkflow()->getWorkflowId());
     }
 
     public function testListSchedules(): void