From 6242126005762493afc350b41f809f3a30fa1687 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Thu, 11 Apr 2024 17:19:26 +0400 Subject: [PATCH 1/3] Add WorkflowIdConflictPolicy into WorkflowOptions and use it in Workflow Client Service --- composer.json | 2 +- .../Workflow/WorkflowExecutionDescription.php | 3 ++ src/Client/WorkflowOptions.php | 31 ++++++++++++-- src/Common/WorkflowIdConflictPolicy.php | 40 +++++++++++++++++++ src/Internal/Client/WorkflowStarter.php | 1 + tests/Unit/DTO/WorkflowOptionsTestCase.php | 12 ++++++ 6 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 src/Common/WorkflowIdConflictPolicy.php diff --git a/composer.json b/composer.json index c1f812d83..f9b759565 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "psr/log": "^2.0 || ^3.0", "ramsey/uuid": "^4.7", "react/promise": "^2.9", - "roadrunner-php/roadrunner-api-dto": "^1.5.0", + "roadrunner-php/roadrunner-api-dto": "dev-update-dtos as 1.7.0", "roadrunner-php/version-checker": "^1.0", "spiral/attributes": "^3.1.4", "spiral/roadrunner": "^2023.3.12 || ^2024.1", diff --git a/src/Client/Workflow/WorkflowExecutionDescription.php b/src/Client/Workflow/WorkflowExecutionDescription.php index 8e1e7242b..82dfe1011 100644 --- a/src/Client/Workflow/WorkflowExecutionDescription.php +++ b/src/Client/Workflow/WorkflowExecutionDescription.php @@ -15,6 +15,9 @@ */ final class WorkflowExecutionDescription { + /** + * @internal + */ public function __construct( public readonly WorkflowExecutionInfo $info, ) { diff --git a/src/Client/WorkflowOptions.php b/src/Client/WorkflowOptions.php index 46867a351..322b4be1d 100644 --- a/src/Client/WorkflowOptions.php +++ b/src/Client/WorkflowOptions.php @@ -21,6 +21,7 @@ use Temporal\Common\MethodRetry; use Temporal\Common\RetryOptions; use Temporal\Common\Uuid; +use Temporal\Common\WorkflowIdConflictPolicy; use Temporal\DataConverter\DataConverterInterface; use Temporal\Internal\Marshaller\Meta\Marshal; use Temporal\Internal\Marshaller\Type\ArrayType; @@ -98,12 +99,19 @@ final class WorkflowOptions extends Options public \DateInterval $workflowTaskTimeout; /** - * Whether server allow reuse of workflow ID, can be useful for deduplication logic. - * If set to {@see IdReusePolicy::POLICY_REJECT_DUPLICATE}. + * Whether server allow reuse of workflow ID. + * + * Can be useful for deduplication logic if set to {@see IdReusePolicy::POLICY_REJECT_DUPLICATE}. */ #[Marshal(name: 'WorkflowIDReusePolicy')] public int $workflowIdReusePolicy = IdReusePolicy::POLICY_ALLOW_DUPLICATE_FAILED_ONLY; + /** + * Defines how to resolve an ID conflict with a *running* workflow. + */ + #[Marshal(name: 'WorkflowIdConflictPolicy')] + public WorkflowIdConflictPolicy $workflowIdConflictPolicy = WorkflowIdConflictPolicy::Unspecified; + /** * Optional retry policy for workflow. If a retry policy is specified, in * case of workflow failure server will start new workflow execution if @@ -319,8 +327,8 @@ public function withWorkflowStartDelay($delay): self } /** - * Specifies server behavior if a completed workflow with the same id - * exists. Note that under no conditions Temporal allows two workflows + * Specifies server behavior if a *closed* workflow with the same id exists. + * Note that under no conditions Temporal allows two workflows * with the same namespace and workflow id run simultaneously. * * - {@see IdReusePolicy::AllowDuplicateFailedOnly}: Is a default @@ -347,6 +355,21 @@ public function withWorkflowIdReusePolicy(IdReusePolicy|int $policy): self return $self; } + /** + * Defines how to resolve an ID conflict with a *running* workflow. + * + * @psalm-suppress ImpureMethodCall + * + * @return $this + */ + #[Pure] + public function withWorkflowIdConflictPolicy(WorkflowIdConflictPolicy $policy): self + { + $self = clone $this; + $self->workflowIdConflictPolicy = $policy; + return $self; + } + /** * RetryOptions that define how child workflow is retried in case of * failure. Default is null which is no reties. diff --git a/src/Common/WorkflowIdConflictPolicy.php b/src/Common/WorkflowIdConflictPolicy.php new file mode 100644 index 000000000..f0f84cd19 --- /dev/null +++ b/src/Common/WorkflowIdConflictPolicy.php @@ -0,0 +1,40 @@ +setCronSchedule($options->cronSchedule ?? '') ->setRetryPolicy($options->retryOptions ? $options->retryOptions->toWorkflowRetryPolicy() : null) ->setWorkflowIdReusePolicy($options->workflowIdReusePolicy) + ->setWorkflowIdConflictPolicy($options->workflowIdConflictPolicy->value) ->setWorkflowRunTimeout(DateInterval::toDuration($options->workflowRunTimeout)) ->setWorkflowExecutionTimeout(DateInterval::toDuration($options->workflowExecutionTimeout)) ->setWorkflowTaskTimeout(DateInterval::toDuration($options->workflowTaskTimeout)) diff --git a/tests/Unit/DTO/WorkflowOptionsTestCase.php b/tests/Unit/DTO/WorkflowOptionsTestCase.php index 820ed9664..afc8c0a5b 100644 --- a/tests/Unit/DTO/WorkflowOptionsTestCase.php +++ b/tests/Unit/DTO/WorkflowOptionsTestCase.php @@ -18,6 +18,7 @@ use Temporal\Common\IdReusePolicy; use Temporal\Common\RetryOptions; use Temporal\Common\Uuid; +use Temporal\Common\WorkflowIdConflictPolicy; use Temporal\DataConverter\DataConverter; class WorkflowOptionsTestCase extends AbstractDTOMarshalling @@ -125,6 +126,17 @@ public function testWorkflowIdReusePolicyChangesNotMutateStateUsingEnum(): void $this->assertSame(IdReusePolicy::AllowDuplicateFailedOnly->value, $dto->workflowIdReusePolicy); } + public function testWorkflowIdConflictPolicy(): void + { + $dto = new WorkflowOptions(); + + $this->assertNotSame($dto, $newDto = $dto->withWorkflowIdConflictPolicy( + WorkflowIdConflictPolicy::Fail + )); + $this->assertSame(WorkflowIdConflictPolicy::Unspecified, $dto->workflowIdConflictPolicy); + $this->assertSame(WorkflowIdConflictPolicy::Fail, $newDto->workflowIdConflictPolicy); + } + public function testRetryOptionsChangesNotMutateState(): void { $dto = new WorkflowOptions(); From d828239b9258b76e99c079a65640d3eac391bef5 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Thu, 11 Apr 2024 17:48:27 +0400 Subject: [PATCH 2/3] Deprecate \Temporal\Common\WorkerVersionStamp::$bundleId because it was removed --- src/Common/WorkerVersionStamp.php | 1 + src/Internal/Mapper/WorkflowExecutionInfoMapper.php | 1 - .../Client/Mapper/WorkflowExecutionInfoMapperTestCase.php | 3 +-- tests/Unit/DTO/WorkflowOptionsTestCase.php | 4 ++++ 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Common/WorkerVersionStamp.php b/src/Common/WorkerVersionStamp.php index f0975bd3c..7acf1cab1 100644 --- a/src/Common/WorkerVersionStamp.php +++ b/src/Common/WorkerVersionStamp.php @@ -17,6 +17,7 @@ final class WorkerVersionStamp { public function __construct( public string $buildId = '', + /** @deprecated that field was removed {@link https://github.com/temporalio/api/pull/393} */ public string $bundleId = '', public bool $useVersioning = false, ) { diff --git a/src/Internal/Mapper/WorkflowExecutionInfoMapper.php b/src/Internal/Mapper/WorkflowExecutionInfoMapper.php index 8dc7a093b..00ef71e33 100644 --- a/src/Internal/Mapper/WorkflowExecutionInfoMapper.php +++ b/src/Internal/Mapper/WorkflowExecutionInfoMapper.php @@ -64,7 +64,6 @@ public function prepareWorkerVersionStamp(?WorkerVersionStamp $versionStamp): ?W ? null : new WorkerVersionStampDto( buildId: $versionStamp->getBuildId(), - bundleId: $versionStamp->getBundleId(), useVersioning: $versionStamp->getUseVersioning(), ); } diff --git a/tests/Unit/Client/Mapper/WorkflowExecutionInfoMapperTestCase.php b/tests/Unit/Client/Mapper/WorkflowExecutionInfoMapperTestCase.php index b7d80a058..cd7c81401 100644 --- a/tests/Unit/Client/Mapper/WorkflowExecutionInfoMapperTestCase.php +++ b/tests/Unit/Client/Mapper/WorkflowExecutionInfoMapperTestCase.php @@ -64,7 +64,7 @@ public function testFromPayload(): void 'state_transition_count' => 1, 'history_size_bytes' => 1, 'most_recent_worker_version_stamp' => (new \Temporal\Api\Common\V1\WorkerVersionStamp()) - ->setBundleId('bundleId') + // ->setBundleId('bundleId') ->setBuildId('buildId') ->setUseVersioning(true), ]), @@ -94,7 +94,6 @@ public function testFromPayload(): void $this->assertTrue($info->autoResetPoints[0]->resettable); $this->assertSame('binaryChecksum', $info->autoResetPoints[0]->binaryChecksum); $this->assertSame(1, $info->historySizeBytes); - $this->assertSame('bundleId', $info->mostRecentWorkerVersionStamp->bundleId); $this->assertSame('buildId', $info->mostRecentWorkerVersionStamp->buildId); $this->assertTrue($info->mostRecentWorkerVersionStamp->useVersioning); } diff --git a/tests/Unit/DTO/WorkflowOptionsTestCase.php b/tests/Unit/DTO/WorkflowOptionsTestCase.php index afc8c0a5b..9737bf29a 100644 --- a/tests/Unit/DTO/WorkflowOptionsTestCase.php +++ b/tests/Unit/DTO/WorkflowOptionsTestCase.php @@ -39,6 +39,10 @@ public function testMarshalling(): void 'WorkflowStartDelay' => 0, 'WorkflowTaskTimeout' => 0, 'WorkflowIDReusePolicy' => 2, + 'WorkflowIdConflictPolicy' => [ + 'name' => 'Unspecified', + 'value' => 0, + ], 'RetryPolicy' => null, 'CronSchedule' => null, 'Memo' => null, From 14796cb8b688fbbb953132322956ee2520505140 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Thu, 11 Apr 2024 18:18:16 +0400 Subject: [PATCH 3/3] Update composer.json --- composer.json | 2 +- src/Client/Common/ServerCapabilities.php | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index f9b759565..25e784cf7 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "psr/log": "^2.0 || ^3.0", "ramsey/uuid": "^4.7", "react/promise": "^2.9", - "roadrunner-php/roadrunner-api-dto": "dev-update-dtos as 1.7.0", + "roadrunner-php/roadrunner-api-dto": "^1.7.0", "roadrunner-php/version-checker": "^1.0", "spiral/attributes": "^3.1.4", "spiral/roadrunner": "^2023.3.12 || ^2024.1", diff --git a/src/Client/Common/ServerCapabilities.php b/src/Client/Common/ServerCapabilities.php index cd889347b..19031940e 100644 --- a/src/Client/Common/ServerCapabilities.php +++ b/src/Client/Common/ServerCapabilities.php @@ -11,6 +11,9 @@ namespace Temporal\Client\Common; +/** + * @see \Temporal\Api\Workflowservice\V1\GetSystemInfoResponse\Capabilities + */ final class ServerCapabilities { /**