From 8fb9c4bf3beeb0d07d8e35495ba0f4dd53c8fd3e Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sat, 24 Mar 2012 23:05:13 +0100 Subject: [PATCH] Svn related coding style/consistency fixes and minor improvements --- src/Composer/Downloader/SvnDownloader.php | 11 +-- src/Composer/Repository/Vcs/SvnDriver.php | 8 +- src/Composer/Util/Svn.php | 82 ++++++++----------- .../Test/Repository/Vcs/SvnDriverTest.php | 3 - 4 files changed, 43 insertions(+), 61 deletions(-) diff --git a/src/Composer/Downloader/SvnDownloader.php b/src/Composer/Downloader/SvnDownloader.php index 6ded2e93f..20c9384f4 100644 --- a/src/Composer/Downloader/SvnDownloader.php +++ b/src/Composer/Downloader/SvnDownloader.php @@ -68,7 +68,7 @@ class SvnDownloader extends VcsDownloader */ protected function enforceCleanDirectory($path) { - $this->process->execute(sprintf('cd %s && svn status', escapeshellarg($path)), $output); + $this->process->execute('svn status', $output, $path); if (trim($output)) { throw new \RuntimeException('Source directory ' . $path . ' has uncommitted changes'); } @@ -90,7 +90,6 @@ class SvnDownloader extends VcsDownloader } // this could be any failure, since SVN exits with 1 always - if (empty($output)) { $output = $this->process->getErrorOutput(); } @@ -100,7 +99,7 @@ class SvnDownloader extends VcsDownloader } // the error is not auth-related - if (false === strpos($output, 'authorization failed:')) { + if (false === stripos($output, 'authorization failed:')) { return $output; } @@ -114,19 +113,17 @@ class SvnDownloader extends VcsDownloader } else { $this->io->write("Authorization failed: {$command}"); } + return $output; } /** - * This is potentially heavy - recreating Util often. - * * @param string $url * * @return \Composer\Util\Svn */ protected function getUtil($url) { - $util = new SvnUtil($url, $this->io); - return $util; + return new SvnUtil($url, $this->io); } } diff --git a/src/Composer/Repository/Vcs/SvnDriver.php b/src/Composer/Repository/Vcs/SvnDriver.php index 4db0df6ec..1183229dd 100644 --- a/src/Composer/Repository/Vcs/SvnDriver.php +++ b/src/Composer/Repository/Vcs/SvnDriver.php @@ -83,7 +83,7 @@ class SvnDriver extends VcsDriver * * @return string */ - public function execute($command, $url) + protected function execute($command, $url) { $svnCommand = $this->util->getCommand($command, $url); @@ -102,7 +102,7 @@ class SvnDriver extends VcsDriver } // the error is not auth-related - if (strpos($output, 'authorization failed:') === false) { + if (false === stripos($output, 'authorization failed:')) { return $output; } @@ -281,11 +281,13 @@ class SvnDriver extends VcsDriver // This is definitely a Subversion repository. return true; } - if (preg_match('/authorization failed/i', $processExecutor->getErrorOutput())) { + + if (false !== stripos($processExecutor->getErrorOutput(), 'authorization failed:')) { // This is likely a remote Subversion repository that requires // authentication. We will handle actual authentication later. return true; } + return false; } diff --git a/src/Composer/Util/Svn.php b/src/Composer/Util/Svn.php index 93de9c474..0688b8451 100644 --- a/src/Composer/Util/Svn.php +++ b/src/Composer/Util/Svn.php @@ -16,11 +16,12 @@ use Composer\IO\IOInterface; /** * @author Till Klampaeckel + * @author Jordi Boggiano */ class Svn { /** - * @var mixed + * @var array */ protected $credentials; @@ -40,10 +41,9 @@ class Svn protected $url; /** - * Cache credentials. * @var bool */ - protected $useCache = false; + protected $cacheCredentials = false; /** * @param string $url @@ -66,30 +66,14 @@ class Svn { $this->io->write("The Subversion server ({$this->url}) requested credentials:"); - $this->hasAuth = true; - $this->credentials = new \stdClass(); + $this->hasAuth = true; + $this->credentials['username'] = $this->io->ask("Username: "); + $this->credentials['password'] = $this->io->askAndHideAnswer("Password: "); - $this->credentials->username = $this->io->ask("Username: "); - $this->credentials->password = $this->io->askAndHideAnswer("Password: "); + $this->cacheCredentials = $this->io->askConfirmation("Should Subversion cache these credentials? (yes/no) ", false); - $pleaseCache = $this->io->askConfirmation("Should Subversion cache these credentials? (yes/no) ", false); - if ($pleaseCache) { - $this->useCache = true; - } return $this; } - /** - * Return the no-auth-cache switch. - * - * @return string - */ - public function getAuthCache() - { - if (!$this->useCache) { - return '--no-auth-cache '; - } - return ''; - } /** * A method to create the svn commands run. @@ -100,7 +84,7 @@ class Svn * * @return string */ - public function getCommand($cmd, $url, $path = '') + public function getCommand($cmd, $url, $path = null) { $cmd = sprintf('%s %s%s %s', $cmd, @@ -108,7 +92,8 @@ class Svn $this->getCredentialString(), escapeshellarg($url) ); - if (!empty($path)) { + + if ($path) { $cmd .= ' ' . escapeshellarg($path); } @@ -121,16 +106,13 @@ class Svn * Adds --no-auth-cache when credentials are present. * * @return string - * @uses self::$useAuth */ public function getCredentialString() { - if ($this->hasAuth === null) { - $this->hasAuth(); - } - if (!$this->hasAuth) { + if (!$this->hasAuth()) { return ''; } + return sprintf( ' %s--username %s --password %s ', $this->getAuthCache(), @@ -148,12 +130,10 @@ class Svn public function getPassword() { if ($this->credentials === null) { - throw new \LogicException("No auth detected."); + throw new \LogicException("No svn auth detected."); } - if (isset($this->credentials->password)) { - return $this->credentials->password; - } - return ''; // could be empty + + return isset($this->credentials['password']) ? $this->credentials['password'] : ''; } /** @@ -165,9 +145,10 @@ class Svn public function getUsername() { if ($this->credentials === null) { - throw new \LogicException("No auth detected."); + throw new \LogicException("No svn auth detected."); } - return $this->credentials->username; + + return $this->credentials['username']; } /** @@ -175,29 +156,34 @@ class Svn * * @param string $url * - * @return \stdClass + * @return Boolean */ public function hasAuth() { - if ($this->hasAuth !== null) { + if (null !== $this->hasAuth) { return $this->hasAuth; } $uri = parse_url($this->url); if (empty($uri['user'])) { - $this->hasAuth = false; - return $this->hasAuth; + return $this->hasAuth = false; } - $this->hasAuth = true; - $this->credentials = new \stdClass(); - - $this->credentials->username = $uri['user']; - + $this->credentials['username'] = $uri['user']; if (!empty($uri['pass'])) { - $this->credentials->password = $uri['pass']; + $this->credentials['password'] = $uri['pass']; } - return $this->hasAuth; + return $this->hasAuth = true; + } + + /** + * Return the no-auth-cache switch. + * + * @return string + */ + protected function getAuthCache() + { + return $this->cacheCredentials ? '' : '--no-auth-cache '; } } \ No newline at end of file diff --git a/tests/Composer/Test/Repository/Vcs/SvnDriverTest.php b/tests/Composer/Test/Repository/Vcs/SvnDriverTest.php index 9e85c5129..5cbdf7b2b 100644 --- a/tests/Composer/Test/Repository/Vcs/SvnDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/SvnDriverTest.php @@ -15,9 +15,6 @@ namespace Composer\Test\Repository\Vcs; use Composer\Repository\Vcs\SvnDriver; use Composer\IO\NullIO; -/** - * @author Till Klampaeckel - */ class SvnDriverTest extends \PHPUnit_Framework_TestCase { /**