diff --git a/src/wp-includes/option.php b/src/wp-includes/option.php index d2ffa675b1..c813695030 100644 --- a/src/wp-includes/option.php +++ b/src/wp-includes/option.php @@ -2125,7 +2125,7 @@ function update_network_option( $network_id, $option, $value ) { wp_protect_special_option( $option ); - $old_value = get_network_option( $network_id, $option, false ); + $old_value = get_network_option( $network_id, $option ); /** * Filters a specific network option before its value is updated. @@ -2144,20 +2144,80 @@ function update_network_option( $network_id, $option, $value ) { */ $value = apply_filters( "pre_update_site_option_{$option}", $value, $old_value, $option, $network_id ); + /* + * To get the actual raw old value from the database, any existing pre filters need to be temporarily disabled. + * Immediately after getting the raw value, they are reinstated. + * The raw value is only used to determine whether a value is present in the database. It is not used anywhere + * else, and is not passed to any of the hooks either. + */ + global $wp_filter; + + /** This filter is documented in wp-includes/option.php */ + $default_value = apply_filters( "default_site_option_{$option}", false, $option, $network_id ); + + $has_site_filter = has_filter( "pre_site_option_{$option}" ); + $has_option_filter = has_filter( "pre_option_{$option}" ); + if ( $has_site_filter || $has_option_filter ) { + if ( $has_site_filter ) { + $old_ms_filters = $wp_filter[ "pre_site_option_{$option}" ]; + unset( $wp_filter[ "pre_site_option_{$option}" ] ); + } + + if ( $has_option_filter ) { + $old_single_site_filters = $wp_filter[ "pre_option_{$option}" ]; + unset( $wp_filter[ "pre_option_{$option}" ] ); + } + + if ( is_multisite() ) { + $raw_old_value = get_network_option( $network_id, $option ); + } else { + $raw_old_value = get_option( $option, $default_value ); + } + + if ( $has_site_filter ) { + $wp_filter[ "pre_site_option_{$option}" ] = $old_ms_filters; + } + if ( $has_option_filter ) { + $wp_filter[ "pre_option_{$option}" ] = $old_single_site_filters; + } + } else { + $raw_old_value = $old_value; + } + + if ( ! is_multisite() ) { + /** This filter is documented in wp-includes/option.php */ + $default_value = apply_filters( "default_option_{$option}", $default_value, $option, true ); + } + /* * If the new and old values are the same, no need to update. * - * Unserialized values will be adequate in most cases. If the unserialized - * data differs, the (maybe) serialized data is checked to avoid - * unnecessary database calls for otherwise identical object instances. + * An exception applies when no value is set in the database, i.e. the old value is the default. + * In that case, the new value should always be added as it may be intentional to store it rather than relying on the default. * - * See https://core.trac.wordpress.org/ticket/44956 + * See https://core.trac.wordpress.org/ticket/44956 and https://core.trac.wordpress.org/ticket/22192 and https://core.trac.wordpress.org/ticket/59360 */ - if ( $value === $old_value || maybe_serialize( $value ) === maybe_serialize( $old_value ) ) { + if ( + $value === $raw_old_value || + ( + false !== $raw_old_value && + /* + * Single site stores values in the `option_value` field, which cannot be set to NULL. + * This means a PHP `null` value will be cast to an empty string, which can be considered + * equal to values such as an empty string, or false when cast to string. + * + * However, Multisite stores values in the `meta_value` field, which can be set to NULL. + * As NULL is unique in the database, skip checking an old or new value of NULL + * against any other value. + */ + ( ! is_multisite() || ! ( null === $raw_old_value || null === $value ) ) && + _is_equal_database_value( $raw_old_value, $value ) + ) + ) { return false; } - if ( false === $old_value ) { + if ( $default_value === $raw_old_value ) { return add_network_option( $network_id, $option, $value ); } diff --git a/tests/phpunit/tests/option/networkOption.php b/tests/phpunit/tests/option/networkOption.php index 7ae19abbee..2b0bed6504 100644 --- a/tests/phpunit/tests/option/networkOption.php +++ b/tests/phpunit/tests/option/networkOption.php @@ -399,7 +399,7 @@ class Tests_Option_NetworkOption extends WP_UnitTestCase { * @param mixed $old_value The old value. * @param mixed $new_value The new value to try to set. */ - public function test_update_option_should_not_update_some_values_from_cache( $old_value, $new_value ) { + public function test_update_network_option_should_not_update_some_values_from_cache( $old_value, $new_value ) { add_network_option( null, 'foo', $old_value ); $num_queries = get_num_queries(); @@ -407,9 +407,7 @@ class Tests_Option_NetworkOption extends WP_UnitTestCase { // Comparison will happen against value cached during add_option() above. $updated = update_network_option( null, 'foo', $new_value ); - $expected_queries = $old_value === $new_value || ! is_multisite() ? 0 : 1; - $this->assertSame( $expected_queries, get_num_queries() - $num_queries, "The number of queries should have increased by $expected_queries." ); - + $this->assertSame( $num_queries, get_num_queries(), 'No additional queries should have run.' ); $this->assertFalse( $updated, 'update_network_option() should have returned false.' ); } @@ -441,14 +439,7 @@ class Tests_Option_NetworkOption extends WP_UnitTestCase { $updated = update_network_option( null, 'foo', $new_value ); - // Mimic the behavior of the database by casting the old value to string. - if ( is_scalar( $old_value ) ) { - $old_value = (string) $old_value; - } - - $expected_queries = $old_value === $new_value || ! is_multisite() ? 1 : 2; - $this->assertSame( $expected_queries, get_num_queries() - $num_queries, "The number of queries should have increased by $expected_queries." ); - + $this->assertSame( 1, get_num_queries() - $num_queries, 'One additional query should have run to update the value.' ); $this->assertFalse( $updated, 'update_network_option() should have returned false.' ); } @@ -480,9 +471,7 @@ class Tests_Option_NetworkOption extends WP_UnitTestCase { * Strictly equal old and new values will cause an early return * with no additional queries. */ - $expected_queries = $old_value === $new_value || ! is_multisite() ? 0 : 1; - $this->assertSame( $expected_queries, get_num_queries() - $num_queries, "The number of queries should have increased by $expected_queries." ); - + $this->assertSame( $num_queries, get_num_queries(), 'No additional queries should have run.' ); $this->assertFalse( $updated, 'update_network_option() should have returned false.' ); } @@ -574,7 +563,7 @@ class Tests_Option_NetworkOption extends WP_UnitTestCase { * * @covers ::update_network_option */ - public function test_update_option_should_handle_a_null_new_value_from_cache() { + public function test_update_network_option_should_handle_a_null_new_value_from_cache() { add_network_option( null, 'foo', '' ); $num_queries = get_num_queries(); @@ -606,7 +595,7 @@ class Tests_Option_NetworkOption extends WP_UnitTestCase { * * @covers ::update_network_option */ - public function test_update_option_should_handle_a_null_new_value_from_db() { + public function test_update_network_option_should_handle_a_null_new_value_from_db() { add_network_option( null, 'foo', '' ); $num_queries = get_num_queries(); @@ -642,7 +631,7 @@ class Tests_Option_NetworkOption extends WP_UnitTestCase { * * @covers ::update_network_option */ - public function test_update_option_should_handle_a_null_new_value_from_refreshed_cache() { + public function test_update_network_option_should_handle_a_null_new_value_from_refreshed_cache() { add_network_option( null, 'foo', '' ); // Delete and refresh cache from DB. @@ -699,4 +688,166 @@ class Tests_Option_NetworkOption extends WP_UnitTestCase { 'null' => array( null ), ); } + + /** + * Tests that a non-existent option is added even when its pre filter returns a value. + * + * @ticket 59360 + * + * @covers ::update_network_option + */ + public function test_update_network_option_with_pre_filter_adds_missing_option() { + $hook_name = is_multisite() ? 'pre_site_option_foo' : 'pre_option_foo'; + + // Force a return value of integer 0. + add_filter( $hook_name, '__return_zero' ); + + /* + * This should succeed, since the 'foo' option does not exist in the database. + * The default value is false, so it differs from 0. + */ + $this->assertTrue( update_network_option( null, 'foo', 0 ) ); + } + + /** + * Tests that an existing option is updated even when its pre filter returns the same value. + * + * @ticket 59360 + * + * @covers ::update_network_option + */ + public function test_update_network_option_with_pre_filter_updates_option_with_different_value() { + $hook_name = is_multisite() ? 'pre_site_option_foo' : 'pre_option_foo'; + + // Add the option with a value of 1 to the database. + update_network_option( null, 'foo', 1 ); + + // Force a return value of integer 0. + add_filter( $hook_name, '__return_zero' ); + + /* + * This should succeed, since the 'foo' option has a value of 1 in the database. + * Therefore it differs from 0 and should be updated. + */ + $this->assertTrue( update_network_option( null, 'foo', 0 ) ); + } + + /** + * Tests that calling update_network_option() does not permanently remove pre filters. + * + * @ticket 59360 + * + * @covers ::update_network_option + */ + public function test_update_network_option_maintains_pre_filters() { + $hook_name = is_multisite() ? 'pre_site_option_foo' : 'pre_option_foo'; + + add_filter( $hook_name, '__return_zero' ); + update_network_option( null, 'foo', 0 ); + + // Assert that the filter is still present. + $this->assertSame( 10, has_filter( $hook_name, '__return_zero' ) ); + } + + /** + * Tests that update_network_option() conditionally applies + * 'pre_site_option_{$option}' and 'pre_option_{$option}' filters. + * + * @ticket 59360 + * + * @covers ::update_network_option + */ + public function test_update_network_option_should_conditionally_apply_pre_site_option_and_pre_option_filters() { + $option = 'foo'; + $site_hook = new MockAction(); + $option_hook = new MockAction(); + + add_filter( "pre_site_option_{$option}", array( $site_hook, 'filter' ) ); + add_filter( "pre_option_{$option}", array( $option_hook, 'filter' ) ); + + update_network_option( null, $option, 'false' ); + + $this->assertSame( 1, $site_hook->get_call_count(), "'pre_site_option_{$option}' filters occurred an unexpected number of times." ); + $this->assertSame( is_multisite() ? 0 : 1, $option_hook->get_call_count(), "'pre_option_{$option}' filters occurred an unexpected number of times." ); + } + + /** + * Tests that update_network_option() conditionally applies + * 'default_site_{$option}' and 'default_option_{$option}' filters. + * + * @ticket 59360 + * + * @covers ::update_network_option + */ + public function test_update_network_option_should_conditionally_apply_site_and_option_default_value_filters() { + $option = 'foo'; + $site_hook = new MockAction(); + $option_hook = new MockAction(); + + add_filter( "default_site_option_{$option}", array( $site_hook, 'filter' ) ); + add_filter( "default_option_{$option}", array( $option_hook, 'filter' ) ); + + update_network_option( null, $option, 'false' ); + + $this->assertSame( 2, $site_hook->get_call_count(), "'default_site_option_{$option}' filters occurred an unexpected number of times." ); + $this->assertSame( is_multisite() ? 0 : 2, $option_hook->get_call_count(), "'default_option_{$option}' filters occurred an unexpected number of times." ); + } + + /** + * Tests that update_network_option() adds a non-existent option that uses a filtered default value. + * + * @ticket 59360 + * + * @covers ::update_network_option + */ + public function test_update_network_option_should_add_option_with_filtered_default_value() { + global $wpdb; + + $option = 'foo'; + $default_site_value = 'default-site-value'; + $default_option_value = 'default-option-value'; + + add_filter( + "default_site_option_{$option}", + static function () use ( $default_site_value ) { + return $default_site_value; + } + ); + + add_filter( + "default_option_{$option}", + static function () use ( $default_option_value ) { + return $default_option_value; + } + ); + + /* + * For a non existing option with the unfiltered default of false, passing false here wouldn't work. + * Because the default is different than false here though, passing false is expected to result in + * a database update. + */ + $this->assertTrue( update_network_option( null, $option, false ), 'update_network_option() should have returned true.' ); + + if ( is_multisite() ) { + $actual = $wpdb->get_row( + $wpdb->prepare( + "SELECT meta_value FROM $wpdb->sitemeta WHERE meta_key = %s LIMIT 1", + $option + ) + ); + } else { + $actual = $wpdb->get_row( + $wpdb->prepare( + "SELECT option_value FROM $wpdb->options WHERE option_name = %s LIMIT 1", + $option + ) + ); + } + + $value_field = is_multisite() ? 'meta_value' : 'option_value'; + + $this->assertIsObject( $actual, 'The option was not added to the database.' ); + $this->assertObjectHasProperty( $value_field, $actual, "The '$value_field' property was not included." ); + $this->assertSame( '', $actual->$value_field, 'The new value was not stored in the database.' ); + } }