When switching themes, set previous theme mods as autoload = no

[igmoweb]

When a theme is switched, previous theme mods are yet autoloaded. This will start filling up the autoloaded options list if you change of Theme often. Normally this won't happen often but we had an issue like this in a big multisite where a user had many sites and he switched themes many times and PHP got out of memory when loading the user in wp-admin.

In any case, it's a good idea to:

• Set autoload = no for previous theme mods
• Set autoload = yes again for the new switched theme

[igmoweb]

Patch

[igmoweb]

Unit Tests

[westonruter]

This will now be greatly facilitated by #58964 which introduced the wp_set_option_autoload() function which will avoid having to delete and add the option as in the current patch.

Using wp_set_option_autoload() to set no to the old theme's autoload option.

Trac ticket: https://core.trac.wordpress.org/ticket/39537

[rajinsharwar]

Used the wp_set_option_autoload() function for setting the Autoload to No for the old theme.

Trac ticket: https://core.trac.wordpress.org/ticket/39537

This pull request (PR) sets autoload=no for the previous themes after switching between different themes, utilizing the ​wp_set_option_autoload function.

Within the switch_theme function, we retrieve two themes: the current theme and the previous theme. However, there isn't a direct method to obtain all previously activated themes. For instance, if a user has used 4-5 themes and all of these themes have theme_mods_* options autoloaded, this method involves querying to retrieve all the theme_mods_* and subsequently setting autoload=no for all themes except the current one.

However, there isn't a direct method to obtain all previously activated themes. For instance, if a user has used 4-5 themes and all of these themes have theme_mods_* options autoloaded, this method involves querying to retrieve all the theme_mods_* and subsequently setting autoload=no for all themes except the current one.

This (multiple previous theme mods with autoload=yes) is basically just a one time scenario, right? After that, only updating the old theme's option would be enough.

This (multiple previous theme mods with autoload=yes) is basically just a one time scenario, right? After that, only updating the old theme's option would be enough.

Yes. Could we add new function in src/wp-admin/includes/upgrade.php for 6.5 and set autoload=no for all stored theme options except current theme? and change the PR code to set autoload option for old theme only?

Yeah we could. This would have the benefit that it will apply this change immediately upon upgrade, and not only upon the next theme change. And the change in switch_theme would become very straightforward.

Thanks @swissspidy @joemcgill and @westonruter for the review. I have updates PR so it will only updates the old and new theme autoload option not for other olds, for that i will open followup issue and raise PR for it.

I have invested some time in examining the behavior of the cache that requires reloading or not. To address this issue, we can consider several options:

Option 1: Utilize the ​wp_set_option_autoload_values function to reset the autoload value. This function deletes all caches in ​this section of the code. Therefore, we should update the autoload option after deleting the cache within the same function.

Option 2: Remove the ​autoload option cache assertions from unit tests. Theoretically, these checks might not be necessary.

Option 3: Revert ​https://github.com/WordPress/wordpress-develop/pull/5706/commits/041eaad9ebae0dbeb5d5bd1af4f59b9aa19d83ac to ensure that the autoload option cache reloads after a theme switch.

In my opinion, the second option seems preferable. When the cache is deleted, it seems logical to run a query to retrieve that option and then reload the cache.

@felixarntz @joemcgill @swissspidy @westonruter, I would greatly appreciate your thoughts on this matter. Thank you!

Option 4: Add wp_load_alloptions() to the test case.

// Make sure that autoloaded options are cached properly.
+               wp_load_alloptions();
                $autoloaded_options = wp_cache_get( 'alloptions', 'options' );
                $this->assertArrayNotHasKey( "theme_mods_$new_theme_stylesheet", $autoloaded_options, "Option theme_mods_$new_theme_stylesheet not deleted from alloptions cache" );
                $this->assertArrayHasKey( "theme_mods_$current_theme_stylesheet", $autoloaded_options, "Option theme_mods_$current_theme_stylesheet unexpectedly deleted from alloptions cache" );

@mukeshpanchal27 I think we should go with option 2. The assertions for whether the cache is refreshed IMO aren't suitable here, as they are related to the wp_set_option_autoload_values() function itself, which already has sufficient coverage.

For this change, the only thing we need to test is that the autoload values have been correctly updated, which is the main purpose of the function and this change.

I agree with @mukeshpanchal27 here:

In my opinion, the second option seems preferable. When the cache is deleted, it seems logical to run a query to retrieve that option and then reload the cache.

The only reason the assertions testing the cache values are useful are if the behavior we want to confirm is that the cache is refreshed when the theme mod's autoload values are updated. I don't think that should be the responsibility of the switch_theme() function, and instead will naturally take place the next time that option is requested.

Thanks for thoughts and feedback. The PR is ready for final review and commit.

Closing in favor of #5706. Thanks for working on this!

Committed in https://core.trac.wordpress.org/changeset/57153

[flixos90]

Fixed via [57153] (wrong ticket reference 🤦‍♂️)

[flixos90]

In 57155:

Themes: Clean up inactive themes' theme mods from being autoloaded as part of upcoming 6.5 upgrade routine.

Alongside [57153], this reduces unnecessarily autoloaded data from inactive themes, which can contribute to slow database performance as part of excessive autoloading. This changeset specifically resolves the issue for existing sites.

Props mukesh27, joemcgill, swissspidy, westonruter, flixos90.
Fixes #59975.
See #39537.