Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce functions to set option autoload value independently in the database #5069

Closed

Conversation

felixarntz
Copy link
Member

Trac ticket: https://core.trac.wordpress.org/ticket/58964

This PR introduces wp_set_option_autoload() and wp_set_options_autoload() functions, which allow updating the autoload value for an option, or multiple using a single DB request.

Both functions also handle updating the respective caches accordingly (alloptions vs individual option cache).


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

src/wp-includes/option.php Outdated Show resolved Hide resolved
src/wp-includes/option.php Outdated Show resolved Hide resolved
Copy link
Member

@joemcgill joemcgill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this. Thanks, @felixarntz. I have a couple questions inline, but am also wondering if we could simplify things by making wp_set_option_autoload() a wapper that passes a single value to wp_set_options_autoload() and avoid duplicating the query logic in two places?

src/wp-includes/option.php Outdated Show resolved Hide resolved
src/wp-includes/option.php Outdated Show resolved Hide resolved
@felixarntz
Copy link
Member Author

@joemcgill In 40714a4, I have updated wp_set_option_autoload() to become a wrapper function for wp_set_options_autoload(). That makes a lot of sense and reduces the amount of complex code added quite a bit.

@felixarntz
Copy link
Member Author

@Tabrisrp In 8ba3b09, I've added the call to wp_protect_special_option() as you had it in your alternative PR #5064.

@felixarntz
Copy link
Member Author

@boonebgorges @joemcgill This is now ready for another full review.

Copy link
Member

@joemcgill joemcgill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks much cleaner after your updates, thanks @felixarntz!

*/
function wp_set_options_autoload( array $options, $autoload ) {
return wp_set_option_autoload_values(
array_fill_keys( $options, $autoload )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever! ✨

Copy link
Member

@boonebgorges boonebgorges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @felixarntz for the PR. Left nonblocking feedback.

src/wp-includes/option.php Outdated Show resolved Hide resolved
src/wp-includes/option.php Outdated Show resolved Hide resolved
src/wp-includes/option.php Outdated Show resolved Hide resolved
src/wp-includes/option.php Outdated Show resolved Hide resolved
Copy link
Contributor

@costdev costdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates @felixarntz! A couple more thoughts:

src/wp-includes/option.php Show resolved Hide resolved
src/wp-includes/option.php Show resolved Hide resolved
Copy link
Contributor

@costdev costdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the responses @felixarntz! That's all from me on this one 🙂

@felixarntz
Copy link
Member Author

@felixarntz felixarntz closed this Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants