Skip to content

[Console] Add Lockable trait #18471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Symfony/Component/Console/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ CHANGELOG
* added `setInputs()` method to CommandTester for ease testing of commands expecting inputs
* added `setStream()` and `getStream()` methods to Input (implement StreamableInputInterface)
* added StreamableInputInterface
* added LockableTrait

3.1.0
-----
Expand Down
63 changes: 63 additions & 0 deletions src/Symfony/Component/Console/Command/LockableTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Console\Command;

use Symfony\Component\Console\Exception\LogicException;
use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Filesystem\LockHandler;

/**
* Basic lock feature for commands.
*
* @author Fabien Potencier <fabien@symfony.com>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geoffrey-brier You should have put your name here. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xabbuh I'm not that important ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, you're the author of this trait. :)

*/
trait LockableTrait
{
protected $lockHandler;

/**
* Locks a command.
*
* @return bool
*/
protected function lock($name = null, $blocking = false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not declaring everything private by default?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protected looks fine to me, moving to private won't provide anything more

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, every class using this trait will now expose the used methods as an extension point which might not be what the user intended.

{
if (!class_exists(LockHandler::class)) {
throw new RuntimeException('To enable the locking feature you must install the symfony/filesystem component.');
}

if (null !== $this->lockHandler) {
throw new LogicException('A lock is already in place.');
}

$this->lockHandler = new LockHandler($name ?: $this->getName());

if (!$this->lockHandler->lock($blocking)) {
$this->lockHandler = null;

return false;
}

return true;
}

/**
* Releases the command lock if there is one.
*/
protected function release()
{
if ($this->lockHandler) {
$this->lockHandler->release();
$this->lockHandler = null;
}
}
}
58 changes: 58 additions & 0 deletions src/Symfony/Component/Console/Tests/Command/LockableTraitTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Console\Tests\Command;

use Symfony\Component\Console\Tester\CommandTester;
use Symfony\Component\Filesystem\LockHandler;

class LockableTraitTest extends \PHPUnit_Framework_TestCase
{
protected static $fixturesPath;

public static function setUpBeforeClass()
{
self::$fixturesPath = __DIR__.'/../Fixtures/';
require_once self::$fixturesPath.'/FooLockCommand.php';
require_once self::$fixturesPath.'/FooLock2Command.php';
}

public function testLockIsReleased()
{
$command = new \FooLockCommand();

$tester = new CommandTester($command);
$this->assertSame(2, $tester->execute(array()));
$this->assertSame(2, $tester->execute(array()));
}

public function testLockReturnsFalseIfAlreadyLockedByAnotherCommand()
{
$command = new \FooLockCommand();

$lock = new LockHandler($command->getName());
$lock->lock();

$tester = new CommandTester($command);
$this->assertSame(1, $tester->execute(array()));

$lock->release();
$this->assertSame(2, $tester->execute(array()));
}

public function testMultipleLockCallsThrowLogicException()
{
$command = new \FooLock2Command();

$tester = new CommandTester($command);
$this->assertSame(1, $tester->execute(array()));
}
}
28 changes: 28 additions & 0 deletions src/Symfony/Component/Console/Tests/Fixtures/FooLock2Command.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Command\LockableTrait;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class FooLock2Command extends Command
{
use LockableTrait;

protected function configure()
{
$this->setName('foo:lock2');
}

protected function execute(InputInterface $input, OutputInterface $output)
{
try {
$this->lock();
$this->lock();
} catch (LogicException $e) {
return 1;
}

return 2;
}
}
27 changes: 27 additions & 0 deletions src/Symfony/Component/Console/Tests/Fixtures/FooLockCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Command\LockableTrait;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class FooLockCommand extends Command
{
use LockableTrait;

protected function configure()
{
$this->setName('foo:lock');
}

protected function execute(InputInterface $input, OutputInterface $output)
{
if (!$this->lock()) {
return 1;
}

$this->release();

return 2;
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Component/Console/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
},
"require-dev": {
"symfony/event-dispatcher": "~2.8|~3.0",
"symfony/filesystem": "~2.8|~3.0",
"symfony/process": "~2.8|~3.0",
"psr/log": "~1.0"
},
"suggest": {
"symfony/event-dispatcher": "",
"symfony/filesystem": "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no proper error handling when this feature is used if the filesystem is not present (rare but possible).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed now

"symfony/process": "",
"psr/log": "For using the console logger"
},
Expand Down