Various code cleanup.

app/Helpers/Report/NetWorth.php

@@ -63,6 +63,7 @@ class NetWorth implements NetWorthInterface
      * @return array
      * @throws JsonException
      * @throws FireflyException
+     * @deprecated
      */
     public function getNetWorthByCurrency(Collection $accounts, Carbon $date): array
     {

app/Helpers/Report/NetWorthInterface.php

@@ -34,6 +34,8 @@ use Illuminate\Support\Collection;
 interface NetWorthInterface
 {
     /**
+     * TODO unsure why this is deprecated.
+     *
      * Returns the user's net worth in an array with the following layout:
      *
      * -

app/Http/Controllers/Json/RecurrenceController.php

@@ -71,6 +71,7 @@ class RecurrenceController extends Controller
      */
     public function events(Request $request): JsonResponse
     {
+        $occurrences = [];
         $return           = [];
         $start            = Carbon::createFromFormat('Y-m-d', $request->get('start'));
         $end              = Carbon::createFromFormat('Y-m-d', $request->get('end'));
@@ -106,20 +107,19 @@ class RecurrenceController extends Controller
         $repetition->weekend           = (int)$request->get('weekend');
         $actualEnd                     = clone $end;
 
-        switch ($endsAt) {
+        if('until_date' === $endsAt) {
-            default:
+            $actualEnd   = $endDate ?? clone $end;
-            case 'forever':
+            $occurrences = $this->recurring->getOccurrencesInRange($repetition, $actualStart, $actualEnd);
-                // simply generate up until $end. No change from default behavior.
-                $occurrences = $this->recurring->getOccurrencesInRange($repetition, $actualStart, $actualEnd);
-                break;
-            case 'until_date':
-                $actualEnd   = $endDate ?? clone $end;
-                $occurrences = $this->recurring->getOccurrencesInRange($repetition, $actualStart, $actualEnd);
-                break;
-            case 'times':
-                $occurrences = $this->recurring->getXOccurrences($repetition, $actualStart, $repetitions);
-                break;
         }
+        if('times' === $endsAt) {
+            $occurrences = $this->recurring->getXOccurrences($repetition, $actualStart, $repetitions);
+        }
+        if('times' !== $endsAt && 'until_date' !== $endsAt) {
+            // 'forever'
+            $occurrences = $this->recurring->getOccurrencesInRange($repetition, $actualStart, $actualEnd);
+
+        }
+
         /** @var Carbon $current */
         foreach ($occurrences as $current) {
             if ($current->gte($start)) {

app/Http/Controllers/Recurring/TriggerController.php

@@ -67,7 +67,7 @@ class TriggerController extends Controller
         foreach ($groups as $group) {
             /** @var TransactionJournal $journal */
             foreach ($group->transactionJournals as $journal) {
-                Log::debug(sprintf('Set date of journal #%d to today!', $journal->id, $date));
+                Log::debug(sprintf('Set date of journal #%d to today!', $journal->id));
                 $journal->date = Carbon::today();
                 $journal->save();
             }

app/Http/Controllers/Rule/EditController.php

@@ -170,9 +170,10 @@ class EditController extends Controller
                     ]
                 )->render();
             } catch (Throwable $e) {
-                Log::debug(sprintf('Throwable was thrown in getPreviousTriggers(): %s', $e->getMessage()));
+                $message = sprintf('Throwable was thrown in getPreviousTriggers(): %s', $e->getMessage());
+                Log::debug($message);
                 Log::error($e->getTraceAsString());
-                throw new FireflyException($result, 0, $e);
+                throw new FireflyException($message, 0, $e);
             }
             $index++;
         }

app/Http/Controllers/System/InstallController.php

@@ -26,6 +26,7 @@ namespace FireflyIII\Http\Controllers\System;
 use Artisan;
 use Cache;
 use Exception;
+use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Support\Facades\Preferences;
 use FireflyIII\Support\Http\Controllers\GetConfigurationData;
@@ -165,7 +166,17 @@ class InstallController extends Controller
                 $index++;
                 continue;
             }
-            $result = $this->executeCommand($command, $args);
+            try {
+                $result = $this->executeCommand($command, $args);
+            } catch (FireflyException $e) {
+                Log::error($e->getMessage());
+                Log::error($e->getTraceAsString());
+                if (strpos($e->getMessage(), 'open_basedir restriction in effect')) {
+                    $this->lastError = self::BASEDIR_ERROR;
+                }
+                $result          = false;
+                $this->lastError = sprintf('%s %s', self::OTHER_ERROR, $e->getMessage());
+            }
             if (false === $result) {
                 $response['errorMessage'] = $this->lastError;
                 $response['error']        = true;
@@ -184,7 +195,7 @@ class InstallController extends Controller
     /**
      * @param  string  $command
      * @param  array  $args
-     *
+     * @throws FireflyException
      * @return bool
      */
     private function executeCommand(string $command, array $args): bool
@@ -199,16 +210,7 @@ class InstallController extends Controller
                 Log::debug(Artisan::output());
             }
         } catch (Exception $e) {
-            Log::error($e->getMessage());
+            throw new FireflyException($e->getMessage(), 0, $e);
-            Log::error($e->getTraceAsString());
-            if (strpos($e->getMessage(), 'open_basedir restriction in effect')) {
-                $this->lastError = self::BASEDIR_ERROR;
-
-                return false;
-            }
-            $this->lastError = sprintf('%s %s', self::OTHER_ERROR, $e->getMessage());
-
-            return false;
         }
         // clear cache as well.
         Cache::clear();

app/Http/Requests/RecurrenceFormRequest.php

@@ -109,27 +109,34 @@ class RecurrenceFormRequest extends FormRequest
         $return['transactions'][0]['source_name']      = null;
         $return['transactions'][0]['destination_id']   = null;
         $return['transactions'][0]['destination_name'] = null;
-        // fill in source and destination account data
+        $throwError                                    = true;
-        switch ($this->convertString('transaction_type')) {
+        $type                                          = $this->convertString('transaction_type');
-            default:
+        if ('withdrawal' === $type) {
-                throw new FireflyException(sprintf('Cannot handle transaction type "%s"', $this->convertString('transaction_type')));
+            $throwError                                  = false;
-            case 'withdrawal':
+            $return['transactions'][0]['source_id']      = $this->convertInteger('source_id');
-                $return['transactions'][0]['source_id']      = $this->convertInteger('source_id');
+            $return['transactions'][0]['destination_id'] = $this->convertInteger('withdrawal_destination_id');
-                $return['transactions'][0]['destination_id'] = $this->convertInteger('withdrawal_destination_id');
+        }
-                break;
+        if ('deposit' === $type) {
-            case 'deposit':
+            $throwError                                  = false;
-                $return['transactions'][0]['source_id']      = $this->convertInteger('deposit_source_id');
+            $return['transactions'][0]['source_id']      = $this->convertInteger('deposit_source_id');
-                $return['transactions'][0]['destination_id'] = $this->convertInteger('destination_id');
+            $return['transactions'][0]['destination_id'] = $this->convertInteger('destination_id');
-                break;
+        }
-            case 'transfer':
+        if ('transfer' === $type) {
-                $return['transactions'][0]['source_id']      = $this->convertInteger('source_id');
+            $throwError                                  = false;
-                $return['transactions'][0]['destination_id'] = $this->convertInteger('destination_id');
+            $return['transactions'][0]['source_id']      = $this->convertInteger('source_id');
-                break;
+            $return['transactions'][0]['destination_id'] = $this->convertInteger('destination_id');
+        }
+        if (true === $throwError) {
+            throw new FireflyException(sprintf('Cannot handle transaction type "%s"', $this->convertString('transaction_type')));
         }
 
         // replace category name with a new category:
         $factory = app(CategoryFactory::class);
         $factory->setUser(auth()->user());
+        /**
+         * @var int $index
+         * @var array $transaction
+         */
         foreach ($return['transactions'] as $index => $transaction) {
             $categoryName = $transaction['category_name'] ?? null;
             if (null !== $categoryName) {
@@ -239,23 +246,19 @@ class RecurrenceFormRequest extends FormRequest
         }
 
         // switch on type to expand rules for source and destination accounts:
-        switch ($this->convertString('transaction_type')) {
+        $type = strtolower($this->convertString('transaction_type'));
-            case strtolower(TransactionType::WITHDRAWAL):
+        if (strtolower(TransactionType::WITHDRAWAL) === $type) {
-                $rules['source_id']        = 'required|exists:accounts,id|belongsToUser:accounts';
+            $rules['source_id']        = 'required|exists:accounts,id|belongsToUser:accounts';
-                $rules['destination_name'] = 'between:1,255|nullable';
+            $rules['destination_name'] = 'between:1,255|nullable';
-                break;
+        }
-            case strtolower(TransactionType::DEPOSIT):
+        if (strtolower(TransactionType::DEPOSIT) === $type) {
-                $rules['source_name']    = 'between:1,255|nullable';
+            $rules['source_name']    = 'between:1,255|nullable';
-                $rules['destination_id'] = 'required|exists:accounts,id|belongsToUser:accounts';
+            $rules['destination_id'] = 'required|exists:accounts,id|belongsToUser:accounts';
-                break;
+        }
-            case strtolower(TransactionType::TRANSFER):
+        if (strtolower(TransactionType::TRANSFER) === $type) {
-                // this may not work:
+            // this may not work:
-                $rules['source_id']      = 'required|exists:accounts,id|belongsToUser:accounts|different:destination_id';
+            $rules['source_id']      = 'required|exists:accounts,id|belongsToUser:accounts|different:destination_id';
-                $rules['destination_id'] = 'required|exists:accounts,id|belongsToUser:accounts|different:source_id';
+            $rules['destination_id'] = 'required|exists:accounts,id|belongsToUser:accounts|different:source_id';
-
-                break;
-            default:
-                throw new FireflyException(sprintf('Cannot handle transaction type of type "%s"', $this->convertString('transaction_type')));
         }
 
         // update some rules in case the user is editing a post:
@@ -309,23 +312,28 @@ class RecurrenceFormRequest extends FormRequest
         $destinationId = null;
 
         // TODO typeOverrule: the account validator may have another opinion the transaction type.
-
+        // TODO either use 'withdrawal' or the strtolower() variant, not both.
-        switch ($this->convertString('transaction_type')) {
+        $type = $this->convertString('transaction_type');
-            default:
+        $throwError = true;
-                throw new FireflyException(sprintf('Cannot handle transaction type "%s"', $this->convertString('transaction_type')));
+        if('withdrawal' === $type) {
-            case 'withdrawal':
+            $throwError = false;
-                $sourceId      = (int)$data['source_id'];
+            $sourceId      = (int)$data['source_id'];
-                $destinationId = (int)$data['withdrawal_destination_id'];
+            $destinationId = (int)$data['withdrawal_destination_id'];
-                break;
-            case 'deposit':
-                $sourceId      = (int)$data['deposit_source_id'];
-                $destinationId = (int)$data['destination_id'];
-                break;
-            case 'transfer':
-                $sourceId      = (int)$data['source_id'];
-                $destinationId = (int)$data['destination_id'];
-                break;
         }
+        if('deposit' === $type) {
+            $throwError = false;
+            $sourceId      = (int)$data['deposit_source_id'];
+            $destinationId = (int)$data['destination_id'];
+        }
+        if('transfer' === $type) {
+            $throwError = false;
+            $sourceId      = (int)$data['source_id'];
+            $destinationId = (int)$data['destination_id'];
+        }
+        if(true === $throwError) {
+            throw new FireflyException(sprintf('Cannot handle transaction type "%s"', $this->convertString('transaction_type')));
+        }
+
         // validate source account.
         $validSource = $accountValidator->validateSource(['id' => $sourceId,]);
 

app/Jobs/CreateAutoBudgetLimits.php

@@ -160,33 +160,36 @@ class CreateAutoBudgetLimits implements ShouldQueue
      */
     private function isMagicDay(AutoBudget $autoBudget): bool
     {
-        switch ($autoBudget->period) {
+        if ('daily' === $autoBudget->period) {
-            default:
+            return true;
-                throw new FireflyException(sprintf('isMagicDay() can\'t handle period "%s"', $autoBudget->period));
-            case 'daily':
-                // every day is magic!
-                return true;
-            case 'weekly':
-                // fire on Monday.
-                return $this->date->isMonday();
-            case 'monthly':
-                return 1 === $this->date->day;
-            case 'quarterly':
-                $format = 'm-d';
-                $value  = $this->date->format($format);
-
-                return in_array($value, ['01-01', '04-01', '07-01', '10-01'], true);
-            case 'half_year':
-                $format = 'm-d';
-                $value  = $this->date->format($format);
-
-                return in_array($value, ['01-01', '07-01'], true);
-            case 'yearly':
-                $format = 'm-d';
-                $value  = $this->date->format($format);
-
-                return '01-01' === $value;
         }
+
+        if ('weekly' === $autoBudget->period) {
+            return $this->date->isMonday();
+        }
+
+        if ('monthly' === $autoBudget->period) {
+            return 1 === $this->date->day;
+        }
+        if ('quarterly' === $autoBudget->period) {
+            $format = 'm-d';
+            $value  = $this->date->format($format);
+
+            return in_array($value, ['01-01', '04-01', '07-01', '10-01'], true);
+        }
+        if ('half_year' === $autoBudget->period) {
+            $format = 'm-d';
+            $value  = $this->date->format($format);
+
+            return in_array($value, ['01-01', '07-01'], true);
+        }
+        if ('yearly' === $autoBudget->period) {
+            $format = 'm-d';
+            $value  = $this->date->format($format);
+
+            return '01-01' === $value;
+        }
+        throw new FireflyException(sprintf('isMagicDay() can\'t handle period "%s"', $autoBudget->period));
     }
 
     /**

app/Jobs/MailError.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace FireflyIII\Jobs;
 
 use Exception;
+use FireflyIII\Exceptions\FireflyException;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Mail\Message;
 use Illuminate\Queue\InteractsWithQueue;
@@ -68,6 +69,7 @@ class MailError extends Job implements ShouldQueue
 
     /**
      * Execute the job.
+     * @throws FireflyException
      */
     public function handle()
     {
@@ -89,7 +91,7 @@ class MailError extends Job implements ShouldQueue
                     }
                 );
             } catch (Exception $e) {
-                Log::error('Exception when mailing: '.$e->getMessage());
+                throw new FireflyException($e->getMessage(), 0, $e);
             }
         }
     }

app/Mail/NewIPAddressWarningMail.php

@@ -24,10 +24,11 @@ declare(strict_types=1);
 
 namespace FireflyIII\Mail;
 
-use Exception;
+use FireflyIII\Exceptions\FireflyException;
 use Illuminate\Bus\Queueable;
 use Illuminate\Mail\Mailable;
 use Illuminate\Queue\SerializesModels;
+use Log;
 
 /**
  * Class NewIPAddressWarningMail
@@ -61,8 +62,9 @@ class NewIPAddressWarningMail extends Mailable
         $this->time = now(config('app.timezone'))->isoFormat((string)trans('config.date_time_js'));
         $this->host = '';
         try {
-            $hostName = gethostbyaddr($this->ipAddress);
+            $hostName = app('steam')->getHostName($this->ipAddress);
-        } catch (Exception $e) {
+        } catch (FireflyException $e) {
+            Log::error($e->getMessage());
             $hostName = $this->ipAddress;
         }
         if ($hostName !== $this->ipAddress) {

app/Notifications/User/UserLogin.php

@@ -25,10 +25,12 @@ declare(strict_types=1);
 namespace FireflyIII\Notifications\User;
 
 use Exception;
+use FireflyIII\Exceptions\FireflyException;
 use Illuminate\Bus\Queueable;
 use Illuminate\Notifications\Messages\MailMessage;
 use Illuminate\Notifications\Messages\SlackMessage;
 use Illuminate\Notifications\Notification;
+use Illuminate\Support\Facades\Log;
 
 class UserLogin extends Notification
 {
@@ -70,8 +72,9 @@ class UserLogin extends Notification
         $time = now(config('app.timezone'))->isoFormat((string)trans('config.date_time_js'));
         $host = '';
         try {
-            $hostName = gethostbyaddr($this->ip);
+            $hostName = app('steam')->getHostName($this->ip);
-        } catch (Exception $e) {
+        } catch (FireflyException $e) {
+            Log::error($e->getMessage());
             $hostName = $this->ip;
         }
         if ($hostName !== $this->ip) {
@@ -93,8 +96,9 @@ class UserLogin extends Notification
     {
         $host = '';
         try {
-            $hostName = gethostbyaddr($this->ip);
+            $hostName = app('steam')->getHostName($this->ip);
-        } catch (Exception $e) {
+        } catch (FireflyException $e) {
+            Log::error($e->getMessage());
             $hostName = $this->ip;
         }
         if ($hostName !== $this->ip) {

app/Repositories/Attachment/AttachmentRepository.php

@@ -34,6 +34,9 @@ use FireflyIII\User;
 use Illuminate\Contracts\Encryption\DecryptException;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Storage;
+use League\Flysystem\UnableToDeleteFile;
+use LogicException;
+use Log;
 
 /**
  * Class AttachmentRepository.
@@ -58,7 +61,7 @@ class AttachmentRepository implements AttachmentRepositoryInterface
         $path = $helper->getAttachmentLocation($attachment);
         try {
             Storage::disk('upload')->delete($path);
-        } catch (Exception $e) {
+        } catch (UnableToDeleteFile $e) {
             // @ignoreException
         }
         $attachment->delete();
@@ -161,7 +164,6 @@ class AttachmentRepository implements AttachmentRepositoryInterface
      * @param  array  $data
      *
      * @return Attachment
-     * @throws Exception
      */
     public function update(Attachment $attachment, array $data): Attachment
     {
@@ -193,7 +195,6 @@ class AttachmentRepository implements AttachmentRepositoryInterface
      * @param  string  $note
      *
      * @return bool
-     * @throws Exception
      */
     public function updateNote(Attachment $attachment, string $note): bool
     {
@@ -202,8 +203,8 @@ class AttachmentRepository implements AttachmentRepositoryInterface
             if (null !== $dbNote) {
                 try {
                     $dbNote->delete();
-                } catch (Exception $e) {
+                } catch (LogicException $e) {
-                    // @ignoreException
+                    Log::error($e->getMessage());
                 }
             }
 

app/Repositories/Bill/BillRepository.php

@@ -383,6 +383,8 @@ class BillRepository implements BillRepositoryInterface
     }
 
     /**
+     * TODO unsure why this is deprecated.
+     *
      * Get the total amount of money paid for the users active bills in the date range given.
      * This amount will be negative (they're expenses). This method is equal to
      * getBillsUnpaidInRange. So the debug comments are gone.
@@ -412,6 +414,8 @@ class BillRepository implements BillRepositoryInterface
     }
 
     /**
+     * TODO unsure why this is deprecated.
+     *
      * Get the total amount of money paid for the users active bills in the date range given,
      * grouped per currency.
      * @param  Carbon  $start
@@ -442,6 +446,8 @@ class BillRepository implements BillRepositoryInterface
     }
 
     /**
+     * TODO unsure why this is deprecated.
+     *
      * Get the total amount of money due for the users active bills in the date range given. This amount will be positive.
      *
      * @param  Carbon  $start
@@ -474,6 +480,8 @@ class BillRepository implements BillRepositoryInterface
     }
 
     /**
+     * TODO unsure why this is deprecated.
+     *
      * Get the total amount of money due for the users active bills in the date range given.
      *
      * @param  Carbon  $start

app/Repositories/Budget/AvailableBudgetRepository.php

@@ -24,7 +24,6 @@ declare(strict_types=1);
 namespace FireflyIII\Repositories\Budget;
 
 use Carbon\Carbon;
-use Exception;
 use FireflyIII\Models\AvailableBudget;
 use FireflyIII\Models\TransactionCurrency;
 use FireflyIII\User;
@@ -52,11 +51,7 @@ class AvailableBudgetRepository implements AvailableBudgetRepositoryInterface
      */
     public function destroyAvailableBudget(AvailableBudget $availableBudget): void
     {
-        try {
+        $availableBudget->delete();
-            $availableBudget->delete();
-        } catch (Exception $e) {
-            // @ignoreException
-        }
     }
 
     /**

app/Repositories/Budget/OperationsRepositoryInterface.php

@@ -89,6 +89,7 @@ interface OperationsRepositoryInterface
     public function spentInPeriodMc(Collection $budgets, Collection $accounts, Carbon $start, Carbon $end): array;
 
     /**
+     * TODO this method was marked as deprecated but I'm not sure why.
      * @param  Carbon  $start
      * @param  Carbon  $end
      * @param  Collection|null  $accounts
@@ -96,7 +97,7 @@ interface OperationsRepositoryInterface
      * @param  TransactionCurrency|null  $currency
      *
      * @return array
-     * @deprecated
+     *
      */
     public function sumExpenses(
         Carbon $start,

app/Support/Steam.php

@@ -25,6 +25,7 @@ namespace FireflyIII\Support;
 
 use Carbon\Carbon;
 use DB;
+use Exception;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\Transaction;
@@ -667,4 +668,19 @@ class Steam
 
         return $amount;
     }
+
+    /**
+     * @param  string  $ipAddress
+     * @return string
+     * @throws FireflyException
+     */
+    public function getHostName(string $ipAddress): string
+    {
+        try {
+            $hostName = gethostbyaddr($ipAddress);
+        } catch (Exception $e) {
+            throw new FireflyException($e->getMessage(), 0, $e);
+        }
+        return $hostName;
+    }
 }

app/Validation/FireflyValidator.php

@@ -126,9 +126,9 @@ class FireflyValidator extends Validator
      *
      * @return bool
      */
-    public function validateIban($attribute, $value): bool
+    public function validateIban(mixed $attribute, mixed $value): bool
     {
-        if (null === $value || !is_string($value) || strlen($value) < 6) {
+        if (!is_string($value) || strlen($value) < 6) {
             return false;
         }
         // strip spaces