Skip to content

Introduce and implement _get_raw_option_value(). #94

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open

Introduce and implement _get_raw_option_value(). #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b1e2615
Introduce and implement `_get_raw_option_value()`.
Oct 13, 2023
ffa3c89
Update Single Site pre-filter tests.
Oct 13, 2023
584ff52
Update Multisite pre-filter tests.
Oct 13, 2023
f30f5e1
Add Joe's tests.
Oct 14, 2023
945a2df
Skip equal check for filtered option values.
Oct 14, 2023
f5fbdec
Yoast BC fix (don't commit with this in the PR)
Oct 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 81 additions & 51 deletions src/wp-includes/option.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,27 +776,19 @@ function update_option( $option, $value, $autoload = null ) {
*/
$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 );

/*
* 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.
*/
if ( has_filter( "pre_option_{$option}" ) ) {
global $wp_filter;

$old_filters = $wp_filter[ "pre_option_{$option}" ];
unset( $wp_filter[ "pre_option_{$option}" ] );

$raw_old_value = get_option( $option );
$wp_filter[ "pre_option_{$option}" ] = $old_filters;
} else {
$raw_old_value = $old_value;
$raw_old_value = $old_value;
// Reviewers: I tried various conditions here. These are the only ones I could get working.
if ( false === $old_value && false !== $default_value ) {
$raw_old_value = _get_raw_option_value( $option, $default_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.
*
Expand All @@ -808,6 +800,9 @@ function update_option( $option, $value, $autoload = null ) {
if (
$value === $raw_old_value ||
(
// Do not check filtered option values.
false === has_filter( "pre_option_{$option}" ) &&
false === has_filter( 'pre_option' ) &&
$raw_old_value !== $default_value &&
_is_equal_database_value( $raw_old_value, $value )
)
Expand Down Expand Up @@ -2144,45 +2139,14 @@ function update_network_option( $network_id, $option, $value ) {
*/
$value = apply_filters( "pre_update_site_option_{$option}", $value, $old_value, $option, $network_id );

/** This filter is documented in wp-includes/option.php */
$default_value = apply_filters( "default_site_option_{$option}", false, $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;
}
$raw_old_value = false === $old_value && false !== $default_value ? _get_raw_option_value( $option, $default_value, $network_id ) : $old_value;

if ( ! is_multisite() ) {
/** This filter is documented in wp-includes/option.php */
Expand Down Expand Up @@ -2217,7 +2181,7 @@ function update_network_option( $network_id, $option, $value ) {
return false;
}

if ( $default_value === $raw_old_value ) {
if ( false === $old_value ) {
return add_network_option( $network_id, $option, $value );
}

Expand Down Expand Up @@ -3032,3 +2996,69 @@ function _is_equal_database_value( $old_value, $new_value ) {
*/
return maybe_serialize( $old_value ) === maybe_serialize( $new_value );
}

/**
* Attempts getting the raw value of an option from the database.
*
* For filtered options and options that do not exist in the database,
* the raw value is the default value.
*
* @since 6.4.0
* @access private
*
* @param string $option The option's name.
* @param mixed $default_value The option's default value.
* @param int|null $network_id Optional. ID of the network.
* Can be null to default to the current network ID.
* Default null.
* @return mixed The raw value.
*/
function _get_raw_option_value( $option, $default_value, $network_id = null ) {
global $wpdb;

// Do not check filtered option values.
if ( ! ( false === has_filter( "pre_option_{$option}" ) || false === has_filter( 'pre_option' ) ) ) {
return $default_value;
}

/*
* Local options take precedence, even on Multisite.
* For reviewers, see Tests_L10n_GetLocale::test_local_option_should_take_precedence_on_multisite
*/
$from_db = $wpdb->get_row(
$wpdb->prepare( "SELECT option_value FROM $wpdb->options WHERE option_name = %s LIMIT 1", $option )
);

if ( isset( $from_db->option_value ) ) {
return $from_db->option_value;
}

if ( is_multisite() ) {
// Do not check filtered option values.
if ( false !== has_filter( "pre_site_option_{$option}" ) ) {
return $default_value;
}

$network_id = (int) $network_id;

// Fallback to the current network if a network ID is not specified.
if ( ! $network_id ) {
$network_id = get_current_network_id();
}

$from_db = $wpdb->get_row(
$wpdb->prepare(
"SELECT meta_value FROM $wpdb->sitemeta WHERE meta_key = %s AND site_id = %i LIMIT 1",
$option,
$network_id
)
);

if ( isset( $from_db->meta_value ) ) {
return $from_db->meta_value;
}
}

// There is no value in the database. Return the default value.
return $default_value;
}
70 changes: 20 additions & 50 deletions tests/phpunit/tests/option/networkOption.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,33 +690,29 @@ public function data_stored_as_empty_string() {
}

/**
* Tests that a non-existent option is added even when its pre filter returns a value.
* Tests that a non-existent option is not 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() {
public function test_update_network_option_with_pre_filter_should_not_add_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 ) );
$this->assertFalse( update_network_option( null, 'foo', 0 ) );
}

/**
* Tests that an existing option is updated even when its pre filter returns the same value.
* Tests that an existing option is not 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() {
public function test_update_network_option_with_pre_filter_should_not_update_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.
Expand All @@ -725,11 +721,7 @@ public function test_update_network_option_with_pre_filter_updates_option_with_d
// 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 ) );
$this->assertFalse( update_network_option( null, 'foo', 0 ) );
}

/**
Expand Down Expand Up @@ -803,9 +795,12 @@ public function test_update_network_option_should_conditionally_apply_site_and_o
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';
if ( ! is_multisite() ) {
$this->markTestSkipped();
}

$option = 'foo';
$default_site_value = 'default-site-value';

add_filter(
"default_site_option_{$option}",
Expand All @@ -814,40 +809,15 @@ static function () use ( $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.' );
$this->assertFalse( update_network_option( null, $option, false ), 'update_network_option() should have returned false.' );

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';
$actual = $wpdb->get_row(
$wpdb->prepare(
"SELECT meta_value FROM $wpdb->sitemeta WHERE meta_key = %s LIMIT 1",
$option
)
);

$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.' );
$this->assertNull( $actual, 'The option was added to the database.' );
}
}
Loading