Filter user meta keys

[wpmuguru]

This is the companion patch to ​http://trac.buddypress.org/attachment/ticket/2561.

I have my plugin written to segregate the BuddyPress content by WordPress network using the bp_core_get_table_prefix filter. However, there are a couple values stored in the user meta which is shared across networks.

The attached patch will give the user meta keys a prefix based on the BP table prefix rather than hardcoded bp_.

[wpmuguru]

I've attached a plugin you can use to test the patch in a multi netowrk install. The plugin needs to go in the mu-plugins folder to ensure the filter is active before BP loads.

[wpmuguru]

2952.diff needs some work. It includes the WP table prefix which we don't want. I'll see if I can whip up another patch tomorrow.

[wpmuguru]

That now filters the meta keys instead of prefixing with the WP table prefix.

[wpmuguru]

refresh of the plugin with user meta filters

[DJPaul]

I like 2952.2.diff much better as it won't potentially break plugins which look for the current names. I'm not keen on putting more stuff into the $bp global, however. Could move it into a wrapper function of sorts. Thoughts?

[boonebgorges]

wpmuguru and I chatted about this earlier. I had the same reaction about putting more stuff into $bp, but I think it's important that they be filterable (an argument against concatenating on the fly). A wrapper function could play a similar role, I guess.

[DJPaul]

Would this need to apply to all usermeta keys that BuddyPress create? There are some like 'notification_groups_admin_promotion' which aren't covered by this patch. (Aside: we need to group up some of these options in the meta tables)

[wpmuguru]

If there are other meta keys or BP based plugins might create user meta data, then we probably should look at doing a single prefix and concatenate keys as necessary.

I wanted to avoid adding filters within the user meta call because on any of the member listing the meta is retrieved per user, so potentially filter * 20.

[wpmuguru]

Replying to DJPaul:

(Aside: we need to group up some of these options in the meta tables)

I would be up for helping with that. Doing that would also make this a single filter/variable as well.

[wpmuguru]

There are more user meta rows per #2987. Since that patch is in, merging the user meta into an array will be easier.

Also, in light of the other user meta data, the user meta patch I wrote above would be much more appropriate if the data were stored in the user object instead of the $bp one. Thoughts?

[DJPaul]

John, what do you think about the approach the patch takes?

[boonebgorges]

I chatted with wpmuguru and we discussed an alternative strategy. Instead of stashing meta keys in the global, what if we had BP-specific wrapper functions for saving usermeta? That way, we could filter *all* meta keys as they get saved. So something like:

function bp_update_user_meta( $user_id, $meta_key, $meta_value ) {
  $meta_key = apply_filters( 'bp_update_user_meta_key', $meta_key );
  
  return update_user_meta( $user_id, $meta_key, $meta_value );
}

The downside is that we'll have to go through the codebase and make the necessary changes, and also that we'll have to remember to use the wrapper in future meta saves. But, on the other hand, it means that we don't have to do a cumbersome apply_filters everywhere where a piece of metadata is saved. Thoughts?

[boonebgorges]

I was thinking about this last night, and I decided that the initial approach that wpmuguru suggested is probably better.

Attached is a huge patch 2952.3.diff that implements the change. It should be totally backward compatible. It means that plugin authors should change the way that their usermeta is saved, but if they don't, it won't break anything unless multi-network is turned on.

The way things are structured in this patch, each meta key is run through its own filter bp_user_meta_keys-$key_name. It's also possible (and probably preferable) to hook a function to bp_setup_globals, with a high priority (maybe 999), and sweep through all of the meta_keys in one fell swoop:

foreach( $bp->user_meta_keys as $key_name => $key_value ) {
  // do your modifications
}

It should be noted that the naming of the keys is inconsistent. Some start with bp_, while most do not. So bp-multi-network.php should probably be a bit more generous about how the prefixing works than it currently is.

wpmuguru, it'd be great to get your feedback on this fairly soon, as the patch is likely to go stale. Thanks!

[wpmuguru]

Reading through it, it looks good. The only thing I saw that I would change is in bp-core-component.php from

apply_filters( "bp_user_meta_keys-$key_name", $key_value );

to

apply_filters( "bp_user_meta_key_$key_name", $key_value );

It's only doing one key on each pass of the filter & in WP string replaced filters & hooks are usually joined with an _ not with a -.

I'll give it a run later today.

[boonebgorges]

Cool. There are some places in WP where filter names are generated with a - in this way (maybe it makes for easier exploding?) but in any case I'm happy to change it to an _.

[wpmuguru]

minor fixes to .3.diff

[wpmuguru]

@boone - you were missing a couple commas/semi-colons which I added in my patch. I didn't change the filter name.

That worked ok in my testing.

[boonebgorges]

OK, thanks wpmuguru. (It was a big patch :) )

DJPaul or jjj, can one of you have a glance to give this approach the thumbs-up or thumbs-down?

[DJPaul]

If the goal is to filter the user meta key names, can't that be done in get_user_meta() etc? If there's a problem about not knowing what keys belong to BP, then we could just change all the keys to begin with "bp_".

[boonebgorges]

DJPaul - The problem is that WP does not have filters on the meta keys in get_user_meta() etc. The user_meta functions do nothing but call get_metadata(), update_metadata(), etc, and those functions do not have any appropriate filters either.

We could suggest adding these filters upstream in WP, if we thought the suggestion had a chance of succeeding.

[wpmuguru]

Even if the filters were given a thumbs up upstream, it will be at least 3.2 before they make it in.

[boonebgorges]

At the rate we're moving relative to the WP team, it's likely that BP 1.3 will require WP 3.2. This change would make that a hard requirement.

Even if WP filtered these keys, the part of this patch that registers BP's user_meta_keys would still be useful. Refactoring so that they are all prefixed with bp_ would be a nightmare, seems easier to check against a whitelist.

[nacin]

At a glance, I have two thoughts:

One, rather than a full user meta keys object, just do: update_user_meta( $user_id, $bp->prefix . 'some_key' ... ).

Two, take that a step further and abstract it:

$bp->update_user_meta( $user_id, 'some_key' ... ) or bp_update_user_meta()

I think the current approach would make it more difficult for plugins to add new keys, when in reality you're just trying to adjust the prefix.

I'm noticing now that some keys are not prefixed, and that really hurts chances for compatibility here.

[boonebgorges]

After extended discussion with jjj and DJPaul, I'm going with an approach similar to 2952.4.diff, with the addition of an additional bp_get_user_meta_key() function for fetching (and filtering) user meta keys.

This method promises maximum backward compatibility, and maximum flexibility for plugin authors.

I'll write up a bpdevel post to summarize the changes.

[boonebgorges]

(In [4372]) Abstracts user_meta keys so that they can be filtered. Fixes #2952

[nacin]

If you're going to have such a helper function, which is a Good Idea, then I don't see why you should also have the $bp->user_meta_keys aspect. Seems useless at this point. Just make bp_get_user_meta_key() return its value, but filtered, no?

Regardless, re-opening if only because user_meta_keys is set up as an array, but then used as an object.

[boonebgorges]

(In [4378]) Refactors user_meta key filtering to remove overhead in the bp global. Fixes #2952. Props nacin.

[boonebgorges]

(In [4380]) Additional bp_get_user_meta_key() applications in query classes, plus wpdb->prepare() cleanup. References #2952. Props nacin.