From bd755bd036d953895d11530ed3c2d60403a9141b Mon Sep 17 00:00:00 2001 From: MGatner Date: Mon, 15 Nov 2021 00:12:43 +0000 Subject: [PATCH 1/4] Add context --- README.md | 30 +++++++++ .../2021-11-14-143905_AddContextColumn.php | 25 +++++++ src/Handlers/BaseHandler.php | 29 ++++++++- src/Handlers/DatabaseHandler.php | 48 ++++++++++---- src/Settings.php | 39 ++++++----- tests/HelperTest.php | 3 +- tests/SettingsTest.php | 65 +++++++++++++++++++ 7 files changed, 206 insertions(+), 33 deletions(-) create mode 100644 src/Database/Migrations/2021-11-14-143905_AddContextColumn.php diff --git a/README.md b/README.md index df723f3..ef595fd 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,34 @@ it effectively resets itself back to the default value in config file, if any. service('setting')->forget('App.siteName') ``` +### Contextual Settings + +In addition to the default behavior describe above, `Settings` can can be used to define "contextual settings". +A context may be anything you want, but common examples are a runtime environment or an authenticated user. +In order to use a context you pass it as an additional parameter to the `get()`/`set()`/`forget()` methods; if +a context setting is requested and does not exist then the default global value will be used. + +Contexts may be any unique string you choose, but a recommended format for supplying some consistency is to +given them a category and identifier, like `environment:production` or `group:42`. + +An example... Say your App config includes the name of a theme to use to enhance your display. By default +your config file specifies `App.theme = 'default'`. When a user changes their theme, you do not want this to +change the theme for all visitors to the site, so you need to provide the user as the *context* for the change: + +```php + $context = 'user:' . user_id(); + service('setting')->set('App.theme', 'dark', $context); +``` + +Now when your filter is determining which theme to apply it can check for the current user as the context: + +```php + $context = 'user:' . user_id(); + $theme = service('setting')->get('App.theme', $context); +``` + +If that context is not found the library falls back to the global value, i.e. `service('setting')->get('App.theme')` + ### Using the Helper The helper provides a shortcut to the using the service. It must first be loaded using the `helper()` method @@ -100,6 +128,8 @@ setting()->set('App.siteName', 'My Great Site'); setting()->forget('App.siteName'); ``` +> Note: Due to the shorthand nature of the helper function it cannot access contextual settings. + ## Known Limitations The following are known limitations of the library: diff --git a/src/Database/Migrations/2021-11-14-143905_AddContextColumn.php b/src/Database/Migrations/2021-11-14-143905_AddContextColumn.php new file mode 100644 index 0000000..8cdc73d --- /dev/null +++ b/src/Database/Migrations/2021-11-14-143905_AddContextColumn.php @@ -0,0 +1,25 @@ +forge->addColumn(config('Settings')->database['table'], [ + 'context' => [ + 'type' => 'varchar', + 'constraint' => 255, + 'null' => true, + 'after' => 'type', + ], + ]); + } + + public function down() + { + $this->forge->dropColumn(config('Settings')->database['table'], 'context'); + } +} diff --git a/src/Handlers/BaseHandler.php b/src/Handlers/BaseHandler.php index a02d250..53177e2 100644 --- a/src/Handlers/BaseHandler.php +++ b/src/Handlers/BaseHandler.php @@ -2,14 +2,21 @@ namespace Sparks\Settings\Handlers; +use RuntimeException; + abstract class BaseHandler { + /** + * Checks whether this handler has a value set. + */ + abstract public function has(string $class, string $property, ?string $context = null): bool; + /** * Returns a single value from the handler, if stored. * * @return mixed */ - abstract public function get(string $class, string $property); + abstract public function get(string $class, string $property, ?string $context = null); /** * If the Handler supports saving values, it @@ -18,11 +25,27 @@ abstract public function get(string $class, string $property); * * @param mixed $value * + * @throws RuntimeException + * + * @return mixed + */ + public function set(string $class, string $property, $value = null, ?string $context = null) + { + throw new RuntimeException('Set method not implemented for current Settings handler.'); + } + + /** + * If the Handler supports forgetting values, it + * MUST override this method to provide that functionality. + * Not all Handlers will support writing values. + * + * @throws RuntimeException + * * @return mixed */ - public function set(string $class, string $property, $value = null) + public function forget(string $class, string $property, ?string $context = null) { - throw new \RuntimeException('Set method not implemented for current Settings handler.'); + throw new RuntimeException('Forget method not implemented for current Settings handler.'); } /** diff --git a/src/Handlers/DatabaseHandler.php b/src/Handlers/DatabaseHandler.php index 6707436..7ea66ed 100644 --- a/src/Handlers/DatabaseHandler.php +++ b/src/Handlers/DatabaseHandler.php @@ -3,6 +3,7 @@ namespace Sparks\Settings\Handlers; use CodeIgniter\I18n\Time; +use RuntimeException; /** * Provides database storage for Settings. @@ -34,6 +35,20 @@ class DatabaseHandler extends BaseHandler */ private $table; + /** + * Checks whether this handler has a value set. + */ + public function has(string $class, string $property, ?string $context = null): bool + { + $this->hydrate(); + + if (! isset($this->settings[$class][$property])) { + return false; + } + + return array_key_exists($context ?? 0, $this->settings[$class][$property]); + } + /** * Attempt to retrieve a value from the database. * To boost performance, all of the values are @@ -42,15 +57,13 @@ class DatabaseHandler extends BaseHandler * * @return mixed|null */ - public function get(string $class, string $property) + public function get(string $class, string $property, ?string $context = null) { - $this->hydrate(); - - if (! isset($this->settings[$class]) || ! isset($this->settings[$class][$property])) { + if (! $this->has($class, $property, $context)) { return null; } - return $this->parseValue(...$this->settings[$class][$property]); + return $this->parseValue(...$this->settings[$class][$property][$context ?? 0]); } /** @@ -60,7 +73,7 @@ public function get(string $class, string $property) * * @return mixed|void */ - public function set(string $class, string $property, $value = null) + public function set(string $class, string $property, $value = null, ?string $context = null) { $this->hydrate(); $time = Time::now()->format('Y-m-d H:i:s'); @@ -68,13 +81,14 @@ public function set(string $class, string $property, $value = null) $value = $this->prepareValue($value); // If we found it in our cache, then we need to update - if (isset($this->settings[$class][$property])) { + if (isset($this->settings[$class][$property][$context ?? 0])) { $result = db_connect()->table($this->table) ->where('class', $class) ->where('key', $property) ->update([ 'value' => $value, 'type' => $type, + 'context' => $context, 'updated_at' => $time, ]); } else { @@ -84,6 +98,7 @@ public function set(string $class, string $property, $value = null) 'key' => $property, 'value' => $value, 'type' => $type, + 'context' => $context, 'created_at' => $time, 'updated_at' => $time, ]); @@ -94,8 +109,11 @@ public function set(string $class, string $property, $value = null) if (! array_key_exists($class, $this->settings)) { $this->settings[$class] = []; } + if (! array_key_exists($property, $this->settings[$class])) { + $this->settings[$class][$property] = []; + } - $this->settings[$class][$property] = [ + $this->settings[$class][$property][$context ?? 0] = [ $value, $type, ]; @@ -108,7 +126,7 @@ public function set(string $class, string $property, $value = null) * Deletes the record from persistent storage, if found, * and from the local cache. */ - public function forget(string $class, string $property) + public function forget(string $class, string $property, ?string $context = null) { $this->hydrate(); @@ -116,6 +134,7 @@ public function forget(string $class, string $property) $result = db_connect()->table($this->table) ->where('class', $class) ->where('key', $property) + ->where('context', $context) ->delete(); if (! $result) { @@ -123,13 +142,15 @@ public function forget(string $class, string $property) } // Delete from local storage - unset($this->settings[$class][$property]); + unset($this->settings[$class][$property][$context ?? 0]); return $result; } /** * Ensures we've pulled all of the values from the database. + * + * @throws RuntimeException */ private function hydrate() { @@ -142,7 +163,7 @@ private function hydrate() $rawValues = db_connect()->table($this->table)->get(); if (is_bool($rawValues)) { - throw new \RuntimeException(db_connect()->error()['message'] ?? 'Error reading from database.'); + throw new RuntimeException(db_connect()->error()['message'] ?? 'Error reading from database.'); } $rawValues = $rawValues->getResultObject(); @@ -151,8 +172,11 @@ private function hydrate() if (! array_key_exists($row->class, $this->settings)) { $this->settings[$row->class] = []; } + if (! array_key_exists($row->key, $this->settings[$row->class])) { + $this->settings[$row->class][$row->key] = []; + } - $this->settings[$row->class][$row->key] = [ + $this->settings[$row->class][$row->key][$row->context ?? 0] = [ $row->value, $row->type, ]; diff --git a/src/Settings.php b/src/Settings.php index 806caf5..013c1c5 100644 --- a/src/Settings.php +++ b/src/Settings.php @@ -2,6 +2,8 @@ namespace Sparks\Settings; +use InvalidArgumentException; +use RuntimeException; use Sparks\Settings\Config\Settings as SettingsConfig; /** @@ -52,23 +54,26 @@ public function __construct(?SettingsConfig $config = null) } /** - * Retrieve a value from either the database + * Retrieve a value from any handler * or from a config file matching the name * file.arg.optionalArg */ - public function get(string $key) + public function get(string $key, ?string $context = null) { [$class, $property, $config] = $this->prepareClassAndProperty($key); - // Try grabbing the values from any of our handlers + // Check each of our handlers foreach ($this->handlers as $name => $handler) { - $value = $handler->get($class, $property); - - if ($value !== null) { - return $value; + if ($handler->has($class, $property, $context)) { + return $handler->get($class, $property, $context); } } + // If no contextual value was found then fall back on a global + if ($context !== null) { + return $this->get($key); + } + return $config->{$property} ?? null; } @@ -79,13 +84,11 @@ public function get(string $key) * * @return void|null */ - public function set(string $key, $value = null) + public function set(string $key, $value = null, ?string $context = null) { [$class, $property] = $this->prepareClassAndProperty($key); - $handler = $this->getWriteHandler(); - - return $handler->set($class, $property, $value); + return $this->getWriteHandler()->set($class, $property, $value, $context); } /** @@ -93,24 +96,24 @@ public function set(string $key, $value = null) * effectively returning the value to the default value * found in the config file, if any. */ - public function forget(string $key) + public function forget(string $key, ?string $context = null) { [$class, $property] = $this->prepareClassAndProperty($key); - $handler = $this->getWriteHandler(); - - return $handler->forget($class, $property); + return $this->getWriteHandler()->forget($class, $property, $context); } /** * Returns the handler that is set to store values. * + * @throws RuntimeException + * * @return mixed */ private function getWriteHandler() { if (empty($this->writeHandler) || ! isset($this->handlers[$this->writeHandler])) { - throw new \RuntimeException('Unable to find a Settings handler that can store values.'); + throw new RuntimeException('Unable to find a Settings handler that can store values.'); } return $this->handlers[$this->writeHandler]; @@ -119,6 +122,8 @@ private function getWriteHandler() /** * Analyzes the given key and breaks it into the class.field parts. * + * @throws InvalidArgumentException + * * @return string[] */ private function parseDotSyntax(string $key): array @@ -127,7 +132,7 @@ private function parseDotSyntax(string $key): array $parts = explode('.', $key); if (count($parts) === 1) { - throw new \RuntimeException('$field must contain both the class and field name, i.e. Foo.bar'); + throw new InvalidArgumentException('$key must contain both the class and field name, i.e. Foo.bar'); } return $parts; diff --git a/tests/HelperTest.php b/tests/HelperTest.php index f1e19aa..5229676 100644 --- a/tests/HelperTest.php +++ b/tests/HelperTest.php @@ -28,7 +28,8 @@ public function testReturnsServiceByDefault() public function testThrowsExceptionWithInvalidField() { - $this->expectException(\RuntimeException::class); + $this->expectException('InvalidArgumentException'); + $this->expectExceptionMessage('$key must contain both the class and field name, i.e. Foo.bar'); setting('Foobar'); } diff --git a/tests/SettingsTest.php b/tests/SettingsTest.php index e017aeb..c41f3db 100644 --- a/tests/SettingsTest.php +++ b/tests/SettingsTest.php @@ -90,6 +90,23 @@ public function testSetInsertsBoolFalse() $this->assertFalse($settings->get('Test.siteName')); } + public function testSetInsertsNull() + { + $settings = new Settings(); + + $results = $settings->set('Test.siteName', null); + + $this->assertTrue($results); + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Test', + 'key' => 'siteName', + 'value' => null, + 'type' => 'NULL', + ]); + + $this->assertNull($settings->get('Test.siteName')); + } + public function testSetInsertsArray() { $settings = new Settings(); @@ -193,4 +210,52 @@ public function testForgetWithNoStoredRecord() $this->assertTrue($results); } + + public function testSetWithContext() + { + $settings = new Settings(); + + $results = $settings->set('Test.siteName', 'Banana', 'environment:test'); + + $this->assertTrue($results); + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Test', + 'key' => 'siteName', + 'value' => 'Banana', + 'type' => 'string', + 'context' => 'environment:test', + ]); + } + + public function testGetWithContext() + { + $settings = new Settings(); + + $settings->set('Test.siteName', 'NoContext'); + $settings->set('Test.siteName', 'YesContext', 'testing:true'); + + $this->assertSame('NoContext', $settings->get('Test.siteName')); + $this->assertSame('YesContext', $settings->get('Test.siteName', 'testing:true')); + } + + public function testGetWithoutContextUsesGlobal() + { + $settings = new Settings(); + + $settings->set('Test.siteName', 'NoContext'); + + $this->assertSame('NoContext', $settings->get('Test.siteName', 'testing:true')); + } + + public function testForgetWithContext() + { + $settings = new Settings(); + + $settings->set('Test.siteName', 'Bar'); + $settings->set('Test.siteName', 'Amnesia', 'category:disease'); + + $settings->forget('Test.siteName', 'category:disease'); + + $this->assertSame('Bar', $settings->get('Test.siteName', 'category:disease')); + } } From fdcffc36ae30181e36f2c2483cb6380a0c0ada29 Mon Sep 17 00:00:00 2001 From: MGatner Date: Mon, 15 Nov 2021 14:06:24 +0000 Subject: [PATCH 2/4] Implement docs suggestions --- README.md | 17 ++++++++++------- UPGRADING.md | 6 ++++++ 2 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 UPGRADING.md diff --git a/README.md b/README.md index ef595fd..9fbdd6a 100644 --- a/README.md +++ b/README.md @@ -85,28 +85,31 @@ service('setting')->forget('App.siteName') In addition to the default behavior describe above, `Settings` can can be used to define "contextual settings". A context may be anything you want, but common examples are a runtime environment or an authenticated user. In order to use a context you pass it as an additional parameter to the `get()`/`set()`/`forget()` methods; if -a context setting is requested and does not exist then the default global value will be used. +a context setting is requested and does not exist then the general value will be used. Contexts may be any unique string you choose, but a recommended format for supplying some consistency is to -given them a category and identifier, like `environment:production` or `group:42`. +give them a category and identifier, like `environment:production` or `group:42`. An example... Say your App config includes the name of a theme to use to enhance your display. By default your config file specifies `App.theme = 'default'`. When a user changes their theme, you do not want this to change the theme for all visitors to the site, so you need to provide the user as the *context* for the change: ```php - $context = 'user:' . user_id(); - service('setting')->set('App.theme', 'dark', $context); +$context = 'user:' . user_id(); +service('setting')->set('App.theme', 'dark', $context); ``` Now when your filter is determining which theme to apply it can check for the current user as the context: ```php - $context = 'user:' . user_id(); - $theme = service('setting')->get('App.theme', $context); +$context = 'user:' . user_id(); +$theme = service('setting')->get('App.theme', $context); + +// or using the helper +setting()->get('App.theme', $context); ``` -If that context is not found the library falls back to the global value, i.e. `service('setting')->get('App.theme')` +If that context is not found the library falls back to the general value, i.e. `service('setting')->get('App.theme')` ### Using the Helper diff --git a/UPGRADING.md b/UPGRADING.md new file mode 100644 index 0000000..19a8741 --- /dev/null +++ b/UPGRADING.md @@ -0,0 +1,6 @@ +# Upgrade Guide + +## Version 1 to 2 +*** + +* Due to the addition of contexts the `BaseHandler` abstract class was changed. Update any handlers that extend this class to include the new and changed methods. From 223ac2aa11f29f20bc421789ee96800e7ccead12 Mon Sep 17 00:00:00 2001 From: MGatner Date: Mon, 15 Nov 2021 15:09:06 +0000 Subject: [PATCH 3/4] Refactor hydration to be context-specific --- src/Handlers/DatabaseHandler.php | 153 +++++++++++++++++++------------ src/Settings.php | 2 +- 2 files changed, 95 insertions(+), 60 deletions(-) diff --git a/src/Handlers/DatabaseHandler.php b/src/Handlers/DatabaseHandler.php index 7ea66ed..5279c8d 100644 --- a/src/Handlers/DatabaseHandler.php +++ b/src/Handlers/DatabaseHandler.php @@ -7,53 +7,64 @@ /** * Provides database storage for Settings. + * Uses local storage to minimize database calls. */ class DatabaseHandler extends BaseHandler { /** - * Stores our cached settings retrieved - * from the database on the first get() call - * to reduce the number of database calls - * at the expense of a little bit of memory. + * The database table to use. * - * @var array + * @var string */ - private $settings = []; + private $table; /** - * Have the settings been read and cached - * from the database yet? + * Storage for cached general settings. + * Format: ['class' => ['property' => ['value', 'type']]] * - * @var bool + * @var array>|null Will be null until hydrated */ - private $hydrated = false; + private $general; /** - * The settings table + * Storage for cached context settings. + * Format: ['context' => ['class' => ['property' => ['value', 'type']]]] * - * @var string + * @var array */ - private $table; + private $contexts = []; + + /** + * Stores the configured database table. + */ + public function __construct() + { + $this->table = config('Settings')->database['table'] ?? 'settings'; + } /** * Checks whether this handler has a value set. */ public function has(string $class, string $property, ?string $context = null): bool { - $this->hydrate(); + $this->hydrate($context); - if (! isset($this->settings[$class][$property])) { - return false; + if ($context === null) { + return isset($this->general[$class]) + ? array_key_exists($property, $this->general[$class]) + : false; } - return array_key_exists($context ?? 0, $this->settings[$class][$property]); + return isset($this->contexts[$context][$class]) + ? array_key_exists($property, $this->contexts[$context][$class]) + : false; } /** * Attempt to retrieve a value from the database. * To boost performance, all of the values are - * read and stored in $this->settings the first - * time, and then used from there the rest of the request. + * read and stored the first call for each contexts + * and then retrieved from storage. * * @return mixed|null */ @@ -63,7 +74,9 @@ public function get(string $class, string $property, ?string $context = null) return null; } - return $this->parseValue(...$this->settings[$class][$property][$context ?? 0]); + return $context === null + ? $this->parseValue(...$this->general[$class][$property]) + : $this->parseValue(...$this->contexts[$context][$class][$property]); } /** @@ -71,17 +84,18 @@ public function get(string $class, string $property, ?string $context = null) * * @param mixed $value * + * @throws RuntimeException For database failures + * * @return mixed|void */ public function set(string $class, string $property, $value = null, ?string $context = null) { - $this->hydrate(); $time = Time::now()->format('Y-m-d H:i:s'); $type = gettype($value); $value = $this->prepareValue($value); - // If we found it in our cache, then we need to update - if (isset($this->settings[$class][$property][$context ?? 0])) { + // If it was stored then we need to update + if ($this->has($class, $property, $context)) { $result = db_connect()->table($this->table) ->where('class', $class) ->where('key', $property) @@ -91,6 +105,7 @@ public function set(string $class, string $property, $value = null, ?string $con 'context' => $context, 'updated_at' => $time, ]); + // ...otherwise insert it } else { $result = db_connect()->table($this->table) ->insert([ @@ -104,19 +119,21 @@ public function set(string $class, string $property, $value = null, ?string $con ]); } - // Update our cache + // Update storage if ($result === true) { - if (! array_key_exists($class, $this->settings)) { - $this->settings[$class] = []; + if ($context === null) { + $this->general[$class][$property] = [ + $value, + $type, + ]; + } else { + $this->contexts[$context][$class][$property] = [ + $value, + $type, + ]; } - if (! array_key_exists($property, $this->settings[$class])) { - $this->settings[$class][$property] = []; - } - - $this->settings[$class][$property][$context ?? 0] = [ - $value, - $type, - ]; + } else { + throw new RuntimeException(db_connect()->error()['message'] ?? 'Error writing to the database.'); } return $result; @@ -128,9 +145,9 @@ public function set(string $class, string $property, $value = null, ?string $con */ public function forget(string $class, string $property, ?string $context = null) { - $this->hydrate(); + $this->hydrate($context); - // Delete from persistent storage + // Delete from the database $result = db_connect()->table($this->table) ->where('class', $class) ->where('key', $property) @@ -142,46 +159,64 @@ public function forget(string $class, string $property, ?string $context = null) } // Delete from local storage - unset($this->settings[$class][$property][$context ?? 0]); + if ($context === null) { + unset($this->general[$class][$property]); + } else { + unset($this->contexts[$context][$class][$property]); + } return $result; } /** - * Ensures we've pulled all of the values from the database. + * Fetches values from the database in bulk to minimize calls. + * General is always fetched once, contexts are fetched in their + * entirety for each new request. * - * @throws RuntimeException + * @throws RuntimeException For database failures */ - private function hydrate() + private function hydrate(?string $context) { - if ($this->hydrated) { - return; - } + if ($context === null) { + // Check for completion + if ($this->general !== null) { + return; + } - $this->table = config('Settings')->database['table'] ?? 'settings'; + $this->general = []; + $query = db_connect()->table($this->table)->where('context', null); + } else { + // Check for completion + if (isset($this->contexts[$context])) { + return; + } - $rawValues = db_connect()->table($this->table)->get(); + $query = db_connect()->table($this->table)->where('context', $context); - if (is_bool($rawValues)) { - throw new RuntimeException(db_connect()->error()['message'] ?? 'Error reading from database.'); - } + // If general has not been hydrated we will do that at the same time + if ($this->general === null) { + $this->general = []; + $query->orWhere('context', null); + } - $rawValues = $rawValues->getResultObject(); + $this->contexts[$context] = []; + } - foreach ($rawValues as $row) { - if (! array_key_exists($row->class, $this->settings)) { - $this->settings[$row->class] = []; - } - if (! array_key_exists($row->key, $this->settings[$row->class])) { - $this->settings[$row->class][$row->key] = []; - } + if (is_bool($result = $query->get())) { + throw new RuntimeException(db_connect()->error()['message'] ?? 'Error reading from database.'); + } - $this->settings[$row->class][$row->key][$row->context ?? 0] = [ + foreach ($result->getResultObject() as $row) { + $tuple = [ $row->value, $row->type, ]; - } - $this->hydrated = true; + if ($row->context === null) { + $this->general[$row->class][$row->key] = $tuple; + } else { + $this->contexts[$row->context][$row->class][$row->key] = $tuple; + } + } } } diff --git a/src/Settings.php b/src/Settings.php index 013c1c5..c596ff5 100644 --- a/src/Settings.php +++ b/src/Settings.php @@ -69,7 +69,7 @@ public function get(string $key, ?string $context = null) } } - // If no contextual value was found then fall back on a global + // If no contextual value was found then fall back to general if ($context !== null) { return $this->get($key); } From 3bc96e88628c4367fbdec8dd999636640b94c4c5 Mon Sep 17 00:00:00 2001 From: MGatner Date: Mon, 15 Nov 2021 15:12:26 +0000 Subject: [PATCH 4/4] Make context fallback more explicit --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9fbdd6a..97fef1e 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,9 @@ $theme = service('setting')->get('App.theme', $context); setting()->get('App.theme', $context); ``` -If that context is not found the library falls back to the general value, i.e. `service('setting')->get('App.theme')` +Contexts are a cascading check, so if a context does not match a value it will fall back on general, +i.e. `service('setting')->get('App.theme')`. Return value priority is as follows: +"Setting with a context > Setting without context > Config value > null". ### Using the Helper