Skip to content

Commit

Permalink
Added protection for "svn log ... --use-merge-history ..." command, t…
Browse files Browse the repository at this point in the history
…hat might hand on large merges
  • Loading branch information
aik099 committed Jan 15, 2025
1 parent 5cdab4a commit 3f5646d
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 9 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### Changed
- Show executed SVN commands in real time (when started; how long was executed) in verbose mode (the `-v` flag).
- The executed SVN command idle timeout changed from 3 minutes to 1 minute.

### Fixed
...
- Handle cases, when `svn log ... --use-merge-history ...` command timeout-out.

## [0.8.0] - 2024-12-18
### Added
Expand Down
55 changes: 49 additions & 6 deletions src/SVNBuddy/Repository/Connector/Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@
use ConsoleHelpers\SVNBuddy\Process\IProcessFactory;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Process\Exception\ProcessFailedException;
use Symfony\Component\Process\Exception\ProcessTimedOutException;
use Symfony\Component\Process\Process;

class Command
{

const IDLE_TIMEOUT = 180; // 3 minutes.
const IDLE_TIMEOUT = 60; // 1 minute.

/**
* Process factory.
Expand Down Expand Up @@ -73,6 +74,13 @@ class Command
*/
private $_cacheOverwrite = false;

/**
* Indicates whether idle timeout recovery is enabled.
*
* @var boolean
*/
private $_idleTimeoutRecovery = false;

/**
* Creates a command instance.
*
Expand Down Expand Up @@ -135,6 +143,20 @@ public function setCacheOverwrite($cache_overwrite)
return $this;
}

/**
* Set idle timeout recovery.
*
* @param boolean $idle_timeout_recovery Idle timeout recovery.
*
* @return self
*/
public function setIdleTimeoutRecovery($idle_timeout_recovery)
{
$this->_idleTimeoutRecovery = $idle_timeout_recovery;

return $this;
}

/**
* Runs the command.
*
Expand Down Expand Up @@ -212,6 +234,7 @@ private function _getCacheKey()
*
* @return string
* @throws RepositoryCommandException When command execution failed.
* @throws ProcessTimedOutException When process timed-out with general timeout type.
*/
private function _doRun($callback = null)
{
Expand All @@ -236,13 +259,15 @@ private function _doRun($callback = null)
$process->mustRun($callback);
}

$output = (string)$process->getOutput();

if ( $this->_io->isDebug() ) {
$this->_io->writeln($output, OutputInterface::OUTPUT_RAW);
return $this->getProcessOutput($process);
}
catch ( ProcessTimedOutException $e ) {
// This happens for "svn log --use-merge-history ..." command when we've got all the output already.
if ( $this->_idleTimeoutRecovery && $e->isIdleTimeout() ) {
return $this->getProcessOutput($process);
}

return $output;
throw $e;
}
catch ( ProcessFailedException $e ) {
throw new RepositoryCommandException(
Expand All @@ -252,6 +277,24 @@ private function _doRun($callback = null)
}
}

/**
* Returns process output.
*
* @param Process $process Process.
*
* @return string
*/
protected function getProcessOutput(Process $process)
{
$output = (string)$process->getOutput();

if ( $this->_io->isDebug() ) {
$this->_io->writeln($output, OutputInterface::OUTPUT_RAW);
}

return $output;
}

/**
* Runs an svn command and displays output in real time.
*
Expand Down
2 changes: 1 addition & 1 deletion src/SVNBuddy/Repository/RevisionLog/RevisionLog.php
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ private function _useRepositoryCollectorPlugins($from_revision, $to_revision, $o
$log_command_arguments
);
$command = $this->_repositoryConnector->getCommand('log', $command_arguments);
$command->setCacheDuration($cache_duration);
$command->setCacheDuration($cache_duration)->setIdleTimeoutRecovery(true);
$svn_log = $command->run();

$this->_parseLog($svn_log, $plugins);
Expand Down
60 changes: 60 additions & 0 deletions tests/SVNBuddy/Repository/Connector/CommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Prophecy\Prophecy\ObjectProphecy;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Process\Exception\ProcessFailedException;
use Symfony\Component\Process\Exception\ProcessTimedOutException;
use Symfony\Component\Process\Process;
use Tests\ConsoleHelpers\SVNBuddy\AbstractTestCase;

Expand Down Expand Up @@ -126,6 +127,65 @@ public static function runWithoutCachingDataProvider()
);
}

/**
* @dataProvider runWithIdleTimeoutRecoveryDataProvider
*/
public function testRunWithIdleTimeoutRecovery($use_recovery, $timeout_type)
{
$command_line = array('svn', 'log', '--xml');
$process_output = '<log><logentry/></log>';

$this->_command = $this->_createCommand($command_line);

if ( $use_recovery ) {
$this->_command->setIdleTimeoutRecovery(true);
}

$this->_process->getCommandLine()
->willReturn(implode(' ', $command_line))
->shouldBeCalled();

if ( $timeout_type === ProcessTimedOutException::TYPE_IDLE ) {
$this->_process->getIdleTimeout()->willReturn(123)->shouldBeCalled();
}
else {
$this->_process->getTimeout()->willReturn(123)->shouldBeCalled();
}

$process_timed_out_exception = new ProcessTimedOutException(
$this->_process->reveal(),
$timeout_type
);

$this->_process->mustRun(null)
->willThrow($process_timed_out_exception)
->shouldBeCalled();

$this->_cacheManager->getCache(Argument::any())->shouldNotBeCalled();

if ( $use_recovery ) {
$this->_process->getOutput()->willReturn($process_output)->shouldBeCalled();
$this->_io->isVerbose()->willReturn(false)->shouldBeCalled();
$this->_io->isDebug()->willReturn(false)->shouldBeCalled();
}
else {
$this->expectException(get_class($process_timed_out_exception));
$this->expectExceptionMessage($process_timed_out_exception->getMessage());
}

$this->assertCommandOutput(null, true, $process_output);
}

public static function runWithIdleTimeoutRecoveryDataProvider()
{
return array(
'with recovery+idle timeout' => array(true, ProcessTimedOutException::TYPE_IDLE),
'with recovery+general timeout' => array(true, ProcessTimedOutException::TYPE_IDLE),
'without recovery+idle timeout' => array(false, ProcessTimedOutException::TYPE_GENERAL),
'without recovery+general timeout' => array(false, ProcessTimedOutException::TYPE_GENERAL),
);
}

/**
* @dataProvider runWithCacheDataProvider
*/
Expand Down
3 changes: 2 additions & 1 deletion tests/SVNBuddy/Repository/RevisionLog/RevisionLogTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,8 @@ protected function expectRepositoryCommand($command_name, array $arguments, $res

$command = $this->prophesize(Command::class);
$command->run()->willReturn($result)->shouldBeCalled();
$command->setCacheDuration('10 years')->shouldBeCalled();
$command->setCacheDuration('10 years')->willReturn($command)->shouldBeCalled();
$command->setIdleTimeoutRecovery(true)->willReturn($command)->shouldBeCalled();

$this->repositoryConnector->getCommand($command_name, $arguments)->willReturn($command)->shouldBeCalled();
}
Expand Down

0 comments on commit 3f5646d

Please sign in to comment.