Skip to content

Commit 8f2effb

Browse files
paulbalandanpaulbalandan
authored
Merge pull request #4628 from yassinedoghri/fix/cache-deleteMatching
fix(cache): add check for redis empty results in deleteMatching
2 parents 9896c24 + b31b38c commit 8f2effb

File tree

9 files changed

+138
-79
lines changed

9 files changed

+138
-79
lines changed

‎system/Cache/Handlers/DummyHandler.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,11 @@ public function delete(string $key)
9393
*
9494
* @param string $pattern Cache items glob-style pattern
9595
*
96-
* @return boolean
96+
* @return integer The number of deleted items
9797
*/
9898
public function deleteMatching(string $pattern)
9999
{
100-
return true;
100+
return 0;
101101
}
102102

103103
//--------------------------------------------------------------------

‎system/Cache/Handlers/FileHandler.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,21 +165,21 @@ public function delete(string $key)
165165
*
166166
* @param string $pattern Cache items glob-style pattern
167167
*
168-
* @return boolean
168+
* @return integer The number of deleted items
169169
*/
170170
public function deleteMatching(string $pattern)
171171
{
172-
$success = true;
172+
$deleted = 0;
173173

174-
foreach (glob($this->path . $pattern) as $filename)
174+
foreach (glob($this->path . $pattern, GLOB_NOSORT) as $filename)
175175
{
176-
if (! is_file($filename) || ! @unlink($filename))
176+
if (is_file($filename) && @unlink($filename))
177177
{
178-
$success = false;
178+
$deleted++;
179179
}
180180
}
181181

182-
return $success;
182+
return $deleted;
183183
}
184184

185185
//--------------------------------------------------------------------

‎system/Cache/Handlers/PredisHandler.php

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,21 +190,19 @@ public function delete(string $key)
190190
*
191191
* @param string $pattern Cache items glob-style pattern
192192
*
193-
* @return boolean
193+
* @return integer The number of deleted items
194194
*/
195195
public function deleteMatching(string $pattern)
196196
{
197-
$success = true;
197+
$deleted = 0;
198+
$matchedKeys = [];
198199

199200
foreach (new Keyspace($this->redis, $pattern) as $key)
200201
{
201-
if ($this->redis->del($key) !== 1)
202-
{
203-
$success = false;
204-
}
202+
$matchedKeys[] = $key;
205203
}
206204

207-
return $success;
205+
return $this->redis->del($matchedKeys);
208206
}
209207

210208
//--------------------------------------------------------------------

‎system/Cache/Handlers/RedisHandler.php

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -228,25 +228,30 @@ public function delete(string $key)
228228
*
229229
* @param string $pattern Cache items glob-style pattern
230230
*
231-
* @return boolean
231+
* @return integer The number of deleted items
232232
*/
233233
public function deleteMatching(string $pattern)
234234
{
235-
$success = true;
235+
$matchedKeys = [];
236236
$iterator = null;
237237

238-
while ($keys = $this->redis->scan($iterator, $pattern))
238+
do
239239
{
240-
foreach ($keys as $key)
240+
// Scan for some keys
241+
$keys = $this->redis->scan($iterator, $pattern);
242+
243+
// Redis may return empty results, so protect against that
244+
if ($keys !== false)
241245
{
242-
if ($this->redis->del($key) !== 1)
246+
foreach ($keys as $key)
243247
{
244-
$success = false;
248+
$matchedKeys[] = $key;
245249
}
246250
}
247251
}
252+
while ($iterator > 0); // @phpstan-ignore-line
248253

249-
return $success;
254+
return $this->redis->del($matchedKeys);
250255
}
251256

252257
//--------------------------------------------------------------------

‎tests/system/Cache/Handlers/DummyHandlerTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public function testDelete()
4343

4444
public function testDeleteMatching()
4545
{
46-
$this->assertTrue($this->dummyHandler->deleteMatching('key*'));
46+
$this->assertSame(0, $this->dummyHandler->deleteMatching('key*'));
4747
}
4848

4949
public function testIncrement()

‎tests/system/Cache/Handlers/FileHandlerTest.php

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,13 @@ class FileHandlerTest extends CIUnitTestCase
1313
private static $key1 = 'key1';
1414
private static $key2 = 'key2';
1515
private static $key3 = 'key3';
16-
private static $key4 = 'another_key';
1716

1817
private static function getKeyArray()
1918
{
2019
return [
2120
self::$key1,
2221
self::$key2,
2322
self::$key3,
24-
self::$key4,
2523
];
2624
}
2725

@@ -135,24 +133,48 @@ public function testDelete()
135133
$this->assertFalse($this->fileHandler->delete(self::$dummy));
136134
}
137135

138-
public function testDeleteMatching()
136+
public function testDeleteMatchingPrefix()
139137
{
140-
$this->fileHandler->save(self::$key1, 'value');
141-
$this->fileHandler->save(self::$key2, 'value2');
142-
$this->fileHandler->save(self::$key3, 'value3');
143-
$this->fileHandler->save(self::$key4, 'value4');
138+
// Save 101 items to match on
139+
for ($i = 1; $i <= 101; $i++)
140+
{
141+
$this->fileHandler->save('key_' . $i, 'value' . $i);
142+
}
144143

145-
$this->assertSame('value', $this->fileHandler->get(self::$key1));
146-
$this->assertSame('value2', $this->fileHandler->get(self::$key2));
147-
$this->assertSame('value3', $this->fileHandler->get(self::$key3));
148-
$this->assertSame('value4', $this->fileHandler->get(self::$key4));
144+
// check that there are 101 items is cache store
145+
$this->assertSame(101, count($this->fileHandler->getCacheInfo()));
149146

150-
$this->assertTrue($this->fileHandler->deleteMatching('key*'));
147+
// Checking that given the prefix "key_1", deleteMatching deletes 13 keys:
148+
// (key_1, key_10, key_11, key_12, key_13, key_14, key_15, key_16, key_17, key_18, key_19, key_100, key_101)
149+
$this->assertSame(13, $this->fileHandler->deleteMatching('key_1*'));
151150

152-
$this->assertNull($this->fileHandler->get(self::$key1));
153-
$this->assertNull($this->fileHandler->get(self::$key2));
154-
$this->assertNull($this->fileHandler->get(self::$key3));
155-
$this->assertSame('value4', $this->fileHandler->get(self::$key4));
151+
// check that there remains (101 - 13) = 88 items is cache store
152+
$this->assertSame(88, count($this->fileHandler->getCacheInfo()));
153+
154+
// Clear all files
155+
$this->fileHandler->clean();
156+
}
157+
158+
public function testDeleteMatchingSuffix()
159+
{
160+
// Save 101 items to match on
161+
for ($i = 1; $i <= 101; $i++)
162+
{
163+
$this->fileHandler->save('key_' . $i, 'value' . $i);
164+
}
165+
166+
// check that there are 101 items is cache store
167+
$this->assertSame(101, count($this->fileHandler->getCacheInfo()));
168+
169+
// Checking that given the suffix "1", deleteMatching deletes 11 keys:
170+
// (key_1, key_11, key_21, key_31, key_41, key_51, key_61, key_71, key_81, key_91, key_101)
171+
$this->assertSame(11, $this->fileHandler->deleteMatching('*1'));
172+
173+
// check that there remains (101 - 13) = 88 items is cache store
174+
$this->assertSame(90, count($this->fileHandler->getCacheInfo()));
175+
176+
// Clear all files
177+
$this->fileHandler->clean();
156178
}
157179

158180
public function testIncrement()

‎tests/system/Cache/Handlers/PredisHandlerTest.php

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,13 @@ class PredisHandlerTest extends CIUnitTestCase
1212
private static $key1 = 'key1';
1313
private static $key2 = 'key2';
1414
private static $key3 = 'key3';
15-
private static $key4 = 'another_key';
1615

1716
private static function getKeyArray()
1817
{
1918
return [
2019
self::$key1,
2120
self::$key2,
2221
self::$key3,
23-
self::$key4,
2422
];
2523
}
2624

@@ -100,24 +98,42 @@ public function testDelete()
10098
$this->assertFalse($this->PredisHandler->delete(self::$dummy));
10199
}
102100

103-
public function testDeleteMatching()
101+
public function testDeleteMatchingPrefix()
104102
{
105-
$this->PredisHandler->save(self::$key1, 'value');
106-
$this->PredisHandler->save(self::$key2, 'value2');
107-
$this->PredisHandler->save(self::$key3, 'value3');
108-
$this->PredisHandler->save(self::$key4, 'value4');
103+
// Save 101 items to match on
104+
for ($i = 1; $i <= 101; $i++)
105+
{
106+
$this->PredisHandler->save('key_' . $i, 'value' . $i);
107+
}
109108

110-
$this->assertSame('value', $this->PredisHandler->get(self::$key1));
111-
$this->assertSame('value2', $this->PredisHandler->get(self::$key2));
112-
$this->assertSame('value3', $this->PredisHandler->get(self::$key3));
113-
$this->assertSame('value4', $this->PredisHandler->get(self::$key4));
109+
// check that there are 101 items is cache store
110+
$this->assertSame('101', $this->PredisHandler->getCacheInfo()['Keyspace']['db0']['keys']);
114111

115-
$this->assertTrue($this->PredisHandler->deleteMatching('key*'));
112+
// Checking that given the prefix "key_1", deleteMatching deletes 13 keys:
113+
// (key_1, key_10, key_11, key_12, key_13, key_14, key_15, key_16, key_17, key_18, key_19, key_100, key_101)
114+
$this->assertSame(13, $this->PredisHandler->deleteMatching('key_1*'));
116115

117-
$this->assertNull($this->PredisHandler->get(self::$key1));
118-
$this->assertNull($this->PredisHandler->get(self::$key2));
119-
$this->assertNull($this->PredisHandler->get(self::$key3));
120-
$this->assertSame('value4', $this->PredisHandler->get(self::$key4));
116+
// check that there remains (101 - 13) = 88 items is cache store
117+
$this->assertSame('88', $this->PredisHandler->getCacheInfo()['Keyspace']['db0']['keys']);
118+
}
119+
120+
public function testDeleteMatchingSuffix()
121+
{
122+
// Save 101 items to match on
123+
for ($i = 1; $i <= 101; $i++)
124+
{
125+
$this->PredisHandler->save('key_' . $i, 'value' . $i);
126+
}
127+
128+
// check that there are 101 items is cache store
129+
$this->assertSame('101', $this->PredisHandler->getCacheInfo()['Keyspace']['db0']['keys']);
130+
131+
// Checking that given the suffix "1", deleteMatching deletes 11 keys:
132+
// (key_1, key_11, key_21, key_31, key_41, key_51, key_61, key_71, key_81, key_91, key_101)
133+
$this->assertSame(11, $this->PredisHandler->deleteMatching('*1'));
134+
135+
// check that there remains (101 - 13) = 88 items is cache store
136+
$this->assertSame('90', $this->PredisHandler->getCacheInfo()['Keyspace']['db0']['keys']);
121137
}
122138

123139
public function testClean()

‎tests/system/Cache/Handlers/RedisHandlerTest.php

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,13 @@ class RedisHandlerTest extends CIUnitTestCase
1212
private static $key1 = 'key1';
1313
private static $key2 = 'key2';
1414
private static $key3 = 'key3';
15-
private static $key4 = 'another_key';
1615

1716
private static function getKeyArray()
1817
{
1918
return [
2019
self::$key1,
2120
self::$key2,
2221
self::$key3,
23-
self::$key4,
2422
];
2523
}
2624

@@ -100,24 +98,46 @@ public function testDelete()
10098
$this->assertFalse($this->redisHandler->delete(self::$dummy));
10199
}
102100

103-
public function testDeleteMatching()
101+
public function testDeleteMatchingPrefix()
104102
{
105-
$this->redisHandler->save(self::$key1, 'value');
106-
$this->redisHandler->save(self::$key2, 'value2');
107-
$this->redisHandler->save(self::$key3, 'value3');
108-
$this->redisHandler->save(self::$key4, 'value4');
103+
// Save 101 items to match on
104+
for ($i = 1; $i <= 101; $i++)
105+
{
106+
$this->redisHandler->save('key_' . $i, 'value' . $i);
107+
}
109108

110-
$this->assertSame('value', $this->redisHandler->get(self::$key1));
111-
$this->assertSame('value2', $this->redisHandler->get(self::$key2));
112-
$this->assertSame('value3', $this->redisHandler->get(self::$key3));
113-
$this->assertSame('value4', $this->redisHandler->get(self::$key4));
109+
// check that there are 101 items is cache store
110+
$dbInfo = explode(',', $this->redisHandler->getCacheInfo()['db0']);
111+
$this->assertSame('keys=101', $dbInfo[0]);
114112

115-
$this->assertTrue($this->redisHandler->deleteMatching('key*'));
113+
// Checking that given the prefix "key_1", deleteMatching deletes 13 keys:
114+
// (key_1, key_10, key_11, key_12, key_13, key_14, key_15, key_16, key_17, key_18, key_19, key_100, key_101)
115+
$this->assertSame(13, $this->redisHandler->deleteMatching('key_1*'));
116116

117-
$this->assertNull($this->redisHandler->get(self::$key1));
118-
$this->assertNull($this->redisHandler->get(self::$key2));
119-
$this->assertNull($this->redisHandler->get(self::$key3));
120-
$this->assertSame('value4', $this->redisHandler->get(self::$key4));
117+
// check that there remains (101 - 13) = 88 items is cache store
118+
$dbInfo = explode(',', $this->redisHandler->getCacheInfo()['db0']);
119+
$this->assertSame('keys=88', $dbInfo[0]);
120+
}
121+
122+
public function testDeleteMatchingSuffix()
123+
{
124+
// Save 101 items to match on
125+
for ($i = 1; $i <= 101; $i++)
126+
{
127+
$this->redisHandler->save('key_' . $i, 'value' . $i);
128+
}
129+
130+
// check that there are 101 items is cache store
131+
$dbInfo = explode(',', $this->redisHandler->getCacheInfo()['db0']);
132+
$this->assertSame('keys=101', $dbInfo[0]);
133+
134+
// Checking that given the suffix "1", deleteMatching deletes 11 keys:
135+
// (key_1, key_11, key_21, key_31, key_41, key_51, key_61, key_71, key_81, key_91, key_101)
136+
$this->assertSame(11, $this->redisHandler->deleteMatching('*1'));
137+
138+
// check that there remains (101 - 13) = 88 items is cache store
139+
$dbInfo = explode(',', $this->redisHandler->getCacheInfo()['db0']);
140+
$this->assertSame('keys=90', $dbInfo[0]);
121141
}
122142

123143
//FIXME: I don't like all Hash logic very much. It's wasting memory.
@@ -132,7 +152,6 @@ public function testDeleteMatching()
132152
public function testClean()
133153
{
134154
$this->redisHandler->save(self::$key1, 1);
135-
$this->redisHandler->save(self::$key2, 'value');
136155

137156
$this->assertTrue($this->redisHandler->clean());
138157
}

‎user_guide_src/source/libraries/caching.rst

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,23 +136,22 @@ Class Reference
136136

137137
$cache->delete('cache_item_id');
138138

139-
.. php:method:: deleteMatching($pattern): bool
139+
.. php:method:: deleteMatching($pattern): integer
140140
141141
:param string $pattern: glob-style pattern to match cached items keys
142-
:returns: ``true`` on success, ``false`` if one of the deletes fails
143-
:rtype: bool
142+
:returns: number of deleted items
143+
:rtype: integer
144144

145145
This method will delete multiple items from the cache store at once by
146-
matching their keys against a glob pattern.
147-
If one of the cache item deletion fails, the method will return FALSE.
146+
matching their keys against a glob-style pattern. It will return the total number of deleted items.
148147

149148
.. important:: This method is only implemented for File, Redis and Predis handlers.
150149
Due to limitations, it couldn't be implemented for Memcached and Wincache handlers.
151150

152151
Example::
153152

154-
$cache->deleteMatching('prefix_*'); // deletes all keys starting with "prefix_"
155-
$cache->deleteMatching('*_suffix'); // deletes all keys ending with "_suffix"
153+
$cache->deleteMatching('prefix_*'); // deletes all items of which keys start with "prefix_"
154+
$cache->deleteMatching('*_suffix'); // deletes all items of which keys end with "_suffix"
156155

157156
For more information on glob-style syntax, please see
158157
`https://en.wikipedia.org/wiki/Glob_(programming) <https://en.wikipedia.org/wiki/Glob_(programming)#Syntax>`_.

0 commit comments

Comments
 (0)