Potential bug in extension management

[3Di]

I think that for the actual issue javiexin is talking about that the likely solution would have to be that is_enabled should return true only if is_available also returns true, which appears to be in the mentioned pull request.

I highly doubt any of this would prevent the error reported and caused by that user though. phpBB already tries to get around it, but if the code is used by that specific area of the ACP there's little that the system could do. Plus making the system work after a user deliberately deletes system files is not something that really needs to be handled nor can it be handled very well in all cases. It's just like if a Windows user deleted some files from the Windows/System32 folder causing the system and programs to malfunction. That's hardly the fault of the operating system and entirely the user's fault. In such a case the recommendation from most technicians would be to reinstall Windows or the affected software. In our case it would be to try and reinstall the extension or, in extreme cases, the entire forum.

The topic I above linked simply doesn't show anymore what I meant to say because the poster right after I solved its issue decided to edit and removing the screenshot from there, the one that's mentioned by me as "clear and visual". Back to this, sure thing it's what you said and sure thinghs are what Javier said as well.
But, if an extension has been erroneously deleted from the the ext folder as its whole by the Noobuser that doesn't mean IMO that we shouldn't do something to prevent such operation, I mean amend in this case.

Think about how many users do/did that, that's why I created such a mini how-to to let them get back to the ACP and have one more chance to fix their own boards.

Something has to be done and put in the dormant status those ext when something like that happens instead of purpose a merely board-wreck in the middle of the ocean.

You could simply reproduce that screenshot renaming the composer.json file of an extension right after having it hard-disabled via the DB, though.

[javiexin]

Ok, second try pushed to GitHub. Still partial implementation, but also with some changes:

• cache is now back in use, with a TTL set at 600 now (10 minutes) and we could make it whatever you think would be ok
• the "configured" but not "available" extensions now show up in the ACP Extension List as DISABLED/INVALID, same as before this change
• the "invalid" extensions are marked as "disabled" by the Extension Mgr (NOT the DB), so they show up in the is_disabled/all_disabled methods, as well as in the is_configured/all_configured methods

Still not complete implementation:

• cache should probably be force-reloaded on Extension Manager List page view
• when an extension is later re-installed in the file system, it will show up as ENABLED (when the cache is refreshed)
• no version validation of "configured" vs "available" extensions

Now, I hope that this is ok from the performance point of view, and we could adjust the TTL to whatever is found useful/usable.
And from the functional point of view, while it might still create some issues, specially with extensions that use the "disable_step" method, it is really much safer than what we have.

Remember than, contrary to what is said about core files, admins/users are encouraged to install new extensions, test them, remove them... And this, like any other manual task, is prone to errors, and we should do something to compensate. Not at any cost, not compromising a lot of performance, but a filesystem check ever five/ten minutes does not seem much for the saved headaches both for users and support team...

Is this the right path, or should we forget about this completely, and then DOCUMENT very clearly these limitations, and the SEMANTICS of different calls to the Extension Mgr?

Thanks,
-javiexin

[MattF]

forget about this completely

This will be a moot point once the Extension manager will be able to download extensions autmatically from our database.

[javiexin]

forget about this completely

This will be a moot point once the Extension manager will be able to download extensions autmatically from our database.

And this is a moot point since that is probably at least one, more likely two years out.

-javiexin

[javiexin]

Ok, I have now ready what would be an almost-complete solution for what I believe is doable without compromising performance, but maintaining the most possible availability and reliability.

I have not pushed it yet to github, as I would prefer to get your comments on the one that is published now before I upload this one.

But, let me highlight the changes, and functional differences:

• cache is in use, with a TTL set at 600 now (10 minutes) and we could make it whatever you think would be ok
• the "configured" but not "available" extensions show up in the ACP Extension List as DISABLED/INVALID, same as before this change
• NEW there is a new "category" of extensions: enabled + disabled = configured; enabled & !available = incomplete
• the "incomplete" extensions are marked as "disabled" by the Extension Mgr (NOT the DB) and stored in the database as DISABLED with a state != 0 (hence "INCOMPLETE"), so they show up in the is_disabled/all_disabled methods, as well as in the is_configured/all_configured methods, but never in is_enabled; the previously disabled extensions do not create any issue as is, so they just show up as disabled/invalid, with no changes in the DB or anything else (underline is new)
• NEW cache is now force-reloaded on Extension Manager List page view
• NEW when an extension is later re-installed in the file system, it shows up as DISABLED, and if it was ENABLED before (and only then), it shows a new action, "Reenable", that will forcefully disable and then enable the extension; this is only visible in the Extension List; if the admin does not get into the Extension List, the extension will remain disabled
• NEW for this last bit to work, I have had to create a new mode in the acp_extensions module (reenable), and create a new progress template file (acp_ext_reenable.html) with a few language entries; all these could be packed into a single template file (acp_ext_enable, acp_ext_disable, acp_ext_delete_data and acp_ext_reenable) with a single additional template switch/var set in acp_extensions; this has not been done, but would be easy to accomplish

Still not done, and probably I will not do it while phpbb itself does not check versions:

• no version validation of "configured" vs "available" extensions

This I would consider a good working solution, with not much runtime impact (only a check every 5 to 10 minutes, or on demand...) and yet a very important reliability improvement (at least in my view).

There are, and will always be, corner cases where the forum will break. But this will probably cover 90%+ of the failures due to extensions being accitentally (or purposely) removed from the file system. I personally think it is worth giving it a try.

Thanks,
-javiexin

PS: If this goes forward, there might be one more thing to do: CLI modifications for extension list and reenable.

[MattF]

forget about this completely

This will be a moot point once the Extension manager will be able to download extensions autmatically from our database.

And this is a moot point since that is probably at least one, more likely two years out.

-javiexin

No, it already exists. It just hasn't been merged in yet, as there are some final things to finish up with it.

[MattF]

This I would consider a good working solution, with not much runtime impact (only a check every 5 to 10 minutes, or on demand...) and yet a very important reliability improvement (at least in my view).

To me this is a bad idea. And for a very minor (almost non-) issue, IMO.

[javiexin]

Any arguments why this is a bad idea? Why reliability is a minor issue? And what about consistency? The performance impact is minimal if any: I have not counted them, but how many files are loaded in each and every page load of phpbb? I doubt that some additional file reads every 10 minutes will make any noticeable difference.

We might be techies, but the average admin does not know much about computers. And errors are human anyway. Of course we cannot prevent all and every error. But if something is likely going to happen, there should be some means to reduce the risks.

And by the way, even if extensions are downloaded directly from the CDB, there will ALWAYS be (a lot of) people using non-CDB extensions (Dev, or private, or custom made, or you name it). And don't tell that they are on their own because they are doing something "out of support". The extension system is precisely that, a SYSTEM to allow EXTENSIONs of any kind. Otherwise, there would not be any need for extensions: they would all be part of the core, as additional functionality, that may be enabled or disabled at will, but with NO possibility to add anything yourself.

Regards,
-javiexin

PS: By the way, having something that "already exists" does not mean it is going to be merged soon. I have had code to fix a core limitation, acknowledged as such, that has been ready for over 18 months, and have not been merged yet. I guess that team priorities are others, and resources are limited. That is why having an improvement that is ready be dismissed without even looking is so frustrating.

[javiexin]

I have pushed to github the latest version. If no interest exists, then I will close it.

The current solution is as follows (slightly modified quote):

• cache is in use, with a TTL set at 600 now (10 minutes) and we could make it whatever you think would be ok
• the "configured" but not "available" extensions show up in the ACP Extension List as DISABLED/INVALID, same as before this change
• NEW there is a new "category" of extensions: enabled + disabled = configured; enabled & !available = incomplete
• NEWthe "incomplete" extensions are marked as "disabled" by the Extension Mgr and stored in the database as DISABLED with a state != 0 (hence "INCOMPLETE"), so they show up in the is_disabled/all_disabled methods, as well as in the is_configured/all_configured methods, but never in is_enabled; the previously disabled extensions do not create any issue as is, so they just show up as disabled/invalid, with no changes in the DB or anything else
• NEW cache is now force-reloaded on Extension Manager List page view
• NEW when an extension is later re-installed in the file system, it shows up as DISABLED, and if it was ENABLED before (and only then), it shows a new action, "Reenable", that will forcefully disable and then enable the extension; this is only visible in the Extension List; if the admin does not get into the Extension List, the extension will remain disabled
• NEW for this last bit to work, I have had to create a new mode in the acp_extensions module (reenable), and create a new progress template file (acp_ext_reenable.html) with a few language entries; all these could be packed into a single template file (acp_ext_enable, acp_ext_disable, acp_ext_delete_data and acp_ext_reenable) with a single additional template switch/var set in acp_extensions; this has not been done, but would be easy to accomplish

Still not done:

• no version validation of "configured" vs "available" extensions, probably I would not do it while phpbb itself does not check versions
• CLI changes to cover the above functions

-javiexin