From 88244d10921f6252fe20710e003b07af2f31c636 Mon Sep 17 00:00:00 2001 From: Bilge Date: Wed, 24 Feb 2021 00:54:44 +0000 Subject: [PATCH] Fixed ScrapeAppDetails not retrying on invalid markup during parsing. --- src/Resource/ScrapeAppDetails.php | 53 ++++++++++++++++++------ src/Scrape/InvalidMarkupException.php | 4 +- test/Fixture/ScrapeAppFixture.php | 7 +++- test/FixtureFactory.php | 35 ++++++++++------ test/Functional/ScrapeAppDetailsTest.php | 39 +++++++++++++++++ 5 files changed, 109 insertions(+), 29 deletions(-) diff --git a/src/Resource/ScrapeAppDetails.php b/src/Resource/ScrapeAppDetails.php index 9fe800c..24295d0 100644 --- a/src/Resource/ScrapeAppDetails.php +++ b/src/Resource/ScrapeAppDetails.php @@ -17,13 +17,19 @@ use ScriptFUSION\Porter\Provider\Resource\ProviderResource; use ScriptFUSION\Porter\Provider\Resource\SingleRecordResource; use ScriptFUSION\Porter\Provider\Steam\Scrape\AppDetailsParser; +use ScriptFUSION\Porter\Provider\Steam\Scrape\InvalidMarkupException; use ScriptFUSION\Porter\Provider\Steam\SteamProvider; +use function Amp\call; +use function ScriptFUSION\Retry\retry; +use function ScriptFUSION\Retry\retryAsync; /** * Scrapes the Steam store page for App details. */ final class ScrapeAppDetails implements ProviderResource, SingleRecordResource, AsyncResource, Url { + public const RETRIES = 5; + private $appId; public function __construct(int $appId) @@ -40,13 +46,19 @@ public function fetch(ImportConnector $connector): \Iterator { $this->configureOptions($connector->findBaseConnector()); - $this->validateResponse($response = $connector->fetch( - (new HttpDataSource($this->getUrl())) - // Enable age-restricted and mature content. - ->addHeader('Cookie: birthtime=0; mature_content=1') - )); - - yield AppDetailsParser::tryParseStorePage($response->getBody()); + yield retry( + self::RETRIES, + function () use ($connector) { + $this->validateResponse($response = $connector->fetch( + (new HttpDataSource($this->getUrl())) + // Enable age-restricted and mature content. + ->addHeader('Cookie: birthtime=0; mature_content=1') + )); + + return AppDetailsParser::tryParseStorePage($response->getBody()); + }, + self::createExceptionHandler() + ); } public function fetchAsync(ImportConnector $connector): Iterator @@ -54,12 +66,20 @@ public function fetchAsync(ImportConnector $connector): Iterator $this->configureAsyncOptions($connector->findBaseConnector()); return new Producer(function (\Closure $emit) use ($connector): \Generator { - /** @var HttpResponse $response */ - $this->validateResponse($response = yield $connector->fetchAsync( - (new AsyncHttpDataSource($this->getUrl())) + yield $emit(retryAsync( + self::RETRIES, + function () use ($connector) { + return call(function () use ($connector) { + /** @var HttpResponse $response */ + $this->validateResponse($response = yield $connector->fetchAsync( + (new AsyncHttpDataSource($this->getUrl())) + )); + + return AppDetailsParser::tryParseStorePage($response->getBody()); + }); + }, + self::createExceptionHandler() )); - - yield $emit(AppDetailsParser::tryParseStorePage($response->getBody())); }); } @@ -101,4 +121,13 @@ private function configureAsyncOptions(AsyncHttpConnector $connector): void // Enable mature content. $cookies->store(new ResponseCookie('mature_content', '1', $cookieAttributes)); } + + private static function createExceptionHandler(): \Closure + { + return static function (\Exception $exception): void { + if (!$exception instanceof InvalidMarkupException) { + throw $exception; + } + }; + } } diff --git a/src/Scrape/InvalidMarkupException.php b/src/Scrape/InvalidMarkupException.php index 30773fe..6977f8d 100644 --- a/src/Scrape/InvalidMarkupException.php +++ b/src/Scrape/InvalidMarkupException.php @@ -3,9 +3,7 @@ namespace ScriptFUSION\Porter\Provider\Steam\Scrape; -use ScriptFUSION\Porter\Connector\Recoverable\RecoverableException; - -class InvalidMarkupException extends ParserException implements RecoverableException +class InvalidMarkupException extends ParserException { // Intentionally empty. } diff --git a/test/Fixture/ScrapeAppFixture.php b/test/Fixture/ScrapeAppFixture.php index bd2ae10..721ba78 100644 --- a/test/Fixture/ScrapeAppFixture.php +++ b/test/Fixture/ScrapeAppFixture.php @@ -25,6 +25,11 @@ public function getProviderClassName(): string public function fetch(ImportConnector $connector): \Iterator { - yield AppDetailsParser::tryParseStorePage(file_get_contents(__DIR__ . "/$this->fixture")); + yield AppDetailsParser::tryParseStorePage(self::getFixture($this->fixture)); + } + + public static function getFixture(string $fixture): string + { + return file_get_contents(__DIR__ . "/$fixture"); } } diff --git a/test/FixtureFactory.php b/test/FixtureFactory.php index 3b031e9..7401431 100644 --- a/test/FixtureFactory.php +++ b/test/FixtureFactory.php @@ -3,6 +3,7 @@ namespace ScriptFUSIONTest\Porter\Provider\Steam; use Amp\Promise; +use Mockery\MockInterface; use Psr\Container\ContainerInterface; use ScriptFUSION\Porter\Porter; use ScriptFUSION\Porter\Provider\StaticDataProvider; @@ -20,19 +21,27 @@ final class FixtureFactory public static function createPorter(): Porter { - return new Porter( - \Mockery::mock(ContainerInterface::class) - ->shouldReceive('has') - ->with(SteamProvider::class) - ->andReturn(true) - ->shouldReceive('has') - ->with(StaticDataProvider::class) - ->andReturn(false) - ->shouldReceive('get') - ->with(SteamProvider::class) - ->andReturn(new SteamProvider) - ->getMock() - ); + return new Porter(self::mockPorterContainer()); + } + + /** + * @return ContainerInterface|MockInterface + */ + public static function mockPorterContainer(): ContainerInterface + { + return \Mockery::mock(ContainerInterface::class) + ->shouldReceive('has') + ->with(SteamProvider::class) + ->andReturn(true) + ->shouldReceive('has') + ->with(StaticDataProvider::class) + ->andReturn(false) + ->shouldReceive('get') + ->with(SteamProvider::class) + ->andReturn(new SteamProvider) + ->byDefault() + ->getMock() + ; } public static function createSession(Porter $porter): Promise diff --git a/test/Functional/ScrapeAppDetailsTest.php b/test/Functional/ScrapeAppDetailsTest.php index 8278004..529bc79 100644 --- a/test/Functional/ScrapeAppDetailsTest.php +++ b/test/Functional/ScrapeAppDetailsTest.php @@ -3,7 +3,15 @@ namespace ScriptFUSIONTest\Porter\Provider\Steam\Functional; +use Amp\Http\Client\Cookie\InMemoryCookieJar; +use Amp\Success; +use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration; +use Mockery\Generator\MockConfigurationBuilder; use PHPUnit\Framework\TestCase; +use ScriptFUSION\Porter\Connector\ImportConnectorFactory; +use ScriptFUSION\Porter\Net\Http\AsyncHttpConnector; +use ScriptFUSION\Porter\Net\Http\AsyncHttpOptions; +use ScriptFUSION\Porter\Net\Http\HttpResponse; use ScriptFUSION\Porter\Porter; use ScriptFUSION\Porter\Provider\Steam\Resource\InvalidAppIdException; use ScriptFUSION\Porter\Provider\Steam\Resource\ScrapeAppDetails; @@ -11,8 +19,10 @@ use ScriptFUSION\Porter\Provider\Steam\Scrape\SteamStoreException; use ScriptFUSION\Porter\Specification\AsyncImportSpecification; use ScriptFUSION\Porter\Specification\ImportSpecification; +use ScriptFUSION\Retry\FailingTooHardException; use ScriptFUSIONTest\Porter\Provider\Steam\Fixture\ScrapeAppFixture; use ScriptFUSIONTest\Porter\Provider\Steam\FixtureFactory; +use function Amp\Iterator\toArray; use function Amp\Promise\wait; /** @@ -20,6 +30,8 @@ */ final class ScrapeAppDetailsTest extends TestCase { + use MockeryPHPUnitIntegration; + /** * @var Porter */ @@ -743,4 +755,31 @@ public function testInvalidMarkup(): void $this->porter->importOne(new ImportSpecification(new ScrapeAppFixture('scuffed.xml'))); } + + /** + * Tests that when InvalidMarkupException is thrown, the request is retried. + */ + public function testImportInvalidMarkup(): void + { + $resource = new ScrapeAppDetails(0); + + // Mock connector to always return a scuffed response. + $builder = new MockConfigurationBuilder(); + $builder->setBlackListedMethods([]); + $builder->addTarget(AsyncHttpConnector::class); + $connector = \Mockery::spy($builder); + $connector->expects('fetchAsync')->andReturn( + new Success(HttpResponse::fromPhpWrapper( + ['HTTP/1.1 200 OK'], + ScrapeAppFixture::getFixture('scuffed.xml') + )) + )->times($resource::RETRIES); + $connector->expects('getOptions')->andReturn(new AsyncHttpOptions()); + $connector->expects('getCookieJar')->andReturn(new InMemoryCookieJar()); + + $importConnector = ImportConnectorFactory::create($connector, new AsyncImportSpecification($resource)); + + $this->expectException(FailingTooHardException::class); + wait(toArray($resource->fetchAsync($importConnector))); + } }