Skip to content

Commit 9f389af

Browse files
xepozzxepozz
committed
feat: produce deprecation when use activity method without the activity attribute
1 parent fde083d commit 9f389af

File tree

8 files changed

+170
-17
lines changed

8 files changed

+170
-17
lines changed

‎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"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ private static function matchClass(PrototypeInterface $prototype, string $class)
5858
{
5959
$reflection = $prototype->getClass();
6060

61-
return $reflection && $reflection->getName() === \trim($class, '\\');
61+
return $reflection->getName() === \trim($class, '\\');
6262
}
6363

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

‎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: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Temporal\Internal\Workflow;
1313

1414
use React\Promise\PromiseInterface;
15+
use Temporal\Activity\ActivityMethod;
1516
use Temporal\Activity\ActivityOptionsInterface;
1617
use Temporal\Interceptor\WorkflowOutboundCalls\ExecuteActivityInput;
1718
use Temporal\Interceptor\WorkflowOutboundCalls\ExecuteLocalActivityInput;
@@ -61,13 +62,24 @@ public function __construct(
6162
*/
6263
public function __call(string $method, array $args = []): PromiseInterface
6364
{
64-
$handler = $this->findPrototypeByHandlerNameOrFail($method);
65-
$type = $handler->getHandler()->getReturnType();
66-
$options = $this->options->mergeWith($handler->getMethodRetry());
65+
$prototype = $this->findPrototypeByHandlerNameOrFail($method);
66+
$type = $prototype->getHandler()->getReturnType();
67+
$options = $this->options->mergeWith($prototype->getMethodRetry());
6768

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

70-
return $handler->isLocalActivity()
71+
if (!$prototype->getHandler()->getAttributes(ActivityMethod::class, \ReflectionAttribute::IS_INSTANCEOF)) {
72+
\trigger_error(
73+
\sprintf(
74+
'Using implicit activity methods is deprecated. Explicitly mark activity method %s with #[%s] attribute instead.',
75+
$prototype->getHandler()->getDeclaringClass()->getName() . '::' . $method,
76+
ActivityMethod::class,
77+
),
78+
\E_USER_DEPRECATED,
79+
);
80+
}
81+
82+
return $prototype->isLocalActivity()
7183
// Run local activity through an interceptor pipeline
7284
? $this->callsInterceptor->with(
7385
fn(ExecuteLocalActivityInput $input): PromiseInterface => $this->ctx
@@ -77,11 +89,11 @@ public function __call(string $method, array $args = []): PromiseInterface
7789
'executeLocalActivity',
7890
)(
7991
new ExecuteLocalActivityInput(
80-
$handler->getID(),
92+
$prototype->getID(),
8193
$args,
8294
$options,
8395
$type,
84-
$handler->getHandler(),
96+
$prototype->getHandler(),
8597
)
8698
)
8799

@@ -94,11 +106,11 @@ public function __call(string $method, array $args = []): PromiseInterface
94106
'executeActivity',
95107
)(
96108
new ExecuteActivityInput(
97-
$handler->getID(),
109+
$prototype->getID(),
98110
$args,
99111
$options,
100112
$type,
101-
$handler->getHandler(),
113+
$prototype->getHandler(),
102114
)
103115
);
104116
}

‎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+
}

‎tests/Acceptance/App/RuntimeBuilder.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PHPUnit\Framework\Attributes\Test;
88
use Temporal\Activity\ActivityInterface;
99
use Temporal\DataConverter\PayloadConverterInterface;
10+
use Temporal\Testing\DeprecationCollector;
1011
use Temporal\Tests\Acceptance\App\Input\Command;
1112
use Temporal\Tests\Acceptance\App\Input\Feature;
1213
use Temporal\Tests\Acceptance\App\Runtime\State;
@@ -69,6 +70,8 @@ public static function createState(Command $command, string $workDir, iterable $
6970
public static function init(): void
7071
{
7172
\ini_set('display_errors', 'stderr');
73+
error_reporting(-1);
74+
DeprecationCollector::register();
7275
// Feature flags
7376
FeatureFlags::$workflowDeferredHandlerStart = true;
7477
FeatureFlags::$cancelAbandonedChildWorkflows = false;

‎tests/Acceptance/Extra/Activity/ActivityMethodTest.php‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Temporal\Tests\Acceptance\Extra\Activity\ActivityMethod;
6+
7+
use Temporal\Activity;
8+
use Temporal\Activity\ActivityMethod;
9+
use Temporal\Client\WorkflowStubInterface;
10+
use Temporal\Exception\Client\WorkflowFailedException;
11+
use Temporal\Testing\DeprecationCollector;
12+
use Temporal\Tests\Acceptance\App\Attribute\Stub;
13+
use Temporal\Tests\Acceptance\App\TestCase;
14+
use Temporal\Workflow;
15+
use Temporal\Workflow\WorkflowInterface;
16+
use Temporal\Workflow\WorkflowMethod;
17+
18+
class ActivityMethodTest extends TestCase
19+
{
20+
public function testMethodWithAttribute(
21+
#[Stub('Extra_Activity_ActivityMethod', args: ['method' => 'withAttribute'])]
22+
WorkflowStubInterface $stub,
23+
): void {
24+
$result = $stub->getResult('array');
25+
self::assertEquals(1, $result['result']);
26+
self::assertCount(0, $result['deprecations'], \print_r($result['deprecations'], true));
27+
}
28+
29+
public function testMethodWithoutAttribute(
30+
#[Stub('Extra_Activity_ActivityMethod', args: ['method' => 'withoutAttribute'])]
31+
WorkflowStubInterface $stub,
32+
): void {
33+
$result = $stub->getResult('array');
34+
self::assertEquals(2, $result['result']);
35+
self::assertCount(1, $result['deprecations']);
36+
self::assertEquals(
37+
\sprintf(
38+
'Using implicit activity methods is deprecated. Explicitly mark activity method %s with #[%s] attribute instead.',
39+
TestActivity::class . '::withoutAttribute',
40+
ActivityMethod::class,
41+
),
42+
$result['deprecations'][0]['message'],
43+
);
44+
}
45+
46+
public function testMagicMethodIsIgnored(
47+
#[Stub('Extra_Activity_ActivityMethod', args: ['method' => '__invoke'])]
48+
WorkflowStubInterface $stub,
49+
): void {
50+
$this->expectException(WorkflowFailedException::class);
51+
$stub->getResult(type: 'int');
52+
}
53+
}
54+
55+
56+
#[WorkflowInterface]
57+
class TestWorkflow
58+
{
59+
#[WorkflowMethod(name: "Extra_Activity_ActivityMethod")]
60+
public function handle(string $method)
61+
{
62+
$activityStub = Workflow::newActivityStub(
63+
TestActivity::class,
64+
Activity\ActivityOptions::new()->withScheduleToCloseTimeout(10),
65+
);
66+
$result = yield $activityStub->{$method}();
67+
68+
return [
69+
'result' => $result,
70+
'deprecations' => DeprecationCollector::getAll(),
71+
];
72+
}
73+
}
74+
75+
#[Activity\ActivityInterface(prefix: 'Extra_Activity_ActivityMethod.')]
76+
class TestActivity
77+
{
78+
#[ActivityMethod]
79+
public function withAttribute()
80+
{
81+
return 1;
82+
}
83+
84+
public function withoutAttribute()
85+
{
86+
return 2;
87+
}
88+
89+
public function __invoke()
90+
{
91+
return 3;
92+
}
93+
}

0 commit comments

Comments
 (0)