WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 3 days ago

#37181 reopened enhancement

Use metadata api in *_network_options

Reported by: spacedmonkey Owned by:
Milestone: 5.0 Priority: normal
Severity: normal Version: 4.4
Component: Networks and Sites Keywords: has-patch ms-roadmap dev-feedback has-unit-tests
Focuses: multisite Cc:

Description

The network (site) options are stored in the database as sitemeta. The table is formatted as a meta table. However the CRUD of this data in get_network_option, update_network_option, add_network_option and delete_network_option doesn't use the metadata api. Using the metadata api has many advantages, such as filters and a more consistent caching api.

Attachments (8)

37181.diff (9.9 KB) - added by spacedmonkey 17 months ago.
37181.1.diff (9.9 KB) - added by spacedmonkey 17 months ago.
37181.2.diff (14.0 KB) - added by spacedmonkey 4 months ago.
37181.3.diff (14.2 KB) - added by spacedmonkey 4 months ago.
37181.4.diff (18.1 KB) - added by spacedmonkey 4 months ago.
37181.5.diff (17.9 KB) - added by spacedmonkey 3 months ago.
37181.6.diff (23.8 KB) - added by spacedmonkey 3 months ago.
37181.7.diff (23.6 KB) - added by spacedmonkey 8 days ago.

Download all attachments as: .zip

Change History (45)

@spacedmonkey
17 months ago

#1 @swissspidy
17 months ago

  • Keywords has-patch needs-unit-tests added

#2 @swissspidy
17 months ago

Note that the default_site_option_ filter needs to be kept in order to not break BC. This includes the documentation for this hook.

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


17 months ago

This ticket was mentioned in Slack in #core by spacedmonkey. View the logs.


17 months ago

#5 @sc0ttkclark
17 months ago

This could utilize the stuff in WP 4.6 for register_meta() now too:

register_meta( 'site', 'my_site_option_name', array( … ) );

For testing, you'd also need to add 'site' to the array in the new wp_object_types filter. However, in the patch, you'd add 'site' in the wp_object_types array within wp_object_type_exists() in the wp-includes/functions.php file.

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


16 months ago

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


16 months ago

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


15 months ago

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


10 months ago

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


10 months ago

#12 @spacedmonkey
10 months ago

I have been looking at this ticket again. There seems to be existing unit tests for the *_network_option functions. I believe me and @jeremyfelt worked on those original tests. The tests seem good to me. We will need some more test, but I would like some advise on what those tests look like.

Related: #28290

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


8 months ago

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


8 months ago

#15 @flixos90
8 months ago

Briefly summarizing today's office-hours proposed goals:

  • Introduce *_network_meta() functions using the Metadata API for direct interaction with the sitemeta table.
  • Add support for meta_query in WP_Network_Query.
  • Use the new functions in the *_network_option() functions. This gives performance benefits and allows us getting rid of the alloptions mess. The *_network_option() functions will continue to maintain their current hooks for BC, while the *_network_meta() functions will be clean as other object metadata functions.
  • Encourage using the new functions and use *_network_meta() in Core going forward. The *_network_option() functions will stay around for BC though, and should not be deprecated.
  • This will also benefit a future /networks REST API endpoint.

See #37923 for related plans of introducing a blogmeta table and surrounding *_site_meta() functions.

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


8 months ago

#17 follow-up: @johnjamesjacoby
7 months ago

I'd like to make a motion to close this ticket in favor of recent activity on #25344.

I believe doing so will concentrate our efforts and be a bit simpler for everyone to pay attention to.

#18 in reply to: ↑ 17 @flixos90
7 months ago

  • Keywords close added

Replying to johnjamesjacoby:

I'd like to make a motion to close this ticket in favor of recent activity on #25344.

I believe doing so will concentrate our efforts and be a bit simpler for everyone to pay attention to.

+1

#19 @spacedmonkey
7 months ago

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

Duplicate of #25344.

#20 @swissspidy
7 months ago

  • Milestone Awaiting Review deleted

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


4 months ago

#22 @spacedmonkey
4 months ago

  • Keywords ms-roadmap added; close removed
  • Resolution duplicate deleted
  • Status changed from closed to reopened

#23 @spacedmonkey
4 months ago

  • Milestone set to Future Release

#24 @spacedmonkey
4 months ago

In 37181.2.diff are the following changes

  • wp_load_core_site_options now uses get_current_network_id to bring into line with _network_option functions.
  • serialize functions returned to _network_option functions which was stopping tests from passing.
  • delete_network_option uses delete_metadata and passes 4 param as an empty string, as per meta api.
  • meta.php and formatting.php moved to earlier in bootstrap process because network options loaded ealier in bootstrap.
  • Test added to prove that first value in meta store is honored.
  • New update_network_meta_cache param added to wp_network_query. Network options are primed in cache, like other query classes.
  • Removed test_site_notoptions test, as no longer valid.
  • Documentation. Poke @DrewAPicture for feedback.

#25 @spacedmonkey
4 months ago

  • Keywords dev-feedback has-unit-tests added; needs-unit-tests removed

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


4 months ago

#27 @spacedmonkey
4 months ago

In 37181.4.diff

  • Introduce the cache group of site_meta as a global group
  • Include site_meta in clean_network_cache

#28 @spacedmonkey
3 months ago

37181.6.diff

Fixes some styling issues with the comments and the code. It now is WP code standards. Also there has been changed to variables since original patch.

One thing to note about this change how the meta api handles caching. The meta api loads all the meta for an object into one cache key. This means, when you request one option, it then loads all options in memory / object cache. Personally, I think this is a good thing, you are effectively autoloading all options, similar to how options works in core. But it may have effects on performance in different use cases. In WP_Network_Query, the network option (Meta) cache is primed. It is reasonable to guess, that if you are query networks that you will likely want the meta as well. As Network query is used in the bootstrap, the network option will be cache will be primed much easier.

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


2 months ago

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


2 months ago

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


2 months ago

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


10 days ago

#34 @spacedmonkey
8 days ago

  • Milestone changed from Future Release to 5.0

@spacedmonkey
8 days ago

#35 @spacedmonkey
8 days ago

In the last patch, is a small performance improvement.

Unsetting the following
update_network_cache
update_network_meta_cache

Means cache key is unchanged on these query variable

#36 @spacedmonkey
3 days ago

  • Component changed from Options, Meta APIs to Networks and Sites

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


3 days ago

Note: See TracTickets for help on using tickets.