• 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
..
Silex/Tests Loading commit data...
bootstrap.php Loading commit data...