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

fix(cache): add check for redis empty results in deleteMatching #4628

Merged

Conversation

yassinedoghri
Copy link
Contributor

@yassinedoghri yassinedoghri commented Apr 30, 2021

Description

The deleteMatching implementation for the RedisHandler is not deleting all matching cache items as intended.
This PR is meant to fix that bug and fix the tests that were supposed to fail.

Also, the deleteMatching method now returns the number of deleted items as it is offers better feedback than true/false (mainly for testing purposes).

Thanks to @benjaminbellamy for the heads up!

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@yassinedoghri yassinedoghri marked this pull request as draft April 30, 2021 13:34
@yassinedoghri yassinedoghri marked this pull request as ready for review April 30, 2021 14:42
system/Cache/Handlers/DummyHandler.php Outdated Show resolved Hide resolved
system/Cache/Handlers/FileHandler.php Outdated Show resolved Hide resolved
system/Cache/Handlers/PredisHandler.php Outdated Show resolved Hide resolved
system/Cache/Handlers/RedisHandler.php Outdated Show resolved Hide resolved
system/Cache/Handlers/RedisHandler.php Outdated Show resolved Hide resolved
@yassinedoghri yassinedoghri marked this pull request as draft May 1, 2021 15:25
- update deleteMatching tests to check prefix and suffix patterns
against a significant pool of items:
--> tests weren't suppose to pass previously, but they did because of the small number of tested items.
- return number of deleted matched items in deleteMatching instead of success or failure
- update user_guide
- perf(FileHandler): do not sort files when using glob
- update docstrings
@yassinedoghri yassinedoghri marked this pull request as ready for review May 1, 2021 16:14
@paulbalandan paulbalandan merged commit 8f2effb into codeigniter4:develop May 1, 2021
@paulbalandan
Copy link
Member

Thank you, @yassinedoghri

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

Successfully merging this pull request may close these issues.

2 participants