Skip to content

Changed the way settings are loaded #4062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ssddanbrown merged 1 commit into from
Feb 23, 2023
Merged

Changed the way settings are loaded #4062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 80 additions & 56 deletions app/Settings/SettingService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,29 @@
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.
* A local array-based cache is used to for setting accesses across a request.
*/
class SettingService
{
protected Setting $setting;
protected Cache $cache;
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.
*/
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 (isset($this->localCache[$key])) {
return $this->localCache[$key];
}

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

return $formatted;
return $this->formatValue($value, $default);
}

/**
Expand Down Expand Up @@ -79,52 +63,78 @@ public function getForCurrentUser(string $key, $default = null)
}

/**
* 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.
*/
protected function formatValue($value, $default)
protected function formatValue(mixed $value, mixed $default): mixed
{
// Change string booleans to actual booleans
if ($value === 'true') {
Expand Down Expand Up @@ -155,21 +165,22 @@ public function has(string $key): bool
* 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->type = 'string';
$setting->value = $value;

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

$setting->value = $value;
$setting->save();
$this->clearFromCache($key);
$this->putValueIntoLocalCache($key, $value);

return true;
}
Expand Down Expand Up @@ -209,7 +220,7 @@ public function putUser(User $user, string $key, string $value): bool
* 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);
}
Expand All @@ -231,15 +242,19 @@ public function remove(string $key): void
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.
*/
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();
}
Expand All @@ -249,7 +264,16 @@ public function deleteUserSettings(string $userId)
*/
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 = [];
}
}
5 changes: 5 additions & 0 deletions tests/Commands/UpdateUrlCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ public function test_command_updates_settings()
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);
}
Expand All @@ -47,6 +49,9 @@ public function test_command_updates_array_settings()
{
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']);
}
Expand Down