merged branch jeromemacias/DoctrineServiceProviderFix (PR #196)
Commits ------- 3871f24c [DoctrineServiceProvider] Fix default options was not used with tests Discussion ---------- [DoctrineServiceProvider] Fix default options was not used --------------------------------------------------------------------------- by fabpot at 2011/10/25 23:13:23 -0700 The code looks correct to as we are modifying the options by reference. --------------------------------------------------------------------------- by jeromemacias at 2011/10/26 05:54:18 -0700 On the line 50 : ```php $tmp = $app['dbs.options']; ``` This is a copy of options array, the reference used after does not has any effect on initial options array. --------------------------------------------------------------------------- by DrBenton at 2011/10/26 06:02:46 -0700 @jeromemacias But in the ``` foreach ($tmp as $name => &$options) ``` the ```$options``` var is referenced (with the use of ```&```), not copied. Thus, if I'm not wrong, this loop *does* have an effect on ```$app['dbs.options']```. --------------------------------------------------------------------------- by jeromemacias at 2011/10/26 12:36:40 -0700 @DrBenton The $options var is a reference to $tmp but $tmp is a copy of $apps['dbs.options'], not a reference. --------------------------------------------------------------------------- by jeromemacias at 2011/10/26 12:58:38 -0700 I used this config (json) : ```json { "db.options": { "dbname": "mydbname", "user": "mydbuser", "password": "mydbpass" } } ``` With the current code : ```php $tmp = $app['dbs.options']; foreach ($tmp as $name => &$options) { $options = array_replace($app['db.default_options'], $options); if (!isset($app['dbs.default'])) { $app['dbs.default'] = $name; } } var_export($app['dbs.options']); die; ``` The result is : ```php array ( 'default' => array ( 'dbname' => 'mydbname', 'user' => 'mydbuser', 'password' => 'mydbpass', ), ) ``` With the new code : ```php $tmp = $app['dbs.options']; foreach ($tmp as $name => &$options) { $options = array_replace($app['db.default_options'], $options); if (!isset($app['dbs.default'])) { $app['dbs.default'] = $name; } } $app['dbs.options'] = $tmp; var_export($app['dbs.options']); die; ``` The result is : ```php array ( 'default' => array ( 'driver' => 'pdo_mysql', 'dbname' => 'mydbname', 'host' => 'localhost', 'user' => 'mydbuser', 'password' => 'mydbpass', ), ) ``` --------------------------------------------------------------------------- by igorw at 2011/10/26 13:59:51 -0700 Issue confirmed, please reopen and merge. EDIT: To be clear: the defaults are not being applied to dbs.options. --------------------------------------------------------------------------- by DrBenton at 2011/10/26 14:05:23 -0700 Sorry, you're right @jeromemacias. Since the values of ```$app['dbs.options']``` seem to be simple scalars, iterating though ```$tmp``` values, even by reference, does not affect ```$app['dbs.options']``` values. I thought ```$app['dbs.options']``` values where Objects (in that case, the code would have been ok). My mistake ! --------------------------------------------------------------------------- by fabpot at 2011/10/26 14:13:41 -0700 We need some unit tests for this. --------------------------------------------------------------------------- by jeromemacias at 2011/10/30 09:28:53 -0700 @fabpot Just added test --------------------------------------------------------------------------- by fabpot at 2011/11/07 01:08:44 -0800 Can you squash your commits? Thanks. --------------------------------------------------------------------------- by jeromemacias at 2011/11/07 15:18:40 -0800 @fabpot Done.
Showing
Please register or sign in to comment