Changeset 56681

Timestamp:

09/25/2023 04:23:52 PM (3 years ago)

Author:

flixos90

Message:

Options, Meta APIs: Improve logic to avoid unnecessary database writes in update_option().

Prior to this change, a strict comparison between the old and new database value could lead to a false negative, since database values are generally stored as strings. For example, passing an integer to update_option() would almost always result in an update given any existing database value for that option would be that number cast to a string.

This changeset adjusts the logic to perform an intentional "loose-y" comparison by casting the values to strings. Extensive coverage previously added in [56648] provides additional confidence that this does not introduce any backward compatibility issues.

Props mukesh27, costdev, spacedmonkey, joemcgill, flixos90, nacin, atimmer, duck_, boonebgorges.
Fixes #22192.

Location:

trunk

Files:

• src/wp-includes/option.php (modified) (2 diffs)
• tests/phpunit/tests/option/isEqualDatabaseValue.php (added)
• tests/phpunit/tests/option/option.php (modified) (1 diff)

trunk/src/wp-includes/option.php

@@ -777 +777 @@
         $value = apply_filters( 'pre_update_option', $value, $option, $old_value );
 
+        /** This filter is documented in wp-includes/option.php */
+        $default_value = apply_filters( "default_option_{$option}", false, $option, false );
+
         /*
          * 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.
-         *
-         * See https://core.trac.wordpress.org/ticket/38903
-         */
-        if ( $value === $old_value || maybe_serialize( $value ) === maybe_serialize( $old_value ) ) {
+         * 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/38903 and https://core.trac.wordpress.org/ticket/22192.
+         */
+        if ( $old_value !== $default_value && _is_equal_database_value( $old_value, $value ) ) {
                 return false;
         }
 
-        /** This filter is documented in wp-includes/option.php */
-        if ( apply_filters( "default_option_{$option}", false, $option, false ) === $old_value ) {
+        if ( $old_value === $default_value ) {
+
                 // Default setting for new options is 'yes'.
                 if ( null === $autoload ) {
@@ -2888 +2890 @@
         return $registered[ $option ]['default'];
 }
+
+/**
+ * Determines whether two values will be equal when stored in the database.
+ *
+ * @since 6.4.0
+ * @access private
+ *
+ * @param mixed $old_value The old value to compare.
+ * @param mixed $new_value The new value to compare.
+ * @return bool True if the values are equal, false otherwise.
+ */
+function _is_equal_database_value( $old_value, $new_value ) {
+        $values = array(
+                'old' => $old_value,
+                'new' => $new_value,
+        );
+
+        foreach ( $values as $_key => &$_value ) {
+                // Cast scalars or null to a string so type discrepancies don't result in cache misses.
+                if ( null === $_value || is_scalar( $_value ) ) {
+                        $_value = (string) $_value;
+                }
+        }
+
+        if ( $values['old'] === $values['new'] ) {
+                return true;
+        }
+
+        /*
+         * 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.
+         *
+         * See https://core.trac.wordpress.org/ticket/38903
+         */
+        return maybe_serialize( $old_value ) === maybe_serialize( $new_value );
+}

trunk/tests/phpunit/tests/option/option.php

@@ -525 +525 @@
                 );
         }
+
+        /**
+         * Tests that update_option() stores an option that uses
+         * an unfiltered default value of (bool) false.
+         *
+         * @ticket 22192
+         *
+         * @covers ::update_option
+         */
+        public function test_update_option_should_store_option_with_default_value_false() {
+                global $wpdb;
+
+                $option = 'update_option_default_false';
+                update_option( $option, false );
+
+                $actual = $wpdb->query(
+                        $wpdb->prepare(
+                                "SELECT option_name FROM $wpdb->options WHERE option_name = %s LIMIT 1",
+                                $option
+                        )
+                );
+
+                $this->assertSame( 1, $actual );
+        }
+
+        /**
+         * Tests that update_option() stores an option that uses
+         * a filtered default value.
+         *
+         * @ticket 22192
+         *
+         * @covers ::update_option
+         */
+        public function test_update_option_should_store_option_with_filtered_default_value() {
+                global $wpdb;
+
+                $option        = 'update_option_custom_default';
+                $default_value = 'default-value';
+
+                add_filter(
+                        "default_option_{$option}",
+                        static function () use ( $default_value ) {
+                                return $default_value;
+                        }
+                );
+
+                update_option( $option, $default_value );
+
+                $actual = $wpdb->query(
+                        $wpdb->prepare(
+                                "SELECT option_name FROM $wpdb->options WHERE option_name = %s LIMIT 1",
+                                $option
+                        )
+                );
+
+                $this->assertSame( 1, $actual );
+        }
 }