Skip to content

Commit eeb9428

Browse files
ChrisSchwerdtpatrickbrouwers
authored andcommitted
Fix issue with nested configuration keys (#39)
The ConfigurationFactory used the Collection class for retrieving configuration keys, but the Collection class does not support nested key names. This caused the retrieval of the schema.filter key to always return the default value, which would never filter any tables. - Switched to using the Repository class because it properly supports nested keys. - Fixed the mock value for the schema configuration in the ConfigurationFactoryTests. It's a nested array, not a flattened array. - Fixed setFilterSchemaAssetsExpression() expectations in unit tests as Mockery::with() detects regular expressions and uses them when performing argument matching. This was causing false negatives when asserting method call arguments matched expected values. Added Mockery::mustBe() to use strict equality checks.
1 parent f27cfe6 commit eeb9428

File tree

3 files changed

+17
-14
lines changed

3 files changed

+17
-14
lines changed

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@
1919
"require": {
2020
"php": ">=5.5.9",
2121
"doctrine/migrations": "~1.1",
22+
"illuminate/config": "~5.1",
2223
"illuminate/contracts": "~5.1",
2324
"illuminate/console": "~5.1",
24-
"illuminate/support": "~5.1",
2525
"laravel-doctrine/orm": "1.0.*|1.1.*|1.2.*"
2626
},
2727
"require-dev": {

src/Configuration/ConfigurationFactory.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
namespace LaravelDoctrine\Migrations\Configuration;
44

55
use Doctrine\DBAL\Connection;
6-
use Illuminate\Contracts\Config\Repository;
6+
use Illuminate\Config\Repository;
7+
use Illuminate\Contracts\Config\Repository as ConfigRepository;
78
use Illuminate\Contracts\Container\Container;
8-
use Illuminate\Support\Collection;
99
use LaravelDoctrine\Migrations\Naming\DefaultNamingStrategy;
1010

1111
class ConfigurationFactory
1212
{
1313
/**
14-
* @var Repository
14+
* @var ConfigRepository
1515
*/
1616
protected $config;
1717

@@ -21,10 +21,10 @@ class ConfigurationFactory
2121
protected $container;
2222

2323
/**
24-
* @param Repository $config
24+
* @param ConfigRepository $config
2525
* @param Container $container
2626
*/
27-
public function __construct(Repository $config, Container $container)
27+
public function __construct(ConfigRepository $config, Container $container)
2828
{
2929
$this->config = $config;
3030
$this->container = $container;
@@ -39,9 +39,9 @@ public function __construct(Repository $config, Container $container)
3939
public function make(Connection $connection, $name = null)
4040
{
4141
if ($name && $this->config->has('migrations.' . $name)) {
42-
$config = new Collection($this->config->get('migrations.' . $name, []));
42+
$config = new Repository($this->config->get('migrations.' . $name, []));
4343
} else {
44-
$config = new Collection($this->config->get('migrations.default', []));
44+
$config = new Repository($this->config->get('migrations.default', []));
4545
}
4646

4747
$configuration = new Configuration($connection);

tests/Configuration/ConfigurationFactoryTest.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,13 @@ public function test_can_make_configuration()
5959
'name' => 'Doctrine Migrations',
6060
'namespace' => 'Database\\Migrations',
6161
'table' => 'migrations',
62-
'schema.filter' => '/^(?).*$/',
62+
'schema' => ['filter' => '/^(?).*$/'],
6363
'directory' => database_path('migrations'),
6464
'naming_strategy' => DefaultNamingStrategy::class,
6565
])
6666
;
6767

68-
$this->configuration->shouldReceive('setFilterSchemaAssetsExpression')->with('/^(?).*$/')->once();
68+
$this->configuration->shouldReceive('setFilterSchemaAssetsExpression')->with(m::mustBe('/^(?).*$/'))->once();
6969

7070
$this->container->shouldReceive('make')
7171
->with(DefaultNamingStrategy::class)
@@ -99,13 +99,16 @@ public function test_can_make_configuration_for_custom_entity_manager()
9999
'name' => 'Migrations',
100100
'namespace' => 'Database\\Migrations\\Custom',
101101
'table' => 'migrations',
102-
'schema.filter' => '/^(?).*$/',
102+
'schema' => ['filter' => '/^(?!^(custom)$).*$/'],
103103
'directory' => database_path('migrations/custom'),
104104
'naming_strategy' => DefaultNamingStrategy::class,
105105
])
106106
;
107107

108-
$this->configuration->shouldReceive('setFilterSchemaAssetsExpression')->with('/^(?).*$/')->once();
108+
$this->configuration->shouldReceive('setFilterSchemaAssetsExpression')
109+
->with(m::mustBe('/^(?!^(custom)$).*$/'))
110+
->once()
111+
;
109112
$this->container->shouldReceive('make')
110113
->with(DefaultNamingStrategy::class)
111114
->once()
@@ -138,13 +141,13 @@ public function test_returns_default_configuration_if_not_defined()
138141
'name' => 'Doctrine Migrations',
139142
'namespace' => 'Database\\Migrations',
140143
'table' => 'migrations',
141-
'schema.filter' => '/^(?).*$/',
144+
'schema' => ['filter' => '/^(?).*$/'],
142145
'directory' => database_path('migrations'),
143146
'naming_strategy' => DefaultNamingStrategy::class,
144147
])
145148
;
146149

147-
$this->configuration->shouldReceive('setFilterSchemaAssetsExpression')->with('/^(?).*$/')->once();
150+
$this->configuration->shouldReceive('setFilterSchemaAssetsExpression')->with(m::mustBe('/^(?).*$/'))->once();
148151
$this->container->shouldReceive('make')
149152
->with(DefaultNamingStrategy::class)
150153
->once()

0 commit comments

Comments
 (0)