itmaybejj / editoria11y-wp

WordPress wrapper for Editoria11y
GNU General Public License v2.0
5 stars 1 forks source link

Issue with Multisite and MySQL 8 #32

Open stphnwlkr opened 2 weeks ago

stphnwlkr commented 2 weeks ago

We are using the plugin in WP VIP multisite and it is filling our logs with errors related to database creation calls. The VIP support team had the following observations:

Problem

Inside editoria11y-accessibility-checker/editoria11y.php from line 181 to line 246 there is logic to alter the database and add these CONSTRAINT

ADD CONSTRAINT ed11y_results_pid ADD CONSTRAINT ed11y_dismissal_pid

The issue with this is that MySQL 8 requires the CONSTRAINT symbol to be unique for the entire database documentation reference. This code is attempting to add this constraint when it already exists for your wp2 site:

SELECT TABLE_NAME, CONSTRAINT_NAME FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS WHERE CONSTRAINT_TYPE = 'FOREIGN KEY' ORDER BY CONSTRAINT_NAME; +------------------------+-------------------------------+ | TABLE_NAME | CONSTRAINT_NAME | +------------------------+-------------------------------+ | wp_2_ed11y_dismissals | ed11y_dismissal_pid | | wp_2_ed11y_results | ed11y_results_pid |

This basically means that any other sites with the plugin activated will attempt to create these ed11y_results_pid and ed11y_dismissal_pid constraints, but under MySQL8 will throw this duplicate key error. If we weren't on MySQL8 it should work but I think that logic between 181 and 246 could be improved to better support multisite. It's not so much a VIP-specific issue but a MySQL 8 issue, but I think it can be improved so that it only creates the constraint once.

Right now the plugin is configured to run these queries to add the constraint with each request and will attempt to drop and recreate these constraints even if it exists. This is a concern since it adds database activity with each request and could be a bigger concern with spikes in origin requests. It should only have to create the constraint once and ideally save the result in the object cache so that it does not need to check the existence as frequently on the database.

Recommendation

Recommend adding in the uniqueness for constraints similar to and also avoid dropping and recreating the constraint on each request.

// Generate unique constraint names $constraint_results_pid = $wpdb->prefix . 'ed11y_results_pid'; $constraint_dismissal_pid = $wpdb->prefix . 'ed11y_dismissal_pid';

itmaybejj commented 2 weeks ago

Argh; It should only run once and then set a site option that the tables are complete. I'll add that prefix, confirm it's only running once, and get a release out this week.

stphnwlkr commented 2 weeks ago

Thank you

itmaybejj commented 2 weeks ago

THANK YOU. I look forward to hearing if this fixes the issue.