#3326 - Robustness of addons
-
By Chris Graham
- Added
- 4 views
| Identifier | #3326 |
|---|---|
| Issue type | Feature request or suggestion |
| Title | Robustness of addons |
| Status | Completed |
| Handling member | Chris Graham |
| Addon | core |
| Description | Very regularly we find users not doing things properly and restoring files from addons they've removed, or matching the wrong set of files and database state together. We've tried to construct good documentation to educate users to do this, but realistically nowadays nobody is going to read documentation (and they may not speak good English anyway) so we need to do better.
Improve addon_installed calls to also do an implicit table_exist on all the addons tables (use db_meta.dat), and also module_installed. Change direct current table_exists calls to addon_installed calls. Change direct current module_installed calls to addon_installed calls. Optimise table_exists to load in all tables at once and cache all their statuses. Every hook must have an addon_installed check, with a no-op result (e.g. null) if it fails. Every block must have an addon_installed check, with a standardised error box linking to admin_addons if it fails. Every module must have an addon_installed check, with a standardised link to admin_addons if it fails. Blocks and addons should no longer auto-upgrade or auto-install prior to being launched. Instead they should provided a standardised link to admin_addons. This is to stop race conditions and automatic reinstall by users if the file is only there as a rogue file. Automated tests should be added to ensure all the above coding standards are met. Document the coding standards properly. admin_addons should be able to detect corrupt addons and allow either repairing them (extract missing files and install modules and blocks), reinstalling, or deleting them. The integrity checker should get a lot smarter. It should be able to detect addons that are not properly installed and provide groups of files to delete under a single checkbox. It should also integrate the above improved admin_addons behaviour. A lot of work needs doing in the integrity checker to make a better API, and it should be unit tested. The MySQL database checker cleanup tool should also be available from within the upgrader. Maybe it should actually only be available there. Same goes with MySQL fix corrupt tables. These should run from a minimal environment, not a full Admin Zone. Link out to the individual upgrader tools from the Cleanup Tools so it's all nicely tied together. As a general test make a quick Composr site with no optional addons installed, then put ALL the files back and make sure nothing crashes. Then make sure the upgrader can cleanup the situation either by repairing addons or by deleting files. |
| Steps to reproduce | |
| Funded? | No |
The system will post a comment when this issue is modified (e.g., status changes). To be notified of this, click "Enable comment notifications".
Comments
So before checking tables etc, addon_installed would also check simple get_value calls to see if there's a flag to say the addon is disabled.
"admin_addons should be able to detect corrupt addons and allow either repairing them (extract missing files and install modules and blocks), reinstalling, or deleting them."
Instead I've made the upgrader a bit nicer, and error messages about addon/block/module issues will now point to both the upgrader and admin_addons, with explanations of what circumstances to use each. I think this is a lot cleaner, as the upgrader already had the role of Composr's repair tool.
"The integrity checker should get a lot smarter. It should be able to detect addons that are not properly installed and provide groups of files to delete under a single checkbox."
I didn't see a good reason to do this. But I have tuned the messaging, it's pretty clear how to deal with a partly-installed addon now.
"A lot of work needs doing in the integrity checker to make a better API, and it should be unit tested."
The upgrader code has been improved a lot. It makes use of a lot more shared code with the rest of Composr. I haven't specifically unit tested though as that would be challenging and quite messy.
"As a general test make a quick Composr site with no optional addons installed, then put ALL the files back and make sure nothing crashes. Then make sure the upgrader can cleanup the situation either by repairing addons or by deleting files."
I haven't done this extensive testing, but the automated testing for checking addon checks is pretty exhaustive, and I've done some testing (intentionally breaking the meta DB, and removing a registry hook - confirming menus update). If there are issues it will come up in automated error emails.
"Additionally to this I want an ability to flag addons disabled without removing files, intentionally. Each addon would get to declare whether this is supported - I don't want coders hacking stuff up, or 3rd party addon makers, to have to make addons this robust - but I want all the bundled addons to support it."
I haven't done the white-listing, as I think that's unnecessary. I've simply documented in the Code Book that not all addons will support it.
The ability to disable addons without removing files is a great addition to Composr, something I've wanted for a long time. It will potentially allow Demonstratr sites to run with different addon sets, although there's no UI for defining what addons are disabled right now.
---
I also made it so addons check their dependencies at run-time.