rdlowrey / auryn

IoC Dependency Injector
MIT License
722 stars 65 forks source link

Support text config files #108

Closed rdlowrey closed 9 years ago

rdlowrey commented 9 years ago

Support the specification of dependencies in a text configuration file.

For example:

OLD

<?php
$injector = new Auryn\Injector;
$injector->define('PhpAmqpLib\Connection\AMQPStreamConnection', ['localhost', 5672, 'guest', 'guest']);
$obj = $injector->make('ClassDependingOnAMQPStreamConnection');

NEW

You could define this in yaml/XML/whatever:

# wiring.yaml
wiring:
    - { class: PhpAmqpLib\Connection\AMQPStreamConnection, params: [localhost, '5672', guest, guest] }
<?php
$injector = new Auryn\Injector;
$injector->parseWiring(file_get_contents('wiring.yaml'));
$obj = $injector->make('ClassDependingOnAMQPStreamConnection');
GRMule commented 9 years ago

I'm doing this in my bootstrap. I have an array in a file that maps between interfaces and concretes that I loop for alias, then another for share. If this were built into the injector, it would be great to have a format for specifying aliases, shares, and parameter binding.

tomwalder commented 9 years ago

Would there be an impact on performance - loading and parsing the wiring file for each process?

We tend to use PHP files so they are auto cached by APC/Opcache.

J7mbo commented 9 years ago

On the performance side, I cache my config files as I do this in my bootstrap as well.

But then we're going into standardising configuration and I'm not sure that's okay. We all know the PSR lot will be all over that soon enough I bet (OMG PLEASE USE CONTAINERINTEROP)...

Not to mention not everyone will be using configuration files to sort out their wiring.

tomwalder commented 9 years ago

I would expect details like cache configuration (e.g. memcache/redis connection details) to be part of the Di configuration... chicken/egg ?

J7mbo commented 9 years ago

I think that the injector as a standalone lib is great, it can fit in anywhere and just work. I'm just worried about putting a load of extra bloat around it. That's what bridges or adapters are for; for different frameworks and use-cases.

Don't get me wrong, I'd personally find something like this useful so I write my own. But if Auryn includes it, we're talking including parsing libs, caching libs, setting new standards, the injector isn't just an injector any more.

rdlowrey commented 9 years ago

Re: performance and caching ...

I was thinking something more along the lines of this:

<?php

$cacheDump = "/path/to/dumped/injector.php";

if (!@include $cacheDump) {
    $yamlConfig = "/path/to/text/config.yml";
    $injector = Auryn\Injector::loadFromYaml($yamlConfig);

    // we only have to do this once
    $injector->dump($cacheDump);
}

// Rest of the application goes here

This way you aren't eating any performance costs. We'd just be providing our own text config format. I think adding support for optimized cache dumping would be a useful (necessary) complement to this functionality.

Thoughts?

tomwalder commented 9 years ago

Something like that would work for us.

We deploy a lot to environments like Google App Engine, where it's a containerised read-only filesystem.

So we would potentially integrate the cache dump phase into our deployment process and require the dump file to be present in "production" environments.

But, like I said, that could work!

tomwalder commented 9 years ago

Also, we could just "not use" the text file .. ;)

rdlowrey commented 9 years ago

Cool. I'll add an additional issue for the cache dumping functionality.

FYI the plan is to tag an official v1.0.0 release in a few hours. None of this stuff will be present in that version ... it wouldn't appear until a fully BC-compatible 1.1 in the future.

J7mbo commented 9 years ago

The caching idea is great, but more thought needs to be put into "just using" the text file.

mattsah commented 9 years ago

I (and I suspect many others), have done this more at the framework level. One of the things I get very annoyed with, particularly now that composer is more popular is that there are a lot of libraries that feel it necessary to include this type of functionality, but they all have varying support/formats.

I generally want my configurations to be using the same language and the same configuration structures/files so that there's not 10 different points of configuration.

I don't feel this needs to be included or should be. It could be a separate package if people need a solution because they don't have another, but there are other solutions out there for unified config and bootstrapping.

J7mbo commented 9 years ago

@mattsah's comment better articulates my point of view :-)

rdlowrey commented 9 years ago

That's fine then. In general I'm of the belief that less is more. The answer is (usually) not more options. If you feel compelled to expose new preferences to the user it's very possible you've made a wrong turn somewhere. Will scrap plans for this now. Folks are free to re-raise the issue in the future if they see fit.

GRMule commented 9 years ago

Thanks, I think that's the best call. It is a good idea, but one that is trivial enough to implement by the package consumers in a way that meshes with their own project's conventions.