Skip to content

Add pluck/patch commands for caches and transients #89

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

Merged
merged 27 commits into from
May 5, 2025

Conversation

petitphp
Copy link
Contributor

@petitphp petitphp commented Jul 25, 2023

Add commands for plucking and patching values from object cache and transient.

Fixes #26

@petitphp petitphp force-pushed the feature/pluck-patch-cache-transient branch from 8787b05 to f31a3dd Compare July 31, 2023 19:49
@petitphp petitphp marked this pull request as ready for review August 23, 2023 22:18
@petitphp petitphp requested a review from a team as a code owner August 23, 2023 22:18
@petitphp
Copy link
Contributor Author

petitphp commented Aug 28, 2023

I cannot reproduce the failures for the functional tests locally :

$ composer behat                                                                 
> run-behat-tests
...................................................................... 70
...................................................................... 140
...................................................................... 210
...................................................................... 280
...................................................................... 350
................

20 scénarios (20 succès)
366 étapes (366 succès)
4m20.27s (10.47Mb)

Seem's like the mu-plugins used to set up those scenarios are not loaded.

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

I cannot reproduce the failures for the functional tests locally :

Yeah, the tests pass locally for me too.

I don't know what the issue might be. @wp-cli/committers Any ideas?

@swissspidy
Copy link
Member

Unfortunately can't reproduce locally either.

Looks like $current_value is null, so the wp_cache_get() call in the patch() method is returning null for some reason..?

Adding some WP_CLI::log( var_export( wp_cache_get( 'my_key' ), true ) ); calls to within $set_foo could help debug this.

@petitphp
Copy link
Contributor Author

petitphp commented Sep 7, 2023

I manage to reproduce the failling tests locally after rebasing main in the branch, I'll look into it.

Scratch that, it was an issue with the rebase 🥲

@petitphp petitphp force-pushed the feature/pluck-patch-cache-transient branch from 4f765d0 to 6e5ecdf Compare September 7, 2023 08:06
@petitphp
Copy link
Contributor Author

petitphp commented Sep 7, 2023

@danielbachhuber I've split the tests into dedicated files and update them to use When I run if they're expected to succeed.

In 327d99a I added two debug statement to try to understand why the tests are failing in Github CI.

@petitphp petitphp force-pushed the feature/pluck-patch-cache-transient branch 10 times, most recently from 6cd56d8 to e75e7c1 Compare September 18, 2023 14:11
@petitphp
Copy link
Contributor Author

@danielbachhuber I was finally able to fix the failures for the functional tests. It was related to the use of STDIN to get the new value, which didn't properly handle the case where it was empty.

All tests are green on my fork.

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Nice work getting the tests working, @petitphp !

I left a few comments to work through.

set_transient( 'my_key_2', ['foo' => ['bar' => 'baz']] );
};

WP_CLI::add_hook( 'before_invoke:transient patch', $set_foo );
Copy link
Member

Choose a reason for hiding this comment

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

Can we use wp transient set to set these initial transient values? The transient cache is persistent in vanilla WordPress, so we shouldn't need to hook into the command invocation in this manner.

Copy link
Contributor Author

@petitphp petitphp Dec 1, 2023

Choose a reason for hiding this comment

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

Originally I was planing on using wp transient set but it is not possible to set array-like data directly like what's possible with wp option add/update (maybe an idea for a future PR to add this).

I've found another solution by using the wp eval command to execute a set_transient

Given a WP install
    And I run `wp eval "set_transient( 'my_key', ['foo' => 'bar'] );"`
    And I run `wp eval "set_transient( 'my_key_2', ['foo' => ['bar' => 'baz']] );"`

$traverser = new RecursiveDataStructureTraverser( $current_value );

try {
$traverser->$action( $key_path, $patch_value );
Copy link
Member

Choose a reason for hiding this comment

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

How does the actual transient value get saved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The transient value is updated later in the elseif condition by using the function store in the $write_func variable.

@johnbillion
Copy link
Contributor

What's left to do on this PR?

@petitphp
Copy link
Contributor Author

Hi @johnbillion, sorry for the delay.

I didn't had the time to look at Daniel comments yet. I'll try to check them this week and finish up this PR for a new review.

@petitphp
Copy link
Contributor Author

Hi @danielbachhuber,

I've refactored the feature tests to address your feedbacks, this should be good to go.

All tests workflows using MySQL are green, the ones with SQLite are failing, but doesn't seem related to this PR specifically (eg: Functional - WP latest on 8.2 with SQLite).

@swissspidy
Copy link
Member

SQLite errors are known, see #92

@petitphp
Copy link
Contributor Author

Hi, this PR should be ready for final review, the failling tests are related to SQLite errors and not to the changes in this PR.

@petitphp petitphp force-pushed the feature/pluck-patch-cache-transient branch from b9d6f5e to d8ef33a Compare April 26, 2024 08:36
petitphp and others added 25 commits August 4, 2024 18:03
Split features for patch/pluck commands into their own file and use `When I run` for
commands when they have a successful outcome.
Co-authored-by: Daniel Bachhuber <[email protected]>
Co-authored-by: Daniel Bachhuber <[email protected]>
@schlessera schlessera force-pushed the feature/pluck-patch-cache-transient branch from 44903e8 to 9eca4d9 Compare August 4, 2024 16:03
Copy link

codecov bot commented May 5, 2025

Codecov Report

Attention: Patch coverage is 84.87395% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/Transient_Command.php 83.33% 10 Missing ⚠️
src/Cache_Command.php 86.44% 8 Missing ⚠️

📢 Thoughts on this report? Let us know!

@schlessera schlessera merged commit 1d789af into wp-cli:main May 5, 2025
42 of 43 checks passed
@schlessera
Copy link
Member

Thanks for all the work on this, @petitphp !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

patch and pluck for caches and transients
5 participants