Remove code duplication in Svn classes

src/Composer/Downloader/SvnDownloader.php

@@ -22,16 +22,6 @@ use Composer\Util\Svn as SvnUtil;
  */
 class SvnDownloader extends VcsDownloader
 {
-    /**
-     * @var bool
-     */
-    protected $useAuth = false;
-
-    /**
-     * @var \Composer\Util\Svn
-     */
-    protected $util;
-
     /**
      * {@inheritDoc}
      */
@@ -40,12 +30,8 @@ class SvnDownloader extends VcsDownloader
         $url =  $package->getSourceUrl();
         $ref =  $package->getSourceReference();
 
-        $util = $this->getUtil($url);
-
-        $command = $util->getCommand("svn co", sprintf("%s/%s", $url, $ref), $path);
-
         $this->io->write("    Checking out ".$package->getSourceReference());
-        $this->execute($command, $util);
+        $this->execute($url, "svn co", sprintf("%s/%s", $url, $ref), null, $path);
     }
 
     /**
@@ -56,11 +42,8 @@ class SvnDownloader extends VcsDownloader
         $url = $target->getSourceUrl();
         $ref = $target->getSourceReference();
 
-        $util    = $this->getUtil($url);
-        $command = $util->getCommand("svn switch", sprintf("%s/%s", $url, $ref));
-
         $this->io->write("    Checking out " . $ref);
-        $this->execute(sprintf('cd %s && %s', $path, $command), $util);
+        $this->execute($url, "svn switch", sprintf("%s/%s", $url, $ref), $path);
     }
 
     /**
@@ -75,55 +58,26 @@ class SvnDownloader extends VcsDownloader
     }
 
     /**
-     * Wrap {@link \Composer\Util\ProcessExecutor::execute().
+     * Execute an SVN command and try to fix up the process with credentials
+     * if necessary.
      *
-     * @param string  $cmd
+     * @param string $baseUrl Base URL of the repository
-     * @param SvnUtil $util
+     * @param string $command SVN command to run
+     * @param string $url     SVN url
+     * @param string $cwd     Working directory
+     * @param string $path    Target for a checkout
      *
      * @return string
      */
-    protected function execute($command, SvnUtil $util)
+    protected function execute($baseUrl, $command, $url, $cwd = null, $path = null)
     {
-        $status = $this->process->execute($command, $output);
+        $util = new SvnUtil($baseUrl, $this->io);
-        if (0 === $status) {
+        try {
-            return $output;
+            return $util->execute($command, $url, $cwd, $path);
+        } catch (\RuntimeException $e) {
+            throw new \RuntimeException(
+                'Package could not be downloaded, '.$e->getMessage()
+            );
         }
-
-        // this could be any failure, since SVN exits with 1 always
-        if (empty($output)) {
-            $output = $this->process->getErrorOutput();
-        }
-
-        if (!$this->io->isInteractive()) {
-            return $output;
-        }
-
-        // the error is not auth-related
-        if (false === stripos($output, 'authorization failed:')) {
-            return $output;
-        }
-
-        // no authorization has been detected so far
-        if (!$this->useAuth) {
-            $this->useAuth = $util->doAuthDance()->hasAuth();
-            $credentials   = $util->getCredentialString();
-
-            // restart the process
-            $output = $this->execute($command . ' ' . $credentials, $util);
-        } else {
-            $this->io->write("Authorization failed: {$command}");
-        }
-
-        return $output;
-    }
-
-    /**
-     * @param string $url
-     *
-     * @return \Composer\Util\Svn
-     */
-    protected function getUtil($url)
-    {
-        return new SvnUtil($url, $this->io);
     }
 }

src/Composer/Repository/Vcs/SvnDriver.php

@@ -29,28 +29,6 @@ class SvnDriver extends VcsDriver
     protected $branches;
     protected $infoCache = array();
 
-    /**
-     * Contains credentials, or not?
-     * @var boolean
-     */
-    protected $useAuth = false;
-
-    /**
-     * To determine if we should cache the credentials supplied by the user. By default: no cache.
-     * @var boolean
-     */
-    protected $useCache = false;
-
-    /**
-     * @var string
-     */
-    protected $svnUsername = '';
-
-    /**
-     * @var string
-     */
-    protected $svnPassword = '';
-
     /**
      * @var \Composer\Util\Svn
      */
@@ -71,13 +49,12 @@ class SvnDriver extends VcsDriver
         if (false !== ($pos = strrpos($url, '/trunk'))) {
             $this->baseUrl = substr($url, 0, $pos);
         }
-        $this->util    = new SvnUtil($this->baseUrl, $io);
+        $this->util    = new SvnUtil($this->baseUrl, $io, $this->process);
-        $this->useAuth = $this->util->hasAuth();
     }
 
     /**
      * Execute an SVN command and try to fix up the process with credentials
-     * if necessary. The command is 'fixed up' with {@link self::getSvnCommand()}.
+     * if necessary.
      *
      * @param string $command The svn command to run.
      * @param string $url     The SVN URL.
@@ -86,37 +63,13 @@ class SvnDriver extends VcsDriver
      */
     protected function execute($command, $url)
     {
-        $svnCommand = $this->util->getCommand($command, $url);
+        try {
-
+            return $this->util->execute($command, $url);
-        $status = $this->process->execute(
+        } catch (\RuntimeException $e) {
-            $svnCommand,
+            throw new \RuntimeException(
-            $output
+                'Repository '.$this->url.' could not be processed, '.$e->getMessage()
             );
-
-        if (0 === $status) {
-            return $output;
         }
-
-        // this could be any failure, since SVN exits with 1 always
-        if (!$this->io->isInteractive()) {
-            return $output;
-        }
-
-        // the error is not auth-related
-        if (false === stripos($output, 'authorization failed:')) {
-            return $output;
-        }
-
-        // no authorization has been detected so far
-        if (!$this->useAuth) {
-            $this->useAuth = $this->util->doAuthDance()->hasAuth();
-
-            // restart the process
-            $output = $this->execute($command, $url);
-        } else {
-            $this->io->write("Authorization failed: {$svnCommand}");
-        }
-        return $output;
     }
 
     /**

src/Composer/Util/Svn.php

@@ -45,16 +45,71 @@ class Svn
      */
     protected $cacheCredentials = true;
 
+    /**
+     * @var ProcessExecutor
+     */
+    protected $process;
+
     /**
      * @param string                   $url
      * @param \Composer\IO\IOInterface $io
-     *
+     * @param ProcessExecutor          $process
-     * @return \Composer\Util\Svn
      */
-    public function __construct($url, IOInterface $io)
+    public function __construct($url, IOInterface $io, ProcessExecutor $process = null)
     {
         $this->url = $url;
         $this->io  = $io;
+        $this->process = $process ?: new ProcessExecutor;
+    }
+
+    /**
+     * Execute an SVN command and try to fix up the process with credentials
+     * if necessary.
+     *
+     * @param string $command SVN command to run
+     * @param string $url     SVN url
+     * @param string $cwd     Working directory
+     * @param string $path Target for a checkout
+     *
+     * @return string
+     *
+     * @throws \RuntimeException
+     */
+    public function execute($command, $url, $cwd = null, $path = null)
+    {
+        $svnCommand = $this->getCommand($command, $url, $path);
+        $status = $this->process->execute($svnCommand, $output, $cwd);
+        if (0 === $status) {
+            return $output;
+        }
+
+        if (empty($output)) {
+            $output = $this->process->getErrorOutput();
+        }
+
+        // the error is not auth-related
+        if (false === stripos($output, 'authorization failed:')) {
+            throw new \RuntimeException('Package could not be downloaded: '.$output);
+        }
+
+        // no auth supported for non interactive calls
+        if (!$this->io->isInteractive()) {
+            throw new \RuntimeException(
+                'Package could not be downloaded, can not ask for authentication in non interactive mode ('.$output.')'
+            );
+        }
+
+        // try to authenticate
+        if (!$this->hasAuth()) {
+            $this->doAuthDance();
+
+            // restart the process
+            return $this->execute($command, $url, $cwd, $path);
+        }
+
+        throw new \RuntimeException(
+            'Repository '.$this->url.' could not be processed, wrong credentials provided ('.$output.')'
+        );
     }
 
     /**
@@ -62,7 +117,7 @@ class Svn
      *
      * @return \Composer\Util\Svn
      */
-    public function doAuthDance()
+    protected function doAuthDance()
     {
         $this->io->write("The Subversion server ({$this->url}) requested credentials:");
 
@@ -80,11 +135,11 @@ class Svn
      *
      * @param string $cmd  Usually 'svn ls' or something like that.
      * @param string $url  Repo URL.
-     * @param string $path The path to run this against (e.g. a 'co' into)
+     * @param string $path Target for a checkout
      *
      * @return string
      */
-    public function getCommand($cmd, $url, $path = null)
+    protected function getCommand($cmd, $url, $path = null)
     {
         $cmd = sprintf('%s %s%s %s',
             $cmd,
@@ -107,7 +162,7 @@ class Svn
      *
      * @return string
      */
-    public function getCredentialString()
+    protected function getCredentialString()
     {
         if (!$this->hasAuth()) {
             return '';
@@ -127,7 +182,7 @@ class Svn
      * @return string
      * @throws \LogicException
      */
-    public function getPassword()
+    protected function getPassword()
     {
         if ($this->credentials === null) {
             throw new \LogicException("No svn auth detected.");
@@ -142,7 +197,7 @@ class Svn
      * @return string
      * @throws \LogicException
      */
-    public function getUsername()
+    protected function getUsername()
     {
         if ($this->credentials === null) {
             throw new \LogicException("No svn auth detected.");
@@ -158,7 +213,7 @@ class Svn
      *
      * @return Boolean
      */
-    public function hasAuth()
+    protected function hasAuth()
     {
         if (null !== $this->hasAuth) {
             return $this->hasAuth;

tests/Composer/Test/Repository/Vcs/SvnDriverTest.php

@@ -18,12 +18,10 @@ use Composer\IO\NullIO;
 class SvnDriverTest extends \PHPUnit_Framework_TestCase
 {
     /**
-     * Test the execute method.
+     * @expectedException RuntimeException
      */
-    public function testExecute()
+    public function testWrongCredentialsInUrl()
     {
-        $this->markTestIncomplete("Currently no way to mock the output value which is passed by reference.");
-
         $console = $this->getMock('Composer\IO\IOInterface');
         $console->expects($this->once())
             ->method('isInteractive')
@@ -37,9 +35,12 @@ class SvnDriverTest extends \PHPUnit_Framework_TestCase
         $process->expects($this->once())
             ->method('execute')
             ->will($this->returnValue(1));
+        $process->expects($this->once())
+            ->method('getErrorOutput')
+            ->will($this->returnValue($output));
 
         $svn = new SvnDriver('http://till:secret@corp.svn.local/repo', $console, $process);
-        $svn->execute('svn ls', 'http://corp.svn.local/repo');
+        $svn->getTags();
     }
 
     private function getCmd($cmd)