From 8bf55d65318e4d7d3cf78a3268f12bf1436f83d0 Mon Sep 17 00:00:00 2001 From: Sybille Peters <sypets@gmx.de> Date: Fri, 23 Nov 2018 16:53:22 +0100 Subject: [PATCH] [FEATURE] Make Locking API configurable Locking API can be configured via $GLOBALS['TYPO3_CONF_VARS']. Now, by changing the priority of a specific lock strategy, it can be effectively disabled. Resolves: #87072 Releases: master Change-Id: Ie632f470a2144f67206e40736a9f70f4296715fa Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/58940 Tested-by: Markus Klein <markus.klein@typo3.org> Tested-by: TYPO3com <noreply@typo3.com> Tested-by: Benni Mack <benni@typo3.org> Reviewed-by: Markus Klein <markus.klein@typo3.org> Reviewed-by: Benni Mack <benni@typo3.org> --- .../core/Classes/Locking/FileLockStrategy.php | 15 +++- .../Classes/Locking/SemaphoreLockStrategy.php | 18 ++++- .../Classes/Locking/SimpleLockStrategy.php | 13 +++- .../Configuration/DefaultConfiguration.php | 28 +++++++ ...ingAddedConfigurationOptionsForLocking.rst | 73 +++++++++++++++++++ .../Unit/Locking/FileLockStrategyTest.php | 19 +++++ .../Tests/Unit/Locking/LockFactoryTest.php | 46 +++++++++++- .../Locking/SemaphoreLockStrategyTest.php | 20 +++++ .../Unit/Locking/SimpleLockStrategyTest.php | 20 +++++ 9 files changed, 244 insertions(+), 8 deletions(-) create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Feature-87072-AddedConfigurationOptionsForLockingAddedConfigurationOptionsForLocking.rst diff --git a/typo3/sysext/core/Classes/Locking/FileLockStrategy.php b/typo3/sysext/core/Classes/Locking/FileLockStrategy.php index cb2b58801bc0..e59c4b86adb1 100644 --- a/typo3/sysext/core/Classes/Locking/FileLockStrategy.php +++ b/typo3/sysext/core/Classes/Locking/FileLockStrategy.php @@ -27,6 +27,8 @@ class FileLockStrategy implements LockingStrategyInterface { const FILE_LOCK_FOLDER = 'lock/'; + const DEFAULT_PRIORITY = 75; + /** * @var resource File pointer if using flock method */ @@ -49,11 +51,17 @@ class FileLockStrategy implements LockingStrategyInterface public function __construct($subject) { /* - * Tests if the directory for simple locks is available. + * Tests if the directory for file locks is available. * If not, the directory will be created. The lock path is usually * below typo3temp/var, typo3temp/var itself should exist already (or root-path/var/ respectively) */ - $path = Environment::getVarPath() . '/' . self::FILE_LOCK_FOLDER; + if ($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['lockFileDir'] ?? false) { + $path = Environment::getProjectPath() . '/' + . trim($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['lockFileDir'], ' /') + . '/'; + } else { + $path = Environment::getVarPath() . '/' . self::FILE_LOCK_FOLDER; + } if (!is_dir($path)) { // Not using mkdir_deep on purpose here, if typo3temp itself // does not exist, this issue should be solved on a different @@ -152,7 +160,8 @@ class FileLockStrategy implements LockingStrategyInterface */ public static function getPriority() { - return 75; + return $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['priority'] + ?? self::DEFAULT_PRIORITY; } /** diff --git a/typo3/sysext/core/Classes/Locking/SemaphoreLockStrategy.php b/typo3/sysext/core/Classes/Locking/SemaphoreLockStrategy.php index 016fd5c5104b..fbff61c1c742 100644 --- a/typo3/sysext/core/Classes/Locking/SemaphoreLockStrategy.php +++ b/typo3/sysext/core/Classes/Locking/SemaphoreLockStrategy.php @@ -26,6 +26,8 @@ class SemaphoreLockStrategy implements LockingStrategyInterface { const FILE_LOCK_FOLDER = 'lock/'; + const DEFAULT_PRIORITY = 25; + /** * @var mixed Identifier used for this lock */ @@ -52,7 +54,18 @@ class SemaphoreLockStrategy implements LockingStrategyInterface */ public function __construct($subject) { - $path = Environment::getVarPath() . '/' . self::FILE_LOCK_FOLDER; + /* + * Tests if the directory for semaphore locks is available. + * If not, the directory will be created. The lock path is usually + * below typo3temp/var, typo3temp/var itself should exist already (or root-path/var/ respectively) + */ + if ($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['lockFileDir'] ?? false) { + $path = Environment::getProjectPath() . '/' + . trim($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['lockFileDir'], ' /') + . '/'; + } else { + $path = Environment::getVarPath() . '/' . self::FILE_LOCK_FOLDER; + } if (!is_dir($path)) { // Not using mkdir_deep on purpose here, if typo3temp/var itself // does not exist, this issue should be solved on a different @@ -147,7 +160,8 @@ class SemaphoreLockStrategy implements LockingStrategyInterface */ public static function getPriority() { - return 25; + return $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['priority'] + ?? self::DEFAULT_PRIORITY; } /** diff --git a/typo3/sysext/core/Classes/Locking/SimpleLockStrategy.php b/typo3/sysext/core/Classes/Locking/SimpleLockStrategy.php index bd792c272cd3..a354485932f7 100644 --- a/typo3/sysext/core/Classes/Locking/SimpleLockStrategy.php +++ b/typo3/sysext/core/Classes/Locking/SimpleLockStrategy.php @@ -26,6 +26,8 @@ class SimpleLockStrategy implements LockingStrategyInterface { const FILE_LOCK_FOLDER = 'lock/'; + const DEFAULT_PRIORITY = 50; + /** * @var string File path used for this lock */ @@ -55,7 +57,13 @@ class SimpleLockStrategy implements LockingStrategyInterface // Tests if the directory for simple locks is available. // If not, the directory will be created. The lock path is usually // below typo3temp/var, typo3temp/var itself should exist already (or getProjectPath . /var/ respectively) - $path = Environment::getVarPath() . '/' . self::FILE_LOCK_FOLDER; + if ($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['lockFileDir'] ?? false) { + $path = Environment::getProjectPath() . '/' + . trim($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['lockFileDir'], ' /') + . '/'; + } else { + $path = Environment::getVarPath() . '/' . self::FILE_LOCK_FOLDER; + } if (!is_dir($path)) { // Not using mkdir_deep on purpose here, if typo3temp/var itself // does not exist, this issue should be solved on a different @@ -183,7 +191,8 @@ class SimpleLockStrategy implements LockingStrategyInterface */ public static function getPriority() { - return 50; + return $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['priority'] + ?? self::DEFAULT_PRIORITY; } /** diff --git a/typo3/sysext/core/Configuration/DefaultConfiguration.php b/typo3/sysext/core/Configuration/DefaultConfiguration.php index 7f91e1bccf18..ddd9b9b77983 100644 --- a/typo3/sysext/core/Configuration/DefaultConfiguration.php +++ b/typo3/sysext/core/Configuration/DefaultConfiguration.php @@ -129,6 +129,34 @@ return [ 'StaticValueMapper' => \TYPO3\CMS\Core\Routing\Aspect\StaticValueMapper::class, ], ], + 'locking' => [ + 'strategies' => [ + \TYPO3\CMS\Core\Locking\FileLockStrategy::class => [ + // if not set: use default priority of FileLockStrategy + //'priority' => 75, + + // if not set: use default path of FileLockStrategy + // If you change this, directory must exist! + // 'lockFileDir' => 'typo3temp/var' + ], + \TYPO3\CMS\Core\Locking\SemaphoreLockStrategy::class => [ + // if not set: use default priority of SemaphoreLockStrategy + // 'priority' => 50 + + // empty: use default path of SemaphoreLockStrategy + // If you change this, directory must exist! + // 'lockFileDir' => 'typo3temp/var' + ], + \TYPO3\CMS\Core\Locking\SimpleLockStrategy::class => [ + // if not set: use default priority of SimpleLockStrategy + //'priority' => 25, + + // empty: use default path of SimpleLockStrategy + // If you change this, directory must exist! + // 'lockFileDir' => 'typo3temp/var' + ] + ] + ], 'caching' => [ 'cacheConfigurations' => [ // The cache_core cache is is for core php code only and must diff --git a/typo3/sysext/core/Documentation/Changelog/master/Feature-87072-AddedConfigurationOptionsForLockingAddedConfigurationOptionsForLocking.rst b/typo3/sysext/core/Documentation/Changelog/master/Feature-87072-AddedConfigurationOptionsForLockingAddedConfigurationOptionsForLocking.rst new file mode 100644 index 000000000000..557eefc0f103 --- /dev/null +++ b/typo3/sysext/core/Documentation/Changelog/master/Feature-87072-AddedConfigurationOptionsForLockingAddedConfigurationOptionsForLocking.rst @@ -0,0 +1,73 @@ +.. include:: ../../Includes.txt + +========================================================= +Feature: #87072 - Added Configuration Options for Locking +========================================================= + +See :issue:`87072` + +Description +=========== + +With change `Feature: #47712 - New Locking API +<https://docs.typo3.org/typo3cms/extensions/core/latest/Changelog/7.2/Feature-47712-NewLockingAPI.html>`__ a new Locking API was introduced. +This API can be extended. It provides three locking strategies and an interface for adding your own locking strategy in an extension. +However, until now, the default behaviour could not be changed using only the TYPO3 core. + +The introduction of new options makes some of the default properties of the locking API configurable: + +* The priority of each locking strategy can be changed. +* The directory where the lock files are written can be configured. + +Configuration example +--------------------- + +typo3conf/AdditionalConfiguration.php:: + + <?php + + if (!defined('TYPO3_MODE')) { + die('Access denied.'); + } + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][\TYPO3\CMS\Core\Locking\FileLockStrategy::class]['priority'] = 10; + // The directory specified here must exist und must be a subdirectory of `Environment::getProjectPath()` + $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][\TYPO3\CMS\Core\Locking\FileLockStrategy::class]['lockFileDir'] = 'mylockdir'; + + +This sets the priority of FileLockStrategy to 10, thus making it the locking strategy with the least priority, which +will be chosen last by the LockFactory. + +The directory for storing the locks is changed to `mylockdir`. + +Impact +====== + +For administrators +------------------ + +Nothing changes by default. The default values are used for the Locking API, same as before this change. + +If AdditionalConfiguration.php is used to change Global Configuration settings for Locking API, and not used with care, +it can seriously compromise the stability of the system. As usual, when overriding Global Configuration with +LocalConfiguration.php or AdditionalConfiguration.php, great caution must be used. + +Specifically, do the following: + +* Test this on a test system first +* If you change the priorities, make sure your system fully supports the locking strategy which will be chosen by default. +* If you change the directory, make sure the directory exists and will always exist in the future. + +For developers +-------------- + +If a locking strategy is added by an extension, the priority and possibly directory for storing locks should be made +configurable as well:: + + public static function getPriority() + { + return $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][self::class]['priority'] + ?? self::DEFAULT_PRIORITY; + } + +.. index:: ext:core diff --git a/typo3/sysext/core/Tests/Unit/Locking/FileLockStrategyTest.php b/typo3/sysext/core/Tests/Unit/Locking/FileLockStrategyTest.php index a83f4174062f..a5a5e7504e1e 100644 --- a/typo3/sysext/core/Tests/Unit/Locking/FileLockStrategyTest.php +++ b/typo3/sysext/core/Tests/Unit/Locking/FileLockStrategyTest.php @@ -42,4 +42,23 @@ class FileLockStrategyTest extends UnitTestCase $lock = $this->getAccessibleMock(FileLockStrategy::class, ['dummy'], ['999999999']); self::assertSame(Environment::getVarPath() . '/' . FileLockStrategy::FILE_LOCK_FOLDER . 'flock_' . md5('999999999'), $lock->_get('filePath')); } + + /** + * @test + */ + public function getPriorityReturnsDefaultPriority() + { + self::assertEquals(FileLockStrategy::getPriority(), FileLockStrategy::DEFAULT_PRIORITY); + } + + /** + * @test + */ + public function setPriority() + { + $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][\TYPO3\CMS\Core\Locking\FileLockStrategy::class]['priority'] = 10; + + self::assertEquals(10, FileLockStrategy::getPriority()); + unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][\TYPO3\CMS\Core\Locking\FileLockStrategy::class]['priority']); + } } diff --git a/typo3/sysext/core/Tests/Unit/Locking/LockFactoryTest.php b/typo3/sysext/core/Tests/Unit/Locking/LockFactoryTest.php index 95fbcf3c2f76..a3c69a8773ba 100644 --- a/typo3/sysext/core/Tests/Unit/Locking/LockFactoryTest.php +++ b/typo3/sysext/core/Tests/Unit/Locking/LockFactoryTest.php @@ -19,6 +19,8 @@ use TYPO3\CMS\Core\Locking\FileLockStrategy; use TYPO3\CMS\Core\Locking\LockFactory; use TYPO3\CMS\Core\Locking\LockingStrategyInterface; use TYPO3\CMS\Core\Locking\SemaphoreLockStrategy; +use TYPO3\CMS\Core\Locking\SimpleLockStrategy; +use TYPO3\CMS\Core\Resource\File; use TYPO3\CMS\Core\Tests\Unit\Locking\Fixtures\DummyLock; use TYPO3\TestingFramework\Core\Unit\UnitTestCase; @@ -32,6 +34,11 @@ class LockFactoryTest extends UnitTestCase */ protected $mockFactory; + /** + * @var array + */ + protected $strategiesConfigBackup = []; + /** * Set up the tests */ @@ -39,6 +46,21 @@ class LockFactoryTest extends UnitTestCase { parent::setUp(); $this->mockFactory = $this->getAccessibleMock(LockFactory::class, ['dummy']); + + // backup global configuration + if (isset($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'])) { + $this->strategiesConfigBackup = $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies']; + } else { + $this->strategiesConfigBackup = []; + } + } + + protected function tearDown(): void + { + // restore global configuration + $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'] = $this->strategiesConfigBackup; + + parent::tearDown(); } /** @@ -67,7 +89,10 @@ class LockFactoryTest extends UnitTestCase public function getLockerReturnsExpectedClass() { $this->mockFactory->_set('lockingStrategy', [FileLockStrategy::class => true, DummyLock::class => true]); - $locker = $this->mockFactory->createLocker('id', LockingStrategyInterface::LOCK_CAPABILITY_EXCLUSIVE | LockingStrategyInterface::LOCK_CAPABILITY_SHARED); + $locker = $this->mockFactory->createLocker( + 'id', + LockingStrategyInterface::LOCK_CAPABILITY_EXCLUSIVE | LockingStrategyInterface::LOCK_CAPABILITY_SHARED + ); self::assertInstanceOf(FileLockStrategy::class, $locker); } @@ -81,6 +106,25 @@ class LockFactoryTest extends UnitTestCase self::assertInstanceOf(DummyLock::class, $locker); } + /** + * @test + */ + public function setPriorityGetLockerReturnsClassWithHighestPriority() + { + $lowestValue = min([ + FileLockStrategy::DEFAULT_PRIORITY, + SimpleLockStrategy::DEFAULT_PRIORITY, + SemaphoreLockStrategy::DEFAULT_PRIORITY + ]) - 1; + $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][FileLockStrategy::class]['priority'] = $lowestValue; + $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][SemaphoreLockStrategy::class]['priority'] = $lowestValue; + $locker = $this->mockFactory->createLocker('id'); + self::assertInstanceOf(SimpleLockStrategy::class, $locker); + + unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][FileLockStrategy::class]['priority']); + unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][SemaphoreLockStrategy::class]['priority']); + } + /** * @test */ diff --git a/typo3/sysext/core/Tests/Unit/Locking/SemaphoreLockStrategyTest.php b/typo3/sysext/core/Tests/Unit/Locking/SemaphoreLockStrategyTest.php index b9c5a60c6d37..e0a2be90e6a2 100644 --- a/typo3/sysext/core/Tests/Unit/Locking/SemaphoreLockStrategyTest.php +++ b/typo3/sysext/core/Tests/Unit/Locking/SemaphoreLockStrategyTest.php @@ -15,6 +15,7 @@ namespace TYPO3\CMS\Core\Tests\Unit\Locking; */ use TYPO3\CMS\Core\Locking\SemaphoreLockStrategy; +use TYPO3\CMS\Core\Locking\SimpleLockStrategy; use TYPO3\TestingFramework\Core\Unit\UnitTestCase; /** @@ -34,4 +35,23 @@ class SemaphoreLockStrategyTest extends UnitTestCase $lock->release(); $lock->destroy(); } + + /** + * @test + */ + public function getPriorityReturnsDefaultPriority() + { + self::assertEquals(SimpleLockStrategy::getPriority(), SimpleLockStrategy::DEFAULT_PRIORITY); + } + + /** + * @test + */ + public function setPriority() + { + $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][\TYPO3\CMS\Core\Locking\SemaphoreLockStrategy::class]['priority'] = 10; + + self::assertEquals(10, SemaphoreLockStrategy::getPriority()); + unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][\TYPO3\CMS\Core\Locking\SemaphoreLockStrategy::class]['priority']); + } } diff --git a/typo3/sysext/core/Tests/Unit/Locking/SimpleLockStrategyTest.php b/typo3/sysext/core/Tests/Unit/Locking/SimpleLockStrategyTest.php index 8c20bf9d7365..2b0fd3b3d163 100644 --- a/typo3/sysext/core/Tests/Unit/Locking/SimpleLockStrategyTest.php +++ b/typo3/sysext/core/Tests/Unit/Locking/SimpleLockStrategyTest.php @@ -16,6 +16,7 @@ namespace TYPO3\CMS\Core\Tests\Unit\Locking; use PHPUnit\Framework\SkippedTestError; use TYPO3\CMS\Core\Core\Environment; +use TYPO3\CMS\Core\Locking\SemaphoreLockStrategy; use TYPO3\CMS\Core\Locking\SimpleLockStrategy; use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\TestingFramework\Core\Unit\UnitTestCase; @@ -124,4 +125,23 @@ class SimpleLockStrategyTest extends UnitTestCase } self::assertTrue($fileExists); } + + /** + * @test + */ + public function getPriorityReturnsDefaultPriority() + { + self::assertEquals(SemaphoreLockStrategy::getPriority(), SemaphoreLockStrategy::DEFAULT_PRIORITY); + } + + /** + * @test + */ + public function setPriority() + { + $GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][\TYPO3\CMS\Core\Locking\SimpleLockStrategy::class]['priority'] = 10; + + self::assertEquals(10, SimpleLockStrategy::getPriority()); + unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['locking']['strategies'][\TYPO3\CMS\Core\Locking\SimpleLockStrategy::class]['priority']); + } } -- GitLab