Extend and decorate definitions (add new items to arrays, inherit definitions, decorate objects or factories…)

[mnapoli]

Modular systems (aka plugins/bundles/extensions) usually work like this: where there is a base config and modules can override or extend it.

Overriding a definition is supported (just set a definition for the same name). However extending is not.

We could extend several kind of definitions:

• arrays: add new items (e.g. a list of log handlers, a module wants to add a new handler to the list)
• factories: we could want to wrap the previous factory (i.e. callable) in another factory to be able to manipulate or decorate the result (e.g. manipulate the event dispatcher to register new listeners)
• objects: we could want to decorate an object by another one (e.g. decorate an object with a proxy that cache method calls, …)

The big question is the syntax: how to make it simple and explicit?

The other question is performances… Should definitions be compiled to PHP code to guarantee minimum overhead? I guess that can be decided once we have a working solution depending on whether performance is a problem or not.

---

Symfony tags

Symfony's approach is to use tags: you add tag to a container entry. Then you write a compiler pass that collects all instances having a specific tag, and you do something with them.

Pros:

• no need to think of a syntax to decorate/manipulate/extend entries
• developers don't need to understand the principles of decorating objects or factories…

Cons:

• not as explicit: instead of configuring a list of "log handlers", you need to tag objects as "log handler" and write some stuff to inject all objects with that tag: setting up "a list of something" is native PHP stuff, "tagging" and "fetching tagged services" is not a PHP concept, it's less intuitive
• you likely need compiler passes (PHP-DI is not compiled, yet)
• you need to write compiler passes, which is very complex (you need to understand a lot more stuff about the container)
• doesn't work for the non-compiled container (which is confusing in Symfony because you can use the container not compiled)

Spring collections

For reference, Spring allows to define collections (list/map…).

How to extend a list in spring config?

Guice multibindings

Guice allows to define lists too, and let it possible for module (in a modular system) add items. They call that Multibindings.

By the way Guice has a "module" system that is pretty similar to what we want to achieve here (e.g. this article). A stripped down version (but understandable for PHP devs) can be found in Ray.Di which is inspired from Guice.

---

What follows is an attempt at a syntax. This is just a suggestion, I've been thinking about that for months now and there doesn't seem to be an obvious solution…

Arrays

Now that #207 is implemented, we can define collections/arrays (in v5):

'log.handlers' => [
    DI\link(FileHandler::class),
    DI\link(DatabaseHandler::class),
],

To add entries in other config files:

'log.handlers' => DI\extend()->add([
    DI\link(LogstashHandler::class),
]),

We could go with a simpler DI\add() function:

'log.handlers' => DI\add([
    DI\link(LogstashHandler::class),
]),

but maybe being consistent with what follows is better?

Factories

Note: with a factory, we can extend any other definitions (that's cool!). E.g. here we'll extend an object definition:

ProductRepositoryInterface::class => DI\object(DatabaseProductRepository::class),

To extend or decorate the previous one:

ProductRepositoryInterface::class => DI\extend()->factory(function($previous) {
    return new CachedProductRepository($previous);
}),

Alternative syntax:

ProductRepositoryInterface::class => DI\decorate(function($previous) {
    return new CachedProductRepository($previous);
}),

Objects

ProductRepositoryInterface::class => DI\object(DatabaseProductRepository::class),

Note that currently for DI\object() it already extends previous definitions instead of overriding them. The reason for this is that we sometimes want to extend the Autowiring or Annotation definition (just set a parameter that is not type-hinted for example).

To extend another definition explicitly:

ProductRepositoryInterface::class => DI\object(CachedProductRepository::class)
    ->extend(/* provide an entry name or nothing */)
    ->constructor(DI\decorated()),

Or with DI\extend():

ProductRepositoryInterface::class => DI\extend()->object(CachedProductRepository::class)
    ->constructor(DI\decorated()),

or DI\link() without service ID could be used to reference the previous definition of the current service?

[ewinslow]

This is a really cool idea. I have been wanting to take our plugin architecture this direction for some time now... Iiuc, these are all currently equivalent, assuming CachedProductRepository type hints ProductRepositoryInterface in its constructor.

ProductRepositoryInterface::class => object(CachedProductRepository::class)
ProductRepositoryInterface::class => extend()->object(CachedProductRepository::class)
ProductRepositoryInterface::class => extend()->object(CachedProductRepository::class)->constructor(decorated())

Is that right?

With the empty link option, this would just be...

extend()->object(CachedProductRepository::class)->constructor(link())

That could work. I'd also see value in having more symmetry between extend/decorated. I.e. use decorator and decoratee. decorated is also just more explicit than empty link, so that's nice.

decorator()->object(CachedProductRepository::class)->constructor(decoratee())

This also keeps that first key word as a noun (object, link, factory, decorator), instead of a verb (object, link, factory, extend??) so I like that symmetry.

But if object already did this then maybe factory should just do this automatically too, assuming the same key is injected.

You're right that it's really not obvious how this should be done... Should everything be a decorator automatically or should we make people get explicit? It seems like if you're injecting the same key that you're defining, then it's obvious your intent is to decorate, which would leave us with:

->object(CachedProductRepository::class)->constructor(link(ProductRepositoryInterface::class))

Do my ramblings spark any new thoughts?

[mnapoli]

Hi @ewinslow thanks for the feedback!

these are all currently equivalent, assuming CachedProductRepository type hints ProductRepositoryInterface in its constructor […] Is that right?

Yes in the future v5 discussed here, that would be equivalent indeed.

Thanks to your suggestions I see something more clear: there's a strong difference between extending an object definition and decorating another object. If you decorate an object you probably don't want to inherit its definition. Same with factory: extending another definition doesn't actually mean anything. So these should probably be separate syntaxes.

So:

	extend	decorate
object	x	x
factory		x
array	x

Decorating

It seems like if you're injecting the same key that you're defining, then it's obvious your intent is to decorate

Yes good point!

And taking the same thinking for factory could also work. Let's imagine for a moment we have #197:

return [
    Service::class => DI\factory(function (Service $decorated) {
        return CachedService($decorated);
    }),
];

That could work without any particular syntax. But that wouldn't work if we can't type-hint… In that case maybe it's just a problem of #197 and we could have something similar to AngularJS:

return [
    'foo' => DI\factory(function ($decorated) {
        return Bar($decorated);
    }, [
        'decorated' => DI\link('foo'),
    ]),
    // Or maybe
    'foo' => DI\factory(function ($decorated) {
        return Bar($decorated);
    }, DI\link('foo')),
];

So maybe problem solved without any syntax addition?

Extending

One problem I see is that we want to add decoration and definition extension, but that already exist for DI\object() today and it's implicit. The reason for this is to take advantage of autowiring/annotations as much as possible, for example:

class Foo {
    public function __construct(Bar $bar, $string) …
}

return [
    'Foo' => DI\object()
        // I only have to define the non type-hinted parameter
        ->constructorParameter('string', 'Hello world'),
];

What I'm wondering is, if we add definition extension and decoration as a main feature with explicit syntax addition (e.g. adding items to arrays, etc.), should we make it also explicit for objects: that would break BC.

return [
    'Foo' => DI\object()
        ->extend()
        ->constructorParameter('string', 'Hello world'),
];

Without ->extend() then the definition is considered as a complete override of any previous existing definition (may it be defined in a file, autowiring or annotations).

To me that's better because:

• it's explicit whether you extend or not
• you can choose not to use it to replace the previous entry
• you can control what you extend (->extend('some_other_custom_entry')) like in Symfony for example
• it would be more consistent with array extension which requires a specific syntax to add instead of replacing the whole array)
• side-effect/bug DI\object()->method() does not support calling same method twice #193

So do we want to break BC here?

By the way, alternative name: ->inherit(), it seems more explicit when no parameter is given ("inherit previous definition").

Ideal world

Let's say we break BC and imagine the best solution possible, options are:

• consistent syntaxes for arrays & objects
• inconsistent syntaxes

'log.handlers' => extend()->array()
     ->add(FileHandler::class),
FileHandler::class => extend()->object()
    ->constructorParameter('filename', 'app.log'),

Nah, now that we have only array and object consistency doesn't matter much more anymore, and that's honestly confusing and ugly.

Something like this looks much clearer:

'log.handlers' => DI\array()
    ->add(FileHandler::class),
FileHandler::class => object()
    ->extend()
    ->constructorParameter('filename', 'app.log'),

DI\array()->add(...) or DI\add(...)? DI\add() is simpler but doesn't really allow to extend an array registered under a different name. And even if the syntax allowed it, it's not really clear where we add entries: in the definition we extend or the current one? Confusing.

'foo' => DI\array('bar')
    ->add('hello'),

Here it's clear that we take an other array and add an entry for foo. bar will be untouched.

Also, I really like how explicit object()->extend() is (could also be object()->inherit() or object()->parent(...) like in Symfony, but confusing with PHP class inheritance…).

So we are closing in on a solution :) But do we want to break BC?

[ewinslow]

Couple more issue with decorators -- how would I register more than one decorator at a time? Library might want to provide several decorators that apps can mix/match.

There's also the issue of control over the order (or do we say in principle that this shouldn't matter?)

Finally, what if a later plugin wants to remove one of the decorators? Is that a bad approach or is there some use case for such a thing? Not totally clear to me...

[mnapoli]

how would I register more than one decorator at a time?

You are right, the way definitions exist right now you can only write a definition once in each file (since it's an indexed array. But I don't personally see that as a problem.

Let's say a plugin wants to decorate twice the logger:

LoggerInterface::class => DI\factory(function (LoggerInterface $logger) {
    $logger = new Decorator1($logger);
    $logger = new Decorator2($logger);
    return $logger;
}),

It would be possible only with factory, but it's such a rare use case that I think it's OK to not be able to do it with object(). My goal with the DI\object() DSL is to cover 80% of the use cases, and leave the rest to pure PHP code in factories.

(by the way of course in my mind each plugin/module has a separate config file else everything falls apart)

There's also the issue of control over the order (or do we say in principle that this shouldn't matter?)

Inside a config file, there is no order. Between config file, the order is the one which you add the config files. So in a plugin system I would register the config file of each plugin in the order they are registered in the plugin system (i.e. usually base/core plugins are registered first, then optional plugins and here the order is a problem for the plugin managing system…). So basically the order should be handled by the user of PHP-DI, not PHP-DI. By the way this was recently a problem in Piwik (matomo-org/matomo#6529) even though we do not use PHP-DI for plugins yet.

Finally, what if a later plugin wants to remove one of the decorators? Is that a bad approach or is there some use case for such a thing? Not totally clear to me...

I don't see a use case too but maybe it exist. Anyway there is no standard way to decorate an object (constructor parameter, call a setter, …?) so that's why you have to write definitions manually. Same goes for removing a decorator, do it manually, e.g.:

LoggerInterface::class => DI\factory(function (LoggerInterface $logger) {
    return $logger->getDecorated();
}),

But I doubt it's a real use case honestly.

[mnapoli]

Damn forgot that DI\array() is impossible because array is a reserved keyword. I'll go with DI\add() for now.

[mnapoli]

I'm closing this: all has been implemented except:

• decorating using a closure: moved to Extend and decorate definitions (add new items to arrays, inherit definitions, decorate objects or factories…) #214
• making "extending another object definition" explicit: I've decided to drop this after trying it (it was too confusing)