Skip to:
Content

BuddyPress.org

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#3549 closed defect (bug) (fixed)

Incompatibility between recent SharDB and bbPress forum setup routine

Reported by: boonebgorges's profile boonebgorges Owned by:
Milestone: 1.5 Priority: normal
Severity: normal Version:
Component: Forums Keywords:
Cc:

Description

BP's integrated bbPress replaces bbPress's default $bbdb object with its own, which is an extension of the WPDB class. http://buddypress.trac.wordpress.org/browser/trunk/bp-forums/bp-forums-bbpress-sa.php#L135

However, recent versions of SharDB (due, I think, to corresponding changes in WP load order) have changed so that the base db class is no longer called WPDB, but is instead called SharDB. So BP ends up extending the wrong class for $bbdb, and the forum component doesn't work.

wpmuguru, any ideas?

Attachments (2)

3549.diff (667 bytes) - added by wpmuguru 13 years ago.
make BPDB pluggable
db.php (21.5 KB) - added by wpmuguru 13 years ago.
Test version of db.php for SharDB

Download all attachments as: .zip

Change History (15)

#1 @boonebgorges
13 years ago

I should note that I was able to solve the problem locally by changing BP's definition of BPDB so that it extends SharDB instead of WPDB. But obviously this is not a generalized solution.

#2 @wpmuguru
13 years ago

It's not so much load order as a change in implementation. In MU, the default DB class was replaced when plugged.

In 3.0 a bit of code was added to transition to the current implementation which enables someone writing a DB class for WP to extend the default class (which is what we hope everyone will do). The advantage of extending the default class is that most bugfixes and improvements in the default class are handed on to the plugin.

HyperDB does the same as SharDB: http://plugins.trac.wordpress.org/browser/hyperdb/trunk/db.php?rev=397499#L42

If your four BBDB_ constants match the global table, bbPres should work ok.

I can't really do much from a SharDB side unless there is a way for me to plug BPDB. Possibly you could put a class_exists() check around the class definition.

#3 follow-up: @boonebgorges
13 years ago

Thanks for the explanation of the 3.0 stuff. Incidentally, I had a problem with SharDB recently, and ended up digging through and identifying that the class had to be renamed - only to look at the latest version of the plugin and see that I'd reversed engineered it for naught, since you'd already done it :)

If your four BBDB_ constants match the global table, bbPres should work ok.

Hm. I will have to experiment more, but it seems as if it does *not* work. $bbdb->get_results seems to be failing because it's pointing at the wrong database. I'll look into it some more, as it might be something on my end.

I can't really do much from a SharDB side unless there is a way for me to plug BPDB. Possibly you could put a class_exists() check around the class definition.

Yeah. The only problem with this approach is that it'd mean accounting in BP for all such plugins, or requiring that all such plugins are specifically BP-aware. Ideally, we'd be able to sniff, out of the existing $wpdb global, the name of the class of which it's an implementation, and then define BPDB as extending *that* class. I don't think such a thing is possible with PHP, is it?

#4 in reply to: ↑ 3 ; follow-up: @wpmuguru
13 years ago

Replying to boonebgorges:

Yeah. The only problem with this approach is that it'd mean accounting in BP for all such plugins, or requiring that all such plugins are specifically BP-aware. Ideally, we'd be able to sniff, out of the existing $wpdb global, the name of the class of which it's an implementation, and then define BPDB as extending *that* class. I don't think such a thing is possible with PHP, is it?

Sorry, I should have been clearer. I meant check for BPDB.

if ( ! class_exists( 'BPDB' ) ) :
class BPDB extends WPDB {

#5 in reply to: ↑ 4 @boonebgorges
13 years ago

Replying to wpmuguru:

I meant check for BPDB.

Right, which means that then plugins like SharDB and HyperDB will be responsible for providing their own BPDB classes? It seems a shame to require such a large amount of code to be duplicated just because of clashing class names. It also means that those plugin authors (like you) will have to port changes to BP's BPDB to their own plugins every time a change is made. Seems onerous.

Maybe we could be tricky here. What if we put all the important stuff in BPDB into a class called BPDB_Core (which extends WPDB), and then created *another* class BPDB which extends BPDB_Core (and does nothing else). That way, plugins like yours could extend BPDB_Core and automatically inherit all BP core modifications. A bit convoluted?

#6 @wpmuguru
13 years ago

It's 75 lines of code which isn't a huge amount.

Out of curiosity, why does BPDB have the escape_deep function? It's a duplicate of escape in wpdb? If it's used in bbPress you could just make it a wrapper for escape and eliminate 10-15 lines.

In your dev install try adding the class exists check, copy those 75 lines to db.php in SharDB and change

class BPDB extends WPDB

to

class BPDB extends SharDB

#7 @boonebgorges
13 years ago

It's 75 lines of code which isn't a huge amount.

True. My biggest concern was not so much the amount of duplication, but the fact that it has to be done at all. If something in that class changes, plugin authors like you will have to be on top of it, and backport to your own plugins. It's probably a small concern, as this code doesn't change very frequently.

Out of curiosity, why does BPDB have the escape_deep function?

Good question. A quick grep shows that bbPress uses it in a few places, and it gets it from BackPress. I'll run some tests, but it looks like there's no reason why we can't do what you've suggested.

In your dev install try adding the class exists check, copy those 75 lines to db.php in SharDB

If you, as the author of SharDB, are volunteering that building explicit BP support into your plugin is the best solution to the problem, then that's good enough for me :)

#8 @boonebgorges
13 years ago

(In [5104]) Eliminate duplicate code by making BPDB::escape_deep() a wrapper for wpdb::escape(). See #3549. Props wpmuguru

@wpmuguru
13 years ago

make BPDB pluggable

@wpmuguru
13 years ago

Test version of db.php for SharDB

#9 @wpmuguru
13 years ago

@Boone - give that a spin

#10 @boonebgorges
13 years ago

  • Milestone changed from Awaiting Review to 1.5

#11 @boonebgorges
13 years ago

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

(In [5117]) Make the BPDB class pluggable, for better support by db.php plugins like SharDB. Fixes #3549. Props wpmuguru

#12 @boonebgorges
13 years ago

The pluggable stuff is all that needs to be done on the BP end. wpmuguru, I'll be in touch with you about the SharDB changes (which look good in preliminary tests)

#13 @wpmuguru
13 years ago

For anyone looking for SharDB of BP 1.5, you can download 2.7.6 here: http://downloads.wordpress.org/plugin/shardb.zip

Note: See TracTickets for help on using tickets.