Skip to content

Conversation

@sasule
Copy link
Contributor

@sasule sasule commented Nov 10, 2025

$this->journal->clean(...) can produce empty array which causes an error when calling Redis del command.

Summary by CodeRabbit

  • Bug Fixes
    • Improved cache cleanup efficiency by preventing unnecessary delete operations when no items are present.

`$this->journal->clean(...)` can produce empty array which causes an error when calling Redis del command.
@coderabbitai
Copy link

coderabbitai bot commented Nov 10, 2025

Walkthrough

In RedisStorage::clean, the post-journal deletion path now validates that the keys result is not null and not an empty array before calling del. Previously, it only checked for non-null, allowing unnecessary delete operations when the journal returns an empty key set.

Changes

Cohort / File(s) Summary
Redis Storage Optimization
src/Caching/RedisStorage.php
Added check to prevent del() call on empty key set in clean method; now validates both non-null and non-empty array conditions before deletion

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Simple conditional enhancement in a single method
  • Adds one additional validation check to existing logic
  • No API changes or behavioral side effects

Poem

🐰 A rabbit checks twice before the sweep,
Why delete when the cupboard's bare and deep?
An empty array, a prudent skip,
Efficiency wins with each tiny tip!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a null/empty array check before the del command to prevent parameter errors.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a19d7d0 and 511d550.

📒 Files selected for processing (1)
  • src/Caching/RedisStorage.php (1 hunks)
🔇 Additional comments (1)
src/Caching/RedisStorage.php (1)

202-204: Good fix to prevent calling del() with an empty array.

The additional check for an empty array prevents a Redis error. When $this->journal->clean($conditions) returns an empty array, calling del([]) would trigger a "wrong number of arguments" error from Redis. This guard condition correctly avoids that error.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@f3l1x f3l1x merged commit f8e2f32 into contributte:master Nov 10, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants