• Fabien Potencier's avatar
    merged branch jeromemacias/DoctrineServiceProviderFix (PR #196) · 9fbd177a
    Fabien Potencier authored
    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.
    9fbd177a
Name
Last commit
Last update
..
Provider Loading commit data...
ApplicationTest.php Loading commit data...
BeforeAfterFilterTest.php Loading commit data...
ControllerCollectionTest.php Loading commit data...
ControllerResolverTest.php Loading commit data...
ControllerTest.php Loading commit data...
ErrorHandlerTest.php Loading commit data...
FunctionalTest.php Loading commit data...
RouterTest.php Loading commit data...
WebTestCaseTest.php Loading commit data...