New Hooks for add / update / delete / get metadata in 3.1

[sc0ttkclark]

I suggest adding new hooks for each of these functions. See the attached patch for the changes I'm suggesting here. Please note, I'm using the {ACTION}_{METATYPE}_metadata format for the filter names since some actions are being applied with the {ACTION}_{METATYPE}_meta which would cause conflicts.

What does this allow? This allows plugins to intercept this function and return their own values and / or do their own functions for Post Types, among the other data types.

SVN Patch for /trunk/wp-includes/meta.php

I'll bless and take responsibility for this one.

L'chaim!

It's been suggested by @nacin that instead of the boolean false check, it should pass null through and check against null. I'm cool with that, as generally something that returns nothing returns null.

+1 to the ticket.

Replying to sc0ttkclark:

It's been suggested by @nacin that instead of the boolean false check, it should pass null through and check against null. I'm cool with that, as generally something that returns nothing returns null.

What value does null provide over false?

The difference being if a filter returns no value at all, null will still be returned.

The difference being that you can return false through the filter.

True that!

Updated filter to use null instead of false

Two things:

• shouldn't each filter return $check instead of true?
• maybe it would be good if each filter had a short explicative comment above it?

@scribu - Not following you on your first item, $check returns null if no filters are applied or if a filter returns nothing, if a filter returns anything else then the following line of code would run.

$check = apply_filters( "add_{$meta_type}_metadata" , null, $object_id, $meta_key, $meta_value, $unique ); 
if ( null !== $check ) 
  return true;

I'm saying that the third line should probably be return $check.

Or even return (bool) $check;

@scribu - What comments do you think should be added above the filters? Previous actions / filters don't have code-specific comments above them, is this a new best-practice?

Refreshed patch to use latest revision of trunk, relative to WP root folder, and returning (bool) $check as per @scribu's suggestion

(In [15983]) Add filters to *_metadata() functions. Props sc0ttkclark. Fixes #14766

Shouldn't the onus be on the plugin to return unserialized data?

I believe having the fail safe in place is a good idea, but for best-practice to be to return the data in the format desired by the function.

In many locations throughout core, we simply trust the plugin to return the right data, and if they don't, then the sky may fall on their head. I don't see why the plugin can't unserialize if they need to, because in many cases they may not need to.

If the standard is to do that throughout all of core, then that should be followed.

Does there need to be a new patch or can that simply just be modified by scribu or you to remove the unserialize check?

It would be nice if you could submit a follow-up patch.

Sure will do tonight.

Before I go completing the patch, anyone have anything against this:

	$check = apply_filters( "get_{$meta_type}_metadata", null, $object_id, $meta_key, $single );
	if ( null !== $check ) {
		if ( $single && is_array( $check ) )
			return $check[0];
		else
			return $check;
	}

Wait a second... @nacin - you may be wrong here - this is what's happening below my code in:

	if ( isset($meta_cache[$meta_key]) ) {
		if ( $single )
			return maybe_unserialize( $meta_cache[$meta_key][0] );
		else
			return array_map('maybe_unserialize', $meta_cache[$meta_key]);
	}

So why remove my use of maybe_unserialize?

Removed use of maybe_unserialized - it's been conveyed that this should be handled by the plugin itself

Attached revised patch - removed use of maybe_unserialize

We have to be consistent. So either:

apply maybe_serialize() to $meta_value before calling the filter in add_metadata() and update_metadata()

OR

remove maybe_unserialize() from get_metadata().

But since this is meant to be an all-or-nothing type of filter, we should go with wp-includes-meta-filters-revised.patch

[14308] caused a bug. $meta_value should still be stripslash()ed before being passed along to any filter.

Fixing delete use of $meta_value vs $_meta_value as used in other meta saving actions; Removed use of maybe_unserialize in get_metadata; Now $meta_value uses stripslashes_deep before filters are called in add / update / delete

Attached patch - Fixing delete use of $meta_value vs $_meta_value as used in other meta saving actions; Removed use of maybe_unserialize in get_metadata; Now $meta_value uses stripslashes_deep before filters are called in add / update / delete

(In [16017]) Fix inconsistencies in metadata filters. Props sc0ttkclark. See #14766