diff --git a/src/wp-includes/option.php b/src/wp-includes/option.php index d2ffa675b1854..78d8ee108e05c 100644 --- a/src/wp-includes/option.php +++ b/src/wp-includes/option.php @@ -2147,13 +2147,18 @@ function update_network_option( $network_id, $option, $value ) { /* * 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/38903 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 === $old_value || + ( + false !== $old_value && + _is_equal_database_value( $old_value, $value ) + ) + ) { return false; } diff --git a/tests/phpunit/tests/option/networkOption.php b/tests/phpunit/tests/option/networkOption.php index 7ae19abbeeaee..0e084d0484aaf 100644 --- a/tests/phpunit/tests/option/networkOption.php +++ b/tests/phpunit/tests/option/networkOption.php @@ -407,9 +407,7 @@ public function test_update_option_should_not_update_some_values_from_cache( $ol // 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.' ); } @@ -446,9 +444,7 @@ public function test_update_network_option_should_not_update_some_values_from_db $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 +476,7 @@ public function test_update_network_option_should_not_update_some_values_from_re * 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.' ); } @@ -567,8 +561,8 @@ public function data_strictly_equal_values() { * On Single Site, this will result in no additional queries as * the option_value database field is not nullable. * - * On Multisite, this will result in one additional query as - * the meta_value database field is nullable. + * On Multisite, this will result in no additional query because + * of early return. * * @ticket 59360 * @@ -582,14 +576,8 @@ public function test_update_option_should_handle_a_null_new_value_from_cache() { // Comparison will happen against value cached during add_option() above. $updated = update_network_option( null, 'foo', null ); - $expected_queries = is_multisite() ? 1 : 0; - $this->assertSame( $expected_queries, get_num_queries() - $num_queries, "The number of queries should have increased by $expected_queries." ); - - if ( is_multisite() ) { - $this->assertTrue( $updated, 'update_network_option() should have returned true.' ); - } else { - $this->assertFalse( $updated, 'update_network_option() should have returned false.' ); - } + $this->assertSame( $num_queries, get_num_queries(), 'No additional queries should have run.' ); + $this->assertFalse( $updated, 'update_network_option() should have returned false.' ); } /** @@ -599,8 +587,8 @@ public function test_update_option_should_handle_a_null_new_value_from_cache() { * On Single Site, this will result in only 1 additional query as * the option_value database field is not nullable. * - * On Multisite, this will result in two additional queries as - * the meta_value database field is nullable. + * On Multisite, this will result in only 1 additional queries as + * the meta_value database field is not nullable. * * @ticket 59360 * @@ -618,14 +606,8 @@ public function test_update_option_should_handle_a_null_new_value_from_db() { $updated = update_network_option( null, 'foo', null ); - $expected_queries = is_multisite() ? 2 : 1; - $this->assertSame( $expected_queries, get_num_queries() - $num_queries, "The number of queries should have increased by $expected_queries." ); - - if ( is_multisite() ) { - $this->assertTrue( $updated, 'update_network_option() should have returned true.' ); - } else { - $this->assertFalse( $updated, 'update_network_option() should have returned false.' ); - } + $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.' ); } /** @@ -635,8 +617,8 @@ public function test_update_option_should_handle_a_null_new_value_from_db() { * On Single Site, this will result in no additional queries as * the option_value database field is not nullable. * - * On Multisite, this will result in one additional query as - * the meta_value database field is nullable. + * On Multisite, this will result in no additional query as + * the meta_value database field is not nullable. * * @ticket 59360 * @@ -652,14 +634,8 @@ public function test_update_option_should_handle_a_null_new_value_from_refreshed $num_queries = get_num_queries(); $updated = update_network_option( null, 'foo', null ); - $expected_queries = is_multisite() ? 1 : 0; - $this->assertSame( $expected_queries, get_num_queries() - $num_queries, "The number of queries should have increased by $expected_queries." ); - - if ( is_multisite() ) { - $this->assertTrue( $updated, 'update_network_option() should have returned true.' ); - } else { - $this->assertFalse( $updated, 'update_network_option() should have returned false.' ); - } + $this->assertSame( $num_queries, get_num_queries(), 'No additional queries should have run.' ); + $this->assertFalse( $updated, 'update_network_option() should have returned false.' ); } /**