From 0726942aa1ce2c8000ecad2513f7a6f6df77000c Mon Sep 17 00:00:00 2001 From: michalsn Date: Sun, 26 Oct 2025 11:26:22 +0100 Subject: [PATCH 1/4] feat: deferred writes --- README.md | 2 +- composer.json | 2 +- docs/configuration.md | 51 ++++++- docs/index.md | 2 +- docs/limitations.md | 18 ++- src/Config/Settings.php | 16 +- .../2025-10-25-194218_AddUniqueKey.php | 35 +++++ src/Handlers/ArrayHandler.php | 79 +++++++++- src/Handlers/BaseHandler.php | 12 ++ src/Handlers/DatabaseHandler.php | 127 +++++++++++++++- src/Handlers/FileHandler.php | 84 ++++++++--- src/Settings.php | 3 + tests/DatabaseHandlerTest.php | 139 ++++++++++++++++++ tests/FileHandlerTest.php | 122 +++++++++++++++ 14 files changed, 631 insertions(+), 61 deletions(-) create mode 100644 src/Database/Migrations/2025-10-25-194218_AddUniqueKey.php diff --git a/README.md b/README.md index 9f7e782..c2c5037 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ config classes for CodeIgniter 4 framework. [![Coverage Status](https://coveralls.io/repos/github/codeigniter4/settings/badge.svg?branch=develop)](https://coveralls.io/github/codeigniter4/settings?branch=develop) ![PHP](https://img.shields.io/badge/PHP-%5E8.1-blue) -![CodeIgniter](https://img.shields.io/badge/CodeIgniter-%5E4.2.3-blue) +![CodeIgniter](https://img.shields.io/badge/CodeIgniter-%5E4.3-blue) ![License](https://img.shields.io/badge/License-MIT-blue) ## Installation diff --git a/composer.json b/composer.json index 79ed4cf..6589f19 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ }, "require-dev": { "codeigniter4/devkit": "^1.3", - "codeigniter4/framework": "^4.2.3" + "codeigniter4/framework": "^4.3.0" }, "minimum-stability": "dev", "prefer-stable": true, diff --git a/docs/configuration.md b/docs/configuration.md index a90effc..05cc03c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -26,6 +26,11 @@ Example: public $handlers = ['database']; ``` +### Deferred writes + +Handlers like `database` and `file` support deferred writes. When `deferWrites` is enabled, multiple `set()` and `forget()` calls +are batched into the minimum number of database calls or file writes. The actual changes happen during the `post_system` event. + ### Multiple handlers Example: @@ -44,6 +49,8 @@ This configuration will: Only handlers marked as `writeable => true` will be used when calling `set()`, `forget()`, or `flush()` methods. +--- + ## DatabaseHandler This handler stores settings in a database table and is production-ready for high-traffic applications. @@ -54,21 +61,36 @@ This handler stores settings in a database table and is production-ready for hig * `table` - The database table name for storing settings. Default: `'settings'` * `group` - The database connection group to use. Default: `null` (uses default connection) * `writeable` - Whether this handler supports write operations. Default: `true` +* `deferWrites` - Whether to defer writes until the end of request (`post_system` event). Default: `false` Example: ```php public $database = [ - 'class' => DatabaseHandler::class, - 'table' => 'settings', - 'group' => null, - 'writeable' => true, + 'class' => DatabaseHandler::class, + 'table' => 'settings', + 'group' => null, + 'writeable' => true, + 'deferWrites' => false, ]; ``` !!! note You need to run migrations to create the settings table: `php spark migrate -n CodeIgniter\\Settings` +**Deferred Writes** + +When `deferWrites` is enabled, multiple `set()` or `forget()` calls are batched into a single database transaction at the end of the request. This significantly reduces database queries: + +```php +// With deferWrites = false: 3 separate queries (INSERT/UPDATE) +$settings->set('Example.prop1', 'value1'); +$settings->set('Example.prop2', 'value2'); +$settings->set('Example.prop3', 'value3'); + +// With deferWrites = true: 1 query updating 3 properties at the end of the request +``` + --- ## FileHandler @@ -80,20 +102,35 @@ This handler stores settings as PHP files and is optimized for production use wi * `class` - The handler class. Default: `FileHandler::class` * `path` - The directory path where settings files are stored. Default: `WRITEPATH . 'settings'` * `writeable` - Whether this handler supports write operations. Default: `true` +* `deferWrites` - Whether to defer writes until the end of request (`post_system` event). Default: `false` Example: ```php public $file = [ - 'class' => FileHandler::class, - 'path' => WRITEPATH . 'settings', - 'writeable' => true, + 'class' => FileHandler::class, + 'path' => WRITEPATH . 'settings', + 'writeable' => true, + 'deferWrites' => false, ]; ``` !!! note The `FileHandler` automatically creates the directory if it doesn't exist and checks write permissions on instantiation. +**Deferred Writes** + +When `deferWrites` is enabled, multiple `set()` or `forget()` calls to the same class are batched into a single file write at the end of the request. This significantly reduces I/O operations: + +```php +// With deferWrites = false: 3 file writes +$settings->set('Example.prop1', 'value1'); +$settings->set('Example.prop2', 'value2'); +$settings->set('Example.prop3', 'value3'); + +// With deferWrites = true: 1 file write at end of request +``` + --- ## ArrayHandler diff --git a/docs/index.md b/docs/index.md index abe0333..2be8412 100644 --- a/docs/index.md +++ b/docs/index.md @@ -30,7 +30,7 @@ service('settings')->forget('App.siteName'); ### Requirements ![PHP](https://img.shields.io/badge/PHP-%5E8.1-red) -![CodeIgniter](https://img.shields.io/badge/CodeIgniter-%5E4.2.3-red) +![CodeIgniter](https://img.shields.io/badge/CodeIgniter-%5E4.3-red) ### Acknowledgements diff --git a/docs/limitations.md b/docs/limitations.md index 5cc6cf3..b467736 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -2,8 +2,16 @@ The following are known limitations of the library: -1. You can currently only store a single setting at a time. While the `DatabaseHandler` and `FileHandler` - uses a local cache to keep performance as high as possible for reads, writes must be done one at a time. -2. You can only access the first level within a property directly. In most config classes this is a non-issue, - since the properties are simple values. Some config files, like the `database` file, contain properties that - are arrays. +1. **Immediate writes (`deferWrites => false`)**: Each setting is written to storage immediately when you call `set()` or `forget()`. + While `DatabaseHandler` and `FileHandler` use an in-memory cache to maintain fast reads, write operations happen one at a time, + which may result in multiple database queries or file writes per request. + +2. **Deferred writes (`deferWrites => true`)**: All settings are batched and written to storage at the end of the request + (during the `post_system` event). This minimizes the number of database queries and file writes, improving performance. + However, this means write operations will not appear in CodeIgniter's Debug Toolbar, since the `post_system` event + executes after the toolbar data is collected. + +3. **First-level property access only**: You can only access the first level of a config property directly. In most config classes + this is not an issue, since properties are simple values. However, some config files (like `Database`) contain properties that + are nested arrays. For example, you cannot directly access `$config->database['default']['hostname']` - you would need to + get the entire `database` property and then access the nested value. diff --git a/src/Config/Settings.php b/src/Config/Settings.php index 61cedc4..face85a 100644 --- a/src/Config/Settings.php +++ b/src/Config/Settings.php @@ -30,18 +30,20 @@ class Settings extends BaseConfig * Database handler settings. */ public $database = [ - 'class' => DatabaseHandler::class, - 'table' => 'settings', - 'group' => null, - 'writeable' => true, + 'class' => DatabaseHandler::class, + 'table' => 'settings', + 'group' => null, + 'writeable' => true, + 'deferWrites' => false, ]; /** * File handler settings. */ public $file = [ - 'class' => FileHandler::class, - 'path' => WRITEPATH . 'settings', - 'writeable' => true, + 'class' => FileHandler::class, + 'path' => WRITEPATH . 'settings', + 'writeable' => true, + 'deferWrites' => false, ]; } diff --git a/src/Database/Migrations/2025-10-25-194218_AddUniqueKey.php b/src/Database/Migrations/2025-10-25-194218_AddUniqueKey.php new file mode 100644 index 0000000..84d807d --- /dev/null +++ b/src/Database/Migrations/2025-10-25-194218_AddUniqueKey.php @@ -0,0 +1,35 @@ +config = config('Settings'); + $this->DBGroup = $this->config->database['group'] ?? null; + + parent::__construct($forge); + } + + public function up() + { + $table = $this->config->database['table']; + + $this->forge->addUniqueKey(['class', 'key', 'context'], 'settings_class_key_context_idx'); + $this->forge->processIndexes($table); + } + + public function down() + { + $table = $this->config->database['table']; + + $this->forge->dropKey($table, 'settings_class_key_context_idx', false); + } +} diff --git a/src/Handlers/ArrayHandler.php b/src/Handlers/ArrayHandler.php index 5b37250..b1a2320 100644 --- a/src/Handlers/ArrayHandler.php +++ b/src/Handlers/ArrayHandler.php @@ -2,6 +2,8 @@ namespace CodeIgniter\Settings\Handlers; +use CodeIgniter\Events\Events; + /** * Array Settings Handler * @@ -15,7 +17,7 @@ class ArrayHandler extends BaseHandler * Storage for general settings. * Format: ['class' => ['property' => ['value', 'type']]] * - * @var array> + * @var array> */ private array $general = []; @@ -23,10 +25,25 @@ class ArrayHandler extends BaseHandler * Storage for context settings. * Format: ['context' => ['class' => ['property' => ['value', 'type']]]] * - * @var array + * @var array>> */ private array $contexts = []; + /** + * Whether to defer writes until the end of request. + * Used by handlers that support deferred writes. + */ + protected bool $deferWrites = false; + + /** + * Array of properties that have been modified but not persisted. + * Used by handlers that support deferred writes. + * Format: ['key' => ['class' => ..., 'property' => ..., 'value' => ..., 'context' => ..., 'delete' => ...]] + * + * @var array + */ + protected array $pendingProperties = []; + public function has(string $class, string $property, ?string $context = null): bool { return $this->hasStored($class, $property, $context); @@ -117,16 +134,62 @@ protected function forgetStored(string $class, string $property, ?string $contex } /** - * Retrieves all stored properties for a specific class and context. + * Marks a property as pending (needs to be persisted). + * Used by handlers that support deferred writes. * - * @return array Format: ['property' => ['value', 'type']] + * @param mixed $value */ - protected function getAllStored(string $class, ?string $context): array + protected function markPending(string $class, string $property, $value, ?string $context, bool $isDelete = false): void { - if ($context === null) { - return $this->general[$class] ?? []; + $key = $class . '::' . $property . ($context === null ? '' : '::' . $context); + $this->pendingProperties[$key] = [ + 'class' => $class, + 'property' => $property, + 'value' => $value, + 'context' => $context, + 'delete' => $isDelete, + ]; + } + + /** + * Groups pending properties by class+context combination. + * Useful for handlers that need to persist changes on a per-class basis. + * Format: ['key' => ['class' => ..., 'context' => ..., 'changes' => [...]]] + * + * @return array}> + */ + protected function getPendingPropertiesGrouped(): array + { + $grouped = []; + + foreach ($this->pendingProperties as $info) { + $key = $info['class'] . ($info['context'] === null ? '' : '::' . $info['context']); + + if (! isset($grouped[$key])) { + $grouped[$key] = [ + 'class' => $info['class'], + 'context' => $info['context'], + 'changes' => [], + ]; + } + + $grouped[$key]['changes'][] = $info; } - return $this->contexts[$context][$class] ?? []; + return $grouped; + } + + /** + * Sets up deferred writes for handlers that support it. + * + * @param bool $enabled Whether deferred writes should be enabled + */ + protected function setupDeferredWrites(bool $enabled): void + { + $this->deferWrites = $enabled; + + if ($this->deferWrites) { + Events::on('post_system', [$this, 'persistPendingProperties']); + } } } diff --git a/src/Handlers/BaseHandler.php b/src/Handlers/BaseHandler.php index d8b01a5..696f854 100644 --- a/src/Handlers/BaseHandler.php +++ b/src/Handlers/BaseHandler.php @@ -62,6 +62,18 @@ public function flush() throw new RuntimeException('Flush method not implemented for current Settings handler.'); } + /** + * All handlers that support deferWrites MUST support this method. + * + * @return void + * + * @throws RuntimeException + */ + public function persistPendingProperties() + { + throw new RuntimeException('PersistPendingProperties method not implemented for current Settings handler.'); + } + /** * Takes care of converting some item types so they can be safely * stored and re-hydrated into the config files. diff --git a/src/Handlers/DatabaseHandler.php b/src/Handlers/DatabaseHandler.php index b3dea4c..0e238c8 100644 --- a/src/Handlers/DatabaseHandler.php +++ b/src/Handlers/DatabaseHandler.php @@ -4,6 +4,7 @@ use CodeIgniter\Database\BaseBuilder; use CodeIgniter\Database\BaseConnection; +use CodeIgniter\Database\Exceptions\DatabaseException; use CodeIgniter\I18n\Time; use CodeIgniter\Settings\Config\Settings; use RuntimeException; @@ -41,6 +42,8 @@ public function __construct() $this->config = config('Settings'); $this->db = db_connect($this->config->database['group']); $this->builder = $this->db->table($this->config->database['table']); + + $this->setupDeferredWrites($this->config->database['deferWrites'] ?? false); } /** @@ -76,6 +79,25 @@ public function get(string $class, string $property, ?string $context = null) * @throws RuntimeException For database failures */ public function set(string $class, string $property, $value = null, ?string $context = null) + { + if ($this->deferWrites) { + $this->markPending($class, $property, $value, $context); + } else { + $this->persist($class, $property, $value, $context); + } + + // Update storage after persistence check + $this->setStored($class, $property, $value, $context); + } + + /** + * Persists a single property to the database. + * + * @param mixed $value + * + * @throws RuntimeException For database failures + */ + private function persist(string $class, string $property, $value, ?string $context): void { $time = Time::now()->format('Y-m-d H:i:s'); $type = gettype($value); @@ -110,9 +132,6 @@ public function set(string $class, string $property, $value = null, ?string $con if ($result !== true) { throw new RuntimeException($this->db->error()['message'] ?? 'Error writing to the database.'); } - - // Update storage - $this->setStored($class, $property, $value, $context); } /** @@ -125,7 +144,23 @@ public function forget(string $class, string $property, ?string $context = null) { $this->hydrate($context); - // Delete from the database + if ($this->deferWrites) { + $this->markPending($class, $property, null, $context, true); + } else { + $this->persistForget($class, $property, $context); + } + + // Delete from local storage + $this->forgetStored($class, $property, $context); + } + + /** + * Deletes a single property from the database. + * + * @throws RuntimeException For database failures + */ + private function persistForget(string $class, string $property, ?string $context): void + { $result = $this->builder ->where('class', $class) ->where('key', $property) @@ -135,9 +170,6 @@ public function forget(string $class, string $property, ?string $context = null) if (! $result) { throw new RuntimeException($this->db->error()['message'] ?? 'Error writing to the database.'); } - - // Delete from local storage - $this->forgetStored($class, $property, $context); } /** @@ -191,4 +223,85 @@ private function hydrate(?string $context): void $this->setStored($row->class, $row->key, $this->parseValue($row->value, $row->type), $row->context); } } + + /** + * Persists all pending properties to the database. + * Called automatically at the end of request via post_system + * event when deferWrites is enabled. + * + * @return void + */ + public function persistPendingProperties() + { + if ($this->pendingProperties === []) { + return; + } + + $time = Time::now()->format('Y-m-d H:i:s'); + + // Separate deletes from upserts + $deletes = []; + $upserts = []; + + foreach ($this->pendingProperties as $info) { + if ($info['delete']) { + $deletes[] = $info; + } else { + $upserts[] = [ + 'class' => $info['class'], + 'key' => $info['property'], + 'value' => $this->prepareValue($info['value']), + 'type' => gettype($info['value']), + 'context' => $info['context'], + 'created_at' => $time, + 'updated_at' => $time, + ]; + } + } + + try { + $this->db->transStart(); + + // Batch upsert all non-delete operations + if ($upserts !== []) { + $this->builder->upsertBatch($upserts); + } + + // Batch delete all delete operations + if ($deletes !== []) { + $this->builder->groupStart(); + + foreach ($deletes as $i => $info) { + if ($i > 0) { + $this->builder->orGroupStart(); + } else { + $this->builder->groupStart(); + } + + $this->builder + ->where('class', $info['class']) + ->where('key', $info['property']) + ->where('context', $info['context']); + + $this->builder->groupEnd(); + } + + $this->builder->groupEnd(); + + $this->builder->delete(); + } + + $this->db->transComplete(); + + if ($this->db->transStatus() === false) { + log_message('error', 'Failed to persist pending properties to database.'); + + return; + } + + $this->pendingProperties = []; + } catch (DatabaseException $e) { + log_message('error', 'Failed to persist pending properties: ' . $e->getMessage()); + } + } } diff --git a/src/Handlers/FileHandler.php b/src/Handlers/FileHandler.php index 2dad4fa..dec21eb 100644 --- a/src/Handlers/FileHandler.php +++ b/src/Handlers/FileHandler.php @@ -41,6 +41,8 @@ public function __construct() if (! is_writable($this->path)) { throw new RuntimeException('Settings directory is not writable: ' . $this->path); } + + $this->setupDeferredWrites($this->config->file['deferWrites'] ?? false); } /** @@ -83,8 +85,16 @@ public function set(string $class, string $property, $value = null, ?string $con // Update in-memory storage first $this->setStored($class, $property, $value, $context); - // Persist to disk with file locking - $this->persist($class, $context); + if ($this->deferWrites) { + $this->markPending($class, $property, $value, $context); + } else { + // For immediate writes, persist only this specific property change + $this->persist($class, $context, [[ + 'property' => $property, + 'value' => $value, + 'delete' => false, + ]]); + } } /** @@ -102,8 +112,16 @@ public function forget(string $class, string $property, ?string $context = null) // Delete from local storage $this->forgetStored($class, $property, $context); - // Persist to disk with file locking - $this->persist($class, $context); + if ($this->deferWrites) { + $this->markPending($class, $property, null, $context, true); + } else { + // For immediate writes, persist only this specific property deletion + $this->persist($class, $context, [[ + 'property' => $property, + 'value' => null, + 'delete' => true, + ]]); + } } /** @@ -219,12 +237,14 @@ private function loadFromFile(string $class, ?string $context): void } /** - * Persists current in-memory settings for a class+context to disk. - * Uses file locking to prevent race conditions during concurrent writes. + * Persists specific property changes to disk. + * Used for both immediate and deferred writes. + * + * @param list $changes Array of property changes to apply * * @throws RuntimeException For file write failures */ - private function persist(string $class, ?string $context): void + private function persist(string $class, ?string $context, array $changes): void { $filePath = $this->getFilePath($class, $context); @@ -261,13 +281,23 @@ private function persist(string $class, ?string $context): void } } - // Merge our changes with current state - $ourData = $this->extractDataForContext($class, $context); - $mergedData = array_merge($currentData, $ourData); + // Apply all pending changes + foreach ($changes as $change) { + if ($change['delete']) { + // Explicitly delete this property + unset($currentData[$change['property']]); + } else { + // Set or update this property + $currentData[$change['property']] = [ + 'value' => $change['value'], + 'type' => gettype($change['value']), + ]; + } + } // Generate PHP file content $content = ' + * Persists all pending properties to disk. + * Called automatically at the end of request via post_system + * event when deferWrites is enabled. */ - private function extractDataForContext(string $class, ?string $context): array + public function persistPendingProperties(): void { - $data = []; - $storedData = $this->getAllStored($class, $context); - - foreach ($storedData as $property => $valueData) { - $data[$property] = [ - 'value' => $valueData[0], - 'type' => $valueData[1], - ]; + if ($this->pendingProperties === []) { + return; + } + + // Group pending properties by class+context using parent helper + $grouped = $this->getPendingPropertiesGrouped(); + + // Persist each class+context group + foreach ($grouped as $group) { + try { + $this->persist($group['class'], $group['context'], $group['changes']); + } catch (RuntimeException $e) { + log_message('error', 'Failed to persist pending properties for ' . $group['class'] . ': ' . $e->getMessage()); + } } - return $data; + $this->pendingProperties = []; } /** diff --git a/src/Settings.php b/src/Settings.php index cd4e577..0e0d62c 100644 --- a/src/Settings.php +++ b/src/Settings.php @@ -2,6 +2,7 @@ namespace CodeIgniter\Settings; +use CodeIgniter\Config\BaseConfig; use CodeIgniter\Settings\Config\Settings as SettingsConfig; use CodeIgniter\Settings\Handlers\BaseHandler; use InvalidArgumentException; @@ -160,6 +161,8 @@ private function parseDotSyntax(string $key): array /** * Given a key in class.property syntax, will split the values * and determine the fully qualified class name, if possible. + * + * @return array{string, string, BaseConfig|null} */ private function prepareClassAndProperty(string $key): array { diff --git a/tests/DatabaseHandlerTest.php b/tests/DatabaseHandlerTest.php index 7e77682..5080232 100644 --- a/tests/DatabaseHandlerTest.php +++ b/tests/DatabaseHandlerTest.php @@ -6,6 +6,7 @@ use CodeIgniter\Settings\Settings; use CodeIgniter\Test\DatabaseTestTrait; use InvalidArgumentException; +use ReflectionClass; use Tests\Support\TestCase; /** @@ -288,4 +289,142 @@ public function testSetUpdatesContextOnly() 'context' => 'context:male', ]); } + + public function testDeferredWritesReducesDatabaseQueries() + { + // Create new settings instance with deferred writes enabled + /** @var \CodeIgniter\Settings\Config\Settings $config */ + $config = config('Settings'); + $config->handlers = ['database']; + $config->database['deferWrites'] = true; + $deferredSettings = new Settings($config); + + // Multiple set calls to same class + $deferredSettings->set('Example.siteName', 'Value1'); + $deferredSettings->set('Example.siteEmail', 'test@example.com'); + $deferredSettings->set('Example.siteTitle', 'Value3'); + + // Database should NOT have the rows yet (writes are deferred) + $this->dontSeeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + ]); + $this->dontSeeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteEmail', + ]); + $this->dontSeeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteTitle', + ]); + + // Trigger the deferred write manually + $reflection = new ReflectionClass($deferredSettings); + $handlersProperty = $reflection->getProperty('handlers'); + $handlers = $handlersProperty->getValue($deferredSettings); + $handlers['database']->persistPendingProperties(); + + // Now all rows should exist in database + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + 'value' => 'Value1', + 'type' => 'string', + ]); + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteEmail', + 'value' => 'test@example.com', + 'type' => 'string', + ]); + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteTitle', + 'value' => 'Value3', + 'type' => 'string', + ]); + } + + public function testDeferredWritesForgetDeletesAfterPersist() + { + // First, insert a record to delete + $this->settings->set('Example.siteName', 'InitialValue'); + + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + 'value' => 'InitialValue', + ]); + + // Create new settings instance with deferred writes enabled + /** @var \CodeIgniter\Settings\Config\Settings $config */ + $config = config('Settings'); + $config->handlers = ['database']; + $config->database['deferWrites'] = true; + $deferredSettings = new Settings($config); + + // Call forget + $deferredSettings->forget('Example.siteName'); + + // Database should STILL have the row (delete is deferred) + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + ]); + + // Trigger the deferred write manually + $reflection = new ReflectionClass($deferredSettings); + $handlersProperty = $reflection->getProperty('handlers'); + $handlers = $handlersProperty->getValue($deferredSettings); + $handlers['database']->persistPendingProperties(); + + // Now the row should be deleted + $this->dontSeeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + ]); + } + + public function testDeferredWritesDeleteThenSet() + { + // First, insert a record + $this->settings->set('Example.siteName', 'InitialValue'); + + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + 'value' => 'InitialValue', + ]); + + // Create new settings instance with deferred writes enabled + /** @var \CodeIgniter\Settings\Config\Settings $config */ + $config = config('Settings'); + $config->handlers = ['database']; + $config->database['deferWrites'] = true; + $deferredSettings = new Settings($config); + + // Delete then set + $deferredSettings->forget('Example.siteName'); + $deferredSettings->set('Example.siteName', 'NewValue'); + + // Database should STILL have the old value (writes are deferred) + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + 'value' => 'InitialValue', + ]); + + // Trigger the deferred write manually + $reflection = new ReflectionClass($deferredSettings); + $handlersProperty = $reflection->getProperty('handlers'); + $handlers = $handlersProperty->getValue($deferredSettings); + $handlers['database']->persistPendingProperties(); + + // Now the row should have the new value (set overwrites delete in pending operations) + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + 'value' => 'NewValue', + ]); + } } diff --git a/tests/FileHandlerTest.php b/tests/FileHandlerTest.php index f794b20..e3bed50 100644 --- a/tests/FileHandlerTest.php +++ b/tests/FileHandlerTest.php @@ -431,4 +431,126 @@ public function testMergesChangesFromDifferentProcesses() $this->assertSame('concurrent@example.com', $newSettings->get('Example.siteEmail')); $this->assertSame('Second', $newSettings->get('Example.siteTitle')); } + + public function testDeferredWritesReducesFileWrites() + { + // Create new settings instance with deferred writes enabled + /** @var ConfigSettings $config */ + $config = config('Settings'); + $config->handlers = ['file']; + $config->file['path'] = $this->path; + $config->file['deferWrites'] = true; + $deferredSettings = new Settings($config); + + // Multiple set calls to same class + $deferredSettings->set('Example.siteName', 'Value1'); + $deferredSettings->set('Example.siteEmail', 'test@example.com'); + $deferredSettings->set('Example.siteTitle', 'Value3'); + + // File should NOT exist yet (writes are deferred) + $files = glob($this->path . '*.php', GLOB_NOSORT); + $this->assertEmpty($files); + + // Trigger the deferred write manually + $reflection = new ReflectionClass($deferredSettings); + $handlersProperty = $reflection->getProperty('handlers'); + $handlers = $handlersProperty->getValue($deferredSettings); + $handlers['file']->persistPendingProperties(); + + // Now file should exist with all three properties + $files = glob($this->path . '*.php', GLOB_NOSORT); + $this->assertCount(1, $files); + + $data = include $files[0]; + $this->assertArrayHasKey('siteName', $data); + $this->assertArrayHasKey('siteEmail', $data); + $this->assertArrayHasKey('siteTitle', $data); + + $this->assertSame('Value1', $data['siteName']['value']); + $this->assertSame('test@example.com', $data['siteEmail']['value']); + $this->assertSame('Value3', $data['siteTitle']['value']); + } + + public function testDeferredWritesForgetDeletesAfterPersist() + { + // First, create a file with a value + $this->settings->set('Example.siteName', 'InitialValue'); + + $files = glob($this->path . '*.php', GLOB_NOSORT); + $this->assertCount(1, $files); + $data = include $files[0]; + $this->assertArrayHasKey('siteName', $data); + $this->assertSame('InitialValue', $data['siteName']['value']); + + // Create new settings instance with deferred writes enabled + /** @var ConfigSettings $config */ + $config = config('Settings'); + $config->handlers = ['file']; + $config->file['deferWrites'] = true; + $deferredSettings = new Settings($config); + + // Call forget + $deferredSettings->forget('Example.siteName'); + + // File should STILL exist with the value (delete is deferred) + $files = glob($this->path . '*.php', GLOB_NOSORT); + $this->assertCount(1, $files); + $data = include $files[0]; + $this->assertArrayHasKey('siteName', $data); + + // Trigger the deferred write manually + $reflection = new ReflectionClass($deferredSettings); + $handlersProperty = $reflection->getProperty('handlers'); + $handlers = $handlersProperty->getValue($deferredSettings); + $handlers['file']->persistPendingProperties(); + + // Now the property should be removed from the file + $files = glob($this->path . '*.php', GLOB_NOSORT); + $this->assertCount(1, $files); + $data = include $files[0]; + $this->assertArrayNotHasKey('siteName', $data); + } + + public function testDeferredWritesDeleteThenSet() + { + // First, create a file with a value + $this->settings->set('Example.siteName', 'InitialValue'); + + $files = glob($this->path . '*.php', GLOB_NOSORT); + $this->assertCount(1, $files); + $data = include $files[0]; + $this->assertArrayHasKey('siteName', $data); + $this->assertSame('InitialValue', $data['siteName']['value']); + + // Create new settings instance with deferred writes enabled + /** @var ConfigSettings $config */ + $config = config('Settings'); + $config->handlers = ['file']; + $config->file['deferWrites'] = true; + $deferredSettings = new Settings($config); + + // Delete then set + $deferredSettings->forget('Example.siteName'); + $deferredSettings->set('Example.siteName', 'NewValue'); + + // File should STILL have the old value (writes are deferred) + $files = glob($this->path . '*.php', GLOB_NOSORT); + $this->assertCount(1, $files); + $data = include $files[0]; + $this->assertArrayHasKey('siteName', $data); + $this->assertSame('InitialValue', $data['siteName']['value']); + + // Trigger the deferred write manually + $reflection = new ReflectionClass($deferredSettings); + $handlersProperty = $reflection->getProperty('handlers'); + $handlers = $handlersProperty->getValue($deferredSettings); + $handlers['file']->persistPendingProperties(); + + // Now the file should have the new value + $files = glob($this->path . '*.php', GLOB_NOSORT); + $this->assertCount(1, $files); + $data = include $files[0]; + $this->assertArrayHasKey('siteName', $data); + $this->assertSame('NewValue', $data['siteName']['value']); + } } From c9f89a7a7517a41e3da03ac69f081d4aea31ec0e Mon Sep 17 00:00:00 2001 From: michalsn Date: Wed, 29 Oct 2025 18:31:42 +0100 Subject: [PATCH 2/4] fix deferred writes for DatabaseHandler --- README.md | 2 +- composer.json | 2 +- docs/configuration.md | 17 +- docs/index.md | 2 +- docs/limitations.md | 11 +- .../2025-10-25-194218_AddUniqueKey.php | 35 ---- src/Handlers/DatabaseHandler.php | 96 ++++++++--- tests/DatabaseHandlerTest.php | 157 +++++++++++++++--- tests/FileHandlerTest.php | 47 ++++-- 9 files changed, 256 insertions(+), 113 deletions(-) delete mode 100644 src/Database/Migrations/2025-10-25-194218_AddUniqueKey.php diff --git a/README.md b/README.md index c2c5037..9f7e782 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ config classes for CodeIgniter 4 framework. [![Coverage Status](https://coveralls.io/repos/github/codeigniter4/settings/badge.svg?branch=develop)](https://coveralls.io/github/codeigniter4/settings?branch=develop) ![PHP](https://img.shields.io/badge/PHP-%5E8.1-blue) -![CodeIgniter](https://img.shields.io/badge/CodeIgniter-%5E4.3-blue) +![CodeIgniter](https://img.shields.io/badge/CodeIgniter-%5E4.2.3-blue) ![License](https://img.shields.io/badge/License-MIT-blue) ## Installation diff --git a/composer.json b/composer.json index 6589f19..79ed4cf 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ }, "require-dev": { "codeigniter4/devkit": "^1.3", - "codeigniter4/framework": "^4.3.0" + "codeigniter4/framework": "^4.2.3" }, "minimum-stability": "dev", "prefer-stable": true, diff --git a/docs/configuration.md b/docs/configuration.md index 05cc03c..22e0528 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -29,7 +29,8 @@ public $handlers = ['database']; ### Deferred writes Handlers like `database` and `file` support deferred writes. When `deferWrites` is enabled, multiple `set()` and `forget()` calls -are batched into the minimum number of database calls or file writes. The actual changes happen during the `post_system` event. +are batched and persisted efficiently at the end of the request during the `post_system` event. This minimizes the number of +database queries or file I/O operations, improving performance for write-heavy operations. ### Multiple handlers @@ -80,17 +81,19 @@ public $database = [ **Deferred Writes** -When `deferWrites` is enabled, multiple `set()` or `forget()` calls are batched into a single database transaction at the end of the request. This significantly reduces database queries: +When `deferWrites` is enabled, multiple `set()` or `forget()` calls are batched and persisted in a single transaction at the end of the request. This significantly reduces database queries: ```php -// With deferWrites = false: 3 separate queries (INSERT/UPDATE) +// With deferWrites = false: 1 SELECT (hydrates all settings for context) + 3 separate INSERT/UPDATE queries $settings->set('Example.prop1', 'value1'); $settings->set('Example.prop2', 'value2'); $settings->set('Example.prop3', 'value3'); -// With deferWrites = true: 1 query updating 3 properties at the end of the request +// With deferWrites = true: 1 SELECT + 1 INSERT batch + 1 UPDATE batch (all batched at end of request) ``` +The deferred approach is especially beneficial when updating existing records or performing many operations in a single request. + --- ## FileHandler @@ -123,14 +126,16 @@ public $file = [ When `deferWrites` is enabled, multiple `set()` or `forget()` calls to the same class are batched into a single file write at the end of the request. This significantly reduces I/O operations: ```php -// With deferWrites = false: 3 file writes +// With deferWrites = false: 1 file read (hydrates all settings for class) + 3 separate file writes $settings->set('Example.prop1', 'value1'); $settings->set('Example.prop2', 'value2'); $settings->set('Example.prop3', 'value3'); -// With deferWrites = true: 1 file write at end of request +// With deferWrites = true: 1 file read + 1 file write (all changes batched at end of request) ``` +The deferred approach is especially beneficial when updating multiple properties in the same class. + --- ## ArrayHandler diff --git a/docs/index.md b/docs/index.md index 2be8412..abe0333 100644 --- a/docs/index.md +++ b/docs/index.md @@ -30,7 +30,7 @@ service('settings')->forget('App.siteName'); ### Requirements ![PHP](https://img.shields.io/badge/PHP-%5E8.1-red) -![CodeIgniter](https://img.shields.io/badge/CodeIgniter-%5E4.3-red) +![CodeIgniter](https://img.shields.io/badge/CodeIgniter-%5E4.2.3-red) ### Acknowledgements diff --git a/docs/limitations.md b/docs/limitations.md index b467736..9822665 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -3,13 +3,16 @@ The following are known limitations of the library: 1. **Immediate writes (`deferWrites => false`)**: Each setting is written to storage immediately when you call `set()` or `forget()`. - While `DatabaseHandler` and `FileHandler` use an in-memory cache to maintain fast reads, write operations happen one at a time, - which may result in multiple database queries or file writes per request. + The first operation hydrates all settings for that context (1 SELECT query), then each subsequent write performs a separate + INSERT or UPDATE. While `DatabaseHandler` and `FileHandler` use an in-memory cache to maintain fast reads, individual write + operations may result in multiple database queries or file writes per request. 2. **Deferred writes (`deferWrites => true`)**: All settings are batched and written to storage at the end of the request (during the `post_system` event). This minimizes the number of database queries and file writes, improving performance. - However, this means write operations will not appear in CodeIgniter's Debug Toolbar, since the `post_system` event - executes after the toolbar data is collected. + However, there are important considerations: + - Write operations will not appear in CodeIgniter's Debug Toolbar, since the `post_system` event executes after toolbar data collection. + - If the request terminates early (fatal error, `exit()`, etc.) before `post_system`, pending writes are lost. + - Write failures are logged but handled silently - no exceptions are thrown back to the calling code. 3. **First-level property access only**: You can only access the first level of a config property directly. In most config classes this is not an issue, since properties are simple values. However, some config files (like `Database`) contain properties that diff --git a/src/Database/Migrations/2025-10-25-194218_AddUniqueKey.php b/src/Database/Migrations/2025-10-25-194218_AddUniqueKey.php deleted file mode 100644 index 84d807d..0000000 --- a/src/Database/Migrations/2025-10-25-194218_AddUniqueKey.php +++ /dev/null @@ -1,35 +0,0 @@ -config = config('Settings'); - $this->DBGroup = $this->config->database['group'] ?? null; - - parent::__construct($forge); - } - - public function up() - { - $table = $this->config->database['table']; - - $this->forge->addUniqueKey(['class', 'key', 'context'], 'settings_class_key_context_idx'); - $this->forge->processIndexes($table); - } - - public function down() - { - $table = $this->config->database['table']; - - $this->forge->dropKey($table, 'settings_class_key_context_idx', false); - } -} diff --git a/src/Handlers/DatabaseHandler.php b/src/Handlers/DatabaseHandler.php index 0e238c8..8b97ab7 100644 --- a/src/Handlers/DatabaseHandler.php +++ b/src/Handlers/DatabaseHandler.php @@ -239,14 +239,20 @@ public function persistPendingProperties() $time = Time::now()->format('Y-m-d H:i:s'); - // Separate deletes from upserts + // Separate deletes from upserts and prepare for database operations $deletes = []; $upserts = []; foreach ($this->pendingProperties as $info) { if ($info['delete']) { - $deletes[] = $info; + // Prepare delete row with correct database column names + $deletes[] = [ + 'class' => $info['class'], + 'key' => $info['property'], + 'context' => $info['context'], + ]; } else { + // Prepare upsert row with correct database column names $upserts[] = [ 'class' => $info['class'], 'key' => $info['property'], @@ -262,31 +268,56 @@ public function persistPendingProperties() try { $this->db->transStart(); - // Batch upsert all non-delete operations + // Handle upserts: fetch existing records matching our pending data if ($upserts !== []) { - $this->builder->upsertBatch($upserts); - } + // Build query to fetch only the specific records we need + $this->buildOrWhereConditions($upserts, 'class', 'key', 'context'); - // Batch delete all delete operations - if ($deletes !== []) { - $this->builder->groupStart(); + $existing = $this->builder->get()->getResultArray(); + + // Build a map of existing records for quick lookup + $existingMap = []; + + foreach ($existing as $row) { + $key = $this->buildCompositeKey($row['class'], $row['key'], $row['context']); + $existingMap[$key] = $row['id']; + } - foreach ($deletes as $i => $info) { - if ($i > 0) { - $this->builder->orGroupStart(); + // Separate into inserts and updates + $inserts = []; + $updates = []; + + foreach ($upserts as $row) { + $key = $this->buildCompositeKey($row['class'], $row['key'], $row['context']); + + if (isset($existingMap[$key])) { + // Record exists - prepare for update + $updates[] = [ + 'id' => $existingMap[$key], + 'value' => $row['value'], + 'type' => $row['type'], + 'updated_at' => $row['updated_at'], + ]; } else { - $this->builder->groupStart(); + // New record - prepare for insert + $inserts[] = $row; } + } - $this->builder - ->where('class', $info['class']) - ->where('key', $info['property']) - ->where('context', $info['context']); + // Batch insert new records + if ($inserts !== []) { + $this->builder->insertBatch($inserts); + } - $this->builder->groupEnd(); + // Batch update existing records + if ($updates !== []) { + $this->builder->updateBatch($updates, 'id'); } + } - $this->builder->groupEnd(); + // Batch delete all delete operations + if ($deletes !== []) { + $this->buildOrWhereConditions($deletes, 'class', 'key', 'context'); $this->builder->delete(); } @@ -295,13 +326,38 @@ public function persistPendingProperties() if ($this->db->transStatus() === false) { log_message('error', 'Failed to persist pending properties to database.'); - - return; } $this->pendingProperties = []; } catch (DatabaseException $e) { log_message('error', 'Failed to persist pending properties: ' . $e->getMessage()); + + $this->pendingProperties = []; + } + } + + /** + * Builds a composite key for lookup purposes. + */ + private function buildCompositeKey(string $class, string $key, ?string $context): string + { + return $class . '::' . $key . ($context === null ? '' : '::' . $context); + } + + /** + * Builds OR WHERE conditions for multiple rows. + */ + private function buildOrWhereConditions(array $rows, string $classKey, string $keyKey, string $contextKey): void + { + foreach ($rows as $row) { + $this->builder->orGroupStart(); + + $this->builder + ->where($classKey, $row[$classKey]) + ->where($keyKey, $row[$keyKey]) + ->where($contextKey, $row[$contextKey]); + + $this->builder->groupEnd(); } } } diff --git a/tests/DatabaseHandlerTest.php b/tests/DatabaseHandlerTest.php index 5080232..80be565 100644 --- a/tests/DatabaseHandlerTest.php +++ b/tests/DatabaseHandlerTest.php @@ -45,6 +45,30 @@ protected function setUp(): void $this->group = $config->database['group']; } + /** + * Creates a Settings instance with deferred writes enabled. + */ + private function createDeferredSettings(): Settings + { + /** @var \CodeIgniter\Settings\Config\Settings $config */ + $config = config('Settings'); + $config->handlers = ['database']; + $config->database['deferWrites'] = true; + + return new Settings($config); + } + + /** + * Manually triggers deferred writes for a Settings instance. + */ + private function persistDeferredWrites(Settings $settings): void + { + $reflection = new ReflectionClass($settings); + $handlersProperty = $reflection->getProperty('handlers'); + $handlers = $handlersProperty->getValue($settings); + $handlers['database']->persistPendingProperties(); + } + public function testSetInsertsNewRows() { $this->settings->set('Example.siteName', 'Foo'); @@ -293,11 +317,7 @@ public function testSetUpdatesContextOnly() public function testDeferredWritesReducesDatabaseQueries() { // Create new settings instance with deferred writes enabled - /** @var \CodeIgniter\Settings\Config\Settings $config */ - $config = config('Settings'); - $config->handlers = ['database']; - $config->database['deferWrites'] = true; - $deferredSettings = new Settings($config); + $deferredSettings = $this->createDeferredSettings(); // Multiple set calls to same class $deferredSettings->set('Example.siteName', 'Value1'); @@ -319,10 +339,7 @@ public function testDeferredWritesReducesDatabaseQueries() ]); // Trigger the deferred write manually - $reflection = new ReflectionClass($deferredSettings); - $handlersProperty = $reflection->getProperty('handlers'); - $handlers = $handlersProperty->getValue($deferredSettings); - $handlers['database']->persistPendingProperties(); + $this->persistDeferredWrites($deferredSettings); // Now all rows should exist in database $this->seeInDatabase($this->table, [ @@ -357,11 +374,7 @@ public function testDeferredWritesForgetDeletesAfterPersist() ]); // Create new settings instance with deferred writes enabled - /** @var \CodeIgniter\Settings\Config\Settings $config */ - $config = config('Settings'); - $config->handlers = ['database']; - $config->database['deferWrites'] = true; - $deferredSettings = new Settings($config); + $deferredSettings = $this->createDeferredSettings(); // Call forget $deferredSettings->forget('Example.siteName'); @@ -373,10 +386,7 @@ public function testDeferredWritesForgetDeletesAfterPersist() ]); // Trigger the deferred write manually - $reflection = new ReflectionClass($deferredSettings); - $handlersProperty = $reflection->getProperty('handlers'); - $handlers = $handlersProperty->getValue($deferredSettings); - $handlers['database']->persistPendingProperties(); + $this->persistDeferredWrites($deferredSettings); // Now the row should be deleted $this->dontSeeInDatabase($this->table, [ @@ -397,11 +407,7 @@ public function testDeferredWritesDeleteThenSet() ]); // Create new settings instance with deferred writes enabled - /** @var \CodeIgniter\Settings\Config\Settings $config */ - $config = config('Settings'); - $config->handlers = ['database']; - $config->database['deferWrites'] = true; - $deferredSettings = new Settings($config); + $deferredSettings = $this->createDeferredSettings(); // Delete then set $deferredSettings->forget('Example.siteName'); @@ -415,10 +421,7 @@ public function testDeferredWritesDeleteThenSet() ]); // Trigger the deferred write manually - $reflection = new ReflectionClass($deferredSettings); - $handlersProperty = $reflection->getProperty('handlers'); - $handlers = $handlersProperty->getValue($deferredSettings); - $handlers['database']->persistPendingProperties(); + $this->persistDeferredWrites($deferredSettings); // Now the row should have the new value (set overwrites delete in pending operations) $this->seeInDatabase($this->table, [ @@ -427,4 +430,104 @@ public function testDeferredWritesDeleteThenSet() 'value' => 'NewValue', ]); } + + public function testNoDuplicatesWhenUpdatingExistingRecords() + { + // Pre-populate database with existing records + $this->settings->set('Example.siteName', 'InitialValue1'); + $this->settings->set('Example.siteEmail', 'InitialValue2'); + $this->settings->set('Example.siteTitle', 'InitialValue3'); + + $totalCount = $this->db->table($this->table) + ->where('class', 'Tests\Support\Config\Example') + ->countAllResults(); + + $this->assertSame(3, $totalCount); + + /** @var \CodeIgniter\Settings\Config\Settings $config */ + $config = config('Settings'); + $config->handlers = ['database']; + $config->database['deferWrites'] = true; + $deferredSettings = new Settings($config); + + $deferredSettings->set('Example.siteName', 'UpdatedValue1'); + $deferredSettings->set('Example.siteName', 'UpdatedValue2'); + $deferredSettings->set('Example.siteEmail', 'UpdatedEmail1'); + $deferredSettings->set('Example.siteEmail', 'UpdatedEmail2'); + + // Trigger the deferred write manually + $this->persistDeferredWrites($deferredSettings); + + // Verify no duplicates - should have exactly 3 records total + $totalCount = $this->db->table($this->table) + ->where('class', 'Tests\Support\Config\Example') + ->countAllResults(); + + $this->assertSame(3, $totalCount); + + // Verify each property has exactly 1 record + $siteNameCount = $this->db->table($this->table) + ->where('class', 'Tests\Support\Config\Example') + ->where('key', 'siteName') + ->countAllResults(); + + $this->assertSame(1, $siteNameCount); + + $siteEmailCount = $this->db->table($this->table) + ->where('class', 'Tests\Support\Config\Example') + ->where('key', 'siteEmail') + ->countAllResults(); + + $this->assertSame(1, $siteEmailCount); + + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteName', + 'value' => 'UpdatedValue2', + ]); + + $this->seeInDatabase($this->table, [ + 'class' => 'Tests\Support\Config\Example', + 'key' => 'siteEmail', + 'value' => 'UpdatedEmail2', + ]); + } + + public function testNoDuplicatesWithMixedNewAndExistingRecords() + { + // Pre-populate database with some existing records + $this->settings->set('Example.siteName', 'ExistingValue'); + $this->settings->set('Example.siteEmail', 'existing@example.com'); + + /** @var \CodeIgniter\Settings\Config\Settings $config */ + $config = config('Settings'); + $config->handlers = ['database']; + $config->database['deferWrites'] = true; + $deferredSettings = new Settings($config); + + $deferredSettings->set('Example.siteName', 'UpdatedValue'); // Update existing + $deferredSettings->set('Example.siteTitle', 'NewValue1'); // Create new + $deferredSettings->set('Example.siteEmail', 'new@example.com'); // Update existing + $deferredSettings->set('Example.tagline', 'NewValue2'); // Create new + + // Trigger the deferred write manually + $this->persistDeferredWrites($deferredSettings); + + // Verify no duplicates - should have exactly 4 records total + $totalCount = $this->db->table($this->table) + ->where('class', 'Tests\Support\Config\Example') + ->countAllResults(); + + $this->assertSame(4, $totalCount); + + // Verify each property has exactly 1 record + foreach (['siteName', 'siteEmail', 'siteTitle', 'tagline'] as $key) { + $count = $this->db->table($this->table) + ->where('class', 'Tests\Support\Config\Example') + ->where('key', $key) + ->countAllResults(); + + $this->assertSame(1, $count, "Expected exactly 1 record for key '{$key}'"); + } + } } diff --git a/tests/FileHandlerTest.php b/tests/FileHandlerTest.php index e3bed50..5aab11e 100644 --- a/tests/FileHandlerTest.php +++ b/tests/FileHandlerTest.php @@ -52,6 +52,31 @@ protected function tearDown(): void } } + /** + * Creates a Settings instance with deferred writes enabled. + */ + private function createDeferredSettings(): Settings + { + /** @var ConfigSettings $config */ + $config = config('Settings'); + $config->handlers = ['file']; + $config->file['path'] = $this->path; + $config->file['deferWrites'] = true; + + return new Settings($config); + } + + /** + * Manually triggers deferred writes for a Settings instance. + */ + private function persistDeferredWrites(Settings $settings): void + { + $reflection = new ReflectionClass($settings); + $handlersProperty = $reflection->getProperty('handlers'); + $handlers = $handlersProperty->getValue($settings); + $handlers['file']->persistPendingProperties(); + } + public function testSetCreatesDirectory() { $this->assertDirectoryExists($this->path); @@ -435,12 +460,7 @@ public function testMergesChangesFromDifferentProcesses() public function testDeferredWritesReducesFileWrites() { // Create new settings instance with deferred writes enabled - /** @var ConfigSettings $config */ - $config = config('Settings'); - $config->handlers = ['file']; - $config->file['path'] = $this->path; - $config->file['deferWrites'] = true; - $deferredSettings = new Settings($config); + $deferredSettings = $this->createDeferredSettings(); // Multiple set calls to same class $deferredSettings->set('Example.siteName', 'Value1'); @@ -452,10 +472,7 @@ public function testDeferredWritesReducesFileWrites() $this->assertEmpty($files); // Trigger the deferred write manually - $reflection = new ReflectionClass($deferredSettings); - $handlersProperty = $reflection->getProperty('handlers'); - $handlers = $handlersProperty->getValue($deferredSettings); - $handlers['file']->persistPendingProperties(); + $this->persistDeferredWrites($deferredSettings); // Now file should exist with all three properties $files = glob($this->path . '*.php', GLOB_NOSORT); @@ -499,10 +516,7 @@ public function testDeferredWritesForgetDeletesAfterPersist() $this->assertArrayHasKey('siteName', $data); // Trigger the deferred write manually - $reflection = new ReflectionClass($deferredSettings); - $handlersProperty = $reflection->getProperty('handlers'); - $handlers = $handlersProperty->getValue($deferredSettings); - $handlers['file']->persistPendingProperties(); + $this->persistDeferredWrites($deferredSettings); // Now the property should be removed from the file $files = glob($this->path . '*.php', GLOB_NOSORT); @@ -541,10 +555,7 @@ public function testDeferredWritesDeleteThenSet() $this->assertSame('InitialValue', $data['siteName']['value']); // Trigger the deferred write manually - $reflection = new ReflectionClass($deferredSettings); - $handlersProperty = $reflection->getProperty('handlers'); - $handlers = $handlersProperty->getValue($deferredSettings); - $handlers['file']->persistPendingProperties(); + $this->persistDeferredWrites($deferredSettings); // Now the file should have the new value $files = glob($this->path . '*.php', GLOB_NOSORT); From e30d778b7d9f85f4b6692613935eed4c2f9b759f Mon Sep 17 00:00:00 2001 From: michalsn Date: Wed, 29 Oct 2025 18:37:57 +0100 Subject: [PATCH 3/4] update rector --- rector.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rector.php b/rector.php index d6575f1..6c4a3ad 100644 --- a/rector.php +++ b/rector.php @@ -28,6 +28,7 @@ use Rector\CodingStyle\Rector\FuncCall\CountArrayToEmptyArrayComparisonRector; use Rector\Config\RectorConfig; use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyRector; +use Rector\DeadCode\Rector\MethodCall\RemoveNullArgOnNullDefaultParamRector; use Rector\EarlyReturn\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector; use Rector\EarlyReturn\Rector\If_\ChangeIfElseValueAssignToEarlyReturnRector; use Rector\EarlyReturn\Rector\If_\RemoveAlwaysElseRector; @@ -80,6 +81,8 @@ // May load view files directly when detecting classes StringClassNameToClassConstantRector::class, + + RemoveNullArgOnNullDefaultParamRector::class ]); $rectorConfig->rule(SimplifyUselessVariableRector::class); $rectorConfig->rule(RemoveAlwaysElseRector::class); From f301c9621fa8795516218cb2303d9820e2ba088a Mon Sep 17 00:00:00 2001 From: michalsn Date: Wed, 29 Oct 2025 18:39:37 +0100 Subject: [PATCH 4/4] cs fix --- rector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rector.php b/rector.php index 6c4a3ad..3eec8d9 100644 --- a/rector.php +++ b/rector.php @@ -82,7 +82,7 @@ // May load view files directly when detecting classes StringClassNameToClassConstantRector::class, - RemoveNullArgOnNullDefaultParamRector::class + RemoveNullArgOnNullDefaultParamRector::class, ]); $rectorConfig->rule(SimplifyUselessVariableRector::class); $rectorConfig->rule(RemoveAlwaysElseRector::class);