Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn about unfinished handlers #488

Merged
merged 13 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
6 changes: 6 additions & 0 deletions src/FeatureFlags.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
2 changes: 2 additions & 0 deletions src/Interceptor/WorkflowInbound/SignalInput.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
70 changes: 21 additions & 49 deletions src/Internal/Client/WorkflowProxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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();
Expand All @@ -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
{
Expand All @@ -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;
}
}
9 changes: 7 additions & 2 deletions src/Internal/Declaration/Instance.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,7 +28,7 @@ abstract class Instance implements InstanceInterface

public function __construct(
Prototype $prototype,
protected readonly object $context,
protected object $context,
) {
$handler = $prototype->getHandler();

Expand Down Expand Up @@ -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);
}
}
20 changes: 20 additions & 0 deletions src/Internal/Declaration/Prototype/QueryDefinition.php
Original file line number Diff line number Diff line change
@@ -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,
) {}
}
22 changes: 22 additions & 0 deletions src/Internal/Declaration/Prototype/SignalDefinition.php
Original file line number Diff line number Diff line change
@@ -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,
) {}
}
23 changes: 23 additions & 0 deletions src/Internal/Declaration/Prototype/UpdateDefinition.php
Original file line number Diff line number Diff line change
@@ -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,
) {}
}
40 changes: 14 additions & 26 deletions src/Internal/Declaration/Prototype/WorkflowPrototype.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];

Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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
{
Expand Down
Loading
Loading