Skip to content

Update PHPDoc for patch commands #107

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions features/cache-patch.feature
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Feature: Patch command available for the object cache
$set_foo = function(){
wp_cache_set( 'my_key', ['foo' => 'bar'] );
wp_cache_set( 'other_key', ['fuz' => 'biz'] );
wp_cache_set( 'my_key_in_group', ['fuz' => 'biz'], 'my_group' );

$complex_key = (object) [
'foo' => (object) [
Expand All @@ -34,12 +35,24 @@ Feature: Patch command available for the object cache
Success: Updated cache key 'complex_key'.
"""

When I try `wp cache patch insert my_key_in_group foo bar --group=my_group`
Then STDOUT should be:
"""
Success: Updated cache key 'my_key_in_group'.
"""

When I try `wp cache patch insert unknown_key foo bar`
Then STDERR should be:
"""
Error: Cannot create key "foo" on data type boolean
"""

When I try `wp cache patch insert other_key foo bar --group=unknown_group`
Then STDERR should be:
"""
Error: Cannot create key "foo" on data type boolean
"""

When I run `wp cache patch update my_key foo test`
Then STDOUT should be:
"""
Expand All @@ -58,6 +71,12 @@ Feature: Patch command available for the object cache
Success: Updated cache key 'complex_key'.
"""

When I try `wp cache patch update my_key_in_group fuz baz --group=my_group`
Then STDOUT should be:
"""
Success: Updated cache key 'my_key_in_group'.
"""

When I try `wp cache patch update unknown_key foo test`
Then STDERR should be:
"""
Expand All @@ -70,12 +89,24 @@ Feature: Patch command available for the object cache
Error: No data exists for key "bar"
"""

When I try `wp cache patch update my_key foo baz --expiration=60`
Then STDOUT should be:
"""
Success: Updated cache key 'my_key'.
"""

When I run `wp cache patch delete my_key foo`
Then STDOUT should be:
"""
Success: Updated cache key 'my_key'.
"""

When I try `wp cache patch delete my_key_in_group fuz --group=my_group`
Then STDOUT should be:
"""
Success: Updated cache key 'my_key_in_group'.
"""

When I try `wp cache patch delete unknown_key foo`
Then STDERR should be:
"""
Expand All @@ -87,3 +118,9 @@ Feature: Patch command available for the object cache
"""
Error: No data exists for key "bar"
"""

When I try `wp cache patch delete my_key foo --group=my_group`
Then STDERR should be:
"""
Error: No data exists for key "foo"
"""
12 changes: 12 additions & 0 deletions features/cache-pluck.feature
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@ Feature: Pluck command available for the object cache
baz
"""

When I run `wp cache pluck my_key_2 foo bar --format=json`
Then STDOUT should be:
"""
"baz"
"""

When I run `wp cache pluck my_key_2 foo --format=json`
Then STDOUT should be:
"""
{"bar":"baz"}
"""

When I run `wp cache pluck my_key_3 foo --group=my_custom_group`
Then STDOUT should be:
"""
Expand Down
12 changes: 12 additions & 0 deletions features/transient-patch.feature
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,18 @@ Feature: Patch command available for the transient cache
{"foo":{"bar":"baz","fuz":"biz"}}
"""

When I run `wp transient patch insert my_key fiz bar --expiration=300`
Then STDOUT should be:
"""
Success: Updated transient 'my_key'.
"""

When I run `wp transient get my_key --format=json`
Then STDOUT should be:
"""
{"foo":"bar","fuz":"baz","fiz":"bar"}
"""

When I try `wp transient patch insert unknown_key foo bar`
Then STDERR should be:
"""
Expand Down
12 changes: 12 additions & 0 deletions features/transient-pluck.feature
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ Feature: Pluck command available for the transient cache
baz
"""

When I run `wp transient pluck my_key_2 foo bar --format=json`
Then STDOUT should be:
"""
"baz"
"""

When I run `wp transient pluck my_key_2 foo --format=json`
Then STDOUT should be:
"""
{"bar":"baz"}
"""

When I try `wp transient pluck unknown_key foo`
Then STDERR should be:
"""
Expand Down
10 changes: 5 additions & 5 deletions src/Cache_Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -499,10 +499,10 @@ function ( $key ) {
* ---
*
* [--expiration=<expiration>]
* : Define how long to keep the value, in seconds. `0` means as long as possible.
* ---
* default: 0
* ---
* : Define how long to keep the value, in seconds. `0` means as long as possible.
* ---
* default: 0
* ---
*
* [--format=<format>]
* : The serialization format for the value.
Expand All @@ -516,7 +516,7 @@ function ( $key ) {
public function patch( $args, $assoc_args ) {
list( $action, $key ) = $args;
$group = Utils\get_flag_value( $assoc_args, 'group' );
$expiration = Utils\get_flag_value( $assoc_args, 'expiration' );
$expiration = (int) Utils\get_flag_value( $assoc_args, 'expiration' );

$key_path = array_map(
function ( $key ) {
Expand Down
5 changes: 4 additions & 1 deletion src/Transient_Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,9 @@ function ( $key ) {
*
* [--expiration=<expiration>]
* : Time until expiration, in seconds.
* ---
* default: 0
* ---
*
* [--network]
* : Get the value of a network|site transient. On single site, this is
Expand All @@ -505,7 +508,7 @@ function ( $key ) {
*/
public function patch( $args, $assoc_args ) {
list( $action, $key ) = $args;
$expiration = (int) Utils\get_flag_value( $assoc_args, 'expiration', 0 );
$expiration = (int) Utils\get_flag_value( $assoc_args, 'expiration' );
Copy link
Member

Choose a reason for hiding this comment

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

I assume you remove the 0 here for consistency with the other commands, but I think this is the only one that is actually correct. For all of the expiration flags, we should:

  1. be explicit with the default value (as it will otherwise default to null)
  2. cast to the expected type (int) to ensure we don't see unexpected changes later in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1/
Yes it was for consistency, I removed the default value here and set it in the PHPDoc like what was done in the cache command.
During my testing, I saw that the default value from the PHPDoc was automatically used without having to set it in get_flag_value. I thought it was cleaner, and I apply the change here.

I can add the 0 back here and in the cache command if you think it's more explicit ?

2/
I've done that in 4bf6850


$read_func = Utils\get_flag_value( $assoc_args, 'network' ) ? 'get_site_transient' : 'get_transient';
$write_func = Utils\get_flag_value( $assoc_args, 'network' ) ? 'set_site_transient' : 'set_transient';
Expand Down