WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#26887 closed enhancement (duplicate)

Make wpdb::base_prefix explicitly public

Reported by: DavidAnderson Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Database Keywords: has-patch
Focuses: Cc:

Description

Currently in the WPDB class, there is no clear/explicit way for a plugin (e.g. a backup plugin that wants to backup all database tables) to know the base table prefix. (Not the current table prefix, but the base one).

The value is stored in wpdb::base_prefix, but this is not explicitly made public. Hence, a coder is likely to look for alternatives. wpdb::get_blog_prefix(0) looks like it is intended to supply this lack (the case of $blog_id == 0 is explicitly coded for in there), but actually fails in the case of pre-3.0 WPMU installs that have been upgraded (i.e. if MULTISITE is not defined).

In the proposed patch, I make explicit that wpdb::base_prefix is a public property, thus allowing coders to rely on it (and hindering future hackers from accidentally setting it as private).

Attachments (4)

tableprefix.diff (565 bytes) - added by DavidAnderson 6 years ago.
Proposed patch
tableprefix.2.diff (568 bytes) - added by DavidAnderson 6 years ago.
Proposed patch
tableprefix.3.diff (349 bytes) - added by DavidAnderson 6 years ago.
26887.diff (461 bytes) - added by DrewAPicture 6 years ago.
fixed docs

Download all attachments as: .zip

Change History (10)

@DavidAnderson
6 years ago

Proposed patch

@DavidAnderson
6 years ago

Proposed patch

#1 follow-up: @DavidAnderson
6 years ago

Please disregard the first patch. Did the trac feature to replace a patch disappear? I couldn't see it.

#2 in reply to: ↑ 1 @nacin
6 years ago

Replying to DavidAnderson:

Please disregard the first patch. Did the trac feature to replace a patch disappear? I couldn't see it.

Yeah, it's been removed. It was too often used improperly, this overwriting previous work and making it tougher to follow progress on a ticket.

Could you explain the switch from isset() to !empty()? I don't see anything about that in your bug report.

I'm not against this, but we should then use the public visibility keyword.

#3 @DavidAnderson
6 years ago

Hi,

I switched from isset() to !empty(), because I believed that explicitly listing it with var would cause isset() to always be true. This belief turns out to be false!

I see no problem with using public explicitly - I was just copying the style of the existing class.

New patch attached.

#4 @DavidAnderson
6 years ago

  • Keywords has-patch added

#5 @pento
6 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 3.9
  • Version set to 3.0

Let's do this.

@DrewAPicture
6 years ago

fixed docs

#6 @SergeyBiryukov
6 years ago

  • Keywords commit removed
  • Milestone 3.9 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #16762.

Note: See TracTickets for help on using tickets.