Opened 19 months ago

Closed 19 months ago

Last modified 19 months ago

#18955 closed defect (bug) (fixed)

get_site_option caches default value when option does not exist

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

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_ 19 months ago.
18955.notoptions.2.diff (3.9 KB) - added by wpmuguru 19 months ago.
18955.notoptions.diff plus wp_load_core_site_options()
18955.opt3.diff (817 bytes) - added by duck_ 19 months ago.
18955.op3.2.diff (377 bytes) - added by duck_ 19 months ago.
18955.filter-tests.diff (2.9 KB) - added by duck_ 19 months ago.

Download all attachments as: .zip

Change History (21)

duck_19 months 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.

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

18955.notoptions.diff plus wp_load_core_site_options()

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

  • Milestone changed from Awaiting Review to 3.3
  • Cc boonebgorges@… added

See also #18971

  • Description modified (diff)

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

duck_19 months ago

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

18955.opt3.diff looks good to me.

duck_19 months ago

comment:12 follow-up: ↓ 13   ryan19 months 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   wpmuguru19 months 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_19 months 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.

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.

#19008 for site options notoptions cache.

Note: See TracTickets for help on using tickets.