WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#18955 closed defect (bug) (fixed)

get_site_option caches default value when option does not exist

Reported by: wpmuguru Owned by: duck_
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.2.1
Component: Multisite Keywords: has-patch
Focuses: Cc:

Description (last modified by wpmuguru)

If get_site_option is passed any default value on a non-existent site option then a subsequent call to update_site_option will not add the site option. Because the site option does not exist, it does not get updated either. To reproduce

  • in the sitemeta table edit the active_sitewide_plugins meta_key to be active_sitewide_plugins_x
  • go to the network plugins screen and network activate any plugin

You get the message that the plugin was activated, but it doesn't because the site option doesn't get saved.

Options for fixing

  1. change the get_site_option in update_site_options to not use cache
  2. add a notsiteoption cache and cache the non-existence of the site option
  3. don't cache the default value in get_site_option

Option 1 is quick & easy
Option 2 is more work but is the way the blog options are handled and it would make things more consistent
Option 3 is probably not the preferable solution

Attachments (5)

18955.notoptions.diff (2.8 KB) - added by duck_ 3 years ago.
18955.notoptions.2.diff (3.9 KB) - added by wpmuguru 3 years ago.
18955.notoptions.diff plus wp_load_core_site_options()
18955.opt3.diff (817 bytes) - added by duck_ 2 years ago.
18955.op3.2.diff (377 bytes) - added by duck_ 2 years ago.
18955.filter-tests.diff (2.9 KB) - added by duck_ 2 years ago.

Download all attachments as: .zip

Change History (21)

duck_3 years ago

comment:1 duck_3 years ago

  • Description modified (diff)
  • Keywords has-patch added

As noted by your steps this bug requires code to ignore the site_option API to be triggered (so the DB and cache aren't in sync) and shouldn't happen in core.

Attached 18955.notoptions.diff example patch for adding a notoptions cache for site-options. I think this is the most sensible option. I'd say that 1 was the least preferable.

comment:2 wpmuguru3 years ago

18955.notoptions.diff worked fine for me. It's also the best solution to this, imo.

wpmuguru3 years ago

18955.notoptions.diff plus wp_load_core_site_options()

comment:3 wpmuguru3 years ago

We should also cache the non existence of site options in wp_load_core_site_options().

comment:4 SergeyBiryukov2 years ago

  • Milestone changed from Awaiting Review to 3.3

comment:5 boonebgorges2 years ago

  • Cc boonebgorges@… added

See also #18971

comment:7 wpmuguru2 years ago

  • Description modified (diff)

comment:9 ryan2 years ago

Hey, I like option 3. :-) Why cache was is not in the DB? It also happens to be a small, beta-friendly patch.

duck_2 years ago

comment:10 ryan2 years ago

I'm not against not option caching, but for 3.3 I like option 3.

comment:11 ryan2 years ago

18955.opt3.diff looks good to me.

duck_2 years ago

comment:12 follow-up: ryan2 years ago

As discussed in IRC, 18955.op3.2.diff circumvents the filter at the end of the function. get_option() already behaves like this. We might make get_site_option() behave in the same fashion.

comment:13 in reply to: ↑ 12 wpmuguru2 years ago

Replying to ryan:

As discussed in IRC, 18955.op3.2.diff circumvents the filter at the end of the function. get_option() already behaves like this. We might make get_site_option() behave in the same fashion.

That looks fine to me although I would like to leave caching non-existence on the table for 3.4.

duck_2 years ago

comment:14 duck_2 years ago

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

In [19012]:

Don't cache default value in get_site_option() for non-existent options. Fixes #18955.

comment:15 duck_2 years ago

18955.op3.2.diff would result in different behaviour for get_site_option() on singlesite vs multisite. When calling get_site_option( $option, 'default' ); for a non-existent option $option site_option_$option would run on singlesite, but not on multisite.

Therefore gone with 18955.opt3.diff per IRC.

comment:16 duck_2 years ago

#19008 for site options notoptions cache.

Note: See TracTickets for help on using tickets.