diff --git a/README.md b/README.md index ccac319..0a9fed8 100644 --- a/README.md +++ b/README.md @@ -52,14 +52,44 @@ namespace PetrKnap\CriticalSection; use Symfony\Component\Lock\NoLock; -/** @param Locked $resource */ -function f(LockedResource $resource) { - echo $resource->value; +/** + * @param Locked $from + * @param Locked $to + */ +function transferValue(LockedResource $from, LockedResource $to, int $volume): void { + if ($from->value < $volume) { + throw new \RuntimeException(); + } + $from->value -= $volume; + $to->value += $volume; + echo "Moved {$volume} from #{$from->id} (current value {$from->value}) to #{$to->id} (current value {$to->value})."; } -$lock = new NoLock(); -$resource = LockableResource::of(new Some\Resource('data'), $lock); -CriticalSection::withLock($lock)(fn () => f($resource)); +$fromLock = new NoLock(); +$lockedFrom = LockableResource::of(new Some\Resource(1, value: 15), $fromLock); +$toLock = new NoLock(); +$lockedTo = LockableResource::of(new Some\Resource(2, value: 5), $toLock); +CriticalSection::withLocks([$fromLock, $toLock])(fn () => transferValue($lockedFrom, $lockedTo, 10)); +``` + +## Do you want to keep code clear? + +To maintain clarity, I recommend using your own named critical sections (as service), +like [`Some\NamedCriticalSectionService`](./tests/Some/NamedCriticalSectionService.php). + +```php +namespace PetrKnap\CriticalSection; + +use Symfony\Component\Lock\LockFactory; +use Symfony\Component\Lock\Store\InMemoryStore; + +$criticalSection = new Some\NamedCriticalSectionService(new LockFactory(new InMemoryStore())); + +$criticalSection->updateSomeResources( + fn (LockedResource $from, LockedResource $to) => transferValue($from, $to, 10), + new Some\Resource(1, value: 15), + new Some\Resource(2, value: 5), +); ``` ## Does your critical section work with database? diff --git a/composer.json b/composer.json index fbae367..25c19ac 100644 --- a/composer.json +++ b/composer.json @@ -3,16 +3,16 @@ "autoload": { "psr-4": { "PetrKnap\\CriticalSection\\": "src" - } - }, - "autoload-dev": { - "psr-4": { - "PetrKnap\\CriticalSection\\": "tests" }, "files": [ "src/aliases.php" ] }, + "autoload-dev": { + "psr-4": { + "PetrKnap\\CriticalSection\\": "tests" + } + }, "config": { "allow-plugins": false, "sort-packages": true @@ -34,6 +34,7 @@ "name": "petrknap/critical-section", "require": { "php": ">=8.1", + "petrknap/optional": "^2.0|^3.0", "petrknap/shorts": "^2.1", "symfony/lock": "^6.0|^7.0" }, @@ -46,11 +47,12 @@ "scripts": { "test": "phpunit --colors=always --testdox tests", "ci-script": [ - "@check-requirements", "@check-implementation", + "@check-requirements", "@test-implementation" ], "check-requirements": [ + "composer update \"petrknap/*\"", "composer outdated \"petrknap/*\" --major-only --strict --ansi --no-interaction" ], "check-implementation": [ diff --git a/src/Symfony/Lock/LockPoolingHelper.php b/src/Symfony/Lock/LockPoolingHelper.php new file mode 100644 index 0000000..f743739 --- /dev/null +++ b/src/Symfony/Lock/LockPoolingHelper.php @@ -0,0 +1,59 @@ + + */ +final class LockPoolingHelper +{ + /** + * @return TLockPool + */ + public static function createLockPool(): array + { + return []; + } + + /** + * @param TLockPool $lockPool + * @param Optional|null $ttl + * @param Optional|null $autoRelease + */ + public static function getOrCreateLock( + array &$lockPool, + LockFactory $lockFactory, + string $resource, + Optional|null $ttl = null, + Optional|null $autoRelease = null, + ): LockInterface { + if (array_key_exists($resource, $lockPool)) { + return $lockPool[$resource]; + } elseif ($ttl === null && $autoRelease === null) { + $lockPool[$resource] = $lockFactory->createLock($resource); + } elseif ($ttl === null) { + $lockPool[$resource] = $lockFactory->createLock($resource, autoRelease: $autoRelease->orElseThrow()); + } elseif ($autoRelease === null) { + $lockPool[$resource] = $lockFactory->createLock($resource, $ttl->isPresent() ? $ttl->get() : null); + } else { + $lockPool[$resource] = $lockFactory->createLock($resource, $ttl->isPresent() ? $ttl->get() : null, $autoRelease->orElseThrow()); + } + return $lockPool[$resource]; + } + + /** + * @param TLockPool $lockPool + */ + public static function createCriticalSection(array $lockPool): CriticalSection + { + ksort($lockPool); + return CriticalSection::withLocks($lockPool); + } +} diff --git a/tests/AliasesTest.php b/tests/AliasesTest.php index ce5e9d6..b23086d 100644 --- a/tests/AliasesTest.php +++ b/tests/AliasesTest.php @@ -14,7 +14,7 @@ public function testLockedResourceAliases(): void self::assertInstanceOf( Locked::class, LockableResource::of( - new Some\Resource(), + new Some\Resource(1), self::createStub(LockInterface::class), ), ); diff --git a/tests/CriticalSectionTestCase.php b/tests/CriticalSectionTestCase.php index f8d9523..64a4b46 100644 --- a/tests/CriticalSectionTestCase.php +++ b/tests/CriticalSectionTestCase.php @@ -33,7 +33,7 @@ function (string $s, int $i, mixed $n) use (&$receivedArgs): void { */ public function testForwardsReturnFromCriticalSection(bool $isBlocking): void { - $expectedReturn = new Some\Resource(); + $expectedReturn = new Some\Resource(1); self::assertSame( $expectedReturn, diff --git a/tests/LockedResourceTestCase.php b/tests/LockedResourceTestCase.php index 36c2b66..35875b8 100644 --- a/tests/LockedResourceTestCase.php +++ b/tests/LockedResourceTestCase.php @@ -25,13 +25,13 @@ public function testCouldGetLockedResource(): void public function testIsMixin(): void { - $string = 'string'; + $value = 2; /** @var Locked $locked */ - $locked = $this->getLockedResource(new Some\Resource()); + $locked = $this->getLockedResource(new Some\Resource(1)); - $locked->value = $string; - self::assertSame($string, $locked->value); - self::assertSame($string, $locked->getValue()); + $locked->value = $value; + self::assertSame($value, $locked->value); + self::assertSame($value, $locked->getValue()); } abstract protected function getUnlockedResource(mixed $resource): LockedResource; diff --git a/tests/ReadmeTest.php b/tests/ReadmeTest.php index 792b9e6..ccf3cf8 100644 --- a/tests/ReadmeTest.php +++ b/tests/ReadmeTest.php @@ -23,7 +23,8 @@ public static function getExpectedOutputsOfPhpExamples(): iterable 'single-lock' => 'string(18) "This was critical."' . PHP_EOL, 'double-lock' => 'string(18) "This was critical."' . PHP_EOL, 'array-lock' => 'string(18) "This was critical."' . PHP_EOL, - 'resources' => 'data', + 'resources' => 'Moved 10 from #1 (current value 5) to #2 (current value 15).', + 'named-sections' => 'Moved 10 from #1 (current value 5) to #2 (current value 15).', 'transactional' => null, ]; } diff --git a/tests/Some/NamedCriticalSectionService.php b/tests/Some/NamedCriticalSectionService.php new file mode 100644 index 0000000..d01a9ab --- /dev/null +++ b/tests/Some/NamedCriticalSectionService.php @@ -0,0 +1,37 @@ + ...$resources): T $do + * + * @return T + */ + public function updateSomeResources(callable $do, Resource ...$resources): mixed + { + $lockPool = LockPoolingHelper::createLockPool(); + foreach ($resources as &$resource) { + $resource = LockableResource::of( + $resource, + LockPoolingHelper::getOrCreateLock($lockPool, $this->lockFactory, 'some_resource-' . $resource->id), + ); + } + return LockPoolingHelper::createCriticalSection($lockPool)($do, ...$resources); + } +} diff --git a/tests/Some/Resource.php b/tests/Some/Resource.php index 1173c67..531a195 100644 --- a/tests/Some/Resource.php +++ b/tests/Some/Resource.php @@ -7,11 +7,12 @@ final class Resource { public function __construct( - public string $value = '', + public readonly int $id, + public int $value = 0, ) { } - public function getValue(): string + public function getValue(): int { return $this->value; } diff --git a/tests/Symfony/Lock/LockPoolingHelperTest.php b/tests/Symfony/Lock/LockPoolingHelperTest.php new file mode 100644 index 0000000..970b498 --- /dev/null +++ b/tests/Symfony/Lock/LockPoolingHelperTest.php @@ -0,0 +1,109 @@ +lock = self::createMock(SharedLockInterface::class); + $this->lockFactory = self::createMock(LockFactory::class); + } + + public function testGetsLockWhenLockIsPresentInPool(): void + { + $this->lockFactory + ->expects(self::never()) + ->method('createLock') + ; + $lockPool = [self::RESOURCE => $this->lock]; + + self::assertSame( + $this->lock, + LockPoolingHelper::getOrCreateLock($lockPool, $this->lockFactory, self::RESOURCE), + ); + } + + /** + * @dataProvider dataCreatesNewLockAndAddsItToPoolWhenLockIsNotPresentInPool + */ + public function testCreatesNewLockAndAddsItToPoolWhenLockIsNotPresentInPool(OptionalFloat|null $ttl, OptionalBool|null $autoRelease): void + { + $this->lockFactory + ->expects(self::once()) + ->method('createLock') + ->with(self::RESOURCE, $ttl === null ? 300.0 : null, $autoRelease === null ? true : false) + ->willReturn($this->lock) + ; + $lockPool = []; + + self::assertSame( + $this->lock, + LockPoolingHelper::getOrCreateLock($lockPool, $this->lockFactory, self::RESOURCE, $ttl, $autoRelease), + ); + + self::assertSame( + [self::RESOURCE => $this->lock], + $lockPool, + ); + } + + public static function dataCreatesNewLockAndAddsItToPoolWhenLockIsNotPresentInPool(): array + { + return [ + 'ttl & autoRelease' => [OptionalFloat::empty(), OptionalBool::of(false)], + 'ttl' => [OptionalFloat::empty(), null], + 'autoRelease' => [null, OptionalBool::of(false)], + 'default values' => [null, null], + ]; + } + + public function testCreatesCriticalSectionWithSortedLocksFromPool(): void + { + $acquired = []; + $lockA = self::createMock(LockInterface::class); + $lockA->method('acquire')->willReturnCallback(static function () use (&$acquired) { + $acquired[] = 'a'; + return true; + }); + $lockB = self::createMock(LockInterface::class); + $lockB->method('acquire')->willReturnCallback(static function () use (&$acquired) { + $acquired[] = 'b'; + return true; + }); + $lockC = self::createMock(LockInterface::class); + $lockC->method('acquire')->willReturnCallback(static function () use (&$acquired) { + $acquired[] = 'c'; + return true; + }); + $lockPool = [ + 'b' => $lockB, + 'a' => $lockA, + 'c' => $lockC, + ]; + + LockPoolingHelper::createCriticalSection($lockPool)(static fn (): bool => true); + + self::assertSame( + ['a', 'b', 'c'], + $acquired, + ); + } +}