PluginManager から $app を削除, Abstract メソッド追加

[nanasess]

概要(Overview・Refs Issue)

• PluginManager から $app を削除
• AbstractPluginManager に abstract メソッド追加
• AbstractPluginManager::migrationSchema() を動くよう修正

テスト（Test)

• PluginManager 修正後のプラグインが動作することを確認

相談（Discussion）

$app を使いたい人もいると思うので、$app を動くようにした方がよいかも

diff --git a/src/Eccube/Plugin/AbstractPluginManager.php b/src/Eccube/Plugin/AbstractPluginManager.php
index 1af6b8173..8759f7dfc 100644
--- a/src/Eccube/Plugin/AbstractPluginManager.php
+++ b/src/Eccube/Plugin/AbstractPluginManager.php
@@ -13,16 +13,27 @@
 
 namespace Eccube\Plugin;
 
+use Eccube\Application;
 use Doctrine\DBAL\Migrations\Configuration\Configuration;
 use Doctrine\DBAL\Migrations\Migration;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
-class AbstractPluginManager
+abstract class AbstractPluginManager
 {
     const MIGRATION_TABLE_PREFIX = 'migration_';
 
-    public function migrationSchema($app, $migrationFilePath, $pluginCode, $version = null)
+    /**
+     * Migrate the schema.
+     *
+     * @param ContainerInterface $container
+     * @param string $migrationFilePath
+     * @param string $pluginCode
+     * @param string $version
+     * @return array An array of migration sql statements. This will be empty if the the $confirm callback declines to execute the migration
+     */
+    public function migrationSchema(ContainerInterface $container, $migrationFilePath, $pluginCode, $version = null)
     {
-        $config = new Configuration($app['db']);
+        $config = new Configuration($container->get('doctrine.dbal.connection'));
         $config->setMigrationsNamespace('DoctrineMigrations');
         $config->setMigrationsDirectory($migrationFilePath);
         $config->registerMigrationsFromDirectory($migrationFilePath);
@@ -31,6 +42,91 @@ class AbstractPluginManager
         $migration->setNoMigrationException(true);
         // null 又は 'last' を渡すと最新バージョンまでマイグレートする
         // 0か'first'を渡すと最初に戻る
-        $migration->migrate($version, false);
+        return $migration->migrate($version, false);
+    }
+
+    /**
+     * Install the plugin.
+     *
+     * @param array $meta
+     * @param Application $app
+     * @param ContainerInterface $container
+     */
+    public function install($meta = null, $app = null, ContainerInterface $container = null)
+    {
+        if ($meta !== null) {
+            assert(is_array($meta));
+        }
+        if ($app !== null) {
+            assert($app instanceof Application);
+        }
+    }
+
+    /**
+     * Update the plugin.
+     *
+     * @param array $meta
+     * @param Application $app
+     * @param ContainerInterface $container
+     */
+    public function update($meta = null, $app = null, ContainerInterface $container = null)
+    {
+        if ($meta !== null) {
+            assert(is_array($meta));
+        }
+        if ($app !== null) {
+            assert($app instanceof Application);
+        }
+    }
+
+    /**
+     * Enable the plugin.
+     *
+     * @param array $meta
+     * @param Application $app
+     * @param ContainerInterface $container
+     */
+    public function enable($meta = null, $app = null, ContainerInterface $container = null)
+    {
+        if ($meta !== null) {
+            assert(is_array($meta));
+        }
+        if ($app !== null) {
+            assert($app instanceof Application);
+        }
+    }
+
+    /**
+     * Disable the plugin.
+     *
+     * @param array $meta
+     * @param Application $app
+     * @param ContainerInterface $container
+     */
+    public function disable($meta = null, $app = null, ContainerInterface $container = null)
+    {
+        if ($meta !== null) {
+            assert(is_array($meta));
+        }
+        if ($app !== null) {
+            assert($app instanceof Application);
+        }
+    }
+
+    /**
+     * Uninstall the plugin.
+     *
+     * @param array $meta
+     * @param Application $app
+     * @param ContainerInterface $container
+     */
+    public function uninstall($meta = null, $app = null, ContainerInterface $container = null)
+    {
+        if ($meta !== null) {
+            assert(is_array($meta));
+        }
+        if ($app !== null) {
+            assert($app instanceof Application);
+        }
     }
 }
diff --git a/src/Eccube/Service/PluginService.php b/src/Eccube/Service/PluginService.php
index 7af88259e..4f5e9387b 100644
--- a/src/Eccube/Service/PluginService.php
+++ b/src/Eccube/Service/PluginService.php
@@ -124,6 +124,7 @@ class PluginService
         $this->environment = $eccubeConfig->get('kernel.environment');
         $this->container = $container;
         $this->cacheUtil = $cacheUtil;
+        $this->app = $this->container->get('app');
     }
 
     /**
@@ -446,7 +447,6 @@ class PluginService
         if (class_exists($class)) {
             $installer = new $class(); // マネージャクラスに所定のメソッドがある場合だけ実行する
             if (method_exists($installer, $method)) {
-                // FIXME appを削除.
                 $installer->$method($meta, $this->app, $this->container);
             }
         }

マイナーバージョン互換性保持のための制限事項チェックリスト

• 既存機能の仕様変更
• フックポイントの呼び出しタイミングの変更
• フックポイントのパラメータの削除・データ型の変更
• twigファイルに渡しているパラメータの削除・データ型の変更
• Serviceクラスの公開関数の、引数の削除・データ型の変更
• 入出力ファイル(CSVなど)のフォーマット変更

[kiy0taka]

abstractメソッドにすると各PluginManagerで全メソッドを書かないといけないので、abstractはなしでいいと思います。

[kiy0taka]

スキーマ更新は本体側で行うので、migrationSchemaメソッドは削除していいと思います。
#2546

[nanasess]

対応しました。

[kiy0taka]

MIGRATION_TABLE_PREFIX も不要かと思います。