Add Redis queue storage driver by SamJUK · Pull Request #11 · SamJUK/m2-module-cache-debounce

SamJUK · 2026-07-10T15:54:20Z

Summary

Stacked on Abstract queue storage behind an interface, fix add-during-purge race #10 — adds a Redis implementation of QueueStorageInterface (SADD/atomic RENAME/SMEMBERS/DEL), giving native dedupe and the same atomic claim/drain guarantee as the DB driver, without touching MySQL.
Connection resolves from a dedicated cache_debounce.redis env.php block, falling back to the existing page_cache Redis backend if absent (with a loud warning about that instance's eviction policy potentially dropping pending tags silently).
Driver selection is a Provider decorator bound once in di.xml, switched via a scoped config value (storage_driver) that has no system.xml field — set only via config:set, never Admin-visible, no per-driver di.xml override module needed.
CI: runs a Redis container alongside the Magento test container so the new integration tests have something to connect to.

Test plan

vendor/bin/phpunit Test/Unit passes (verified against a real Magento install's classes, including the Credis mock-destructor edge case)
vendor/bin/phpstan analyse clean at the repo's configured level
dev/tests/integration against the CI Redis service
Manually confirm config:set samjuk_cache_debounce/general/storage_driver redis switches the driver and the field doesn't appear in Stores > Configuration

Copilot

Pull request overview

Adds a Redis-backed queue storage driver to the Cache Debounce module, allowing pending purge-tag dedupe and atomic claim/drain without relying on MySQL, and introduces a configurable driver-selection pool.

Changes:

Introduces Redis + Redis\ConnectionResolver implementing QueueStorageInterface using Redis sets + atomic RENAME.
Adds Pool as the QueueStorageInterface preference, selecting db vs redis via samjuk_cache_debounce_advanced/general/storage_driver.
Adds unit + integration tests and updates admin/system config + ACL + README to support/configure the new driver.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
Test/Unit/Model/Storage/RedisTest.php	Unit tests for Redis storage behavior (add/claim/tags/clear).
Test/Unit/Model/Storage/Redis/ConnectionResolverTest.php	Unit tests for dedicated vs fallback Redis connection resolution.
Test/Unit/Model/Storage/PoolTest.php	Unit tests for driver selection/caching behavior in the pool.
Test/Unit/Model/Config/Source/StorageDriverTest.php	Verifies storage driver option source exposes expected values.
Test/Integration/Model/Storage/RedisTest.php	Integration coverage for Redis round-trips and race scenarios.
Test/Integration/Model/Storage/Redis/ConnectionResolverTest.php	Integration coverage for dedicated/fallback Redis connection resolution.
README.md	Documents the storage driver config path and Redis env.php configuration.
Model/Storage/Redis/ConnectionResolver.php	Resolves Redis connection details from env.php with fallback to page_cache.
Model/Storage/Redis.php	Redis queue driver using SADD/RENAME/SMEMBERS/DEL.
Model/Storage/Pool.php	Driver selector implementing the storage interface and caching resolution.
Model/Config/Source/StorageDriver.php	Provides system.xml option source for driver selection.
etc/di.xml	Switches interface preference to Pool and registers driver map.
etc/config.xml	Adds default config value for advanced storage_driver.
etc/adminhtml/system.xml	Adds Advanced config section + storage driver select field.
etc/adminhtml/acl.xml	Adds ACL resource for the advanced storage driver config section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

SamJUK · 2026-07-12T21:13:24Z

+        $this->client = $this->getMockBuilder(\Credis_Client::class)
+            ->disableOriginalConstructor()
+            ->addMethods(['sAdd', 'rename', 'sMembers', 'del'])
+            ->getMock();


Claude: Not a real issue. Credis_Client dispatches sAdd/rename/sMembers/del via __call magic rather than declaring them as real methods, so addMethods() doesn't collide. CI is green across php74–php84, which confirms the mock builds fine. Fluff — skip.

SamJUK · 2026-07-12T21:13:25Z

+        $config = $this->deploymentConfig->get(self::CONFIG_PATH_DEDICATED) ?: $this->resolveFallback();
+
+        if (!$config) {
+            throw new LocalizedException(__(
+                'SamJUK_CacheDebounce: no Redis connection could be resolved. Configure the '
+                . '"cache_debounce.redis" block in app/etc/env.php, or configure a Redis backend '
+                . 'for the "page_cache" cache frontend.'
+            ));
+        }
+
+        $this->client = new \Credis_Client(
+            $config['host'],
+            (int)$config['port'],
+            null,
+            '',
+            (int)$config['database'],
+            $config['password'] ?: null
+        );


Claude: Confirmed, real gap. The fallback branch already defends missing keys with ?? defaults but the dedicated env.php branch doesn't — a config block missing e.g. password or database hits an undefined-index warning here. Cheap fix: mirror resolveFallback()'s null-coalescing pattern for the dedicated path.

SamJUK · 2026-07-12T21:13:27Z

+    protected function setUp(): void
+    {
+        $reader = ObjectManager::getInstance()->get(DeploymentConfig\Reader::class);
+        $deploymentConfig = new DeploymentConfig($reader, ['cache_debounce' => ['redis' => self::CONNECTION_CONFIG]]);
+
+        $this->storage = new Redis(new ConnectionResolver($deploymentConfig));
+
+        // A dedicated raw client, independent of the class under test, so
+        // cleanup can't be broken by the same bug it's meant to guard against.
+        $this->rawClient = new \Credis_Client(
+            self::CONNECTION_CONFIG['host'],
+            (int)self::CONNECTION_CONFIG['port'],
+            null,
+            '',
+            (int)self::CONNECTION_CONFIG['database']
+        );
+        $this->rawClient->flushDb();
+    }


Claude: Confirmed, real risk, partially mitigated already — this uses Redis DB index 2 rather than 0, so it's not wiping the default DB. Still worth an explicit opt-in env var gate in case a dev's local Redis uses index 2 for something else.

SamJUK · 2026-07-12T21:13:28Z

+    protected function setUp(): void
+    {
+        $this->reader = ObjectManager::getInstance()->get(DeploymentConfig\Reader::class);
+
+        (new \Credis_Client('127.0.0.1', 6379, null, '', self::DEDICATED_DATABASE))->flushDb();
+        (new \Credis_Client('127.0.0.1', 6379, null, '', self::FALLBACK_DATABASE))->flushDb();
+    }


Claude: Same as the sibling RedisTest comment — real risk, same fix (opt-in env var gate before allowing destructive flushDb calls in this test).

SamJUK · 2026-07-12T21:13:57Z

Claude: Overall — good driver implementation, three things worth resolving before merge: (1) ConnectionResolver needs null-coalescing on the dedicated env.php config keys, same as the fallback branch already has; (2) the storage_driver Admin UI visibility contradicts the README's CLI-only claim — default Administrator role has full ACL access so it will show up, needs a decision either way; (3) both Redis integration test suites do destructive flushDb() with no opt-in gate — low risk since they use non-default DB indices, but worth gating explicitly. The MockBuilder addMethods comment is fluff, CI already proves it's fine.

sonarqubecloud · 2026-07-13T00:16:06Z

Quality Gate failed

Failed conditions
5.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Redis implementation of QueueStorageInterface (SADD/RENAME/SMEMBERS/DEL, SUNIONSTORE for release), resolved via a Pool that picks db vs redis from samjuk_cache_debounce_advanced/general/storage_driver. That field is CLI-only: hidden from Admin UI (showInDefault/Website/Store all "0") and gated behind its own ACL resource, since switching storage drivers is a deploy-time infra decision, not a store setting. ConnectionResolver reads a dedicated app/etc/env.php block first, falling back to whatever Redis backend the page_cache frontend already uses so it works zero-config once switched on. Missing keys in either config fall back to sane defaults instead of hitting an undefined-index notice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

SamJUK force-pushed the feat/redis-queue-storage-driver branch from b25b2a1 to f116b1b Compare July 10, 2026 18:35

SamJUK force-pushed the feat/queue-storage-abstraction branch from 527287c to 9d4411a Compare July 10, 2026 18:39

SamJUK force-pushed the feat/redis-queue-storage-driver branch from f116b1b to ebc2a9b Compare July 10, 2026 18:46

SamJUK mentioned this pull request Jul 10, 2026

Add staggered/batched purge release (spec 003) #12

Open

4 tasks

SamJUK force-pushed the feat/queue-storage-abstraction branch from 9d4411a to 43348f3 Compare July 10, 2026 21:32

SamJUK requested a review from Copilot July 12, 2026 16:32

Copilot started reviewing on behalf of SamJUK July 12, 2026 16:33 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

SamJUK force-pushed the feat/redis-queue-storage-driver branch 3 times, most recently from 30b71be to ef94f80 Compare July 12, 2026 22:59

SamJUK force-pushed the feat/queue-storage-abstraction branch from f47df6d to e8872c0 Compare July 12, 2026 23:19

SamJUK force-pushed the feat/redis-queue-storage-driver branch from 523d0a8 to 01b2871 Compare July 12, 2026 23:29

SamJUK force-pushed the feat/queue-storage-abstraction branch from e8872c0 to 9fdf93d Compare July 12, 2026 23:35

SamJUK force-pushed the feat/redis-queue-storage-driver branch from 01b2871 to 35709d9 Compare July 12, 2026 23:37

SamJUK force-pushed the feat/queue-storage-abstraction branch from 9fdf93d to 7c1f22a Compare July 12, 2026 23:42

SamJUK force-pushed the feat/redis-queue-storage-driver branch from 35709d9 to d871903 Compare July 12, 2026 23:42

SamJUK force-pushed the feat/queue-storage-abstraction branch from 7c1f22a to b932ccf Compare July 13, 2026 00:05

SamJUK force-pushed the feat/redis-queue-storage-driver branch 2 times, most recently from 9255ab1 to 8ee2c3a Compare July 13, 2026 00:15

SamJUK force-pushed the feat/queue-storage-abstraction branch from b932ccf to ad5a3df Compare July 13, 2026 08:34

SamJUK force-pushed the feat/redis-queue-storage-driver branch from 8ee2c3a to 60d514d Compare July 13, 2026 08:34

SamJUK force-pushed the feat/queue-storage-abstraction branch from ad5a3df to 8d9da1f Compare July 13, 2026 08:59

SamJUK force-pushed the feat/redis-queue-storage-driver branch from 60d514d to 368869f Compare July 13, 2026 09:00

SamJUK merged commit 78a3771 into feat/queue-storage-abstraction Jul 13, 2026
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Redis queue storage driver#11

Add Redis queue storage driver#11
SamJUK merged 1 commit into
feat/queue-storage-abstractionfrom
feat/redis-queue-storage-driver

SamJUK commented Jul 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

SamJUK Jul 12, 2026

Uh oh!

SamJUK Jul 12, 2026

Uh oh!

Uh oh!

SamJUK Jul 12, 2026

Uh oh!

SamJUK Jul 12, 2026

Uh oh!

SamJUK commented Jul 12, 2026

Uh oh!

sonarqubecloud Bot commented Jul 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SamJUK commented Jul 10, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

SamJUK Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

SamJUK Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SamJUK Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

SamJUK Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

SamJUK commented Jul 12, 2026

Uh oh!

sonarqubecloud Bot commented Jul 13, 2026

Quality Gate failed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants