Merge pull request #488: Warn about unfinished handlers

psalm-baseline.xml

@@ -533,11 +533,6 @@
       <code><![CDATA[$reflection]]></code>
     </RedundantCondition>
   </file>
-  <file src="src/Internal/Declaration/Prototype/WorkflowPrototype.php">
-    <PropertyTypeCoercion>
-      <code><![CDATA[$this->queryHandlers]]></code>
-    </PropertyTypeCoercion>
-  </file>
   <file src="src/Internal/Declaration/Reader/ActivityReader.php">
     <ArgumentTypeCoercion>
       <code><![CDATA[$contextualRetry]]></code>
@@ -558,7 +553,6 @@
       <code><![CDATA[$method]]></code>
       <code><![CDATA[$name]]></code>
       <code><![CDATA[$retry]]></code>
-      <code><![CDATA[$signal->name ?? $ctx->getName()]]></code>
     </ArgumentTypeCoercion>
     <InvalidArgument>
       <code><![CDATA[\reset($prototypes)]]></code>

src/FeatureFlags.php

@@ -21,4 +21,10 @@ final class FeatureFlags
      * @link https://github.com/temporalio/sdk-php/issues/457
      */
     public static bool $workflowDeferredHandlerStart = false;
+
+    /**
+     * Warn about running Signal and Update handlers on Workflow finish.
+     * It uses `error_log()` function to output a warning message.
+     */
+    public static bool $warnOnWorkflowUnfinishedHandlers = true;
 }

src/Interceptor/WorkflowInbound/SignalInput.php

@@ -19,6 +19,8 @@
 class SignalInput
 {
     /**
+     * @param non-empty-string $signalName
+     *
      * @no-named-arguments
      * @internal Don't use the constructor. Use {@see self::with()} instead.
      */

src/Internal/Client/WorkflowProxy.php

@@ -24,28 +24,24 @@
 
 /**
  * @template-covariant T of object
+ * @internal
  */
 final class WorkflowProxy extends Proxy
 {
     private const ERROR_UNDEFINED_METHOD =
         'The given workflow class "%s" does not contain a workflow, query or signal method named "%s"';
 
-    /**
-     * @param WorkflowClient $client
-     * @param WorkflowStubInterface $stub
-     * @param WorkflowPrototype $prototype
-     */
     public function __construct(
         public WorkflowClient $client,
         private readonly WorkflowStubInterface $stub,
         private readonly WorkflowPrototype $prototype,
-    ) {
-    }
+    ) {}
 
     /**
-     * @param string $method
-     * @param array $args
+     * @param non-empty-string $method
      * @return mixed|void
+     *
+     * @psalm-suppress MoreSpecificImplementedParamType
      */
     public function __call(string $method, array $args)
     {
@@ -60,43 +56,37 @@ public function __call(string $method, array $args)
             return $this->client
                 ->start($this, ...$args)
                 ->getResult(
-                    type: $returnType !== null ? Type::create($returnType) : null
+                    type: $returnType !== null ? Type::create($returnType) : null,
                 );
         }
 
         // Otherwise, we try to find a suitable workflow "query" method.
-        foreach ($this->prototype->getQueryHandlers() as $name => $query) {
-            if ($query->getName() === $method) {
-                $args = Reflection::orderArguments($query, $args);
+        foreach ($this->prototype->getQueryHandlers() as $name => $definition) {
+            if ($definition->method->getName() === $method) {
+                $args = Reflection::orderArguments($definition->method, $args);
 
-                return $this->stub->query($name, ...$args)?->getValue(0, $query->getReturnType());
+                return $this->stub->query($name, ...$args)?->getValue(0, $definition->returnType);
             }
         }
 
         // Otherwise, we try to find a suitable workflow "signal" method.
-        foreach ($this->prototype->getSignalHandlers() as $name => $signal) {
-            if ($signal->getName() === $method) {
-                $args = Reflection::orderArguments($signal, $args);
-
+        foreach ($this->prototype->getSignalHandlers() as $name => $definition) {
+            if ($definition->method->getName() === $method) {
+                $args = Reflection::orderArguments($definition->method, $args);
                 $this->stub->signal($name, ...$args);
 
                 return;
             }
         }
 
         // Otherwise, we try to find a suitable workflow "update" method.
-        foreach ($this->prototype->getUpdateHandlers() as $name => $update) {
-            if ($update->getName() === $method) {
-                $args = Reflection::orderArguments($update, $args);
-                $attrs = $update->getAttributes(ReturnType::class);
-
-                $returnType = \array_key_exists(0, $attrs)
-                    ? $attrs[0]->newInstance()
-                    : $update->getReturnType();
+        foreach ($this->prototype->getUpdateHandlers() as $name => $definition) {
+            if ($definition->method->getName() === $method) {
+                $args = Reflection::orderArguments($definition->method, $args);
 
                 $options = UpdateOptions::new($name)
                     ->withUpdateName($name)
-                    ->withResultType($returnType)
+                    ->withResultType($definition->returnType)
                     ->withWaitPolicy(WaitPolicy::new()->withLifecycleStage(LifecycleStage::StageCompleted));
 
                 return $this->stub->startUpdate($options, ...$args)->getResult();
@@ -106,15 +96,12 @@ public function __call(string $method, array $args)
         $class = $this->prototype->getClass();
 
         throw new \BadMethodCallException(
-            \sprintf(self::ERROR_UNDEFINED_METHOD, $class->getName(), $method)
+            \sprintf(self::ERROR_UNDEFINED_METHOD, $class->getName(), $method),
         );
     }
 
     /**
      * TODO rename: Method names cannot use underscore (PSR conflict)
-     *
-     * @return WorkflowStubInterface
-     * @internal
      */
     public function __getUntypedStub(): WorkflowStubInterface
     {
@@ -123,47 +110,32 @@ public function __getUntypedStub(): WorkflowStubInterface
 
     /**
      * TODO rename: Method names cannot use underscore (PSR conflict)
-     *
-     * @return ReturnType|null
-     * @internal
      */
     public function __getReturnType(): ?ReturnType
     {
         return $this->prototype->getReturnType();
     }
 
-    /**
-     * @return bool
-     * @internal
-     */
     public function hasHandler(): bool
     {
         return $this->prototype->getHandler() !== null;
     }
 
     /**
      * @return \ReflectionMethod
-     * @internal
      */
     public function getHandlerReflection(): \ReflectionMethod
     {
         return $this->prototype->getHandler() ?? throw new \LogicException(
-            'The workflow does not contain a handler method.'
+            'The workflow does not contain a handler method.',
         );
     }
 
     /**
      * @param non-empty-string $name Signal name
-     * @return \ReflectionFunctionAbstract|null
      */
-    public function findSignalReflection(string $name): ?\ReflectionFunctionAbstract
+    public function findSignalReflection(string $name): ?\ReflectionMethod
     {
-        foreach ($this->prototype->getSignalHandlers() as $method => $reflection) {
-            if ($method === $name) {
-                return $reflection;
-            }
-        }
-
-        return null;
+        return ($this->prototype->getSignalHandlers()[$name] ?? null)?->method;
     }
 }

src/Internal/Declaration/Instance.php

@@ -19,7 +19,7 @@
 /**
  * @psalm-import-type DispatchableHandler from InstanceInterface
  */
-abstract class Instance implements InstanceInterface
+abstract class Instance implements InstanceInterface, Destroyable
 {
     /**
      * @var \Closure(ValuesInterface): mixed
@@ -28,7 +28,7 @@ abstract class Instance implements InstanceInterface
 
     public function __construct(
         Prototype $prototype,
-        protected readonly object $context,
+        protected object $context,
     ) {
         $handler = $prototype->getHandler();
 
@@ -68,4 +68,9 @@ protected function createHandler(\ReflectionFunctionAbstract $func): \Closure
         $context = $this->context;
         return static fn (ValuesInterface $values): mixed => $valueMapper->dispatchValues($context, $values);
     }
+
+    public function destroy(): void
+    {
+        unset($this->handler, $this->context);
+    }
 }

src/Internal/Declaration/Prototype/QueryDefinition.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Temporal\Internal\Declaration\Prototype;
+
+/**
+ * @internal
+ */
+final class QueryDefinition
+{
+    /**
+     * @param non-empty-string $name
+     */
+    public function __construct(
+        public readonly string $name,
+        public readonly mixed $returnType,
+        public readonly \ReflectionMethod $method,
+    ) {}
+}

src/Internal/Declaration/Prototype/SignalDefinition.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Temporal\Internal\Declaration\Prototype;
+
+use Temporal\Workflow\HandlerUnfinishedPolicy;
+
+/**
+ * @internal
+ */
+final class SignalDefinition
+{
+    /**
+     * @param non-empty-string $name
+     */
+    public function __construct(
+        public readonly string $name,
+        public readonly HandlerUnfinishedPolicy $policy,
+        public readonly \ReflectionMethod $method,
+    ) {}
+}

src/Internal/Declaration/Prototype/UpdateDefinition.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Temporal\Internal\Declaration\Prototype;
+
+use Temporal\Workflow\HandlerUnfinishedPolicy;
+
+/**
+ * @internal
+ */
+final class UpdateDefinition
+{
+    /**
+     * @param non-empty-string $name
+     */
+    public function __construct(
+        public readonly string $name,
+        public readonly HandlerUnfinishedPolicy $policy,
+        public readonly mixed $returnType,
+        public readonly \ReflectionMethod $method,
+    ) {}
+}

src/Internal/Declaration/Prototype/WorkflowPrototype.php

@@ -18,17 +18,17 @@
 final class WorkflowPrototype extends Prototype
 {
     /**
-     * @var array<non-empty-string, \ReflectionFunctionAbstract>
+     * @var array<non-empty-string, QueryDefinition>
      */
     private array $queryHandlers = [];
 
     /**
-     * @var array<non-empty-string, \ReflectionFunctionAbstract>
+     * @var array<non-empty-string, SignalDefinition>
      */
     private array $signalHandlers = [];
 
     /**
-     * @var array<non-empty-string, \ReflectionFunctionAbstract>
+     * @var array<non-empty-string, UpdateDefinition>
      */
     private array $updateHandlers = [];
 
@@ -100,47 +100,35 @@ public function setReturnType(?ReturnType $attribute): void
         $this->returnType = $attribute;
     }
 
-    /**
-     * @param string $name
-     * @param \ReflectionFunctionAbstract $fun
-     */
-    public function addQueryHandler(string $name, \ReflectionFunctionAbstract $fun): void
+    public function addQueryHandler(QueryDefinition $definition): void
     {
-        $this->queryHandlers[$name] = $fun;
+        $this->queryHandlers[$definition->name] = $definition;
     }
 
     /**
-     * @return iterable<non-empty-string, \ReflectionFunctionAbstract>
+     * @return array<non-empty-string, QueryDefinition>
      */
-    public function getQueryHandlers(): iterable
+    public function getQueryHandlers(): array
     {
         return $this->queryHandlers;
     }
 
-    /**
-     * @param non-empty-string $name
-     * @param \ReflectionFunctionAbstract $fun
-     */
-    public function addSignalHandler(string $name, \ReflectionFunctionAbstract $fun): void
+    public function addSignalHandler(SignalDefinition $definition): void
     {
-        $this->signalHandlers[$name] = $fun;
+        $this->signalHandlers[$definition->name] = $definition;
     }
 
     /**
-     * @return iterable<non-empty-string, \ReflectionFunctionAbstract>
+     * @return array<non-empty-string, SignalDefinition>
      */
-    public function getSignalHandlers(): iterable
+    public function getSignalHandlers(): array
     {
         return $this->signalHandlers;
     }
 
-    /**
-     * @param non-empty-string $name
-     * @param \ReflectionFunctionAbstract $fun
-     */
-    public function addUpdateHandler(string $name, \ReflectionFunctionAbstract $fun): void
+    public function addUpdateHandler(UpdateDefinition $definition): void
     {
-        $this->updateHandlers[$name] = $fun;
+        $this->updateHandlers[$definition->name] = $definition;
     }
 
     /**
@@ -153,7 +141,7 @@ public function addValidateUpdateHandler(string $name, \ReflectionFunctionAbstra
     }
 
     /**
-     * @return array<non-empty-string, \ReflectionFunctionAbstract>
+     * @return array<non-empty-string, UpdateDefinition>
      */
     public function getUpdateHandlers(): array
     {