From 1cb157b13669687521f5ebe7736088fbcf55b3b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Nov=C3=A1k?= Date: Mon, 21 Oct 2024 13:25:09 +0200 Subject: [PATCH] refactor: PST-2117 clearer naming of the enum for DWH connection type, move DwhConnectionType to the constructor of DbtService --- src/Component.php | 12 ++--- ...tionEnum.php => DwhConnectionTypeEnum.php} | 2 +- src/DwhProvider/DwhProviderInterface.php | 2 +- src/DwhProvider/LocalBigQueryProvider.php | 4 +- src/DwhProvider/LocalSnowflakeProvider.php | 4 +- src/DwhProvider/RemoteProvider.php | 4 +- src/Service/DbtService.php | 31 ++++++------ tests/phpunit/Service/DbtServiceTest.php | 48 ++++++++++--------- 8 files changed, 54 insertions(+), 53 deletions(-) rename src/DwhProvider/{DwhLocationEnum.php => DwhConnectionTypeEnum.php} (80%) diff --git a/src/Component.php b/src/Component.php index 37b66be..ca72a59 100644 --- a/src/Component.php +++ b/src/Component.php @@ -9,7 +9,7 @@ use DbtTransformation\Configuration\SyncAction\DbtDocsDefinition; use DbtTransformation\Configuration\SyncAction\DbtRunResultsDefinition; use DbtTransformation\Configuration\SyncAction\GitRepositoryDefinition; -use DbtTransformation\DwhProvider\DwhLocationEnum; +use DbtTransformation\DwhProvider\DwhConnectionTypeEnum; use DbtTransformation\DwhProvider\DwhProviderFactory; use DbtTransformation\Exception\ArtifactNotFoundException; use DbtTransformation\FileDumper\OutputManifest\DbtManifestParser; @@ -211,18 +211,18 @@ protected function logExecutedSqls(): void /** * @throws \Keboola\Component\UserException */ - protected function executeStep(string $step, DwhLocationEnum $dwhLocation): void + protected function executeStep(string $step, DwhConnectionTypeEnum $dwhLocation): void { $this->getLogger()->info(sprintf('Executing command "%s"', $step)); - $dbtService = new DbtService($this->projectPath); + $dbtService = new DbtService($this->projectPath, $dwhLocation); if ($step === DbtService::COMMAND_DEPS) { //some deps could be installed from git, so retry for "shallow file has changed" is needed /** @var string $output */ - $output = $this->gitRetryProxy->call(function () use ($dbtService, $step, $dwhLocation): string { - return $dbtService->runCommand($step, $dwhLocation); + $output = $this->gitRetryProxy->call(function () use ($dbtService, $step): string { + return $dbtService->runCommand($step); }); } else { - $output = $dbtService->runCommand($step, $dwhLocation); + $output = $dbtService->runCommand($step); } foreach (ParseDbtOutputHelper::getMessagesFromOutput($output) as $log) { diff --git a/src/DwhProvider/DwhLocationEnum.php b/src/DwhProvider/DwhConnectionTypeEnum.php similarity index 80% rename from src/DwhProvider/DwhLocationEnum.php rename to src/DwhProvider/DwhConnectionTypeEnum.php index a2bd190..a1ded71 100644 --- a/src/DwhProvider/DwhLocationEnum.php +++ b/src/DwhProvider/DwhConnectionTypeEnum.php @@ -4,7 +4,7 @@ namespace DbtTransformation\DwhProvider; -enum DwhLocationEnum +enum DwhConnectionTypeEnum { case LOCAL; case REMOTE; diff --git a/src/DwhProvider/DwhProviderInterface.php b/src/DwhProvider/DwhProviderInterface.php index 8c6da86..08416b0 100644 --- a/src/DwhProvider/DwhProviderInterface.php +++ b/src/DwhProvider/DwhProviderInterface.php @@ -8,5 +8,5 @@ interface DwhProviderInterface { public function createDbtYamlFiles(): void; - public function getDwhLocation(): DwhLocationEnum; + public function getDwhLocation(): DwhConnectionTypeEnum; } diff --git a/src/DwhProvider/LocalBigQueryProvider.php b/src/DwhProvider/LocalBigQueryProvider.php index 6100de1..024d2bb 100644 --- a/src/DwhProvider/LocalBigQueryProvider.php +++ b/src/DwhProvider/LocalBigQueryProvider.php @@ -127,8 +127,8 @@ public function __destruct() $this->temp->remove(); } - public function getDwhLocation(): DwhLocationEnum + public function getDwhLocation(): DwhConnectionTypeEnum { - return DwhLocationEnum::LOCAL; + return DwhConnectionTypeEnum::LOCAL; } } diff --git a/src/DwhProvider/LocalSnowflakeProvider.php b/src/DwhProvider/LocalSnowflakeProvider.php index 0b3dce6..4571350 100644 --- a/src/DwhProvider/LocalSnowflakeProvider.php +++ b/src/DwhProvider/LocalSnowflakeProvider.php @@ -137,8 +137,8 @@ public static function getDbtParams(): array ]; } - public function getDwhLocation(): DwhLocationEnum + public function getDwhLocation(): DwhConnectionTypeEnum { - return DwhLocationEnum::LOCAL; + return DwhConnectionTypeEnum::LOCAL; } } diff --git a/src/DwhProvider/RemoteProvider.php b/src/DwhProvider/RemoteProvider.php index e43b7d9..11f5d67 100644 --- a/src/DwhProvider/RemoteProvider.php +++ b/src/DwhProvider/RemoteProvider.php @@ -59,8 +59,8 @@ abstract public static function getRequiredConnectionParams(): array; */ abstract public static function getDbtParams(): array; - public function getDwhLocation(): DwhLocationEnum + public function getDwhLocation(): DwhConnectionTypeEnum { - return DwhLocationEnum::REMOTE; + return DwhConnectionTypeEnum::REMOTE; } } diff --git a/src/Service/DbtService.php b/src/Service/DbtService.php index 6a31020..319340f 100644 --- a/src/Service/DbtService.php +++ b/src/Service/DbtService.php @@ -4,7 +4,7 @@ namespace DbtTransformation\Service; -use DbtTransformation\DwhProvider\DwhLocationEnum; +use DbtTransformation\DwhProvider\DwhConnectionTypeEnum; use DbtTransformation\Helper\ParseDbtOutputHelper; use Keboola\Component\UserException; use Symfony\Component\Console\Input\StringInput; @@ -31,20 +31,19 @@ class DbtService public const COMMAND_RUN = 'dbt run'; public const COMMAND_DEPS = 'dbt deps'; - private string $projectPath; - - public function __construct(string $projectPath) - { - $this->projectPath = $projectPath; + public function __construct( + private readonly string $projectPath, + private readonly DwhConnectionTypeEnum $dwhConnectionType, + ) { } /** * @throws \Keboola\Component\UserException */ - public function runCommand(string $command, DwhLocationEnum $dwhLocation, string $target = 'kbc_prod'): string + public function runCommand(string $command, string $target = 'kbc_prod'): string { try { - $command = $this->prepareCommand($command, $dwhLocation, $target); + $command = $this->prepareCommand($command, $target); $process = new Process($command, $this->projectPath, getenv(), null, null); $process->mustRun(); return $process->getOutput(); @@ -65,14 +64,14 @@ public function runCommand(string $command, DwhLocationEnum $dwhLocation, string * @return array * @throws \Keboola\Component\UserException */ - protected function prepareCommand(string $command, DwhLocationEnum $dwhLocation, string $target): array + protected function prepareCommand(string $command, string $target): array { return [ 'dbt', '--log-format', 'json', '--no-use-colors', - ...$this->getCommandWithoutDbt($command, $dwhLocation), + ...$this->getCommandWithoutDbt($command), '-t', $target, '--profiles-dir', @@ -84,12 +83,12 @@ protected function prepareCommand(string $command, DwhLocationEnum $dwhLocation, * @return array * @throws \Keboola\Component\UserException */ - protected function getCommandWithoutDbt(string $commandString, DwhLocationEnum $dwhLocation): array + protected function getCommandWithoutDbt(string $commandString): array { $stringInput = new StringInput($commandString); $command = $stringInput->getRawTokens(true); foreach ($command as $commandPart) { - $foundOption = $this->findDisallowedOption($commandPart, $dwhLocation); + $foundOption = $this->findDisallowedOption($commandPart); if ($foundOption !== null) { throw new UserException("You cannot override option {$foundOption} in your dbt command. " . 'Please remove it.'); @@ -99,11 +98,11 @@ protected function getCommandWithoutDbt(string $commandString, DwhLocationEnum $ return $command; } - private function findDisallowedOption(string $commandPart, DwhLocationEnum $dwhLocation): ?string + private function findDisallowedOption(string $commandPart): ?string { - $disallowedOptions = match ($dwhLocation) { - DwhLocationEnum::LOCAL => self::DISALLOWED_OPTIONS_LOCAL_DWH, - DwhLocationEnum::REMOTE => self::DISALLOWED_OPTIONS_REMOTE_DWH, + $disallowedOptions = match ($this->dwhConnectionType) { + DwhConnectionTypeEnum::LOCAL => self::DISALLOWED_OPTIONS_LOCAL_DWH, + DwhConnectionTypeEnum::REMOTE => self::DISALLOWED_OPTIONS_REMOTE_DWH, }; foreach ($disallowedOptions as $option) { diff --git a/tests/phpunit/Service/DbtServiceTest.php b/tests/phpunit/Service/DbtServiceTest.php index bc5e7ce..e65acf6 100644 --- a/tests/phpunit/Service/DbtServiceTest.php +++ b/tests/phpunit/Service/DbtServiceTest.php @@ -7,7 +7,7 @@ use ColinODell\PsrTestLogger\TestLogger; use DbtTransformation\Config; use DbtTransformation\Configuration\ConfigDefinition; -use DbtTransformation\DwhProvider\DwhLocationEnum; +use DbtTransformation\DwhProvider\DwhConnectionTypeEnum; use DbtTransformation\DwhProvider\DwhProviderFactory; use DbtTransformation\Helper\DbtCompileHelper; use DbtTransformation\Helper\ParseDbtOutputHelper; @@ -32,7 +32,7 @@ public function setUp(): void $this->gitRepositoryService = new GitRepositoryService($this->dataDir); $logger = new TestLogger(); - $this->dbtService = new DbtService($this->getProjectPath()); + $this->dbtService = new DbtService($this->getProjectPath(), DwhConnectionTypeEnum::LOCAL); $this->dwhProviderFactory = new DwhProviderFactory($logger); } @@ -71,10 +71,11 @@ private function getConfig(string $backend, string $executeStep): Config /** * @dataProvider validDbtOptionsProvider */ - public function testValidDbtOptions(string $command, DwhLocationEnum $dwhLocation): void + public function testValidDbtOptions(string $command, DwhConnectionTypeEnum $dwhConnectionType): void { + $dbtService = new DbtService($this->getProjectPath(), $dwhConnectionType); try { - $this->dbtService->runCommand($command, $dwhLocation); + $dbtService->runCommand($command); } catch (UserException $e) { $this->fail('Command should not fail with valid options: ' . $e->getMessage()); } catch (Throwable) { @@ -88,61 +89,62 @@ public function testValidDbtOptions(string $command, DwhLocationEnum $dwhLocatio */ public function testInvalidDbtOptions( string $command, - DwhLocationEnum $dwhLocation, + DwhConnectionTypeEnum $dwhConnectionType, string $expectedExceptionMessage, ): void { $this->expectException(UserException::class); $this->expectExceptionMessage($expectedExceptionMessage); - $this->dbtService->runCommand($command, $dwhLocation); + $dbtService = new DbtService($this->getProjectPath(), $dwhConnectionType); + $dbtService->runCommand($command); } /** - * @return Generator + * @return Generator */ public function validDbtOptionsProvider(): Generator { - yield 'valid option resource-type local dwh' => ['dbt ls --resource-type model', DwhLocationEnum::LOCAL]; - yield 'valid option models local dwh' => ['dbt run --models my_model', DwhLocationEnum::LOCAL]; - yield 'valid option resource-type remote dwh' => ['dbt ls --resource-type model', DwhLocationEnum::REMOTE]; - yield 'valid option models remote dwh' => ['dbt run --models my_model', DwhLocationEnum::REMOTE]; - yield 'valid option target remote dwh' => ['dbt run --target dev', DwhLocationEnum::REMOTE]; - yield 'valid option shorthand target remote dwh' => ['dbt run -t dev', DwhLocationEnum::REMOTE]; + yield 'valid option resource-type local' => ['dbt ls --resource-type model', DwhConnectionTypeEnum::LOCAL]; + yield 'valid option models local' => ['dbt run --models my_model', DwhConnectionTypeEnum::LOCAL]; + yield 'valid option resource-type remote' => ['dbt ls --resource-type model', DwhConnectionTypeEnum::REMOTE]; + yield 'valid option models remote' => ['dbt run --models my_model', DwhConnectionTypeEnum::REMOTE]; + yield 'valid option target remote' => ['dbt run --target dev', DwhConnectionTypeEnum::REMOTE]; + yield 'valid option shorthand target remote' => ['dbt run -t dev', DwhConnectionTypeEnum::REMOTE]; } /** - * @return Generator + * @return Generator */ public function invalidDbtOptionsProvider(): Generator { yield 'invalid option profiles-dir local dwh' => [ 'dbt run --profiles-dir dir', - DwhLocationEnum::LOCAL, + DwhConnectionTypeEnum::LOCAL, 'You cannot override option --profiles-dir in your dbt command. Please remove it.', ]; yield 'invalid option log-format local dwh' => [ 'dbt run --log-format json', - DwhLocationEnum::LOCAL, + DwhConnectionTypeEnum::LOCAL, 'You cannot override option --log-format in your dbt command. Please remove it.', ]; yield 'invalid option target local dwh' => [ 'dbt run --target dev', - DwhLocationEnum::LOCAL, + DwhConnectionTypeEnum::LOCAL, 'You cannot override option --target in your dbt command. Please remove it.', ]; yield 'invalid option shorthand target local dwh' => [ 'dbt run -t dev', - DwhLocationEnum::LOCAL, + DwhConnectionTypeEnum::LOCAL, 'You cannot override option -t in your dbt command. Please remove it.', ]; yield 'invalid option profiles-dir remote dwh' => [ 'dbt run --profiles-dir dir', - DwhLocationEnum::REMOTE, + DwhConnectionTypeEnum::REMOTE, 'You cannot override option --profiles-dir in your dbt command. Please remove it.', ]; yield 'invalid option log-format remote dwh' => [ 'dbt run --log-format json', - DwhLocationEnum::REMOTE, + DwhConnectionTypeEnum::REMOTE, 'You cannot override option --log-format in your dbt command. Please remove it.', ]; } @@ -161,7 +163,7 @@ public function testDbtDebug(string $backend): void $provider = $this->dwhProviderFactory->getProvider($config, $this->getProjectPath()); $provider->createDbtYamlFiles(); - $output = $this->dbtService->runCommand(DbtService::COMMAND_DEBUG, DwhLocationEnum::LOCAL); + $output = $this->dbtService->runCommand(DbtService::COMMAND_DEBUG); self::assertStringContainsString('profiles.yml file', $output); self::assertStringContainsString('dbt_project.yml file', $output); @@ -182,7 +184,7 @@ public function testDbtCompile(string $backend, string $firstSql, string $second $this->gitRepositoryService->clone('https://github.com/keboola/dbt-test-project-public.git'); $provider = $this->dwhProviderFactory->getProvider($config, $this->getProjectPath()); $provider->createDbtYamlFiles(); - $output = $this->dbtService->runCommand(DbtService::COMMAND_COMPILE, DwhLocationEnum::LOCAL); + $output = $this->dbtService->runCommand(DbtService::COMMAND_COMPILE); $parsedOutput = iterator_to_array(ParseDbtOutputHelper::getMessagesFromOutput($output)); $stringsToFind = [ @@ -235,7 +237,7 @@ public function testDbtRunWithVars(): void $provider = $this->dwhProviderFactory->getProvider($config, $this->getProjectPath()); $provider->createDbtYamlFiles(); - $output = $this->dbtService->runCommand($command, DwhLocationEnum::LOCAL); + $output = $this->dbtService->runCommand($command); self::assertStringContainsString('"vars": "{\'var1\': \'value1\', \'var2\': \'value2\'}', $output); }