Skip to content

Defensive coding to fix some errors when using SQLite - #19389

Closed
aristath wants to merge 1 commit into
trunkfrom
fix/sqlite-erorrs-defensive-coding
Closed

aristath wants to merge 1 commit into
trunkfrom
fix/sqlite-erorrs-defensive-coding

Conversation

@aristath

Copy link
Copy Markdown
Contributor

Context

In a few days, the next release of the Performance Lab plugin will be released. This version will include an SQLite implementation which is a proposal for WP-Core (at some point in the distant future).

When testing this implementation with the Yoast SEO plugin, I was getting some PHP warnings in my log:

PHP Warning:  array_map(): Expected parameter 2 to be an array, null given in wordpress-seo/src/actions/indexing/post-link-indexing-action.php on line 66
PHP Warning:  Invalid argument supplied for foreach() in wordpress-seo/src/actions/indexing/abstract-link-indexing-action.php on line 62
PHP Warning:  Invalid argument supplied for foreach() in wordpress-seo/lib/orm.php on line 1970

Summary

This PR adds some defensive coding to avoid errors in case a query returns null or is empty.

This PR can be summarized in the following changelog entry:

  • Proactively add checks to avoid warnings when a database query returns invalid results.

Relevant technical choices:

  • Added a couple of simple PHP checks, to ensure that variables are actually arrays before running functions like array_map or foreach loops.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • Install and activate the Performance Lab plugin. If v1.8.0 is not officially released yet, then this can be tested using the GitHub version from https://github.com/WordPress/performance
  • Go to the plugin's settings and activate the SQLite module
  • Once SQLite is active, the site is using an SQLite db. Activate the Yoast plugin, and ensure there are no errors/warnings triggered from the plugin in the PHP error log.

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Block/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

  • I have written documentation for this change.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unit tests to verify the code works as intended
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
  • I have written this PR in accordance with my team's definition of done.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label and noted the work hours.

Fixes #

@aristath

Copy link
Copy Markdown
Contributor Author

Closing this one as it is no longer relevant. The issues and errors have been fixed in the SQLite implementation itself (see WordPress/performance#600)

@aristath aristath closed this Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant