Unexpected behavior when switching autoload between update_option

[pentatonicfunk]

Autoload update, gives unexpected behavior. Replicated with this flow :

1. Update x option to value "A" with autoload = yes, all good here. DB updated, and alloptions cache updated.
2. Update x option to value = "B" with autoload = no. Function in effect update_option
   • Expected behavior :
     • Its should $wpdb->update
     • Its should delete it from alloptions cache,
     • Its should update "x" cache options
   • Actual behavior :
     • Its succesfully execute $wpdb->update ( correct )
     • Its updating alloptions cache, even the "autoload" is set as no
     • Its not updating "x" cache options
3. delete x option. Function in effect delete_option
   • Expected behavior :
     • Its should $wpdb->delete
     • Its should delete "x" cache options ( since autoload = no )
   • Actual behavior :
     • Its succesfully execute $wpdb->delete -- correct
     • Its deleting "x" cache options ( since autoload = no ) -- correct
     • alloptions cache still has x option, because of (2) behavior
4. get x option. Function in effect get_option
   • Expected behavior :
     • It should returns FALSE since its already deleted on (3)
   • Actual behavior :
     • It returns "B", because get_option prioritze wp_load_alloptions, and alloptions cache is still exists for x, see (2) and (3)
5. update x option to value "C" with autoload = no
   • Expected behavior :
     • Its should calls add_option, because its already deleted on (3)
     • Its should delete it from alloptions cache,
     • Its should update "x" cache options
   • Actual behavior :
     • Its not executing add_option, since update_option check old value by using get_option which returning "B" at the moment, so it will attempting to $wpdb->update instead, which eventually will fail, since its DB entry alreeady deleted on (3)
     • Its not deleting alloptions cache, because DB update fails
     • Its not updating "x" cache options, because DB update fails

Test code :

<?php
// just to make sure we use unique option name for each variant, so it won't interfering
$option_name = "x_" . wp_generate_uuid4();

error_log( "###VARIANT #1 : switch autoload from yes to no---" );//phpcs:ignore
/**
 * #1
 */
error_log( "---UPDATING TO A, AUTOLOAD = YES---" );//phpcs:ignore
$success = update_option( $option_name, "A", "yes" );
error_log( "Should updated successfully : YES, Actual result : " . ( $success ? "YES" : "NO" ) );// phpcs:ignore
error_log( "Should return : 'A', Actual Return : " . var_export( get_option( $option_name ), true ) );// phpcs:ignore

/**
 * #2
 */
error_log( "\n---UPDATING TO B, AUTOLOAD = NO---" );//phpcs:ignore
$success = update_option( $option_name, "B", "no" );
error_log( "Should updated successfully : YES, Actual result : " . ( $success ? "YES" : "NO" ) );// phpcs:ignore
error_log( "Should return : 'B', Actual Return : " . var_export( get_option( $option_name ), true ) );// phpcs:ignore

/**
 * #3
 */
error_log( "\n---DELETING---" );//phpcs:ignore
$success = delete_option( $option_name );
error_log( "Should deleted successfully : YES, Actual result : " . ( $success ? "YES" : "NO" ) );// phpcs:ignore
/**
 * #4
 */
error_log( "Should return : false, Actual Return : " . var_export( get_option( $option_name ), true ) );// phpcs:ignore

/**
 * #5
 */
error_log( "\n---UPDATING TO C, AUTOLOAD = NO---" );//phpcs:ignore
$success = update_option( $option_name, "C" );
error_log( "Should updated successfully : YES, Actual result : " . ( $success ? "YES" : "NO" ) );// phpcs:ignore
/**
 * #4
 */
error_log( "Should return : 'C', Actual Return : " . var_export( get_option( $option_name ), true ) );// phpcs:ignore


// just to make sure we use unique option name for each test variant, so it won't interfering
$option_name = "x_" . wp_generate_uuid4();
error_log( "\n-----------------------" );//phpcs:ignore
error_log( "\n###VARIANT #2: switch autoload from no to yes---" );//phpcs:ignore
/**
 * #1
 */
error_log( "---UPDATING TO A, AUTOLOAD = NO---" );//phpcs:ignore
$success = update_option( $option_name, "A", "no" );
error_log( "Should updated successfully : YES, Actual result : " . ( $success ? "YES" : "NO" ) );// phpcs:ignore
error_log( "Should return : 'A', Actual Return : " . var_export( get_option( $option_name ), true ) );// phpcs:ignore

/**
 * #2
 */
error_log( "\n---UPDATING TO B, AUTOLOAD = YES---" );//phpcs:ignore
$success = update_option( $option_name, "B", "yes" );
error_log( "Should updated successfully : YES, Actual result : " . ( $success ? "YES" : "NO" ) );// phpcs:ignore
error_log( "Should return : 'B', Actual Return : " . var_export( get_option( $option_name ), true ) );// phpcs:ignore

/**
 * #3
 */
error_log( "\n---DELETING---" );//phpcs:ignore
$success = delete_option( $option_name );
error_log( "Should deleted successfully : YES, Actual result : " . ( $success ? "YES" : "NO" ) );// phpcs:ignore
/**
 * #4
 */
error_log( "Should return : false, Actual Return : " . var_export( get_option( $option_name ), true ) );// phpcs:ignore

/**
 * #5
 */
error_log( "\n---UPDATING TO C, AUTOLOAD = YES---" );//phpcs:ignore
$success = update_option( $option_name, "C", "yes" );
error_log( "Should updated successfully : YES, Actual result : " . ( $success ? "YES" : "NO" ) );// phpcs:ignore
/**
 * #4
 */
error_log( "Should return : 'C', Actual Return : " . var_export( get_option( $option_name ), true ) );// phpcs:ignore

Changes :

• Use autoload flag from function argument, or fallback to previous autoload value.
• Invalidates old cache and update cache based on current autoload.

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

[SergeyBiryukov]

Related: #51372

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

[kkmuffme]

Just to get this rolling again, since many related issues have actually already been fixed.

The issue above is in 2) only (which is what the existing PR fixes): the issue is basically that "autoload" value is not updated in cache, which then causes non-autoloaded options to be incorrectly cached and vice versa.

However the PR has some performance issues, so I created a new PR on trunk on the latest version.

The PR fixes the current issue + probably #51372 (this was 99% fixed already though in the current WP version) + probably #38931.

This really is a major issue that should get merged with WP 6.3. Could you @SergeyBiryukov be so kind and let me know what is needed for my PR to get merged?

[SergeyBiryukov]

Nice, thanks for the PR! Moving to 6.3 for review.