Skip to content

Commit 1ec2e61

Browse files
xepozzxepozz
authored
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
1 parent 9093e48 commit 1ec2e61

12 files changed

Lines changed: 194 additions & 24 deletions

File tree

‎phpunit.xml.dist‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
stopOnFailure="false"
1010
stopOnError="false"
1111
stderr="true"
12+
failOnDeprecation="true"
13+
failOnPhpunitDeprecation="true"
14+
stopOnDeprecation="true"
15+
displayDetailsOnPhpunitDeprecations="true"
1216
displayDetailsOnIncompleteTests="true"
1317
displayDetailsOnSkippedTests="true"
1418
displayDetailsOnTestsThatTriggerDeprecations="true"

‎psalm-baseline.xml‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,6 @@
140140
<code><![CDATA[$counter]]></code>
141141
<code><![CDATA[$runID]]></code>
142142
<code><![CDATA[$signal]]></code>
143-
<code><![CDATA[$workflow->getID()]]></code>
144-
<code><![CDATA[$workflow->getID()]]></code>
145143
</ArgumentTypeCoercion>
146144
<DeprecatedClass>
147145
<code><![CDATA[new AnnotationReader()]]></code>
@@ -588,9 +586,6 @@
588586
<InvalidReturnType>
589587
<code><![CDATA[T|null]]></code>
590588
</InvalidReturnType>
591-
<RedundantCondition>
592-
<code><![CDATA[$reflection]]></code>
593-
</RedundantCondition>
594589
</file>
595590
<file src="src/Internal/Declaration/Reader/ActivityReader.php">
596591
<ArgumentTypeCoercion>
@@ -1064,8 +1059,6 @@
10641059
</file>
10651060
<file src="src/Internal/Workflow/ActivityProxy.php">
10661061
<ArgumentTypeCoercion>
1067-
<code><![CDATA[$handler->getID()]]></code>
1068-
<code><![CDATA[$handler->getID()]]></code>
10691062
<code><![CDATA[$options]]></code>
10701063
<code><![CDATA[$options]]></code>
10711064
</ArgumentTypeCoercion>

‎src/Internal/Declaration/Prototype/Prototype.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
abstract class Prototype implements PrototypeInterface
1717
{
18+
/**
19+
* @param non-empty-string $name
20+
*/
1821
public function __construct(
1922
protected string $name,
2023
protected ?\ReflectionMethod $handler,
@@ -39,6 +42,9 @@ public static function find(string $class, string $method, RepositoryInterface $
3942
return null;
4043
}
4144

45+
/**
46+
* @return non-empty-string
47+
*/
4248
public function getID(): string
4349
{
4450
return $this->name;
@@ -58,7 +64,7 @@ private static function matchClass(PrototypeInterface $prototype, string $class)
5864
{
5965
$reflection = $prototype->getClass();
6066

61-
return $reflection && $reflection->getName() === \trim($class, '\\');
67+
return $reflection->getName() === \trim($class, '\\');
6268
}
6369

6470
private static function matchMethod(PrototypeInterface $prototype, string $method): bool

‎src/Internal/Declaration/Prototype/WorkflowPrototype.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ final class WorkflowPrototype extends Prototype
4646
private bool $hasInitializer = false;
4747
private VersioningBehavior $versioningBehavior;
4848

49+
/**
50+
* @param non-empty-string $name
51+
*/
4952
public function __construct(
5053
string $name,
5154
?\ReflectionMethod $handler,

‎src/Internal/Declaration/Reader/ActivityReader.php‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,6 @@ private function getMethodGroups(ClassNode $graph, \ReflectionMethod $root): arr
111111
$contextualRetry = $contextualRetry ? $retry->mergeWith($contextualRetry) : $retry;
112112
}
113113

114-
//
115-
// In the future, activity methods are available only in
116-
// those classes that contain the attribute:
117-
//
118-
// - #[ActivityInterface]
119-
// - #[LocalActivityInterface]
120-
//
121114
$interface = $this->reader->firstClassMetadata($ctx->getReflection(), ActivityInterface::class);
122115

123116
if ($interface === null) {

‎src/Internal/Workflow/ActivityProxy.php‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
namespace Temporal\Internal\Workflow;
1313

1414
use React\Promise\PromiseInterface;
15+
use Temporal\Activity\ActivityMethod;
1516
use Temporal\Activity\ActivityOptionsInterface;
17+
use Temporal\Worker\FeatureFlags;
1618
use Temporal\Interceptor\WorkflowOutboundCalls\ExecuteActivityInput;
1719
use Temporal\Interceptor\WorkflowOutboundCalls\ExecuteLocalActivityInput;
1820
use Temporal\Interceptor\WorkflowOutboundCallsInterceptor;
@@ -61,13 +63,24 @@ public function __construct(
6163
*/
6264
public function __call(string $method, array $args = []): PromiseInterface
6365
{
64-
$handler = $this->findPrototypeByHandlerNameOrFail($method);
65-
$type = $handler->getHandler()->getReturnType();
66-
$options = $this->options->mergeWith($handler->getMethodRetry());
66+
$prototype = $this->findPrototypeByHandlerNameOrFail($method);
67+
$type = $prototype->getHandler()->getReturnType();
68+
$options = $this->options->mergeWith($prototype->getMethodRetry());
6769

68-
$args = Reflection::orderArguments($handler->getHandler(), $args);
70+
$args = Reflection::orderArguments($prototype->getHandler(), $args);
6971

70-
return $handler->isLocalActivity()
72+
if (FeatureFlags::$warnOnActivityMethodWithoutAttribute && !$prototype->getHandler()->getAttributes(ActivityMethod::class, \ReflectionAttribute::IS_INSTANCEOF)) {
73+
\trigger_error(
74+
\sprintf(
75+
'Using implicit activity methods is deprecated. Explicitly mark activity method %s with #[%s] attribute instead.',
76+
$prototype->getHandler()->getDeclaringClass()->getName() . '::' . $method,
77+
ActivityMethod::class,
78+
),
79+
\E_USER_DEPRECATED,
80+
);
81+
}
82+
83+
return $prototype->isLocalActivity()
7184
// Run local activity through an interceptor pipeline
7285
? $this->callsInterceptor->with(
7386
fn(ExecuteLocalActivityInput $input): PromiseInterface => $this->ctx
@@ -77,11 +90,11 @@ public function __call(string $method, array $args = []): PromiseInterface
7790
'executeLocalActivity',
7891
)(
7992
new ExecuteLocalActivityInput(
80-
$handler->getID(),
93+
$prototype->getID(),
8194
$args,
8295
$options,
8396
$type,
84-
$handler->getHandler(),
97+
$prototype->getHandler(),
8598
)
8699
)
87100

@@ -94,11 +107,11 @@ public function __call(string $method, array $args = []): PromiseInterface
94107
'executeActivity',
95108
)(
96109
new ExecuteActivityInput(
97-
$handler->getID(),
110+
$prototype->getID(),
98111
$args,
99112
$options,
100113
$type,
101-
$handler->getHandler(),
114+
$prototype->getHandler(),
102115
)
103116
);
104117
}

‎src/Worker/FeatureFlags.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,12 @@ final class FeatureFlags
5656
* @since SDK 2.16.0
5757
*/
5858
public static bool $throwDestructMemorizedInstanceException = true;
59+
60+
/**
61+
* Trigger deprecation warning when activity methods are used without explicit #[ActivityMethod] attribute.
62+
*
63+
* @see ActivityMethod
64+
* @since SDK 2.17.0
65+
*/
66+
public static bool $warnOnActivityMethodWithoutAttribute = true;
5967
}

‎src/Workflow/WorkflowMethod.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,15 @@ final class WorkflowMethod
2929
* Be careful about names that contain special characters. These names can
3030
* be used as metric tags. And systems like prometheus ignore metrics which
3131
* have tags with unsupported characters.
32+
*
33+
* @var non-empty-string|null
3234
*/
3335
#[Immutable]
3436
public ?string $name = null;
3537

38+
/**
39+
* @param non-empty-string|null $name
40+
*/
3641
public function __construct(?string $name = null)
3742
{
3843
$this->name = $name;

‎testing/src/DeprecationCollector.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Temporal\Testing;
6+
7+
class DeprecationCollector
8+
{
9+
/** @var DeprecationMessage[] */
10+
private static array $deprecations = [];
11+
12+
public static function register(): void
13+
{
14+
\set_error_handler([self::class, 'handle'], E_USER_DEPRECATED);
15+
}
16+
17+
public static function handle(int $errno, string $message, string $file, int $line): bool
18+
{
19+
if ($errno !== E_USER_DEPRECATED) {
20+
return false;
21+
}
22+
23+
$trace = \debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
24+
25+
self::$deprecations[] = new DeprecationMessage($message, $file, $line, $trace);
26+
return true;
27+
}
28+
29+
public static function getAll(): array
30+
{
31+
return self::$deprecations;
32+
}
33+
}

‎testing/src/DeprecationMessage.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Temporal\Testing;
6+
7+
class DeprecationMessage
8+
{
9+
public function __construct(
10+
public readonly string $message,
11+
public readonly string $file,
12+
public readonly int $line,
13+
public readonly array $trace,
14+
) {}
15+
}

0 commit comments

Comments
 (0)