From a0860bb8c9e39cdf135237d73b5c35930ab38764 Mon Sep 17 00:00:00 2001 From: Dmitrii Derepko Date: Mon, 23 Feb 2026 08:16:03 +0400 Subject: [PATCH 1/2] feat: produce deprecation when use activity method without the activity attribute (#677) * feat: produce deprecation when use activity method without the activity attribute * feat: add feature flag * fix: make psalm pass * fix: update baseline --- phpunit.xml.dist | 4 + psalm-baseline.xml | 7 -- src/Common/TypedSearchAttributes.php | 4 +- .../Declaration/Prototype/Prototype.php | 8 +- .../Prototype/WorkflowPrototype.php | 3 + .../Declaration/Reader/ActivityReader.php | 7 -- src/Internal/Workflow/ActivityProxy.php | 31 +++++-- src/Worker/FeatureFlags.php | 8 ++ src/Workflow/WorkflowMethod.php | 5 + testing/src/DeprecationCollector.php | 33 +++++++ testing/src/DeprecationMessage.php | 15 +++ tests/Acceptance/App/RuntimeBuilder.php | 4 + .../Extra/Activity/ActivityMethodTest.php | 93 +++++++++++++++++++ 13 files changed, 196 insertions(+), 26 deletions(-) create mode 100644 testing/src/DeprecationCollector.php create mode 100644 testing/src/DeprecationMessage.php create mode 100644 tests/Acceptance/Extra/Activity/ActivityMethodTest.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 43bb60c76..9ad8ad21d 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -9,6 +9,10 @@ stopOnFailure="false" stopOnError="false" stderr="true" + failOnDeprecation="true" + failOnPhpunitDeprecation="true" + stopOnDeprecation="true" + displayDetailsOnPhpunitDeprecations="true" displayDetailsOnIncompleteTests="true" displayDetailsOnSkippedTests="true" displayDetailsOnTestsThatTriggerDeprecations="true" diff --git a/psalm-baseline.xml b/psalm-baseline.xml index fdae75730..10e3ad5fd 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -140,8 +140,6 @@ - getID()]]> - getID()]]> @@ -588,9 +586,6 @@ - - - @@ -1064,8 +1059,6 @@ - getID()]]> - getID()]]> diff --git a/src/Common/TypedSearchAttributes.php b/src/Common/TypedSearchAttributes.php index a3ee97155..99582892f 100644 --- a/src/Common/TypedSearchAttributes.php +++ b/src/Common/TypedSearchAttributes.php @@ -62,8 +62,8 @@ public static function fromJsonArray(array $array): self $collection = new \SplObjectStorage(); foreach ($array as $name => ['type' => $type, 'value' => $value]) { try { - $vt = ValueType::from($type); - $key = SearchAttributeKey::for($vt, $name); + $valueType = ValueType::from($type); + $key = SearchAttributeKey::for($valueType, $name); $collection->offsetSet($key, $key->valueSet($value)->value); } catch (\Throwable) { // Ignore invalid values. diff --git a/src/Internal/Declaration/Prototype/Prototype.php b/src/Internal/Declaration/Prototype/Prototype.php index bbf693679..6ed440f9e 100644 --- a/src/Internal/Declaration/Prototype/Prototype.php +++ b/src/Internal/Declaration/Prototype/Prototype.php @@ -15,6 +15,9 @@ abstract class Prototype implements PrototypeInterface { + /** + * @param non-empty-string $name + */ public function __construct( protected string $name, protected ?\ReflectionMethod $handler, @@ -39,6 +42,9 @@ public static function find(string $class, string $method, RepositoryInterface $ return null; } + /** + * @return non-empty-string + */ public function getID(): string { return $this->name; @@ -58,7 +64,7 @@ private static function matchClass(PrototypeInterface $prototype, string $class) { $reflection = $prototype->getClass(); - return $reflection && $reflection->getName() === \trim($class, '\\'); + return $reflection->getName() === \trim($class, '\\'); } private static function matchMethod(PrototypeInterface $prototype, string $method): bool diff --git a/src/Internal/Declaration/Prototype/WorkflowPrototype.php b/src/Internal/Declaration/Prototype/WorkflowPrototype.php index da5d310ac..cf47d7fae 100644 --- a/src/Internal/Declaration/Prototype/WorkflowPrototype.php +++ b/src/Internal/Declaration/Prototype/WorkflowPrototype.php @@ -46,6 +46,9 @@ final class WorkflowPrototype extends Prototype private bool $hasInitializer = false; private VersioningBehavior $versioningBehavior; + /** + * @param non-empty-string $name + */ public function __construct( string $name, ?\ReflectionMethod $handler, diff --git a/src/Internal/Declaration/Reader/ActivityReader.php b/src/Internal/Declaration/Reader/ActivityReader.php index 541fa7b57..c97c22a4c 100644 --- a/src/Internal/Declaration/Reader/ActivityReader.php +++ b/src/Internal/Declaration/Reader/ActivityReader.php @@ -111,13 +111,6 @@ private function getMethodGroups(ClassNode $graph, \ReflectionMethod $root): arr $contextualRetry = $contextualRetry ? $retry->mergeWith($contextualRetry) : $retry; } - // - // In the future, activity methods are available only in - // those classes that contain the attribute: - // - // - #[ActivityInterface] - // - #[LocalActivityInterface] - // $interface = $this->reader->firstClassMetadata($ctx->getReflection(), ActivityInterface::class); if ($interface === null) { diff --git a/src/Internal/Workflow/ActivityProxy.php b/src/Internal/Workflow/ActivityProxy.php index 22528988d..54017faaa 100644 --- a/src/Internal/Workflow/ActivityProxy.php +++ b/src/Internal/Workflow/ActivityProxy.php @@ -12,7 +12,9 @@ namespace Temporal\Internal\Workflow; use React\Promise\PromiseInterface; +use Temporal\Activity\ActivityMethod; use Temporal\Activity\ActivityOptionsInterface; +use Temporal\Worker\FeatureFlags; use Temporal\Interceptor\WorkflowOutboundCalls\ExecuteActivityInput; use Temporal\Interceptor\WorkflowOutboundCalls\ExecuteLocalActivityInput; use Temporal\Interceptor\WorkflowOutboundCallsInterceptor; @@ -61,13 +63,24 @@ public function __construct( */ public function __call(string $method, array $args = []): PromiseInterface { - $handler = $this->findPrototypeByHandlerNameOrFail($method); - $type = $handler->getHandler()->getReturnType(); - $options = $this->options->mergeWith($handler->getMethodRetry()); + $prototype = $this->findPrototypeByHandlerNameOrFail($method); + $type = $prototype->getHandler()->getReturnType(); + $options = $this->options->mergeWith($prototype->getMethodRetry()); - $args = Reflection::orderArguments($handler->getHandler(), $args); + $args = Reflection::orderArguments($prototype->getHandler(), $args); - return $handler->isLocalActivity() + if (FeatureFlags::$warnOnActivityMethodWithoutAttribute && !$prototype->getHandler()->getAttributes(ActivityMethod::class, \ReflectionAttribute::IS_INSTANCEOF)) { + \trigger_error( + \sprintf( + 'Using implicit activity methods is deprecated. Explicitly mark activity method %s with #[%s] attribute instead.', + $prototype->getHandler()->getDeclaringClass()->getName() . '::' . $method, + ActivityMethod::class, + ), + \E_USER_DEPRECATED, + ); + } + + return $prototype->isLocalActivity() // Run local activity through an interceptor pipeline ? $this->callsInterceptor->with( fn(ExecuteLocalActivityInput $input): PromiseInterface => $this->ctx @@ -77,11 +90,11 @@ public function __call(string $method, array $args = []): PromiseInterface 'executeLocalActivity', )( new ExecuteLocalActivityInput( - $handler->getID(), + $prototype->getID(), $args, $options, $type, - $handler->getHandler(), + $prototype->getHandler(), ) ) @@ -94,11 +107,11 @@ public function __call(string $method, array $args = []): PromiseInterface 'executeActivity', )( new ExecuteActivityInput( - $handler->getID(), + $prototype->getID(), $args, $options, $type, - $handler->getHandler(), + $prototype->getHandler(), ) ); } diff --git a/src/Worker/FeatureFlags.php b/src/Worker/FeatureFlags.php index 724298f42..d814149c8 100644 --- a/src/Worker/FeatureFlags.php +++ b/src/Worker/FeatureFlags.php @@ -56,4 +56,12 @@ final class FeatureFlags * @since SDK 2.16.0 */ public static bool $throwDestructMemorizedInstanceException = true; + + /** + * Trigger deprecation warning when activity methods are used without explicit #[ActivityMethod] attribute. + * + * @see ActivityMethod + * @since SDK 2.17.0 + */ + public static bool $warnOnActivityMethodWithoutAttribute = true; } diff --git a/src/Workflow/WorkflowMethod.php b/src/Workflow/WorkflowMethod.php index 1951937aa..4195f84b4 100644 --- a/src/Workflow/WorkflowMethod.php +++ b/src/Workflow/WorkflowMethod.php @@ -29,10 +29,15 @@ final class WorkflowMethod * Be careful about names that contain special characters. These names can * be used as metric tags. And systems like prometheus ignore metrics which * have tags with unsupported characters. + * + * @var non-empty-string|null */ #[Immutable] public ?string $name = null; + /** + * @param non-empty-string|null $name + */ public function __construct(?string $name = null) { $this->name = $name; diff --git a/testing/src/DeprecationCollector.php b/testing/src/DeprecationCollector.php new file mode 100644 index 000000000..4a49c5017 --- /dev/null +++ b/testing/src/DeprecationCollector.php @@ -0,0 +1,33 @@ + 'withAttribute'])] + WorkflowStubInterface $stub, + ): void { + $result = $stub->getResult('array'); + self::assertEquals(1, $result['result']); + self::assertCount(0, $result['deprecations'], \print_r($result['deprecations'], true)); + } + + public function testMethodWithoutAttribute( + #[Stub('Extra_Activity_ActivityMethod', args: ['method' => 'withoutAttribute'])] + WorkflowStubInterface $stub, + ): void { + $result = $stub->getResult('array'); + self::assertEquals(2, $result['result']); + self::assertCount(1, $result['deprecations']); + self::assertEquals( + \sprintf( + 'Using implicit activity methods is deprecated. Explicitly mark activity method %s with #[%s] attribute instead.', + TestActivity::class . '::withoutAttribute', + ActivityMethod::class, + ), + $result['deprecations'][0]['message'], + ); + } + + public function testMagicMethodIsIgnored( + #[Stub('Extra_Activity_ActivityMethod', args: ['method' => '__invoke'])] + WorkflowStubInterface $stub, + ): void { + $this->expectException(WorkflowFailedException::class); + $stub->getResult(type: 'int'); + } +} + + +#[WorkflowInterface] +class TestWorkflow +{ + #[WorkflowMethod(name: "Extra_Activity_ActivityMethod")] + public function handle(string $method) + { + $activityStub = Workflow::newActivityStub( + TestActivity::class, + Activity\ActivityOptions::new()->withScheduleToCloseTimeout(10), + ); + $result = yield $activityStub->{$method}(); + + return [ + 'result' => $result, + 'deprecations' => DeprecationCollector::getAll(), + ]; + } +} + +#[Activity\ActivityInterface(prefix: 'Extra_Activity_ActivityMethod.')] +class TestActivity +{ + #[ActivityMethod] + public function withAttribute() + { + return 1; + } + + public function withoutAttribute() + { + return 2; + } + + public function __invoke() + { + return 3; + } +} From bc8853a035de0fffb0527436b876a38d00e84368 Mon Sep 17 00:00:00 2001 From: Dmitriy Derepko Date: Wed, 25 Feb 2026 10:35:04 +0400 Subject: [PATCH 2/2] feat: Make search attribute type more lenient when parsing --- src/Common/SearchAttributes/ValueType.php | 26 ++++ src/Common/TypedSearchAttributes.php | 6 +- .../Common/TypedSearchAttributesTestCase.php | 127 ++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) diff --git a/src/Common/SearchAttributes/ValueType.php b/src/Common/SearchAttributes/ValueType.php index e04d8265b..7476ac3c6 100644 --- a/src/Common/SearchAttributes/ValueType.php +++ b/src/Common/SearchAttributes/ValueType.php @@ -16,4 +16,30 @@ enum ValueType: string case KeywordList = 'keyword_list'; case Text = 'string'; case Datetime = 'datetime'; + + /** + * Parse a type string leniently, accepting the canonical form (e.g. "keyword"), + * PascalCase form (e.g. "Keyword"), and SCREAMING_SNAKE_CASE proto enum form + * (e.g. "INDEXED_VALUE_TYPE_KEYWORD"). + */ + public static function fromMetadata(string $type): ?self + { + // Try canonical form first (e.g. "keyword", "int64") + $result = self::tryFrom($type); + if ($result !== null) { + return $result; + } + + // PascalCase and SCREAMING_SNAKE_CASE forms + return match ($type) { + 'Bool', 'INDEXED_VALUE_TYPE_BOOL' => self::Bool, + 'Double', 'INDEXED_VALUE_TYPE_DOUBLE' => self::Float, + 'Int', 'INDEXED_VALUE_TYPE_INT' => self::Int, + 'Keyword', 'INDEXED_VALUE_TYPE_KEYWORD' => self::Keyword, + 'KeywordList', 'INDEXED_VALUE_TYPE_KEYWORD_LIST' => self::KeywordList, + 'Text', 'INDEXED_VALUE_TYPE_TEXT' => self::Text, + 'Datetime', 'INDEXED_VALUE_TYPE_DATETIME' => self::Datetime, + default => null, + }; + } } diff --git a/src/Common/TypedSearchAttributes.php b/src/Common/TypedSearchAttributes.php index 99582892f..9e241f09a 100644 --- a/src/Common/TypedSearchAttributes.php +++ b/src/Common/TypedSearchAttributes.php @@ -62,7 +62,11 @@ public static function fromJsonArray(array $array): self $collection = new \SplObjectStorage(); foreach ($array as $name => ['type' => $type, 'value' => $value]) { try { - $valueType = ValueType::from($type); + $valueType = ValueType::fromMetadata($type); + if ($valueType === null) { + continue; + } + $key = SearchAttributeKey::for($valueType, $name); $collection->offsetSet($key, $key->valueSet($value)->value); } catch (\Throwable) { diff --git a/tests/Unit/Common/TypedSearchAttributesTestCase.php b/tests/Unit/Common/TypedSearchAttributesTestCase.php index 1cc1c17eb..c172ae005 100644 --- a/tests/Unit/Common/TypedSearchAttributesTestCase.php +++ b/tests/Unit/Common/TypedSearchAttributesTestCase.php @@ -5,6 +5,7 @@ namespace Temporal\Tests\Unit\Common; use Temporal\Common\SearchAttributes\SearchAttributeKey; +use Temporal\Common\SearchAttributes\ValueType; use Temporal\Common\TypedSearchAttributes; use PHPUnit\Framework\TestCase; @@ -196,6 +197,92 @@ public function testFromJsonArray(): void self::assertSame(['foo', 'bar'], $collection->get(SearchAttributeKey::forKeywordList('name8'))); } + public function testFromJsonArrayWithPascalCaseType(): void + { + $collection = TypedSearchAttributes::fromJsonArray([ + 'name1' => [ + 'type' => 'Bool', + 'value' => true, + ], + 'name2' => [ + 'type' => 'Int', + 'value' => 42, + ], + 'name3' => [ + 'type' => 'Double', + 'value' => 3.14, + ], + 'name4' => [ + 'type' => 'Keyword', + 'value' => 'bar', + ], + 'name5' => [ + 'type' => 'Text', + 'value' => 'foo', + ], + 'name6' => [ + 'type' => 'Datetime', + 'value' => '2021-01-01T00:00:00+00:00', + ], + 'name7' => [ + 'type' => 'KeywordList', + 'value' => ['foo', 'bar'], + ], + ]); + + self::assertCount(7, $collection); + self::assertTrue($collection->get(SearchAttributeKey::forBool('name1'))); + self::assertSame(42, $collection->get(SearchAttributeKey::forInteger('name2'))); + self::assertSame(3.14, $collection->get(SearchAttributeKey::forFloat('name3'))); + self::assertSame('bar', $collection->get(SearchAttributeKey::forKeyword('name4'))); + self::assertSame('foo', $collection->get(SearchAttributeKey::forText('name5'))); + self::assertInstanceOf(\DateTimeImmutable::class, $collection->get(SearchAttributeKey::forDatetime('name6'))); + self::assertSame(['foo', 'bar'], $collection->get(SearchAttributeKey::forKeywordList('name7'))); + } + + public function testFromJsonArrayWithScreamingSnakeCaseType(): void + { + $collection = TypedSearchAttributes::fromJsonArray([ + 'name1' => [ + 'type' => 'INDEXED_VALUE_TYPE_BOOL', + 'value' => true, + ], + 'name2' => [ + 'type' => 'INDEXED_VALUE_TYPE_INT', + 'value' => 42, + ], + 'name3' => [ + 'type' => 'INDEXED_VALUE_TYPE_DOUBLE', + 'value' => 3.14, + ], + 'name4' => [ + 'type' => 'INDEXED_VALUE_TYPE_KEYWORD', + 'value' => 'bar', + ], + 'name5' => [ + 'type' => 'INDEXED_VALUE_TYPE_TEXT', + 'value' => 'foo', + ], + 'name6' => [ + 'type' => 'INDEXED_VALUE_TYPE_DATETIME', + 'value' => '2021-01-01T00:00:00+00:00', + ], + 'name7' => [ + 'type' => 'INDEXED_VALUE_TYPE_KEYWORD_LIST', + 'value' => ['foo', 'bar'], + ], + ]); + + self::assertCount(7, $collection); + self::assertTrue($collection->get(SearchAttributeKey::forBool('name1'))); + self::assertSame(42, $collection->get(SearchAttributeKey::forInteger('name2'))); + self::assertSame(3.14, $collection->get(SearchAttributeKey::forFloat('name3'))); + self::assertSame('bar', $collection->get(SearchAttributeKey::forKeyword('name4'))); + self::assertSame('foo', $collection->get(SearchAttributeKey::forText('name5'))); + self::assertInstanceOf(\DateTimeImmutable::class, $collection->get(SearchAttributeKey::forDatetime('name6'))); + self::assertSame(['foo', 'bar'], $collection->get(SearchAttributeKey::forKeywordList('name7'))); + } + public function testFromUntypedCollection(): void { $collection = TypedSearchAttributes::fromCollection([ @@ -219,6 +306,46 @@ public function testFromUntypedCollection(): void self::assertSame(['foo', 'bar'], $collection->get(SearchAttributeKey::forKeywordList('name8'))); } + public function testValueTypeFromMetadataCanonical(): void + { + self::assertSame(ValueType::Bool, ValueType::fromMetadata('bool')); + self::assertSame(ValueType::Float, ValueType::fromMetadata('float64')); + self::assertSame(ValueType::Int, ValueType::fromMetadata('int64')); + self::assertSame(ValueType::Keyword, ValueType::fromMetadata('keyword')); + self::assertSame(ValueType::KeywordList, ValueType::fromMetadata('keyword_list')); + self::assertSame(ValueType::Text, ValueType::fromMetadata('string')); + self::assertSame(ValueType::Datetime, ValueType::fromMetadata('datetime')); + } + + public function testValueTypeFromMetadataPascalCase(): void + { + self::assertSame(ValueType::Bool, ValueType::fromMetadata('Bool')); + self::assertSame(ValueType::Float, ValueType::fromMetadata('Double')); + self::assertSame(ValueType::Int, ValueType::fromMetadata('Int')); + self::assertSame(ValueType::Keyword, ValueType::fromMetadata('Keyword')); + self::assertSame(ValueType::KeywordList, ValueType::fromMetadata('KeywordList')); + self::assertSame(ValueType::Text, ValueType::fromMetadata('Text')); + self::assertSame(ValueType::Datetime, ValueType::fromMetadata('Datetime')); + } + + public function testValueTypeFromMetadataScreamingSnakeCase(): void + { + self::assertSame(ValueType::Bool, ValueType::fromMetadata('INDEXED_VALUE_TYPE_BOOL')); + self::assertSame(ValueType::Float, ValueType::fromMetadata('INDEXED_VALUE_TYPE_DOUBLE')); + self::assertSame(ValueType::Int, ValueType::fromMetadata('INDEXED_VALUE_TYPE_INT')); + self::assertSame(ValueType::Keyword, ValueType::fromMetadata('INDEXED_VALUE_TYPE_KEYWORD')); + self::assertSame(ValueType::KeywordList, ValueType::fromMetadata('INDEXED_VALUE_TYPE_KEYWORD_LIST')); + self::assertSame(ValueType::Text, ValueType::fromMetadata('INDEXED_VALUE_TYPE_TEXT')); + self::assertSame(ValueType::Datetime, ValueType::fromMetadata('INDEXED_VALUE_TYPE_DATETIME')); + } + + public function testValueTypeFromMetadataUnknown(): void + { + self::assertNull(ValueType::fromMetadata('unknown')); + self::assertNull(ValueType::fromMetadata('')); + self::assertNull(ValueType::fromMetadata('INDEXED_VALUE_TYPE_UNKNOWN')); + } + public function testValues() { $collection = TypedSearchAttributes::empty()