From 904de5b39aa20e692e9b2d1dcda77736b5928dc2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 6 Apr 2014 14:50:22 +0200 Subject: [PATCH 1/3] [ticket/12362] Throw exception in schema generator on unresolvable dependency Make sure we throw an exception in the schema generator if we come across an unresolvable dependency. Otherwise we'll get stuck in a infinite loop that needs to be cancelled by the user or the maximum execution time. PHPBB3-12362 --- phpBB/phpbb/db/migration/schema_generator.php | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/phpBB/phpbb/db/migration/schema_generator.php b/phpBB/phpbb/db/migration/schema_generator.php index a7e2fa8f06..dbb50e80ca 100644 --- a/phpBB/phpbb/db/migration/schema_generator.php +++ b/phpBB/phpbb/db/migration/schema_generator.php @@ -40,6 +40,9 @@ class schema_generator /** @var array */ protected $tables; + /** @var array */ + protected $dependencies = array(); + /** * Constructor */ @@ -69,11 +72,13 @@ class schema_generator $migrations = $this->class_names; $tree = array(); + $check_dependencies = true; while (!empty($migrations)) { foreach ($migrations as $migration_class) { $open_dependencies = array_diff($migration_class::depends_on(), $tree); + if (empty($open_dependencies)) { $migration = new $migration_class($this->config, $this->db, $this->db_tools, $this->phpbb_root_path, $this->php_ext, $this->table_prefix); @@ -170,10 +175,41 @@ class schema_generator } unset($migrations[$migration_key]); } + else if ($check_dependencies) + { + $this->dependencies = array_merge($this->dependencies, $open_dependencies); + } + } + + // Only run this check after the first run + if ($check_dependencies) + { + $this->check_dependencies(); + $check_dependencies = false; } } ksort($this->tables); return $this->tables; } + + /** + * Check if one of the migrations files' dependencies can't be resolved + * by the supplied list of migrations + * + * @throws UnexpectedValueException If a dependency can't be resolved + */ + protected function check_dependencies() + { + // Strip duplicate values from array + $this->dependencies = array_unique($this->dependencies); + + foreach ($this->dependencies as $dependency) + { + if (!in_array($dependency, $this->class_names)) + { + throw new \UnexpectedValueException("Unable to resolve the dependency '$dependency'"); + } + } + } } From f123512280adbde8196ded39bdcaf8bd1a30bde0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 6 Apr 2014 20:33:12 +0200 Subject: [PATCH 2/3] [ticket/12362] Add tests for schema generator PHPBB3-12362 --- tests/migrator/schema_generator_test.php | 55 ++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 tests/migrator/schema_generator_test.php diff --git a/tests/migrator/schema_generator_test.php b/tests/migrator/schema_generator_test.php new file mode 100644 index 0000000000..9f9213779c --- /dev/null +++ b/tests/migrator/schema_generator_test.php @@ -0,0 +1,55 @@ +createXMLDataSet(dirname(__FILE__) . '/../dbal/fixtures/config.xml'); + } + + public function setUp() + { + parent::setUp(); + + $this->config = new \phpbb\config\config(array()); + $this->db = $this->new_dbal(); + $this->db_tools = new \phpbb\db\tools($this->db); + $this->table_prefix = 'phpbb_'; + } + + protected function get_schema_generator(array $class_names) + { + $this->generator = new \phpbb\db\migration\schema_generator($class_names, $this->config, $this->db, $this->db_tools, $this->phpbb_root_path, $this->php_ext, $this->table_prefix); + + return $this->generator; + } + + /** + * @expectedException \UnexpectedValueException + */ + public function test_check_dependencies_fail() + { + $this->get_schema_generator(array('\phpbb\db\migration\data\v310\forgot_password')); + + $this->generator->get_schema(); + } + + public function test_get_schema_success() + { + $this->get_schema_generator(array( + '\phpbb\db\migration\data\v30x\release_3_0_1_rc1', + '\phpbb\db\migration\data\v30x\release_3_0_0', + '\phpbb\db\migration\data\v310\boardindex' + )); + + $this->assertArrayHasKey('phpbb_users', $this->generator->get_schema()); + } +} From 44a9bfc07142a023e7e5f62245f2665be9f09359 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 6 Apr 2014 22:04:00 +0200 Subject: [PATCH 3/3] [ticket/12362] Do not use database test case as it's not needed PHPBB3-12362 --- tests/migrator/schema_generator_test.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/migrator/schema_generator_test.php b/tests/migrator/schema_generator_test.php index 9f9213779c..4bac447229 100644 --- a/tests/migrator/schema_generator_test.php +++ b/tests/migrator/schema_generator_test.php @@ -7,20 +7,14 @@ * */ -class schmema_generator_test extends phpbb_database_test_case +class schmema_generator_test extends phpbb_test_case { - // Use some bogus fixture as we don't actually need to access the database - public function getDataSet() - { - return $this->createXMLDataSet(dirname(__FILE__) . '/../dbal/fixtures/config.xml'); - } - public function setUp() { parent::setUp(); $this->config = new \phpbb\config\config(array()); - $this->db = $this->new_dbal(); + $this->db = new \phpbb\db\driver\sqlite(); $this->db_tools = new \phpbb\db\tools($this->db); $this->table_prefix = 'phpbb_'; }