Opened 14 years ago
Closed 14 years ago
#14766 closed task (blessed) (fixed)
New Hooks for add / update / delete / get metadata in 3.1
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.1 | Priority: | normal |
Severity: | normal | Version: | 3.0.1 |
Component: | Plugins | Keywords: | has-patch |
Focuses: | Cc: |
Description
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.
Attachments (5)
Change History (34)
#2
@
14 years ago
- Milestone changed from Awaiting Review to 3.1
- Owner set to markjaquith
- Status changed from new to assigned
- Type changed from enhancement to task (blessed)
- Version set to 3.0.1
I'll bless and take responsibility for this one.
#4
follow-up:
↓ 5
@
14 years ago
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.
#5
in reply to:
↑ 4
@
14 years ago
+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
?
#6
@
14 years ago
The difference being if a filter returns no value at all, null will still be returned.
#9
@
14 years ago
- Keywords has-patch added; metadata removed
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?
#10
@
14 years ago
@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.
#11
@
14 years ago
$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
.
#13
@
14 years ago
@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?
@
14 years ago
Refreshed patch to use latest revision of trunk, relative to WP root folder, and returning (bool) $check as per @scribu's suggestion
#16
@
14 years ago
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.
#17
@
14 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
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.
#18
@
14 years ago
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?
#21
@
14 years ago
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; }
#22
@
14 years ago
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?
@
14 years ago
Removed use of maybe_unserialized - it's been conveyed that this should be handled by the plugin itself
#24
@
14 years ago
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().
#25
@
14 years ago
But since this is meant to be an all-or-nothing type of filter, we should go with wp-includes-meta-filters-revised.patch
#26
@
14 years ago
[14308] caused a bug. $meta_value should still be stripslash()ed before being passed along to any filter.
@
14 years ago
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
#27
@
14 years ago
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
SVN Patch for /trunk/wp-includes/meta.php