Additional Util\RemoteFileSystem tests (#8960)

* RemoteFilesystemTest: simplifying some mock expectations calls

- will($this->returnValue()) to willReturn()
- will($this->returnCallBack()) to willReturnCallback()

* RemoteFilesystemTest: extracting identical mocks for IOInterface into a separate getIOInterfaceMock() method

* RemoteFilesystemTest: converting protected helper methods to private.

* RemoteFilesystemTest: moving getConfigMock() private method after the public methods (with other private methods)

* adding RemoteFileSystemTest::testCopyWithRetryAuthFailureFalse() unit test.

* Allow optional injecting of AuthHelper into RemoteFilesystem constructor.

* adding RemoteFileSystemTest::testCopyWithSuccessOnRetry() unit test.

* using backward compatible @expectedException in RemoteFilesystemTest.php

* RemoteFilesystemTest: extracting RemoteFilesystem with mocked method creation into a separate method.

* RemoteFilesystemTest: extracting AuthHelper with mocked method creation into a separate method.

src/Composer/Util/RemoteFilesystem.php

@@ -54,8 +54,9 @@ class RemoteFilesystem
      * @param Config      $config     The config
      * @param array       $options    The options
      * @param bool        $disableTls
+     * @param AuthHelper  $authHelper
      */
-    public function __construct(IOInterface $io, Config $config, array $options = array(), $disableTls = false)
+    public function __construct(IOInterface $io, Config $config, array $options = array(), $disableTls = false, AuthHelper $authHelper = null)
     {
         $this->io = $io;
 
@@ -70,7 +71,7 @@ class RemoteFilesystem
         // handle the other externally set options normally.
         $this->options = array_replace_recursive($this->options, $options);
         $this->config = $config;
-        $this->authHelper = new AuthHelper($io, $config);
+        $this->authHelper = isset($authHelper) ? $authHelper : new AuthHelper($io, $config);
     }
 
     /**

tests/Composer/Test/Util/RemoteFilesystemTest.php

@@ -12,32 +12,25 @@
 
 namespace Composer\Test\Util;
 
+use Composer\Config;
+use Composer\IO\ConsoleIO;
+use Composer\IO\IOInterface;
+use Composer\Util\AuthHelper;
 use Composer\Util\RemoteFilesystem;
 use Composer\Test\TestCase;
+use PHPUnit\Framework\MockObject\MockObject;
+use ReflectionMethod;
+use ReflectionProperty;
 
 class RemoteFilesystemTest extends TestCase
 {
-    private function getConfigMock()
-    {
-        $config = $this->getMockBuilder('Composer\Config')->getMock();
-        $config->expects($this->any())
-            ->method('get')
-            ->will($this->returnCallback(function ($key) {
-                if ($key === 'github-domains' || $key === 'gitlab-domains') {
-                    return array();
-                }
-            }));
-
-        return $config;
-    }
-
     public function testGetOptionsForUrl()
     {
-        $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
+        $io = $this->getIOInterfaceMock();
         $io
             ->expects($this->once())
             ->method('hasAuthentication')
-            ->will($this->returnValue(false))
+            ->willReturn(false)
         ;
 
         $res = $this->callGetOptionsForUrl($io, array('http://example.org', array()));
@@ -46,16 +39,16 @@ class RemoteFilesystemTest extends TestCase
 
     public function testGetOptionsForUrlWithAuthorization()
     {
-        $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
+        $io = $this->getIOInterfaceMock();
         $io
             ->expects($this->once())
             ->method('hasAuthentication')
-            ->will($this->returnValue(true))
+            ->willReturn(true)
         ;
         $io
             ->expects($this->once())
             ->method('getAuthentication')
-            ->will($this->returnValue(array('username' => 'login', 'password' => 'password')))
+            ->willReturn(array('username' => 'login', 'password' => 'password'))
         ;
 
         $options = $this->callGetOptionsForUrl($io, array('http://example.org', array()));
@@ -71,17 +64,17 @@ class RemoteFilesystemTest extends TestCase
 
     public function testGetOptionsForUrlWithStreamOptions()
     {
-        $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
+        $io = $this->getIOInterfaceMock();
         $io
             ->expects($this->once())
             ->method('hasAuthentication')
-            ->will($this->returnValue(true))
+            ->willReturn(true)
         ;
 
         $io
             ->expects($this->once())
             ->method('getAuthentication')
-            ->will($this->returnValue(array('username' => null, 'password' => null)))
+            ->willReturn(array('username' => null, 'password' => null))
         ;
 
         $streamOptions = array('ssl' => array(
@@ -94,17 +87,17 @@ class RemoteFilesystemTest extends TestCase
 
     public function testGetOptionsForUrlWithCallOptionsKeepsHeader()
     {
-        $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
+        $io = $this->getIOInterfaceMock();
         $io
             ->expects($this->once())
             ->method('hasAuthentication')
-            ->will($this->returnValue(true))
+            ->willReturn(true)
         ;
 
         $io
             ->expects($this->once())
             ->method('getAuthentication')
-            ->will($this->returnValue(array('username' => null, 'password' => null)))
+            ->willReturn(array('username' => null, 'password' => null))
         ;
 
         $streamOptions = array('http' => array(
@@ -127,14 +120,14 @@ class RemoteFilesystemTest extends TestCase
 
     public function testCallbackGetFileSize()
     {
-        $fs = new RemoteFilesystem($this->getMockBuilder('Composer\IO\IOInterface')->getMock(), $this->getConfigMock());
+        $fs = new RemoteFilesystem($this->getIOInterfaceMock(), $this->getConfigMock());
         $this->callCallbackGet($fs, STREAM_NOTIFY_FILE_SIZE_IS, 0, '', 0, 0, 20);
         $this->assertAttributeEquals(20, 'bytesMax', $fs);
     }
 
     public function testCallbackGetNotifyProgress()
     {
-        $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
+        $io = $this->getIOInterfaceMock();
         $io
             ->expects($this->once())
             ->method('overwriteError')
@@ -150,21 +143,21 @@ class RemoteFilesystemTest extends TestCase
 
     public function testCallbackGetPassesThrough404()
     {
-        $fs = new RemoteFilesystem($this->getMockBuilder('Composer\IO\IOInterface')->getMock(), $this->getConfigMock());
+        $fs = new RemoteFilesystem($this->getIOInterfaceMock(), $this->getConfigMock());
 
         $this->assertNull($this->callCallbackGet($fs, STREAM_NOTIFY_FAILURE, 0, 'HTTP/1.1 404 Not Found', 404, 0, 0));
     }
 
     public function testGetContents()
     {
-        $fs = new RemoteFilesystem($this->getMockBuilder('Composer\IO\IOInterface')->getMock(), $this->getConfigMock());
+        $fs = new RemoteFilesystem($this->getIOInterfaceMock(), $this->getConfigMock());
 
         $this->assertContains('testGetContents', $fs->getContents('http://example.org', 'file://'.__FILE__));
     }
 
     public function testCopy()
     {
-        $fs = new RemoteFilesystem($this->getMockBuilder('Composer\IO\IOInterface')->getMock(), $this->getConfigMock());
+        $fs = new RemoteFilesystem($this->getIOInterfaceMock(), $this->getConfigMock());
 
         $file = tempnam(sys_get_temp_dir(), 'c');
         $this->assertTrue($fs->copy('http://example.org', 'file://'.__FILE__, $file));
@@ -173,17 +166,96 @@ class RemoteFilesystemTest extends TestCase
         unlink($file);
     }
 
+    /**
+     * @expectedException \Composer\Downloader\TransportException
+     */
+    public function testCopyWithNoRetryOnFailure()
+    {
+        $fs = $this->getRemoteFilesystemWithMockedMethods(array('getRemoteContents'));
+
+        $fs->expects($this->once())->method('getRemoteContents')
+            ->willReturnCallback(function ($originUrl, $fileUrl, $ctx, &$http_response_header) {
+
+                $http_response_header = array('http/1.1 401 unauthorized');
+
+                return '';
+
+            });
+
+
+        $file = tempnam(sys_get_temp_dir(), 'z');
+        unlink($file);
+
+        $fs->copy(
+            'http://example.org',
+            'file://' . __FILE__,
+            $file,
+            true,
+            array('retry-auth-failure' => false)
+        );
+    }
+
+    public function testCopyWithSuccessOnRetry()
+    {
+        $authHelper = $this->getAuthHelperWithMockedMethods(array('promptAuthIfNeeded'));
+        $fs = $this->getRemoteFilesystemWithMockedMethods(array('getRemoteContents'), $authHelper);
+
+        $authHelper->expects($this->once())
+            ->method('promptAuthIfNeeded')
+            ->willReturn(array(
+                'storeAuth' => true,
+                'retry' => true
+            ));
+
+        $fs->expects($this->at(0))
+            ->method('getRemoteContents')
+            ->willReturnCallback(function ($originUrl, $fileUrl, $ctx, &$http_response_header) {
+
+                $http_response_header = array('http/1.1 401 unauthorized');
+
+                return '';
+
+            });
+
+        $fs->expects($this->at(1))
+            ->method('getRemoteContents')
+            ->willReturnCallback(function ($originUrl, $fileUrl, $ctx, &$http_response_header) {
+
+                $http_response_header = array('http/1.1 200 OK');
+
+                return '<?php $copied = "Copied"; ';
+
+            });
+
+
+        $file = tempnam(sys_get_temp_dir(), 'z');
+
+        $copyResult = $fs->copy(
+            'http://example.org',
+            'file://' . __FILE__,
+            $file,
+            true,
+            array('retry-auth-failure' => true)
+        );
+
+        $this->assertTrue($copyResult);
+        $this->assertFileExists($file);
+        $this->assertContains('Copied', file_get_contents($file));
+
+        unlink($file);
+    }
+
     /**
      * @group TLS
      */
     public function testGetOptionsForUrlCreatesSecureTlsDefaults()
     {
-        $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
+        $io = $this->getIOInterfaceMock();
 
         $res = $this->callGetOptionsForUrl($io, array('example.org', array('ssl' => array('cafile' => '/some/path/file.crt'))), array(), 'http://www.example.org');
 
         $this->assertTrue(isset($res['ssl']['ciphers']));
-        $this->assertRegExp("|!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5:!PSK:!aECDH:!EDH-DSS-DES-CBC3-SHA:!EDH-RSA-DES-CBC3-SHA:!KRB5-DES-CBC3-SHA|", $res['ssl']['ciphers']);
+        $this->assertRegExp('|!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5:!PSK:!aECDH:!EDH-DSS-DES-CBC3-SHA:!EDH-RSA-DES-CBC3-SHA:!KRB5-DES-CBC3-SHA|', $res['ssl']['ciphers']);
         $this->assertTrue($res['ssl']['verify_peer']);
         $this->assertTrue($res['ssl']['SNI_enabled']);
         $this->assertEquals(7, $res['ssl']['verify_depth']);
@@ -220,6 +292,7 @@ class RemoteFilesystemTest extends TestCase
      */
     public function testBitBucketPublicDownload($url, $contents)
     {
+        /** @var ConsoleIO $io */
         $io = $this
             ->getMockBuilder('Composer\IO\ConsoleIO')
             ->disableOriginalConstructor()
@@ -242,6 +315,7 @@ class RemoteFilesystemTest extends TestCase
      */
     public function testBitBucketPublicDownloadWithAuthConfigured($url, $contents)
     {
+        /** @var MockObject|ConsoleIO $io */
         $io = $this
             ->getMockBuilder('Composer\IO\ConsoleIO')
             ->disableOriginalConstructor()
@@ -249,13 +323,12 @@ class RemoteFilesystemTest extends TestCase
 
         $domains = array();
         $io
-            ->expects($this->any())
             ->method('hasAuthentication')
-            ->will($this->returnCallback(function ($arg) use (&$domains) {
+            ->willReturnCallback(function ($arg) use (&$domains) {
                 $domains[] = $arg;
                 // first time is called with bitbucket.org, then it redirects to bbuseruploads.s3.amazonaws.com so next time we have no auth configured
                 return $arg === 'bitbucket.org';
-            }));
+            });
         $io
             ->expects($this->at(1))
             ->method('getAuthentication')
@@ -275,11 +348,11 @@ class RemoteFilesystemTest extends TestCase
         $this->assertEquals(array('bitbucket.org', 'bbuseruploads.s3.amazonaws.com'), $domains);
     }
 
-    protected function callGetOptionsForUrl($io, array $args = array(), array $options = array(), $fileUrl = '')
+    private function callGetOptionsForUrl($io, array $args = array(), array $options = array(), $fileUrl = '')
     {
         $fs = new RemoteFilesystem($io, $this->getConfigMock(), $options);
-        $ref = new \ReflectionMethod($fs, 'getOptionsForUrl');
+        $ref = new ReflectionMethod($fs, 'getOptionsForUrl');
-        $prop = new \ReflectionProperty($fs, 'fileUrl');
+        $prop = new ReflectionProperty($fs, 'fileUrl');
         $ref->setAccessible(true);
         $prop->setAccessible(true);
 
@@ -288,17 +361,80 @@ class RemoteFilesystemTest extends TestCase
         return $ref->invokeArgs($fs, $args);
     }
 
-    protected function callCallbackGet(RemoteFilesystem $fs, $notificationCode, $severity, $message, $messageCode, $bytesTransferred, $bytesMax)
+    /**
+     * @return MockObject|Config
+     */
+    private function getConfigMock()
     {
-        $ref = new \ReflectionMethod($fs, 'callbackGet');
+        $config = $this->getMockBuilder('Composer\Config')->getMock();
+        $config
+            ->method('get')
+            ->willReturnCallback(function ($key) {
+                if ($key === 'github-domains' || $key === 'gitlab-domains') {
+                    return array();
+                }
+
+                return null;
+            });
+
+        return $config;
+    }
+
+    private function callCallbackGet(RemoteFilesystem $fs, $notificationCode, $severity, $message, $messageCode, $bytesTransferred, $bytesMax)
+    {
+        $ref = new ReflectionMethod($fs, 'callbackGet');
         $ref->setAccessible(true);
         $ref->invoke($fs, $notificationCode, $severity, $message, $messageCode, $bytesTransferred, $bytesMax);
     }
 
-    protected function setAttribute($object, $attribute, $value)
+    private function setAttribute($object, $attribute, $value)
     {
-        $attr = new \ReflectionProperty($object, $attribute);
+        $attr = new ReflectionProperty($object, $attribute);
         $attr->setAccessible(true);
         $attr->setValue($object, $value);
     }
+
+    /**
+     * @return MockObject|IOInterface
+     */
+    private function getIOInterfaceMock()
+    {
+        return $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
+    }
+
+    /**
+     * @param array $mockedMethods
+     * @param AuthHelper $authHelper
+     *
+     * @return RemoteFilesystem|MockObject
+     */
+    private function getRemoteFilesystemWithMockedMethods(array $mockedMethods, AuthHelper $authHelper = null)
+    {
+        return $this->getMockBuilder('Composer\Util\RemoteFilesystem')
+            ->setConstructorArgs(array(
+                $this->getIOInterfaceMock(),
+                $this->getConfigMock(),
+                array(),
+                false,
+                $authHelper
+            ))
+            ->setMethods($mockedMethods)
+            ->getMock();
+    }
+
+    /**
+     * @param array $mockedMethods
+     *
+     * @return AuthHelper|MockObject
+     */
+    private function getAuthHelperWithMockedMethods(array $mockedMethods)
+    {
+        return $this->getMockBuilder('Composer\Util\AuthHelper')
+            ->setConstructorArgs(array(
+                $this->getIOInterfaceMock(),
+                $this->getConfigMock()
+            ))
+            ->setMethods($mockedMethods)
+            ->getMock();
+    }
 }