From a093c28d1adf18d10208f05fca18e2e653f03b80 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Wed, 10 Sep 2014 12:01:01 +0200 Subject: [PATCH 1/5] [ticket/12963] Fix the list of migrations in database_update.php Without this patch the finder grab all the class available in the ext folder and not only the migrations. This change is backported for the one done before to the cli tool db:migrate. (see the commit 3420f8f3201ac337434f73ee00bda6df7b378212) PHPBB3-12963 --- phpBB/install/database_update.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 6a91033dbb..7299c0aa2a 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -181,7 +181,17 @@ $finder = $phpbb_extension_manager->get_finder(); $migrations = $finder ->core_path('phpbb/db/migration/data/') + ->extension_directory('/migrations') ->get_classes(); + +// @deprecated to be removed in 3.2 final +$migrations_deprecated = $phpbb_extension_manager + ->get_finder() + ->extension_directory('/migrations') + ->get_classes(); + +$migrations = array_merge($migrations, $migrations_deprecated); + $migrator->set_migrations($migrations); // What is a safe limit of execution time? Half the max execution time should be safe. From ff872a79707200ed7e3e7c25b4ebc7f6f4313b6c Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Wed, 10 Sep 2014 19:17:37 +0200 Subject: [PATCH 2/5] [ticket/12963] Don't use static var in \extension\base\get_migration_file_list The static var was global to all instance of \phpbb\base and so if two different instances (for two different extensions) were created by the same script they shared the same migrations list. PHPBB3-12963 --- phpBB/phpbb/extension/base.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/extension/base.php b/phpBB/phpbb/extension/base.php index 288fb7d19c..e0ccb4c65b 100644 --- a/phpBB/phpbb/extension/base.php +++ b/phpBB/phpbb/extension/base.php @@ -35,6 +35,9 @@ class base implements \phpbb\extension\extension_interface /** @var string */ protected $extension_path; + /** @var string[] */ + private $migrations = false; + /** * Constructor * @@ -122,11 +125,9 @@ class base implements \phpbb\extension\extension_interface */ protected function get_migration_file_list() { - static $migrations = false; - - if ($migrations !== false) + if ($this->migrations !== false) { - return $migrations; + return $this->migrations; } // Only have the finder search in this extension path directory From 72ee4b3a7c4fca67cb226dab3ed27a8afa903144 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Wed, 10 Sep 2014 12:08:13 +0200 Subject: [PATCH 3/5] [ticket/12963] Load extensions migrations from /migration PHPBB3-12963 --- phpBB/install/database_update.php | 6 +++--- phpBB/phpbb/console/command/db/migrate.php | 9 +++++++++ phpBB/phpbb/extension/base.php | 12 +++++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 7299c0aa2a..f0091ac995 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -177,11 +177,11 @@ $migrator = $phpbb_container->get('migrator'); $migrator->create_migrations_table(); $phpbb_extension_manager = $phpbb_container->get('ext.manager'); -$finder = $phpbb_extension_manager->get_finder(); -$migrations = $finder +$migrations = $phpbb_extension_manager + ->get_finder() ->core_path('phpbb/db/migration/data/') - ->extension_directory('/migrations') + ->extension_directory('/migration') ->get_classes(); // @deprecated to be removed in 3.2 final diff --git a/phpBB/phpbb/console/command/db/migrate.php b/phpBB/phpbb/console/command/db/migrate.php index c3caae5f70..a25886fb82 100644 --- a/phpBB/phpbb/console/command/db/migrate.php +++ b/phpBB/phpbb/console/command/db/migrate.php @@ -117,8 +117,17 @@ class migrate extends \phpbb\console\command\command $migrations = $this->extension_manager ->get_finder() ->core_path('phpbb/db/migration/data/') + ->extension_directory('/migration') + ->get_classes(); + + // @deprecated to be removed in 3.2 final + $migrations_deprecated = $this->extension_manager + ->get_finder() ->extension_directory('/migrations') ->get_classes(); + + $migrations = array_merge($migrations, $migrations_deprecated); + $this->migrator->set_migrations($migrations); } diff --git a/phpBB/phpbb/extension/base.php b/phpBB/phpbb/extension/base.php index e0ccb4c65b..9cd37ec8b2 100644 --- a/phpBB/phpbb/extension/base.php +++ b/phpBB/phpbb/extension/base.php @@ -132,9 +132,19 @@ class base implements \phpbb\extension\extension_interface // Only have the finder search in this extension path directory $migrations = $this->extension_finder + ->extension_directory('/migration') + ->find_from_extension($this->extension_name, $this->extension_path); + + $migrations = $this->extension_finder->get_classes_from_files($migrations); + + // @deprecated to be removed in 3.2 final + $migrations_deprecated = $this->extension_finder ->extension_directory('/migrations') ->find_from_extension($this->extension_name, $this->extension_path); - $migrations = $this->extension_finder->get_classes_from_files($migrations); + + $migrations_deprecated = $this->extension_finder->get_classes_from_files($migrations_deprecated); + + $migrations = array_merge($migrations, $migrations_deprecated); return $migrations; } From 04e0d305d0c822299204e5eb28f2571e7b128b9a Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Wed, 10 Sep 2014 19:21:00 +0200 Subject: [PATCH 4/5] [ticket/12963] Add unit tests PHPBB3-12963 --- .../ext/vendor2/bar/migration/migration.php | 18 ++++ .../ext/vendor2/bar/migrations/migration.php | 18 ++++ .../ext/vendor2/foo/migrations/migration.php | 18 ++++ .../ext/vendor3/bar/migration/migration.php | 18 ++++ tests/extension/extension_base_test.php | 85 +++++++++++++++++++ tests/mock/extension_manager.php | 3 +- tests/mock/migrator.php | 55 ++++++++++++ 7 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 tests/extension/ext/vendor2/bar/migration/migration.php create mode 100644 tests/extension/ext/vendor2/bar/migrations/migration.php create mode 100644 tests/extension/ext/vendor2/foo/migrations/migration.php create mode 100644 tests/extension/ext/vendor3/bar/migration/migration.php create mode 100644 tests/extension/extension_base_test.php create mode 100644 tests/mock/migrator.php diff --git a/tests/extension/ext/vendor2/bar/migration/migration.php b/tests/extension/ext/vendor2/bar/migration/migration.php new file mode 100644 index 0000000000..fc27656d10 --- /dev/null +++ b/tests/extension/ext/vendor2/bar/migration/migration.php @@ -0,0 +1,18 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace vendor2\bar\migration; + +class migration extends \phpbb\db\migration\migration +{ +} diff --git a/tests/extension/ext/vendor2/bar/migrations/migration.php b/tests/extension/ext/vendor2/bar/migrations/migration.php new file mode 100644 index 0000000000..71caa34fd9 --- /dev/null +++ b/tests/extension/ext/vendor2/bar/migrations/migration.php @@ -0,0 +1,18 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace vendor2\bar\migrations; + +class migration extends \phpbb\db\migration\migration +{ +} diff --git a/tests/extension/ext/vendor2/foo/migrations/migration.php b/tests/extension/ext/vendor2/foo/migrations/migration.php new file mode 100644 index 0000000000..63085497e3 --- /dev/null +++ b/tests/extension/ext/vendor2/foo/migrations/migration.php @@ -0,0 +1,18 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace vendor2\foo\migrations; + +class migration extends \phpbb\db\migration\migration +{ +} diff --git a/tests/extension/ext/vendor3/bar/migration/migration.php b/tests/extension/ext/vendor3/bar/migration/migration.php new file mode 100644 index 0000000000..e7280a7d5e --- /dev/null +++ b/tests/extension/ext/vendor3/bar/migration/migration.php @@ -0,0 +1,18 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace vendor3\bar\migration; + +class migration extends \phpbb\db\migration\migration +{ +} diff --git a/tests/extension/extension_base_test.php b/tests/extension/extension_base_test.php new file mode 100644 index 0000000000..5535c91fc5 --- /dev/null +++ b/tests/extension/extension_base_test.php @@ -0,0 +1,85 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ +require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; + +class phpbb_extension_extension_base_test extends phpbb_test_case +{ + protected static $reflection_method_get_migration_file_list; + + public static function setUpBeforeClass() + { + parent::setUpBeforeClass(); + + $reflection_class = new ReflectionClass('\phpbb\extension\base'); + self::$reflection_method_get_migration_file_list = $reflection_class->getMethod('get_migration_file_list'); + self::$reflection_method_get_migration_file_list->setAccessible(true); + } + + public function setUp() + { + $container = new phpbb_mock_container_builder(); + $migrator = new phpbb_mock_migrator(); + $container->set('migrator', $migrator); + + $this->extension_manager = new phpbb_mock_extension_manager( + dirname(__FILE__) . '/', + array( + 'vendor2/foo' => array( + 'ext_name' => 'vendor2/foo', + 'ext_active' => '1', + 'ext_path' => 'ext/vendor2/foo/', + ), + 'vendor3/bar' => array( + 'ext_name' => 'vendor3/bar', + 'ext_active' => '1', + 'ext_path' => 'ext/vendor3/bar/', + ), + 'vendor2/bar' => array( + 'ext_name' => 'vendor2/bar', + 'ext_active' => '1', + 'ext_path' => 'ext/vendor2/bar/', + ), + ), + $container); + } + + public function data_test_suffix_get_classes() + { + return array( + array( + 'vendor3/bar', + array('\vendor3\bar\migration\migration'), + ), + array( + 'vendor2/foo', + array('\vendor2\foo\migrations\migration'), + ), + array( + 'vendor2/bar', + array( + '\vendor2\bar\migration\migration', + '\vendor2\bar\migrations\migration', + ), + ), + ); + } + + /** + * @dataProvider data_test_suffix_get_classes + */ + public function test_suffix_get_classes($extension_name, $expected) + { + $extension = $this->extension_manager->get_extension($extension_name); + $this->assertEquals($expected, self::$reflection_method_get_migration_file_list->invoke($extension)); + } +} diff --git a/tests/mock/extension_manager.php b/tests/mock/extension_manager.php index 1a475f62e0..3b759fbbc2 100644 --- a/tests/mock/extension_manager.php +++ b/tests/mock/extension_manager.php @@ -13,11 +13,12 @@ class phpbb_mock_extension_manager extends \phpbb\extension\manager { - public function __construct($phpbb_root_path, $extensions = array()) + public function __construct($phpbb_root_path, $extensions = array(), $container = null) { $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = 'php'; $this->extensions = $extensions; $this->filesystem = new \phpbb\filesystem(); + $this->container = $container; } } diff --git a/tests/mock/migrator.php b/tests/mock/migrator.php new file mode 100644 index 0000000000..293f115335 --- /dev/null +++ b/tests/mock/migrator.php @@ -0,0 +1,55 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +class phpbb_mock_migrator extends \phpbb\db\migrator +{ + public function __construct() + { + } + + public function load_migration_state() + { + } + + public function set_migrations($class_names) + { + } + + public function update() + { + } + + public function revert($migration) + { + } + + public function unfulfillable($name) + { + } + + public function finished() + { + } + + public function migration_state($migration) + { + } + + public function populate_migrations($migrations) + { + } + + public function create_migrations_table() + { + } +} From 8b16d3141396a11db3ab4ade49af91f2477f5fb6 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Tue, 16 Sep 2014 20:18:10 +0200 Subject: [PATCH 5/5] [ticket/12963] Edit deprecation message PHPBB3-12963 --- phpBB/install/database_update.php | 2 +- phpBB/phpbb/console/command/db/migrate.php | 2 +- phpBB/phpbb/extension/base.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index f0091ac995..5cf01fec79 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -184,7 +184,7 @@ $migrations = $phpbb_extension_manager ->extension_directory('/migration') ->get_classes(); -// @deprecated to be removed in 3.2 final +// @deprecated 3.1.0-RC4 (To be removed: 3.2.0) $migrations_deprecated = $phpbb_extension_manager ->get_finder() ->extension_directory('/migrations') diff --git a/phpBB/phpbb/console/command/db/migrate.php b/phpBB/phpbb/console/command/db/migrate.php index a25886fb82..68638a9515 100644 --- a/phpBB/phpbb/console/command/db/migrate.php +++ b/phpBB/phpbb/console/command/db/migrate.php @@ -120,7 +120,7 @@ class migrate extends \phpbb\console\command\command ->extension_directory('/migration') ->get_classes(); - // @deprecated to be removed in 3.2 final + // @deprecated 3.1.0-RC4 (To be removed: 3.2.0) $migrations_deprecated = $this->extension_manager ->get_finder() ->extension_directory('/migrations') diff --git a/phpBB/phpbb/extension/base.php b/phpBB/phpbb/extension/base.php index 9cd37ec8b2..b74026e6ab 100644 --- a/phpBB/phpbb/extension/base.php +++ b/phpBB/phpbb/extension/base.php @@ -137,7 +137,7 @@ class base implements \phpbb\extension\extension_interface $migrations = $this->extension_finder->get_classes_from_files($migrations); - // @deprecated to be removed in 3.2 final + // @deprecated 3.1.0-RC4 (To be removed: 3.2.0) $migrations_deprecated = $this->extension_finder ->extension_directory('/migrations') ->find_from_extension($this->extension_name, $this->extension_path);