Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 11 years ago

#18196 closed defect (bug) (fixed)

Post Meta API Cleanup

Reported by: ryan's profile ryan Owned by:
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.2.1
Component: General Keywords: has-patch
Focuses: Cc:

Description

delete_meta() , get_post_meta_by_id(), has_meta(), update_meta(), delete_post_meta_by_key() should be wrappers around the metadata API. The metadata functions may need to emit *_postmeta actions for back compat.

Attachments (5)

18196.patch (4.7 KB) - added by jgadbois 13 years ago.
- fixed a bug in the patch
18196-v2.diff (5.7 KB) - added by jgadbois 13 years ago.
Use new * metadata_by_mid methods
18196.diff (5.8 KB) - added by ryan 13 years ago.
Retain stripslashes in update_meta()
18196.2.diff (6.5 KB) - added by ryan 13 years ago.
cache cleaning for delete_all
18196.do_all_pings.diff (2.0 KB) - added by duck_ 13 years ago.

Download all attachments as: .zip

Change History (22)

#1 @WraithKenny
13 years ago

  • Cc Ken@… added

#2 @jgadbois
13 years ago

Just took a crack at it. Happy to change if you see any problems.

@jgadbois
13 years ago

  • fixed a bug in the patch

#3 @jgadbois
13 years ago

  • Cc jgadbois added
  • Keywords has-patch added

This patch doesn't change has_meta as the get_metadata function doesn't return all the information that the metabox building code expects

#4 @ryan
13 years ago

Looking good. Now that #18195 is in this can use update_metadata_by_mid() and delete_metadata_by_mid().

@jgadbois
13 years ago

Use new * metadata_by_mid methods

#5 @jgadbois
13 years ago

Calling the new functions now. Also added the legacy *_postmeta actions to them.

Is there a plan to add way in the meta API to get a lot of meta with the ids? With that I could move the logic out of has_meta() and make it a wrapper. Alternatively, the query in there could just be ported to the meta API?

@ryan
13 years ago

Retain stripslashes in update_meta()

#6 @ryan
13 years ago

WPTestPostMeta and WPTestIncludesMeta unit tests pass except for test_delete_post_meta_by_key(). delete_post_meta_by_key() is failing. This is due to a bug in update_metadata() where the cache is not cleared for all posts when doing $delete_all. It needs to fetch all object IDs and do a wp_cache_delete() for each similar to what delete_post_meta_by_key() did.

http://unit-tests.trac.wordpress.org/browser/wp-testcase/test_includes_post.php
http://unit-tests.trac.wordpress.org/browser/wp-testcase/test_includes_meta.php

@ryan
13 years ago

cache cleaning for delete_all

#7 @jgadbois
13 years ago

Anything else related to this you want done?

#8 @ryan
13 years ago

In [18500]:

Turn delete_meta() , get_post_meta_by_id(), update_meta(), delete_post_meta_by_key() into wrappers around the metadata API. Add back compat *_postmeta actions to metadata API. Props jgadbois. see #18196

#9 @sirzooro
13 years ago

  • Cc sirzooro added

Please provide update_postmeta, updated_postmeta, delete_postmeta and deleted_postmeta filters for other metadata types too. Of course include proper metadata name in filter name instead of 'post' (e.g. update_usermeta, and so on). This will allow to create new types of plugins.

Version 0, edited 13 years ago by sirzooro (next)

#10 @ryan
13 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Let's leave has_meta() as is for now. If we come across more situations where we need all mids for an object, we can either extend get_metadata() or introduce a new metadata function.

#11 @duck_
13 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

There are some more places where we can make better use of the metadata API.

Patched:

  • do_all_pings() can use delete_metadata_by_mid() and we don't need to perform the second query to retrieve the meta_id since we already get it.

Other possibilities which didn't work out straight away:

  • do_enclose(): unfortunately the code could produce an array of meta IDs to delete... but I'm mentioning it because in it's current state this doesn't work anyway as preparing the implode will produce something like IN('1,2,3')

The metadata addition also uses a direct query, maybe this could be swapped for add_post_meta or add_metadata (though this would require back compat _postmeta actions)

  • wp_delete_post() and wp_delete_attachment(): both delete an array of meta IDs.
  • _publish_post_hook(): adding post meta with a query, see above for note about switching to add_post_meta or add_metadata

Splitting some of the above out into separate tickets for discussion, particularly do_enclose() which really needs a clean up, might be best.

#12 @ryan
13 years ago

18196.do_all_pings.diff looks good.

#13 @ryan
13 years ago

#14493 has a pretty big do_enclose() cleanup patch. Maybe that could be base of a fresh patch.

#14 @duck_
13 years ago

In [18855]:

Use metadata API in do_all_pings() to delete post meta (cleaner, plays better with cache). Remove redundant query to retrieve meta_id. See #18196.

#15 @ryan
13 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Let's resolve this ticket and spawn new tickets for anything further.

#16 @mikeschinkel
13 years ago

  • Cc mikeschinkel@… added

#17 @nacin
11 years ago

#24581 was marked as a duplicate.

Note: See TracTickets for help on using tickets.