Use WP's metadata API

[sbrajesh]

Hi,
I am working on some code for a friend of mine and I need to filter on the total member count.
As these values are stored in the group meta, we need to filter on groups_get_groupmeta value.

Currently groups_get_groupmeta does not allow filtering. Since BuddyPress is following the footprints of WordPress, I believe we should have this functionality in BuddyPress. WordPress metadata api does provide early filtering.

If the devs approve this, I will put a patch quickly.

[boonebgorges]

I'm OK with this. Can you please make sure that our filters work just like WP's (same syntax and location)? Also, if you could patch our other meta functions (activity, xprofile, blog), that would be great :)

[sbrajesh]

Thank You Boone.
I will be more than happy to do that :)
Will be putting the patch by tomorrow :)

[sbrajesh]

Hi,
Here is a patch attached which is modeled after wordpress meta api.
This patch introduces a meta api for BuddyPress. Any plugin can leverage the api to query for meta data.

Also, It includes BP_Meta_Query class which is a derivative of Wp_Meta_Query class and works exactly in the same manner.

This patch is against r6465 on the bp 1.6 branch. Please check and let us improve it. Is it acceptable to have this short of api ? Introducing the api does reduces the code for different component meta functions.

Please note, this patch includes updates for activity,blog,group meta using the new api but not for xprofile meta as xprofile metas have a little bit different organization strategy.

Looking forward to hear the feedbacks.

Please let me know if we should have it against the 1.7 trunk?

[sbrajesh]

Meta API patch for Bp 1.6 Trunk

[DJPaul]

In general terms, patches for new features are best against Trunk because I think it is unlikely we will add a major feature or API change into a 1.6.x point release; we've been guilty of this in the past, but we've learnt that lesson :)

Looking forward to checking out the patch once I get some time. Quick question. Are we unable to use WP_Meta_Query directly? I see the patch only implements one method.

[sbrajesh]

Hi Paul,
Thank you for taking a look and asking.
The problem with WP_Meta_Query is it uses the function _get_meta_table in the method get_sql which does not allow filtering, so we could not pass our custom table names there. Otherwise, we can use rest of its code.

The current patch introduces _bp_get_meta_table which allows filtering. If it is possible, and we can get a patch for it in wp core, we may not even need the meta data api and just use wordpress's built in meta data api. The only issue is the use of _get_meta_table in those functions which restricts us from using it.

I will be putting a patch for 1.7 trunk too. I put the patch for bp 1.6 trunk as my Friend Hans needed it for Bp 1.6 :)

[DJPaul]

Has a patch, but needs to be reviewed and refreshed for trunk. Moving to future release.

[DJPaul]

sbrajesh - interested in getting this into BP 1.8? Patch needs a refresh. :)

[r-a-y]

I like the idea behind the patch, but I don't like how we're duplicating the entire WP meta API to accomplish this because we are unable to override some variables.

sbrajesh is correct that we should pass a patch upstream to filter _get_meta_table() and to the various hardcoded variables in the *_metadata() functions like ​this.

This would prevent so much code duplication. The other issue I can see is maintaining backpat cache with our own cache keys (if that is even necessary).

I'll post a patch upstream and will update this ticket, but until then I'm pushing this back to Future Release.

[DJPaul]

@r-a-y, @sbrajesh

Was a ticket ever made for this on the WP core trac? I'm a bit unclear as to what parameters we'd need filterable (beyond the linked example on line 114), but happy to do it if someone can give me a hand.

[boonebgorges]

In 7833:

Unit tests for groupmeta functions.

Precursor for migrating to WP meta API. See #4551

[boonebgorges]

In 7834:

Updates groupmeta unit tests to work with new WP API functions as well

The WP API has some additional caching requirements, so to make the tests work
for both the old and new functions, a few additional cache-clearings are
necessary.

See #4551

[boonebgorges]

4551.patch is a proof-of-concept for groupmeta. The strategy will apply directly to activitymeta and forummeta.

Technical notes:

• I have preserved all the quirks of the existing meta functions (like returning an unstructured array of values when passing a null meta key to groups_get_groupmeta()). My first goal is to get it working exactly like the existing functions, then we can worry about improvements.
• To make WP see the table name, I put it into the $wpdb global, as DJPaul suggests above.
• WP's functions hardcode meta_id into their SQL queries. We use id for our primary columns. To get around this, I introduce a little str_replace() filter (bp_filter_metaid_column_name()) that gets added to 'query' just before we run the core function, and removed right after. This is the least elegant part of the patch, but it works well.
• The functions now have pretty complete unit test coverage. 4551.patch passes the tests.

Would like another set of eyes on this before moving on to the other components.

[johnjamesjacoby]

This is great, Boone. Really nicely done.

4551.02.patch expands on Boone's concept above, but goes further into creating an API for registering metadata tables in the BP_Component class.

Indirectly as a result of the above changes, I've introduced a BP_Component method to help handle and store global table data to match the metadata table registration process.

[johnjamesjacoby]

Also matches up metadata function parameters ($delete_all, etc...)

[johnjamesjacoby]

I'm comfortable with this getting committed into trunk.

[johnjamesjacoby]

Same as 2, but also converts Activity component

[boonebgorges]

In 7839:

Introduce meta_tables param for BP_Component

This utility method is used to set up configuration for $wpdb integration
and use of the WordPress metadata API

Also includes restructuring of the global_tables set in BP_Component.

See #4551

Props johnjamesjacoby

[boonebgorges]

In 7840:

Migrate groupmeta functions to the WP metadata API

See #4551

Props boonebgorges, johnjamesjacoby

[boonebgorges]

Thanks, johnjamesjacoby.

4551.4.patch is the bp-activity portion of your previous patch. It's broken a number of the existing unit tests, and in addition we should have complete unit test coverage of the functions before committing. I can try to get on this in the next few days, but I'm posting .4 here for time being to keep track of it (and in case anyone gets to it first).

In the meantime, certain group cache-priming needs to be cleaned up.

[boonebgorges]

In 7843:

Modify bp_update_meta_cache() for new metadata cache schema

Now that we are using the WP metadata API, our cache keys have changed, and
the logic for priming meta caches must mirror these changes.

See #4551

[boonebgorges]

In 7844:

Bust group object cache when updating groupmeta

This is important because some group metadata gets stored with the group
object (like last activity)

See #4551

[boonebgorges]

In 7852:

In bp_activity_delete_meta(), clear entire activity item cache when deleting all meta

This fix will become moot after migrating to WP metadata API, but I'm fixing
it just to get the unit tests to pass in the meantime :)

See #4551

[boonebgorges]

In 7853:

Unit tests for activitymeta functions

See #4551

[boonebgorges]

In 7854:

Migrate activity meta functions to the WP metadata API.

See #4551

Props boonebgorges, johnjamesjacoby

[boonebgorges]

In 7856:

Add unit tests for xprofile meta functions. See #4551

[boonebgorges]

In 7862:

Migrate xprofilemeta functions to the WP metadata API

The case of XProfile metadata is complex, because xprofile groups, fields, and
data all store their metadata in a single table, differentiated by object_type.
This complication requires additional filters on WP's core metadata queries.

See #4551

[boonebgorges]

In 7868:

Don't run meta_value through esc_sql() in bp_blogs_update_blogmeta()

The necessary sanitization happens in $wpdb, and the esc_sql() re-adds slashes
that were removed by stripslashes(). Fixing this bug just in time to refactor
it out :)

See #4551

[boonebgorges]

In 7869:

Add tests for blogmeta functions.

See #4551

[boonebgorges]

In 7870:

Migrate blogmeta functions to the WP metadata API

See #4551

[boonebgorges]

All of our custom meta functions have now been migrated. Hooray!

A big bonus is that we now have decent test coverage for these functions. Double hooray!

A review of metadata caching is now needed, especially for those components where it was never fully implemented in the first place (xprofile and blogs). I've opened a new ticket for those who would like to follow along: #5398

Marking as resolved.