61467 - Filter metadata api for site ( network ) meta. #8161

spacedmonkey · 2025-01-21T23:33:38Z

Add filters for site metadata handling in multisite setups

Introduce filters for get, update, add, and delete site metadata, specific to multisite installations. These functions extend metadata API compatibility while marking their use as deprecated in favor of network options functions. Warnings are added to guide developers toward preferred alternatives.

Trac ticket: https://core.trac.wordpress.org/ticket/61467

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Introduce filters for get, update, add, and delete site metadata, specific to multisite installations. These functions extend metadata API compatibility while marking their use as deprecated in favor of network options functions. Warnings are added to guide developers toward preferred alternatives.

The $unique parameter was passed but not used in add_network_option. This change cleans up the function call for consistency and clarity. It aligns with the intended usage of add_network_option.

github-actions · 2025-01-21T23:33:54Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props spacedmonkey.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions · 2025-01-21T23:48:36Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

felixarntz

@spacedmonkey I like the overall idea behind this, routing the site metadata function calls through to the corresponding *_network_option() function. However, I think we can improve the approach a little:

I think all the warnings and messages related to deprecation are unnecessary.
We should always either use the corresponding *_network_option() function or return a specific short-circuit value, but we should never return $current here, because that would create inconsistent behavior where sometimes the regular metadata logic would still run.

felixarntz · 2025-01-22T19:29:08Z

src/wp-includes/option.php

+	if ( ! is_multisite() ) {
+		_doing_it_wrong( 'get_metadata', __( 'This function is only available in multisite installations.' ), '6.8.0' );
+		return false;
+	}


On non-multisite, get_network_option() still works and falls through to get_option(). So it would make sense to me to align with that. We could remove this condition since get_network_option() already handles it.

felixarntz · 2025-01-22T19:33:40Z

src/wp-includes/option.php

+		_doing_it_wrong( 'get_metadata', __( 'The $single parameter is not supported in this function.' ), '6.8.0' );
+		return $current;


While it is correct that ! $single does not work when using with network options, I don't think we should allow the function to pass through when $single is false. I think we have two options here:

Either return an empty array because it's not valid.

Or return an array with just the option value as the sole element.

Here is an example for the 2nd option:

Suggested change

_doing_it_wrong( 'get_metadata', __( 'The $single parameter is not supported in this function.' ), '6.8.0' );

return $current;

$option_value = get_network_option( $object_id, $meta_key, false );

if ( false !== $option_value ) {

return array( $option_value );

}

return array();

felixarntz · 2025-01-22T19:34:29Z

src/wp-includes/option.php

+		return $current;
+	}
+
+	_doing_it_wrong( 'get_metadata', __( 'This function is deprecated. Use get_network_option() instead.' ), '6.8.0' );


Per my overarching comment, I don't think all those warnings and deprecation messages are needed. We can make this work without them.

felixarntz · 2025-01-22T19:35:12Z

src/wp-includes/option.php

+	if ( ! is_multisite() ) {
+		_doing_it_wrong( 'update_metadata', __( 'This function is only available in multisite installations.' ), '6.8.0' );
+		return false;
+	}
+
+	_doing_it_wrong( 'update_metadata', __( 'This function is deprecated. Use update_network_option() instead.' ), '6.8.0' );
+
+	return update_network_option( $object_id, $meta_key, $meta_value );


See above, I think this could simply become:

Suggested change

if ( ! is_multisite() ) {

_doing_it_wrong( 'update_metadata', __( 'This function is only available in multisite installations.' ), '6.8.0' );

return false;

}

_doing_it_wrong( 'update_metadata', __( 'This function is deprecated. Use update_network_option() instead.' ), '6.8.0' );

return update_network_option( $object_id, $meta_key, $meta_value );

return update_network_option( $object_id, $meta_key, $meta_value );

felixarntz · 2025-01-22T19:37:46Z

src/wp-includes/option.php

+	if ( ! is_multisite() ) {
+		_doing_it_wrong( 'add_metadata', __( 'This function is only available in multisite installations.' ), '6.8.0' );
+		return false;
+	}


See above: We can remove this check and call add_network_option() as it works outside of Multisite too.

felixarntz · 2025-01-22T19:41:01Z

src/wp-includes/option.php

+	if ( ! $unique ) {
+		_doing_it_wrong( 'get_metadata', __( 'The $unique parameter is not supported in this function.' ), '6.8.0' );
+		return $current;
+	}


I think we can make this a bit safer: If $unique is false, this is only a problem if there is already a value for that option. Also, we shouldn't return $current as that would allow the function to proceed with its regular metadata logic, which would not be aligned with the changes this PR otherwise makes.

For example we could do this:

Suggested change

if ( ! $unique ) {

_doing_it_wrong( 'get_metadata', __( 'The $unique parameter is not supported in this function.' ), '6.8.0' );

return $current;

}

if ( ! $unique ) {

$option_value = get_network_option( $object_id, $meta_key, false );

if ( false !== $option_value ) {

return false;

}

}

felixarntz · 2025-01-22T19:41:33Z

src/wp-includes/option.php

+		return $current;
+	}
+
+	_doing_it_wrong( 'add_metadata', __( 'This function is deprecated. Use add_network_option() instead.' ), '6.8.0' );


See above, these deprecation warnings are not necessary, and they would be confusing because those functions are actually not deprecated.

felixarntz · 2025-01-22T19:44:18Z

src/wp-includes/option.php

+	if ( ! is_multisite() ) {
+		_doing_it_wrong( 'delete_metadata', __( 'This function is only available in multisite installations.' ), '6.8.0' );
+		return false;
+	}
+
+	if ( $delete_all ) {
+		_doing_it_wrong( 'delete_metadata', __( 'The $delete_all parameter is not supported in this function.' ), '6.8.0' );
+		return $current;
+	}
+
+	if ( $meta_value ) {
+		_doing_it_wrong( 'delete_metadata', __( 'The $meta_value parameter is not supported in this function.' ), '6.8.0' );
+		return $current;
+	}
+
+	_doing_it_wrong( 'delete_metadata', __( 'This function is deprecated. Use delete_network_option() instead.' ), '6.8.0' );
+
+	return delete_network_option( $object_id, $meta_key );


Based on the above feedback, this can be simplified to:

Suggested change

if ( ! is_multisite() ) {

_doing_it_wrong( 'delete_metadata', __( 'This function is only available in multisite installations.' ), '6.8.0' );

return false;

}

if ( $delete_all ) {

_doing_it_wrong( 'delete_metadata', __( 'The $delete_all parameter is not supported in this function.' ), '6.8.0' );

return $current;

}

if ( $meta_value ) {

_doing_it_wrong( 'delete_metadata', __( 'The $meta_value parameter is not supported in this function.' ), '6.8.0' );

return $current;

}

_doing_it_wrong( 'delete_metadata', __( 'This function is deprecated. Use delete_network_option() instead.' ), '6.8.0' );

return delete_network_option( $object_id, $meta_key );

if ( $delete_all ) {

return false;

}

if ( $meta_value ) {

$option_value = get_network_option( $object_id, $meta_key, false );

if ( $meta_value !== $option_value ) {

return false;

}

}

return delete_network_option( $object_id, $meta_key );

spacedmonkey added 2 commits January 21, 2025 18:30

Remove unused $unique parameter from add_metadata call.

26d62da

The $unique parameter was passed but not used in add_network_option. This change cleans up the function call for consistency and clarity. It aligns with the intended usage of add_network_option.

spacedmonkey requested review from felixarntz, JJJ and peterwilsoncc January 21, 2025 23:42

felixarntz requested changes Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

61467 - Filter metadata api for site ( network ) meta. #8161

61467 - Filter metadata api for site ( network ) meta. #8161

spacedmonkey commented Jan 21, 2025

github-actions bot commented Jan 21, 2025

github-actions bot commented Jan 21, 2025

felixarntz left a comment

felixarntz Jan 22, 2025

felixarntz Jan 22, 2025

felixarntz Jan 22, 2025

felixarntz Jan 22, 2025

felixarntz Jan 22, 2025

felixarntz Jan 22, 2025

felixarntz Jan 22, 2025

felixarntz Jan 22, 2025

		_doing_it_wrong( 'get_metadata', __( 'The $single parameter is not supported in this function.' ), '6.8.0' );
		return $current;

-		_doing_it_wrong( 'get_metadata', __( 'The $single parameter is not supported in this function.' ), '6.8.0' );
-		return $current;
+		$option_value = get_network_option( $object_id, $meta_key, false );
+		if ( false !== $option_value ) {
+			return array( $option_value );
+		}
+		return array();

61467 - Filter metadata api for site ( network ) meta. #8161

Are you sure you want to change the base?

61467 - Filter metadata api for site ( network ) meta. #8161

Conversation

spacedmonkey commented Jan 21, 2025

github-actions bot commented Jan 21, 2025

github-actions bot commented Jan 21, 2025

Test using WordPress Playground

Some things to be aware of

felixarntz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment