Fix possible null pointer.

This commit is contained in:
James Cole 2018-03-30 22:40:20 +02:00
parent 08ff3d8ad0
commit 8f3e84df4d
No known key found for this signature in database
GPG Key ID: C16961E655E74B5E
16 changed files with 437 additions and 35 deletions

View File

@ -26,6 +26,7 @@ use FireflyIII\Models\TransactionJournal;
use Illuminate\Queue\SerializesModels;
/**
* @codeCoverageIgnore
* Class StoredTransactionJournal.
*/
class StoredTransactionJournal extends Event

View File

@ -27,6 +27,9 @@ use Illuminate\Queue\SerializesModels;
/**
* Class UpdatedTransactionJournal.
*
* @codeCoverageIgnore
*
*/
class UpdatedTransactionJournal extends Event
{

View File

@ -24,6 +24,8 @@ declare(strict_types=1);
namespace FireflyIII\Factory;
use FireflyIII\Models\TransactionCurrency;
use Illuminate\Database\QueryException;
use Log;
/**
* Class TransactionCurrencyFactory
@ -33,21 +35,26 @@ class TransactionCurrencyFactory
/**
* @param array $data
*
* @return TransactionCurrency
* @return TransactionCurrency|null
*/
public function create(array $data): TransactionCurrency
public function create(array $data): ?TransactionCurrency
{
/** @var TransactionCurrency $currency */
$currency = TransactionCurrency::create(
[
'name' => $data['name'],
'code' => $data['code'],
'symbol' => $data['symbol'],
'decimal_places' => $data['decimal_places'],
]
);
$result = null;
try {
/** @var TransactionCurrency $currency */
$result = TransactionCurrency::create(
[
'name' => $data['name'],
'code' => $data['code'],
'symbol' => $data['symbol'],
'decimal_places' => $data['decimal_places'],
]
);
} catch (QueryException $e) {
Log::error(sprintf('Could not create new currency: %s', $e->getMessage()));
}
return $currency;
return $result;
}
/**

View File

@ -43,7 +43,7 @@ class TransactionJournalMetaFactory
$value = $data['data'];
/** @var TransactionJournalMeta $entry */
$entry = $data['journal']->transactionJournalMeta()->where('name', $data['name'])->first();
if (is_null($value) && !is_null($entry)) {
if (null === $value && null !== $entry) {
try {
$entry->delete();
} catch (Exception $e) { // @codeCoverageIgnore
@ -56,9 +56,9 @@ class TransactionJournalMetaFactory
if ($data['data'] instanceof Carbon) {
$value = $data['data']->toW3cString();
}
if (strlen(strval($value)) === 0) {
if ((string)$value === '') {
// don't store blank strings.
if (!is_null($entry)) {
if (null !== $entry) {
try {
$entry->delete();
} catch (Exception $e) { // @codeCoverageIgnore
@ -69,7 +69,6 @@ class TransactionJournalMetaFactory
return null;
}
if (null === $entry) {
$entry = new TransactionJournalMeta();
$entry->transactionJournal()->associate($data['journal']);

View File

@ -24,6 +24,7 @@ namespace FireflyIII\Handlers\Events;
use FireflyIII\Events\AdminRequestedTestMessage;
use FireflyIII\Mail\AdminTestMail;
use FireflyIII\Repositories\User\UserRepositoryInterface;
use Log;
use Mail;
use Session;
@ -43,6 +44,15 @@ class AdminEventHandler
*/
public function sendTestMessage(AdminRequestedTestMessage $event): bool
{
/** @var UserRepositoryInterface $repository */
$repository = app(UserRepositoryInterface::class);
// is user even admin?
if (!$repository->hasRole($event->user, 'owner')) {
return true;
}
$email = $event->user->email;
$ipAddress = $event->ipAddress;

View File

@ -26,6 +26,7 @@ namespace FireflyIII\Handlers\Events;
use FireflyConfig;
use FireflyIII\Events\RequestedVersionCheckStatus;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Repositories\User\UserRepositoryInterface;
use FireflyIII\Services\Github\Object\Release;
use FireflyIII\Services\Github\Request\UpdateRequest;
use FireflyIII\User;
@ -43,22 +44,26 @@ class VersionCheckEventHandler
public function checkForUpdates(RequestedVersionCheckStatus $event)
{
// in Sandstorm, cannot check for updates:
$sandstorm = 1 === intval(getenv('SANDSTORM'));
$sandstorm = 1 === (int)getenv('SANDSTORM');
if ($sandstorm === true) {
return;
return; // @codeCoverageIgnore
}
/** @var UserRepositoryInterface $repository */
$repository = app(UserRepositoryInterface::class);
/** @var User $user */
$user = $event->user;
if (!$user->hasRole('owner')) {
if (!$repository->hasRole($user, 'owner')) {
return;
}
$permission = FireflyConfig::get('permission_update_check', -1);
$lastCheckTime = FireflyConfig::get('last_update_check', time());
$now = time();
if ($now - $lastCheckTime->data < 604800) {
$diff = $now - $lastCheckTime->data;
Log::debug(sprintf('Difference is %d seconds.', $diff));
if ($diff < 604800) {
Log::debug(sprintf('Checked for updates less than a week ago (on %s).', date('Y-m-d H:i:s', $lastCheckTime->data)));
return;
@ -70,7 +75,7 @@ class VersionCheckEventHandler
// have actual permission?
if ($permission->data === -1) {
// never asked before.
session()->flash('info', strval(trans('firefly.check_for_updates_permission', ['link' => route('admin.update-check')])));
session()->flash('info', (string)trans('firefly.check_for_updates_permission', ['link' => route('admin.update-check')]));
return;
}
@ -79,7 +84,7 @@ class VersionCheckEventHandler
/** @var UpdateRequest $request */
$request = app(UpdateRequest::class);
$check = -2;
$first = new Release(['id' => '0', 'title' => '0', 'updated' => '2017-01-01', 'content' => '']);
$first = new Release(['id' => '0', 'title' => '0.2', 'updated' => '2017-01-01', 'content' => '']);
try {
$request->call();
$releases = $request->getReleases();
@ -87,6 +92,7 @@ class VersionCheckEventHandler
/** @var Release $first */
$first = reset($releases);
$check = version_compare($current, $first->getTitle());
Log::debug(sprintf('Comparing %s with %s, result is %s', $current, $first->getTitle(), $check));
FireflyConfig::set('last_update_check', time());
} catch (FireflyException $e) {
Log::error(sprintf('Could not check for updates: %s', $e->getMessage()));
@ -98,11 +104,13 @@ class VersionCheckEventHandler
if ($check === -1) {
// there is a new FF version!
$monthAndDayFormat = (string)trans('config.month_and_day');
$string = strval(
trans(
'firefly.update_new_version_alert',
['your_version' => $current, 'new_version' => $first->getTitle(), 'date' => $first->getUpdated()->formatLocalized($monthAndDayFormat)]
)
$string = (string)trans(
'firefly.update_new_version_alert',
[
'your_version' => $current,
'new_version' => $first->getTitle(),
'date' => $first->getUpdated()->formatLocalized($monthAndDayFormat),
]
);
}
if ($check !== 0) {

View File

@ -238,12 +238,15 @@ class BoxController extends Controller
/** @var Account $account */
foreach ($accounts as $account) {
$accountCurrency = $currency;
$accountCurrency = null;
$balance = $balances[$account->id] ?? '0';
$currencyId = (int)$repository->getMetaValue($account, 'currency_id');
if ($currencyId !== 0) {
$accountCurrency = $currencyRepos->findNull($currencyId);
}
if (null === $accountCurrency) {
$accountCurrency = $currency;
}
// if the account is a credit card, subtract the virtual balance from the balance,
// to better reflect that this is not money that is actually "yours".

View File

@ -49,4 +49,4 @@ class EncryptService
file_put_contents($path, $content);
}
}
}

View File

@ -223,6 +223,7 @@ class User extends Authenticatable
*
* @param string $name
*
* @deprecated
* @return bool
*/
public function hasRole(string $name): bool

View File

@ -181,4 +181,4 @@ class CreateExportTest extends TestCase
$this->assertEquals(1, $output);
}
}
}

View File

@ -280,4 +280,4 @@ class CreateImportTest extends TestCase
$this->assertEquals(1, $output);
}
}
}

View File

@ -139,6 +139,41 @@ class AccountFactoryTest extends TestCase
$this->assertEquals('', $account->getMeta('accountRole'));
}
/**
* Create an expense account. This overrules the virtual balance.
* Role should not be set.
*
* @covers \FireflyIII\Factory\AccountFactory
* @covers \FireflyIII\Factory\AccountMetaFactory
* @covers \FireflyIII\Services\Internal\Support\AccountServiceTrait
*/
public function testCreateBasicExpenseFullType()
{
$data = [
'account_type_id' => null,
'accountType' => 'Expense account',
'iban' => null,
'name' => 'Basic expense account #' . random_int(1, 1000),
'virtualBalance' => '1243',
'active' => true,
'accountRole' => 'defaultAsset',
];
/** @var AccountFactory $factory */
$factory = app(AccountFactory::class);
$factory->setUser($this->user());
$account = $factory->create($data);
// assert stuff about account:
$this->assertEquals($account->name, $data['name']);
$this->assertEquals(AccountType::EXPENSE, $account->accountType->type);
$this->assertEquals('', $account->iban);
$this->assertTrue($account->active);
$this->assertEquals('0', $account->virtual_balance);
$this->assertEquals('', $account->getMeta('accountRole'));
}
/**
* Submit IB data for asset account.
*

View File

@ -33,6 +33,29 @@ use Tests\TestCase;
*/
class TransactionCurrencyFactoryTest extends TestCase
{
/**
* @covers \FireflyIII\Factory\TransactionCurrencyFactory
*/
public function testCreate()
{
/** @var TransactionCurrencyFactory $factory */
$factory = app(TransactionCurrencyFactory::class);
$result = $factory->create(['name' => 'OK', 'code' => 'XXA', 'symbol' => 'Z', 'decimal_places' => 2]);
$this->assertNotNull($result);
$this->assertEquals('XXA', $result->code);
}
/**
* @covers \FireflyIII\Factory\TransactionCurrencyFactory
*/
public function testCreateEmpty()
{
/** @var TransactionCurrencyFactory $factory */
$factory = app(TransactionCurrencyFactory::class);
$result = $factory->create(['name' => null, 'code' => null, 'symbol' => null, 'decimal_places' => null]);
$this->assertNull($result);
}
/**
* @covers \FireflyIII\Factory\TransactionCurrencyFactory
*/

View File

@ -40,8 +40,9 @@ class TransactionJournalMetaFactoryTest extends TestCase
public function testUpdateOrCreateBasic()
{
/** @var TransactionJournal $journal */
$journal = $this->user()->transactionJournals()->where('transaction_type_id', 1)->first();
$set = [
$journal = $this->user()->transactionJournals()->inRandomOrder()->first();
$journal->transactionJournalMeta()->delete();
$set = [
'journal' => $journal,
'name' => 'hello',
'data' => 'bye!',
@ -60,8 +61,9 @@ class TransactionJournalMetaFactoryTest extends TestCase
public function testUpdateOrCreateDate()
{
/** @var TransactionJournal $journal */
$journal = $this->user()->transactionJournals()->where('transaction_type_id', 2)->first();
$set = [
$journal = $this->user()->transactionJournals()->inRandomOrder()->first();
$journal->transactionJournalMeta()->delete();
$set = [
'journal' => $journal,
'name' => 'hello',
'data' => new Carbon('2012-01-01'),
@ -101,5 +103,61 @@ class TransactionJournalMetaFactoryTest extends TestCase
$this->assertEquals(0, $journal->transactionJournalMeta()->count());
}
/**
* @covers \FireflyIII\Factory\TransactionJournalMetaFactory
*/
public function testUpdateOrCreateEmpty()
{
/** @var TransactionJournal $journal */
$journal = $this->user()->transactionJournals()->inRandomOrder()->first();
$journal->transactionJournalMeta()->delete();
$set = [
'journal' => $journal,
'name' => 'hello',
'data' => '',
];
/** @var TransactionJournalMetaFactory $factory */
$factory = app(TransactionJournalMetaFactory::class);
$result = $factory->updateOrCreate($set);
$this->assertEquals(0, $journal->transactionJournalMeta()->count());
$this->assertNull($result);
}
/**
* @covers \FireflyIII\Factory\TransactionJournalMetaFactory
*/
public function testUpdateOrCreateExistingEmpty()
{
/** @var TransactionJournal $journal */
$journal = $this->user()->transactionJournals()->inRandomOrder()->first();
$journal->transactionJournalMeta()->delete();
$set = [
'journal' => $journal,
'name' => 'hello',
'data' => 'SomeData',
];
/** @var TransactionJournalMetaFactory $factory */
$factory = app(TransactionJournalMetaFactory::class);
$result = $factory->updateOrCreate($set);
$this->assertEquals(1, $journal->transactionJournalMeta()->count());
$this->assertNotNull($result);
// overrule with empty entry:
$set = [
'journal' => $journal,
'name' => 'hello',
'data' => '',
];
/** @var TransactionJournalMetaFactory $factory */
$factory = app(TransactionJournalMetaFactory::class);
$result = $factory->updateOrCreate($set);
$this->assertEquals(0, $journal->transactionJournalMeta()->count());
$this->assertNull($result);
}
}

View File

@ -0,0 +1,82 @@
<?php
/**
* AdminEventHandlerTest.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
*
* This file is part of Firefly III.
*
* Firefly III is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Firefly III is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
declare(strict_types=1);
namespace Tests\Unit\Handlers\Events;
use FireflyIII\Events\AdminRequestedTestMessage;
use FireflyIII\Handlers\Events\AdminEventHandler;
use FireflyIII\Mail\AdminTestMail;
use FireflyIII\Repositories\User\UserRepositoryInterface;
use Illuminate\Support\Facades\Mail;
use Mockery;
use Tests\TestCase;
/**
* Class AdminEventHandlerTest
*/
class AdminEventHandlerTest extends TestCase
{
/**
* @covers \FireflyIII\Handlers\Events\AdminEventHandler::sendTestMessage
* @covers \FireflyIII\Events\AdminRequestedTestMessage
*/
public function testSendNoMessage()
{
$repository = $this->mock(UserRepositoryInterface::class);
$event = new AdminRequestedTestMessage($this->user(), '127.0.0.1');
$repository->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->andReturn(false)->once();
$listener = new AdminEventHandler();
$this->assertTrue($listener->sendTestMessage($event));
}
/**
* @covers \FireflyIII\Handlers\Events\AdminEventHandler::sendTestMessage
* @covers \FireflyIII\Events\AdminRequestedTestMessage
*/
public function testSendTestMessage()
{
Mail::fake();
$repository = $this->mock(UserRepositoryInterface::class);
$event = new AdminRequestedTestMessage($this->user(), '127.0.0.1');
$repository->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->andReturn(true)->once();
$listener = new AdminEventHandler();
$this->assertTrue($listener->sendTestMessage($event));
// assert a message was sent.
Mail::assertSent(
AdminTestMail::class, function ($mail) {
return $mail->hasTo('thegrumpydictator@gmail.com') && '127.0.0.1' === $mail->ipAddress;
}
);
}
}

View File

@ -0,0 +1,172 @@
<?php
/**
* VersionCheckEventHandlerTest.php
* Copyright (c) 2018 thegrumpydictator@gmail.com
*
* This file is part of Firefly III.
*
* Firefly III is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Firefly III is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
*/
declare(strict_types=1);
namespace Tests\Unit\Handlers\Events;
use FireflyConfig;
use FireflyIII\Events\RequestedVersionCheckStatus;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Handlers\Events\VersionCheckEventHandler;
use FireflyIII\Models\Configuration;
use FireflyIII\Repositories\User\UserRepositoryInterface;
use FireflyIII\Services\Github\Object\Release;
use FireflyIII\Services\Github\Request\UpdateRequest;
use Mockery;
use Tests\TestCase;
/**
* Class VersionCheckEventHandlerTest
*/
class VersionCheckEventHandlerTest extends TestCase
{
/**
*
*/
public function testCheckForUpdatesError()
{
$updateConfig = new Configuration;
$updateConfig->data = 1;
$checkConfig = new Configuration;
$checkConfig->data = time() - 604810;
$event = new RequestedVersionCheckStatus($this->user());
$request = $this->mock(UpdateRequest::class);
$repos = $this->mock(UserRepositoryInterface::class);
$repos->shouldReceive('hasRole')->andReturn(true)->once();
// report on config variables:
FireflyConfig::shouldReceive('get')->withArgs(['permission_update_check', -1])->once()->andReturn($updateConfig);
FireflyConfig::shouldReceive('get')->withArgs(['last_update_check', Mockery::any()])->once()->andReturn($checkConfig);
// request thing:
$request->shouldReceive('call')->once();
$request->shouldReceive('getReleases')->once()->andThrow(new FireflyException('Errrr'));
$handler = new VersionCheckEventHandler;
$handler->checkForUpdates($event);
}
/**
* @covers \FireflyIII\Events\RequestedVersionCheckStatus
* @covers \FireflyIII\Handlers\Events\VersionCheckEventHandler
*/
public function testCheckForUpdatesNewer()
{
$updateConfig = new Configuration;
$updateConfig->data = 1;
$checkConfig = new Configuration;
$checkConfig->data = time() - 604800;
$event = new RequestedVersionCheckStatus($this->user());
$request = $this->mock(UpdateRequest::class);
$repos = $this->mock(UserRepositoryInterface::class);
$repos->shouldReceive('hasRole')->andReturn(true)->once();
// is newer than default return:
$version = config('firefly.version');
$first = new Release(['id' => '1', 'title' => $version . '.1', 'updated' => '2017-05-01', 'content' => '']);
// report on config variables:
FireflyConfig::shouldReceive('get')->withArgs(['permission_update_check', -1])->once()->andReturn($updateConfig);
FireflyConfig::shouldReceive('get')->withArgs(['last_update_check', Mockery::any()])->once()->andReturn($checkConfig);
FireflyConfig::shouldReceive('set')->withArgs(['last_update_check', Mockery::any()])->once()->andReturn($checkConfig);
// request thing:
$request->shouldReceive('call')->once();
$request->shouldReceive('getReleases')->once()->andReturn([$first]);
$handler = new VersionCheckEventHandler;
$handler->checkForUpdates($event);
}
/**
*
*/
public function testCheckForUpdatesNoAdmin()
{
$updateConfig = new Configuration;
$updateConfig->data = 1;
$checkConfig = new Configuration;
$checkConfig->data = time() - 604800;
$event = new RequestedVersionCheckStatus($this->user());
$repos = $this->mock(UserRepositoryInterface::class);
$repos->shouldReceive('hasRole')->andReturn(false)->once();
$handler = new VersionCheckEventHandler;
$handler->checkForUpdates($event);
}
/**
*
*/
public function testCheckForUpdatesNoPermission()
{
$updateConfig = new Configuration;
$updateConfig->data = -1;
$checkConfig = new Configuration;
$checkConfig->data = time() - 604800;
$event = new RequestedVersionCheckStatus($this->user());
$repos = $this->mock(UserRepositoryInterface::class);
$repos->shouldReceive('hasRole')->andReturn(true)->once();
// report on config variables:
FireflyConfig::shouldReceive('get')->withArgs(['permission_update_check', -1])->once()->andReturn($updateConfig);
FireflyConfig::shouldReceive('get')->withArgs(['last_update_check', Mockery::any()])->once()->andReturn($checkConfig);
$handler = new VersionCheckEventHandler;
$handler->checkForUpdates($event);
}
/**
*
*/
public function testCheckForUpdatesTooRecent()
{
$updateConfig = new Configuration;
$updateConfig->data = 1;
$checkConfig = new Configuration;
$checkConfig->data = time() - 800;
$event = new RequestedVersionCheckStatus($this->user());
$repos = $this->mock(UserRepositoryInterface::class);
$repos->shouldReceive('hasRole')->andReturn(true)->once();
// report on config variables:
FireflyConfig::shouldReceive('get')->withArgs(['permission_update_check', -1])->once()->andReturn($updateConfig);
FireflyConfig::shouldReceive('get')->withArgs(['last_update_check', Mockery::any()])->once()->andReturn($checkConfig);
$handler = new VersionCheckEventHandler;
$handler->checkForUpdates($event);
}
}