Prestashop: [SF Migration] Make it easy to add a Bundle to PrestaShop Symfony

[matks]

Type: tech-issue

There are dozen of wonderful Bundles for Symfony out there

• https://symfony.com/doc/current/bundles.html
• https://symfony.com/blog/the-30-most-useful-symfony-bundles-and-making-them-even-better
• https://packagist.org/packages/symfony/

However if a PrestaShop developer wants to use one, he must modify:

• a configuration YAML file
• add the Bundle in AppKernel
  Which will result in Core PrestaShop files modified

We should provide a system to help PS developers install + enable Bundles without having to modify Core files.
Maybe a gitignored config file that would be included in PS ? And a "hook" in AppKernel to add more Bundles ?

[matks]

@PrestaShop/prestashop-core-developers I have the following idea:

Modules can already modify the Symfony configuration through a services.yml file.
What is missing is a way to register more Bundles without modifying AppKernel.php file.

I dont want to use a hook because the hook system is heavy and registerBundles() must be quick. I'm thinking about a class ThirdPartyBundleEnabler (it would implement SymfonyBundleEnablerInterface) that would be able to go through all active modules (like the processing being performed in AppKernel::enableComposerAutoloaderOnModules() and load the relevant bundles.

How would module specify "I need bundles X, Y, Z" ? YAML file ? I dont like because parsing is heavy although we could put it in cache. XML and JSON are slightly better but not very human readable.

interface SymfonyBundleEnablerInterface
{
    /**
     * @return BundleInterface[]
     */
    public function getBundlesToEnable();
}

[PierreRambaud]

Good idea 👍

[matks]

An idea to obtain both flexibility and performance:

Have a bundles.php file next to AppKernel. This file is edited each time a module with bundles is installed. The module adds the required bundles in it, and removes them when uninstalls.

Module with bundle = module with config/bundles.yml file

Wdyt @Quetzacoalt91 ? I think you've seen the dangers and complex usecases of modules installing/uninstalling things 😄

[matks]

OK, now I think I made up my mind. I will go for the following:

On Module side

Provide new functions for Modules, to be called in the install() and uninstall() steps:

     public function registerBundle($fullyQualifiedClassName): bool
     public function unregisterBundle($fullyQualifiedClassName): bool

Example of usage, from a Module:

    public function install()
    {
        return parent::install() &&
        $this->registerBundle('OldSound\RabbitMqBundle\OldSoundRabbitMqBundle');
    }

These functions will manipulate 2 gitignored files in app/ folder:

• third_party_bundles.yml
• third_party_bundles.php

Registering a bundle will add a new line into third_party_bundles.yml. Each line is a fully qualified class name of a bundle, that has been validated to be a valid php class that is loadable and implements Symfony\Component\HttpKernel\Bundle\BundleInterface.
Unregistering the bundle will remove this line from third_party_bundles.yml.

Example of the content of one third_party_bundles.yml:

bundles:
    - 'OldSound\RabbitMqBundle\OldSoundRabbitMqBundle'
    - 'Snc\RedisBundle\SncRedisBundle'

After the YAML file modification, functions unregisterBundle() or registerBundle() will trigger the computation or re-computation of the file third_party_bundles.php.

third_party_bundles.php is a php file that returns an array of the third_party_bundles.yml content. Example of a third_party_bundles.php, generated from the previous YAML file:

<?php

return [
    'OldSound\RabbitMqBundle\OldSoundRabbitMqBundle',
    'Snc\RedisBundle\SncRedisBundle',
];

So to sum it up:

• modules are able to add or remove Bundle class names from third_party_bundles.yml, so this YAML file is the list of all 3rd-party enabled bundles enabled on the shop
• third_party_bundles.php is a generated duplicate of third_party_bundles.yml, but it's a callable php script that return its content as an array

Why 2 files ? because:

• I dont want to parse a YAML file on each call to AppKernel::registerBundles(), I prefer to require a php file
• I dont want Modules to add and remove lines from a php file, it's easy to introduce a mistake that is syntaxically wrong, I prefer them to modify a YAML file
• If someone modifies manually third_party_bundles.php and introduces a mistake, he can re-compute it from the YAML list to solve his issue*

• Maybe, if PrestaShop detects that third_party_bundles.php is not a valid php file (because something messed it up) it could re-compute it automatically ? See at the end **

On PrestaShop side

I'll add one ThirdPartyBundleFileComputer to handle the php file computation from the YAML file.

Then I'll add one SymfonyBundleEnablerInterface

interface SymfonyBundleEnablerInterface
{
    /**
     * @return BundleInterface[]
     */
    public function getBundlesToEnable();
}

AppKernel::registerBundle() will call the available enablers.

I'll also add one ThirdPartyBundleEnabler which implements SymfonyBundleEnablerInterface. This class will simply require the third_party_bundles.php file and return the list of bundles in it after having (again) validated they are valid php classes that implement BundleInterface.

**Example of a usecase:

• module MyBeautifulModule enables bundle MyGreatBundle
• merchant removes MyBeautifulModule by deleting the module folder
• so YAML and php files still contain the reference to MyGreatBundle although MyGreatBundle code is gone
  What should we do here ? When we see that MyGreatBundle is not loadable, we ignore it ? Remove it from the YAML file ?

[matks]

@mickaelandrieu @zalexki I'd be happy to hear your feedback about this topic and I think you are interested in it 😄

[Bonobomagno]

Making a module mutate core file, are you sure?
Why don't move the bundle file compilation on cache generation/refresh?
(just proc cache refresh on module install, and i think that prestashop actually just do this)

If i didn't missread, module config.yml files are reading in this way. Who care if is slow, if you are doing this only one in a time? This would make easy to remove the bundle on uninstall.

In this way we can follow symfony flex in some way, maybe using packages.yml and packages directory, and why not...prestashop compatible recipes?

[matks]

@Bonobomagno The idea is good but I'm a bit worried 🤔 because then we "hack" the container cache role.

The cache, if I'm not wrong, is supposed to be a compiled version of the configuration in order to avoid to re-compile it everytime. It's just an optimization capability added on top of the bundle/services resolution system provided by Symfony.

Following your idea, it means transforming the cache into a compilable result that we actually need to update to perform the "searching in modules for bundle files". I feel like we are hijacking a standard Symfony workflow to suit our needs, and this might backfire.

However the cool things from your idea removing the module folder (followed by a clear cache) would immediately unregister the bundle, no need to modify a file. Adding a new bundle is very easy too.

My worries might be over-careful here though 😅 maybe I'm paranoid so I'd be happy to hear more feedback about your ideas. ping @PrestaShop/prestashop-core-developers

[Bonobomagno]

My idea was more about: DRY. You can just do the same thing used for module config files, but now you are checking a static function on the module class, or (quicker) the return of a bundleXX.php file inside the module dir.

But there is a really big issue: conflicts.
How to do for let two modules use the same bundle in 2 different version? Or, more common, how to handle 2 modules use the same bundle? And the updates of the bundle?
Actually, will be better to let prestashop himself handle that.
Maybe handling bundles as a special modules? With something like registerAsBundle() ?
Zero hooks and a different dir for no conflicts.
For module that want bundles, a required bundle files could make it easy.

The downside is that now you have to do official modules for the bundles.
Worth it? Idk.

[matks]

How to do for let two modules use the same bundle in 2 different version?

that's a very good question 🤔 but we have the same issue for composer dependencies
and right now the (bad) answer is "the first to get loaded wins"

Or, more common, how to handle 2 modules use the same bundle?

this is easy: in the registerBundles(), remove duplicates

[Bonobomagno]

Ofc, i'm suggesting a different way to handle bundle autoloading. Or actually you will have a situation where module A load package P 1.2(but it don't use it as bundle on appKernel) and module B, that use package P 2.0, will add the bundle but it will not work.
Maybe everything will not work. Who knows if 1.x is bundle ready?

But maybe you can just load first module that use bundles, this will help (?)

[matks]

Mmmm ... I'm afraid anyway we're screwed 😅

If 2 modules load the same bundle, same version and use it configured the same way, it will work.
But it's more likely that they will load 2 different bundle config (for example API endpoint) 🤔

So for example if module A uses GuzzleBundle with SSL enabled and module B uses GuzzleBundle with SSL disabled, one of them is going to be disappointed 😭

That's pretty harsh because some Bundle (like DoctrineBundle) are likely to be used with a lot of modules that might end up being installed on the same shop.

Argh 😭it seems a pretty challenging limitation

[Bonobomagno]

This is why i suggested using a special module/ unique bundle( like symfony flex recipe) with own configuration/ autoloader installed on a global space.

The module installation can proc this bundle installation.
About config: i don't think is a true problem. Configuration can be "namespaced".
for GuzzleBundle you can have multiple configuration and every module will call the own service (that define own configuration).
For api , ad eg, every module can have own routes etc.

_An other idea_, why not just use some default modules on addons?
Give priority to this default modules on autoload and use a combination of yaml/ prestashop module Configuration service for piloting it.
In this way we need only that modules define what bundle requirement have on own config.xml file.
manual installation just work: upload 2 modules (one as bundle, one as the true module).

Also, in special/custom use case, every one can make own bundle, or just use a custom version if want to lock bundle module update.

For doing that, suggest to add a is_bundle flag on bundle module config.xml.

[jbenezech]

Just droping that here for reference as it basically deals with the same kind of dependency issue in Symfony. https://github.com/mmoreram/symfony-bundle-dependencies

Of course doesn't solve the multi-configurations issue...

[matks]

@jbenezech very nice reference 😊