Default widgets with alternative option name causing unnecessary database queries

[Chouby]

This ticket is a follow up to #26876, as the same bug still occurs for the widget recent comments and the widget recent posts because both have an alternative option name.

The bug was not detectable at that time because both widgets were used by default, and thus options were always filled. However, this is no more the case since the version 5.8. Now if no option have been saved, then we can see that 4 useless database queries are triggered:

SELECT option_value
FROM wp_options
WHERE option_name = 'widget_recent-posts'
LIMIT 1	

SELECT option_value
FROM wp_options
WHERE option_name = 'widget_recent_entries'
LIMIT 1	

SELECT option_value
FROM wp_options
WHERE option_name = 'widget_recent-comments'
LIMIT 1	

SELECT option_value
FROM wp_options
WHERE option_name = 'widget_recent_comments'
LIMIT 1

The bug can easily reproduced on a fresh WP 5.8 install with Query Monitor to see the useless queries triggered by WP_Widget->get_settings().

[mvraghavan]

Created patch

[costdev]

Per the discussion in the bug scrub, I'm adding the needs-testing keyword in hopes that this might be tested ahead of RC1 on Tuesday.

[costdev]

With 6.0 RC1 starting soon, I'm moving this to the 6.1 milestone.

[peterwilsoncc]

It looks like 54677.patch​ prioritizes $this->alt_option_name over the current option name $this->option_name. My reading of the code is that if both options exist, then option_name is likely to hold the correct data.

I am still testing but I think WP_Widget::save_settings() should remain unchanged while WP_Widget::get_settings() should start with something along the lines of the following code.

<?php
$settings = get_option( $this->option_name );

if ( false === $settings ) {
        $settings = array();
        if ( isset( $this->alt_option_name ) ) {
                // Get from legacy (alternative) option name.
                $settings = get_option( $this->alt_option_name, array() );

                // Delete the legacy option as the new option will be created.
                delete_option( $this->alt_option_name );
        }
        // Save an option so it can be autoloaded next time.
        $this->save_settings( $settings );
}

https://core.trac.wordpress.org/ticket/54677

[peterwilsoncc]

In the ​linked pull request:

• no change to WP_Widget::save_settings() it always uses $this->option_name.
• WP_Widget::get_settings() uses $this->option_name if it it exists
• WP_Widget::get_settings() falls back $this->alt_option_name if it it exists
• WP_Widget::get_settings() initializes $this->option_name if neither exist
• WP_Widget::get_settings() migrates $this->alt_option_name to $this->option_name only the alt option is set
• If both options exist then no changes are made to the database -- doing so would create extra db queries if the alt option was not set.

Testing notes:

• no options creates options_name
• alt option exists and is migrated to option_name
• new option exists does not create alt_option_name
• if both options exist then both remain in the database

[spacedmonkey]

Someone from my team also looked into this issue. Here is their PR ​#12

[peterwilsoncc]

At @spacedmonkey's suggestion I investigated using an upgrade routine ​https://github.com/WordPress/wordpress-develop/pull/2951 the linked pull request].

On a single site install the upgrade routine is pointless, as wp_widgets_init() is run on the init hook and fires prior to the redirect to the upgrade screen.

An upgrade routine may be beneficial for Multisite installs (I'm not sure either way) but, given the options will be upgraded regardless, I am inclined not to add one.

The presence of an upgrade routine causes a complete cache flush, so any benefit to MS installs needs to be weighed against the impact for single site installs.

[mukesh27]

@peterwilsoncc I left some small feedback on PR.

Thanks @peterwilsoncc for the update. PR look good to me.

@costdev Can you please review it when you get the time so it can be merged?

I've applied many of @costdev's suggestions in 8ae2130a493e5ae210a1eabe7901df630740086b

• test uses initializes
• consistent reference to "alternative (legacy) option" on first appearance in a comment, from then on "alternative option"

[peterwilsoncc]

In 54112:

Widgets: Store default options for uninitialized widgets.

Prevent unnecessary database queries on page load by initializing widget options. On sites with uninitialized widgets, this prevents one or two database queries per uninitialized widget on each page load.

Props Chouby, mvraghavan, costdev, peterwilsoncc, spacedmonkey, mukesh27.
Fixes #54677.

https://core.trac.wordpress.org/changeset/54112 / f042981494