Files
jekkosandobjecttothis 1cf9cb9120 fix(appconfig): refresh cached settings once and avoid stale same-request reads (#4732)
* fix(appconfig): refresh cached settings once and avoid stale same-request reads

Addresses issue #3693, which has two related problems:

1. Repeated cache refreshes. Appconfig::batch_save() called save() per key,
   and save() unconditionally deleted + reloaded + re-cached the settings after
   every single write. A settings page with dozens of keys therefore refreshed
   the cache dozens of times in one request. batch_save() now persists all keys
   in the transaction and refreshes the cached settings exactly once after it
   commits successfully. save() keeps refreshing so direct single saves still
   work.

2. Stale same-request reads. Controllers copy config(OSPOS::class)->settings at
   construction time, so after a save in the same request the local copy is out
   of date. Config::postSaveInvoice() read the stale invoice_enable /
   default_register_mode values and sent the user to the wrong screen (e.g.
   opening a new sale instead of the invoice form after saving an enabled
   invoice). It now reads the freshly refreshed settings.

Adds tests/Models/AppconfigTest.php covering batch_save() and save() refreshing
the cached settings after a successful write.

* test(3693): adopt test{Method}_{Purpose} naming convention

Align AppconfigTest method names with the convention from #4730
(testBatchSave_PersistsAllKeysAndRefreshesCachedSettings, etc.), per review.

* style(config): remove inline comments per review feedback

---------

Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
2026-10-08 08:53:52 +02:00

189 lines
4.8 KiB
PHP

<?php
namespace App\Models;
use CodeIgniter\Database\ResultInterface;
use CodeIgniter\Model;
use Config\OSPOS;
use ReflectionException;
/**
* Appconfig class
*
*
*/
class Appconfig extends Model
{
protected $table = 'app_config';
protected $primaryKey = 'key';
protected $useAutoIncrement = false;
protected $useSoftDeletes = false;
protected $allowedFields = [
'key',
'value'
];
/**
* Checks to see if a given configuration exists in the database.
*
* @param string $key Key name to be searched.
* @return bool True if the key is found in the database or false if it does not exist.
*/
public function exists(string $key): bool
{
$builder = $this->db->table('app_config');
$builder->where('key', $key);
return ($builder->get()->getNumRows() === 1);
}
/**
* Get all OpenSourcePOS configuration values from the database.
*
* @return ResultInterface
*/
public function get_all(): ResultInterface
{
$builder = $this->db->table('app_config');
$builder->orderBy('key', 'asc');
return $builder->get();
}
/**
* @param string $key
* @param string $default
* @return string
*/
public function get_value(string $key, string $default = ''): string
{
$builder = $this->db->table('app_config');
$query = $builder->getWhere(['key' => $key], 1);
if ($query->getNumRows() === 1) {
return $query->getRow()->value;
}
return $default;
}
/**
* Calls the parent save() from BaseModel and updates the cached reference.
*
* @param array|object $data
* @return bool true when the save was successful and false if the save failed.
* @throws ReflectionException
*/
public function save($data): bool
{
$key = array_keys($data)[0];
$value = $data[$key];
$save_data = ['key' => $key, 'value' => $value];
$success = parent::save($save_data);
if ($success) {
config(OSPOS::class)->update_settings();
}
return $success;
}
/**
* Persists many key/value pairs in a single transaction and refreshes the
* cached settings exactly once after the transaction commits successfully.
*
* @param array $data Map of setting keys to their new values.
* @return bool true when the save was successful and false if it failed.
* @throws ReflectionException
*/
public function batch_save(array $data): bool
{
$success = true;
$this->db->transStart();
foreach ($data as $key => $value) {
$success &= parent::save(['key' => $key, 'value' => $value]);
}
$this->db->transComplete();
$success &= $this->db->transStatus();
if ($success) {
config(OSPOS::class)->update_settings();
}
return $success;
}
/**
* Deletes a row from the Appconfig table given the name of the setting to delete.
*
* @param ?string $id The field name to be deleted in the Appconfig table.
* @param bool $purge A hard delete is conducted if true and soft delete on false.
* @return bool Result of the delete operation.
*/
public function delete($id = null, bool $purge = false): bool
{
$builder = $this->db->table('app_config');
return $builder->delete(['key' => $id]);
}
/**
* @return bool
*/
public function delete_all(): bool // TODO: This function is never used in the code. Consider removing it.
{
$builder = $this->db->table('app_config');
return $builder->emptyTable();
}
/**
* @throws ReflectionException
*/
public function acquire_next_invoice_sequence(bool $save = true): string
{
$config = config(OSPOS::class)->settings;
$last_used = (int)$config['last_used_invoice_number'] + 1;
if ($save) {
$this->save(['last_used_invoice_number' => $last_used]);
}
return $last_used;
}
/**
* @throws ReflectionException
*/
public function acquire_next_quote_sequence(bool $save = true): string
{
$config = config(OSPOS::class)->settings;
$last_used = (int)$config['last_used_quote_number'] + 1;
if ($save) {
$this->save(['last_used_quote_number' => $last_used]);
}
return $last_used;
}
/**
* @throws ReflectionException
*/
public function acquire_next_work_order_sequence(bool $save = true): string
{
$config = config(OSPOS::class)->settings;
$last_used = (int)$config['last_used_work_order_number'] + 1;
if ($save) {
$this->save(['last_used_work_order_number' => $last_used]);
}
return $last_used;
}
}