Handle nonsense folders in /extensions/ about symphonycms

[nilshoerrmann]

This is what I'd expect:

Nonsense folders

A warning in the logs:

Warning: An unrecognized extension was found at /path/to/that/folder

Missing extensions

A nice message (not an exception):

Symphony could not find "Export Ensemble" extension. Please make sure all files and folders are available at /extensions/export_ensemble/ (double-check the folder name).

[brendo]

If only it was that easy. Symphony finds extension folder first. That is, it gets all the directories in EXTENSIONS and then looks for the extension.driver.php. If the extension driver isn't found, an error is thrown.

In your scenario, it's difficult to tell the difference (at least with the current Extension Manager), between an empty directory and an misnamed folder. @rowan-lewis and I spent several hours getting a solution out that involved using Iterators and Reflection classes before just throwing it away. The overhead it added wasn't worth the education.

It's a very difficult thing to accomplish with the current Extension Manager, it would need to be rewritten. I'm not totally against this, but there's definitely more pressing (and exciting) things to focus our efforts on.

In the meantime, #680 is probably the issue you were after.

[nilshoerrmann]

In a first step, wouldn't it be possible to make Symphony skip all folders inside /extensions/ that do not match the naming conventions and log the mentioned warning. So "officially" extensions with wrong folder names (those Github download names) or nonsense folders would not be available in the system.

This should be okay until Symphony requires an extension while executing the backend.

In a second step, couldn't Symphony just throw an error when it actually "misses" an extension? This way the system would not have to determine if an misnamed extension is installed or not when initialising Extension Manager (this is the problem as far as I understand).

[brendo]

The first step has actually already been commited. It doesn't log anything though since it uses the General::listDirStructure which excludes the illnamed folders from being returned to the Extension Manager in the first case.

The second step is currently what happens, Symphony doesn't call an extension until it's required.

[nilshoerrmann]

If the first step is already in the core, how could it be that a plain, empty test folder breaks the system?

[nilshoerrmann]

I was imprecise in describing my first step: I meant skipping all folders that don't match the naming conventions and do not contain extension.driver.php. Those folders classified as "something is wrong with this extension".

[brendo]

And that's where the overhead kicks in ;)

[nilshoerrmann]

I'm stunned – shouldn't this just require an additional check? First check if the folder name is valid (which is already done as you pointed out) and then check if extension.driver.php exists inside this folder before continuing.

Apart from that: what about nicer error messages at least? This shouldn't be much overhead, right?

[brendo]

The above commit should fix the bug outlined in the title. The check to alert a developer that an extension needed updating was iterating over the entire extensions folder. This wasn't needed, as if an extension isn't installed, there's nothing that can be 'updated' about it, so that part was a big oversight by me in 2.2.

In regards to the nicer error messages, it is actually a challenging problem to solve elegantly. The hardest part is getting the extension name, Export Ensemble as it's only available through the about() function, which means the class has to be initialised first. Some of the solutions Rowan and I tried include:

• Parsing the file with regular expressions
• Trying to 'dehandlise' the folder name
• Including the extension driver and comparing the output of get_included_classes, using the 'odd' class out and creating an instance of it with Reflection, which then enabled us to call about()

All added overhead and code to the point where we just said, it's not worth it.

The most elegant solution in my opinion is the one I proposed on the forum, having an extension.meta.xml file that has the same information as the about function. In that case there is no class discovery needed, that file would always exist, and if an error occurred loading the extension, it could be relied on to get various details to show the user (Name is the only one I can think of at the moment)

[creativedutchmen]

I disagree with your objections on the first point, brendo. All you will need is to catch the exception in the listAll function (and other functions listing the extensions), and log the message instead of letting it pass.

[brendo]

Sorry mate, but which point were you referring to?

[creativedutchmen]

Oh yes, sorry for being unclear. I was referring to this:

If only it was that easy. Symphony finds extension folder first. That is, it gets all the directories in EXTENSIONS and then looks for the extension.driver.php. If the extension driver isn't found, an error is thrown.

Because there is no need to distinguish between a proper extension with an invalid folder and an improper extension with a bugus folder, simply catching the exception and writing it to the log should suffice.

As you said later, the missing extension problem is handled somewhere else (hence the different Exception being thrown), so this can also be changed at that location.

I am getting a bit tired - been working all day again - so if I am still unclear, please let me know!