Apply suggestions

This commit is contained in:
Philipp Holzer 2023-01-03 17:24:05 +01:00
parent 072464119a
commit b439df892a
Signed by: nupplaPhil
GPG key ID: 24A7501396EB5432
14 changed files with 69 additions and 66 deletions

View file

@ -100,7 +100,7 @@ HELP;
$enabled = intval($this->getArgument(0)); $enabled = intval($this->getArgument(0));
$transactionConfig = $this->config->transactional(); $transactionConfig = $this->config->beginTransaction();
$transactionConfig->set('system', 'maintenance', $enabled); $transactionConfig->set('system', 'maintenance', $enabled);
@ -112,7 +112,7 @@ HELP;
$transactionConfig->delete('system', 'maintenance_reason'); $transactionConfig->delete('system', 'maintenance_reason');
} }
$transactionConfig->save(); $transactionConfig->commit();
if ($enabled) { if ($enabled) {
$mode_str = "maintenance mode"; $mode_str = "maintenance mode";

View file

@ -101,9 +101,10 @@ HELP;
$old_host = str_replace('http://', '@', Strings::normaliseLink($old_url)); $old_host = str_replace('http://', '@', Strings::normaliseLink($old_url));
$this->out('Entering maintenance mode'); $this->out('Entering maintenance mode');
$this->config->set('system', 'maintenance', true, false); $this->config->beginTransaction()
$this->config->set('system', 'maintenance_reason', 'Relocating node to ' . $new_url, false); ->set('system', 'maintenance', true)
->set('system', 'maintenance_reason', 'Relocating node to ' . $new_url)
->commit();
try { try {
if (!$this->database->transaction()) { if (!$this->database->transaction()) {
throw new \Exception('Unable to start a transaction, please retry later.'); throw new \Exception('Unable to start a transaction, please retry later.');
@ -189,10 +190,10 @@ HELP;
return 1; return 1;
} finally { } finally {
$this->out('Leaving maintenance mode'); $this->out('Leaving maintenance mode');
$this->config->transactional() $this->config->beginTransaction()
->set('system', 'maintenance', false) ->set('system', 'maintenance', false)
->delete('system', 'maintenance_reason') ->delete('system', 'maintenance_reason')
->save(); ->commit();
} }
// send relocate // send relocate

View file

@ -94,9 +94,9 @@ interface IManageConfigValues
* *
* It relies on the current instance, so after save(), the values of this config class will get altered at once too. * It relies on the current instance, so after save(), the values of this config class will get altered at once too.
* *
* @return ISetConfigValuesTransactional * @return ISetConfigValuesTransactionally
*/ */
public function transactional(): ISetConfigValuesTransactional; public function beginTransaction(): ISetConfigValuesTransactionally;
/** /**
* Deletes the given key from the system configuration. * Deletes the given key from the system configuration.

View file

@ -27,7 +27,7 @@ use Friendica\Core\Config\Exception\ConfigPersistenceException;
* Interface for transactional saving of config values * Interface for transactional saving of config values
* It buffers every set/delete until "save()" is called * It buffers every set/delete until "save()" is called
*/ */
interface ISetConfigValuesTransactional interface ISetConfigValuesTransactionally
{ {
/** /**
* Get a particular user's config variable given the category name * Get a particular user's config variable given the category name
@ -76,9 +76,9 @@ interface ISetConfigValuesTransactional
public function delete(string $cat, string $key): self; public function delete(string $cat, string $key): self;
/** /**
* Saves the node specific config values * Commits the changes of the current transaction
* *
* @throws ConfigPersistenceException In case the persistence layer throws errors * @throws ConfigPersistenceException In case the persistence layer throws errors
*/ */
public function save(): void; public function commit(): void;
} }

View file

@ -22,7 +22,7 @@
namespace Friendica\Core\Config\Model; namespace Friendica\Core\Config\Model;
use Friendica\Core\Config\Capability\IManageConfigValues; use Friendica\Core\Config\Capability\IManageConfigValues;
use Friendica\Core\Config\Capability\ISetConfigValuesTransactional; use Friendica\Core\Config\Capability\ISetConfigValuesTransactionally;
use Friendica\Core\Config\Exception\ConfigFileException; use Friendica\Core\Config\Exception\ConfigFileException;
use Friendica\Core\Config\Exception\ConfigPersistenceException; use Friendica\Core\Config\Exception\ConfigPersistenceException;
use Friendica\Core\Config\Util\ConfigFileManager; use Friendica\Core\Config\Util\ConfigFileManager;
@ -63,9 +63,9 @@ class Config implements IManageConfigValues
} }
/** {@inheritDoc} */ /** {@inheritDoc} */
public function transactional(): ISetConfigValuesTransactional public function beginTransaction(): ISetConfigValuesTransactionally
{ {
return new TransactionalConfig($this); return new ConfigTransaction($this);
} }
/** /**

View file

@ -22,14 +22,14 @@
namespace Friendica\Core\Config\Model; namespace Friendica\Core\Config\Model;
use Friendica\Core\Config\Capability\IManageConfigValues; use Friendica\Core\Config\Capability\IManageConfigValues;
use Friendica\Core\Config\Capability\ISetConfigValuesTransactional; use Friendica\Core\Config\Capability\ISetConfigValuesTransactionally;
use Friendica\Core\Config\Exception\ConfigPersistenceException; use Friendica\Core\Config\Exception\ConfigPersistenceException;
use Friendica\Core\Config\ValueObject\Cache; use Friendica\Core\Config\ValueObject\Cache;
/** /**
* config class, which sets values into a temporary buffer until "save()" is called * Transaction class for configurations, which sets values into a temporary buffer until "save()" is called
*/ */
class TransactionalConfig implements ISetConfigValuesTransactional class ConfigTransaction implements ISetConfigValuesTransactionally
{ {
/** @var IManageConfigValues */ /** @var IManageConfigValues */
protected $config; protected $config;
@ -54,7 +54,7 @@ class TransactionalConfig implements ISetConfigValuesTransactional
} }
/** {@inheritDoc} */ /** {@inheritDoc} */
public function set(string $cat, string $key, $value): ISetConfigValuesTransactional public function set(string $cat, string $key, $value): ISetConfigValuesTransactionally
{ {
$this->cache->set($cat, $key, $value, Cache::SOURCE_DATA); $this->cache->set($cat, $key, $value, Cache::SOURCE_DATA);
@ -63,7 +63,7 @@ class TransactionalConfig implements ISetConfigValuesTransactional
/** {@inheritDoc} */ /** {@inheritDoc} */
public function delete(string $cat, string $key): ISetConfigValuesTransactional public function delete(string $cat, string $key): ISetConfigValuesTransactionally
{ {
$this->cache->delete($cat, $key); $this->cache->delete($cat, $key);
$this->delCache->set($cat, $key, 'deleted'); $this->delCache->set($cat, $key, 'deleted');
@ -72,7 +72,7 @@ class TransactionalConfig implements ISetConfigValuesTransactional
} }
/** {@inheritDoc} */ /** {@inheritDoc} */
public function save(): void public function commit(): void
{ {
try { try {
$newCache = $this->config->getCache()->merge($this->cache); $newCache = $this->config->getCache()->merge($this->cache);

View file

@ -332,10 +332,8 @@ class Cache
$keys = array_keys($cache->config[$category]); $keys = array_keys($cache->config[$category]);
foreach ($keys as $key) { foreach ($keys as $key) {
if (!is_null($newConfig[$category][$key] ?? null)) { unset($newConfig[$category][$key]);
unset($newConfig[$category][$key]); unset($newSource[$category][$key]);
unset($newSource[$category][$key]);
}
} }
} }
} }

View file

@ -160,10 +160,10 @@ class Update
Logger::warning('Pre update failed', ['version' => $version]); Logger::warning('Pre update failed', ['version' => $version]);
DI::config()->set('system', 'update', Update::FAILED); DI::config()->set('system', 'update', Update::FAILED);
DI::lock()->release('dbupdate'); DI::lock()->release('dbupdate');
DI::config()->transactional() DI::config()->beginTransaction()
->set('system', 'maintenance', false) ->set('system', 'maintenance', false)
->delete('system', 'maintenance_reason') ->delete('system', 'maintenance_reason')
->save(); ->commit();
return $r; return $r;
} else { } else {
Logger::notice('Pre update executed.', ['version' => $version]); Logger::notice('Pre update executed.', ['version' => $version]);
@ -183,10 +183,10 @@ class Update
Logger::error('Update ERROR.', ['from' => $stored, 'to' => $current, 'retval' => $retval]); Logger::error('Update ERROR.', ['from' => $stored, 'to' => $current, 'retval' => $retval]);
DI::config()->set('system', 'update', Update::FAILED); DI::config()->set('system', 'update', Update::FAILED);
DI::lock()->release('dbupdate'); DI::lock()->release('dbupdate');
DI::config()->transactional() DI::config()->beginTransaction()
->set('system', 'maintenance', false) ->set('system', 'maintenance', false)
->delete('system', 'maintenance_reason') ->delete('system', 'maintenance_reason')
->save(); ->commit();
return $retval; return $retval;
} else { } else {
Logger::notice('Database structure update finished.', ['from' => $stored, 'to' => $current]); Logger::notice('Database structure update finished.', ['from' => $stored, 'to' => $current]);
@ -202,10 +202,10 @@ class Update
Logger::warning('Post update failed', ['version' => $version]); Logger::warning('Post update failed', ['version' => $version]);
DI::config()->set('system', 'update', Update::FAILED); DI::config()->set('system', 'update', Update::FAILED);
DI::lock()->release('dbupdate'); DI::lock()->release('dbupdate');
DI::config()->transactional() DI::config()->beginTransaction()
->set('system', 'maintenance', false) ->set('system', 'maintenance', false)
->delete('system', 'maintenance_reason') ->delete('system', 'maintenance_reason')
->save(); ->commit();
return $r; return $r;
} else { } else {
DI::config()->set('system', 'build', $version); DI::config()->set('system', 'build', $version);
@ -216,10 +216,10 @@ class Update
DI::config()->set('system', 'build', $current); DI::config()->set('system', 'build', $current);
DI::config()->set('system', 'update', Update::SUCCESS); DI::config()->set('system', 'update', Update::SUCCESS);
DI::lock()->release('dbupdate'); DI::lock()->release('dbupdate');
DI::config()->transactional() DI::config()->beginTransaction()
->set('system', 'maintenance', false) ->set('system', 'maintenance', false)
->delete('system', 'maintenance_reason') ->delete('system', 'maintenance_reason')
->save(); ->commit();
Logger::notice('Update success.', ['from' => $stored, 'to' => $current]); Logger::notice('Update success.', ['from' => $stored, 'to' => $current]);
if ($sendMail) { if ($sendMail) {

View file

@ -182,10 +182,10 @@ class DBStructure
$status = self::update($verbose, true); $status = self::update($verbose, true);
if ($enable_maintenance_mode) { if ($enable_maintenance_mode) {
DI::config()->transactional() DI::config()->beginTransaction()
->set('system', 'maintenance', false) ->set('system', 'maintenance', false)
->delete('system', 'maintenance_reason') ->delete('system', 'maintenance_reason')
->save(); ->commit();
} }
return $status; return $status;

View file

@ -144,7 +144,7 @@ class Site extends BaseAdmin
$relay_user_tags = !empty($_POST['relay_user_tags']); $relay_user_tags = !empty($_POST['relay_user_tags']);
$active_panel = (!empty($_POST['active_panel']) ? "#" . trim($_POST['active_panel']) : ''); $active_panel = (!empty($_POST['active_panel']) ? "#" . trim($_POST['active_panel']) : '');
$transactionConfig = DI::config()->transactional(); $transactionConfig = DI::config()->beginTransaction();
// Has the directory url changed? If yes, then resubmit the existing profiles there // Has the directory url changed? If yes, then resubmit the existing profiles there
if ($global_directory != DI::config()->get('system', 'directory') && ($global_directory != '')) { if ($global_directory != DI::config()->get('system', 'directory') && ($global_directory != '')) {
@ -320,7 +320,7 @@ class Site extends BaseAdmin
$transactionConfig->set('system', 'relay_deny_tags' , $relay_deny_tags); $transactionConfig->set('system', 'relay_deny_tags' , $relay_deny_tags);
$transactionConfig->set('system', 'relay_user_tags' , $relay_user_tags); $transactionConfig->set('system', 'relay_user_tags' , $relay_user_tags);
$transactionConfig->save(); $transactionConfig->commit();
DI::baseUrl()->redirect('admin/site' . $active_panel); DI::baseUrl()->redirect('admin/site' . $active_panel);
} }

View file

@ -369,16 +369,20 @@ class CacheTest extends MockedTest
$configCache->set('system', 'test_2','with_data', Cache::SOURCE_DATA); $configCache->set('system', 'test_2','with_data', Cache::SOURCE_DATA);
$configCache->set('config', 'test_override','with_another_data', Cache::SOURCE_DATA); $configCache->set('config', 'test_override','with_another_data', Cache::SOURCE_DATA);
$configCache->set('old_category', 'test_45','given category', Cache::SOURCE_DATA);
$newCache = new Cache(); $newCache = new Cache();
$newCache->set('config', 'test_override','override it again', Cache::SOURCE_DATA); $newCache->set('config', 'test_override','override it again', Cache::SOURCE_DATA);
$newCache->set('system', 'test_3','new value', Cache::SOURCE_DATA); $newCache->set('system', 'test_3','new value', Cache::SOURCE_DATA);
$newCache->set('new_category', 'test_23','added category', Cache::SOURCE_DATA);
$mergedCache = $configCache->merge($newCache); $mergedCache = $configCache->merge($newCache);
self::assertEquals('with_data', $mergedCache->get('system', 'test_2')); self::assertEquals('with_data', $mergedCache->get('system', 'test_2'));
self::assertEquals('override it again', $mergedCache->get('config', 'test_override')); self::assertEquals('override it again', $mergedCache->get('config', 'test_override'));
self::assertEquals('new value', $mergedCache->get('system', 'test_3')); self::assertEquals('new value', $mergedCache->get('system', 'test_3'));
self::assertEquals('given category', $mergedCache->get('old_category', 'test_45'));
self::assertEquals('added category', $mergedCache->get('new_category', 'test_23'));
} }
/** /**

View file

@ -2,15 +2,15 @@
namespace Friendica\Test\src\Core\Config; namespace Friendica\Test\src\Core\Config;
use Friendica\Core\Config\Capability\ISetConfigValuesTransactional; use Friendica\Core\Config\Capability\ISetConfigValuesTransactionally;
use Friendica\Core\Config\Model\Config; use Friendica\Core\Config\Model\Config;
use Friendica\Core\Config\Model\TransactionalConfig; use Friendica\Core\Config\Model\ConfigTransaction;
use Friendica\Core\Config\Util\ConfigFileManager; use Friendica\Core\Config\Util\ConfigFileManager;
use Friendica\Core\Config\ValueObject\Cache; use Friendica\Core\Config\ValueObject\Cache;
use Friendica\Test\MockedTest; use Friendica\Test\MockedTest;
use Friendica\Test\Util\VFSTrait; use Friendica\Test\Util\VFSTrait;
class TransactionalConfigTest extends MockedTest class ConfigTransactionTest extends MockedTest
{ {
use VFSTrait; use VFSTrait;
@ -37,14 +37,14 @@ class TransactionalConfigTest extends MockedTest
public function testInstance() public function testInstance()
{ {
$config = new Config($this->configFileManager, new Cache()); $config = new Config($this->configFileManager, new Cache());
$transactionalConfig = new TransactionalConfig($config); $configTransaction = new ConfigTransaction($config);
self::assertInstanceOf(ISetConfigValuesTransactional::class, $transactionalConfig); self::assertInstanceOf(ISetConfigValuesTransactionally::class, $configTransaction);
self::assertInstanceOf(TransactionalConfig::class, $transactionalConfig); self::assertInstanceOf(ConfigTransaction::class, $configTransaction);
} }
public function testTransactionalConfig() public function testConfigTransaction()
{ {
$config = new Config($this->configFileManager, new Cache()); $config = new Config($this->configFileManager, new Cache());
$config->set('config', 'key1', 'value1'); $config->set('config', 'key1', 'value1');
@ -52,24 +52,24 @@ class TransactionalConfigTest extends MockedTest
$config->set('system', 'keyDel', 'valueDel'); $config->set('system', 'keyDel', 'valueDel');
$config->set('delete', 'keyDel', 'catDel'); $config->set('delete', 'keyDel', 'catDel');
$transactionalConfig = new TransactionalConfig($config); $configTransaction = new ConfigTransaction($config);
self::assertEquals('value1', $transactionalConfig->get('config', 'key1')); self::assertEquals('value1', $configTransaction->get('config', 'key1'));
self::assertEquals('value2', $transactionalConfig->get('system', 'key2')); self::assertEquals('value2', $configTransaction->get('system', 'key2'));
self::assertEquals('valueDel', $transactionalConfig->get('system', 'keyDel')); self::assertEquals('valueDel', $configTransaction->get('system', 'keyDel'));
self::assertEquals('catDel', $transactionalConfig->get('delete', 'keyDel')); self::assertEquals('catDel', $configTransaction->get('delete', 'keyDel'));
// the config file knows it as well immediately // the config file knows it as well immediately
$tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE; $tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE;
self::assertEquals('value1', $tempData['config']['key1'] ?? null); self::assertEquals('value1', $tempData['config']['key1'] ?? null);
self::assertEquals('value2', $tempData['system']['key2'] ?? null); self::assertEquals('value2', $tempData['system']['key2'] ?? null);
// new key-value // new key-value
$transactionalConfig->set('transaction', 'key3', 'value3'); $configTransaction->set('transaction', 'key3', 'value3');
// overwrite key-value // overwrite key-value
$transactionalConfig->set('config', 'key1', 'changedValue1'); $configTransaction->set('config', 'key1', 'changedValue1');
// delete key-value // delete key-value
$transactionalConfig->delete('system', 'keyDel'); $configTransaction->delete('system', 'keyDel');
// delete last key of category - so the category is gone // delete last key of category - so the category is gone
$transactionalConfig->delete('delete', 'keyDel'); $configTransaction->delete('delete', 'keyDel');
// The main config still doesn't know about the change // The main config still doesn't know about the change
self::assertNull($config->get('transaction', 'key3')); self::assertNull($config->get('transaction', 'key3'));
@ -77,10 +77,10 @@ class TransactionalConfigTest extends MockedTest
self::assertEquals('valueDel', $config->get('system', 'keyDel')); self::assertEquals('valueDel', $config->get('system', 'keyDel'));
self::assertEquals('catDel', $config->get('delete', 'keyDel')); self::assertEquals('catDel', $config->get('delete', 'keyDel'));
// but the transaction config of course knows it // but the transaction config of course knows it
self::assertEquals('value3', $transactionalConfig->get('transaction', 'key3')); self::assertEquals('value3', $configTransaction->get('transaction', 'key3'));
self::assertEquals('changedValue1', $transactionalConfig->get('config', 'key1')); self::assertEquals('changedValue1', $configTransaction->get('config', 'key1'));
self::assertNull($transactionalConfig->get('system', 'keyDel')); self::assertNull($configTransaction->get('system', 'keyDel'));
self::assertNull($transactionalConfig->get('delete', 'keyDel')); self::assertNull($configTransaction->get('delete', 'keyDel'));
// The config file still doesn't know it either // The config file still doesn't know it either
$tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE; $tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE;
self::assertEquals('value1', $tempData['config']['key1'] ?? null); self::assertEquals('value1', $tempData['config']['key1'] ?? null);
@ -89,16 +89,16 @@ class TransactionalConfigTest extends MockedTest
self::assertNull($tempData['transaction']['key3'] ?? null); self::assertNull($tempData['transaction']['key3'] ?? null);
// save it back! // save it back!
$transactionalConfig->save(); $configTransaction->commit();
// Now every config and file knows the change // Now every config and file knows the change
self::assertEquals('changedValue1', $config->get('config', 'key1')); self::assertEquals('changedValue1', $config->get('config', 'key1'));
self::assertEquals('value3', $config->get('transaction', 'key3')); self::assertEquals('value3', $config->get('transaction', 'key3'));
self::assertNull($config->get('system', 'keyDel')); self::assertNull($config->get('system', 'keyDel'));
self::assertNull($config->get('delete', 'keyDel')); self::assertNull($config->get('delete', 'keyDel'));
self::assertEquals('value3', $transactionalConfig->get('transaction', 'key3')); self::assertEquals('value3', $configTransaction->get('transaction', 'key3'));
self::assertEquals('changedValue1', $transactionalConfig->get('config', 'key1')); self::assertEquals('changedValue1', $configTransaction->get('config', 'key1'));
self::assertNull($transactionalConfig->get('system', 'keyDel')); self::assertNull($configTransaction->get('system', 'keyDel'));
$tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE; $tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE;
self::assertEquals('changedValue1', $tempData['config']['key1'] ?? null); self::assertEquals('changedValue1', $tempData['config']['key1'] ?? null);
self::assertEquals('value2', $tempData['system']['key2'] ?? null); self::assertEquals('value2', $tempData['system']['key2'] ?? null);

View file

@ -1184,13 +1184,13 @@ function update_1508()
{ {
$config = DBA::selectToArray('config'); $config = DBA::selectToArray('config');
$newConfig = DI::config()->transactional(); $newConfig = DI::config()->beginTransaction();
foreach ($config as $entry) { foreach ($config as $entry) {
$newConfig->set($entry['cat'], $entry['k'], $entry['v']); $newConfig->set($entry['cat'], $entry['k'], $entry['v']);
} }
$newConfig->save(); $newConfig->commit();
DBA::e("DELETE FROM `config`"); DBA::e("TRUNCATE TABLE `config`");
} }

View file

@ -49,7 +49,7 @@ $login_bg_color = '';
$modified = time(); $modified = time();
if (DI::mode()->has(\Friendica\App\Mode::MAINTENANCEDISABLED)) { if (DI::mode()->has(\Friendica\App\Mode::MAINTENANCEDISABLED)) {
DI::config()->reload('frio'); DI::config()->reload();
// Default to hard-coded values for empty settings // Default to hard-coded values for empty settings
$scheme = DI::config()->get('frio', 'scheme', DI::config()->get('frio', 'schema')); $scheme = DI::config()->get('frio', 'scheme', DI::config()->get('frio', 'schema'));