search

octobercms / october

Self-hosted CMS platform based on the Laravel PHP Framework.
https://octobercms.com/
Other
11.03k stars 2.21k forks source link

Kernel does not register vendor/autoload.php file in plugin folder. #5110

Closed kharanenka closed 4 years ago

kharanenka commented 4 years ago

After change in modules\system\classes\PluginManager.php (line 229), file /vendor/autoload.php from plugin is not registered. Therefore, if you install Lovata.Shopaholic plugin from backend, an "Class not found" or "Trait not found" error occurs, because installed packages from Lovata.Toolbox plugin are not included.

The error does not reproduce if you install Lovata.Shopaholic plugin through php artisan plugin:install Lovata.Shopaholic command.

The error does not reproduce if public $elevated = true flag is set in Plugin.php file of Lovata.Toolbox plugin.

Plugins works successfully in 465 kernel version. We, as plugin developers, receive reports from our customers, because they cannot install plugins. We must be sure that there are no problems in our plugin.

Steps To Reproduce:

  1. Install Lovata.Toolbox plugin from backend.
  2. install Lovata.Shopaholic plugin from backend.
LukeTowers commented 4 years ago

Tested this and it seems to only be an issue on the installation, after they're installed they work just fine. Might be some sort of race condition where the required plugin's autoload file is not loaded before the requiring plugin runs its migrations? Seemed to only happen in the completeInstall step, in $this->updatePlugin() of the update() method of the UpdateManager class.

@bennothommo could you take a look?

bennothommo commented 4 years ago

@LukeTowers when you tested this, what was installed first? Shopaholic or Toolbox?

LukeTowers commented 4 years ago

@bennothommo Shopaholic and then Toolbox. Note that some of the migrations worked but some of the ones related to the settings section seem to have failed because navigating to the settings pages for the plugin give me table not found errors.

bennothommo commented 4 years ago

@LukeTowers ah I see, so it might indeed be a race condition. Toolbox should be installed first because it is the dependency. That being said, plugins are not supposed to be loaded in any update functionality, which is why there's a different outcome when the plugin is elevated.

@kharanenka does Shopaholic rely on Toolbox as part of the migrations?

kharanenka commented 4 years ago

Hi @bennothommo ! Yes, some plugin migrations of Shopaholic plugin add tables and transfer data. Data transfer is associated with calling of model class objects. The model class contains traits from Toolbox plugin. Traits are installed as composer packages in Toolbox plugin.

bennothommo commented 4 years ago

@kharanenka Thanks for the info. So in that case, it's definitely a race condition in that Shopaholic is installed first, and its migrations run, before Toolbox. We'll get a fix ready for it in build 467 which should hopefully be out within a week or two. In the meantime, you may need to instruct your users to install Toolbox first, then Shopaholic.

kharanenka commented 4 years ago

Thanks for quick fix! Have a nice day

LukeTowers commented 4 years ago

@kharanenka can you test out #5120?