Skip to content

Commit

Permalink
Fix ::delete method to return true also if key does not exists
Browse files Browse the repository at this point in the history
  • Loading branch information
overclokk committed Apr 25, 2023
1 parent 330743c commit d967ca5
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 10 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ It is a bad decision?
I leave the decision to you :-D
I think this method is not necessary, but I decided to leave it there for the sake of completeness.

### The return value of the `Class::delete()` method

As always in WordPress there is no real standard between API (f**k), and the `Class::delete()` method is another example about this.
The `\remove_theme_mod()` is the only function that does not return anything, all the other functions return `true` if the value has been deleted or `false` if the value does not exist.
Now, to make this more similar to the PSR-16 specification, the `Class::delete()` method return `true` if the value has been deleted and if the value does not exist, the only way to return a false value is to provide an empty string as the key.

## Basic Usage

Remember that the maximum length of the key used for [transients](https://developer.wordpress.org/reference/functions/set_transient/) is <=172 characters, more characters will rise an Exception.
Expand Down
15 changes: 11 additions & 4 deletions src/Cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ public function update(string $key, $value, ?int $ttl = null): bool

public function delete(string $key): bool
{
if (empty($key)) {
return false;
}

if (!$this->get($key)) {
return true;
}

return \wp_cache_delete($key, $this->group);
}

Expand Down Expand Up @@ -112,10 +120,9 @@ public function getMultiple(iterable $keys, $default = null): iterable

public function deleteMultiple(iterable $keys): bool
{
$newValues = $this->iteratorToArray($keys);
$result = \wp_cache_delete_multiple($newValues, $this->group);
foreach ($result as $value) {
if (!$value) {
/** @var string $key */
foreach ($keys as $key) {
if (!$this->delete($key)) {
return false;
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/MultipleTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ public function deleteMultiple(iterable $keys): bool
{
/** @var string[] $keys */
foreach ($keys as $key) {
if ($this->delete($key)) {
continue;
if (!$this->delete($key)) {
return false;
}
return false;
}

return true;
Expand Down
8 changes: 8 additions & 0 deletions src/Option.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ public function update(string $key, $value): bool
*/
public function delete(string $key): bool
{
if (empty($key)) {
return false;
}

if (!$this->get($key)) {
return true;
}

return \delete_option($key);
}
}
9 changes: 8 additions & 1 deletion src/Transient.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,14 @@ public function update(string $key, $value, ?int $ttl = null): bool

public function delete(string $key): bool
{
$this->assertKeyLength($key);
if (empty($key)) {
return false;
}

if (!$this->get($key)) {
return true;
}

return \delete_transient($key);
}
}
22 changes: 20 additions & 2 deletions tests/src/CommonTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,28 @@ public function deleteMultiple(iterable $keys, iterable $expected = [])
/**
* @test
*/
public function deleteMultipleReturnFalse()
public function deleteMultipleReturnTrueWithNotExistentValue()
{
$this->assertNull($this->makeInstance()->get('key1'));
$this->assertNull($this->makeInstance()->get('key3'));
$this->assertFalse($this->makeInstance()->deleteMultiple(['key1', 'key3']), '');
$this->assertTrue($this->makeInstance()->deleteMultiple(['key1', 'key3']), '');
}

/**
* @test
* @todo In the future make this test pass
*/
public function deleteNotExistingValue()
{
$this->assertTrue($this->makeInstance()->delete('key1'), '');
}

/**
* @test
*/
public function deleteFromEmptyKeyShouldReturnFalse()
{
$this->assertFalse($this->makeInstance()->delete(''), '');
$this->assertFalse($this->makeInstance()->deleteMultiple(['']), '');
}
}
9 changes: 9 additions & 0 deletions tests/src/ModTestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,13 @@ public function deleteMultipleReturnFalse()
// Deleting a theme mod will always return true even if the mod doesn't exist
$this->assertTrue($this->makeInstance()->deleteMultiple(['key1', 'key3']), '');
}

/**
* @test
*/
public function deleteFromEmptyKeyShouldReturnFalse()
{
$this->assertTrue($this->makeInstance()->delete(''), '');
$this->assertTrue($this->makeInstance()->deleteMultiple(['']), '');
}
}
1 change: 1 addition & 0 deletions tests/src/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ protected function _after() {
$this->store = [];
$this->set_return_value = true;
$this->delete_return_value = true;
$this->ttl = 0;
// $this->prophet->checkPredictions();
}

Expand Down

0 comments on commit d967ca5

Please sign in to comment.