diff --git a/src/Internal/Declaration/Graph/ClassNode.php b/src/Internal/Declaration/Graph/ClassNode.php index edeea1abb..ff86cf0a8 100644 --- a/src/Internal/Declaration/Graph/ClassNode.php +++ b/src/Internal/Declaration/Graph/ClassNode.php @@ -11,13 +11,10 @@ namespace Temporal\Internal\Declaration\Graph; +use Temporal\Tests\Workflow\Inheritance\ExtendingWorkflow; + final class ClassNode implements NodeInterface { - /** - * @var \ReflectionClass - */ - private \ReflectionClass $class; - /** * @var array|null */ @@ -26,17 +23,35 @@ final class ClassNode implements NodeInterface /** * @param \ReflectionClass $class */ - public function __construct(\ReflectionClass $class) + public function __construct( + private \ReflectionClass $class, + ) {} + + public function getReflection(): \ReflectionClass { - $this->class = $class; + return $this->class; } /** - * @return \ReflectionClass + * Get all methods from the class and its parents without duplicates. + * + * @return array + * + * @throws \ReflectionException */ - public function getReflection(): \ReflectionClass - { - return $this->class; + public function getAllMethods(): array { + /** @var array $result */ + $result = []; + + foreach ($this->getInheritance() as $classes) { + foreach ($classes as $class) { + foreach ($class->getReflection()->getMethods() as $method) { + $result[$method->getName()] ??= $method; + } + } + } + + return $result; } /** @@ -50,8 +65,9 @@ public function count(): int } /** - * @param string $name - * @param bool $reverse + * Get a method with all the declared classes. + * + * @param non-empty-string $name * @return \Traversable * @throws \ReflectionException */ diff --git a/src/Internal/Declaration/Instantiator/WorkflowInstantiator.php b/src/Internal/Declaration/Instantiator/WorkflowInstantiator.php index e26c60841..c6ac77857 100644 --- a/src/Internal/Declaration/Instantiator/WorkflowInstantiator.php +++ b/src/Internal/Declaration/Instantiator/WorkflowInstantiator.php @@ -48,11 +48,11 @@ public function instantiate(PrototypeInterface $prototype): WorkflowInstance */ protected function getInstance(PrototypeInterface $prototype): object { - $handler = $prototype->getHandler() ?? throw new InstantiationException(\sprintf( + $prototype->getHandler() ?? throw new InstantiationException(\sprintf( 'Unable to instantiate workflow "%s" without handler method', $prototype->getID(), )); - return $handler->getDeclaringClass()->newInstanceWithoutConstructor(); + return $prototype->getClass()->newInstanceWithoutConstructor(); } } diff --git a/src/Internal/Declaration/Reader/WorkflowReader.php b/src/Internal/Declaration/Reader/WorkflowReader.php index 1b5557ffa..5b8183f6f 100644 --- a/src/Internal/Declaration/Reader/WorkflowReader.php +++ b/src/Internal/Declaration/Reader/WorkflowReader.php @@ -107,9 +107,7 @@ public function fromObject(object $object): WorkflowPrototype */ protected function getWorkflowPrototypes(ClassNode $graph): \Traversable { - $class = $graph->getReflection(); - - foreach ($class->getMethods() as $reflection) { + foreach ($graph->getAllMethods() as $reflection) { if (!$this->isValidMethod($reflection)) { continue; } @@ -305,7 +303,7 @@ private function getPrototype(ClassNode $graph, \ReflectionMethod $handler): ?Wo // $contextualRetry = $previousRetry; - foreach ($group as $ctx => $method) { + foreach ($group as $method) { /** @var MethodRetry $retry */ $retry = $this->reader->firstFunctionMetadata($method, MethodRetry::class); @@ -335,15 +333,29 @@ private function getPrototype(ClassNode $graph, \ReflectionMethod $handler): ?Wo // // - #[WorkflowInterface] // - $interface = $this->reader->firstClassMetadata($ctx->getReflection(), WorkflowInterface::class); + /** @var \ReflectionClass|null $context */ + $interface = $context = null; + foreach ($graph->getIterator() as $edges) { + foreach ($edges as $node) { + $interface = $this->reader->firstClassMetadata( + $context = $node->getReflection(), + WorkflowInterface::class, + ); + + if ($interface !== null) { + break 2; + } + } + } // In case if ($interface === null) { continue; } + \assert($context !== null); if ($prototype === null) { - $prototype = $this->findProto($handler, $method); + $prototype = $this->findProto($handler, $method, $context, $graph->getReflection()); } if ($prototype !== null && $retry !== null) { @@ -371,15 +383,18 @@ private function getDefaultPrototype(ClassNode $graph): WorkflowPrototype } /** - * @param \ReflectionMethod $handler - * @param \ReflectionMethod $ctx + * @param \ReflectionMethod $handler First method in the inheritance chain + * @param \ReflectionMethod $ctx Current method in the inheritance chain + * @param \ReflectionClass $interface Class or Interface with #[WorkflowInterface] attribute + * @param \ReflectionClass $class Target class * @return WorkflowPrototype|null */ - private function findProto(\ReflectionMethod $handler, \ReflectionMethod $ctx): ?WorkflowPrototype - { - $reflection = $ctx->getDeclaringClass(); - - // + private function findProto( + \ReflectionMethod $handler, + \ReflectionMethod $ctx, + \ReflectionClass $interface, + \ReflectionClass $class, + ): ?WorkflowPrototype { // The name of the workflow handler must be generated based // method's name which can be redefined using #[WorkflowMethod] // attribute. @@ -418,8 +433,8 @@ private function findProto(\ReflectionMethod $handler, \ReflectionMethod $ctx): ); } - $name = $info->name ?? $reflection->getShortName(); + $name = $info->name ?? $interface->getShortName(); - return new WorkflowPrototype($name, $handler, $reflection); + return new WorkflowPrototype($name, $handler, $class); } } diff --git a/src/Internal/Workflow/Process/Process.php b/src/Internal/Workflow/Process/Process.php index 7b15a9dc1..bbf30e9aa 100644 --- a/src/Internal/Workflow/Process/Process.php +++ b/src/Internal/Workflow/Process/Process.php @@ -174,7 +174,7 @@ function (\Throwable $e): void { /** * @param \Closure(ValuesInterface): mixed $handler */ - public function start(\Closure $handler, ValuesInterface $values = null, bool $deferred): void + public function start(\Closure $handler, ?ValuesInterface $values, bool $deferred): void { try { $this->makeCurrent(); diff --git a/src/Internal/Workflow/Process/Scope.php b/src/Internal/Workflow/Process/Scope.php index 6fb022343..f36f2aeff 100644 --- a/src/Internal/Workflow/Process/Scope.php +++ b/src/Internal/Workflow/Process/Scope.php @@ -136,7 +136,7 @@ public function getContext(): WorkflowContext /** * @param \Closure(ValuesInterface): mixed $handler */ - public function start(\Closure $handler, ValuesInterface $values = null, bool $deferred): void + public function start(\Closure $handler, ?ValuesInterface $values, bool $deferred): void { // Create a coroutine generator $this->coroutine = $this->call($handler, $values ?? EncodedValues::empty()); diff --git a/tests/Fixtures/src/Workflow/Inheritance/BaseWorkflowWithHandler.php b/tests/Fixtures/src/Workflow/Inheritance/BaseWorkflowWithHandler.php new file mode 100644 index 000000000..4f3deb314 --- /dev/null +++ b/tests/Fixtures/src/Workflow/Inheritance/BaseWorkflowWithHandler.php @@ -0,0 +1,23 @@ +getResult(Type::TYPE_ARRAY) + $run->getResult(Type::TYPE_ARRAY, 3) ); } diff --git a/tests/Functional/Client/UntypedWorkflowStubTestCase.php b/tests/Functional/Client/UntypedWorkflowStubTestCase.php index dd6b5d202..f1188e09a 100644 --- a/tests/Functional/Client/UntypedWorkflowStubTestCase.php +++ b/tests/Functional/Client/UntypedWorkflowStubTestCase.php @@ -18,7 +18,6 @@ use Temporal\Exception\Failure\TerminatedFailure; use Temporal\Exception\IllegalStateException; use Temporal\Exception\InvalidArgumentException; -use Temporal\Tests\Unit\Declaration\Fixture\WorkflowWithoutHandler; use Temporal\Workflow\WorkflowExecutionStatus; /** @@ -264,7 +263,7 @@ public function testSignalRunningWorkflowWithInheritedSignal() $signaller = $client->newUntypedRunningWorkflowStub($workflowId, $workflowRunId); $signaller->signal('addValue', 'test1'); - $result = $workflowRun->getResult(); + $result = $workflowRun->getResult(timeout: 10); $this->assertEquals(['test1'], $result); } } diff --git a/tests/Functional/SimpleWorkflowTestCase.php b/tests/Functional/SimpleWorkflowTestCase.php index 1d9747229..5e6fb9583 100644 --- a/tests/Functional/SimpleWorkflowTestCase.php +++ b/tests/Functional/SimpleWorkflowTestCase.php @@ -10,6 +10,7 @@ use Temporal\Client\WorkflowOptions; use Temporal\Testing\ActivityMocker; use Temporal\Tests\TestCase; +use Temporal\Tests\Workflow\Inheritance\ExtendingWorkflow; use Temporal\Tests\Workflow\SimpleWorkflow; use Temporal\Tests\Workflow\YieldGeneratorWorkflow; use Temporal\Tests\Workflow\YieldScalarsWorkflow; @@ -119,6 +120,13 @@ public function testYieldGenerator(): void $this->assertSame('bar', $run->getResult()); } + public function testWorkflowMethodInAbstractParent(): void + { + $workflow = $this->workflowClient->newWorkflowStub(ExtendingWorkflow::class); + $run = $this->workflowClient->start($workflow); + $this->assertNull($run->getResult(timeout: 5)); + } + private function assertContainsEvent(WorkflowExecution $execution, int $event): void { $history = $this->workflowClient->getWorkflowHistory( diff --git a/tests/Functional/worker.php b/tests/Functional/worker.php index 08a3b2fd8..0f1a2816c 100644 --- a/tests/Functional/worker.php +++ b/tests/Functional/worker.php @@ -56,7 +56,12 @@ // register all workflows foreach ($getClasses(__DIR__ . '/../Fixtures/src/Workflow', 'Temporal\\Tests\\Workflow\\') as $class) { - if (class_exists($class) && !\interface_exists($class)) { + if (\class_exists($class) && !\interface_exists($class)) { + $wfRef = new \ReflectionClass($class); + if ($wfRef->isAbstract()) { + continue; + } + \array_walk( $workers, static fn (WorkerInterface $worker) => $worker->registerWorkflowTypes($class), diff --git a/tests/Unit/Declaration/Fixture/Inheritance/BaseWorkflowWithHandler.php b/tests/Unit/Declaration/Fixture/Inheritance/BaseWorkflowWithHandler.php new file mode 100644 index 000000000..a878bb480 --- /dev/null +++ b/tests/Unit/Declaration/Fixture/Inheritance/BaseWorkflowWithHandler.php @@ -0,0 +1,23 @@ +assertSame('ExampleWorkflowName', $prototype->getID()); } + + public function testHierarchicalWorkflow(): void + { + $instantiator = new WorkflowInstantiator(new \Temporal\Interceptor\SimplePipelineProvider()); + + $instance = $instantiator->instantiate( + new WorkflowPrototype( + 'extending', + new \ReflectionMethod(ExtendingWorkflow::class, 'handler'), + new \ReflectionClass(ExtendingWorkflow::class), + ), + ); + + $this->assertInstanceOf(ExtendingWorkflow::class, $instance->getContext()); + } + + public function testWorkflowWithInterface(): void + { + $reader = new WorkflowReader(new AttributeReader()); + + $result = $reader->fromClass(AggregatedWorkflowImpl::class); + + $this->assertSame('AggregatedWorkflow', $result->getID()); + } + + public function testInstantiateWorkflowWithInterface(): void + { + $instantiator = new WorkflowInstantiator(new \Temporal\Interceptor\SimplePipelineProvider()); + $reader = new WorkflowReader(new AttributeReader()); + $prototype = $reader->fromClass(AggregatedWorkflowImpl::class); + + $instance = $instantiator->instantiate($prototype); + + $this->assertInstanceOf(AggregatedWorkflowImpl::class, $instance->getContext()); + $this->assertSame('AggregatedWorkflow', $prototype->getID()); + } } diff --git a/tests/Unit/Declaration/WorkflowNegativeDeclarationTestCase.php b/tests/Unit/Declaration/WorkflowNegativeDeclarationTestCase.php index c96e627b0..a28fde97a 100644 --- a/tests/Unit/Declaration/WorkflowNegativeDeclarationTestCase.php +++ b/tests/Unit/Declaration/WorkflowNegativeDeclarationTestCase.php @@ -21,7 +21,6 @@ use Temporal\Tests\Unit\Declaration\Fixture\WorkflowWithMultipleMethods; use Temporal\Tests\Unit\Declaration\Fixture\WorkflowWithoutHandler; use Temporal\Workflow\WorkflowInterface; -use Temporal\Workflow\WorkflowMethod; /** * @group unit