From ebf97f710ffde48d78aadd7f3a46bf390686508d Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 21 May 2018 09:40:19 +0200 Subject: [PATCH] Refactor code and fix tests. --- .../JobConfiguration/FileJobConfiguration.php | 6 +-- .../SpectreJobConfiguration.php | 37 +++++++++++-------- app/Import/Routine/SpectreRoutine.php | 6 +-- .../File/ConfigureMappingHandler.php | 2 +- .../File/ConfigureRolesHandler.php | 2 +- .../File/ConfigureUploadHandler.php | 2 +- ...ace.php => FileConfigurationInterface.php} | 4 +- .../File/NewFileJobHandler.php | 2 +- ...igHandler.php => AuthenticatedHandler.php} | 5 ++- ...eAccount.php => ChooseAccountsHandler.php} | 4 +- .../Spectre/ChooseLoginHandler.php | 4 +- ...teConfig.php => DoAuthenticateHandler.php} | 2 +- ...NewConfig.php => NewSpectreJobHandler.php} | 2 +- ....php => SpectreConfigurationInterface.php} | 2 +- .../SpectreJobConfigurationTest.php | 27 ++++++++------ .../Import/Routine/SpectreRoutineTest.php | 8 ++-- 16 files changed, 63 insertions(+), 52 deletions(-) rename app/Support/Import/JobConfiguration/File/{ConfigurationInterface.php => FileConfigurationInterface.php} (94%) rename app/Support/Import/JobConfiguration/Spectre/{AuthenticatedConfigHandler.php => AuthenticatedHandler.php} (95%) rename app/Support/Import/JobConfiguration/Spectre/{ChooseAccount.php => ChooseAccountsHandler.php} (98%) rename app/Support/Import/JobConfiguration/Spectre/{AuthenticateConfig.php => DoAuthenticateHandler.php} (98%) rename app/Support/Import/JobConfiguration/Spectre/{NewConfig.php => NewSpectreJobHandler.php} (96%) rename app/Support/Import/JobConfiguration/Spectre/{SpectreJobConfig.php => SpectreConfigurationInterface.php} (97%) diff --git a/app/Import/JobConfiguration/FileJobConfiguration.php b/app/Import/JobConfiguration/FileJobConfiguration.php index 48254d7115..cfc1d18966 100644 --- a/app/Import/JobConfiguration/FileJobConfiguration.php +++ b/app/Import/JobConfiguration/FileJobConfiguration.php @@ -28,7 +28,7 @@ namespace FireflyIII\Import\JobConfiguration; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\ImportJob; 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\ConfigureRolesHandler; use FireflyIII\Support\Import\JobConfiguration\File\ConfigureUploadHandler; @@ -128,10 +128,10 @@ class FileJobConfiguration implements JobConfigurationInterface /** * Get the configuration handler for this specific stage. * - * @return ConfigurationInterface + * @return FileConfigurationInterface * @throws FireflyException */ - private function getConfigurationObject(): ConfigurationInterface + private function getConfigurationObject(): FileConfigurationInterface { $class = 'DoNotExist'; switch ($this->importJob->stage) { diff --git a/app/Import/JobConfiguration/SpectreJobConfiguration.php b/app/Import/JobConfiguration/SpectreJobConfiguration.php index f75ba7a1b5..6d7c0f0c87 100644 --- a/app/Import/JobConfiguration/SpectreJobConfiguration.php +++ b/app/Import/JobConfiguration/SpectreJobConfiguration.php @@ -26,12 +26,16 @@ namespace FireflyIII\Import\JobConfiguration; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\ImportJob; 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\AuthenticatedHandler; +use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticateHandler; 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\DoAuthenticateHandler; 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 Log; @@ -40,7 +44,7 @@ use Log; */ class SpectreJobConfiguration implements JobConfigurationInterface { - /** @var SpectreJobConfig */ + /** @var SpectreConfigurationInterface */ private $handler; /** @var ImportJob */ private $importJob; @@ -104,42 +108,43 @@ class SpectreJobConfiguration implements JobConfigurationInterface } /** - * @return SpectreJobConfig + * @return SpectreConfigurationInterface * @throws FireflyException */ - private function getHandler(): SpectreJobConfig + private function getHandler(): SpectreConfigurationInterface { Log::debug(sprintf('Now in SpectreJobConfiguration::getHandler() with stage "%s"', $this->importJob->stage)); $handler = null; switch ($this->importJob->stage) { case 'new': - $handler = app(NewConfig::class); + /** @var NewSpectreJobHandler $handler */ + $handler = app(NewSpectreJobHandler::class); $handler->setImportJob($this->importJob); break; - case 'authenticate': - /** @var SpectreJobConfig $handler */ - $handler = app(AuthenticateConfig::class); + case 'do-authenticate': + /** @var DoAuthenticateHandler $handler */ + $handler = app(DoAuthenticateHandler::class); $handler->setImportJob($this->importJob); break; case 'choose-login': - /** @var SpectreJobConfig $handler */ + /** @var ChooseLoginHandler $handler */ $handler = app(ChooseLoginHandler::class); $handler->setImportJob($this->importJob); break; case 'authenticated': - /** @var AuthenticatedConfigHandler $handler */ - $handler = app(AuthenticatedConfigHandler::class); + /** @var AuthenticatedHandler $handler */ + $handler = app(AuthenticatedHandler::class); $handler->setImportJob($this->importJob); break; - case 'choose-account': - /** @var ChooseAccount $handler */ - $handler = app(ChooseAccount::class); + case 'choose-accounts': + /** @var ChooseAccountsHandler $handler */ + $handler = app(ChooseAccountsHandler::class); $handler->setImportJob($this->importJob); break; default: // @codeCoverageIgnoreStart throw new FireflyException(sprintf('Firefly III cannot create a configuration handler for stage "%s"', $this->importJob->stage)); - // @codeCoverageIgnoreEnd + // @codeCoverageIgnoreEnd } return $handler; diff --git a/app/Import/Routine/SpectreRoutine.php b/app/Import/Routine/SpectreRoutine.php index b31eacef07..1c51d8d697 100644 --- a/app/Import/Routine/SpectreRoutine.php +++ b/app/Import/Routine/SpectreRoutine.php @@ -67,7 +67,7 @@ class SpectreRoutine implements RoutineInterface // if count logins is zero, go to authenticate stage 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'); return; @@ -76,7 +76,7 @@ class SpectreRoutine implements RoutineInterface $this->repository->setStage($this->importJob, 'choose-login'); $this->repository->setStatus($this->importJob, 'need_job_config'); break; - case 'authenticate': + case 'do-authenticate': // set job to require config. $this->repository->setStatus($this->importJob, 'need_job_config'); @@ -90,7 +90,7 @@ class SpectreRoutine implements RoutineInterface $handler->run(); // 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'); break; case 'go-for-import': diff --git a/app/Support/Import/JobConfiguration/File/ConfigureMappingHandler.php b/app/Support/Import/JobConfiguration/File/ConfigureMappingHandler.php index 4b5c41c559..5e4c6df1f2 100644 --- a/app/Support/Import/JobConfiguration/File/ConfigureMappingHandler.php +++ b/app/Support/Import/JobConfiguration/File/ConfigureMappingHandler.php @@ -40,7 +40,7 @@ use Log; /** * Class ConfigureMappingHandler */ -class ConfigureMappingHandler implements ConfigurationInterface +class ConfigureMappingHandler implements FileConfigurationInterface { /** @var AttachmentHelperInterface */ private $attachments; diff --git a/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php b/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php index b9b33c297e..699917e711 100644 --- a/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php +++ b/app/Support/Import/JobConfiguration/File/ConfigureRolesHandler.php @@ -39,7 +39,7 @@ use Log; /** * Class ConfigureRolesHandler */ -class ConfigureRolesHandler implements ConfigurationInterface +class ConfigureRolesHandler implements FileConfigurationInterface { /** @var AttachmentHelperInterface */ private $attachments; diff --git a/app/Support/Import/JobConfiguration/File/ConfigureUploadHandler.php b/app/Support/Import/JobConfiguration/File/ConfigureUploadHandler.php index a5e674f73a..144d472f86 100644 --- a/app/Support/Import/JobConfiguration/File/ConfigureUploadHandler.php +++ b/app/Support/Import/JobConfiguration/File/ConfigureUploadHandler.php @@ -33,7 +33,7 @@ use Log; /** * Class ConfigureUploadHandler */ -class ConfigureUploadHandler implements ConfigurationInterface +class ConfigureUploadHandler implements FileConfigurationInterface { /** @var ImportJob */ private $importJob; diff --git a/app/Support/Import/JobConfiguration/File/ConfigurationInterface.php b/app/Support/Import/JobConfiguration/File/FileConfigurationInterface.php similarity index 94% rename from app/Support/Import/JobConfiguration/File/ConfigurationInterface.php rename to app/Support/Import/JobConfiguration/File/FileConfigurationInterface.php index 084d56ca25..3597ec4206 100644 --- a/app/Support/Import/JobConfiguration/File/ConfigurationInterface.php +++ b/app/Support/Import/JobConfiguration/File/FileConfigurationInterface.php @@ -26,9 +26,9 @@ use FireflyIII\Models\ImportJob; use Illuminate\Support\MessageBag; /** - * Class ConfigurationInterface. + * Class FileConfigurationInterface. */ -interface ConfigurationInterface +interface FileConfigurationInterface { /** * Store data associated with current stage. diff --git a/app/Support/Import/JobConfiguration/File/NewFileJobHandler.php b/app/Support/Import/JobConfiguration/File/NewFileJobHandler.php index 5023dc80ab..27ba527bc2 100644 --- a/app/Support/Import/JobConfiguration/File/NewFileJobHandler.php +++ b/app/Support/Import/JobConfiguration/File/NewFileJobHandler.php @@ -37,7 +37,7 @@ use Log; /** * Class NewFileJobHandler */ -class NewFileJobHandler implements ConfigurationInterface +class NewFileJobHandler implements FileConfigurationInterface { /** @var AttachmentHelperInterface */ private $attachments; diff --git a/app/Support/Import/JobConfiguration/Spectre/AuthenticatedConfigHandler.php b/app/Support/Import/JobConfiguration/Spectre/AuthenticatedHandler.php similarity index 95% rename from app/Support/Import/JobConfiguration/Spectre/AuthenticatedConfigHandler.php rename to app/Support/Import/JobConfiguration/Spectre/AuthenticatedHandler.php index 32b9da53a1..b2e4cf97ef 100644 --- a/app/Support/Import/JobConfiguration/Spectre/AuthenticatedConfigHandler.php +++ b/app/Support/Import/JobConfiguration/Spectre/AuthenticatedHandler.php @@ -28,7 +28,10 @@ use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface; use Illuminate\Support\MessageBag; use Log; -class AuthenticatedConfigHandler implements SpectreJobConfig +/** + * Class AuthenticatedHandler + */ +class AuthenticatedHandler implements SpectreConfigurationInterface { /** @var ImportJob */ private $importJob; diff --git a/app/Support/Import/JobConfiguration/Spectre/ChooseAccount.php b/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php similarity index 98% rename from app/Support/Import/JobConfiguration/Spectre/ChooseAccount.php rename to app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php index c098e4b5ce..586df3db55 100644 --- a/app/Support/Import/JobConfiguration/Spectre/ChooseAccount.php +++ b/app/Support/Import/JobConfiguration/Spectre/ChooseAccountsHandler.php @@ -38,10 +38,10 @@ use Illuminate\Support\MessageBag; use Log; /** - * Class ChooseAccount + * Class ChooseAccountsHandler * */ -class ChooseAccount implements SpectreJobConfig +class ChooseAccountsHandler implements SpectreConfigurationInterface { /** @var AccountRepositoryInterface */ diff --git a/app/Support/Import/JobConfiguration/Spectre/ChooseLoginHandler.php b/app/Support/Import/JobConfiguration/Spectre/ChooseLoginHandler.php index 9bf314f3d5..89b66f3c99 100644 --- a/app/Support/Import/JobConfiguration/Spectre/ChooseLoginHandler.php +++ b/app/Support/Import/JobConfiguration/Spectre/ChooseLoginHandler.php @@ -40,7 +40,7 @@ use Log; * Class ChooseLoginHandler * */ -class ChooseLoginHandler implements SpectreJobConfig +class ChooseLoginHandler implements SpectreConfigurationInterface { /** @var ImportJob */ private $importJob; @@ -93,7 +93,7 @@ class ChooseLoginHandler implements SpectreJobConfig $config['token'] = $token->toArray(); $this->repository->setConfiguration($this->importJob, $config); // 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; diff --git a/app/Support/Import/JobConfiguration/Spectre/AuthenticateConfig.php b/app/Support/Import/JobConfiguration/Spectre/DoAuthenticateHandler.php similarity index 98% rename from app/Support/Import/JobConfiguration/Spectre/AuthenticateConfig.php rename to app/Support/Import/JobConfiguration/Spectre/DoAuthenticateHandler.php index 81a8100e43..2e5395e02b 100644 --- a/app/Support/Import/JobConfiguration/Spectre/AuthenticateConfig.php +++ b/app/Support/Import/JobConfiguration/Spectre/DoAuthenticateHandler.php @@ -38,7 +38,7 @@ use Log; * Class AuthenticateConfig * */ -class AuthenticateConfig implements SpectreJobConfig +class DoAuthenticateHandler implements SpectreConfigurationInterface { /** @var ImportJob */ private $importJob; diff --git a/app/Support/Import/JobConfiguration/Spectre/NewConfig.php b/app/Support/Import/JobConfiguration/Spectre/NewSpectreJobHandler.php similarity index 96% rename from app/Support/Import/JobConfiguration/Spectre/NewConfig.php rename to app/Support/Import/JobConfiguration/Spectre/NewSpectreJobHandler.php index 54d5518bac..b42216373f 100644 --- a/app/Support/Import/JobConfiguration/Spectre/NewConfig.php +++ b/app/Support/Import/JobConfiguration/Spectre/NewSpectreJobHandler.php @@ -31,7 +31,7 @@ use Illuminate\Support\MessageBag; * Class NewConfig * */ -class NewConfig implements SpectreJobConfig +class NewSpectreJobHandler implements SpectreConfigurationInterface { /** diff --git a/app/Support/Import/JobConfiguration/Spectre/SpectreJobConfig.php b/app/Support/Import/JobConfiguration/Spectre/SpectreConfigurationInterface.php similarity index 97% rename from app/Support/Import/JobConfiguration/Spectre/SpectreJobConfig.php rename to app/Support/Import/JobConfiguration/Spectre/SpectreConfigurationInterface.php index b85de22df4..f3b30bc336 100644 --- a/app/Support/Import/JobConfiguration/Spectre/SpectreJobConfig.php +++ b/app/Support/Import/JobConfiguration/Spectre/SpectreConfigurationInterface.php @@ -31,7 +31,7 @@ use Illuminate\Support\MessageBag; * Interface SpectreJobConfig * */ -interface SpectreJobConfig +interface SpectreConfigurationInterface { /** * Return true when this stage is complete. diff --git a/tests/Unit/Import/JobConfiguration/SpectreJobConfigurationTest.php b/tests/Unit/Import/JobConfiguration/SpectreJobConfigurationTest.php index 55af66cfda..226a6e5dce 100644 --- a/tests/Unit/Import/JobConfiguration/SpectreJobConfigurationTest.php +++ b/tests/Unit/Import/JobConfiguration/SpectreJobConfigurationTest.php @@ -26,11 +26,11 @@ namespace Tests\Unit\Import\JobConfiguration; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Import\JobConfiguration\SpectreJobConfiguration; use FireflyIII\Models\ImportJob; -use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticateConfig; -use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticatedConfigHandler; -use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseAccount; +use FireflyIII\Support\Import\JobConfiguration\Spectre\AuthenticatedHandler; +use FireflyIII\Support\Import\JobConfiguration\Spectre\ChooseAccountsHandler; 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 Tests\TestCase; @@ -54,8 +54,8 @@ class SpectreJobConfigurationTest extends TestCase $job->configuration = []; $job->save(); - // expect "NewConfig" to be created because job is new. - $handler = $this->mock(NewConfig::class); + // expect "NewSpectreJobHandler" to be created because job is new. + $handler = $this->mock(NewSpectreJobHandler::class); $handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('configurationComplete')->once()->andReturn(true); @@ -77,7 +77,7 @@ class SpectreJobConfigurationTest extends TestCase $job->user_id = $this->user()->id; $job->key = 'spectre_jc_B' . random_int(1, 1000); $job->status = 'new'; - $job->stage = 'authenticate'; + $job->stage = 'do-authenticate'; $job->provider = 'spectre'; $job->file_type = ''; $job->configuration = []; @@ -86,8 +86,8 @@ class SpectreJobConfigurationTest extends TestCase $return = new MessageBag(); $return->add('some', 'return message'); - // expect "NewConfig" to be created because job is new. - $handler = $this->mock(AuthenticateConfig::class); + // expect "DoAuthenticateHandler" to be created because job is in "do-authenticate". + $handler = $this->mock(DoAuthenticateHandler::class); $handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('configureJob')->once()->withArgs([$configData])->andReturn($return); @@ -116,6 +116,7 @@ class SpectreJobConfigurationTest extends TestCase $job->save(); $data = ['ssome' => 'values']; + // Expect choose-login handler because of state. $handler = $this->mock(ChooseLoginHandler::class); $handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('getNextData')->once()->andReturn($data); @@ -144,7 +145,8 @@ class SpectreJobConfigurationTest extends TestCase $job->configuration = []; $job->save(); - $handler = $this->mock(AuthenticatedConfigHandler::class); + // expect "AuthenticatedHandler" because of state. + $handler = $this->mock(AuthenticatedHandler::class); $handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('getNextView')->once()->andReturn('import.fake.view'); @@ -166,13 +168,14 @@ class SpectreJobConfigurationTest extends TestCase $job->user_id = $this->user()->id; $job->key = 'spectre_jc_E' . random_int(1, 1000); $job->status = 'new'; - $job->stage = 'choose-account'; + $job->stage = 'choose-accounts'; $job->provider = 'spectre'; $job->file_type = ''; $job->configuration = []; $job->save(); - $handler = $this->mock(ChooseAccount::class); + // expect "ChooseAccountsHandler" because of state. + $handler = $this->mock(ChooseAccountsHandler::class); $handler->shouldReceive('setImportJob')->once(); $handler->shouldReceive('getNextView')->once()->andReturn('import.fake.view2'); diff --git a/tests/Unit/Import/Routine/SpectreRoutineTest.php b/tests/Unit/Import/Routine/SpectreRoutineTest.php index f12b1a8762..89d5947590 100644 --- a/tests/Unit/Import/Routine/SpectreRoutineTest.php +++ b/tests/Unit/Import/Routine/SpectreRoutineTest.php @@ -42,13 +42,13 @@ class SpectreRoutineTest extends TestCase /** * @covers \FireflyIII\Import\Routine\SpectreRoutine */ - public function testRunAuthenticate(): void + public function testRunDoAuthenticate(): void { $job = new ImportJob; $job->user_id = $this->user()->id; $job->key = 'SRA' . random_int(1, 1000); $job->status = 'ready_to_run'; - $job->stage = 'authenticate'; + $job->stage = 'do-authenticate'; $job->provider = 'spectre'; $job->file_type = ''; $job->configuration = []; @@ -93,7 +93,7 @@ class SpectreRoutineTest extends TestCase $repository->shouldReceive('setUser')->once(); $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->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 $handler->shouldReceive('setImportJob')->once(); @@ -210,7 +210,7 @@ class SpectreRoutineTest extends TestCase $repository->shouldReceive('setUser')->once(); $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->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 $handler->shouldReceive('setImportJob')->once();