Improve error handling in API.

This commit is contained in:
James Cole
2019-10-30 20:02:21 +01:00
parent bed182cf13
commit 9a028d5002
18 changed files with 131 additions and 103 deletions

View File

@@ -98,15 +98,15 @@ class AttachmentController extends Controller
public function download(Attachment $attachment): LaravelResponse public function download(Attachment $attachment): LaravelResponse
{ {
if (false === $attachment->uploaded) { if (false === $attachment->uploaded) {
throw new FireflyException(trans('api.error_no_upload')); throw new FireflyException('200000: File has not been uploaded (yet).');
} }
if (0 === $attachment->size) { if (0 === $attachment->size) {
throw new FireflyException(trans('api.error_no_upload')); throw new FireflyException('200000: File has not been uploaded (yet).');
} }
if ($this->repository->exists($attachment)) { if ($this->repository->exists($attachment)) {
$content = $this->repository->getContent($attachment); $content = $this->repository->getContent($attachment);
if ('' === $content) { if ('' === $content) {
throw new FireflyException(trans('api.error_no_upload')); throw new FireflyException('200002: File is empty (zero bytes).');
} }
$quoted = sprintf('"%s"', addcslashes(basename($attachment->filename), '"\\')); $quoted = sprintf('"%s"', addcslashes(basename($attachment->filename), '"\\'));
@@ -125,7 +125,7 @@ class AttachmentController extends Controller
return $response; return $response;
} }
throw new FireflyException(trans('api.error_file_lost')); throw new FireflyException('200003: File does not exist.');
} }
/** /**

View File

@@ -211,20 +211,16 @@ class BillController extends Controller
*/ */
public function store(BillRequest $request): JsonResponse public function store(BillRequest $request): JsonResponse
{ {
$bill = $this->repository->store($request->getAll()); $bill = $this->repository->store($request->getAll());
if (null !== $bill) { $manager = $this->getManager();
$manager = $this->getManager();
/** @var BillTransformer $transformer */ /** @var BillTransformer $transformer */
$transformer = app(BillTransformer::class); $transformer = app(BillTransformer::class);
$transformer->setParameters($this->parameters); $transformer->setParameters($this->parameters);
$resource = new Item($bill, $transformer, 'bills'); $resource = new Item($bill, $transformer, 'bills');
return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json');
}
throw new FireflyException(trans('api.error_store_bill')); // @codeCoverageIgnore
return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json');
} }
/** /**

View File

@@ -187,19 +187,16 @@ class BudgetController extends Controller
*/ */
public function store(BudgetRequest $request): JsonResponse public function store(BudgetRequest $request): JsonResponse
{ {
$budget = $this->repository->store($request->getAll()); $budget = $this->repository->store($request->getAll());
if (null !== $budget) { $manager = $this->getManager();
$manager = $this->getManager();
/** @var BudgetTransformer $transformer */ /** @var BudgetTransformer $transformer */
$transformer = app(BudgetTransformer::class); $transformer = app(BudgetTransformer::class);
$transformer->setParameters($this->parameters); $transformer->setParameters($this->parameters);
$resource = new Item($budget, $transformer, 'budgets'); $resource = new Item($budget, $transformer, 'budgets');
return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json');
}
throw new FireflyException(trans('api.error_store_budget')); // @codeCoverageIgnore
} }
/** /**

View File

@@ -165,7 +165,7 @@ class BudgetLimitController extends Controller
$data = $request->getAll(); $data = $request->getAll();
$budget = $this->repository->findNull($data['budget_id']); $budget = $this->repository->findNull($data['budget_id']);
if (null === $budget) { if (null === $budget) {
throw new FireflyException(trans('api.error_unknown_budget')); throw new FireflyException('200004: Budget does not exist.');
} }
$data['budget'] = $budget; $data['budget'] = $budget;
$budgetLimit = $this->blRepository->storeBudgetLimit($data); $budgetLimit = $this->blRepository->storeBudgetLimit($data);

View File

@@ -152,18 +152,15 @@ class CategoryController extends Controller
public function store(CategoryRequest $request): JsonResponse public function store(CategoryRequest $request): JsonResponse
{ {
$category = $this->repository->store($request->getAll()); $category = $this->repository->store($request->getAll());
if (null !== $category) { $manager = $this->getManager();
$manager = $this->getManager();
/** @var CategoryTransformer $transformer */ /** @var CategoryTransformer $transformer */
$transformer = app(CategoryTransformer::class); $transformer = app(CategoryTransformer::class);
$transformer->setParameters($this->parameters); $transformer->setParameters($this->parameters);
$resource = new Item($category, $transformer, 'categories'); $resource = new Item($category, $transformer, 'categories');
return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json');
}
throw new FireflyException(trans('api.error_store_new_category')); // @codeCoverageIgnore
} }
/** /**

View File

@@ -57,7 +57,7 @@ class ConfigurationController extends Controller
$admin = auth()->user(); $admin = auth()->user();
if (!$this->repository->hasRole($admin, 'owner')) { if (!$this->repository->hasRole($admin, 'owner')) {
throw new FireflyException(trans('api.error_no_access')); // @codeCoverageIgnore throw new FireflyException('200005: You need the "owner" role to do this.'); // @codeCoverageIgnore
} }
return $next($request); return $next($request);

View File

@@ -313,10 +313,10 @@ class CurrencyController extends Controller
if (!$this->userRepository->hasRole($admin, 'owner')) { if (!$this->userRepository->hasRole($admin, 'owner')) {
// access denied: // access denied:
throw new FireflyException(trans('api.error_no_access_ownership')); // @codeCoverageIgnore throw new FireflyException('200005: You need the "owner" role to do this.'); // @codeCoverageIgnore
} }
if ($this->repository->currencyInUse($currency)) { if ($this->repository->currencyInUse($currency)) {
throw new FireflyException(trans('api.error_no_access_currency_in_use')); // @codeCoverageIgnore throw new FireflyException('200006: Currency in use.'); // @codeCoverageIgnore
} }
$this->repository->destroy($currency); $this->repository->destroy($currency);
@@ -574,26 +574,21 @@ class CurrencyController extends Controller
public function store(CurrencyRequest $request): JsonResponse public function store(CurrencyRequest $request): JsonResponse
{ {
$currency = $this->repository->store($request->getAll()); $currency = $this->repository->store($request->getAll());
if (true === $request->boolean('default')) {
if (null !== $currency) { app('preferences')->set('currencyPreference', $currency->code);
if (true === $request->boolean('default')) { app('preferences')->mark();
app('preferences')->set('currencyPreference', $currency->code);
app('preferences')->mark();
}
$manager = $this->getManager();
$defaultCurrency = app('amount')->getDefaultCurrencyByUser(auth()->user());
$this->parameters->set('defaultCurrency', $defaultCurrency);
/** @var CurrencyTransformer $transformer */
$transformer = app(CurrencyTransformer::class);
$transformer->setParameters($this->parameters);
$resource = new Item($currency, $transformer, 'currencies');
return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json');
} }
throw new FireflyException(trans('api.error_store_new_currency')); // @codeCoverageIgnore $manager = $this->getManager();
$defaultCurrency = app('amount')->getDefaultCurrencyByUser(auth()->user());
$this->parameters->set('defaultCurrency', $defaultCurrency);
/** @var CurrencyTransformer $transformer */
$transformer = app(CurrencyTransformer::class);
$transformer->setParameters($this->parameters);
$resource = new Item($currency, $transformer, 'currencies');
return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json');
} }
/** /**

View File

@@ -24,10 +24,12 @@ declare(strict_types=1);
namespace FireflyIII\Factory; namespace FireflyIII\Factory;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Bill; use FireflyIII\Models\Bill;
use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionCurrency;
use FireflyIII\Services\Internal\Support\BillServiceTrait; use FireflyIII\Services\Internal\Support\BillServiceTrait;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Database\QueryException;
use Log; use Log;
/** /**
@@ -55,6 +57,7 @@ class BillFactory
* @param array $data * @param array $data
* *
* @return Bill|null * @return Bill|null
* @throws FireflyException
*/ */
public function create(array $data): ?Bill public function create(array $data): ?Bill
{ {
@@ -64,28 +67,31 @@ class BillFactory
$currency = $factory->find((int)($data['currency_id'] ?? null), (string)($data['currency_code'] ?? null)); $currency = $factory->find((int)($data['currency_id'] ?? null), (string)($data['currency_code'] ?? null));
if (null === $currency) { if (null === $currency) {
// use default currency:
$currency = app('amount')->getDefaultCurrencyByUser($this->user); $currency = app('amount')->getDefaultCurrencyByUser($this->user);
} }
try {
/** @var Bill $bill */
$bill = Bill::create(
[
'name' => $data['name'],
'match' => 'MIGRATED_TO_RULES',
'amount_min' => $data['amount_min'],
'user_id' => $this->user->id,
'transaction_currency_id' => $currency->id,
'amount_max' => $data['amount_max'],
'date' => $data['date'],
'repeat_freq' => $data['repeat_freq'],
'skip' => $data['skip'],
'automatch' => true,
'active' => $data['active'] ?? true,
]
);
} catch(QueryException $e) {
Log::error($e->getMessage());
Log::error($e->getTraceAsString());
throw new FireflyException('400000: Could not store bill.');
}
/** @var Bill $bill */
$bill = Bill::create(
[
'name' => $data['name'],
'match' => 'MIGRATED_TO_RULES',
'amount_min' => $data['amount_min'],
'user_id' => $this->user->id,
'transaction_currency_id' => $currency->id,
'amount_max' => $data['amount_max'],
'date' => $data['date'],
'repeat_freq' => $data['repeat_freq'],
'skip' => $data['skip'],
'automatch' => true,
'active' => $data['active'] ?? true,
]
);
// update note:
if (isset($data['notes'])) { if (isset($data['notes'])) {
$this->updateNote($bill, $data['notes']); $this->updateNote($bill, $data['notes']);
} }

View File

@@ -24,8 +24,10 @@ declare(strict_types=1);
namespace FireflyIII\Factory; namespace FireflyIII\Factory;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Category; use FireflyIII\Models\Category;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Database\QueryException;
use Log; use Log;
/** /**
@@ -62,7 +64,7 @@ class CategoryFactory
* @param null|string $categoryName * @param null|string $categoryName
* *
* @return Category|null * @return Category|null
* * @throws FireflyException
*/ */
public function findOrCreate(?int $categoryId, ?string $categoryName): ?Category public function findOrCreate(?int $categoryId, ?string $categoryName): ?Category
{ {
@@ -88,13 +90,17 @@ class CategoryFactory
if (null !== $category) { if (null !== $category) {
return $category; return $category;
} }
try {
return Category::create( return Category::create(
[ [
'user_id' => $this->user->id, 'user_id' => $this->user->id,
'name' => $categoryName, 'name' => $categoryName,
] ]
); );
} catch (QueryException $e) {
Log::error($e->getMessage());
throw new FireflyException('400003: Could not store new category.');
}
} }
return null; return null;

View File

@@ -27,6 +27,7 @@ declare(strict_types=1);
namespace FireflyIII\Factory; namespace FireflyIII\Factory;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionCurrency;
use Illuminate\Database\QueryException; use Illuminate\Database\QueryException;
use Log; use Log;
@@ -51,9 +52,10 @@ class TransactionCurrencyFactory
/** /**
* @param array $data * @param array $data
* *
* @return TransactionCurrency|null * @return TransactionCurrency
* @throws FireflyException
*/ */
public function create(array $data): ?TransactionCurrency public function create(array $data): TransactionCurrency
{ {
try { try {
/** @var TransactionCurrency $currency */ /** @var TransactionCurrency $currency */
@@ -69,6 +71,7 @@ class TransactionCurrencyFactory
} catch (QueryException $e) { } catch (QueryException $e) {
$result = null; $result = null;
Log::error(sprintf('Could not create new currency: %s', $e->getMessage())); Log::error(sprintf('Could not create new currency: %s', $e->getMessage()));
throw new FireflyException('400004: Could not store new currency.');
} }
return $result; return $result;

View File

@@ -24,6 +24,7 @@ namespace FireflyIII\Repositories\Bill;
use Carbon\Carbon; use Carbon\Carbon;
use DB; use DB;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Factory\BillFactory; use FireflyIII\Factory\BillFactory;
use FireflyIII\Models\Bill; use FireflyIII\Models\Bill;
use FireflyIII\Models\Note; use FireflyIII\Models\Note;
@@ -662,9 +663,10 @@ class BillRepository implements BillRepositoryInterface
/** /**
* @param array $data * @param array $data
* *
* @return Bill|null * @return Bill
* @throws FireflyException
*/ */
public function store(array $data): ?Bill public function store(array $data): Bill
{ {
/** @var BillFactory $factory */ /** @var BillFactory $factory */
$factory = app(BillFactory::class); $factory = app(BillFactory::class);

View File

@@ -23,6 +23,7 @@ declare(strict_types=1);
namespace FireflyIII\Repositories\Bill; namespace FireflyIII\Repositories\Bill;
use Carbon\Carbon; use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Bill; use FireflyIII\Models\Bill;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Pagination\LengthAwarePaginator;
@@ -266,9 +267,10 @@ interface BillRepositoryInterface
/** /**
* @param array $data * @param array $data
* *
* @return Bill|null * @return Bill
* @throws FireflyException
*/ */
public function store(array $data): ?Bill; public function store(array $data): Bill;
/** /**
* @param Bill $bill * @param Bill $bill

View File

@@ -25,6 +25,7 @@ namespace FireflyIII\Repositories\Budget;
use Carbon\Carbon; use Carbon\Carbon;
use DB; use DB;
use Exception; use Exception;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Budget; use FireflyIII\Models\Budget;
use FireflyIII\Models\BudgetLimit; use FireflyIII\Models\BudgetLimit;
use FireflyIII\Models\RecurrenceTransactionMeta; use FireflyIII\Models\RecurrenceTransactionMeta;
@@ -32,6 +33,7 @@ use FireflyIII\Models\RuleAction;
use FireflyIII\Models\RuleTrigger; use FireflyIII\Models\RuleTrigger;
use FireflyIII\Services\Internal\Destroy\BudgetDestroyService; use FireflyIII\Services\Internal\Destroy\BudgetDestroyService;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Database\QueryException;
use Illuminate\Support\Collection; use Illuminate\Support\Collection;
use Log; use Log;
@@ -269,16 +271,20 @@ class BudgetRepository implements BudgetRepositoryInterface
* @param array $data * @param array $data
* *
* @return Budget * @return Budget
* @throws FireflyException
*/ */
public function store(array $data): Budget public function store(array $data): Budget
{ {
$newBudget = new Budget( try {
[ $newBudget = Budget::create(
'user_id' => $this->user->id, [
'name' => $data['name'], 'user_id' => $this->user->id,
] 'name' => $data['name'],
); ]
$newBudget->save(); );
} catch(QueryException $e) {
throw new FireflyException('400002: Could not store budget.');
}
return $newBudget; return $newBudget;
} }

View File

@@ -23,6 +23,7 @@ declare(strict_types=1);
namespace FireflyIII\Repositories\Budget; namespace FireflyIII\Repositories\Budget;
use Carbon\Carbon; use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Budget; use FireflyIII\Models\Budget;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Support\Collection; use Illuminate\Support\Collection;
@@ -132,6 +133,7 @@ interface BudgetRepositoryInterface
* @param array $data * @param array $data
* *
* @return Budget * @return Budget
* @throws FireflyException
*/ */
public function store(array $data): Budget; public function store(array $data): Budget;

View File

@@ -24,6 +24,7 @@ namespace FireflyIII\Repositories\Category;
use Carbon\Carbon; use Carbon\Carbon;
use DB; use DB;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Factory\CategoryFactory; use FireflyIII\Factory\CategoryFactory;
use FireflyIII\Models\Category; use FireflyIII\Models\Category;
use FireflyIII\Models\RecurrenceTransactionMeta; use FireflyIII\Models\RecurrenceTransactionMeta;
@@ -239,6 +240,7 @@ class CategoryRepository implements CategoryRepositoryInterface
* @param array $data * @param array $data
* *
* @return Category * @return Category
* @throws FireflyException
*/ */
public function store(array $data): Category public function store(array $data): Category
{ {
@@ -246,7 +248,13 @@ class CategoryRepository implements CategoryRepositoryInterface
$factory = app(CategoryFactory::class); $factory = app(CategoryFactory::class);
$factory->setUser($this->user); $factory->setUser($this->user);
return $factory->findOrCreate(null, $data['name']); $category = $factory->findOrCreate(null, $data['name']);
if (null === $category) {
throw new FireflyException(sprintf('400003: Could not store new category with name "%s"', $data['name']));
}
return $category;
} }
/** /**

View File

@@ -23,6 +23,7 @@ declare(strict_types=1);
namespace FireflyIII\Repositories\Category; namespace FireflyIII\Repositories\Category;
use Carbon\Carbon; use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Category; use FireflyIII\Models\Category;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Support\Collection; use Illuminate\Support\Collection;
@@ -117,7 +118,7 @@ interface CategoryRepositoryInterface
/** /**
* @param array $data * @param array $data
* * @throws FireflyException
* @return Category * @return Category
*/ */
public function store(array $data): Category; public function store(array $data): Category;

View File

@@ -23,6 +23,7 @@ declare(strict_types=1);
namespace FireflyIII\Repositories\Currency; namespace FireflyIII\Repositories\Currency;
use Carbon\Carbon; use Carbon\Carbon;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Factory\TransactionCurrencyFactory; use FireflyIII\Factory\TransactionCurrencyFactory;
use FireflyIII\Models\AccountMeta; use FireflyIII\Models\AccountMeta;
use FireflyIII\Models\AvailableBudget; use FireflyIII\Models\AvailableBudget;
@@ -489,14 +490,20 @@ class CurrencyRepository implements CurrencyRepositoryInterface
/** /**
* @param array $data * @param array $data
* *
* @return TransactionCurrency|null * @return TransactionCurrency
* @throws FireflyException
*/ */
public function store(array $data): ?TransactionCurrency public function store(array $data): TransactionCurrency
{ {
/** @var TransactionCurrencyFactory $factory */ /** @var TransactionCurrencyFactory $factory */
$factory = app(TransactionCurrencyFactory::class); $factory = app(TransactionCurrencyFactory::class);
$result = $factory->create($data);
return $factory->create($data); if (null === $result) {
throw new FireflyException('400004: Could not store new currency.');
}
return $result;
} }
/** /**

View File

@@ -234,9 +234,9 @@ interface CurrencyRepositoryInterface
/** /**
* @param array $data * @param array $data
* *
* @return TransactionCurrency|null * @return TransactionCurrency
*/ */
public function store(array $data): ?TransactionCurrency; public function store(array $data): TransactionCurrency;
/** /**
* @param TransactionCurrency $currency * @param TransactionCurrency $currency