From 5a55d434ab1e0bc6e7725a7cfdb3e707da2138c6 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 11 Jun 2018 20:04:12 +0100 Subject: [PATCH] Nuke plugin loaders from orbit This features a near-total rewrite of PluginLoaders and some code associated with them. Highlights: - PluginManager->registerInterface() does not return anything, and now accepts a PluginLoader instance instead of a string. - PluginLoader itself is drastically simplified. getPluginFilters(), enablePlugin() and disablePlugin() are now removed. loadPlugin() responsibilities are now solely confined to doing whatever is necessary to make the plugin's classes visible by the server, and does not emit log messages or check for data directories. - PluginBase->init() and PluginBase->isInitialized() have been removed. - Plugin interface now declares a signature for the constructor which implementations must comply with. - Plugin interface now declares setEnabled(). --- src/pocketmine/Server.php | 8 +- src/pocketmine/plugin/PharPluginLoader.php | 95 +++----------------- src/pocketmine/plugin/Plugin.php | 8 +- src/pocketmine/plugin/PluginBase.php | 43 ++++----- src/pocketmine/plugin/PluginLoader.php | 41 ++------- src/pocketmine/plugin/PluginManager.php | 75 ++++++++++------ src/pocketmine/plugin/ScriptPluginLoader.php | 91 ++----------------- tests/plugins/PocketMine-DevTools | 2 +- 8 files changed, 101 insertions(+), 262 deletions(-) diff --git a/src/pocketmine/Server.php b/src/pocketmine/Server.php index a45547e45..75cf7debc 100644 --- a/src/pocketmine/Server.php +++ b/src/pocketmine/Server.php @@ -1639,8 +1639,8 @@ class Server{ $this->pluginManager = new PluginManager($this, $this->commandMap); $this->pluginManager->subscribeToPermission(Server::BROADCAST_CHANNEL_ADMINISTRATIVE, $this->consoleSender); $this->profilingTickRate = (float) $this->getProperty("settings.profile-report-trigger", 20); - $this->pluginManager->registerInterface(PharPluginLoader::class); - $this->pluginManager->registerInterface(ScriptPluginLoader::class); + $this->pluginManager->registerInterface(new PharPluginLoader($this->autoloader)); + $this->pluginManager->registerInterface(new ScriptPluginLoader()); register_shutdown_function([$this, "crashDump"]); @@ -1994,8 +1994,8 @@ class Server{ $this->getNetwork()->blockAddress($entry->getName(), -1); } - $this->pluginManager->registerInterface(PharPluginLoader::class); - $this->pluginManager->registerInterface(ScriptPluginLoader::class); + $this->pluginManager->registerInterface(new PharPluginLoader($this->autoloader)); + $this->pluginManager->registerInterface(new ScriptPluginLoader()); $this->pluginManager->loadPlugins($this->pluginPath); $this->enablePlugins(PluginLoadOrder::STARTUP); $this->enablePlugins(PluginLoadOrder::POSTWORLD); diff --git a/src/pocketmine/plugin/PharPluginLoader.php b/src/pocketmine/plugin/PharPluginLoader.php index 40c7a9064..a257f8b92 100644 --- a/src/pocketmine/plugin/PharPluginLoader.php +++ b/src/pocketmine/plugin/PharPluginLoader.php @@ -32,45 +32,25 @@ use pocketmine\Server; */ class PharPluginLoader implements PluginLoader{ - /** @var Server */ - private $server; + /** @var \ClassLoader */ + private $loader; - /** - * @param Server $server - */ - public function __construct(Server $server){ - $this->server = $server; + public function __construct(\ClassLoader $loader){ + $this->loader = $loader; + } + + public function canLoadPlugin(string $path) : bool{ + $ext = ".phar"; + return is_file($path) and substr($path, -strlen($ext)) === $ext; } /** * Loads the plugin contained in $file * * @param string $file - * - * @return Plugin|null */ - public function loadPlugin(string $file){ - if(($description = $this->getPluginDescription($file)) instanceof PluginDescription){ - $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.load", [$description->getFullName()])); - $dataFolder = dirname($file) . DIRECTORY_SEPARATOR . $description->getName(); - if(file_exists($dataFolder) and !is_dir($dataFolder)){ - throw new \InvalidStateException("Projected dataFolder '" . $dataFolder . "' for " . $description->getName() . " exists and is not a directory"); - } - $file = "phar://$file"; - $className = $description->getMain(); - $this->server->getLoader()->addPath("$file/src"); - - if(class_exists($className, true)){ - $plugin = new $className(); - $this->initPlugin($plugin, $description, $dataFolder, $file); - - return $plugin; - }else{ - throw new PluginException("Couldn't load plugin " . $description->getName() . ": main class not found"); - } - } - - return null; + public function loadPlugin(string $file) : void{ + $this->loader->addPath("phar://$file/src"); } /** @@ -80,7 +60,7 @@ class PharPluginLoader implements PluginLoader{ * * @return null|PluginDescription */ - public function getPluginDescription(string $file){ + public function getPluginDescription(string $file) : ?PluginDescription{ $phar = new \Phar($file); if(isset($phar["plugin.yml"])){ $pluginYml = $phar["plugin.yml"]; @@ -91,55 +71,4 @@ class PharPluginLoader implements PluginLoader{ return null; } - - /** - * Returns the filename patterns that this loader accepts - * - * @return string - */ - public function getPluginFilters() : string{ - return "/\\.phar$/i"; - } - - public function canLoadPlugin(string $path) : bool{ - $ext = ".phar"; - return is_file($path) and substr($path, -strlen($ext)) === $ext; - } - - /** - * @param PluginBase $plugin - * @param PluginDescription $description - * @param string $dataFolder - * @param string $file - */ - private function initPlugin(PluginBase $plugin, PluginDescription $description, string $dataFolder, string $file){ - $plugin->init($this, $this->server, $description, $dataFolder, $file); - $plugin->onLoad(); - } - - /** - * @param Plugin $plugin - */ - public function enablePlugin(Plugin $plugin){ - if($plugin instanceof PluginBase and !$plugin->isEnabled()){ - $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.enable", [$plugin->getDescription()->getFullName()])); - - $plugin->setEnabled(true); - - $this->server->getPluginManager()->callEvent(new PluginEnableEvent($plugin)); - } - } - - /** - * @param Plugin $plugin - */ - public function disablePlugin(Plugin $plugin){ - if($plugin instanceof PluginBase and $plugin->isEnabled()){ - $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.disable", [$plugin->getDescription()->getFullName()])); - - $this->server->getPluginManager()->callEvent(new PluginDisableEvent($plugin)); - - $plugin->setEnabled(false); - } - } } diff --git a/src/pocketmine/plugin/Plugin.php b/src/pocketmine/plugin/Plugin.php index 867b9a046..77d5dd964 100644 --- a/src/pocketmine/plugin/Plugin.php +++ b/src/pocketmine/plugin/Plugin.php @@ -34,10 +34,11 @@ use pocketmine\utils\Config; /** * It is recommended to use PluginBase for the actual plugin - * */ interface Plugin extends CommandExecutor{ + public function __construct(PluginLoader $loader, Server $server, PluginDescription $description, string $dataFolder, string $file); + /** * Called when the plugin is loaded, before calling onEnable() */ @@ -53,6 +54,11 @@ interface Plugin extends CommandExecutor{ */ public function isEnabled() : bool; + /** + * @param bool $enabled + */ + public function setEnabled(bool $enabled = true) : void; + /** * Called when the plugin is disabled * Use this to free open things and finish actions diff --git a/src/pocketmine/plugin/PluginBase.php b/src/pocketmine/plugin/PluginBase.php index cca32d8b6..9a78cdddd 100644 --- a/src/pocketmine/plugin/PluginBase.php +++ b/src/pocketmine/plugin/PluginBase.php @@ -41,9 +41,6 @@ abstract class PluginBase implements Plugin{ /** @var bool */ private $isEnabled = false; - /** @var bool */ - private $initialized = false; - /** @var PluginDescription */ private $description; @@ -62,6 +59,17 @@ abstract class PluginBase implements Plugin{ /** @var TaskScheduler */ private $scheduler; + public function __construct(PluginLoader $loader, Server $server, PluginDescription $description, string $dataFolder, string $file){ + $this->loader = $loader; + $this->server = $server; + $this->description = $description; + $this->dataFolder = rtrim($dataFolder, "\\/") . "/"; + $this->file = rtrim($file, "\\/") . "/"; + $this->configFile = $this->dataFolder . "config.yml"; + $this->logger = new PluginLogger($this); + $this->scheduler = new TaskScheduler($this->logger); + } + /** * Called when the plugin is loaded, before calling onEnable() */ @@ -85,11 +93,11 @@ abstract class PluginBase implements Plugin{ } /** - * @param bool $boolean + * @param bool $enabled */ - final public function setEnabled(bool $boolean = true){ - if($this->isEnabled !== $boolean){ - $this->isEnabled = $boolean; + final public function setEnabled(bool $enabled = true) : void{ + if($this->isEnabled !== $enabled){ + $this->isEnabled = $enabled; if($this->isEnabled){ $this->onEnable(); }else{ @@ -113,20 +121,6 @@ abstract class PluginBase implements Plugin{ return $this->description; } - final public function init(PluginLoader $loader, Server $server, PluginDescription $description, string $dataFolder, string $file){ - if(!$this->initialized){ - $this->initialized = true; - $this->loader = $loader; - $this->server = $server; - $this->description = $description; - $this->dataFolder = rtrim($dataFolder, "\\/") . "/"; - $this->file = rtrim($file, "\\/") . "/"; - $this->configFile = $this->dataFolder . "config.yml"; - $this->logger = new PluginLogger($this); - $this->scheduler = new TaskScheduler($this->logger); - } - } - /** * @return PluginLogger */ @@ -134,13 +128,6 @@ abstract class PluginBase implements Plugin{ return $this->logger; } - /** - * @return bool - */ - final public function isInitialized() : bool{ - return $this->initialized; - } - /** * @param string $name * diff --git a/src/pocketmine/plugin/PluginLoader.php b/src/pocketmine/plugin/PluginLoader.php index f5c71a556..51bcfae22 100644 --- a/src/pocketmine/plugin/PluginLoader.php +++ b/src/pocketmine/plugin/PluginLoader.php @@ -28,31 +28,6 @@ namespace pocketmine\plugin; */ interface PluginLoader{ - /** - * Loads the plugin contained in $file - * - * @param string $file - * - * @return Plugin|null - */ - public function loadPlugin(string $file); - - /** - * Gets the PluginDescription from the file - * - * @param string $file - * - * @return null|PluginDescription - */ - public function getPluginDescription(string $file); - - /** - * Returns the filename regex patterns that this loader accepts - * - * @return string - */ - public function getPluginFilters() : string; - /** * Returns whether this PluginLoader can load the plugin in the given path. * @@ -63,18 +38,18 @@ interface PluginLoader{ public function canLoadPlugin(string $path) : bool; /** - * @param Plugin $plugin + * Loads the plugin contained in $file * - * @return void + * @param string $file */ - public function enablePlugin(Plugin $plugin); + public function loadPlugin(string $file) : void; /** - * @param Plugin $plugin + * Gets the PluginDescription from the file * - * @return void + * @param string $file + * + * @return null|PluginDescription */ - public function disablePlugin(Plugin $plugin); - - + public function getPluginDescription(string $file) : ?PluginDescription; } diff --git a/src/pocketmine/plugin/PluginManager.php b/src/pocketmine/plugin/PluginManager.php index 9d749652a..3d92c1403 100644 --- a/src/pocketmine/plugin/PluginManager.php +++ b/src/pocketmine/plugin/PluginManager.php @@ -29,6 +29,8 @@ use pocketmine\event\Event; use pocketmine\event\EventPriority; use pocketmine\event\HandlerList; use pocketmine\event\Listener; +use pocketmine\event\plugin\PluginDisableEvent; +use pocketmine\event\plugin\PluginEnableEvent; use pocketmine\network\mcpe\protocol\ProtocolInfo; use pocketmine\permission\Permissible; use pocketmine\permission\Permission; @@ -118,20 +120,10 @@ class PluginManager{ } /** - * @param string $loaderName A PluginLoader class name - * - * @return bool + * @param PluginLoader $loader */ - public function registerInterface(string $loaderName) : bool{ - if(is_subclass_of($loaderName, PluginLoader::class)){ - $loader = new $loaderName($this->server); - }else{ - return false; - } - - $this->fileAssociations[$loaderName] = $loader; - - return true; + public function registerInterface(PluginLoader $loader) : void{ + $this->fileAssociations[get_class($loader)] = $loader; } /** @@ -147,11 +139,12 @@ class PluginManager{ * * @return Plugin|null */ - public function loadPlugin(string $path, array $loaders = null){ + public function loadPlugin(string $path, array $loaders = null) : ?Plugin{ foreach($loaders ?? $this->fileAssociations as $loader){ - if(preg_match($loader->getPluginFilters(), basename($path)) > 0){ + if($loader->canLoadPlugin($path)){ $description = $loader->getPluginDescription($path); if($description instanceof PluginDescription){ + $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.load", [$description->getFullName()])); try{ $description->checkRequiredExtensions(); }catch(PluginException $ex){ @@ -159,18 +152,40 @@ class PluginManager{ return null; } + $dataFolder = dirname($path) . DIRECTORY_SEPARATOR . $description->getName(); + if(file_exists($dataFolder) and !is_dir($dataFolder)){ + $this->server->getLogger()->error("Projected dataFolder '" . $dataFolder . "' for " . $description->getName() . " exists and is not a directory"); + return null; + } + + $loader->loadPlugin($path); + + $mainClass = $description->getMain(); + if(!class_exists($mainClass, true)){ + $this->server->getLogger()->error("Main class for plugin " . $description->getName() . " not found"); + return null; + } + if(!is_a($mainClass, Plugin::class, true)){ + $this->server->getLogger()->error("Main class for plugin " . $description->getName() . " is not an instance of " . Plugin::class); + return null; + } + try{ - if(($plugin = $loader->loadPlugin($path)) instanceof Plugin){ - $this->plugins[$plugin->getDescription()->getName()] = $plugin; + /** + * @var Plugin $plugin + * @see Plugin::__construct() + */ + $plugin = new $mainClass($loader, $this->server, $description, $dataFolder, $path); + $plugin->onLoad(); + $this->plugins[$plugin->getDescription()->getName()] = $plugin; - $pluginCommands = $this->parseYamlCommands($plugin); + $pluginCommands = $this->parseYamlCommands($plugin); - if(count($pluginCommands) > 0){ - $this->commandMap->registerAll($plugin->getDescription()->getName(), $pluginCommands); - } - - return $plugin; + if(count($pluginCommands) > 0){ + $this->commandMap->registerAll($plugin->getDescription()->getName(), $pluginCommands); } + + return $plugin; }catch(\Throwable $e){ $this->server->getLogger()->logException($e); return null; @@ -206,7 +221,7 @@ class PluginManager{ $loaders = $this->fileAssociations; } foreach($loaders as $loader){ - foreach(new \RegexIterator(new \DirectoryIterator($directory), $loader->getPluginFilters()) as $file){ + foreach(new \DirectoryIterator($directory) as $file){ if($file === "." or $file === ".."){ continue; } @@ -567,11 +582,15 @@ class PluginManager{ public function enablePlugin(Plugin $plugin){ if(!$plugin->isEnabled()){ try{ + $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.enable", [$plugin->getDescription()->getFullName()])); + foreach($plugin->getDescription()->getPermissions() as $perm){ $this->addPermission($perm); } $plugin->getScheduler()->setEnabled(true); - $plugin->getPluginLoader()->enablePlugin($plugin); + $plugin->setEnabled(true); + + $this->server->getPluginManager()->callEvent(new PluginEnableEvent($plugin)); }catch(\Throwable $e){ $this->server->getLogger()->logException($e); $this->disablePlugin($plugin); @@ -647,12 +666,14 @@ class PluginManager{ */ public function disablePlugin(Plugin $plugin){ if($plugin->isEnabled()){ + $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.disable", [$plugin->getDescription()->getFullName()])); + $this->callEvent(new PluginDisableEvent($plugin)); + try{ - $plugin->getPluginLoader()->disablePlugin($plugin); + $plugin->setEnabled(false); }catch(\Throwable $e){ $this->server->getLogger()->logException($e); } - $plugin->getScheduler()->shutdown(); HandlerList::unregisterAll($plugin); foreach($plugin->getDescription()->getPermissions() as $perm){ diff --git a/src/pocketmine/plugin/ScriptPluginLoader.php b/src/pocketmine/plugin/ScriptPluginLoader.php index 368620757..4934ac46c 100644 --- a/src/pocketmine/plugin/ScriptPluginLoader.php +++ b/src/pocketmine/plugin/ScriptPluginLoader.php @@ -33,46 +33,18 @@ use pocketmine\Server; */ class ScriptPluginLoader implements PluginLoader{ - /** @var Server */ - private $server; - - /** - * @param Server $server - */ - public function __construct(Server $server){ - $this->server = $server; + public function canLoadPlugin(string $path) : bool{ + $ext = ".php"; + return is_file($path) and substr($path, -strlen($ext)) === $ext; } /** * Loads the plugin contained in $file * * @param string $file - * - * @return Plugin|null */ - public function loadPlugin(string $file){ - if(($description = $this->getPluginDescription($file)) instanceof PluginDescription){ - $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.load", [$description->getFullName()])); - $dataFolder = dirname($file) . DIRECTORY_SEPARATOR . $description->getName(); - if(file_exists($dataFolder) and !is_dir($dataFolder)){ - throw new \InvalidStateException("Projected dataFolder '" . $dataFolder . "' for " . $description->getName() . " exists and is not a directory"); - } - - include_once($file); - - $className = $description->getMain(); - - if(class_exists($className, true)){ - $plugin = new $className(); - $this->initPlugin($plugin, $description, $dataFolder, $file); - - return $plugin; - }else{ - throw new PluginException("Couldn't load plugin " . $description->getName() . ": main class not found"); - } - } - - return null; + public function loadPlugin(string $file) : void{ + include_once $file; } /** @@ -82,7 +54,7 @@ class ScriptPluginLoader implements PluginLoader{ * * @return null|PluginDescription */ - public function getPluginDescription(string $file){ + public function getPluginDescription(string $file) : ?PluginDescription{ $content = file($file, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); $data = []; @@ -114,55 +86,4 @@ class ScriptPluginLoader implements PluginLoader{ return null; } - - /** - * Returns the filename patterns that this loader accepts - * - * @return string - */ - public function getPluginFilters() : string{ - return "/\\.php$/i"; - } - - public function canLoadPlugin(string $path) : bool{ - $ext = ".php"; - return is_file($path) and substr($path, -strlen($ext)) === $ext; - } - - /** - * @param PluginBase $plugin - * @param PluginDescription $description - * @param string $dataFolder - * @param string $file - */ - private function initPlugin(PluginBase $plugin, PluginDescription $description, string $dataFolder, string $file){ - $plugin->init($this, $this->server, $description, $dataFolder, $file); - $plugin->onLoad(); - } - - /** - * @param Plugin $plugin - */ - public function enablePlugin(Plugin $plugin){ - if($plugin instanceof PluginBase and !$plugin->isEnabled()){ - $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.enable", [$plugin->getDescription()->getFullName()])); - - $plugin->setEnabled(true); - - $this->server->getPluginManager()->callEvent(new PluginEnableEvent($plugin)); - } - } - - /** - * @param Plugin $plugin - */ - public function disablePlugin(Plugin $plugin){ - if($plugin instanceof PluginBase and $plugin->isEnabled()){ - $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.plugin.disable", [$plugin->getDescription()->getFullName()])); - - $this->server->getPluginManager()->callEvent(new PluginDisableEvent($plugin)); - - $plugin->setEnabled(false); - } - } } diff --git a/tests/plugins/PocketMine-DevTools b/tests/plugins/PocketMine-DevTools index f4d7fc001..eb1af5638 160000 --- a/tests/plugins/PocketMine-DevTools +++ b/tests/plugins/PocketMine-DevTools @@ -1 +1 @@ -Subproject commit f4d7fc001e942e97048c5775cc2fd259637c8b66 +Subproject commit eb1af563827fc748bbad8286c6c5796b7501848f