diff --git a/ProcessMaker/Cache/CacheInterface.php b/ProcessMaker/Cache/CacheInterface.php index a89024c9d5..24b127750d 100644 --- a/ProcessMaker/Cache/CacheInterface.php +++ b/ProcessMaker/Cache/CacheInterface.php @@ -14,6 +14,18 @@ interface CacheInterface */ public function get(string $key, mixed $default = null): mixed; + /** + * Fetches a value from the cache, or stores the value from the callback if the key exists. + * + * @param string $key The unique key of this item in the cache. + * @param callable $callback The callback that will return the value to store in the cache. + * + * @return mixed The value of the item from the cache, or $default in case of cache miss. + * + * @throws \InvalidArgumentException + */ + public function getOrCache(string $key, callable $callback): mixed; + /** * Persists data in the cache, uniquely referenced by a key with an optional expiration TTL time. * diff --git a/ProcessMaker/Cache/Settings/SettingCacheManager.php b/ProcessMaker/Cache/Settings/SettingCacheManager.php index a330ac5cb6..f9f5ff3814 100644 --- a/ProcessMaker/Cache/Settings/SettingCacheManager.php +++ b/ProcessMaker/Cache/Settings/SettingCacheManager.php @@ -2,9 +2,7 @@ namespace ProcessMaker\Cache\Settings; -use Exception; use Illuminate\Cache\CacheManager; -use Illuminate\Support\Facades\Log; use ProcessMaker\Cache\CacheInterface; class SettingCacheManager implements CacheInterface @@ -38,13 +36,40 @@ public function __call($method, $arguments): mixed */ public function get(string $key, mixed $default = null): mixed { + return $this->cacheManager->get($key, $default); + } + + /** + * Get a value from the settings cache, or store the value from the callback if the key exists. + * + * @param string $key + * @param callable $callback + * + * @return mixed + * + * @throws \InvalidArgumentException + */ + public function getOrCache(string $key, callable $callback): mixed + { + $value = $this->get($key); + + if ($value !== null) { + return $value; + } + try { - return $this->cacheManager->get($key, $default); - } catch (Exception $e) { - Log::error('Cache error: ' . $e->getMessage()); + $value = $callback(); + + if ($value === null) { + throw new \InvalidArgumentException('The key does not exist.'); + } + } catch (\Exception $e) { + throw new \InvalidArgumentException('The key does not exist.'); } - return null; + $this->set($key, $value); + + return $value; } /** diff --git a/ProcessMaker/Http/Controllers/Api/SettingController.php b/ProcessMaker/Http/Controllers/Api/SettingController.php index 4561d72bb3..d219c605e0 100644 --- a/ProcessMaker/Http/Controllers/Api/SettingController.php +++ b/ProcessMaker/Http/Controllers/Api/SettingController.php @@ -252,6 +252,9 @@ public function update(Setting $setting, Request $request) // Register the Event SettingsUpdated::dispatch($setting, $setting->getChanges(), $original); + // Store the setting in the cache + \SettingCache::set($setting->key, $setting->refresh()); + return response([], 204); } diff --git a/ProcessMaker/Models/Setting.php b/ProcessMaker/Models/Setting.php index e6167a2517..9fe9d2fd10 100644 --- a/ProcessMaker/Models/Setting.php +++ b/ProcessMaker/Models/Setting.php @@ -148,7 +148,18 @@ public static function messages() */ public static function byKey(string $key) { - return (new self)->where('key', $key)->first(); + $setting = \SettingCache::get($key); + + if ($setting === null) { + $setting = (new self)->where('key', $key)->first(); + + // Store the setting in the cache if it exists + if ($setting !== null) { + \SettingCache::set($key, $setting); + } + } + + return $setting; } /** diff --git a/tests/Feature/Cache/SettingCacheManagerTest.php b/tests/Feature/Cache/SettingCacheManagerTest.php new file mode 100644 index 0000000000..c14349e06f --- /dev/null +++ b/tests/Feature/Cache/SettingCacheManagerTest.php @@ -0,0 +1,102 @@ +with($key, $default) + ->andReturn($expected); + + $result = \SettingCache::get($key, $default); + + $this->assertEquals($expected, $result); + } + + public function testSet() + { + $key = 'test_key'; + $value = 'test_value'; + $ttl = 60; + + \SettingCache::shouldReceive('set') + ->with($key, $value, $ttl) + ->andReturn(true); + + $result = \SettingCache::set($key, $value, $ttl); + + $this->assertTrue($result); + } + + public function testDelete() + { + $key = 'test_key'; + + \SettingCache::shouldReceive('delete') + ->with($key) + ->andReturn(true); + + $result = \SettingCache::delete($key); + + $this->assertTrue($result); + } + + public function testClear() + { + \SettingCache::shouldReceive('clear') + ->andReturn(true); + + $result = \SettingCache::clear(); + + $this->assertTrue($result); + } + + public function testHas() + { + $key = 'test_key'; + + \SettingCache::shouldReceive('has') + ->with($key) + ->andReturn(true); + + $result = \SettingCache::has($key); + + $this->assertTrue($result); + } + + public function testMissing() + { + $key = 'test_key'; + + \SettingCache::shouldReceive('missing') + ->with($key) + ->andReturn(false); + + $result = \SettingCache::missing($key); + + $this->assertFalse($result); + } + + public function testCall() + { + $method = 'add'; + $arguments = ['arg1', 'arg2']; + $expected = 'cached_value'; + + \SettingCache::shouldReceive($method) + ->with(...$arguments) + ->andReturn($expected); + + $result = \SettingCache::__call($method, $arguments); + + $this->assertEquals($expected, $result); + } +} diff --git a/tests/Feature/Cache/SettingCacheTest.php b/tests/Feature/Cache/SettingCacheTest.php index 56288bec63..de68e68829 100644 --- a/tests/Feature/Cache/SettingCacheTest.php +++ b/tests/Feature/Cache/SettingCacheTest.php @@ -2,101 +2,117 @@ namespace Tests\Feature\Cache; +use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\DB; +use ProcessMaker\Models\Setting; +use Tests\Feature\Shared\RequestHelper; use Tests\TestCase; class SettingCacheTest extends TestCase { - public function testGet() - { - $key = 'test_key'; - $default = 'default_value'; - $expected = 'cached_value'; - - \SettingCache::shouldReceive('get') - ->with($key, $default) - ->andReturn($expected); - - $result = \SettingCache::get($key, $default); + use RequestHelper; + use RefreshDatabase; - $this->assertEquals($expected, $result); + private function upgrade() + { + $this->artisan('migrate', [ + '--path' => 'upgrades/2023_11_30_185738_add_password_policies_settings.php', + ])->run(); } - public function testSet() + public static function trackQueries(): void { - $key = 'test_key'; - $value = 'test_value'; - $ttl = 60; + DB::enableQueryLog(); + } - \SettingCache::shouldReceive('set') - ->with($key, $value, $ttl) - ->andReturn(true); + public static function flushQueryLog(): void + { + DB::flushQueryLog(); + } - $result = \SettingCache::set($key, $value, $ttl); + public static function getQueriesExecuted(): array + { + return DB::getQueryLog(); + } - $this->assertTrue($result); + public static function getQueryCount(): int + { + return count(self::getQueriesExecuted()); } - public function testDelete() + public function testGetSettingByKeyCached(): void { - $key = 'test_key'; + $this->upgrade(); + + $key = 'password-policies.users_can_change'; - \SettingCache::shouldReceive('delete') - ->with($key) - ->andReturn(true); + $setting = Setting::where('key', $key)->first(); + \SettingCache::set($key, $setting); - $result = \SettingCache::delete($key); + $this->trackQueries(); - $this->assertTrue($result); + $setting = Setting::byKey($key); + + $this->assertEquals(0, self::getQueryCount()); + $this->assertEquals($key, $setting->key); } - public function testClear() + public function testGetSettingByKeyNotCached(): void { - \SettingCache::shouldReceive('clear') - ->andReturn(true); + $key = 'password-policies.uppercase'; + + $this->upgrade(); + $this->trackQueries(); - $result = \SettingCache::clear(); + $setting = Setting::byKey($key); - $this->assertTrue($result); + $this->assertEquals(1, self::getQueryCount()); + $this->assertEquals($key, $setting->key); + + $this->flushQueryLog(); + + $setting = Setting::byKey($key); + $this->assertEquals(0, self::getQueryCount()); + $this->assertNotNull($setting); + $this->assertEquals($key, $setting->key); } - public function testHas() + public function testGetSettingByKeyCachedAfterUpdate(): void { - $key = 'test_key'; + $key = 'password-policies.special'; - \SettingCache::shouldReceive('has') - ->with($key) - ->andReturn(true); + $this->upgrade(); + $this->trackQueries(); - $result = \SettingCache::has($key); + $setting = Setting::byKey($key); - $this->assertTrue($result); - } + $this->assertEquals(1, self::getQueryCount()); + $this->assertEquals($key, $setting->key); + $this->assertEquals($setting->config, 1); - public function testMissing() - { - $key = 'test_key'; + $data = array_merge($setting->toArray(), ['config' => false]); - \SettingCache::shouldReceive('missing') - ->with($key) - ->andReturn(false); + $response = $this->apiCall('PUT', route('api.settings.update', ['setting' => $setting->id]), $data); + $response->assertStatus(204); - $result = \SettingCache::missing($key); + $this->flushQueryLog(); - $this->assertFalse($result); + $setting = Setting::byKey($key); + $this->assertEquals(0, self::getQueryCount()); + $this->assertEquals($key, $setting->key); + $this->assertEquals($setting->config, 0); } - public function testCall() + public function testGetSettingByNotExistingKey() { - $method = 'add'; - $arguments = ['arg1', 'arg2']; - $expected = 'cached_value'; + $this->withoutExceptionHandling(); + $key = 'non-existing-key'; - \SettingCache::shouldReceive($method) - ->with(...$arguments) - ->andReturn($expected); + $callback = fn() => Setting::where('key', $key)->first(); - $result = \SettingCache::__call($method, $arguments); + $this->expectException(\InvalidArgumentException::class); + $setting = \SettingCache::getOrCache($key, $callback); - $this->assertEquals($expected, $result); + $this->assertNull($setting); } }