diff --git a/core/components/minishop3/src/Services/GridConfigService.php b/core/components/minishop3/src/Services/GridConfigService.php index e1d913d8..014d00ea 100644 --- a/core/components/minishop3/src/Services/GridConfigService.php +++ b/core/components/minishop3/src/Services/GridConfigService.php @@ -14,6 +14,22 @@ class GridConfigService /** @var modX */ protected $modx; + /** + * Request-scoped memo for getGridConfig(): gridKey:includeHidden → column config. + * + * This cache is safe only because GridConfigService is registered in the + * MODX DI container as a request-scoped singleton (see ServiceRegistry): + * one instance per request, never shared across requests. The memo must + * never be persisted to a long-lived process cache or serialized — it + * holds no TTL and would leak stale data across requests. Mutation + * methods (saveGridConfig, addField, updateField, deleteField) invalidate + * the affected gridKey entries via invalidateGridConfigCache() so the next + * read re-loads from the database. + * + * @var array>> + */ + private array $gridConfigCache = []; + /** * @param modX $modx */ @@ -29,6 +45,22 @@ public function __construct(modX $modx) * @return array */ public function getGridConfig(string $gridKey, bool $includeHidden = false): array + { + $cacheKey = $this->gridConfigCacheKey($gridKey, $includeHidden); + if (array_key_exists($cacheKey, $this->gridConfigCache)) { + return $this->gridConfigCache[$cacheKey]; + } + + $fields = $this->loadGridConfig($gridKey, $includeHidden); + $this->gridConfigCache[$cacheKey] = $fields; + + return $fields; + } + + /** + * @return list> + */ + private function loadGridConfig(string $gridKey, bool $includeHidden): array { $query = $this->modx->newQuery(msGridField::class); $where = ['grid_key' => $gridKey]; @@ -74,6 +106,27 @@ public function getGridConfig(string $gridKey, bool $includeHidden = false): arr return $fields; } + private function gridConfigCacheKey(string $gridKey, bool $includeHidden): string + { + return $gridKey . ':' . ($includeHidden ? '1' : '0'); + } + + /** + * Call at the start of any msGridField mutation so request-scoped cache stays consistent. + */ + private function beginGridMutation(string $gridKey): void + { + $this->invalidateGridConfigCache($gridKey); + } + + private function invalidateGridConfigCache(string $gridKey): void + { + unset( + $this->gridConfigCache[$this->gridConfigCacheKey($gridKey, false)], + $this->gridConfigCache[$this->gridConfigCacheKey($gridKey, true)] + ); + } + /** * Get label considering lexicon_key * @@ -112,6 +165,8 @@ protected function resolveLabel(msGridField $field): string */ public function saveGridConfig(string $gridKey, array $fields): bool { + $this->beginGridMutation($gridKey); + try { // Get list of field names to keep $fieldNamesToKeep = []; @@ -253,6 +308,8 @@ public function saveGridConfig(string $gridKey, array $fields): bool */ public function deleteField(string $gridKey, string $fieldName): array { + $this->beginGridMutation($gridKey); + try { $field = $this->modx->getObject(msGridField::class, [ 'grid_key' => $gridKey, @@ -308,6 +365,8 @@ public function deleteField(string $gridKey, string $fieldName): array */ public function addField(string $gridKey, array $data): array { + $this->beginGridMutation($gridKey); + try { // Basic validation if (empty($data['field_name'])) { @@ -450,6 +509,8 @@ public function addField(string $gridKey, array $data): array */ public function updateField(string $gridKey, string $fieldName, array $data): array { + $this->beginGridMutation($gridKey); + try { // Find existing field $field = $this->modx->getObject(msGridField::class, [ diff --git a/core/components/minishop3/tests/Unit/Services/GridConfigServiceMemoTest.php b/core/components/minishop3/tests/Unit/Services/GridConfigServiceMemoTest.php new file mode 100644 index 00000000..73345b16 --- /dev/null +++ b/core/components/minishop3/tests/Unit/Services/GridConfigServiceMemoTest.php @@ -0,0 +1,53 @@ +getGridConfig('orders', true); + $second = $service->getGridConfig('orders', true); + + self::assertSame($first, $second); + self::assertSame(1, $modx->getCollectionCalls); + self::assertSame([], $first); + } + + public function testIncludeHiddenUsesSeparateCacheEntry(): void + { + $modx = new GridConfigModxStub(); + $service = new GridConfigService($modx); + + $service->getGridConfig('orders', false); + $service->getGridConfig('orders', true); + + self::assertSame(2, $modx->getCollectionCalls); + } + + public function testSaveGridConfigInvalidatesMemoForSameRequest(): void + { + $modx = new GridConfigModxStub(); + $service = new GridConfigService($modx); + + $service->getGridConfig('orders', true); + $service->saveGridConfig('orders', []); + $service->getGridConfig('orders', true); + + // 1st read + saveGridConfig delete scan + 2nd read after cache invalidation + self::assertSame(3, $modx->getCollectionCalls); + } +} diff --git a/core/components/minishop3/tests/stubs/GridConfigModxStub.php b/core/components/minishop3/tests/stubs/GridConfigModxStub.php new file mode 100644 index 00000000..498cca90 --- /dev/null +++ b/core/components/minishop3/tests/stubs/GridConfigModxStub.php @@ -0,0 +1,53 @@ + */ + public array $gridFields = []; + + /** @var object */ + public object $lexicon; + + public function __construct() + { + parent::__construct(); + $this->lexicon = new class { + public function load(string $topic): void + { + } + + public function __invoke(string $key): string + { + return $key; + } + }; + } + + public function newQuery($class = ''): GridConfigQueryStub + { + return new GridConfigQueryStub(); + } + + public function getCollection($className, $criteria = null): array + { + $this->getCollectionCalls++; + + return $this->gridFields; + } + + public function getObject($className, $criteria = null, $cacheFlag = true): ?object + { + return null; + } +} diff --git a/core/components/minishop3/tests/stubs/GridConfigQueryStub.php b/core/components/minishop3/tests/stubs/GridConfigQueryStub.php new file mode 100644 index 00000000..0d16ea35 --- /dev/null +++ b/core/components/minishop3/tests/stubs/GridConfigQueryStub.php @@ -0,0 +1,26 @@ +