-
-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
[Decorator] Add component for callable decorators #58076
base: 7.2
Are you sure you want to change the base?
Conversation
724a9f9
to
03e909b
Compare
@94noni In principle, yes, but not by default for every service. It would require a middleware layer to hook into it before the method is called. Currently, only proxies can do that for services. Somehow, collect the class/method decorators for each service and call them from within? but not sure about performance though. |
✅ Added Framework integration and controller decoration support ✅ As well as a new Doctrine transaction decorator through #[Route('/tasks', methods: 'POST')]
class CreateTaskController
{
public function __construct(private TaskRepositoryInterface $repository)
{
}
#[Serialize(format: 'json')]
#[Transactional]
public function __invoke(#[MapRequestPayload] TaskPayload $payload): Task
{
$task = new Task($payload->description, $payload->dueDate);
$this->repository->add($task); // $this->entityManager->persist($task);
return $task;
}
} Keeping the controller code free of persistence and presentation deps. The This is just an example. There are more approaches and architectures where decorators can be useful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan on putting readonly
on each and every class. I'm not sure it does make much sense on services, it makes more sense on DTO/VOs to me. But that's just my two cents. 🙂
However, I like very much the idea behind this component. Thank you for proposing it!
3dc5c9c
to
8017d9c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New round 😄
src/Symfony/Bridge/Doctrine/Decorator/DoctrineTransactionDecorator.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bridge/Doctrine/Tests/Decorator/DoctrineTransactionDecoratorTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bridge/Doctrine/Tests/Decorator/DoctrineTransactionDecoratorTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bridge/Doctrine/Tests/Decorator/DoctrineTransactionDecoratorTest.php
Outdated
Show resolved
Hide resolved
I updated the PR description to include the Pros and Cons |
Interesting! If I understand correctly, I see another useful decorators like “memoize” to cache the result of a method. Looking forward to seeing more feedback 😉 |
29a47fc
to
b1a6ae6
Compare
The last commit simplifies decorator definition by allowing it to extend Example: #[\Attribute(\Attribute::TARGET_METHOD)]
class MyDecorator extends DecoratorAttribute implements DecoratorInterface
{
public function decorate(\Closure $func): \Closure
{
return function (mixed ...$args) use ($func): mixed
{
echo "Do something before\n";
$result = $func(...$args);
echo "Do something after\n";
return $result;
};
}
public function decoratedBy(): string
{
return self::class;
}
}
class GreetingController
{
#[MyDecorator]
public function sayHello(string $name): void
{
echo "Hello $name!\n";
}
} |
Well, we could also apply this simple approach for decorator services by implementing #[\Attribute(\Attribute::TARGET_METHOD)]
class MyDecorator extends DecoratorAttribute implements DecoratorInterface, ServiceSubscriberInterface
{
use ServiceMethodsSubscriberTrait;
public function __construct(public string $foo = 'bar')
{
}
public function decorate(\Closure $func, self $metadata = new self()): \Closure
{
// do something with $this->someDependency()
return function (mixed ...$args) use ($func, $metadata): mixed
{
echo "Do something with $metadata->foo before\n";
$result = $func(...$args);
echo "Do something with $metadata->foo after\n";
return $result;
};
}
public function decoratedBy(): string
{
return self::class;
}
#[SubscribedService]
protected function someDependency(): SomeDependency
{
return $this->container->get(__METHOD__);
}
}
class GreetingController
{
#[MyDecorator(foo: 'baz')]
public function sayHello(string $name): void
{
echo "Hello $name!\n";
}
} |
Status: Improving proposal design Still not happy with the way attribute options are passed through the decorator implementation, it feels a bit hacky and requires knowledge/documentation. parent::__construct(self::class, ['foo' => $foo]); The way this function decoration works will require knowledge to wrap the function call: return function (mixed ...$args) use ($func): mixed { // <-- not obvious you've to do it like this to wrap the func call
// return $func(...$args)
} I was looking into the Constraint/Validator design and I found it very similar to what I'd like to achieve here. So I'm going to make some changes to improve the DX. |
3540c1b
to
3b2dbc4
Compare
Proposal update for better DX, please recheck PR description. |
I had a need for this couple times, but it was always with passing inheritance checks. This component is not doing that, it doesn't try to create a class that follows the hierarchy of decorated class, does it? I'm doing that eg. at https://github.com/snc/SncRedisBundle/blob/c095a0dadc6fd263e54e064f82b9dc4c8e7f9823/src/Factory/PhpredisClientFactory.php#L320 (thanks to proxy-manager). Without this, component is going to be much less useful, as you can't inject it in place whatever else was being injected before without changing the place where it's getting injected. |
3738e84
to
4fb4628
Compare
d2a7c34
to
4e0e297
Compare
c2a80cf
to
333c3a5
Compare
@ostrolucky I understand that this might be a bit confusing at first, especially since we’re already familiar with a decoration method that addresses this kind of problem. However, this new component and its integration are intentionally designed to solve the decoration issue in a different way for two main reasons:
In other words, instead of decorating the service using inheritance (as we used to do by making the decorator aware of the target class and methods), this decoration method targets a generic callable. This means that multiple decorators can be applied to any callable, and these decorators can be reused without needing to know the details of the classes or methods they’re decorating. Of course, this approach is best suited for situations where this kind of decoration is beneficial, such as in handlers or controllers. |
I (at least I think so) like this! 🤩 #[CollectCacheTags]
#[Serialize(format: 'json')]
#[Validate]
public function __invoke(#[MapRequestPayload] TaskPayload $payload): Task Is there a way to combine these three to eg |
@RobinHoutevelts great idea! It’s definitely possible to add such a nice feature. In the meantime, I’m backporting it into yceruto/decorator-bundle with support for |
This is an attempt to solve the linked issue in a generic way, making it applicable in other areas where it might be useful as well.
Note
Inspired by Python decorators
This component implements the Decorator Pattern around any PHP callable, allowing you to:
It solves issues related to subclass explosion and inflexibility in inheritance-based designs. The pattern is useful for extending object behavior without modifying the original code, making it ideal for scenarios where different combinations of behaviors are needed.
The component contains two main building blocks that users must know:
DecoratorInterface
adapter (the decorator): contains the decoration implementation, essentially defining what should be done before and after the targeted callable is executed.DecoratorAttribute
: links a callable to a decorator and collects its options if needed.Example:
The
Debug
attribute holds all metadata needed for the linked decorator, which must be referenced using theDecoratorAttribute::decoratedBy()
method. By default, this method returnsstatic::class.'Decorator'
, following a convention similar to that of Constraint/Validator.Decorators can be nested, so the order in which you define them around the targeted callable really matters. This is a visual representation of multiple decorators around a function and how they wrap around each other:
In short, the closer a decorator is to the targeted callable, the higher its execution priority.
Decorators might require some options. To handle this, you can define them using the constructor method and add the metadata attribute as a new argument in your decorator implementation:
Final usage:
The component requires a middleware layer to wrap the targeted callable before it's actually called. So, after the framework integration, a service named
decorator.callable_decorator
->CallableDecorator
-> aliasing toDecoratorInterface
will be available with all collected decorator services tagged withdecorator
(seeDecoratorsPass
). Then, you'll do:callable
into multiple decoratorscallable
that implements many possible variants of behavior into several smallercallable
.Refer to the tests for more details on use cases and how decorators are executed in the final stage.
Important
Why create a new component? Because it can be implemented across various components of our Symfony project and application, such as HttpKernel/Controllers, Messenger/Handlers, and potentially other custom request/response mechanisms that contain a middleware layer.
TODO:
Cheers!