-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
[RFC]: Delegator factories for alias and service name #168
Comments
Hm, I worked on this quite a lot and had some serious problems which I try to summarize here: internal service caching
Example: use Laminas\ServiceManager\Factory\InvokableFactory;
use Laminas\ServiceManager\ServiceManager;
class Service
{}
class DelegatedService extends Service
{}
$config = [
'factories' => [
'service' => InvokableFactory::class,
],
'aliases' => [
'alias' => 'service',
],
'delegators' => [
'alias' => [
fn () => new DelegatedService(),
],
],
];
$sm = new ServiceManager($config);
$service = $sm->get('service');
$serviceViaAlias = $sm->get('alias');
assert($service === $serviceViaAlias); // passes
assert( !$serviceViaAlias instanceof DelegatedService); // passes as the `service` request put the requested service to internal cache service requestShould services requested by their service name (not the alias) also apply all delegators of its aliases? <?php
use Laminas\ServiceManager\Factory\InvokableFactory;
use Laminas\ServiceManager\ServiceManager;
class Service
{}
class DelegatedService extends Service
{
public function __construct(public readonly int $alias)
{}
}
$config = [
'factories' => [
'service' => InvokableFactory::class,
],
'aliases' => [
'alias' => 'service',
'alias2' => 'service',
],
'delegators' => [
'alias' => [
fn () => new DelegatedService(1),
],
'alias2' => [
fn () => new DelegatedService(2),
],
],
];
$sm = new ServiceManager($config);
$service = $sm->get('service');
// should we receive delegated service at all? there are delegators registered for aliases pointing to the service we are fetching At this point, I am not feeling good applying the changes proposed in this RFC. Does any1 have an idea regarding these issues? |
This is just an initial commit and should provide the idea and a base to play around. There are still some problems which can be reviewed in laminas#168 Signed-off-by: Maximilian Bösing <[email protected]>
@boesing FWIW I think regardless of the alias used it should be the same instance. Would it be feasible to resolve all aliased delegators to their service name prior to checking for the delegators to add? That could be done in the constructor to ensure its only done once. Then when assigning delegators, you only need to check for the service name? If a library provides an interface and consumes it, but an app needs a delegator on their specific alias to function correctly, I think the lib would also need that. That's my thinking. |
Aliases are not If I understand you correct, you suggest to unalias aliased delgators and merge these with the service? Don't have the mood to dive into that right now. Will see if I find some time until thursday. |
I was just using the interface for the example lib + app situation. But yes unalias the aliases, since they are resolved to the service name (the one that is invokable or provides a factory) after being called, you could then check only for that when checking what delegators to apply. |
Since aliases refer to the same shared service and because behavior would be incosistent and unpredictably changing at runtime otherwise, alias delegators need to be always applied to the service created or never. They can not be applied only when service is requested via alias. Alternative suggestion to consider: what if instead of doing this in service manager we forbid delegators on aliases (it would be an exception) but provide configuration pre-processing that would rewrite delegators to be on a factory for the service the alias was pointing to? |
Not sure if I understand this proposal. |
From a user's POV, I'd prefer that either delegators on aliases work or don't - pre-processing config seems magical - I guess unless it was strictly opt-in. If supporting delegators for aliases is not feasible, I'd like to see a hard fail. At the moment, if you delegate around an alias, nothing happens, the delegator doesn't run and you scratch your head for a while trying to figure out why, before remembering 'it doesn't work'. Perhaps forcing delegators to be $config = [
'factories' => [
'Service' => ServiceFactory::class,
],
'aliases' => [
'alias' => 'Service',
'alias2' => 'alias', // It _is_ possible to alias aliases right?
'alias3' => 'Service',
],
'delegators' => [
'Service' => [
ServiceDelegator::class,
],
'alias' => [
AliasDelegator::class,
],
'alias2' => [
AliasDelegator::class,
],
'alias3' => [
AnotherAliasDelegator::class,
],
],
];
// Resolved list of delegators:
$delegators = [
ServiceDelegator::class,
AliasDelegator::class,
AnotherAliasDelegator::class,
]; The problem is that it would require resolving all aliases up-front in order to compile, order, and possibly de-duplicate delegators in a predictable way. Whilst delegators can be any callable, it would not be possible to 'de-duplicate' them if that's even a footgun we'd attempt to avoid for users. With that in mind, I can see how config post-processing would alleviate ServiceManager of this complexity. |
Detaching this from v4.0.0. This feature won't introduce a BC break as it is not available as of now and thus once its released, can be considered as an addition. |
RFC
Goal
Allow users to have delegator factories for aliases.
Background
As of now, only delegator factories are applied to services registered to
factories
. In the past, we put services without its FQCN into thefactories
while aliasing the FQCN to that service name. That was done in laminas-cache, laminas-hydrator, etc.In one minor release, that was changed which ended up being a BC break because upstream projects already registered delegator factories to these services. AFAIR that change got reverted afterwards.
Considerations
This would not require any change from upstream projects as only additional delegators are called.
Proposal(s)
I'd retrieve delegator factories for both service and alias (if aliased and requested via that alias).
Appendix
It might be problematic when it comes to aliases referring to aliases referring to aliases.
AFAIR, the
ServiceManager
resolves aliases until either a factory or an invokable could be found. Thats where we might lose information.I'd say we can add this as a known limitation and still implement the feature.
Usually, the most common usecase is something like:
The text was updated successfully, but these errors were encountered: