From 09e3a26076d8a5f8fb0d5a8bf15a58a92c9ad31d Mon Sep 17 00:00:00 2001 From: Catherine Doherty Date: Tue, 30 Oct 2018 11:57:43 +0000 Subject: [PATCH 1/3] fixing tests --- tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php | 2 +- tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php b/tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php index 26baa99..50319d4 100644 --- a/tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php +++ b/tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php @@ -43,7 +43,7 @@ public function testDeletedPushedToCyclops() $list->items = [$createEntity('test@test.com'), $createEntity('test@testtest.com')]; $useCase = new PushDeletedToCyclopsUseCase($service); - $useCase->execute($list); + $useCase->execute($list, function() {}); verify($deletedCount)->equals(2); } } diff --git a/tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php b/tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php index d8c2983..c064f28 100644 --- a/tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php +++ b/tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php @@ -30,7 +30,7 @@ public function testStalePushedToCyclops() verify($staleCount)->equals(0); - $createEntity = function($email) use ($service) { + $createEntity = function ($email) use ($service) { $id = new CyclopsIdentityEntity(); $id->email = $email; return $service->loadCustomer($id); @@ -40,7 +40,7 @@ public function testStalePushedToCyclops() $list->items = [$createEntity('test@test.com'), $createEntity('test@testtest.com')]; $useCase = new PushStaleToCyclopsUseCase($service); - $useCase->execute($list); + $useCase->execute($list, function () {}); verify($staleCount)->equals(2); } } From 7480643759b53770dcf27c65d85728e2d8b52d07 Mon Sep 17 00:00:00 2001 From: Catherine Doherty Date: Tue, 30 Oct 2018 11:57:56 +0000 Subject: [PATCH 2/3] handling errors in one place, and logging them!! --- src/Services/CyclopsService.php | 106 +++++++++++++------------------- 1 file changed, 42 insertions(+), 64 deletions(-) diff --git a/src/Services/CyclopsService.php b/src/Services/CyclopsService.php index 2dcee8f..c03ed49 100644 --- a/src/Services/CyclopsService.php +++ b/src/Services/CyclopsService.php @@ -10,6 +10,7 @@ use Gcd\Cyclops\Exceptions\UserForbiddenException; use Gcd\Cyclops\Http\CurlHttpClient; use Gcd\Cyclops\Http\HttpRequest; +use Gcd\Cyclops\Http\HttpResponse; use Gcd\Cyclops\Settings\CyclopsSettings; class CyclopsService @@ -31,6 +32,38 @@ public function __construct(int $brandId) $this->authorization = base64_encode($settings->authorizationUsername . ':' . $settings->authorizationPassword); } + private function logCyclopsErrors(HttpRequest $request, HttpResponse $response) + { + $loggableRequest = clone $request; + if (isset($loggableRequest->getHeaders()['Authorization'])) { + $loggableRequest->addHeader('Authorization', '[[REDACTED]]'); + } + error_log('Cyclops exception response: ' . $response->getResponseCode() . ' ' . $response->getResponseBody() . + "\n Request: " . var_export($loggableRequest, true)); + } + + private function handleResponseCodes(HttpResponse $response, HttpRequest $request) + { + switch ($response->getResponseCode()) { + case 200: + break; + case 403: + $this->logCyclopsErrors($request, $response); + throw new UserForbiddenException(); + break; + case 404: + $this->logCyclopsErrors($request, $response); + throw new CustomerNotFoundException(); + break; + case 409: + $this->logCyclopsErrors($request, $response); + throw new ConflictException(); + default: + $this->logCyclopsErrors($request, $response); + throw new CyclopsException(); + } + } + public function loadCustomer(CyclopsIdentityEntity $identityEntity): CustomerEntity { if ($identityEntity->id != '') { @@ -44,23 +77,13 @@ public function loadCustomer(CyclopsIdentityEntity $identityEntity): CustomerEnt $request->addHeader('Authorization', 'Basic ' . $this->authorization); $response = $this->doCyclopsRequest($request); - $responseBody = json_decode($response->getResponseBody()); - if ($responseBody) { - $identityEntity->id = $responseBody->data[0]->cyclopsId; - } } - switch ($response->getResponseCode()) { - case 200: - break; - case 403: - throw new UserForbiddenException(); - break; - case 404: - throw new CustomerNotFoundException(); - break; - default: - throw new CyclopsException(); + $this->handleResponseCodes($response, $request); + + $responseBody = json_decode($response->getResponseBody()); + if ($responseBody) { + $identityEntity->id = $responseBody->data[0]->cyclopsId; } $customer = new CustomerEntity(); @@ -80,19 +103,7 @@ public function deleteCustomer(CyclopsIdentityEntity $identityEntity) $request->addHeader('Authorization', 'Basic ' . $this->authorization); $response = $this->doCyclopsRequest($request); - switch ($response->getResponseCode()) { - case 200: - break; - case 403: - throw new UserForbiddenException(); - break; - case 404: - throw new CustomerNotFoundException(); - break; - default: - throw new CyclopsException(); - } - + $this->handleResponseCodes($response, $request); return $response; } @@ -104,18 +115,7 @@ public function getBrandOptInStatus(CustomerEntity $customerEntity): bool $request->addHeader('Authorization', 'Basic ' . $this->authorization); $response = $this->doCyclopsRequest($request); - switch ($response->getResponseCode()) { - case 200: - break; - case 403: - throw new UserForbiddenException(); - break; - case 404: - throw new CustomerNotFoundException(); - break; - default: - throw new CyclopsException(); - } + $this->handleResponseCodes($response, $request); if ($responseBody = json_decode($response->getResponseBody())) { foreach ($responseBody->data as $data) { @@ -138,21 +138,7 @@ public function setBrandOptInStatus(CustomerEntity $customerEntity) $request->addHeader('Content-Type', 'application/json'); $response = $this->doCyclopsRequest($request); - switch ($response->getResponseCode()) { - case 200: - break; - case 403: - throw new UserForbiddenException(); - break; - case 404: - throw new CustomerNotFoundException(); - break; - case 409: - throw new ConflictException(); - default: - throw new CyclopsException(); - } - + $this->handleResponseCodes($response, $request); return $response; } @@ -163,15 +149,7 @@ public function getBrandOptInStatusChanges(\DateTime $startingDate, int $page = $request->addHeader('Authorization', 'Basic ' . $this->authorization); $response = $this->doCyclopsRequest($request); - switch ($response->getResponseCode()) { - case 200: - break; - case 403: - throw new UserForbiddenException(); - break; - default: - throw new CyclopsException(); - } + $this->handleResponseCodes($response, $request); $changes = []; if ($responseBody = json_decode($response->getResponseBody())) { From 0bf3a4a6bcae45a78e419f2a4cd2c8ab8287af64 Mon Sep 17 00:00:00 2001 From: Catherine Doherty Date: Wed, 31 Oct 2018 15:53:38 +0000 Subject: [PATCH 3/3] making callable default to null --- src/UseCases/PushDeletedToCyclopsUseCase.php | 12 ++++++++---- src/UseCases/PushStaleToCyclopsUseCase.php | 10 +++++++--- .../UseCases/PushDeletedToCyclopsUseCaseTest.php | 2 +- .../unit/UseCases/PushStaleToCyclopsUseCaseTest.php | 2 +- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/UseCases/PushDeletedToCyclopsUseCase.php b/src/UseCases/PushDeletedToCyclopsUseCase.php index d5e6cf0..2c590cb 100644 --- a/src/UseCases/PushDeletedToCyclopsUseCase.php +++ b/src/UseCases/PushDeletedToCyclopsUseCase.php @@ -19,14 +19,18 @@ public function __construct(CyclopsService $cyclopsService) $this->cyclopsService = $cyclopsService; } - public function execute(CyclopsCustomerListEntity $list, callable $onCustomerDeleted) + public function execute(CyclopsCustomerListEntity $list, callable $onCustomerDeleted = null) { foreach ($list->items as $item) { try { $this->cyclopsService->deleteCustomer($item->identity); - $onCustomerDeleted($item); - } catch (CustomerNotFoundException $exception) { - $onCustomerDeleted($item); + if ($onCustomerDeleted !== null) { + $onCustomerDeleted($item); + } + } catch (CustomerNotFoundException $exception) { + if ($onCustomerDeleted !== null) { + $onCustomerDeleted($item); + } } catch (CyclopsException $exception) { } } diff --git a/src/UseCases/PushStaleToCyclopsUseCase.php b/src/UseCases/PushStaleToCyclopsUseCase.php index 9051130..dd88941 100644 --- a/src/UseCases/PushStaleToCyclopsUseCase.php +++ b/src/UseCases/PushStaleToCyclopsUseCase.php @@ -18,19 +18,23 @@ public function __construct(CyclopsService $cyclopsService) $this->cyclopsService = $cyclopsService; } - public function execute(CyclopsCustomerListEntity $list, callable $onCustomerCreated) + public function execute(CyclopsCustomerListEntity $list, callable $onCustomerCreated = null) { foreach ($list->items as $item) { try { $this->cyclopsService->setBrandOptInStatus($item); - $onCustomerCreated($item); + if ($onCustomerCreated !== null) { + $onCustomerCreated($item); + } } catch (CustomerNotFoundException $exception) { $customer = $this->cyclopsService->loadCustomer($item->identity); $customer->brandOptIn = $item->brandOptIn; $this->cyclopsService->setBrandOptInStatus($customer); - $onCustomerCreated($customer); + if ($onCustomerCreated !== null) { + $onCustomerCreated($customer); + } } } } diff --git a/tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php b/tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php index 50319d4..26baa99 100644 --- a/tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php +++ b/tests/unit/UseCases/PushDeletedToCyclopsUseCaseTest.php @@ -43,7 +43,7 @@ public function testDeletedPushedToCyclops() $list->items = [$createEntity('test@test.com'), $createEntity('test@testtest.com')]; $useCase = new PushDeletedToCyclopsUseCase($service); - $useCase->execute($list, function() {}); + $useCase->execute($list); verify($deletedCount)->equals(2); } } diff --git a/tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php b/tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php index c064f28..74dffd3 100644 --- a/tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php +++ b/tests/unit/UseCases/PushStaleToCyclopsUseCaseTest.php @@ -40,7 +40,7 @@ public function testStalePushedToCyclops() $list->items = [$createEntity('test@test.com'), $createEntity('test@testtest.com')]; $useCase = new PushStaleToCyclopsUseCase($service); - $useCase->execute($list, function () {}); + $useCase->execute($list); verify($staleCount)->equals(2); } }