Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update the Migrations class to support the seeding feature #182

Merged
merged 1 commit into from
Jan 10, 2016
Merged

Update the Migrations class to support the seeding feature #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/CakeManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ public function resetMigrations()
$this->migrations = null;
}

/**
* Reset the seeds stored in the object
*
* @return void
*/
public function resetSeeds()
{
$this->seeds = null;
}

/**
* Prints the specified environment's migration status.
*
Expand Down
2 changes: 1 addition & 1 deletion src/ConfigurationTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function getConfig($forceRefresh = false)

$source = $this->input->getOption('source');
if ($source) {
if ($this instanceof Seed) {
if ($this instanceof Seed || ($this instanceof Migrations && $this->getCommand() === 'seed')) {
$seedsFolder = $source;
} else {
$migrationsFolder = $source;
Expand Down
62 changes: 62 additions & 0 deletions src/Migrations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ class Migrations
*/
protected $default = [];

/**
* Current command being run.
* Useful if some logic needs to be applied in the ConfigurationTrait depending
* on the command
*
* @var array
*/
protected $command;

/**
* Constructor
* @param array $default Default option to be used when calling a method.
Expand All @@ -65,6 +74,28 @@ public function __construct(array $default = [])
}
}

/**
* Sets the command
*
* @param string $command Command name to store.
* @return self
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far we have been using $this.
But I would favor if we migrated to self for CakePHP in the near future.

*/
public function setCommand($command)
{
$this->command = $command;
return $this;
}

/**
* Gets the command
*
* @return string Command name
*/
public function getCommand()
{
return $this->command;
}

/**
* Returns the status of each migrations based on the options passed
*
Expand All @@ -80,6 +111,7 @@ public function __construct(array $default = [])
*/
public function status($options = [])
{
$this->setCommand('status');
$input = $this->getInput('Status', [], $options);
$params = ['default', $input->getOption('format')];

Expand All @@ -103,6 +135,7 @@ public function status($options = [])
*/
public function migrate($options = [])
{
$this->setCommand('migrate');
$input = $this->getInput('Migrate', [], $options);
$method = 'migrate';
$params = ['default', $input->getOption('target')];
Expand Down Expand Up @@ -133,6 +166,7 @@ public function migrate($options = [])
*/
public function rollback($options = [])
{
$this->setCommand('rollback');
$input = $this->getInput('Rollback', [], $options);
$method = 'rollback';
$params = ['default', $input->getOption('target')];
Expand Down Expand Up @@ -161,6 +195,7 @@ public function rollback($options = [])
*/
public function markMigrated($version, $options = [])
{
$this->setCommand('mark_migrated');
$input = $this->getInput('MarkMigrated', ['version' => $version], $options);
$params = [$version];

Expand All @@ -175,6 +210,29 @@ public function markMigrated($version, $options = [])
return true;
}


/**
* Seed the database using a seed file
*
* @param array $options Options to pass to the command
* Available options are :
*
* - `connection` The datasource connection to use
* - `source` The folder where migrations are in
* - `plugin` The plugin containing the migrations
* - `seed` The seed file to use
*
* @return bool Success
*/
public function seed($options = [])
{
$this->setCommand('seed');
$input = $this->getInput('Seed', [], $options);
$params = ['default', $input->getOption('seed')];
$this->run('Seed', $params, $input);
return true;
}

/**
* Runs the method needed to execute and return
*
Expand All @@ -189,6 +247,7 @@ protected function run($method, $params, $input)
{
if ($this->configuration instanceof Config) {
$migrationPath = $this->getConfig()->getMigrationPath();
$seedPath = $this->getConfig()->getSeedPath();
}

$this->setInput($input);
Expand All @@ -198,6 +257,9 @@ protected function run($method, $params, $input)
if (isset($migrationPath) && $newConfig->getMigrationPath() !== $migrationPath) {
$manager->resetMigrations();
}
if (isset($seedPath) && $newConfig->getSeedPath() !== $seedPath) {
$manager->resetSeeds();
}

return call_user_func_array([$manager, $method], $params);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/TestCase/Command/SeedTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public function testExecuteCustomParams()
->execute()->fetchAll('assoc');
$expected = [
[
'id' => '1',
'id' => '2',
'number' => '5',
'radix' => '10'
]
Expand Down
137 changes: 137 additions & 0 deletions tests/TestCase/MigrationsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,143 @@ public function testMigrateDateOption()
$this->assertEquals($expectedStatus, $this->migrations->status());
}

/**
* Tests seeding the database
*
* @return void
*/
public function testSeed()
{
$this->migrations->migrate();
$seed = $this->migrations->seed(['source' => 'Seeds']);
$this->assertTrue($seed);

$result = $this->Connection->newQuery()
->select(['*'])
->from('numbers')
->execute()->fetchAll('assoc');
$expected = [
[
'id' => '1',
'number' => '10',
'radix' => '10'
]
];
$this->assertEquals($expected, $result);

$seed = $this->migrations->seed(['source' => 'Seeds']);
$this->assertTrue($seed);
$result = $this->Connection->newQuery()
->select(['*'])
->from('numbers')
->execute()->fetchAll('assoc');
$expected = [
[
'id' => '1',
'number' => '10',
'radix' => '10'
],
[
'id' => '2',
'number' => '10',
'radix' => '10'
]
];
$this->assertEquals($expected, $result);

$seed = $this->migrations->seed(['source' => 'AltSeeds']);
$this->assertTrue($seed);
$result = $this->Connection->newQuery()
->select(['*'])
->from('numbers')
->execute()->fetchAll('assoc');
$expected = [
[
'id' => '1',
'number' => '10',
'radix' => '10'
],
[
'id' => '2',
'number' => '10',
'radix' => '10'
],
[
'id' => '3',
'number' => '2',
'radix' => '10'
],
[
'id' => '4',
'number' => '5',
'radix' => '10'
]
];
$this->assertEquals($expected, $result);
$this->migrations->rollback(['target' => 0]);
}

/**
* Tests seeding the database with seeder
*
* @return void
*/
public function testSeedOneSeeder()
{
$this->migrations->migrate();

$seed = $this->migrations->seed(['source' => 'AltSeeds', 'seed' => 'AnotherNumbersSeed']);
$this->assertTrue($seed);
$result = $this->Connection->newQuery()
->select(['*'])
->from('numbers')
->execute()->fetchAll('assoc');

$expected = [
[
'id' => '1',
'number' => '2',
'radix' => '10'
]
];
$this->assertEquals($expected, $result);

$seed = $this->migrations->seed(['source' => 'AltSeeds', 'seed' => 'NumbersAltSeed']);
$this->assertTrue($seed);
$result = $this->Connection->newQuery()
->select(['*'])
->from('numbers')
->execute()->fetchAll('assoc');

$expected = [
[
'id' => '1',
'number' => '2',
'radix' => '10'
],
[
'id' => '2',
'number' => '5',
'radix' => '10'
]
];
$this->assertEquals($expected, $result);

$this->migrations->rollback(['target' => 0]);
}

/**
* Tests that requesting a unexistant seed throws an exception
*
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The seed class "DerpSeed" does not exist
* @return void
*/
public function testSeedWrongSeed()
{
$this->migrations->seed(['source' => 'AltSeeds', 'seed' => 'DerpSeed']);
}

/**
* Tests migrating the baked snapshots
*
Expand Down
29 changes: 29 additions & 0 deletions tests/test_app/config/AltSeeds/AnotherNumbersSeed.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php
use Phinx\Seed\AbstractSeed;

/**
* NumbersSeed seed.
*/
class AnotherNumbersSeed extends AbstractSeed
{
/**
* Run Method.
*
* Write your database seeder using this method.
*
* More information on writing seeders is available here:
* http://docs.phinx.org/en/latest/seeding.html
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return void

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.
The bake template also needs to be updated with this.

public function run()
{
$data = [
[
'number' => '2',
'radix' => '10'
]
];

$table = $this->table('numbers');
$table->insert($data)->save();
}
}
1 change: 0 additions & 1 deletion tests/test_app/config/AltSeeds/NumbersAltSeed.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ public function run()
{
$data = [
[
'id' => '1',
'number' => '5',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not include primary keys in seed data?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to not manually add the primary key. SQL server, for example does not like that, whereas Postgres will not advance the sequence automatically

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow really? Do you have documentation for those two cases? I think it might be important to note for users creating seed data.

Note: I've not had that issue on mysql, but mysql is the php of databases so...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a documented feature of those databases. It is possible to workaround those cases, but it requires specific code and special configuration for each of the drivers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can. Tests pass without problems with the change.
How it will behave is tied to the schema of your database : it will be no problem if you have this column defined as autoincrement (which is the case here).
All examples of seed in phinx doc doesn't include a value for the PK fields (well, I suppose there is a PK since there's no schema provided)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the change to test the "call the same seeder multiple times" case, because I realized that with the PK defined, the values were not added on top of each other (it did an update).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we build relational fixtures then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, you can do it.
The tests from PR #181 passed with the PK hard-coded for all db drivers (however, we do not test against SQLServer).
But according to @lorenzo, you are probably going to need some special code depending on the db driver you are using.
Apparently for Postgres, you will have to manually update the sequence index to the next value - something postgres does automatically if you do not provide a value for a PK sequence in an INSERT.

As @josegonzalez said, maybe it would be a good thing to add a paragraph in the README / doc about this as it might confuse users.
For instance, I did not even know about this limitation for SQL Server and Postgres. From my experience, it does not cause any trouble with MySQL, so I thought it would not with the others...

'radix' => '10'
]
Expand Down
1 change: 0 additions & 1 deletion tests/test_app/config/Seeds/NumbersSeed.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ public function run()
{
$data = [
[
'id' => '1',
'number' => '10',
'radix' => '10'
]
Expand Down