WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 8 weeks ago

#18196 closed defect (bug) (fixed)

Post Meta API Cleanup

Reported by: 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 3 years ago.
- fixed a bug in the patch
18196-v2.diff (5.7 KB) - added by jgadbois 3 years ago.
Use new * metadata_by_mid methods
18196.diff (5.8 KB) - added by ryan 3 years ago.
Retain stripslashes in update_meta()
18196.2.diff (6.5 KB) - added by ryan 3 years ago.
cache cleaning for delete_all
18196.do_all_pings.diff (2.0 KB) - added by duck_ 3 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 WraithKenny3 years ago

  • Cc Ken@… added

comment:2 jgadbois3 years ago

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

jgadbois3 years ago

  • fixed a bug in the patch

comment:3 jgadbois3 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

comment:4 ryan3 years ago

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

jgadbois3 years ago

Use new * metadata_by_mid methods

comment:5 jgadbois3 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?

ryan3 years ago

Retain stripslashes in update_meta()

comment:6 ryan3 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

ryan3 years ago

cache cleaning for delete_all

comment:7 jgadbois3 years ago

Anything else related to this you want done?

comment:8 ryan3 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

comment:9 sirzooro3 years ago

  • Cc sirzooro added
Last edited 3 years ago by sirzooro (previous) (diff)

comment:10 ryan3 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.

comment:11 duck_3 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.

duck_3 years ago

comment:12 ryan3 years ago

18196.do_all_pings.diff looks good.

comment:13 ryan3 years ago

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

comment:14 duck_3 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.

comment:15 ryan3 years ago

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

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

comment:16 mikeschinkel3 years ago

  • Cc mikeschinkel@… added

comment:17 nacin8 weeks ago

#24581 was marked as a duplicate.

Note: See TracTickets for help on using tickets.