From 307181ccc49b5d6362fffe2ab0d02e80c1c87b89 Mon Sep 17 00:00:00 2001 From: Lorenzo Pisani Date: Fri, 1 Jul 2011 23:18:05 -0700 Subject: [PATCH 1/4] Moving the util methods to the model (refs #11) --- classes/minion/migration/manager.php | 22 ++--- classes/minion/migration/util.php | 124 ------------------------ classes/model/minion/migration.php | 140 ++++++++++++++++++++++++--- 3 files changed, 138 insertions(+), 148 deletions(-) delete mode 100644 classes/minion/migration/util.php diff --git a/classes/minion/migration/manager.php b/classes/minion/migration/manager.php index 386688e..2ef2c62 100644 --- a/classes/minion/migration/manager.php +++ b/classes/minion/migration/manager.php @@ -1,8 +1,8 @@ @@ -58,7 +58,7 @@ class Minion_Migration_Manager { /** * Set the database connection to be used - * + * * @param Kohana_Database Database connection * @return Minion_Migration_Manager */ @@ -96,7 +96,7 @@ class Minion_Migration_Manager { } /** - * Returns a set of queries that would've been executed had dry run not been + * Returns a set of queries that would've been executed had dry run not been * enabled. If dry run was not enabled, this returns an empty array * * @return array SQL Queries @@ -141,20 +141,20 @@ class Minion_Migration_Manager { return; } - $filename = Minion_Migration_Util::get_filename_from_migration($migration); + $filename = $this->_model->get_filename_from_migration($migration); if ( ! ($file = Kohana::find_file('migrations', $filename, FALSE))) { throw new Kohana_Exception( - 'Cannot load migration :migration (:file)', + 'Cannot load migration :migration (:file)', array( - ':migration' => $migration['id'], + ':migration' => $migration['id'], ':file' => $filename ) ); } - $class = Minion_Migration_Util::get_class_from_migration($migration); + $class = $this->_model->get_class_from_migration($migration); include_once $file; @@ -162,7 +162,7 @@ class Minion_Migration_Manager { $db = $this->_get_db_instance($instance->get_database_connection()); - try + try { $instance->$method($db); } @@ -204,7 +204,7 @@ class Minion_Migration_Manager { // If this migration has since been deleted if (isset($installed[$migration]) AND ! isset($available[$migration])) { - // We should only delete a record of this migration if it does + // We should only delete a record of this migration if it does // not exist in the "real world" if ($installed[$migration]['applied'] === '0') { @@ -216,7 +216,7 @@ class Minion_Migration_Manager { { $this->_model->add_migration($available[$migration]); } - // Somebody changed the description of the migration, make sure we + // Somebody changed the description of the migration, make sure we // update it in the db as we use this to build the filename! elseif ($installed[$migration]['description'] !== $available[$migration]['description']) { diff --git a/classes/minion/migration/util.php b/classes/minion/migration/util.php deleted file mode 100644 index 47caeb2..0000000 --- a/classes/minion/migration/util.php +++ /dev/null @@ -1,124 +0,0 @@ - - */ -class Minion_Migration_Util { - - /** - * Parses a set of files generated by Kohana::find_files and compiles it - * down into an array of migrations - * - * @param array Available files - * @return array Available Migrations - */ - public static function compile_migrations_from_files(array $files) - { - $migrations = array(); - - foreach ($files as $file => $path) - { - // If this is a directory we're dealing with - if (is_array($path)) - { - $migrations += Minion_Migration_Util::compile_migrations_from_files($path); - } - else - { - $migration = Minion_Migration_Util::get_migration_from_filename($file); - - $migrations[$migration['group'].':'.$migration['timestamp']] = $migration; - } - } - - return $migrations; - } - - /** - * Extracts information about a migration from its filename. - * - * Returns an array like: - * - * array( - * 'group' => 'mygroup', - * 'timestamp' => '1293214439', - * 'description' => 'initial-setup', - * 'id' => 'mygroup:1293214439' - * ); - * - * @param string The migration's filename - * @return array Array of components about the migration - */ - public static function get_migration_from_filename($file) - { - $migration = array(); - - // Get rid of the file's "migrations/" prefix, the file extension and then - // the filename itself. The "group" is essentially a slash delimited - // path from the migrations folder to the migration file - $migration['group'] = dirname(substr($file, 11, -strlen(EXT))); - - if(strpos(basename($file), "_")) - { - list($migration['timestamp'], $migration['description']) - = explode('_', basename($file, EXT), 2); - } - else - { - $migration['timestamp'] = basename($file, EXT); - $migration['description'] = ""; - } - $migration['id'] = $migration['group'].':'.$migration['timestamp']; - - return $migration; - } - - /** - * Gets a migration file from its timestamp, description and group - * - * @param integer|array The migration's ID or an array of timestamp, description - * @param string The migration group - * @return string Path to the migration file - */ - public static function get_filename_from_migration(array $migration) - { - $group = $migration['group']; - - if(!empty($migration['description'])) - { - $migration = $migration['timestamp'].'_'.$migration['description']; - } - else - { - $migration = $migration['timestamp']; - } - - $group = ( ! empty($group)) ? (rtrim($group, '/').'/') : ''; - - return $group.$migration.EXT; - } - - /** - * Allows you to work out the class name from either an array of migration - * info, or from a migration id - * - * @param string|array The migration's ID or array of migration data - * @return string The migration class name - */ - public static function get_class_from_migration($migration) - { - if (is_string($migration)) - { - $migration = str_replace(array(':', '/'), ' ', $migration); - } - else - { - $migration = str_replace('/', ' ', $migration['group']).'_'.$migration['timestamp']; - } - - return 'Migration_'.str_replace(array(' ', '-'), '_', ucwords($migration)); - } -} diff --git a/classes/model/minion/migration.php b/classes/model/minion/migration.php index d90c15f..ee33c8c 100644 --- a/classes/model/minion/migration.php +++ b/classes/model/minion/migration.php @@ -18,7 +18,7 @@ class Model_Minion_Migration extends Model protected $_table = 'minion_migrations'; /** - * Constructs the model, taking a Database connection as the first and only + * Constructs the model, taking a Database connection as the first and only * parameter * * @param Kohana_Database Database connection to use @@ -37,11 +37,125 @@ class Model_Minion_Migration extends Model { $files = Kohana::list_files('migrations'); - return Minion_Migration_Util::compile_migrations_from_files($files); + return $this->compile_migrations_from_files($files); } /** - * Checks to see if the minion migrations table exists and attempts to + * Parses a set of files generated by Kohana::find_files and compiles it + * down into an array of migrations + * + * @param array Available files + * @return array Available Migrations + */ + public function compile_migrations_from_files(array $files) + { + $migrations = array(); + + foreach ($files as $file => $path) + { + // If this is a directory we're dealing with + if (is_array($path)) + { + $migrations += $this->compile_migrations_from_files($path); + } + else + { + $migration = $this->get_migration_from_filename($file); + + $migrations[$migration['group'].':'.$migration['timestamp']] = $migration; + } + } + + return $migrations; + } + + /** + * Extracts information about a migration from its filename. + * + * Returns an array like: + * + * array( + * 'group' => 'mygroup', + * 'timestamp' => '1293214439', + * 'description' => 'initial-setup', + * 'id' => 'mygroup:1293214439' + * ); + * + * @param string The migration's filename + * @return array Array of components about the migration + */ + public function get_migration_from_filename($file) + { + $migration = array(); + + // Get rid of the file's "migrations/" prefix, the file extension and then + // the filename itself. The "group" is essentially a slash delimited + // path from the migrations folder to the migration file + $migration['group'] = dirname(substr($file, 11, -strlen(EXT))); + + if(strpos(basename($file), "_")) + { + list($migration['timestamp'], $migration['description']) + = explode('_', basename($file, EXT), 2); + } + else + { + $migration['timestamp'] = basename($file, EXT); + $migration['description'] = ""; + } + $migration['id'] = $migration['group'].':'.$migration['timestamp']; + + return $migration; + } + + /** + * Gets a migration file from its timestamp, description and group + * + * @param integer|array The migration's ID or an array of timestamp, description + * @param string The migration group + * @return string Path to the migration file + */ + public function get_filename_from_migration(array $migration) + { + $group = $migration['group']; + + if(!empty($migration['description'])) + { + $migration = $migration['timestamp'].'_'.$migration['description']; + } + else + { + $migration = $migration['timestamp']; + } + + $group = ( ! empty($group)) ? (rtrim($group, '/').'/') : ''; + + return $group.$migration.EXT; + } + + /** + * Allows you to work out the class name from either an array of migration + * info, or from a migration id + * + * @param string|array The migration's ID or array of migration data + * @return string The migration class name + */ + public function get_class_from_migration($migration) + { + if (is_string($migration)) + { + $migration = str_replace(array(':', '/'), ' ', $migration); + } + else + { + $migration = str_replace('/', ' ', $migration['group']).'_'.$migration['timestamp']; + } + + return 'Migration_'.str_replace(array(' ', '-'), '_', ucwords($migration)); + } + + /** + * Checks to see if the minion migrations table exists and attempts to * create it if it doesn't * * @return boolean @@ -101,8 +215,8 @@ class Model_Minion_Migration extends Model } /** - * Creates a new select query which includes all fields in the migrations - * table plus a `id` field which is a combination of the timestamp and the + * Creates a new select query which includes all fields in the migrations + * table plus a `id` field which is a combination of the timestamp and the * description * * @return Database_Query_Builder_Select @@ -188,7 +302,7 @@ class Model_Minion_Migration extends Model public function update_migration(array $current, array $new) { $set = array(); - + foreach ($new as $key => $value) { if ($key !== 'id' AND $current[$key] !== $value) @@ -242,7 +356,7 @@ class Model_Minion_Migration extends Model /** * Fetches the latest version for all installed groups * - * If a group does not have any applied migrations then no result will be + * If a group does not have any applied migrations then no result will be * returned for it * * @return Kohana_Database_Result @@ -265,7 +379,7 @@ class Model_Minion_Migration extends Model /** * Fetches a list of groups * - * @return array + * @return array */ public function fetch_groups($group_as_key = FALSE) { @@ -277,7 +391,7 @@ class Model_Minion_Migration extends Model } /** - * Fetch a list of migrations that need to be applied in order to reach the + * Fetch a list of migrations that need to be applied in order to reach the * required version * * @param string The groups to get migrations for @@ -334,7 +448,7 @@ class Model_Minion_Migration extends Model $query->where('timestamp', '>=', $target); } } - + } // Absolute timestamp else @@ -353,7 +467,7 @@ class Model_Minion_Migration extends Model $query->where('timestamp', '>', $target); } } - + // If we're migrating up if ($up) { @@ -408,7 +522,7 @@ class Model_Minion_Migration extends Model $timestamp = $group_applied ? $statuses[$group]['timestamp'] : NULL; $amount = substr($target, 1); $up = $target[0] === '+'; - + if ($up) { if ($group_applied) @@ -421,7 +535,7 @@ class Model_Minion_Migration extends Model if ( ! $group_applied) { throw new Kohana_Exception( - "Cannot migrate group :group down as none of its migrations have been applied", + "Cannot migrate group :group down as none of its migrations have been applied", array(':group' => $group) ); } From 0c65956d6bfaa45c14345c16a3a7e9c603752759 Mon Sep 17 00:00:00 2001 From: Lorenzo Pisani Date: Fri, 1 Jul 2011 23:22:23 -0700 Subject: [PATCH 2/4] adjusting the old util classes to run in the model tests (refs #11) --- tests/minion/migration/model.php | 166 +++++++++++++++++++++++++++++- tests/minion/migration/util.php | 169 ------------------------------- 2 files changed, 162 insertions(+), 173 deletions(-) delete mode 100644 tests/minion/migration/util.php diff --git a/tests/minion/migration/model.php b/tests/minion/migration/model.php index 76dad70..277e7ea 100644 --- a/tests/minion/migration/model.php +++ b/tests/minion/migration/model.php @@ -44,7 +44,7 @@ class Minion_Migration_ModelTest extends Kohana_Unittest_Database_TestCase } /** - * Get an instance of the migration model, pre-loaded with a connection to + * Get an instance of the migration model, pre-loaded with a connection to * the test database * * @return Model_Minion_Migration @@ -331,7 +331,7 @@ class Minion_Migration_ModelTest extends Kohana_Unittest_Database_TestCase } /** - * Tests that Model_Minion_Migration::get_migration can get a migration from + * Tests that Model_Minion_Migration::get_migration can get a migration from * the database * * @test @@ -363,7 +363,7 @@ class Minion_Migration_ModelTest extends Kohana_Unittest_Database_TestCase } /** - * If invalid input is passed to get_migration then it should throw an + * If invalid input is passed to get_migration then it should throw an * exception * * @test @@ -418,7 +418,7 @@ class Minion_Migration_ModelTest extends Kohana_Unittest_Database_TestCase } /** - * Tests that Model_Minion_Migration::mark_migration() changes the applied + * Tests that Model_Minion_Migration::mark_migration() changes the applied * status of a migration * * @test @@ -495,4 +495,162 @@ class Minion_Migration_ModelTest extends Kohana_Unittest_Database_TestCase $this->getModel()->resolve_target( (array) $group, $target) ); } + + /** + * Provides test data for test_compile_migrations_from_files() + * + * @return array + */ + public function provider_compile_migrations_from_files() + { + return array( + array( + array( + 'myapp:015151051' => array('group' => 'myapp', 'description' => 'setup', 'timestamp' => '015151051', 'id' => 'myapp:015151051'), + 'myapp:015161051' => array('group' => 'myapp', 'description' => 'add-comments', 'timestamp' => '015161051', 'id' => 'myapp:015161051'), + ), + array( + 'migrations/myapp' => array( + 'migrations/myapp/015151051_setup.php' + => '/var/www/app/groups/myapp/migrations/myapp/015151051_setup.php', + 'migrations/myapp/015161051_add-comments.php' + => '/var/www/app/groups/myapp/migrations/myapp/015161051_add-comments.php', + ), + ) + ), + ); + } + + /** + * Test that Minion_Migration_Util::compile_migrations_from_files accurately + * compiles a set of files down into a set of migration files + * + * @test + * @covers Minion_Migration_Util::compile_migrations_from_files + * @dataProvider provider_compile_migrations_from_files + * @param array Expected output + * @param array Input Files + */ + public function test_compile_migrations_from_files($expected, array $files) + { + $this->assertSame( + $expected, + $this->getModel()->compile_migrations_from_files($files) + ); + } + + /** + * Provides test data for test_extract_migration_info_from_filename + * + * @return array Test Data + */ + public function provider_get_migration_from_filename() + { + return array( + array( + array( + 'group' => 'myapp', + 'description' => 'initial-setup', + 'timestamp' => '1293214439', + 'id' => 'myapp:1293214439', + ), + 'migrations/myapp/1293214439_initial-setup.php', + ), + ); + } + + /** + * Tests that Minion_Migration_Util::get_migration_info_from_filename() + * correctly extracts information about the migration from its filename + * + * @test + * @covers Minion_Migration_Util::get_migration_from_filename + * @dataProvider provider_get_migration_from_filename + * @param array Expected output + * @param string Input filename + */ + public function test_get_migration_from_filename($expected, $file) + { + $this->assertSame( + $expected, + $this->getModel()->get_migration_from_filename($file) + ); + } + + /** + * Provides test data for test_convert_migration_to_filename + * + * @return array Test Data + */ + public function provider_get_filename_from_migration() + { + return array( + array( + 'myapp/1293214439_initial-setup.php', + array( + 'group' => 'myapp', + 'timestamp' => '1293214439', + 'description' => 'initial-setup', + 'id' => 'myapp:1293214439' + ), + 'myapp', + ), + ); + } + + /** + * Tests that Minion_Migration_Util::get_filename_from_migration generates + * accurate filenames when given a variety of migration information + * + * @test + * @covers Minion_Migration_Util::get_filename_from_migration + * @dataProvider provider_get_filename_from_migration + * @param string Expected output + * @param mixed Migration id + * @param mixed group + */ + public function test_get_filename_from_migration($expected, $migration, $group) + { + $this->assertSame( + $expected, + $this->getModel()->get_filename_from_migration($migration, $group) + ); + } + + /** + * Provides test data for test_get_class_from_migration + * + * @return array Test Data + */ + public function provider_get_class_from_migration() + { + return array( + array( + 'Migration_Kohana_201012290258', + 'kohana:201012290258', + ), + array( + 'Migration_Kohana_201012290258', + array('group' => 'kohana', 'timestamp' => '201012290258'), + ), + ); + } + + /** + * Tests that Minion_Migration_Util::get_class_from_migration can generate + * a class name from information about a migration + * + * @test + * @covers Minion_Migration_Util::get_class_from_migration + * @dataProvider provider_get_class_from_migration + * @param string Expected output + * @param string|array Migration info + */ + public function test_get_class_from_migration($expected, $migration) + { + $this->assertSame( + $expected, + $this->getModel()->get_class_from_migration($migration) + ); + } } diff --git a/tests/minion/migration/util.php b/tests/minion/migration/util.php deleted file mode 100644 index 7f78912..0000000 --- a/tests/minion/migration/util.php +++ /dev/null @@ -1,169 +0,0 @@ - array('group' => 'myapp', 'description' => 'setup', 'timestamp' => '015151051', 'id' => 'myapp:015151051'), - 'myapp:015161051' => array('group' => 'myapp', 'description' => 'add-comments', 'timestamp' => '015161051', 'id' => 'myapp:015161051'), - ), - array( - 'migrations/myapp' => array( - 'migrations/myapp/015151051_setup.php' - => '/var/www/app/groups/myapp/migrations/myapp/015151051_setup.php', - 'migrations/myapp/015161051_add-comments.php' - => '/var/www/app/groups/myapp/migrations/myapp/015161051_add-comments.php', - ), - ) - ), - ); - } - - /** - * Test that Minion_Migration_Util::compile_migrations_from_files accurately - * compiles a set of files down into a set of migration files - * - * @test - * @covers Minion_Migration_Util::compile_migrations_from_files - * @dataProvider provider_compile_migrations_from_files - * @param array Expected output - * @param array Input Files - */ - public function test_compile_migrations_from_files($expected, array $files) - { - $this->assertSame( - $expected, - Minion_Migration_Util::compile_migrations_from_files($files) - ); - } - - /** - * Provides test data for test_extract_migration_info_from_filename - * - * @return array Test Data - */ - public function provider_get_migration_from_filename() - { - return array( - array( - array( - 'group' => 'myapp', - 'description' => 'initial-setup', - 'timestamp' => '1293214439', - 'id' => 'myapp:1293214439', - ), - 'migrations/myapp/1293214439_initial-setup.php', - ), - ); - } - - /** - * Tests that Minion_Migration_Util::get_migration_info_from_filename() - * correctly extracts information about the migration from its filename - * - * @test - * @covers Minion_Migration_Util::get_migration_from_filename - * @dataProvider provider_get_migration_from_filename - * @param array Expected output - * @param string Input filename - */ - public function test_get_migration_from_filename($expected, $file) - { - $this->assertSame( - $expected, - Minion_Migration_Util::get_migration_from_filename($file) - ); - } - - /** - * Provides test data for test_convert_migration_to_filename - * - * @return array Test Data - */ - public function provider_get_filename_from_migration() - { - return array( - array( - 'myapp/1293214439_initial-setup.php', - array( - 'group' => 'myapp', - 'timestamp' => '1293214439', - 'description' => 'initial-setup', - 'id' => 'myapp:1293214439' - ), - 'myapp', - ), - ); - } - - /** - * Tests that Minion_Migration_Util::get_filename_from_migration generates - * accurate filenames when given a variety of migration information - * - * @test - * @covers Minion_Migration_Util::get_filename_from_migration - * @dataProvider provider_get_filename_from_migration - * @param string Expected output - * @param mixed Migration id - * @param mixed group - */ - public function test_get_filename_from_migration($expected, $migration, $group) - { - $this->assertSame( - $expected, - Minion_Migration_Util::get_filename_from_migration($migration, $group) - ); - } - - /** - * Provides test data for test_get_class_from_migration - * - * @return array Test Data - */ - public function provider_get_class_from_migration() - { - return array( - array( - 'Migration_Kohana_201012290258', - 'kohana:201012290258', - ), - array( - 'Migration_Kohana_201012290258', - array('group' => 'kohana', 'timestamp' => '201012290258'), - ), - ); - } - - /** - * Tests that Minion_Migration_Util::get_class_from_migration can generate - * a class name from information about a migration - * - * @test - * @covers Minion_Migration_Util::get_class_from_migration - * @dataProvider provider_get_class_from_migration - * @param string Expected output - * @param string|array Migration info - */ - public function test_get_class_from_migration($expected, $migration) - { - $this->assertSame( - $expected, - Minion_Migration_Util::get_class_from_migration($migration) - ); - } -} From cc193c43c6e7b5d6951b4081273fff92401fecda Mon Sep 17 00:00:00 2001 From: Lorenzo Pisani Date: Fri, 1 Jul 2011 23:30:15 -0700 Subject: [PATCH 3/4] filtering the files in the migrations folder (refs #18) --- classes/model/minion/migration.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/classes/model/minion/migration.php b/classes/model/minion/migration.php index ee33c8c..32e5bcd 100644 --- a/classes/model/minion/migration.php +++ b/classes/model/minion/migration.php @@ -60,6 +60,10 @@ class Model_Minion_Migration extends Model } else { + // Skip files without an extension of EXT + if ('.'.pathinfo($file, PATHINFO_EXTENSION) !== EXT) + continue; + $migration = $this->get_migration_from_filename($file); $migrations[$migration['group'].':'.$migration['timestamp']] = $migration; From 0f201b0906a2bc8325e113053ac32f897b326461 Mon Sep 17 00:00:00 2001 From: Lorenzo Pisani Date: Fri, 1 Jul 2011 23:32:31 -0700 Subject: [PATCH 4/4] adding a test for sql files in the migrations folder (refs #18) --- tests/minion/migration/model.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/minion/migration/model.php b/tests/minion/migration/model.php index 277e7ea..5a69223 100644 --- a/tests/minion/migration/model.php +++ b/tests/minion/migration/model.php @@ -511,6 +511,9 @@ class Minion_Migration_ModelTest extends Kohana_Unittest_Database_TestCase ), array( 'migrations/myapp' => array( + // This file should be ignored + 'migrations/myapp/015151051_setup.sql' + => '/var/www/app/groups/myapp/migrations/myapp/015151051_setup.sql', 'migrations/myapp/015151051_setup.php' => '/var/www/app/groups/myapp/migrations/myapp/015151051_setup.php', 'migrations/myapp/015161051_add-comments.php'