Changed the way settings are loaded

This new method batch-loads them from the database, and removes the
cache-layer with the intention that a couple of batch fetches from the
DB is more efficient than hitting the cache each time.

diff --git a/app/Settings/SettingService.php b/app/Settings/SettingService.php
index d1bac164da8103771e7120c5443273736764d424..9bcd6c7ec5ca6c33b4bc5e09395832ca721c6e22 100644 (file)
--- a/app/Settings/SettingService.php
+++ b/app/Settings/SettingService.php
@@ -3,45 +3,29 @@
 namespace BookStack\Settings;
 
 use BookStack\Auth\User;
 namespace BookStack\Settings;
 
 use BookStack\Auth\User;

-use Illuminate\Contracts\Cache\Repository as Cache;

 
 /**
  * Class SettingService
  * The settings are a simple key-value database store.
  * For non-authenticated users, user settings are stored via the session instead.
 
 /**
  * Class SettingService
  * The settings are a simple key-value database store.
  * For non-authenticated users, user settings are stored via the session instead.

+ * A local array-based cache is used to for setting accesses across a request.

  */
 class SettingService
 {
  */
 class SettingService
 {

-    protected Setting $setting;
-    protected Cache $cache;

     protected array $localCache = [];
     protected array $localCache = [];

-    protected string $cachePrefix = 'setting-';
-
-    public function __construct(Setting $setting, Cache $cache)
-    {
-        $this->setting = $setting;
-        $this->cache = $cache;
-    }

 
     /**
      * Gets a setting from the database,
      * If not found, Returns default, Which is false by default.
      */
 
     /**
      * Gets a setting from the database,
      * If not found, Returns default, Which is false by default.
      */

-    public function get(string $key, $default = null)
+    public function get(string $key, $default = null): mixed

     {
         if (is_null($default)) {
             $default = config('setting-defaults.' . $key, false);
         }
 
     {
         if (is_null($default)) {
             $default = config('setting-defaults.' . $key, false);
         }
 

-        if (isset($this->localCache[$key])) {
-            return $this->localCache[$key];
-        }
-

         $value = $this->getValueFromStore($key) ?? $default;
         $value = $this->getValueFromStore($key) ?? $default;

-        $formatted = $this->formatValue($value, $default);
-        $this->localCache[$key] = $formatted;
-
-        return $formatted;
+        return $this->formatValue($value, $default);

     }
 
     /**
     }
 
     /**


@@ -79,52 +63,78 @@ class SettingService
     }
 
     /**
     }
 
     /**

-     * Gets a setting value from the cache or database.
-     * Looks at the system defaults if not cached or in database.
-     * Returns null if nothing is found.
+     * Gets a setting value from the local cache.
+     * Will load the local cache if not previously loaded.

      */
      */

-    protected function getValueFromStore(string $key)
+    protected function getValueFromStore(string $key): mixed

     {
     {

-        // Check the cache
-        $cacheKey = $this->cachePrefix . $key;
-        $cacheVal = $this->cache->get($cacheKey, null);
-        if ($cacheVal !== null) {
-            return $cacheVal;
+        $cacheCategory = $this->localCacheCategory($key);
+        if (!isset($this->localCache[$cacheCategory])) {
+            $this->loadToLocalCache($cacheCategory);

         }
 
         }
 

-        // Check the database
-        $settingObject = $this->getSettingObjectByKey($key);
-        if ($settingObject !== null) {
-            $value = $settingObject->value;
+        return $this->localCache[$cacheCategory][$key] ?? null;
+    }

 
 

-            if ($settingObject->type === 'array') {
-                $value = json_decode($value, true) ?? [];
-            }
+    /**
+     * Put the given value into the local cached under the given key.
+     */
+    protected function putValueIntoLocalCache(string $key, mixed $value): void
+    {
+        $cacheCategory = $this->localCacheCategory($key);
+        if (!isset($this->localCache[$cacheCategory])) {
+            $this->loadToLocalCache($cacheCategory);
+        }

 
 

-            $this->cache->forever($cacheKey, $value);
+        $this->localCache[$cacheCategory][$key] = $value;
+    }

 
 

-            return $value;
+    /**
+     * Get the category for the given setting key.
+     * Will return 'app' for a general app setting otherwise 'user:<user_id>' for a user setting.
+     */
+    protected function localCacheCategory(string $key): string
+    {
+        if (str_starts_with($key, 'user:')) {
+            return implode(':', array_slice(explode(':', $key), 0, 2));

         }
 
         }
 

-        return null;
+        return 'app';

     }
 
     /**
     }
 
     /**

-     * Clear an item from the cache completely.
+     * For the given category, load the relevant settings from the database into the local cache.

      */
      */

-    protected function clearFromCache(string $key)
+    protected function loadToLocalCache(string $cacheCategory): void

     {
     {

-        $cacheKey = $this->cachePrefix . $key;
-        $this->cache->forget($cacheKey);
-        if (isset($this->localCache[$key])) {
-            unset($this->localCache[$key]);
+        $query = Setting::query();
+
+        if ($cacheCategory === 'app') {
+            $query->where('setting_key', 'not like', 'user:%');
+        } else {
+            $query->where('setting_key', 'like', $cacheCategory . ':%');
+        }
+        $settings = $query->toBase()->get();
+
+        if (!isset($this->localCache[$cacheCategory])) {
+            $this->localCache[$cacheCategory] = [];
+        }
+
+        foreach ($settings as $setting) {
+            $value = $setting->value;
+
+            if ($setting->type === 'array') {
+                $value = json_decode($value, true) ?? [];
+            }
+
+            $this->localCache[$cacheCategory][$setting->setting_key] = $value;

         }
     }
 
     /**
      * Format a settings value.
      */
         }
     }
 
     /**
      * Format a settings value.
      */

-    protected function formatValue($value, $default)
+    protected function formatValue(mixed $value, mixed $default): mixed

     {
         // Change string booleans to actual booleans
         if ($value === 'true') {
     {
         // Change string booleans to actual booleans
         if ($value === 'true') {


@@ -155,21 +165,22 @@ class SettingService
      * Add a setting to the database.
      * Values can be an array or a string.
      */
      * Add a setting to the database.
      * Values can be an array or a string.
      */

-    public function put(string $key, $value): bool
+    public function put(string $key, mixed $value): bool

     {
     {

-        $setting = $this->setting->newQuery()->firstOrNew([
+        $setting = Setting::query()->firstOrNew([

             'setting_key' => $key,
         ]);
             'setting_key' => $key,
         ]);

+

         $setting->type = 'string';
         $setting->type = 'string';

+        $setting->value = $value;

 
         if (is_array($value)) {
             $setting->type = 'array';
 
         if (is_array($value)) {
             $setting->type = 'array';

-            $value = $this->formatArrayValue($value);
+            $setting->value = $this->formatArrayValue($value);

         }
 
         }
 

-        $setting->value = $value;

         $setting->save();
         $setting->save();

-        $this->clearFromCache($key);
+        $this->putValueIntoLocalCache($key, $value);

 
         return true;
     }
 
         return true;
     }


@@ -209,7 +220,7 @@ class SettingService
      * Can only take string value types since this may use
      * the session which is less flexible to data types.
      */
      * Can only take string value types since this may use
      * the session which is less flexible to data types.
      */

-    public function putForCurrentUser(string $key, string $value)
+    public function putForCurrentUser(string $key, string $value): bool

     {
         return $this->putUser(user(), $key, $value);
     }
     {
         return $this->putUser(user(), $key, $value);
     }


@@ -231,15 +242,19 @@ class SettingService
         if ($setting) {
             $setting->delete();
         }
         if ($setting) {
             $setting->delete();
         }

-        $this->clearFromCache($key);
+
+        $cacheCategory = $this->localCacheCategory($key);
+        if (isset($this->localCache[$cacheCategory])) {
+            unset($this->localCache[$cacheCategory][$key]);
+        }

     }
 
     /**
      * Delete settings for a given user id.
      */
     }
 
     /**
      * Delete settings for a given user id.
      */

-    public function deleteUserSettings(string $userId)
+    public function deleteUserSettings(string $userId): void

     {
     {

-        return $this->setting->newQuery()
+        Setting::query()

             ->where('setting_key', 'like', $this->userKey($userId) . '%')
             ->delete();
     }
             ->where('setting_key', 'like', $this->userKey($userId) . '%')
             ->delete();
     }


@@ -249,7 +264,16 @@ class SettingService
      */
     protected function getSettingObjectByKey(string $key): ?Setting
     {
      */
     protected function getSettingObjectByKey(string $key): ?Setting
     {

-        return $this->setting->newQuery()
-            ->where('setting_key', '=', $key)->first();
+        return Setting::query()
+            ->where('setting_key', '=', $key)
+            ->first();
+    }
+
+    /**
+     * Empty the local setting value cache used by this service.
+     */
+    public function flushCache(): void
+    {
+        $this->localCache = [];

     }
 }
     }
 }

diff --git a/tests/Commands/UpdateUrlCommandTest.php b/tests/Commands/UpdateUrlCommandTest.php
index c07a8031248e6c990f83c2ca83149907b5c500d3..1788e9452a697983f2ecfa4feb500f58b6f69b73 100644 (file)
--- a/tests/Commands/UpdateUrlCommandTest.php
+++ b/tests/Commands/UpdateUrlCommandTest.php
@@ -39,6 +39,8 @@ class UpdateUrlCommandTest extends TestCase
         setting()->put('my-custom-item', 'https://example.com/donkey/cat');
         $this->runUpdate('https://example.com', 'https://cats.example.com');
 
         setting()->put('my-custom-item', 'https://example.com/donkey/cat');
         $this->runUpdate('https://example.com', 'https://cats.example.com');
 

+        setting()->flushCache();
+

         $settingVal = setting('my-custom-item');
         $this->assertEquals('https://cats.example.com/donkey/cat', $settingVal);
     }
         $settingVal = setting('my-custom-item');
         $this->assertEquals('https://cats.example.com/donkey/cat', $settingVal);
     }


@@ -47,6 +49,9 @@ class UpdateUrlCommandTest extends TestCase
     {
         setting()->put('my-custom-array-item', [['name' => 'a https://example.com/donkey/cat url']]);
         $this->runUpdate('https://example.com', 'https://cats.example.com');
     {
         setting()->put('my-custom-array-item', [['name' => 'a https://example.com/donkey/cat url']]);
         $this->runUpdate('https://example.com', 'https://cats.example.com');

+
+        setting()->flushCache();
+

         $settingVal = setting('my-custom-array-item');
         $this->assertEquals('a https://cats.example.com/donkey/cat url', $settingVal[0]['name']);
     }
         $settingVal = setting('my-custom-array-item');
         $this->assertEquals('a https://cats.example.com/donkey/cat url', $settingVal[0]['name']);
     }