Closed oscarotero closed 11 months ago
I like the convention over configuration state of mind but I can see multiple drawbacks:
The interface define a static method. It may be allowed by php but it feel a bit tricky to me. Interfaces are meant to describe how an instance of a class can be used, not the class.
How to have multiple aliases ? For example a database service provider can take a name as constructor parameter so multiple named database instances can be provided.
How to provide scalar/configuration values ? For example an int for session time to live.
Hi, thanks for your reply. I'll try to clarify your doubts:
The interface define a static method. It may be allowed by php but it feel a bit tricky to me. Interfaces are meant to describe how an instance of a class can be used, not the class.
Yeah. We can define a convention in order to remove the interface, for example "If a class has a static method named getFactory()
, it is interpreted as the factory to build a new instance of the class". But I found this a bit vague. Anyway, whatever convention decided, It should be something that can be discovered easily.
How to have multiple aliases ? For example a database service provider can take a name as constructor parameter so multiple named database instances can be provided.
To me, this spec is to generate something ready to consume, in order to just execute $container->get('name-of-the-service')
and create the final instance (or return the previous generated instance). So if you need various intances with different configurations, you should create a factory for each one. Example:
class MainDatabase implements FactoryInterface
{
public function create(ContainerInterface $container) {
return new Database('main');
}
}
class SecondaryDatabase implements FactoryInterface
{
public function create(ContainerInterface $container) {
return new Database('secondary');
}
}
// ...
$main = $container->get('MainDatabase');
$secondary = $container->get('SecondaryDatabase');
How to provide scalar/configuration values ? For example an int for session time to live.
An option is use the container to store config values, like in the example of Plugins above. Example:
class SessionFactory implements FactoryInterface
{
public function create(ContainerInterface $container) {
$config = $container->get('session_config');
return new Session($config);
}
}
Other option is using env variables:
class SessionFactory implements FactoryInterface
{
public function create(ContainerInterface $container) {
return new Session([
'ttl' => getenv('SESSION_TTL')
]);
}
}
Or a good thing to have is a way to get the factory before create the instance. Because the container returns always the same instance, we can get the factory and use it to store the configuration before execute it. Because ContainerInterface
has not a method getFactory()
, let's say that, by convention, that we can return the factory using the ::getFactory
prefix:
//Get the factory and change the config
$factory = $container->get('Session::getFactory');
$factory->setTTL(3600);
//Now on create the instance, the same factory will be used with the new configuration:
$session = $container->get('Session');
I know this is a bit tricky and I don't think it's the best way to do this. But any of the previous two examples are perfectly valid, in my opinion.
I agree the example with config value is valid, it could even be possible to do so:
class SessionFactory implements FactoryInterface
{
public function create(ContainerInterface $container) {
// User provided value when defined - suitable default value when not
$config = $container->has('session_ttl')
? $container->get('session_ttl')
: 3600;
return new Session($config);
}
}
With the current spec I would do:
class SessionServiceProvider implements ServiceProviderInterface
{
public function getFactories()
{
return [
Session::class => [$this, 'getSession'],
];
}
public function getExtensions()
{
return [
'session.ttl' => [$this, 'getSessionTtl'],
];
}
public function getSession($container): Session
{
$ttl = $container->get('session.ttl');
return new Session($ttl);
}
// fallback value if 'session.ttl' is not registered in the container.
public function getSessionTtl($container, int $ttl = 3600): int
{
return $ttl;
}
}
Ok it is way more verbose but the main advantage I see with this approach is it is easy to list what is provided. For example it would allow some yet to be developed composer command plugin to display:
I think it would be possible with the approach you are proposing but trickier. Should somehow loop over all classes and select the ones implementing FactoryInterface
. But no way to list config values. I like the way how everything is listed in a single place with the current spec.
For the second point I'm not sure I agree, in your example the end developer should create a new factory for each database connection. It means he have to manually get from the container what the service needs to be injected every time he need a new version of the service. With the current spec you can do:
<?php
// database service providers
return [
// Provides 'app.database[main]'.
new DatabaseServiceProvider('main'),
// Provides 'app.database[secondary]'.
new DatabaseServiceProvider('secondary'),
// They can also set DatabaseInterface to be an alias of 'app.database[main]' by default.
];
Another use case I have is loading env specific service providers. With the current spec I can load a dev service provider, extending my http kernel by adding some of your cool middleware around it. I don't see how I would do it with your proposed approach.
I'm passionate about autodiscovery of services too. By testing the current spec in my projects I came to the point it is ok as it is (a bit verbose ok but it is package library developer work - not the end user) and maybe we should only develop standard utilities around it. I was thinking about a composer installer plugin to auto import service providers in a project and a composer command plugin to explore them (like described above). What would you think about it? I think it could be very lightview, I'll expose the idea here someday.
For the second point, I didn't understand your example. I though you was asking for two different services that returns instances of the same class, for example if your app needs two different databases for different purposes. But if you want to use only one service using different configuration depending of the environment, it can be done easily using env variables or passing config options to the container:
class Database implements FactoryInterface
{
public function create(ContainerInterface $container) {
if (getenv('ENV') === 'prod') {
return new Database('main');
}
return new Database('secondary');
}
}
Other advantage of using a FactoryInterface
class instead an array of callables is that we can provide more methods if we think they're usefull. For example to return a list of dependencies:
interface FactoryInterface
{
public function create(Container $container);
public function getDependencies(): array;
}
So to get the list of dependencies used by a factory:
$factory = Database::getFactory();
foreach ($factory->getDependencies() as $name => $type) {
echo "Database deppends on $name of type $type";
}
This is only an idea. The aim with this issue is not discuss the signature of the FactoryInterface
(this is something that can be discussed deeply later) but the fact of having an autodiscovered FactoryInterface
instead an array of callables.
Note also that the ability of autodiscover is an implementation feature of the container, so if someone want more control and register manually all dependencies, it can use a container that does not try to discover the dependencies (or has an option to disable it). This approach only ease this feature but without force it. You could have something like this:
$container->registerFactories([
'dependency1' => 'Factory1',
'dependency2' => 'Factory2',
]);
Hey Oscar,
Sorry for not replying earlier. For some reason, I did not see your proposal until you mentioned it on the PHP-FIG mailing list)
I see a few things that are show-stopper for me.
Your proposal plays very nicely with containers that do auto-wiring. In those containers, usually, one class = one service. For these kind of containers (Laravel is one of them), the convention over configuration works really great.
However, there can be issues when you have several services of the same class. You already spoke about this in your example above with many databases. But it can be a bit cumbersome to have one class per entry in the container.
Here is a sample. I'm writing an application that has a menu (represented as a Menu
class in the container). The application is modular. Each feature has its own service provider, and each service provider creates a MenuItem
that is added in the Menu
class.
It means that have 20 or 30 entries of type MenuItem
in my container. It's really a big advantage for me not to have to create a new XxxMenuItemFactory
for each new menu item.
Also, it is not entirely clear to me how we can handle these features:
Interestingly enough, you could implement your idea by building a decorator around any PSR-11 container which can be a good starting point if you want to experiment your idea further.
As a side note, I never really got a chance to tell you I'm a big fan of your PSR-15 middlewares. I'm using "Payload" and "Whoops" every day on all my projects, so thank you!
Hi David. Thanks so much! I'm glad you like the PSR-15 middlewares. I intend this to be a collaborative project, so if you like to contribute with more middlewares, do not hesitate to ask to join to the organization.
Ok, let's go to your issues about this proposal:
Your example with different MenuItem
factories could be solved using just one factory and passing arguments to its constructor, because each class can instantiate its own factory. Example:
/**
* This is an universal factory used by all menu items
*/
class MenuItemFactory implements FactoryInterface
{
private $class;
public function __construct(string $class)
{
$this->class = $class;
}
public function create(ContainerInterface $container)
{
return new $this->class();
}
}
/**
* Menu items could use different instances of same factory class
*/
class MenuItem1
{
public static function getFactory(): FactoryInterface
{
return new MenuItemFactory(self::class);
}
}
class MenuItem2
{
public static function getFactory(): FactoryInterface
{
return new MenuItemFactory(self::class);
}
}
class MenuItem3
{
public static function getFactory(): FactoryInterface
{
return new MenuItemFactory(self::class);
}
}
/**
* The menu factory creates the Menu instance and insert the menu items
*/
class MenuFactory implements FactoryInterface
{
public function create(ContainerInterface $container)
{
$menu = new Menu();
$menu->add($container->get('MenuItem1'));
$menu->add($container->get('MenuItem2'));
$menu->add($container->get('MenuItem3'));
return $menu;
}
}
The service returned by a factory can be anything: not only an object but also arrays or scalar values. For example, we could create a service to return the list of menu items we want to add to our menu:
class MenuConfig implements FactoryInterface
{
public function create(ContainerInterface $container) {
return [
'MenuItem1',
'MenuItem3',
];
}
}
so, our MenuFactory
could use this configuration to build the menu:
class MenuFactory implements FactoryInterface
{
public function create(ContainerInterface $container)
{
$menu = new Menu();
$options = $container->get('MenuConfig');
foreach ($options as $option) {
$menu->add($container->get($option));
}
return $menu;
}
}
Anyway, I'll implement this idea as you say and let's see if it's good enough or not š
Hi Oscar,
Thanks for the samples. I think I properly understood those.
My issues are about the number of classes you have to create.
In your sample, you create 3 classes: MenuItem1
, MenuItem2
, MenuItem3
. Each has a different "getFactory" method. But that's about what makes it different. I mean, all those classes could very well extend the same base MenuItem
class. This is somewhat weird. It means we have to extend the class for each menu item instance we want to create.
class MenuItem1 extends MenuItem
{
public static function getFactory(): FactoryInterface
{
return new MenuItemFactory(self::class);
}
}
class MenuItem2 extends MenuItem
{
public static function getFactory(): FactoryInterface
{
return new MenuItemFactory(self::class);
}
}
class MenuItem3 extends MenuItem
{
public static function getFactory(): FactoryInterface
{
return new MenuItemFactory(self::class);
}
}
That a lot of classes when all we want is really different instances of the same MenuItem
class.
Also, we have issues if the base MenuItem
class is declared final (which would be completely possible as many people are pushing to declare classes final by default).
Also, I understand you example about the MenuConfig
. You can create a "MenuConfig" entry in the container that is an array of services. This is true. But what I want to do is a bit more complex. I want each service provider to be able to alter this MenuConfig
array.
So if I register a new package/bundle/module, I want this new module to be able to add in an existing "MenuConfig" an additional menu item. The "MenuConfig" is already declared by an existing factory and another "factory", declared in another package should be able to "mutate/alter" it.
Actually, discussing this with you is helping me figuring out the scope I want for the PSR :)
Hi David.
I think this proposal is flexible enough to solve many of these issues in diferent ways. The two rules to create a FactoryInterface
of a service are:
getFactory()
static method, use this method to return the factory.Factory
, use it to create the service.This allows to implement factories with final classes. For example, if MenuItem
is final, you can create MenuItemFactory
to create the instances.
Because the container should return always the same instance of each service, a way to generate several instances of the same class is creating a service that can get the factories of other services. Example:
class MenuItemsFactory implements FactoryInterface
{
public function create(ContainerInterface $container)
{
$factory = MenuItem::getFactory();
$option1 = $factory->create($container);
$option1->title = 'First option';
$option2 = $factory->create($container);
$option2->title = 'Second option';
return [$option1, $option2];
}
}
As an improvement, we could allow to pass arbitrary arguments to the getFactory()
method in order to create different factories:
class MenuItemsFactory implements FactoryInterface
{
public function create(ContainerInterface $container)
{
return [
MenuItem::getFactory('First option')->create($container),
MenuItem::getFactory('Second option')->create($container)
];
}
}
Factories that use other factories to create the final service is useful to override the default behaviour of a service. For example, we could use Plates with some extensions loaded, so, we can create our own service provider:
namespace MyApp;
class PlatesFactory implements FactoryInterface
{
public function create(ContainerInterface $container)
{
$plates = League\Plates\Engine::getFactory()->create($container);
$extension = League\Plates\Extension\Asset::getFactory()->create($container);
$plates->loadExtension($extension);
return $plates;
}
}
The ability to alter a service is archieve creating another factory that extends the previous factory. Other way is using an object, instead an array to store configuration. For example:
namespace Library;
class Plugin
{
public static function register(ContainerInterface $container)
{
$config = $container->get('Library\\Plugins');
$config->registerPlugin(self::class);
}
}
This requires some user work (call manually Library\Plugin::register($container)
), but as far as I know, the other proposals need to do manual work too.
And surely there's more ways to archieve this.
@moufmouf Hi again.
I've created this repo implementing this idea: https://github.com/oscarotero/service-provider-concept
The app/Container
directory contains a simple Psr-11 container with support for service providers. And app/Services
contains the services used by the app.
The idea is the app could define its own services using this interface and these services should be loaded automatically by the container. If this spec were approved, many libraries could include their own providers, so you wouldn't need to create many of them, like ResponseTimeFactory
.
This concept is similar to node modules, in which each module is in its own file and exports only the desired values (a function, object, array, etc). In this case we are using factory classes to get these values.
I agree with @moufmouf, it looks nice but way too many classes (factories) need to be created to make things work. We really need to strive for something simple, the more configuration code a user needs to create, the less likely the adoption of this idea will be. In my perfect world we find something simple like a simple annotation (like in Java) or a really simple configuration class. I really like funky (obviously I guess), but I do understand that forcing everyone to use annotations will be "problematic".
That's also the reason why I was so quite in the working group the last couple of months. I still do not have a good idea on how to achieve this.
Ok, I understand. To be fair, if this spec were approved, I would expect many libraries include the factories in their packages, so the user could not create all these classes. And due each service is provided using an individual factory class, they could be reused accross many projects. For example, if I use always the same middlewares, I could create packages to provide these middlewares and load them as a dependency, instead create the factories in my project code.
It's hard to create this PSR covering all use cases and also be simple enough to be widely adopted. I just wanted to suggest an idea to cover some concepts that I consider important:
This idea is probably too far removed from the way existing DI containers work?
Since the goal is interoperability for existing containers, it's not clear to me how this idea can be seen as an alternative.
The idea seems half-baked as well - on one has brought up the issue of order, but if you were to essentially lazy-/auto-load providers, how would you make sure they arrive in the correct order?
There are some cool ideas in this thread - but unless anyone objects, I'd like to close this, so we can focus on interoperability?
I'm okay with closing this. š
Thanks, Oscar :-)
I love the idea of having a way to autoload services but I found the current spec a bit verbose and requires a lot of work to get it working. You must create a class that returns an array with a bunch of factories and other array with extensions.
I'd like to provide a different approach and know your opinion. Maybe something similar was discussed before and discarded. In this case, my apologies:
I like PSR-4 because provides a way to autoload classes without configuration but convention. You only need to save each class in a php file with a specific name in a directory with the same name than the namespace. With this simple rule, the entire php ecosystem has an autoloading and autodiscovering mecanism.
The service provider complements PSR-4 because defines the way of how these classes are instantiated once they are autoloaded. So I think this spec could be based also in convention above configuration.
My first idea:
Each class has it's own factory class. This factory must implement a standard interface, for example:
Interop\Container\FactoryInterface
and, by convention, must be named like the class that instantiate + the suffixFactory
. For example, the classFoo\Bar
has the factoryFoo\BarFactory
.So, on execute
$container->get('Foo\\Bar')
, the container will check ifFoo\BarFactory
exists and if do, create an instance of the factory and this factory create the final instance.Improving the idea: Defining a factory class:
The problem of having a 1:1 class/factory relationship is that you may need to create a lot of similar factories. A solution can be that instead use the convention for the factory class, each class instantiates its own factory. The convention here could be a static class named
getFactory
. For example:This allows to use the same factory for similar classes because each class passes arguments to the factory constructor:
Or use anonimous classes:
With this convention, on execute
$container->get('Foo\\Bar')
, the container only has to check whether Bar has the static methodgetFactory()
. A better way is create aHasFactoryInterface
that can be implemented byFoo\Bar
. Example:Use cases
Use the container to pass configuration options to the factory
Packages can use the container to get configuration values. Let's say we want to use plugins:
Use a custom factory to override the default behaviour or rename the original factory
Example of how we can create a factory to provide a database to the app. The same class implements both interfaces:
Advantages of this approach
composer dumpautoload -o
)Surely this idea requires more work but, in essence, is to use psr-4 + Factories to provide the services.
What do you think?