Refactor code and fix tests.

This commit is contained in:
James Cole
2018-05-21 09:40:19 +02:00
parent 714b54ed06
commit ebf97f710f
16 changed files with 63 additions and 52 deletions

View File

@@ -28,7 +28,7 @@ namespace FireflyIII\Import\JobConfiguration;
use FireflyIII\Exceptions\FireflyException; use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\ImportJob; use FireflyIII\Models\ImportJob;
use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface; use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface;
use FireflyIII\Support\Import\JobConfiguration\File\ConfigurationInterface; use FireflyIII\Support\Import\JobConfiguration\File\FileConfigurationInterface;
use FireflyIII\Support\Import\JobConfiguration\File\ConfigureMappingHandler; use FireflyIII\Support\Import\JobConfiguration\File\ConfigureMappingHandler;
use FireflyIII\Support\Import\JobConfiguration\File\ConfigureRolesHandler; use FireflyIII\Support\Import\JobConfiguration\File\ConfigureRolesHandler;
use FireflyIII\Support\Import\JobConfiguration\File\ConfigureUploadHandler; use FireflyIII\Support\Import\JobConfiguration\File\ConfigureUploadHandler;
@@ -128,10 +128,10 @@ class FileJobConfiguration implements JobConfigurationInterface
/** /**
* Get the configuration handler for this specific stage. * Get the configuration handler for this specific stage.
* *
* @return ConfigurationInterface * @return FileConfigurationInterface
* @throws FireflyException * @throws FireflyException
*/ */
private function getConfigurationObject(): ConfigurationInterface private function getConfigurationObject(): FileConfigurationInterface
{ {
$class = 'DoNotExist'; $class = 'DoNotExist';
switch ($this->importJob->stage) { switch ($this->importJob->stage) {

View File

@@ -26,12 +26,16 @@ namespace FireflyIII\Import\JobConfiguration;
use FireflyIII\Exceptions\FireflyException; use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\ImportJob; use FireflyIII\Models\ImportJob;
use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface; use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface;
use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticateConfig;
use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticatedConfigHandler; use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticatedConfigHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticatedHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticateHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseAccount; use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseAccount;
use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseAccountsHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseLoginHandler; use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseLoginHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\DoAuthenticateHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\NewConfig; use FireflyIII\Support\Import\JobConfiguration\Spectre\NewConfig;
use FireflyIII\Support\Import\JobConfiguration\Spectre\SpectreJobConfig; use FireflyIII\Support\Import\JobConfiguration\Spectre\NewSpectreJobHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\SpectreConfigurationInterface;
use Illuminate\Support\MessageBag; use Illuminate\Support\MessageBag;
use Log; use Log;
@@ -40,7 +44,7 @@ use Log;
*/ */
class SpectreJobConfiguration implements JobConfigurationInterface class SpectreJobConfiguration implements JobConfigurationInterface
{ {
/** @var SpectreJobConfig */ /** @var SpectreConfigurationInterface */
private $handler; private $handler;
/** @var ImportJob */ /** @var ImportJob */
private $importJob; private $importJob;
@@ -104,42 +108,43 @@ class SpectreJobConfiguration implements JobConfigurationInterface
} }
/** /**
* @return SpectreJobConfig * @return SpectreConfigurationInterface
* @throws FireflyException * @throws FireflyException
*/ */
private function getHandler(): SpectreJobConfig private function getHandler(): SpectreConfigurationInterface
{ {
Log::debug(sprintf('Now in SpectreJobConfiguration::getHandler() with stage "%s"', $this->importJob->stage)); Log::debug(sprintf('Now in SpectreJobConfiguration::getHandler() with stage "%s"', $this->importJob->stage));
$handler = null; $handler = null;
switch ($this->importJob->stage) { switch ($this->importJob->stage) {
case 'new': case 'new':
$handler = app(NewConfig::class); /** @var NewSpectreJobHandler $handler */
$handler = app(NewSpectreJobHandler::class);
$handler->setImportJob($this->importJob); $handler->setImportJob($this->importJob);
break; break;
case 'authenticate': case 'do-authenticate':
/** @var SpectreJobConfig $handler */ /** @var DoAuthenticateHandler $handler */
$handler = app(AuthenticateConfig::class); $handler = app(DoAuthenticateHandler::class);
$handler->setImportJob($this->importJob); $handler->setImportJob($this->importJob);
break; break;
case 'choose-login': case 'choose-login':
/** @var SpectreJobConfig $handler */ /** @var ChooseLoginHandler $handler */
$handler = app(ChooseLoginHandler::class); $handler = app(ChooseLoginHandler::class);
$handler->setImportJob($this->importJob); $handler->setImportJob($this->importJob);
break; break;
case 'authenticated': case 'authenticated':
/** @var AuthenticatedConfigHandler $handler */ /** @var AuthenticatedHandler $handler */
$handler = app(AuthenticatedConfigHandler::class); $handler = app(AuthenticatedHandler::class);
$handler->setImportJob($this->importJob); $handler->setImportJob($this->importJob);
break; break;
case 'choose-account': case 'choose-accounts':
/** @var ChooseAccount $handler */ /** @var ChooseAccountsHandler $handler */
$handler = app(ChooseAccount::class); $handler = app(ChooseAccountsHandler::class);
$handler->setImportJob($this->importJob); $handler->setImportJob($this->importJob);
break; break;
default: default:
// @codeCoverageIgnoreStart // @codeCoverageIgnoreStart
throw new FireflyException(sprintf('Firefly III cannot create a configuration handler for stage "%s"', $this->importJob->stage)); throw new FireflyException(sprintf('Firefly III cannot create a configuration handler for stage "%s"', $this->importJob->stage));
// @codeCoverageIgnoreEnd // @codeCoverageIgnoreEnd
} }
return $handler; return $handler;

View File

@@ -67,7 +67,7 @@ class SpectreRoutine implements RoutineInterface
// if count logins is zero, go to authenticate stage // if count logins is zero, go to authenticate stage
if ($handler->getCountLogins() === 0) { if ($handler->getCountLogins() === 0) {
$this->repository->setStage($this->importJob, 'authenticate'); $this->repository->setStage($this->importJob, 'do-authenticate');
$this->repository->setStatus($this->importJob, 'ready_to_run'); $this->repository->setStatus($this->importJob, 'ready_to_run');
return; return;
@@ -76,7 +76,7 @@ class SpectreRoutine implements RoutineInterface
$this->repository->setStage($this->importJob, 'choose-login'); $this->repository->setStage($this->importJob, 'choose-login');
$this->repository->setStatus($this->importJob, 'need_job_config'); $this->repository->setStatus($this->importJob, 'need_job_config');
break; break;
case 'authenticate': case 'do-authenticate':
// set job to require config. // set job to require config.
$this->repository->setStatus($this->importJob, 'need_job_config'); $this->repository->setStatus($this->importJob, 'need_job_config');
@@ -90,7 +90,7 @@ class SpectreRoutine implements RoutineInterface
$handler->run(); $handler->run();
// return to config to select account(s). // return to config to select account(s).
$this->repository->setStage($this->importJob, 'choose-account'); $this->repository->setStage($this->importJob, 'choose-accounts');
$this->repository->setStatus($this->importJob, 'need_job_config'); $this->repository->setStatus($this->importJob, 'need_job_config');
break; break;
case 'go-for-import': case 'go-for-import':

View File

@@ -40,7 +40,7 @@ use Log;
/** /**
* Class ConfigureMappingHandler * Class ConfigureMappingHandler
*/ */
class ConfigureMappingHandler implements ConfigurationInterface class ConfigureMappingHandler implements FileConfigurationInterface
{ {
/** @var AttachmentHelperInterface */ /** @var AttachmentHelperInterface */
private $attachments; private $attachments;

View File

@@ -39,7 +39,7 @@ use Log;
/** /**
* Class ConfigureRolesHandler * Class ConfigureRolesHandler
*/ */
class ConfigureRolesHandler implements ConfigurationInterface class ConfigureRolesHandler implements FileConfigurationInterface
{ {
/** @var AttachmentHelperInterface */ /** @var AttachmentHelperInterface */
private $attachments; private $attachments;

View File

@@ -33,7 +33,7 @@ use Log;
/** /**
* Class ConfigureUploadHandler * Class ConfigureUploadHandler
*/ */
class ConfigureUploadHandler implements ConfigurationInterface class ConfigureUploadHandler implements FileConfigurationInterface
{ {
/** @var ImportJob */ /** @var ImportJob */
private $importJob; private $importJob;

View File

@@ -26,9 +26,9 @@ use FireflyIII\Models\ImportJob;
use Illuminate\Support\MessageBag; use Illuminate\Support\MessageBag;
/** /**
* Class ConfigurationInterface. * Class FileConfigurationInterface.
*/ */
interface ConfigurationInterface interface FileConfigurationInterface
{ {
/** /**
* Store data associated with current stage. * Store data associated with current stage.

View File

@@ -37,7 +37,7 @@ use Log;
/** /**
* Class NewFileJobHandler * Class NewFileJobHandler
*/ */
class NewFileJobHandler implements ConfigurationInterface class NewFileJobHandler implements FileConfigurationInterface
{ {
/** @var AttachmentHelperInterface */ /** @var AttachmentHelperInterface */
private $attachments; private $attachments;

View File

@@ -28,7 +28,10 @@ use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface;
use Illuminate\Support\MessageBag; use Illuminate\Support\MessageBag;
use Log; use Log;
class AuthenticatedConfigHandler implements SpectreJobConfig /**
* Class AuthenticatedHandler
*/
class AuthenticatedHandler implements SpectreConfigurationInterface
{ {
/** @var ImportJob */ /** @var ImportJob */
private $importJob; private $importJob;

View File

@@ -38,10 +38,10 @@ use Illuminate\Support\MessageBag;
use Log; use Log;
/** /**
* Class ChooseAccount * Class ChooseAccountsHandler
* *
*/ */
class ChooseAccount implements SpectreJobConfig class ChooseAccountsHandler implements SpectreConfigurationInterface
{ {
/** @var AccountRepositoryInterface */ /** @var AccountRepositoryInterface */

View File

@@ -40,7 +40,7 @@ use Log;
* Class ChooseLoginHandler * Class ChooseLoginHandler
* *
*/ */
class ChooseLoginHandler implements SpectreJobConfig class ChooseLoginHandler implements SpectreConfigurationInterface
{ {
/** @var ImportJob */ /** @var ImportJob */
private $importJob; private $importJob;
@@ -93,7 +93,7 @@ class ChooseLoginHandler implements SpectreJobConfig
$config['token'] = $token->toArray(); $config['token'] = $token->toArray();
$this->repository->setConfiguration($this->importJob, $config); $this->repository->setConfiguration($this->importJob, $config);
// move job to correct stage to redirect to Spectre: // move job to correct stage to redirect to Spectre:
$this->repository->setStage($this->importJob, 'authenticate'); $this->repository->setStage($this->importJob, 'do-authenticate');
return new MessageBag; return new MessageBag;

View File

@@ -38,7 +38,7 @@ use Log;
* Class AuthenticateConfig * Class AuthenticateConfig
* *
*/ */
class AuthenticateConfig implements SpectreJobConfig class DoAuthenticateHandler implements SpectreConfigurationInterface
{ {
/** @var ImportJob */ /** @var ImportJob */
private $importJob; private $importJob;

View File

@@ -31,7 +31,7 @@ use Illuminate\Support\MessageBag;
* Class NewConfig * Class NewConfig
* *
*/ */
class NewConfig implements SpectreJobConfig class NewSpectreJobHandler implements SpectreConfigurationInterface
{ {
/** /**

View File

@@ -31,7 +31,7 @@ use Illuminate\Support\MessageBag;
* Interface SpectreJobConfig * Interface SpectreJobConfig
* *
*/ */
interface SpectreJobConfig interface SpectreConfigurationInterface
{ {
/** /**
* Return true when this stage is complete. * Return true when this stage is complete.

View File

@@ -26,11 +26,11 @@ namespace Tests\Unit\Import\JobConfiguration;
use FireflyIII\Exceptions\FireflyException; use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Import\JobConfiguration\SpectreJobConfiguration; use FireflyIII\Import\JobConfiguration\SpectreJobConfiguration;
use FireflyIII\Models\ImportJob; use FireflyIII\Models\ImportJob;
use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticateConfig; use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticatedHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticatedConfigHandler; use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseAccountsHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseAccount;
use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseLoginHandler; use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseLoginHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\NewConfig; use FireflyIII\Support\Import\JobConfiguration\Spectre\DoAuthenticateHandler;
use FireflyIII\Support\Import\JobConfiguration\Spectre\NewSpectreJobHandler;
use Illuminate\Support\MessageBag; use Illuminate\Support\MessageBag;
use Tests\TestCase; use Tests\TestCase;
@@ -54,8 +54,8 @@ class SpectreJobConfigurationTest extends TestCase
$job->configuration = []; $job->configuration = [];
$job->save(); $job->save();
// expect "NewConfig" to be created because job is new. // expect "NewSpectreJobHandler" to be created because job is new.
$handler = $this->mock(NewConfig::class); $handler = $this->mock(NewSpectreJobHandler::class);
$handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('setImportJob')->once();
$handler->shouldReceive('configurationComplete')->once()->andReturn(true); $handler->shouldReceive('configurationComplete')->once()->andReturn(true);
@@ -77,7 +77,7 @@ class SpectreJobConfigurationTest extends TestCase
$job->user_id = $this->user()->id; $job->user_id = $this->user()->id;
$job->key = 'spectre_jc_B' . random_int(1, 1000); $job->key = 'spectre_jc_B' . random_int(1, 1000);
$job->status = 'new'; $job->status = 'new';
$job->stage = 'authenticate'; $job->stage = 'do-authenticate';
$job->provider = 'spectre'; $job->provider = 'spectre';
$job->file_type = ''; $job->file_type = '';
$job->configuration = []; $job->configuration = [];
@@ -86,8 +86,8 @@ class SpectreJobConfigurationTest extends TestCase
$return = new MessageBag(); $return = new MessageBag();
$return->add('some', 'return message'); $return->add('some', 'return message');
// expect "NewConfig" to be created because job is new. // expect "DoAuthenticateHandler" to be created because job is in "do-authenticate".
$handler = $this->mock(AuthenticateConfig::class); $handler = $this->mock(DoAuthenticateHandler::class);
$handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('setImportJob')->once();
$handler->shouldReceive('configureJob')->once()->withArgs([$configData])->andReturn($return); $handler->shouldReceive('configureJob')->once()->withArgs([$configData])->andReturn($return);
@@ -116,6 +116,7 @@ class SpectreJobConfigurationTest extends TestCase
$job->save(); $job->save();
$data = ['ssome' => 'values']; $data = ['ssome' => 'values'];
// Expect choose-login handler because of state.
$handler = $this->mock(ChooseLoginHandler::class); $handler = $this->mock(ChooseLoginHandler::class);
$handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('setImportJob')->once();
$handler->shouldReceive('getNextData')->once()->andReturn($data); $handler->shouldReceive('getNextData')->once()->andReturn($data);
@@ -144,7 +145,8 @@ class SpectreJobConfigurationTest extends TestCase
$job->configuration = []; $job->configuration = [];
$job->save(); $job->save();
$handler = $this->mock(AuthenticatedConfigHandler::class); // expect "AuthenticatedHandler" because of state.
$handler = $this->mock(AuthenticatedHandler::class);
$handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('setImportJob')->once();
$handler->shouldReceive('getNextView')->once()->andReturn('import.fake.view'); $handler->shouldReceive('getNextView')->once()->andReturn('import.fake.view');
@@ -166,13 +168,14 @@ class SpectreJobConfigurationTest extends TestCase
$job->user_id = $this->user()->id; $job->user_id = $this->user()->id;
$job->key = 'spectre_jc_E' . random_int(1, 1000); $job->key = 'spectre_jc_E' . random_int(1, 1000);
$job->status = 'new'; $job->status = 'new';
$job->stage = 'choose-account'; $job->stage = 'choose-accounts';
$job->provider = 'spectre'; $job->provider = 'spectre';
$job->file_type = ''; $job->file_type = '';
$job->configuration = []; $job->configuration = [];
$job->save(); $job->save();
$handler = $this->mock(ChooseAccount::class); // expect "ChooseAccountsHandler" because of state.
$handler = $this->mock(ChooseAccountsHandler::class);
$handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('setImportJob')->once();
$handler->shouldReceive('getNextView')->once()->andReturn('import.fake.view2'); $handler->shouldReceive('getNextView')->once()->andReturn('import.fake.view2');

View File

@@ -42,13 +42,13 @@ class SpectreRoutineTest extends TestCase
/** /**
* @covers \FireflyIII\Import\Routine\SpectreRoutine * @covers \FireflyIII\Import\Routine\SpectreRoutine
*/ */
public function testRunAuthenticate(): void public function testRunDoAuthenticate(): void
{ {
$job = new ImportJob; $job = new ImportJob;
$job->user_id = $this->user()->id; $job->user_id = $this->user()->id;
$job->key = 'SRA' . random_int(1, 1000); $job->key = 'SRA' . random_int(1, 1000);
$job->status = 'ready_to_run'; $job->status = 'ready_to_run';
$job->stage = 'authenticate'; $job->stage = 'do-authenticate';
$job->provider = 'spectre'; $job->provider = 'spectre';
$job->file_type = ''; $job->file_type = '';
$job->configuration = []; $job->configuration = [];
@@ -93,7 +93,7 @@ class SpectreRoutineTest extends TestCase
$repository->shouldReceive('setUser')->once(); $repository->shouldReceive('setUser')->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->once(); $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'need_job_config'])->once(); $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'need_job_config'])->once();
$repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'choose-account'])->once(); $repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'choose-accounts'])->once();
// mock calls for handler // mock calls for handler
$handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('setImportJob')->once();
@@ -210,7 +210,7 @@ class SpectreRoutineTest extends TestCase
$repository->shouldReceive('setUser')->once(); $repository->shouldReceive('setUser')->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->once(); $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'ready_to_run'])->once(); $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'ready_to_run'])->once();
$repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'authenticate'])->once(); $repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'do-authenticate'])->once();
// mock calls for handler // mock calls for handler
$handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('setImportJob')->once();