From 24f75c3e008670804f1da7aa9448f9e9f02280e7 Mon Sep 17 00:00:00 2001 From: Paul Mitchum Date: Thu, 19 Jan 2023 16:36:21 -0800 Subject: [PATCH 1/5] bandaid fixes for header parsing --- composer.json | 12 +++++++++-- src/Processor/Remote.php | 18 +++++++++++++--- test/Mock/FakeRemote.php | 4 +--- test/Processor/RemoteTest.php | 40 +++++++++++++++++++++++++++++------ 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/composer.json b/composer.json index 6f1747e..8bc22d7 100644 --- a/composer.json +++ b/composer.json @@ -15,12 +15,20 @@ ], "autoload": { "psr-4": { - "FileFetcher\\": "src/", + "FileFetcher\\": "src/" + } + }, + "autoload-dev": { + "psr-4": { "FileFetcherTests\\": "test/" } }, "require": { + "ext-curl": "*", "getdkan/procrastinator": "^4.0.0", - "ext-curl": "*" + "php": ">=7.4, <8.1" + }, + "scripts": { + "phpunit": "./vendor/bin/phpunit" } } diff --git a/src/Processor/Remote.php b/src/Processor/Remote.php index 27795ab..f3bee40 100644 --- a/src/Processor/Remote.php +++ b/src/Processor/Remote.php @@ -9,7 +9,9 @@ class Remote extends AbstractChunkedProcessor protected function getFileSize(string $filePath): int { $headers = $this->getHeaders($filePath); - return $headers['content-length']; + // @todo Handle situation where the file size cannot be adequately + // determined. + return $headers['content-length'] ?? 0; } public function isServerCompatible(array $state): bool @@ -37,11 +39,21 @@ protected function getHeaders($url) curl_setopt($ch, CURLOPT_HEADER, true); curl_setopt($ch, CURLOPT_NOBODY, true); - $headers = $this->parseHeaders($this->php->curl_exec($ch)); + $headers = static::parseHeaders($this->php->curl_exec($ch)); curl_close($ch); return $headers; } + /** + * Parse headers as array from header string. + * + * @param $string + * The headers. + * + * @return array + * The headers as an associative array. Header names are converted to + * lower case. + */ public static function parseHeaders($string) { $headers = []; @@ -49,7 +61,7 @@ public static function parseHeaders($string) foreach ($lines as $line) { $line = trim($line); $keyvalue = self::getKeyValueFromLine($line); - $headers[$keyvalue['key']] = $keyvalue['value']; + $headers[strtolower($keyvalue['key'])] = $keyvalue['value']; } return $headers; } diff --git a/test/Mock/FakeRemote.php b/test/Mock/FakeRemote.php index 4f35100..39ca03a 100644 --- a/test/Mock/FakeRemote.php +++ b/test/Mock/FakeRemote.php @@ -16,9 +16,7 @@ protected function getChunk(string $filePath, int $start, int $end) { $data = ""; $numberOfBytes = $end - $start; - for ($i = 1; $i < $numberOfBytes; $i++) { - $data .= "A"; - } + $data = str_pad($data, $numberOfBytes, 'A'); return !empty(trim($data)) ? $data . PHP_EOL : false; } } diff --git a/test/Processor/RemoteTest.php b/test/Processor/RemoteTest.php index 68cfd6b..1be1f0f 100644 --- a/test/Processor/RemoteTest.php +++ b/test/Processor/RemoteTest.php @@ -6,6 +6,7 @@ use FileFetcher\FileFetcher; use FileFetcher\PhpFunctionsBridge; use FileFetcher\Processor\Remote; +use FileFetcherTests\Mock\FakeRemote; use MockChain\Chain; use MockChain\Options; use PHPUnit\Framework\TestCase; @@ -17,14 +18,14 @@ class RemoteTest extends TestCase public function testCopyAFileWithRemoteProcessor() { - $fetcher = FileFetcher::get( + $this->assertNotSame(FALSE, $fetcher = FileFetcher::get( "1", new Memory(), [ "filePath" => 'http://notreal.blah/notacsv.csv', - "processors" => [\FileFetcherTests\Mock\FakeRemote::class] + "processors" => [FakeRemote::class] ] - ); + )); $fetcher->setTimeLimit(1); @@ -36,9 +37,8 @@ public function testCopyAFileWithRemoteProcessor() $state = $fetcher->getState(); - $this->assertTrue(true); - $this->assertEquals($state['processor'], 'FileFetcherTests\Mock\FakeRemote'); - $this->assertEquals($state['destination'], 'http://notreal.blah/notacsv.csv'); + $this->assertEquals(FakeRemote::class, $state['processor']); + $this->assertEquals('/tmp/notreal_blah_notacsv.csv', $state['destination']); } public function testCurlCopy() @@ -80,4 +80,32 @@ public function testIsServerCompatible() $this->assertFalse($processor->isServerCompatible(['source' => 'https://invalid'])); $this->assertFalse($processor->isServerCompatible(['source' => 'ftp://example.org'])); } + + public function provideFileSizeHeaders() { + return [ + [1, ['content-length' => 1]], + [0, ['content-length' => 0]], + 'wrong_type' => [23, ['content-length' => '23']], + 'wrong_type_null' => [0, ['content-length' => NULL]], + 'no_header' => [0, []], + 'wrong_case' => [1, ['Content-Length' => 1]], + ]; + } + + /** + * @covers \FileFetcher\Processor\Remote::getFileSize() + * @dataProvider provideFileSizeHeaders + */ + public function testGetFileSize($expected, $headers) { + $remote = $this->getMockBuilder(Remote::class) + ->onlyMethods(['getFileSize', 'getHeaders']) + ->getMock(); + $remote->method('getHeaders') + ->willReturn($headers); + $ref_getFileSize = (new \ReflectionClass(Remote::class)) + ->getMethod('getFileSize'); + $ref_getFileSize->setAccessible(TRUE); + + $this->assertSame($expected, $ref_getFileSize->invokeArgs($remote, ['filepath'])); + } } From 77a0f68f864479eed90be33b9e99434143002bd3 Mon Sep 17 00:00:00 2001 From: Paul Mitchum Date: Thu, 19 Jan 2023 16:37:57 -0800 Subject: [PATCH 2/5] unneeded test case --- test/Processor/RemoteTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Processor/RemoteTest.php b/test/Processor/RemoteTest.php index 1be1f0f..49e9a8f 100644 --- a/test/Processor/RemoteTest.php +++ b/test/Processor/RemoteTest.php @@ -88,7 +88,6 @@ public function provideFileSizeHeaders() { 'wrong_type' => [23, ['content-length' => '23']], 'wrong_type_null' => [0, ['content-length' => NULL]], 'no_header' => [0, []], - 'wrong_case' => [1, ['Content-Length' => 1]], ]; } From 07a7c0f1c7424d7891670f0c0dfb90baa834b756 Mon Sep 17 00:00:00 2001 From: Paul Mitchum Date: Fri, 20 Jan 2023 10:52:51 -0800 Subject: [PATCH 3/5] try to fix circleci --- .circleci/config.yml | 22 +++++++++++++--------- phpunit.xml | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f13e205..9e9c107 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -9,19 +9,23 @@ jobs: name: composer install command: composer install - run: - name: Setup Code Climate test-reporter + name: Set up Code Climate test-reporter command: | + sudo -E install-php-extensions xdebug curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter chmod +x ./cc-test-reporter - ./cc-test-reporter before-build - - run: - name: PHPUnit Tests - environment: - XDEBUG_MODE: coverage - command: phpdbg -qrr vendor/bin/phpunit --coverage-clover clover.xml - run: - name: Report test coverage - command: ./cc-test-reporter after-build --coverage-input-type clover --exit-code $? + name: Run tests + command: | + composer update --prefer-lowest + ./vendor/bin/phpunit --testsuite all + composer update + ./cc-test-reporter before-build + sudo docker-php-ext-enable xdebug + ./vendor/bin/phpunit --testsuite all --coverage-clover clover.xml + EXIT_CODE=$? + sed -i 's+/var/www/html/+/home/circleci/repo/+g' clover.xml + ./cc-test-reporter after-build --coverage-input-type clover --exit-code $EXIT_CODE workflows: test-workflow: diff --git a/phpunit.xml b/phpunit.xml index 444f5b1..143f167 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,7 +7,7 @@ - test + test From 07f0d70e5be48dad033d0a4666ffe991bf417cb6 Mon Sep 17 00:00:00 2001 From: Paul Mitchum Date: Fri, 20 Jan 2023 11:03:33 -0800 Subject: [PATCH 4/5] xdebug_mode --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9e9c107..9f3293b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -22,7 +22,7 @@ jobs: composer update ./cc-test-reporter before-build sudo docker-php-ext-enable xdebug - ./vendor/bin/phpunit --testsuite all --coverage-clover clover.xml + XDEBUG_MODE=coverage ./vendor/bin/phpunit --testsuite all --coverage-clover clover.xml EXIT_CODE=$? sed -i 's+/var/www/html/+/home/circleci/repo/+g' clover.xml ./cc-test-reporter after-build --coverage-input-type clover --exit-code $EXIT_CODE From ef08c534664df8783664f89cc536756611b50fbd Mon Sep 17 00:00:00 2001 From: Paul Mitchum Date: Fri, 20 Jan 2023 11:22:43 -0800 Subject: [PATCH 5/5] CS improvements --- .codeclimate.yml | 3 +-- composer.json | 7 +++++-- phpcs.xml | 14 ++++++++++++++ src/Processor/Remote.php | 2 ++ test/Processor/RemoteTest.php | 34 ++++++++++++++++++---------------- 5 files changed, 40 insertions(+), 20 deletions(-) create mode 100644 phpcs.xml diff --git a/.codeclimate.yml b/.codeclimate.yml index d4bd732..017aca6 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,4 +1,4 @@ -version: 2 +version: "2" plugins: phpcodesniffer: enabled: true @@ -14,4 +14,3 @@ exclude_patterns: - ".circleci/" - ".gitignore" - "docs/" - diff --git a/composer.json b/composer.json index 8bc22d7..12e7a30 100644 --- a/composer.json +++ b/composer.json @@ -5,7 +5,8 @@ "minimum-stability": "dev", "require-dev": { "phpunit/phpunit": "^9.0", - "getdkan/mock-chain": "dev-master" + "getdkan/mock-chain": "dev-master", + "squizlabs/php_codesniffer": "~3@stable" }, "authors": [ { @@ -29,6 +30,8 @@ "php": ">=7.4, <8.1" }, "scripts": { - "phpunit": "./vendor/bin/phpunit" + "phpunit": "./vendor/bin/phpunit", + "phpcs": "./vendor/bin/phpcs -ps", + "phpcbf": "./vendor/bin/phpcbf" } } diff --git a/phpcs.xml b/phpcs.xml new file mode 100644 index 0000000..d9f7afc --- /dev/null +++ b/phpcs.xml @@ -0,0 +1,14 @@ + + + + PHP CodeSniffer configuration for File Fetcher + + .circleci/ + .ddev/ + vendor/ + + . + + + + diff --git a/src/Processor/Remote.php b/src/Processor/Remote.php index f3bee40..27915f8 100644 --- a/src/Processor/Remote.php +++ b/src/Processor/Remote.php @@ -4,7 +4,9 @@ class Remote extends AbstractChunkedProcessor { + // phpcs:disable protected const HTTP_URL_REGEX = '%^(?:https?://)?(?:\S+(?::\S*)?@|\d{1,3}(?:\.\d{1,3}){3}|(?:(?:[a-z\d\x{00a1}-\x{ffff}]+-?)*[a-z\d\x{00a1}-\x{ffff}]+)(?:\.(?:[a-z\d\x{00a1}-\x{ffff}]+-?)*[a-z\d\x{00a1}-\x{ffff}]+)*(?:\.[a-z\x{00a1}-\x{ffff}]{2,6}))(?::\d+)?(?:[^\s]*)?$%iu'; + // phpcs:enable protected function getFileSize(string $filePath): int { diff --git a/test/Processor/RemoteTest.php b/test/Processor/RemoteTest.php index 49e9a8f..eb0dfc5 100644 --- a/test/Processor/RemoteTest.php +++ b/test/Processor/RemoteTest.php @@ -18,7 +18,7 @@ class RemoteTest extends TestCase public function testCopyAFileWithRemoteProcessor() { - $this->assertNotSame(FALSE, $fetcher = FileFetcher::get( + $this->assertNotSame(false, $fetcher = FileFetcher::get( "1", new Memory(), [ @@ -81,30 +81,32 @@ public function testIsServerCompatible() $this->assertFalse($processor->isServerCompatible(['source' => 'ftp://example.org'])); } - public function provideFileSizeHeaders() { - return [ + public function provideFileSizeHeaders() + { + return [ [1, ['content-length' => 1]], [0, ['content-length' => 0]], 'wrong_type' => [23, ['content-length' => '23']], - 'wrong_type_null' => [0, ['content-length' => NULL]], + 'wrong_type_null' => [0, ['content-length' => null]], 'no_header' => [0, []], - ]; + ]; } /** * @covers \FileFetcher\Processor\Remote::getFileSize() * @dataProvider provideFileSizeHeaders */ - public function testGetFileSize($expected, $headers) { - $remote = $this->getMockBuilder(Remote::class) - ->onlyMethods(['getFileSize', 'getHeaders']) - ->getMock(); - $remote->method('getHeaders') - ->willReturn($headers); - $ref_getFileSize = (new \ReflectionClass(Remote::class)) - ->getMethod('getFileSize'); - $ref_getFileSize->setAccessible(TRUE); - - $this->assertSame($expected, $ref_getFileSize->invokeArgs($remote, ['filepath'])); + public function testGetFileSize($expected, $headers) + { + $remote = $this->getMockBuilder(Remote::class) + ->onlyMethods(['getFileSize', 'getHeaders']) + ->getMock(); + $remote->method('getHeaders') + ->willReturn($headers); + $ref_getFileSize = (new \ReflectionClass(Remote::class)) + ->getMethod('getFileSize'); + $ref_getFileSize->setAccessible(true); + + $this->assertSame($expected, $ref_getFileSize->invokeArgs($remote, ['filepath'])); } }