diff --git a/src/Processor/Remote.php b/src/Processor/Remote.php index d073e1f..aff9834 100644 --- a/src/Processor/Remote.php +++ b/src/Processor/Remote.php @@ -32,8 +32,15 @@ public function setupState(array $state): array public function copy(array $state, Result $result, int $timeLimit = PHP_INT_MAX): array { + $state['source'] = $state['source'] ?? ''; $client = $this->getClient(); try { + // Use a HEAD request to discover whether the source URL is valid. + // If the HTTP client throws an exception, then we can't/shouldn't + // allow the contents to be written to destination. See the + // subclass hierarchy of GuzzleException for all the cases this + // handles. + $client->head($state['source']); $fout = fopen($state['destination'], "w"); $client->get($state['source'], ['sink' => $fout]); $result->setStatus(Result::DONE); @@ -49,7 +56,10 @@ public function copy(array $state, Result $result, int $timeLimit = PHP_INT_MAX) protected function getFileSize($path): int { clearstatcache(); - return filesize($path); + if ($size = @filesize($path) !== false) { + return $size; + } + return 0; } protected function getClient(): Client diff --git a/test/Mock/FakeRemote.php b/test/Mock/FakeRemote.php index ce1b1cb..7d887ef 100644 --- a/test/Mock/FakeRemote.php +++ b/test/Mock/FakeRemote.php @@ -19,6 +19,9 @@ protected function getClient(): Client private function getMockHandler() { $mock = new MockHandler([ + // Multiple copies because Remote can end up making more than one + // request. + new Response(200, ['X-Foo' => 'Bar'], 'Hello, World'), new Response(200, ['X-Foo' => 'Bar'], 'Hello, World'), ]); return $mock; diff --git a/test/Processor/RemoteTest.php b/test/Processor/RemoteTest.php index 5d96b89..9341234 100644 --- a/test/Processor/RemoteTest.php +++ b/test/Processor/RemoteTest.php @@ -12,6 +12,7 @@ /** * @covers \FileFetcher\Processor\Remote + * @coversDefaultClass \FileFetcher\Processor\Remote */ class RemoteTest extends TestCase { @@ -19,10 +20,10 @@ class RemoteTest extends TestCase public function testCopyAFileWithRemoteProcessor() { $config = [ - "filePath" => 'http://notreal.blah/notacsv.csv', - "processors" => [FakeRemote::class] + 'filePath' => 'http://notreal.blah/notacsv.csv', + 'processors' => [FakeRemote::class] ]; - $fetcher = FileFetcher::get("1", new Memory(), $config); + $fetcher = FileFetcher::get('1', new Memory(), $config); $fetcher->setTimeLimit(1); @@ -49,6 +50,8 @@ public function provideIsServerCompatible(): array /** * Test the \FileFetcher\Processor\Remote::isServerCompatible() method. * + * @covers ::isServerCompatible + * * @dataProvider provideIsServerCompatible */ public function testIsServerCompatible($expected, $source) @@ -57,19 +60,43 @@ public function testIsServerCompatible($expected, $source) $this->assertSame($expected, $processor->isServerCompatible(['source' => $source])); } + /** + * Ensure the status object contains the message from an exception. + */ public function testCopyException() { - // Ensure the status object contains the message from an exception. - // We'll use vfsstream to mock a file system with no permissions to - // throw an error. - $root = vfsStream::setup('root', 0000); - $state = ['destination' => $root->url()]; + $root = vfsStream::setup('no-write', 0000); + + $remote = new Remote(); + $result = new Result(); + + // State does not have a source property, so there should be a curl error. + $remote->copy(['destination' => $root->url() . 'file.txt'], $result); + + $this->assertSame(Result::ERROR, $result->getStatus()); + $this->assertStringContainsString('cURL error 3', $result->getError()); + } + + /** + * Ensure that 404 responses do not write content to disk. + */ + public function test404DoesNotCreateFile() + { + $root = vfsStream::setup('test404'); + $root_url = $root->url() . '/bad_file.csv'; + $state = [ + 'source' => 'http://example.com/bad_file.csv', + 'destination' => $root_url, + ]; $remote = new Remote(); $result = new Result(); $remote->copy($state, $result); + // Assert that there was a 404 error. $this->assertSame(Result::ERROR, $result->getStatus()); - $this->assertStringContainsString('ailed to open stream', $result->getError()); + $this->assertStringContainsString('resulted in a `404 Not Found` response', $result->getError()); + // Assert that the file was not created. + $this->assertFileDoesNotExist($root_url); } }