Make WordPress Core

Opened 12 years ago

Last modified 3 months ago

#21432 assigned defect (bug)

Deprecate *_blog_option()

Reported by: ryan's profile ryan Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4.1
Component: Options, Meta APIs Keywords: needs-patch dev-feedback
Focuses: Cc:

Description (last modified by ryan)

Deprecate get_blog_option(), add_blog_option(), update_blog_option(), and delete_blog_option(). The regular *_option() functions wrapped with switch_to_blog() and restore_current_blog() should be used instead.

Previous discussion:

http://core.trac.wordpress.org/ticket/21270#comment:11

Attachments (4)

21432.diff (12.1 KB) - added by ryan 12 years ago.
Incomplete and untested
21432.2.diff (17.7 KB) - added by ryan 12 years ago.
21432.3.diff (19.3 KB) - added by ryan 12 years ago.
21432.4.diff (1.1 KB) - added by ryan 12 years ago.

Download all attachments as: .zip

Change History (24)

#1 @ryan
12 years ago

  • Description modified (diff)

#2 @ryan
12 years ago

  • Description modified (diff)

@ryan
12 years ago

Incomplete and untested

@ryan
12 years ago

@ryan
12 years ago

#3 @ryan
12 years ago

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

In [21414]:

Deprecate get_blog_option(), add_blog_option(), update_blog_option(), and delete_blog_option().

Use the regular option functions wrapped in switch_to_blog() and restore_current_blog() instead.

Group multiple operations within a single switch where possible.

fixes #21432

#4 @johnbillion
12 years ago

Might be an idea to pass the phrase "the regular option functions wrapped in switch_to_blog() and restore_current_blog()" to the $replacement parameter of _deprecated_function().

#5 @dd32
12 years ago

While this change makes sense to me, I don't see any need to stop using *_blog_option and/or deprecating them.

As they stand, these functions perform a very specific task, Grab another site's option without extra code required by the calling functions.

Although it may be more efficient to use switch_to_blog() when multiple options are being requested, it doesn't seem like enough of a reason to deprecate the functions, especially when it seemingly doesn't provide anything extra to the calling function, let alone changing the semantics of the actual operation ("Get blog xyz's option blah" vs "Switch to the other blog, grab the option, then come home again") - that's low-level stuff that the calling function shouldn't care about:

foreach ( $blogs as $blog_id )
   $sites[ $blog_id ] = get_blog_option( $blog_id, 'some_option' );

vs

foreach ( $blogs as $blog_id ) {
   switch_to_blog( $blog_id );
   $sites[ $blog_id ] = get_option( 'some_option' );
   restore_current_blog();
}

#6 @ryan
12 years ago

In [21480]:

Undeprecate *_blog_option() by popular demand. Put them back in ms-blogs.php since direct inclusion of ms-blogs.php/ms-functions.php is unforntunately common.

see #21432

#7 @ryan
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
PHP Fatal error:  Call to undefined function get_current_blog_id() in /.../wp-includes/ms-blogs.php on line 340

Looks like the blog option functions are being called before general-template.php is loaded. Alternatives are to go back to using the blog_id global or perhaps moving get_current_blog_id() to load.php.

@ryan
12 years ago

#8 @scribu
12 years ago

  • Cc scribu added

#9 @ryan
12 years ago

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

In [21484]:

Move get_current_blog_id() to load.php so it is available during multisite bootstrap. fixes #21432

#10 @nacin
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

ryan and I would still lean toward deprecating _blog_option(). We'd just have to leave them in ms-blogs.php, which is fine with me.

Deprecating them emphasizes that they are not more performant than switch_to_blog() (which they were, hypothetically, under earlier versions, albeit with caching issues), and that switch_to_blog() should be used if you have more than one operation.

I'm willing to suggest a deprecation of current_user_can_for_blog() as well, for the same reasons.

#11 @nacin
12 years ago

  • Keywords 2nd-opinion added

#12 @scribu
12 years ago

I, for one, agree with the deprecation; having two ways of doing essentially the same thing is confusing.

#13 @nacin
12 years ago

We still need to figure out whether options filters should be skipped (or alternative ones should run) when we are switched. I guess we'll find out soon what kind of breakage we might be looking at.

We should also re-deprecate _blog_option(). See also #21459 where I mention current_user_can_for_blog() should go too.

Last edited 12 years ago by nacin (previous) (diff)

#14 @ryan
12 years ago

  • Milestone changed from 3.5 to Future Release

#15 @nacin
11 years ago

  • Component changed from General to Options and Meta

#16 @ryan
10 years ago

  • Owner ryan deleted
  • Status changed from reopened to assigned

#17 @chriscct7
9 years ago

  • Keywords needs-patch added; 2nd-opinion removed

#18 @chriscct7
9 years ago

  • Keywords dev-feedback added
Last edited 9 years ago by chriscct7 (previous) (diff)

This ticket was mentioned in Slack in #core-multisite by desrosj. View the logs.


7 years ago

#20 @desrosj
3 months ago

@nacin are you able to see this one over the finish line?

Note: See TracTickets for help on using tickets.