Respect / Config

A powerful, small, deadly simple configurator and dependency injection container DSL made to be easy
http://respect.github.io/Config
Other
98 stars 7 forks source link

Lazy Load and dependency problems #14

Closed augustohp closed 11 years ago

augustohp commented 12 years ago

When some configuration depends on another instance of the same configuration file, things don't go as one would expect:

The configuration

[pdo Pdo]
dsn      = [dsn]
username = [db_user]
password = [db_passwd]

[connectionParams]
pdo      = [pdo]
dbname   = [db_name]
user     = [db_user]
password = [db_passwd]
host     = [db_host]
driver   = [db_driver]

[config Doctrine\DBAL\Configuration]

[event Doctrine\Common\EventManager]

[cache Doctrine\Common\Cache\ArrayCache]

[config Doctrine\ORM\Tools\Setup]
createAnnotationMetadataConfiguration[] = [[orm_entity_dir], [orm_proxy_autogenerate]]

[dbal Doctrine\DBAL\DriverManager]
getConnection[] = [[connectionParams], [config], [event]]

[entityManager Doctrine\ORM\EntityManager]
create[] = [[dbal], [config], [event]] 

The test code

Now, suppose I want to test the given code... it is quite normal to inject my pdo instance like that

class DoctrineTest extends \PHPUnit_Framework_TestCase
{
    protected $container;

    public function setUp()
    {
        $this->container = new Container('config.ini');
        $pdo = new PDO('sqlite::memmory:');
        $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        $this->container->pdo = $pdo;
    }
 }

The problem

The $this->container->pdo is actually the instance I gave, but everything that uses it inside the config is binded to the old PDO instance (the one that the Instantiator will create).

alganet commented 12 years ago

I've researched this a little. It can double up our code base quickly, 'cause we need to track all Instantiators created from a particular Container instance. This means proxying Instantiator creation through a "parent" Container and update the references as the're set.

alganet commented 12 years ago

Also, in some cases is impossible to exchange the declared instance for other. For example, we cannot change an instance used in the constructor of another class.

augustohp commented 12 years ago

We can if the instance was not yet used. And that is what I am trying to do.

augustohp commented 12 years ago

@alganet: The fix is not working. I created a PHPT in a different branch so you can execute it. Merge it if you want, I created another phpt tests as well to use as examples of usage.

You can have a look to the specific test file here: https://github.com/Respect/Config/blob/phpt/tests/examples/injection-multiple.phpt

alganet commented 12 years ago

This is working for you, @augustohp?

augustohp commented 12 years ago

Not working, made a simple unit test to simulate the issue. Should I commit it? It will break travis, the test is not passing =(

alganet commented 12 years ago

Should work for instances, for primitives this is a little harder. Could be solved using &references the same way we solved primary key auto-increment feedback on Relational. I'll work on that when I have some time.

alganet commented 12 years ago

This is insane. I've tried adding references to the whole project but gave it up when I saw that references were impossible for string substitutions: date = "[day]/[month]/[year]" for example.

I've thought on another approach. We currently have a single step for loading/interpreting in the constructor. If we change this to only load on the constructor then interpret everything on the first __get/getItem, this issue should be solved. As long as the user doesn't change anything after the first getItem. This works even for the day/month/year sample above. Not in the mood fo implementing this now though.

This issue doesn't exists for objects, just scalars. Another approach would be convert any scalars to object. I dislike this very much.

augustohp commented 12 years ago

+1 for the two step solution on loading and interpreting the INI file. I will see how can I implement it.

nickl- commented 11 years ago

Solved test case and added support for instances in the same fashion see #26

augustohp commented 11 years ago

:+1: