Merge pull request #443: fix Workflow hang in cases where a non-promi…

…se value is yielded
temporalio · May 27, 2024 · 337f6ee · 337f6ee
2 parents e6d9588 + 49c3f52
commit 337f6ee
Show file tree

Hide file tree

Showing 20 changed files with 115 additions and 158 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -509,9 +509,6 @@
     <InvalidDocblock>
       <code><![CDATA[final class ActivityInstantiator extends Instantiator]]></code>
     </InvalidDocblock>
-    <PossiblyNullArgument>
-      <code><![CDATA[$this->getInstance($prototype)]]></code>
-    </PossiblyNullArgument>
   </file>
   <file src="src/Internal/Declaration/Instantiator/Instantiator.php">
     <MissingTemplateParam>
@@ -527,12 +524,6 @@
     <InvalidDocblock>
       <code><![CDATA[final class WorkflowInstantiator extends Instantiator]]></code>
     </InvalidDocblock>
-    <PossiblyNullArgument>
-      <code><![CDATA[$this->getInstance($prototype)]]></code>
-    </PossiblyNullArgument>
-    <RedundantCondition>
-      <code><![CDATA[$class !== null]]></code>
-    </RedundantCondition>
   </file>
   <file src="src/Internal/Declaration/Prototype/Prototype.php">
     <InvalidReturnStatement>
@@ -595,12 +586,10 @@
     </InvalidPropertyAssignmentValue>
     <MissingClosureReturnType>
       <code><![CDATA[function (QueryInput $input) use ($fn) {]]></code>
-      <code><![CDATA[function (UpdateInput $input) use ($fn) {]]></code>
     </MissingClosureReturnType>
     <MoreSpecificImplementedParamType>
       <code><![CDATA[$handler]]></code>
       <code><![CDATA[$handler]]></code>
-      <code><![CDATA[$name]]></code>
     </MoreSpecificImplementedParamType>
     <PropertyNotSetInConstructor>
       <code><![CDATA[$queryExecutor]]></code>
@@ -615,7 +604,6 @@
   <file src="src/Internal/Declaration/WorkflowInstance/SignalQueue.php">
     <ArgumentTypeCoercion>
       <code><![CDATA[$signal]]></code>
-      <code><![CDATA[$signal]]></code>
     </ArgumentTypeCoercion>
     <MissingConstructor>
       <code><![CDATA[$onSignal]]></code>
@@ -752,13 +740,9 @@
     </PossiblyFalseArgument>
   </file>
   <file src="src/Internal/Marshaller/Type/ArrayType.php">
-    <DocblockTypeContradiction>
-      <code><![CDATA[\is_array($value)]]></code>
-    </DocblockTypeContradiction>
     <MoreSpecificImplementedParamType>
       <code><![CDATA[$current]]></code>
       <code><![CDATA[$value]]></code>
-      <code><![CDATA[$value]]></code>
     </MoreSpecificImplementedParamType>
   </file>
   <file src="src/Internal/Marshaller/Type/DateIntervalType.php">
@@ -1144,10 +1128,6 @@
       <code><![CDATA[findValidateUpdateHandler]]></code>
       <code><![CDATA[getUpdateHandlerNames]]></code>
     </UndefinedInterfaceMethod>
-    <UnusedVariable>
-      <code><![CDATA[$resolver]]></code>
-      <code><![CDATA[$resolver]]></code>
-    </UnusedVariable>
   </file>
   <file src="src/Internal/Transport/Router/StartWorkflow.php">
     <MissingClosureReturnType>

diff --git a/src/Internal/Declaration/Dispatcher/AutowiredPayloads.php b/src/Internal/Declaration/Dispatcher/AutowiredPayloads.php
@@ -16,15 +16,11 @@
 
 /**
  * @psalm-type FunctionExecutor = \Closure(object|null, array): mixed
+ * @internal
  */
 class AutowiredPayloads extends Dispatcher
 {
-    /**
-     * @param object|null $ctx
-     * @param ValuesInterface $values
-     * @return mixed
-     */
-    public function dispatchValues(?object $ctx, ValuesInterface $values)
+    public function dispatchValues(object $ctx, ValuesInterface $values): mixed
     {
         $arguments = [];
         for ($i = 0; $i < $values->count(); $i++) {

diff --git a/src/Internal/Declaration/Dispatcher/Dispatcher.php b/src/Internal/Declaration/Dispatcher/Dispatcher.php
@@ -15,7 +15,7 @@
 use ReflectionType;
 
 /**
- * @psalm-type FunctionExecutor = \Closure(object|null, array): mixed
+ * @psalm-type FunctionExecutor = \Closure(object, array): mixed
  */
 class Dispatcher implements DispatcherInterface
 {
@@ -30,8 +30,8 @@ class Dispatcher implements DispatcherInterface
     public const SCOPE_STATIC = 0x02;
 
     /**
+     * @var \Closure(object, array): mixed
      * @psalm-var FunctionExecutor
-     * @var \Closure
      */
     private \Closure $executor;
 
@@ -95,12 +95,7 @@ public function getArgumentTypes(): array
         return $this->types;
     }
 
-    /**
-     * @param object|null $ctx
-     * @param array $arguments
-     * @return mixed
-     */
-    public function dispatch(?object $ctx, array $arguments)
+    public function dispatch(object $ctx, array $arguments): mixed
     {
         return ($this->executor)($ctx, $arguments);
     }
@@ -119,13 +114,13 @@ private function scopeMatches(int $scope): bool
      * @psalm-return FunctionExecutor
      *
      * @param \ReflectionMethod $fun
-     * @return \Closure
+     * @return \Closure(object, array): mixed
      */
     private function createExecutorFromMethod(\ReflectionMethod $fun): \Closure
     {
-        return static function (?object $ctx, array $arguments) use ($fun) {
+        return static function (object $object, array $arguments) use ($fun) {
             try {
-                return $fun->invokeArgs($ctx, $arguments);
+                return $fun->invokeArgs($object, $arguments);
             } catch (\ReflectionException $e) {
                 throw new \BadMethodCallException($e->getMessage(), $e->getCode(), $e);
             }
@@ -136,15 +131,11 @@ private function createExecutorFromMethod(\ReflectionMethod $fun): \Closure
      * @psalm-return FunctionExecutor
      *
      * @param \ReflectionFunction $fun
-     * @return \Closure
+     * @return \Closure(object, array): mixed
      */
     private function createExecutorFromFunction(\ReflectionFunction $fun): \Closure
     {
-        return static function (?object $ctx, array $arguments) use ($fun) {
-            if ($ctx === null) {
-                return $fun->invoke(...$arguments);
-            }
-
+        return static function (object $ctx, array $arguments) use ($fun) {
             $closure = $fun->getClosure();
 
             try {

diff --git a/src/Internal/Declaration/Dispatcher/DispatcherInterface.php b/src/Internal/Declaration/Dispatcher/DispatcherInterface.php
@@ -11,14 +11,12 @@
 
 namespace Temporal\Internal\Declaration\Dispatcher;
 
+/**
+ * @internal
+ */
 interface DispatcherInterface
 {
-    /**
-     * @param object|null $ctx
-     * @param array $arguments
-     * @return mixed
-     */
-    public function dispatch(?object $ctx, array $arguments);
+    public function dispatch(object $ctx, array $arguments): mixed;
 
     /**
      * @return array<\ReflectionType>

diff --git a/src/Internal/Declaration/Instance.php b/src/Internal/Declaration/Instance.php
@@ -21,18 +21,15 @@
  */
 abstract class Instance implements InstanceInterface
 {
-    protected object $context;
     /**
      * @var \Closure(ValuesInterface): mixed
      */
     private \Closure $handler;
 
-    /**
-     * @param Prototype $prototype
-     * @param object $context
-     */
-    public function __construct(Prototype $prototype, object $context)
-    {
+    public function __construct(
+        Prototype $prototype,
+        protected readonly object $context,
+    ) {
         $handler = $prototype->getHandler();
 
         if ($handler === null) {
@@ -42,14 +39,10 @@ public function __construct(Prototype $prototype, object $context)
             ));
         }
 
-        $this->context = $context;
         $this->handler = $this->createHandler($handler);
     }
 
-    /**
-     * @return object|null
-     */
-    public function getContext(): ?object
+    public function getContext(): object
     {
         return $this->context;
     }

diff --git a/src/Internal/Declaration/InstanceInterface.php b/src/Internal/Declaration/InstanceInterface.php
@@ -23,8 +23,5 @@ interface InstanceInterface
      */
     public function getHandler(): callable;
 
-    /**
-     * @return object|null
-     */
-    public function getContext(): ?object;
+    public function getContext(): object;
 }
diff --git a/src/Internal/Declaration/Instantiator/Instantiator.php b/src/Internal/Declaration/Instantiator/Instantiator.php
@@ -17,24 +17,11 @@ abstract class Instantiator implements InstantiatorInterface
 {
     /**
      * @param PrototypeInterface $prototype
-     * @return \ReflectionClass|null
-     */
-    protected function getClass(PrototypeInterface $prototype): ?\ReflectionClass
-    {
-        return $prototype->getClass();
-    }
-
-    /**
-     * @param PrototypeInterface $prototype
-     * @return object|null
+     * @return object
      * @throws \ReflectionException
      */
-    protected function getInstance(PrototypeInterface $prototype): ?object
+    protected function getInstance(PrototypeInterface $prototype): object
     {
-        if ($class = $this->getClass($prototype)) {
-            return $class->newInstance();
-        }
-
-        return null;
+        return $prototype->getClass()->newInstance();
     }
 }
diff --git a/src/Internal/Declaration/Instantiator/WorkflowInstantiator.php b/src/Internal/Declaration/Instantiator/WorkflowInstantiator.php
@@ -12,6 +12,7 @@
 namespace Temporal\Internal\Declaration\Instantiator;
 
 use Temporal\Exception\InstantiationException;
+use Temporal\Interceptor\PipelineProvider;
 use Temporal\Interceptor\WorkflowInboundCallsInterceptor;
 use Temporal\Internal\Declaration\Prototype\PrototypeInterface;
 use Temporal\Internal\Declaration\Prototype\WorkflowPrototype;
@@ -23,7 +24,7 @@
 final class WorkflowInstantiator extends Instantiator
 {
     public function __construct(
-        private \Temporal\Interceptor\PipelineProvider $interceptorProvider,
+        private PipelineProvider $interceptorProvider,
     ) {
     }
 
@@ -43,26 +44,16 @@ public function instantiate(PrototypeInterface $prototype): WorkflowInstance
 
     /**
      * @param PrototypeInterface $prototype
-     * @return object|null
+     * @return object
      * @throws \ReflectionException
      */
-    protected function getInstance(PrototypeInterface $prototype): ?object
+    protected function getInstance(PrototypeInterface $prototype): object
     {
-        $handler = $prototype->getHandler();
+        $handler = $prototype->getHandler() ?? throw new InstantiationException(\sprintf(
+            'Unable to instantiate workflow "%s" without handler method',
+            $prototype->getID(),
+        ));
 
-        if ($handler === null) {
-            throw new InstantiationException(\sprintf(
-                'Unable to instantiate workflow "%s" without handler method',
-                $prototype->getID(),
-            ));
-        }
-
-        $class = $handler->getDeclaringClass();
-
-        if ($class !== null) {
-            return $class->newInstanceWithoutConstructor();
-        }
-
-        return null;
+        return $handler->getDeclaringClass()->newInstanceWithoutConstructor();
     }
 }
diff --git a/src/Internal/Declaration/WorkflowInstance.php b/src/Internal/Declaration/WorkflowInstance.php
@@ -11,22 +11,22 @@
 
 namespace Temporal\Internal\Declaration;
 
+use React\Promise\PromiseInterface;
 use Temporal\DataConverter\ValuesInterface;
 use Temporal\Interceptor\WorkflowInbound\QueryInput;
 use Temporal\Interceptor\WorkflowInbound\UpdateInput;
 use Temporal\Interceptor\WorkflowInboundCallsInterceptor;
 use Temporal\Internal\Declaration\Prototype\WorkflowPrototype;
 use Temporal\Internal\Declaration\WorkflowInstance\SignalQueue;
-use Temporal\Internal\Declaration\WorkflowInstance\UpdateQueue;
 use Temporal\Internal\Interceptor;
 
 /**
  * @psalm-import-type DispatchableHandler from InstanceInterface
  * @psalm-type QueryHandler = \Closure(QueryInput): mixed
- * @psalm-type UpdateHandler = \Closure(UpdateInput): mixed
+ * @psalm-type UpdateHandler = \Closure(UpdateInput): PromiseInterface
  * @psalm-type ValidateUpdateHandler = \Closure(UpdateInput): void
  * @psalm-type QueryExecutor = \Closure(QueryInput, callable(ValuesInterface): mixed): mixed
- * @psalm-type UpdateExecutor = \Closure(UpdateInput, callable(ValuesInterface): mixed): mixed
+ * @psalm-type UpdateExecutor = \Closure(UpdateInput, callable(ValuesInterface): mixed): PromiseInterface
  * @psalm-type ValidateUpdateExecutor = \Closure(UpdateInput, callable(ValuesInterface): mixed): mixed
  * @psalm-type UpdateValidator = \Closure(UpdateInput, UpdateHandler): void
  */
@@ -65,7 +65,7 @@ final class WorkflowInstance extends Instance implements WorkflowInstanceInterfa
 
     /**
      * @param WorkflowPrototype $prototype
-     * @param object $context
+     * @param object $context Workflow object
      * @param Interceptor\Pipeline<WorkflowInboundCallsInterceptor, mixed> $pipeline
      */
     public function __construct(
@@ -141,7 +141,7 @@ public function setUpdateValidator(\Closure $validator): self
      */
     public function initConstructor(): void
     {
-        if (method_exists($this->context, '__construct')) {
+        if (\method_exists($this->context, '__construct')) {
             $this->context->__construct();
         }
     }
@@ -156,8 +156,8 @@ public function getSignalQueue(): SignalQueue
 
     /**
      * @param non-empty-string $name
-     * @return null|\Closure(ValuesInterface):mixed
      *
+     * @return null|\Closure(QueryInput): mixed
      * @psalm-return QueryHandler|null
      */
     public function findQueryHandler(string $name): ?\Closure
@@ -166,8 +166,10 @@ public function findQueryHandler(string $name): ?\Closure
     }
 
     /**
-     * @param string $name
-     * @return \Closure
+     * @param non-empty-string $name
+     *
+     * @return null|\Closure(UpdateInput): PromiseInterface
+     * @psalm-return UpdateHandler|null
      */
     public function findUpdateHandler(string $name): ?\Closure
     {
@@ -176,6 +178,9 @@ public function findUpdateHandler(string $name): ?\Closure
 
     /**
      * @param non-empty-string $name
+     *
+     * @return null|\Closure(UpdateInput): void
+     * @psalm-return ValidateUpdateHandler|null
      */
     public function findValidateUpdateHandler(string $name): ?\Closure
     {
@@ -234,10 +239,6 @@ public function getUpdateHandlerNames(): array
         return \array_keys($this->updateHandlers);
     }
 
-    /**
-     * @param string $name
-     * @return \Closure
-     */
     public function getSignalHandler(string $name): \Closure
     {
         return fn (ValuesInterface $values) => $this->signalQueue->push($name, $values);

diff --git a/src/Internal/Declaration/WorkflowInstance/SignalQueue.php b/src/Internal/Declaration/WorkflowInstance/SignalQueue.php
@@ -36,7 +36,7 @@ final class SignalQueue
     private $onSignal;
 
     /**
-     * @param string $signal
+     * @param non-empty-string $signal
      * @param ValuesInterface $values
      */
     public function push(string $signal, ValuesInterface $values): void