From 17a01ed185f38f87362eb7588908dc95ec6cbe23 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Mon, 30 Oct 2023 22:56:25 +0000 Subject: [PATCH] Options, Meta APIs: Fast follow fixes for option cache priming functions. A collection of fixes for `wp_prime_option_caches()`: * cache arrays and objects in their serialized form for consistency with `get_option()` and `wp_load_alloptions()` * prevent repeat database queries for falsey and known non-existent options (notoptions) Additional tests for `wp_prime_option_caches()` to ensure: * additional database queries are not made repriming options (known, known-unknown and alloptions) * cache is primed consistently * `get_option()` returns a consistent value regardless of how it is primed * database queries do not contain earlier primed options * `get_option` does not prime the cache when testing the cache has been successfully primed Fixes a test for `wp_prime_option_caches_by_group()` to ensure `get_option` does not prime the cache when testing the cache has been successfully primed. Follow up to [56445],[56990],[57013]. Props peterwilsoncc, costdev, flixos90, hellofromTonya, mikeschroder, joemcgill. Fixes #59738. See #58962. git-svn-id: https://develop.svn.wordpress.org/trunk@57029 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/option.php | 23 +- .../tests/option/wpPrimeOptionCaches.php | 341 +++++++++++++++++- .../option/wpPrimeOptionCachesByGroup.php | 13 +- 3 files changed, 361 insertions(+), 16 deletions(-) diff --git a/src/wp-includes/option.php b/src/wp-includes/option.php index e587931173..13f5d49c87 100644 --- a/src/wp-includes/option.php +++ b/src/wp-includes/option.php @@ -261,11 +261,19 @@ function get_option( $option, $default_value = false ) { function wp_prime_option_caches( $options ) { $alloptions = wp_load_alloptions(); $cached_options = wp_cache_get_multiple( $options, 'options' ); + $notoptions = wp_cache_get( 'notoptions', 'options' ); + if ( ! is_array( $notoptions ) ) { + $notoptions = array(); + } // Filter options that are not in the cache. $options_to_prime = array(); foreach ( $options as $option ) { - if ( ( ! isset( $cached_options[ $option ] ) || ! $cached_options[ $option ] ) && ! isset( $alloptions[ $option ] ) ) { + if ( + ( ! isset( $cached_options[ $option ] ) || false === $cached_options[ $option ] ) + && ! isset( $alloptions[ $option ] ) + && ! isset( $notoptions[ $option ] ) + ) { $options_to_prime[] = $option; } } @@ -288,7 +296,12 @@ function wp_prime_option_caches( $options ) { $options_found = array(); foreach ( $results as $result ) { - $options_found[ $result->option_name ] = maybe_unserialize( $result->option_value ); + /* + * The cache is primed with the raw value (i.e. not maybe_unserialized). + * + * `get_option()` will handle unserializing the value as needed. + */ + $options_found[ $result->option_name ] = $result->option_value; } wp_cache_set_multiple( $options_found, 'options' ); @@ -299,12 +312,6 @@ function wp_prime_option_caches( $options ) { $options_not_found = array_diff( $options_to_prime, array_keys( $options_found ) ); - $notoptions = wp_cache_get( 'notoptions', 'options' ); - - if ( ! is_array( $notoptions ) ) { - $notoptions = array(); - } - // Add the options that were not found to the cache. $update_notoptions = false; foreach ( $options_not_found as $option_name ) { diff --git a/tests/phpunit/tests/option/wpPrimeOptionCaches.php b/tests/phpunit/tests/option/wpPrimeOptionCaches.php index 68b7d526b4..8b75bc286c 100644 --- a/tests/phpunit/tests/option/wpPrimeOptionCaches.php +++ b/tests/phpunit/tests/option/wpPrimeOptionCaches.php @@ -41,15 +41,19 @@ class Tests_Option_WpPrimeOptionCaches extends WP_UnitTestCase { // Check that options are only in the 'options' cache group. foreach ( $options_to_prime as $option ) { $this->assertSame( + "value_$option", wp_cache_get( $option, 'options' ), - get_option( $option ), "$option was not primed in the 'options' cache group." ); - $this->assertFalse( - wp_cache_get( $option, 'notoptions' ), - get_option( $option ), - "$option was primed in the 'notoptions' cache group." + $new_notoptions = wp_cache_get( $option, 'notoptions' ); + if ( ! is_array( $new_notoptions ) ) { + $new_notoptions = array(); + } + $this->assertArrayNotHasKey( + $option, + $new_notoptions, + "$option was primed in the 'notoptions' cache." ); } @@ -61,10 +65,70 @@ class Tests_Option_WpPrimeOptionCaches extends WP_UnitTestCase { ); } + /** + * Tests that wp_prime_option_caches() handles a mix of primed and unprimed options. + * + * @ticket 58962 + */ + public function test_wp_prime_option_caches_handles_a_mix_of_primed_and_unprimed_options() { + global $wpdb; + // Create some options to prime. + $options_to_prime = array( + 'option1', + 'option2', + 'option3', + ); + + /* + * Set values for the options, + * clear the cache for the options, + * check options are not in cache initially. + */ + foreach ( $options_to_prime as $option ) { + update_option( $option, "value_$option", false ); + wp_cache_delete( $option, 'options' ); + $this->assertFalse( wp_cache_get( $option, 'options' ), "$option was not deleted from the cache." ); + } + + // Add non-existent option to the options to prime. + $options_to_prime[] = 'option404notfound'; + + // Prime the first option with a non-existent option. + wp_prime_option_caches( array( 'option1', 'option404notfound' ) ); + + // Store the initial database query count. + $initial_query_count = get_num_queries(); + + // Prime all the options, including the pre-primed option. + wp_prime_option_caches( $options_to_prime ); + + // Ensure an additional database query was made. + $this->assertSame( + 1, + get_num_queries() - $initial_query_count, + 'Additional database queries were not made.' + ); + + // Ensure the last query does not contain the pre-primed option. + $this->assertStringNotContainsString( + "\'option1\'", + $wpdb->last_query, + 'The last query should not contain the pre-primed option.' + ); + + // Ensure the last query does not contain the pre-primed notoption. + $this->assertStringNotContainsString( + "\'option404notfound\'", + $wpdb->last_query, + 'The last query should not contain the pre-primed non-existent option.' + ); + } + /** * Tests wp_prime_option_caches() with options that do not exist in the database. * * @ticket 58962 + * @ticket 59738 */ public function test_wp_prime_option_caches_with_nonexistent_options() { // Create some options to prime. @@ -96,27 +160,55 @@ class Tests_Option_WpPrimeOptionCaches extends WP_UnitTestCase { foreach ( $options_to_prime as $option ) { $this->assertArrayHasKey( $option, $new_notoptions, "$option was not added to the notoptions cache." ); } + + // Check getting and re-priming the options does not result in additional database queries. + $initial_query_count = get_num_queries(); + foreach ( $options_to_prime as $option ) { + get_option( $option ); + $this->assertSame( + 0, + get_num_queries() - $initial_query_count, + "Additional database queries were made getting option $option." + ); + } + + wp_prime_option_caches( $options_to_prime ); + $this->assertSame( + 0, + get_num_queries() - $initial_query_count, + 'Additional database queries were made re-priming the options.' + ); } /** * Tests wp_prime_option_caches() with an empty array. * * @ticket 58962 + * @ticket 59738 */ public function test_wp_prime_option_caches_with_empty_array() { $alloptions = wp_load_alloptions(); $notoptions = wp_cache_get( 'notoptions', 'options' ); + $initial_query_count = get_num_queries(); wp_prime_option_caches( array() ); $this->assertSame( $alloptions, wp_cache_get( 'alloptions', 'options' ), 'The alloptions cache was modified.' ); $this->assertSame( $notoptions, wp_cache_get( 'notoptions', 'options' ), 'The notoptions cache was modified.' ); + + // Check priming an empty array does not result in additional database queries. + $this->assertSame( + 0, + get_num_queries() - $initial_query_count, + 'Additional database queries were made.' + ); } /** * Tests that wp_prime_option_caches() handles an empty "notoptions" cache. * * @ticket 58962 + * @ticket 59738 */ public function test_wp_prime_option_caches_handles_empty_notoptions_cache() { wp_cache_delete( 'notoptions', 'options' ); @@ -126,5 +218,244 @@ class Tests_Option_WpPrimeOptionCaches extends WP_UnitTestCase { $notoptions = wp_cache_get( 'notoptions', 'options' ); $this->assertIsArray( $notoptions, 'The notoptions cache should be an array.' ); $this->assertArrayHasKey( 'nonexistent_option', $notoptions, 'nonexistent_option was not added to notoptions.' ); + + // Check getting and re-priming the options does not result in additional database queries. + $initial_query_count = get_num_queries(); + + get_option( 'nonexistent_option' ); + $this->assertSame( + 0, + get_num_queries() - $initial_query_count, + 'Additional database queries were made getting nonexistent_option.' + ); + + wp_prime_option_caches( array( 'nonexistent_option' ) ); + $this->assertSame( + 0, + get_num_queries() - $initial_query_count, + 'Additional database queries were made.' + ); + } + + /** + * Test options primed by the wp_prime_option_caches() function are identical to those primed by get_option(). + * + * @ticket 59738 + * + * @dataProvider data_option_types + * + * @param mixed $option_value An option value. + */ + public function test_get_option_should_return_identical_value_when_pre_primed_by_wp_prime_option_caches( $option_value ) { + // As this includes a test setting the value to `(bool) false`, update_option() can not be used so add_option() is used instead. + add_option( 'type_of_option', $option_value, '', false ); + wp_cache_delete( 'type_of_option', 'options' ); + + $this->assertFalse( wp_cache_get( 'type_of_option', 'options' ), 'type_of_option was not deleted from the cache for priming.' ); + + // Call the wp_prime_option_caches function to prime the options. + wp_prime_option_caches( array( 'type_of_option' ) ); + $value_after_pre_priming = get_option( 'type_of_option' ); + + // Clear the cache and call get_option directly. + wp_cache_delete( 'type_of_option', 'options' ); + $this->assertFalse( wp_cache_get( 'type_of_option', 'options' ), 'type_of_option was not deleted from the cache for get_option.' ); + $value_after_get_option = get_option( 'type_of_option' ); + + /* + * If the option value is an object, use assertEquals() to compare the values. + * + * This is to compare the shape of the object rather than the identity of the object. + */ + if ( is_object( $option_value ) ) { + $this->assertEquals( $value_after_get_option, $value_after_pre_priming, 'The values should be equal.' ); + } else { + $this->assertSame( $value_after_get_option, $value_after_pre_priming, 'The values should be identical.' ); + } + } + + /** + * Tests that wp_prime_option_caches() shapes the cache in the same fashion as get_option() + * + * @ticket 59738 + * + * @dataProvider data_option_types + * + * @param mixed $option_value An option value. + */ + public function test_wp_prime_option_caches_cache_should_be_identical_to_get_option_cache( $option_value ) { + // As this includes a test setting the value to `(bool) false`, update_option() can not be used so add_option() is used instead. + add_option( 'type_of_option', $option_value, '', false ); + wp_cache_delete( 'type_of_option', 'options' ); + + $this->assertFalse( wp_cache_get( 'type_of_option', 'options' ), 'type_of_option was not deleted from the cache for wp_prime_option_caches().' ); + + // Call the wp_prime_option_caches function to prime the options. + wp_prime_option_caches( array( 'type_of_option' ) ); + $value_from_priming = wp_cache_get( 'type_of_option', 'options' ); + + wp_cache_delete( 'type_of_option', 'options' ); + $this->assertFalse( wp_cache_get( 'type_of_option', 'options' ), 'type_of_option was not deleted from the cache for get_option().' ); + + // Call get_option() to prime the options. + get_option( 'type_of_option' ); + $value_from_get_option = wp_cache_get( 'type_of_option', 'options' ); + + $this->assertIsString( $value_from_get_option, 'Cache from get_option() should always be a string' ); + $this->assertIsString( $value_from_priming, 'Cache from wp_prime_option_caches() should always be a string' ); + $this->assertSame( $value_from_get_option, $value_from_priming, 'The values should be identical.' ); + } + + /** + * Tests that wp_prime_option_caches() doesn't trigger DB queries on already primed options. + * + * @ticket 59738 + * + * @dataProvider data_option_types + * + * @param mixed $option_value An option value. + */ + public function test_wp_prime_option_caches_does_not_trigger_db_queries_repriming_options( $option_value ) { + // As this includes a test setting the value to `(bool) false`, update_option() can not be used so add_option() is used instead. + add_option( 'double_primed_option', $option_value, '', false ); + wp_cache_delete( 'double_primed_option', 'options' ); + $options_to_prime = array( 'double_primed_option' ); + + $this->assertFalse( wp_cache_get( 'double_primed_option', 'options' ), 'double_primed_option was not deleted from the cache.' ); + + // Call the wp_prime_option_caches function to prime the options. + wp_prime_option_caches( $options_to_prime ); + + // Store the initial database query count. + $initial_query_count = get_num_queries(); + + // Check that options are only in the 'options' cache group. + foreach ( $options_to_prime as $option ) { + $this->assertNotFalse( + wp_cache_get( $option, 'options' ), + "$option was not primed in the 'options' cache group." + ); + + $new_notoptions = wp_cache_get( $option, 'notoptions' ); + if ( ! is_array( $new_notoptions ) ) { + $new_notoptions = array(); + } + $this->assertArrayNotHasKey( + $option, + $new_notoptions, + "$option was primed in the 'notoptions' cache." + ); + } + + // Call the wp_prime_option_caches function to prime the options. + wp_prime_option_caches( $options_to_prime ); + + // Ensure no additional database queries were made. + $this->assertSame( + $initial_query_count, + get_num_queries(), + 'Additional database queries were made.' + ); + } + + /** + * Tests that wp_prime_option_caches() doesn't trigger DB queries for items primed in alloptions. + * + * @ticket 59738 + * + * @dataProvider data_option_types + * + * @param mixed $option_value An option value. + */ + public function test_wp_prime_option_caches_does_not_trigger_db_queries_for_alloptions( $option_value ) { + // As this includes a test setting the value to `(bool) false`, update_option() can not be used so add_option() is used instead. + add_option( 'option_in_alloptions', $option_value, '', true ); + wp_cache_delete( 'alloptions', 'options' ); + wp_cache_delete( 'option_in_alloptions', 'options' ); + $options_to_prime = array( 'option_in_alloptions' ); + + $this->assertFalse( wp_cache_get( 'option_in_alloptions', 'options' ), 'option_in_alloptions was not deleted from the cache.' ); + $this->assertFalse( wp_cache_get( 'alloptions', 'options' ), 'alloptions was not deleted from the cache.' ); + + // Prime the alloptions cache. + wp_load_alloptions(); + + // Store the initial database query count. + $initial_query_count = get_num_queries(); + + // Call the wp_prime_option_caches function to reprime the option. + wp_prime_option_caches( $options_to_prime ); + + // Check that options are in the 'alloptions' cache only. + foreach ( $options_to_prime as $option ) { + $this->assertFalse( + wp_cache_get( $option, 'options' ), + "$option was primed in the 'options' cache group." + ); + + $new_notoptions = wp_cache_get( $option, 'notoptions' ); + if ( ! is_array( $new_notoptions ) ) { + $new_notoptions = array(); + } + $this->assertArrayNotHasKey( + $option, + $new_notoptions, + "$option was primed in the 'notoptions' cache." + ); + + $new_alloptions = wp_cache_get( 'alloptions', 'options' ); + if ( ! is_array( $new_alloptions ) ) { + $new_alloptions = array(); + } + $this->assertArrayHasKey( + $option, + $new_alloptions, + "$option was not primed in the 'alloptions' cache." + ); + } + + // Ensure no additional database queries were made. + $this->assertSame( + 0, + get_num_queries() - $initial_query_count, + 'Additional database queries were made.' + ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_option_types() { + return array( + 'null' => array( null ), + '(bool) false' => array( false ), + '(bool) true' => array( true ), + '(int) 0' => array( 0 ), + '(int) -0' => array( -0 ), + '(int) 1' => array( 1 ), + '(int) -1' => array( -1 ), + '(float) 0.0' => array( 0.0 ), + '(float) -0.0' => array( -0.0 ), + '(float) 1.0' => array( 1.0 ), + 'empty string' => array( '' ), + 'string with only tabs' => array( "\t\t" ), + 'string with only newlines' => array( "\n\n" ), + 'string with only carriage returns' => array( "\r\r" ), + 'string with only spaces' => array( ' ' ), + 'populated string' => array( 'string' ), + 'string (1)' => array( '1' ), + 'string (0)' => array( '0' ), + 'string (0.0)' => array( '0.0' ), + 'string (-0)' => array( '-0' ), + 'string (-0.0)' => array( '-0.0' ), + 'empty array' => array( array() ), + 'populated array' => array( array( 'string' ) ), + 'empty object' => array( new stdClass() ), + 'populated object' => array( (object) array( 'string' ) ), + 'INF' => array( INF ), + 'NAN' => array( NAN ), + ); } } diff --git a/tests/phpunit/tests/option/wpPrimeOptionCachesByGroup.php b/tests/phpunit/tests/option/wpPrimeOptionCachesByGroup.php index dac621122d..33ba6bc07d 100644 --- a/tests/phpunit/tests/option/wpPrimeOptionCachesByGroup.php +++ b/tests/phpunit/tests/option/wpPrimeOptionCachesByGroup.php @@ -47,9 +47,16 @@ class Tests_Option_WpPrimeOptionCachesByGroup extends WP_UnitTestCase { // Call the wp_prime_option_caches_by_group function to prime the options. wp_prime_option_caches_by_group( 'group1' ); - // Check that options are now in the cache. - $this->assertSame( get_option( 'option1' ), wp_cache_get( 'option1', 'options' ), 'option1\'s cache was not primed.' ); - $this->assertSame( get_option( 'option2' ), wp_cache_get( 'option2', 'options' ), 'option2\'s cache was not primed.' ); + /* + * Check that options are now in the cache. + * + * Repeat the string here rather than using get_option as get_option + * will prime the cache before the call to wp_cache_get if the option + * is not in the cache. Thus causing the tests to pass when they should + * fail. + */ + $this->assertSame( 'value_option1', wp_cache_get( 'option1', 'options' ), 'option1\'s cache was not primed.' ); + $this->assertSame( 'value_option2', wp_cache_get( 'option2', 'options' ), 'option2\'s cache was not primed.' ); // Make sure option3 is still not in cache. $this->assertFalse( wp_cache_get( 'option3', 'options' ), 'option3 was not deleted from the cache.' );