From 4acebcec7e16ce37e847e4273d40bbee924d8a1e Mon Sep 17 00:00:00 2001 From: shibafu Date: Mon, 10 Aug 2020 12:25:00 +0900 Subject: [PATCH 1/5] Add metadata error attrs migration --- app/Metadata.php | 2 +- ...8_10_114944_add_error_data_to_metadata.php | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 database/migrations/2020_08_10_114944_add_error_data_to_metadata.php diff --git a/app/Metadata.php b/app/Metadata.php index 2abd321..546f155 100644 --- a/app/Metadata.php +++ b/app/Metadata.php @@ -13,7 +13,7 @@ class Metadata extends Model protected $fillable = ['url', 'title', 'description', 'image', 'expires_at']; protected $visible = ['url', 'title', 'description', 'image', 'expires_at', 'tags']; - protected $dates = ['created_at', 'updated_at', 'expires_at']; + protected $dates = ['created_at', 'updated_at', 'expires_at', 'error_at']; public function tags() { diff --git a/database/migrations/2020_08_10_114944_add_error_data_to_metadata.php b/database/migrations/2020_08_10_114944_add_error_data_to_metadata.php new file mode 100644 index 0000000..2938ccd --- /dev/null +++ b/database/migrations/2020_08_10_114944_add_error_data_to_metadata.php @@ -0,0 +1,36 @@ +timestamp('error_at')->nullable(); + $table->string('error_exception_class')->nullable(); + $table->integer('error_http_code')->nullable(); + $table->text('error_body')->nullable(); + $table->integer('error_count')->default(0); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('metadata', function (Blueprint $table) { + $table->dropColumn(['error_at', 'error_exception_class', 'error_http_code', 'error_body', 'error_count']); + }); + } +} From 578b9934f5ec67de8d3020d63b2ad27af32ef6b9 Mon Sep 17 00:00:00 2001 From: shibafu Date: Mon, 10 Aug 2020 13:32:47 +0900 Subject: [PATCH 2/5] =?UTF-8?q?=E3=83=A1=E3=82=BF=E3=83=87=E3=83=BC?= =?UTF-8?q?=E3=82=BF=E5=8F=96=E5=BE=97=E3=82=A8=E3=83=A9=E3=83=BC=E3=81=AE?= =?UTF-8?q?=E8=A8=98=E9=8C=B2=E3=81=A8=E3=83=AA=E3=83=88=E3=83=A9=E3=82=A4?= =?UTF-8?q?=E5=88=B6=E9=99=90=E3=81=AE=E9=81=A9=E7=94=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/Metadata.php | 58 ++++++++++++++ .../ResolverCircuitBreakException.php | 16 ++++ .../UncaughtResolverException.php | 10 +++ app/Services/MetadataResolveService.php | 80 ++++++++++++++----- 4 files changed, 143 insertions(+), 21 deletions(-) create mode 100644 app/MetadataResolver/ResolverCircuitBreakException.php create mode 100644 app/MetadataResolver/UncaughtResolverException.php diff --git a/app/Metadata.php b/app/Metadata.php index 546f155..61ee308 100644 --- a/app/Metadata.php +++ b/app/Metadata.php @@ -2,6 +2,8 @@ namespace App; +use Carbon\CarbonInterface; +use GuzzleHttp\Exception\RequestException; use Illuminate\Database\Eloquent\Model; class Metadata extends Model @@ -19,4 +21,60 @@ class Metadata extends Model { return $this->belongsToMany(Tag::class)->withTimestamps(); } + + public function needRefresh(): bool + { + return $this->isExpired() || $this->error_at !== null; + } + + public function isExpired(): bool + { + return $this->expires_at !== null && $this->expires_at < now(); + } + + public function storeException(CarbonInterface $error_at, \Exception $exception): self + { + $this->prepareFieldsOnError(); + $this->error_at = $error_at; + $this->error_exception_class = get_class($exception); + $this->error_body = $exception->getMessage(); + if ($exception instanceof RequestException) { + $this->error_http_code = $exception->getCode(); + } else { + $this->error_http_code = null; + } + $this->error_count++; + + return $this; + } + + public function storeError(CarbonInterface $error_at, string $body, ?int $httpCode = null): self + { + $this->prepareFieldsOnError(); + $this->error_at = $error_at; + $this->error_exception_class = null; + $this->error_body = $body; + $this->error_http_code = $httpCode; + $this->error_count++; + + return $this; + } + + public function clearError(): self + { + $this->error_at = null; + $this->error_exception_class = null; + $this->error_body = null; + $this->error_http_code = null; + $this->error_count = 0; + + return $this; + } + + private function prepareFieldsOnError() + { + $this->title = $this->title ?? ''; + $this->description = $this->description ?? ''; + $this->image = $this->image ?? ''; + } } diff --git a/app/MetadataResolver/ResolverCircuitBreakException.php b/app/MetadataResolver/ResolverCircuitBreakException.php new file mode 100644 index 0000000..748c545 --- /dev/null +++ b/app/MetadataResolver/ResolverCircuitBreakException.php @@ -0,0 +1,16 @@ +formatter = $formatter; } + /** + * メタデータをキャッシュまたはリモートに問い合わせて取得します。 + * @param string $url メタデータを取得したいURL + * @return Metadata 取得できたメタデータ + * @throws DeniedHostException アクセス先がブラックリスト入りしているため取得できなかった場合にスロー + * @throws UncaughtResolverException Resolver内で例外が発生して取得できなかった場合にスロー + */ public function execute(string $url): Metadata { // URLの正規化 @@ -34,34 +45,61 @@ class MetadataResolveService throw new DeniedHostException($url); } - return DB::transaction(function () use ($url) { + DB::beginTransaction(); + try { + $metadata = Metadata::find($url); + // 無かったら取得 // TODO: ある程度古かったら再取得とかありだと思う - $metadata = Metadata::find($url); - if ($metadata == null || ($metadata->expires_at !== null && $metadata->expires_at < now())) { + if ($metadata == null || $metadata->needRefresh()) { + if ($metadata === null) { + $metadata = new Metadata(['url' => $url]); + } + + if ($metadata->error_count >= self::CIRCUIT_BREAK_COUNT) { + throw new ResolverCircuitBreakException($metadata->error_count, $url); + } + try { $resolved = $this->resolver->resolve($url); - $metadata = Metadata::updateOrCreate(['url' => $url], [ - 'title' => $resolved->title, - 'description' => $resolved->description, - 'image' => $resolved->image, - 'expires_at' => $resolved->expires_at - ]); - - $tagIds = []; - foreach ($resolved->normalizedTags() as $tagName) { - $tag = Tag::firstOrCreate(['name' => $tagName]); - $tagIds[] = $tag->id; - } - $metadata->tags()->sync($tagIds); - } catch (TransferException $e) { - // 何らかの通信エラーによってメタデータの取得に失敗した時、とりあえずエラーログにURLを残す + } catch (\Exception $e) { Log::error(self::class . ': メタデータの取得に失敗 URL=' . $url); - throw $e; + + // TODO: 何か制御用の例外を下位で使ってないか確認したほうが良い？その場合、雑catchできない + $metadata->storeException(now(), $e); + $metadata->save(); + throw new UncaughtResolverException(implode(': ', [ + $metadata->error_count . '回目のメタデータ取得失敗', get_class($e), $e->getMessage() + ]), 0, $e); } + + $metadata->fill([ + 'title' => $resolved->title, + 'description' => $resolved->description, + 'image' => $resolved->image, + 'expires_at' => $resolved->expires_at + ]); + $metadata->clearError(); + $metadata->save(); + + $tagIds = []; + foreach ($resolved->normalizedTags() as $tagName) { + $tag = Tag::firstOrCreate(['name' => $tagName]); + $tagIds[] = $tag->id; + } + $metadata->tags()->sync($tagIds); } + DB::commit(); + return $metadata; - }); + } catch (UncaughtResolverException $e) { + // Metadataにエラー情報を記録するため + DB::commit(); + throw $e; + } catch (\Exception $e) { + DB::rollBack(); + throw $e; + } } } From c372c118672dd702ebdb8f299dcb5895c7dd0464 Mon Sep 17 00:00:00 2001 From: shibafu Date: Mon, 10 Aug 2020 13:39:19 +0900 Subject: [PATCH 3/5] rm Log --- app/Services/MetadataResolveService.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/Services/MetadataResolveService.php b/app/Services/MetadataResolveService.php index 101c9d5..fec23d6 100644 --- a/app/Services/MetadataResolveService.php +++ b/app/Services/MetadataResolveService.php @@ -10,7 +10,6 @@ use App\MetadataResolver\UncaughtResolverException; use App\Tag; use App\Utilities\Formatter; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Log; class MetadataResolveService { @@ -63,9 +62,6 @@ class MetadataResolveService try { $resolved = $this->resolver->resolve($url); } catch (\Exception $e) { - Log::error(self::class . ': メタデータの取得に失敗 URL=' . $url); - - // TODO: 何か制御用の例外を下位で使ってないか確認したほうが良い？その場合、雑catchできない $metadata->storeException(now(), $e); $metadata->save(); throw new UncaughtResolverException(implode(': ', [ From da7be61698c7c9f82e06a86c816f84b3c4668293 Mon Sep 17 00:00:00 2001 From: shibafu Date: Mon, 10 Aug 2020 13:40:40 +0900 Subject: [PATCH 4/5] Don't report circuit break exception --- app/Exceptions/Handler.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index dcb27e0..1911732 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -20,6 +20,7 @@ class Handler extends ExceptionHandler \Illuminate\Database\Eloquent\ModelNotFoundException::class, \Illuminate\Session\TokenMismatchException::class, \Illuminate\Validation\ValidationException::class, + \App\MetadataResolver\ResolverCircuitBreakException::class, ]; /** From ca070e773ac09d7d8fb3f9b2b30fe74fb6006643 Mon Sep 17 00:00:00 2001 From: shibafu Date: Mon, 10 Aug 2020 15:57:02 +0900 Subject: [PATCH 5/5] add test --- .../Services/MetadataResolverServiceTest.php | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 tests/Unit/Services/MetadataResolverServiceTest.php diff --git a/tests/Unit/Services/MetadataResolverServiceTest.php b/tests/Unit/Services/MetadataResolverServiceTest.php new file mode 100644 index 0000000..d8c7528 --- /dev/null +++ b/tests/Unit/Services/MetadataResolverServiceTest.php @@ -0,0 +1,182 @@ +seed(); + Carbon::setTestNow('2020-07-21 19:19:19'); + } + + protected function tearDown(): void + { + parent::tearDown(); + Carbon::setTestNow(); + } + + public function testOnRuntimeException() + { + $this->mock(MetadataResolver::class, function (MockInterface $mock) { + $mock->shouldReceive('resolve')->andReturnUsing(function ($url) { + throw new \RuntimeException('Something happened!'); + }); + }); + + try { + $service = app()->make(MetadataResolveService::class); + $service->execute('http://example.com'); + } catch (UncaughtResolverException $e) { + $this->assertDatabaseHas('metadata', [ + 'url' => 'http://example.com', + 'error_at' => new Carbon('2020-07-21 19:19:19'), + 'error_count' => 1, + 'error_exception_class' => \RuntimeException::class, + 'error_http_code' => null, + 'error_body' => 'Something happened!', + ]); + + return; + } + $this->fail(); + } + + public function testOnHttpClientError() + { + $handler = HandlerStack::create(new MockHandler([new Response(404)])); + $client = new Client(['handler' => $handler]); + $this->instance(Client::class, $client); + + try { + $service = app()->make(MetadataResolveService::class); + $service->execute('http://example.com'); + } catch (UncaughtResolverException $e) { + $this->assertDatabaseHas('metadata', [ + 'url' => 'http://example.com', + 'error_at' => new Carbon('2020-07-21 19:19:19'), + 'error_count' => 1, + 'error_exception_class' => ClientException::class, + 'error_http_code' => 404, + ]); + + return; + } + $this->fail(); + } + + public function testOnHttpServerError() + { + $handler = HandlerStack::create(new MockHandler([new Response(503), new Response(503)])); + $client = new Client(['handler' => $handler]); + $this->instance(Client::class, $client); + + try { + $service = app()->make(MetadataResolveService::class); + $service->execute('http://example.com'); + } catch (UncaughtResolverException $e) { + $this->assertDatabaseHas('metadata', [ + 'url' => 'http://example.com', + 'error_at' => new Carbon('2020-07-21 19:19:19'), + 'error_count' => 1, + 'error_exception_class' => ServerException::class, + 'error_http_code' => 503, + ]); + + return; + } + $this->fail(); + } + + public function testCircuitBreak() + { + $this->mock(MetadataResolver::class, function (MockInterface $mock) { + $mock->shouldReceive('resolve')->andReturnUsing(function ($url) { + throw new \RuntimeException('Something happened!'); + }); + }); + + try { + for ($i = 0; $i < 6; $i++) { + try { + $service = app()->make(MetadataResolveService::class); + $service->execute('http://example.com'); + } catch (UncaughtResolverException $e) { + } + } + } catch (ResolverCircuitBreakException $e) { + $this->assertDatabaseHas('metadata', [ + 'url' => 'http://example.com', + 'error_at' => new Carbon('2020-07-21 19:19:19'), + 'error_count' => 5, + 'error_exception_class' => \RuntimeException::class, + 'error_http_code' => null, + 'error_body' => 'Something happened!', + ]); + + return; + } + $this->fail(); + } + + public function testOnResurrect() + { + $successBody = << + + + + + + Test Document + + + + +HTML; + $handler = HandlerStack::create(new MockHandler([ + new Response(404), + new Response(200, ['Content-Type' => 'text/html'], $successBody), + ])); + $client = new Client(['handler' => $handler]); + $this->instance(Client::class, $client); + + for ($i = 0; $i < 2; $i++) { + try { + $service = app()->make(MetadataResolveService::class); + $service->execute('http://example.com'); + } catch (UncaughtResolverException $e) { + } + } + + $this->assertDatabaseHas('metadata', [ + 'url' => 'http://example.com', + 'title' => 'OGP Title', + 'description' => 'OGP Description', + 'image' => '', + 'error_at' => null, + 'error_count' => 0, + 'error_exception_class' => null, + 'error_http_code' => null, + 'error_body' => null, + ]); + } +}