Skip to content

use type info in argument resolver #814

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.

Sign up for GitHub

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

Open
DavidBadura wants to merge 1 commit into
base: 4.0.x
Choose a base branch
from
Open

use type info in argument resolver #814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 1 addition & 7 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,6 @@ parameters:
count: 1
path: tests/Integration/Subscription/SubscriptionTest.php

-
message: '#^Instantiated class Patchlevel\\EventSourcing\\Tests\\Integration\\Subscription\\MigrateAggregateToStreamStoreSubscriber not found\.$#'
identifier: class.notFound
count: 1
path: tests/Integration/Subscription/SubscriptionTest.php

-
message: '#^Parameter \#4 \$maxAttempts of class Patchlevel\\EventSourcing\\Subscription\\RetryStrategy\\ClockBasedRetryStrategy constructor expects int\<1, max\>, 0 given\.$#'
identifier: argument.type
Expand Down Expand Up @@ -433,7 +427,7 @@ parameters:
path: tests/Unit/Message/Translator/ReplaceEventTranslatorTest.php

-
message: '#^Method class@anonymous/tests/Unit/Metadata/Subscriber/AttributeSubscriberMetadataFactoryTest\.php\:236\:\:profileVisited\(\) has parameter \$message with no type specified\.$#'
message: '#^Method class@anonymous/tests/Unit/Metadata/Subscriber/AttributeSubscriberMetadataFactoryTest\.php\:234\:\:profileVisited\(\) has parameter \$message with no type specified\.$#'
identifier: missingType.parameter
count: 1
path: tests/Unit/Metadata/Subscriber/AttributeSubscriberMetadataFactoryTest.php
Expand Down
5 changes: 3 additions & 2 deletions src/Metadata/Subscriber/ArgumentMetadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@

namespace Patchlevel\EventSourcing\Metadata\Subscriber;

use Symfony\Component\TypeInfo\Type;

final class ArgumentMetadata
{
public function __construct(
public readonly string $name,
public readonly string $type,
public readonly bool $allowsNull = false,
public readonly Type $type,
) {
}
}
12 changes: 0 additions & 12 deletions src/Metadata/Subscriber/ArgumentTypeNotSupported.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,4 @@ public static function missingType(string $class, string $method, string $argume
),
);
}

public static function onlyNamedTypeSupported(string $class, string $method, string $argumentName): self
{
return new self(
sprintf(
'Argument type for method "%s" in class "%s" is not supported. Argument "%s" must not have a union or intersection type.',
$method,
$class,
$argumentName,
),
);
}
}
20 changes: 9 additions & 11 deletions src/Metadata/Subscriber/AttributeSubscriberMetadataFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,20 @@
use ReflectionAttribute;
use ReflectionClass;
use ReflectionMethod;
use ReflectionNamedType;
use Symfony\Component\TypeInfo\TypeResolver\TypeResolver;

use function array_key_exists;
use function count;

final class AttributeSubscriberMetadataFactory implements SubscriberMetadataFactory
{
private readonly TypeResolver $typeResolver;

public function __construct()
{
$this->typeResolver = TypeResolver::create();
}

/** @var array<class-string, SubscriberMetadata> */
private array $subscriberMetadata = [];

Expand Down Expand Up @@ -171,18 +178,9 @@ private function subscribeMethod(ReflectionMethod $method): SubscribeMethodMetad
);
}

if (!$type instanceof ReflectionNamedType) {
throw ArgumentTypeNotSupported::onlyNamedTypeSupported(
$method->getDeclaringClass()->getName(),
$method->getName(),
$parameter->getName(),
);
}

$arguments[] = new ArgumentMetadata(
$parameter->getName(),
$type->getName(),
$parameter->allowsNull(),
$this->typeResolver->resolve($type),
);
}

Expand Down
5 changes: 1 addition & 4 deletions src/Subscription/Subscriber/ArgumentResolver/EventArgumentResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
use Patchlevel\EventSourcing\Message\Message;
use Patchlevel\EventSourcing\Metadata\Subscriber\ArgumentMetadata;

use function class_exists;
use function is_a;

final class EventArgumentResolver implements ArgumentResolver
{
public function resolve(ArgumentMetadata $argument, Message $message): object
Expand All @@ -19,6 +16,6 @@ public function resolve(ArgumentMetadata $argument, Message $message): object

public function support(ArgumentMetadata $argument, string $eventClass): bool
{
return class_exists($argument->type) && is_a($eventClass, $argument->type, true);
return $argument->type->isIdentifiedBy($eventClass);
}
}
2 changes: 1 addition & 1 deletion src/Subscription/Subscriber/ArgumentResolver/LookupResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ public function resolve(ArgumentMetadata $argument, Message $message): Lookup

public function support(ArgumentMetadata $argument, string $eventClass): bool
{
return $argument->type === Lookup::class;
return $argument->type->isIdentifiedBy(Lookup::class);
}
}
2 changes: 1 addition & 1 deletion src/Subscription/Subscriber/ArgumentResolver/MessageArgumentResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ public function resolve(ArgumentMetadata $argument, Message $message): Message

public function support(ArgumentMetadata $argument, string $eventClass): bool
{
return $argument->type === Message::class;
return $argument->type->isIdentifiedBy(Message::class);
}
}
2 changes: 1 addition & 1 deletion src/Subscription/Subscriber/ArgumentResolver/RecordedOnArgumentResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ public function resolve(ArgumentMetadata $argument, Message $message): DateTimeI

public function support(ArgumentMetadata $argument, string $eventClass): bool
{
return $argument->type === DateTimeImmutable::class;
return $argument->type->isIdentifiedBy(DateTimeImmutable::class);
}
}
42 changes: 5 additions & 37 deletions tests/Unit/Metadata/Subscriber/AttributeSubscriberMetadataFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
use Patchlevel\EventSourcing\Tests\Unit\Fixture\ProfileVisited;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;
use Stringable;
use Symfony\Component\TypeInfo\Type;

#[CoversClass(AttributeSubscriberMetadataFactory::class)]
final class AttributeSubscriberMetadataFactoryTest extends TestCase
Expand Down Expand Up @@ -189,14 +189,14 @@ public function profileCreated(ProfileCreated $profileCreated, string $aggregate
[
ProfileVisited::class => new SubscribeMethodMetadata(
'profileVisited',
[new ArgumentMetadata('message', Message::class)],
[new ArgumentMetadata('message', Type::object(Message::class))],
),

ProfileCreated::class => new SubscribeMethodMetadata(
'profileCreated',
[
new ArgumentMetadata('profileCreated', ProfileCreated::class),
new ArgumentMetadata('aggregateId', 'string'),
new ArgumentMetadata('profileCreated', Type::object(ProfileCreated::class)),
new ArgumentMetadata('aggregateId', Type::string()),
],
),
],
Expand All @@ -220,7 +220,7 @@ public function profileVisited(ProfileVisited|null $message): void
self::assertEquals(
[
ProfileVisited::class => new SubscribeMethodMetadata('profileVisited', [
new ArgumentMetadata('message', ProfileVisited::class, true),
new ArgumentMetadata('message', Type::nullable(Type::object(ProfileVisited::class))),
]),
],
$metadata->subscribeMethods,
Expand All @@ -245,38 +245,6 @@ public function profileVisited($message): void
$metadataFactory->metadata($subscriber::class);
}

public function testUnionTypeNotSupported(): void
Copy link
Member

@DanielBadura DanielBadura Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are now supporting these cases, we should cover them, no?

Copy link
Member Author

@DavidBadura DavidBadura Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, that doesn't make sense, since we neither actively support it nor don't. We add the argument type to the argument metadata (this is tested), and the argument resolver decides whether something is supported (this is also tested). type info supports union types as well as collections, array shapes, etc. (we haven't tested this, we don't test the functionality of external code).

{
$this->expectException(ArgumentTypeNotSupported::class);

$subscriber = new #[Subscriber('foo', RunMode::FromBeginning)]
class {
#[Subscribe(ProfileVisited::class)]
public function profileVisited(ProfileVisited|ProfileCreated $event): void
{
}
};

$metadataFactory = new AttributeSubscriberMetadataFactory();
$metadataFactory->metadata($subscriber::class);
}

public function testIntersectionTypeNotSupported(): void
{
$this->expectException(ArgumentTypeNotSupported::class);

$subscriber = new #[Subscriber('foo', RunMode::FromBeginning)]
class {
#[Subscribe(ProfileVisited::class)]
public function profileVisited(ProfileVisited&Stringable $event): void
{
}
};

$metadataFactory = new AttributeSubscriberMetadataFactory();
$metadataFactory->metadata($subscriber::class);
}

public function testSubscribeAllWithExplicitSubscribeMethod(): void
{
$this->expectException(DuplicateSubscribeMethod::class);
Expand Down
3 changes: 0 additions & 3 deletions tests/Unit/Subscription/Engine/DefaultSubscriptionEngineTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@
use Patchlevel\EventSourcing\Attribute\Teardown;
use Patchlevel\EventSourcing\Message\Message;
use Patchlevel\EventSourcing\Store\ArrayStream;
use Patchlevel\EventSourcing\Store\Criteria\Criteria;
use Patchlevel\EventSourcing\Store\Criteria\FromIndexCriterion;
use Patchlevel\EventSourcing\Store\Store;
use Patchlevel\EventSourcing\Subscription\Cleanup\CleanupFailed;
use Patchlevel\EventSourcing\Subscription\Cleanup\CleanupTaskHandler;
use Patchlevel\EventSourcing\Subscription\Cleanup\Dbal\DropTableTask;
Expand Down
7 changes: 4 additions & 3 deletions tests/Unit/Subscription/Subscriber/ArgumentResolver/EventArgumentResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Patchlevel\EventSourcing\Tests\Unit\Fixture\ProfileVisited;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;
use Symfony\Component\TypeInfo\Type;

#[CoversClass(EventArgumentResolver::class)]
final class EventArgumentResolverTest extends TestCase
Expand All @@ -22,14 +23,14 @@ public function testSupport(): void

self::assertTrue(
$resolver->support(
new ArgumentMetadata('foo', ProfileCreated::class, false),
new ArgumentMetadata('foo', Type::object(ProfileCreated::class)),
ProfileCreated::class,
),
);

self::assertFalse(
$resolver->support(
new ArgumentMetadata('foo', ProfileVisited::class, false),
new ArgumentMetadata('foo', Type::object(ProfileVisited::class)),
ProfileCreated::class,
),
);
Expand All @@ -45,7 +46,7 @@ public function testResolve(): void
self::assertSame(
$event,
$resolver->resolve(
new ArgumentMetadata('foo', ProfileVisited::class, false),
new ArgumentMetadata('foo', Type::object(ProfileVisited::class)),
$message,
),
);
Expand Down
7 changes: 4 additions & 3 deletions tests/Unit/Subscription/Subscriber/ArgumentResolver/LookupResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Patchlevel\EventSourcing\Tests\Unit\Fixture\ProfileVisited;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;
use Symfony\Component\TypeInfo\Type;

#[CoversClass(LookupResolver::class)]
final class LookupResolverTest extends TestCase
Expand All @@ -29,14 +30,14 @@ public function testSupport(): void

self::assertTrue(
$resolver->support(
new ArgumentMetadata('lookup', Lookup::class, false),
new ArgumentMetadata('lookup', Type::object(Lookup::class)),
ProfileCreated::class,
),
);

self::assertFalse(
$resolver->support(
new ArgumentMetadata('foo', ProfileCreated::class, false),
new ArgumentMetadata('foo', Type::object(ProfileCreated::class)),
ProfileCreated::class,
),
);
Expand All @@ -56,7 +57,7 @@ public function testResolve(): void
);

$lookup = $resolver->resolve(
new ArgumentMetadata('foo', Lookup::class, false),
new ArgumentMetadata('foo', Type::object(Lookup::class)),
$message,
);

Expand Down
7 changes: 4 additions & 3 deletions tests/Unit/Subscription/Subscriber/ArgumentResolver/MessageArgumentResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;
use stdClass;
use Symfony\Component\TypeInfo\Type;

#[CoversClass(MessageArgumentResolver::class)]
final class MessageArgumentResolverTest extends TestCase
Expand All @@ -20,14 +21,14 @@ public function testSupport(): void

self::assertTrue(
$resolver->support(
new ArgumentMetadata('foo', Message::class, false),
new ArgumentMetadata('foo', Type::object(Message::class)),
'qux',
),
);

self::assertFalse(
$resolver->support(
new ArgumentMetadata('foo', 'bar', false),
new ArgumentMetadata('foo', Type::string()),
'qux',
),
);
Expand All @@ -41,7 +42,7 @@ public function testResolve(): void
self::assertSame(
$message,
$resolver->resolve(
new ArgumentMetadata('foo', Message::class, false),
new ArgumentMetadata('foo', Type::object(Message::class)),
$message,
),
);
Expand Down
9 changes: 5 additions & 4 deletions tests/Unit/Subscription/Subscriber/ArgumentResolver/RecordedOnArgumentResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;
use stdClass;
use Symfony\Component\TypeInfo\Type;

#[CoversClass(RecordedOnArgumentResolver::class)]
final class RecordedOnArgumentResolverTest extends TestCase
Expand All @@ -22,14 +23,14 @@ public function testSupport(): void

self::assertTrue(
$resolver->support(
new ArgumentMetadata('foo', DateTimeImmutable::class, false),
new ArgumentMetadata('foo', Type::object(DateTimeImmutable::class)),
'qux',
),
);

self::assertFalse(
$resolver->support(
new ArgumentMetadata('foo', 'bar', false),
new ArgumentMetadata('foo', Type::string()),
'qux',
),
);
Expand All @@ -47,7 +48,7 @@ public function testResolveFromAggregateHeader(): void
self::assertSame(
$date,
$resolver->resolve(
new ArgumentMetadata('foo', DateTimeImmutable::class, false),
new ArgumentMetadata('foo', Type::object(DateTimeImmutable::class)),
$message,
),
);
Expand All @@ -63,7 +64,7 @@ public function testResolveFromRecordedOnHeader(): void
self::assertSame(
$date,
$resolver->resolve(
new ArgumentMetadata('foo', DateTimeImmutable::class, false),
new ArgumentMetadata('foo', Type::object(DateTimeImmutable::class)),
$message,
),
);
Expand Down
Loading