diff --git a/Model/Config/Source/StorageDriver.php b/Model/Config/Source/StorageDriver.php new file mode 100644 index 0000000..dcc37b2 --- /dev/null +++ b/Model/Config/Source/StorageDriver.php @@ -0,0 +1,21 @@ + 'db', 'label' => __('Database (default)')], + ['value' => 'redis', 'label' => __('Redis')], + ]; + } +} diff --git a/Model/Storage/Pool.php b/Model/Storage/Pool.php new file mode 100644 index 0000000..30cec8a --- /dev/null +++ b/Model/Storage/Pool.php @@ -0,0 +1,95 @@ +objectManager = $objectManager; + $this->scopeConfig = $scopeConfig; + $this->drivers = $drivers; + } + + /** + * @inheritDoc + */ + public function add(array $tags): void + { + $this->driver()->add($tags); + } + + /** + * @inheritDoc + */ + public function claim(): string + { + return $this->driver()->claim(); + } + + /** + * @inheritDoc + */ + public function tags(string $batchId): array + { + return $this->driver()->tags($batchId); + } + + /** + * @inheritDoc + */ + public function clear(string $batchId): void + { + $this->driver()->clear($batchId); + } + + /** + * @inheritDoc + */ + public function release(string $batchId): void + { + $this->driver()->release($batchId); + } + + /** + * @inheritDoc + */ + public function activeBatch(): string + { + return $this->driver()->activeBatch(); + } + + private function driver(): QueueStorageInterface + { + if ($this->resolved === null) { + $key = (string)$this->scopeConfig->getValue(self::XML_PATH_STORAGE_DRIVER) ?: self::DEFAULT_DRIVER; + $class = $this->drivers[$key] ?? $this->drivers[self::DEFAULT_DRIVER]; + $this->resolved = $this->objectManager->create($class); + } + + return $this->resolved; + } +} diff --git a/Model/Storage/Redis.php b/Model/Storage/Redis.php new file mode 100644 index 0000000..85d7109 --- /dev/null +++ b/Model/Storage/Redis.php @@ -0,0 +1,99 @@ +connectionResolver = $connectionResolver; + } + + /** + * @inheritDoc + */ + public function add(array $tags): void + { + if (!$tags) { + return; + } + + $this->connectionResolver->getClient()->sAdd(self::KEY_LIVE, ...$tags); + } + + /** + * @inheritDoc + */ + public function claim(): string + { + if ($this->activeBatch() !== '') { + return ''; + } + + $client = $this->connectionResolver->getClient(); + + try { + $client->rename(self::KEY_LIVE, $this->batchKey(self::ACTIVE_BATCH_ID)); + } catch (\CredisException $e) { + if (stripos($e->getMessage(), 'no such key') !== false) { + return ''; + } + throw $e; + } + + return self::ACTIVE_BATCH_ID; + } + + /** + * @inheritDoc + */ + public function tags(string $batchId): array + { + return $this->connectionResolver->getClient()->sMembers($this->batchKey($batchId)); + } + + /** + * @inheritDoc + */ + public function clear(string $batchId): void + { + $this->connectionResolver->getClient()->del($this->batchKey($batchId)); + } + + /** + * @inheritDoc + */ + public function release(string $batchId): void + { + $client = $this->connectionResolver->getClient(); + $client->sUnionStore(self::KEY_LIVE, self::KEY_LIVE, $this->batchKey($batchId)); + $client->del($this->batchKey($batchId)); + } + + /** + * @inheritDoc + */ + public function activeBatch(): string + { + return $this->connectionResolver->getClient()->sCard($this->batchKey(self::ACTIVE_BATCH_ID)) > 0 + ? self::ACTIVE_BATCH_ID + : ''; + } + + private function batchKey(string $batchId): string + { + return self::KEY_BATCH_PREFIX . $batchId; + } +} diff --git a/Model/Storage/Redis/ConnectionResolver.php b/Model/Storage/Redis/ConnectionResolver.php new file mode 100644 index 0000000..15e6520 --- /dev/null +++ b/Model/Storage/Redis/ConnectionResolver.php @@ -0,0 +1,88 @@ +deploymentConfig = $deploymentConfig; + } + + /** + * @throws LocalizedException when no usable connection can be resolved. + */ + public function getClient(): \Credis_Client + { + if ($this->client !== null) { + return $this->client; + } + + $dedicated = $this->deploymentConfig->get(self::CONFIG_PATH_DEDICATED); + $config = $dedicated ? $this->normalize($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 + ); + + return $this->client; + } + + private function resolveFallback(): ?array + { + $backendOptions = $this->deploymentConfig->get(self::CONFIG_PATH_SHARED_FALLBACK); + + if (!$backendOptions) { + return null; + } + + return [ + 'host' => $backendOptions['server'] ?? '127.0.0.1', + 'port' => $backendOptions['port'] ?? 6379, + 'password' => $backendOptions['password'] ?? '', + 'database' => $backendOptions['database'] ?? 0, + ]; + } + + /** + * Fill in defaults for whichever of host/port/database/password the + * "cache_debounce.redis" env.php block leaves out. + */ + private function normalize(array $config): array + { + return [ + 'host' => $config['host'] ?? '127.0.0.1', + 'port' => $config['port'] ?? 6379, + 'password' => $config['password'] ?? '', + 'database' => $config['database'] ?? 0, + ]; + } +} diff --git a/README.md b/README.md index 35ecd2c..50336a9 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,45 @@ Option | Config Path | Default | Description --- | --- | --- | --- Enabled | `samjuk_cache_debounce/general/enabled` | `0` | Feature flag to toggle functionality of the module Flush Schedule | `samjuk_cache_debounce/cron/flush_schedule` | `*/5 0 0 0 0` | Cron schedule to run the scheduled flush +Storage Driver | `samjuk_cache_debounce_advanced/general/storage_driver` | `db` | Queue storage backend — `db` or `redis`. A deploy-time infra decision, not a store setting — see below + +This one is a deliberate exception to "Configuration can be handled via System configuration" above: it's hidden from Stores > Configuration entirely (`showInDefault`/`showInWebsite`/`showInStore` all `0`) and gated behind its own ACL resource (`SamJUK_CacheDebounce::storage_driver`) that isn't granted to any role by default. It exists only so `config:set` has a registered path to write to. Set it with: +```sh +php bin/magento config:set samjuk_cache_debounce_advanced/general/storage_driver redis +``` +Add `-e` to lock it into `app/etc/env.php` instead of `core_config_data`, removing Admin overridability entirely. + +### Redis storage driver + +The `redis` driver connects via a dedicated `app/etc/env.php` block: +```php +'cache_debounce' => [ + 'redis' => [ + 'host' => '127.0.0.1', + 'port' => '6379', + 'password' => '', + 'database' => '2', + ], +], +``` + +If that block is absent, the module falls back to whatever Redis backend is already configured for the `page_cache` cache frontend, so it works zero-config once the driver is switched on. + +> **Note:** sharing the page cache's Redis instance means pending purge tags are subject to that instance's `maxmemory-policy`. If it's `allkeys-lru`/`allkeys-lfu` (common for FPC), tags can be silently evicted under memory pressure. Configure a dedicated block above to avoid this. + +### Adding your own storage driver + +Drivers are resolved from an array bound in `etc/di.xml`, keyed by the value of `storage_driver`. Any module can add an entry to extend it, without touching this module's `di.xml`: +```xml + + + + Vendor\Module\Model\Storage\MyDriver + + + +``` +`MyDriver` just needs to implement `QueueStorageInterface`. Then `bin/magento config:set samjuk_cache_debounce_advanced/general/storage_driver my_driver`. ## Will this help my store? diff --git a/Test/Integration/Model/Storage/Redis/ConnectionResolverTest.php b/Test/Integration/Model/Storage/Redis/ConnectionResolverTest.php new file mode 100644 index 0000000..9c8bd9e --- /dev/null +++ b/Test/Integration/Model/Storage/Redis/ConnectionResolverTest.php @@ -0,0 +1,69 @@ +get(DeploymentConfig\Reader::class); + + $dedicatedConfig = new DeploymentConfig($reader, ['cache_debounce' => ['redis' => [ + 'host' => '127.0.0.1', + 'port' => '6379', + 'password' => '', + 'database' => (string)self::DEDICATED_DATABASE, + ]]]); + $fallbackConfig = new DeploymentConfig($reader, ['cache' => ['frontend' => ['page_cache' => [ + 'backend_options' => [ + 'server' => '127.0.0.1', + 'port' => '6379', + 'password' => '', + 'database' => (string)self::FALLBACK_DATABASE, + ], + ]]]]); + + $this->dedicatedResolver = new ConnectionResolver($dedicatedConfig); + $this->fallbackResolver = new ConnectionResolver($fallbackConfig); + + $this->dedicatedResolver->getClient()->flushDb(); + $this->fallbackResolver->getClient()->flushDb(); + } + + protected function tearDown(): void + { + $this->dedicatedResolver->getClient()->flushDb(); + $this->fallbackResolver->getClient()->flushDb(); + } + + public function testResolvesTheDedicatedConnection() + { + $client = $this->dedicatedResolver->getClient(); + + $client->set(self::CANARY_KEY, '1'); + $this->assertSame('1', $client->get(self::CANARY_KEY)); + } + + public function testFallsBackToThePageCacheConnectionWhenNoDedicatedBlockIsConfigured() + { + $client = $this->fallbackResolver->getClient(); + + $client->set(self::CANARY_KEY, '1'); + $this->assertSame('1', $client->get(self::CANARY_KEY)); + } +} diff --git a/Test/Integration/Model/Storage/RedisTest.php b/Test/Integration/Model/Storage/RedisTest.php new file mode 100644 index 0000000..4511c1f --- /dev/null +++ b/Test/Integration/Model/Storage/RedisTest.php @@ -0,0 +1,133 @@ + '127.0.0.1', + 'port' => '6379', + 'password' => '', + 'database' => '2', + ]; + + private $storage; + private $rawClient; + + 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)); + + // Independent resolver instance, not the class under test. + $this->rawClient = (new ConnectionResolver($deploymentConfig))->getClient(); + $this->rawClient->flushDb(); + } + + protected function tearDown(): void + { + $this->rawClient->flushDb(); + } + + public function testAddClaimTagsClearRoundTrip() + { + $this->storage->add(['cat_c_1', 'cat_c_2']); + + $batchId = $this->storage->claim(); + $this->assertNotSame('', $batchId); + + $tags = $this->storage->tags($batchId); + sort($tags); + $this->assertEquals(['cat_c_1', 'cat_c_2'], $tags); + + $this->storage->clear($batchId); + $this->assertEquals([], $this->storage->tags($batchId)); + } + + public function testAddDuringClaimIsNotLostOrDuplicated() + { + $this->storage->add(['cat_c_1']); + + $batchId = $this->storage->claim(); + + $this->storage->add(['cat_c_1', 'cat_c_2']); + + $this->assertEquals(['cat_c_1'], $this->storage->tags($batchId)); + + $this->storage->clear($batchId); + + $nextBatchId = $this->storage->claim(); + $tags = $this->storage->tags($nextBatchId); + sort($tags); + $this->assertEquals(['cat_c_1', 'cat_c_2'], $tags); + + $this->storage->clear($nextBatchId); + } + + public function testSecondConcurrentClaimReturnsEmptyString() + { + $this->storage->add(['cat_c_1']); + + $firstBatchId = $this->storage->claim(); + $this->assertNotSame('', $firstBatchId); + + $this->assertSame('', $this->storage->claim()); + + $this->storage->clear($firstBatchId); + } + + public function testReleaseMakesTheBatchClaimableAgain() + { + $this->storage->add(['cat_c_1', 'cat_c_2']); + $batchId = $this->storage->claim(); + + $this->storage->release($batchId); + + $this->assertEquals([], $this->storage->tags($batchId)); + + $nextBatchId = $this->storage->claim(); + $tags = $this->storage->tags($nextBatchId); + sort($tags); + $this->assertEquals(['cat_c_1', 'cat_c_2'], $tags); + } + + public function testReleaseMergesCleanlyWithTagsReQueuedWhileClaimed() + { + $this->storage->add(['cat_c_1']); + $batchId = $this->storage->claim(); + + // Same tag re-queued while the original batch is still in flight — + // release() must not duplicate it; Redis sets dedupe natively. + $this->storage->add(['cat_c_1']); + + $this->storage->release($batchId); + + $nextBatchId = $this->storage->claim(); + $this->assertEquals(['cat_c_1'], $this->storage->tags($nextBatchId)); + } + + public function testActiveBatchTracksClaimedWorkUntilItIsCleared() + { + $this->assertSame('', $this->storage->activeBatch()); + + $this->storage->add(['cat_c_1']); + $batchId = $this->storage->claim(); + + $this->assertSame($batchId, $this->storage->activeBatch()); + + $this->storage->clear($batchId); + + $this->assertSame('', $this->storage->activeBatch()); + } +} diff --git a/Test/Unit/Model/Config/Source/StorageDriverTest.php b/Test/Unit/Model/Config/Source/StorageDriverTest.php new file mode 100644 index 0000000..9d65f05 --- /dev/null +++ b/Test/Unit/Model/Config/Source/StorageDriverTest.php @@ -0,0 +1,16 @@ +toOptionArray(), 'value'); + + $this->assertSame(['db', 'redis'], $values); + } +} diff --git a/Test/Unit/Model/Storage/PoolTest.php b/Test/Unit/Model/Storage/PoolTest.php new file mode 100644 index 0000000..d232228 --- /dev/null +++ b/Test/Unit/Model/Storage/PoolTest.php @@ -0,0 +1,98 @@ + Database::class, + 'redis' => Redis::class, + ]; + + private $objectManager; + private $scopeConfig; + private $database; + private $redis; + + protected function setUp(): void + { + $this->objectManager = $this->createMock(ObjectManagerInterface::class); + $this->scopeConfig = $this->createMock(ScopeConfigInterface::class); + $this->database = $this->createMock(Database::class); + $this->redis = $this->createMock(Redis::class); + + $this->objectManager->method('create')->willReturnMap([ + [Database::class, [], $this->database], + [Redis::class, [], $this->redis], + ]); + } + + private function pool(): Pool + { + return new Pool($this->objectManager, $this->scopeConfig, self::DRIVERS); + } + + public function testUsesDatabaseDriverByDefault() + { + $this->scopeConfig->method('getValue')->with(self::XML_PATH_STORAGE_DRIVER)->willReturn(null); + + $this->database->expects($this->once())->method('add')->with(self::CACHE_TAGS); + + $this->pool()->add(self::CACHE_TAGS); + } + + public function testUsesDatabaseDriverForAnyValueOtherThanRedis() + { + $this->scopeConfig->method('getValue')->willReturn('db'); + + $this->database->expects($this->once())->method('claim')->willReturn('batch-123'); + + $this->pool()->claim(); + } + + public function testUsesRedisDriverWhenConfiguredAndNeverTouchesDatabase() + { + $this->scopeConfig->method('getValue')->willReturn('redis'); + + $this->database->expects($this->never())->method('add'); + $this->redis->expects($this->once())->method('add')->with(self::CACHE_TAGS); + + $this->pool()->add(self::CACHE_TAGS); + } + + public function testDriverIsOnlyResolvedOnceAcrossMultipleCalls() + { + $objectManager = $this->createMock(ObjectManagerInterface::class); + $objectManager->expects($this->once())->method('create')->with(Redis::class)->willReturn($this->redis); + $this->scopeConfig->method('getValue')->willReturn('redis'); + + $pool = new Pool($objectManager, $this->scopeConfig, self::DRIVERS); + $pool->add(['cat_c_1']); + $pool->claim(); + } + + public function testReleaseDelegatesToTheResolvedDriver() + { + $this->scopeConfig->method('getValue')->willReturn('db'); + $this->database->expects($this->once())->method('release')->with('batch-123'); + + $this->pool()->release('batch-123'); + } + + public function testActiveBatchDelegatesToTheResolvedDriver() + { + $this->scopeConfig->method('getValue')->willReturn('redis'); + $this->redis->expects($this->once())->method('activeBatch')->willReturn('batch-123'); + + $this->assertSame('batch-123', $this->pool()->activeBatch()); + } +} diff --git a/Test/Unit/Model/Storage/Redis/ConnectionResolverTest.php b/Test/Unit/Model/Storage/Redis/ConnectionResolverTest.php new file mode 100644 index 0000000..5a5e756 --- /dev/null +++ b/Test/Unit/Model/Storage/Redis/ConnectionResolverTest.php @@ -0,0 +1,69 @@ + '10.0.0.1', 'port' => '6390', 'password' => 'x', 'database' => '2']; + private const FALLBACK_CONFIG = ['server' => '10.0.0.2', 'port' => '6379', 'password' => '', 'database' => '0']; + + private $deploymentConfig; + + protected function setUp(): void + { + $this->deploymentConfig = $this->createMock(DeploymentConfig::class); + } + + public function testDedicatedConfigTakesPriorityOverTheSharedFallback() + { + $this->deploymentConfig->method('get')->willReturnMap([ + ['cache_debounce/redis', null, self::DEDICATED_CONFIG], + ['cache/frontend/page_cache/backend_options', null, self::FALLBACK_CONFIG], + ]); + + $this->assertResolvesToClient(); + } + + public function testFallsBackToThePageCacheBackendWhenDedicatedConfigIsAbsent() + { + $this->deploymentConfig->method('get')->willReturnMap([ + ['cache_debounce/redis', null, null], + ['cache/frontend/page_cache/backend_options', null, self::FALLBACK_CONFIG], + ]); + + $this->assertResolvesToClient(); + } + + public function testThrowsLocalizedExceptionWhenNeitherConfigIsAvailable() + { + $this->deploymentConfig->method('get')->willReturn(null); + + $resolver = new ConnectionResolver($this->deploymentConfig); + + $this->expectException(LocalizedException::class); + + $resolver->getClient(); + } + + public function testDedicatedConfigMissingKeysFallsBackToDefaultsInsteadOfErroring() + { + $this->deploymentConfig->method('get')->willReturnMap([ + ['cache_debounce/redis', null, ['host' => '10.0.0.9']], + ['cache/frontend/page_cache/backend_options', null, null], + ]); + + $this->assertResolvesToClient(); + } + + private function assertResolvesToClient(): void + { + $resolver = new ConnectionResolver($this->deploymentConfig); + + $this->assertInstanceOf(\Credis_Client::class, $resolver->getClient()); + } +} diff --git a/Test/Unit/Model/Storage/RedisTest.php b/Test/Unit/Model/Storage/RedisTest.php new file mode 100644 index 0000000..d449663 --- /dev/null +++ b/Test/Unit/Model/Storage/RedisTest.php @@ -0,0 +1,130 @@ +client = $this->getMockBuilder(\Credis_Client::class) + ->disableOriginalConstructor() + ->addMethods(['sAdd', 'rename', 'sMembers', 'del', 'sUnionStore', 'sCard']) + ->getMock(); + $this->connectionResolver = $this->createMock(ConnectionResolver::class); + $this->connectionResolver->method('getClient')->willReturn($this->client); + + $this->storage = new Redis($this->connectionResolver); + } + + public function testAddWithEmptyTagsDoesNotTouchConnection() + { + $this->client->expects($this->never())->method('sAdd'); + + $this->storage->add([]); + } + + public function testAddSaddsTagsIntoTheLiveSet() + { + $this->client->expects($this->once()) + ->method('sAdd') + ->with(self::KEY_LIVE, 'cat_c_1', 'cat_c_2'); + + $this->storage->add(['cat_c_1', 'cat_c_2']); + } + + public function testClaimReturnsEmptyStringWhenLiveKeyDoesNotExist() + { + $this->client->method('sCard')->willReturn(0); + $this->client->method('rename')->willThrowException(new \CredisException('ERR no such key')); + + $this->assertSame('', $this->storage->claim()); + } + + public function testClaimDoesNotSwallowOtherRedisErrors() + { + $this->client->method('sCard')->willReturn(0); + $this->client->method('rename')->willThrowException(new \CredisException('ERR something else')); + + $this->expectException(\CredisException::class); + $this->expectExceptionMessage('ERR something else'); + + $this->storage->claim(); + } + + public function testClaimRenamesLiveKeyToABatchKeyAndReturnsItsId() + { + $this->client->method('sCard')->willReturn(0); + $this->client->expects($this->once()) + ->method('rename') + ->with(self::KEY_LIVE, self::KEY_BATCH_PREFIX . self::ACTIVE_BATCH_ID); + + $batchId = $this->storage->claim(); + + $this->assertSame(self::ACTIVE_BATCH_ID, $batchId); + } + + public function testClaimReturnsEmptyStringWhenAnotherBatchIsAlreadyClaimed() + { + $this->client->method('sCard')->willReturn(1); + $this->client->expects($this->never())->method('rename'); + + $this->assertSame('', $this->storage->claim()); + } + + public function testTagsReadsFromTheBatchKey() + { + $this->client->expects($this->once()) + ->method('sMembers') + ->with(self::KEY_BATCH_PREFIX . 'batch-123') + ->willReturn(['cat_c_1']); + + $this->assertSame(['cat_c_1'], $this->storage->tags('batch-123')); + } + + public function testClearDeletesTheBatchKey() + { + $this->client->expects($this->once()) + ->method('del') + ->with(self::KEY_BATCH_PREFIX . self::ACTIVE_BATCH_ID); + + $this->storage->clear(self::ACTIVE_BATCH_ID); + } + + public function testReleaseMergesBatchSetBackIntoLiveSetThenDeletesTheBatch() + { + $this->client->expects($this->once()) + ->method('sUnionStore') + ->with(self::KEY_LIVE, self::KEY_LIVE, self::KEY_BATCH_PREFIX . self::ACTIVE_BATCH_ID); + $this->client->expects($this->once()) + ->method('del') + ->with(self::KEY_BATCH_PREFIX . self::ACTIVE_BATCH_ID); + + $this->storage->release(self::ACTIVE_BATCH_ID); + } + + public function testActiveBatchReturnsEmptyStringWhenNoBatchIsClaimed() + { + $this->client->method('sCard')->with(self::KEY_BATCH_PREFIX . self::ACTIVE_BATCH_ID)->willReturn(0); + + $this->assertSame('', $this->storage->activeBatch()); + } + + public function testActiveBatchReturnsIdOfAlreadyClaimedBatch() + { + $this->client->method('sCard')->with(self::KEY_BATCH_PREFIX . self::ACTIVE_BATCH_ID)->willReturn(1); + + $this->assertSame(self::ACTIVE_BATCH_ID, $this->storage->activeBatch()); + } +} diff --git a/etc/adminhtml/acl.xml b/etc/adminhtml/acl.xml new file mode 100644 index 0000000..6bda1e9 --- /dev/null +++ b/etc/adminhtml/acl.xml @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + + + + + diff --git a/etc/adminhtml/system.xml b/etc/adminhtml/system.xml index c51d38b..3e1e6fc 100644 --- a/etc/adminhtml/system.xml +++ b/etc/adminhtml/system.xml @@ -23,5 +23,20 @@ + + +
+ + samjuk + SamJUK_CacheDebounce::storage_driver + + + + + Do not change unless you know what you are doing. See README.md. + SamJUK\CacheDebounce\Model\Config\Source\StorageDriver + + +
diff --git a/etc/config.xml b/etc/config.xml index fcd33e3..119af94 100644 --- a/etc/config.xml +++ b/etc/config.xml @@ -9,5 +9,10 @@ */15 * * * * + + + db + + diff --git a/etc/di.xml b/etc/di.xml index 68f549a..d5080d3 100644 --- a/etc/di.xml +++ b/etc/di.xml @@ -1,6 +1,15 @@ - + + + + + + SamJUK\CacheDebounce\Model\Storage\Database + SamJUK\CacheDebounce\Model\Storage\Redis + + +