From f9daf7751aa5880881e6479c6ad4494f0526c067 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 21 Jan 2023 12:21:06 +0100 Subject: [PATCH] Catch and verify various errors --- app/Api/V1/Controllers/Controller.php | 12 ++++++------ app/Api/V1/Controllers/Data/DestroyController.php | 2 +- .../Models/TransactionCurrency/DestroyController.php | 3 +++ app/Api/V1/Controllers/Webhook/AttemptController.php | 6 +++--- app/Api/V1/Controllers/Webhook/DestroyController.php | 6 +++--- app/Api/V1/Controllers/Webhook/MessageController.php | 2 +- app/Api/V2/Controllers/Controller.php | 8 ++++---- .../Commands/Correction/FixTransactionTypes.php | 12 ++++++------ app/Console/Commands/Export/ExportData.php | 2 +- app/Console/Commands/VerifiesAccessToken.php | 2 +- app/Rules/IsValidAttachmentModel.php | 2 +- app/Validation/CurrencyValidation.php | 3 +++ 12 files changed, 33 insertions(+), 27 deletions(-) diff --git a/app/Api/V1/Controllers/Controller.php b/app/Api/V1/Controllers/Controller.php index d4a17f1157..c80ee5c2e0 100644 --- a/app/Api/V1/Controllers/Controller.php +++ b/app/Api/V1/Controllers/Controller.php @@ -99,18 +99,18 @@ abstract class Controller extends BaseController foreach ($dates as $field) { try { $date = request()->query->get($field); - } catch(BadRequestException $e) { + } catch (BadRequestException $e) { Log::error(sprintf('Request field "%s" contains a non-scalar value. Value set to NULL.', $field)); Log::error($e->getMessage()); $value = null; } - $obj = null; + $obj = null; if (null !== $date) { try { $obj = Carbon::parse($date); } catch (InvalidDateException|InvalidFormatException $e) { // don't care - app('log')->warning(sprintf('Ignored invalid date "%s" in API controller parameter check: %s', $date, $e->getMessage())); + app('log')->warning(sprintf('Ignored invalid date "%s" in API controller parameter check: %s', substr($date, 0, 20), $e->getMessage())); } } $bag->set($field, $obj); @@ -121,7 +121,7 @@ abstract class Controller extends BaseController foreach ($integers as $integer) { try { $value = request()->query->get($integer); - } catch(BadRequestException $e) { + } catch (BadRequestException $e) { Log::error(sprintf('Request field "%s" contains a non-scalar value. Value set to NULL.', $integer)); Log::error($e->getMessage()); $value = null; @@ -144,8 +144,8 @@ abstract class Controller extends BaseController { $sortParameters = []; try { - $param = (string)request()->query->get('sort'); - } catch(BadRequestException $e) { + $param = (string)request()->query->get('sort'); + } catch (BadRequestException $e) { Log::error('Request field "sort" contains a non-scalar value. Value set to NULL.'); Log::error($e->getMessage()); $param = ''; diff --git a/app/Api/V1/Controllers/Data/DestroyController.php b/app/Api/V1/Controllers/Data/DestroyController.php index 8154301663..6809353a61 100644 --- a/app/Api/V1/Controllers/Data/DestroyController.php +++ b/app/Api/V1/Controllers/Data/DestroyController.php @@ -69,7 +69,7 @@ class DestroyController extends Controller $this->unused = $request->boolean('unused', false); switch ($objects) { default: - throw new FireflyException(sprintf('This endpoint can\'t handle object "%s"', $objects)); + throw new FireflyException(sprintf('200033: This endpoint can\'t handle object "%s"', $objects)); case 'budgets': $this->destroyBudgets(); break; diff --git a/app/Api/V1/Controllers/Models/TransactionCurrency/DestroyController.php b/app/Api/V1/Controllers/Models/TransactionCurrency/DestroyController.php index 3b16508dd5..3effd85202 100644 --- a/app/Api/V1/Controllers/Models/TransactionCurrency/DestroyController.php +++ b/app/Api/V1/Controllers/Models/TransactionCurrency/DestroyController.php @@ -28,6 +28,7 @@ use FireflyIII\Api\V1\Controllers\Controller; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\TransactionCurrency; use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; +use FireflyIII\Repositories\User\UserRepositoryInterface; use FireflyIII\User; use Illuminate\Http\JsonResponse; @@ -37,6 +38,7 @@ use Illuminate\Http\JsonResponse; class DestroyController extends Controller { private CurrencyRepositoryInterface $repository; + private UserRepositoryInterface $userRepository; /** * CurrencyRepository constructor. @@ -49,6 +51,7 @@ class DestroyController extends Controller $this->middleware( function ($request, $next) { $this->repository = app(CurrencyRepositoryInterface::class); + $this->userRepository = app(UserRepositoryInterface::class); $this->repository->setUser(auth()->user()); return $next($request); diff --git a/app/Api/V1/Controllers/Webhook/AttemptController.php b/app/Api/V1/Controllers/Webhook/AttemptController.php index 7b705a0a6a..e8efa205a6 100644 --- a/app/Api/V1/Controllers/Webhook/AttemptController.php +++ b/app/Api/V1/Controllers/Webhook/AttemptController.php @@ -73,7 +73,7 @@ class AttemptController extends Controller public function index(Webhook $webhook, WebhookMessage $message): JsonResponse { if ($message->webhook_id !== $webhook->id) { - throw new FireflyException('Webhook and webhook message are no match'); + throw new FireflyException('200040: Webhook and webhook message are no match'); } $manager = $this->getManager(); @@ -112,10 +112,10 @@ class AttemptController extends Controller public function show(Webhook $webhook, WebhookMessage $message, WebhookAttempt $attempt): JsonResponse { if ($message->webhook_id !== $webhook->id) { - throw new FireflyException('Webhook and webhook message are no match'); + throw new FireflyException('200040: Webhook and webhook message are no match'); } if ($attempt->webhook_message_id !== $message->id) { - throw new FireflyException('Webhook message and webhook attempt are no match'); + throw new FireflyException('200041: Webhook message and webhook attempt are no match'); } $manager = $this->getManager(); diff --git a/app/Api/V1/Controllers/Webhook/DestroyController.php b/app/Api/V1/Controllers/Webhook/DestroyController.php index 329e9f5e56..c89d79e9ec 100644 --- a/app/Api/V1/Controllers/Webhook/DestroyController.php +++ b/app/Api/V1/Controllers/Webhook/DestroyController.php @@ -91,10 +91,10 @@ class DestroyController extends Controller public function destroyAttempt(Webhook $webhook, WebhookMessage $message, WebhookAttempt $attempt): JsonResponse { if ($message->webhook_id !== $webhook->id) { - throw new FireflyException('Webhook and webhook message are no match'); + throw new FireflyException('200040: Webhook and webhook message are no match'); } if ($attempt->webhook_message_id !== $message->id) { - throw new FireflyException('Webhook message and webhook attempt are no match'); + throw new FireflyException('200041: Webhook message and webhook attempt are no match'); } $this->repository->destroyAttempt($attempt); @@ -119,7 +119,7 @@ class DestroyController extends Controller public function destroyMessage(Webhook $webhook, WebhookMessage $message): JsonResponse { if ($message->webhook_id !== $webhook->id) { - throw new FireflyException('Webhook and webhook message are no match'); + throw new FireflyException('200040: Webhook and webhook message are no match'); } $this->repository->destroyMessage($message); app('preferences')->mark(); diff --git a/app/Api/V1/Controllers/Webhook/MessageController.php b/app/Api/V1/Controllers/Webhook/MessageController.php index 54a77fd2c5..6ec715ef5c 100644 --- a/app/Api/V1/Controllers/Webhook/MessageController.php +++ b/app/Api/V1/Controllers/Webhook/MessageController.php @@ -103,7 +103,7 @@ class MessageController extends Controller public function show(Webhook $webhook, WebhookMessage $message): JsonResponse { if ($message->webhook_id !== $webhook->id) { - throw new FireflyException('Webhook and webhook message are no match'); + throw new FireflyException('200040: Webhook and webhook message are no match'); } $manager = $this->getManager(); diff --git a/app/Api/V2/Controllers/Controller.php b/app/Api/V2/Controllers/Controller.php index 91a4865019..d7a57e4c26 100644 --- a/app/Api/V2/Controllers/Controller.php +++ b/app/Api/V2/Controllers/Controller.php @@ -94,18 +94,18 @@ class Controller extends BaseController foreach ($dates as $field) { try { $date = request()->query->get($field); - } catch(BadRequestException $e) { + } catch (BadRequestException $e) { Log::error(sprintf('Request field "%s" contains a non-scalar value. Value set to NULL.', $field)); Log::error($e->getMessage()); $value = null; } - $obj = null; + $obj = null; if (null !== $date) { try { $obj = Carbon::parse($date); } catch (InvalidDateException|InvalidFormatException $e) { // don't care - app('log')->warning(sprintf('Ignored invalid date "%s" in API v2 controller parameter check: %s', $date, $e->getMessage())); + app('log')->warning(sprintf('Ignored invalid date "%s" in API v2 controller parameter check: %s', substr($date, 0, 20), $e->getMessage())); } } $bag->set($field, $obj); @@ -115,7 +115,7 @@ class Controller extends BaseController foreach ($integers as $integer) { try { $value = request()->query->get($integer); - } catch(BadRequestException $e) { + } catch (BadRequestException $e) { Log::error(sprintf('Request field "%s" contains a non-scalar value. Value set to NULL.', $integer)); Log::error($e->getMessage()); $value = null; diff --git a/app/Console/Commands/Correction/FixTransactionTypes.php b/app/Console/Commands/Correction/FixTransactionTypes.php index 9f461813b1..5a9faf2faf 100644 --- a/app/Console/Commands/Correction/FixTransactionTypes.php +++ b/app/Console/Commands/Correction/FixTransactionTypes.php @@ -139,17 +139,17 @@ class FixTransactionTypes extends Command } ); if (0 === $collection->count()) { - throw new FireflyException(sprintf('Journal #%d has no source transaction.', $journal->id)); + throw new FireflyException(sprintf('300001: Journal #%d has no source transaction.', $journal->id)); } if (1 !== $collection->count()) { - throw new FireflyException(sprintf('Journal #%d has multiple source transactions.', $journal->id)); + throw new FireflyException(sprintf('300002: Journal #%d has multiple source transactions.', $journal->id)); } /** @var Transaction $transaction */ $transaction = $collection->first(); /** @var Account|null $account */ $account = $transaction->account; if (null === $account) { - throw new FireflyException(sprintf('Journal #%d, transaction #%d has no source account.', $journal->id, $transaction->id)); + throw new FireflyException(sprintf('300003: Journal #%d, transaction #%d has no source account.', $journal->id, $transaction->id)); } return $account; @@ -169,17 +169,17 @@ class FixTransactionTypes extends Command } ); if (0 === $collection->count()) { - throw new FireflyException(sprintf('Journal #%d has no destination transaction.', $journal->id)); + throw new FireflyException(sprintf('300004: Journal #%d has no destination transaction.', $journal->id)); } if (1 !== $collection->count()) { - throw new FireflyException(sprintf('Journal #%d has multiple destination transactions.', $journal->id)); + throw new FireflyException(sprintf('300005: Journal #%d has multiple destination transactions.', $journal->id)); } /** @var Transaction $transaction */ $transaction = $collection->first(); /** @var Account|null $account */ $account = $transaction->account; if (null === $account) { - throw new FireflyException(sprintf('Journal #%d, transaction #%d has no destination account.', $journal->id, $transaction->id)); + throw new FireflyException(sprintf('300006: Journal #%d, transaction #%d has no destination account.', $journal->id, $transaction->id)); } return $account; diff --git a/app/Console/Commands/Export/ExportData.php b/app/Console/Commands/Export/ExportData.php index 108e2d0b8e..1664b50287 100644 --- a/app/Console/Commands/Export/ExportData.php +++ b/app/Console/Commands/Export/ExportData.php @@ -249,7 +249,7 @@ class ExportData extends Command } } if (0 === $final->count()) { - throw new FireflyException('Ended up with zero valid accounts to export from.'); + throw new FireflyException('300007: Ended up with zero valid accounts to export from.'); } return $final; diff --git a/app/Console/Commands/VerifiesAccessToken.php b/app/Console/Commands/VerifiesAccessToken.php index d5b4783acc..1f209c4329 100644 --- a/app/Console/Commands/VerifiesAccessToken.php +++ b/app/Console/Commands/VerifiesAccessToken.php @@ -48,7 +48,7 @@ trait VerifiesAccessToken $repository = app(UserRepositoryInterface::class); $user = $repository->find($userId); if (null === $user) { - throw new FireflyException('User is unexpectedly NULL'); + throw new FireflyException('300000: User is unexpectedly NULL'); } return $user; diff --git a/app/Rules/IsValidAttachmentModel.php b/app/Rules/IsValidAttachmentModel.php index 8de8a7205f..455c7cb339 100644 --- a/app/Rules/IsValidAttachmentModel.php +++ b/app/Rules/IsValidAttachmentModel.php @@ -112,7 +112,7 @@ class IsValidAttachmentModel implements Rule TransactionJournal::class => 'validateJournal', ]; if (!array_key_exists($this->model, $methods)) { - Log::error(sprintf('Cannot validate model "%s" in %s.', $this->model, __METHOD__)); + Log::error(sprintf('Cannot validate model "%s" in %s.', substr($this->model, 0, 20), __METHOD__)); return false; } diff --git a/app/Validation/CurrencyValidation.php b/app/Validation/CurrencyValidation.php index 41ee577a8b..b5481ac986 100644 --- a/app/Validation/CurrencyValidation.php +++ b/app/Validation/CurrencyValidation.php @@ -46,6 +46,9 @@ trait CurrencyValidation $transactions = $this->getTransactionsArray($validator); foreach ($transactions as $index => $transaction) { + if(!is_array($transaction)) { + continue; + } // if foreign amount is present, then the currency must be as well. if (array_key_exists('foreign_amount', $transaction) && !(array_key_exists('foreign_currency_id', $transaction)