From 5b9b7eff2494ef16fd6864c3b5b3aa59695b32dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Fejfar?= Date: Wed, 13 Mar 2019 14:56:30 +0100 Subject: [PATCH 1/5] Remove any loading before config is loaded --- src/BaseComponent.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/BaseComponent.php b/src/BaseComponent.php index 771d0f17..57dd7aa4 100644 --- a/src/BaseComponent.php +++ b/src/BaseComponent.php @@ -85,7 +85,6 @@ protected function loadConfig(): void } catch (InvalidConfigurationException $e) { throw new UserException($e->getMessage(), 0, $e); } - $this->logger->debug('Config loaded'); } protected function loadInputState(): void From 6da164205388e8017acb69eefa6e264f6e722821 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Fejfar?= Date: Wed, 13 Mar 2019 16:29:38 +0100 Subject: [PATCH 2/5] Add test for run action --- tests/BaseComponentTest.php | 24 +++++++++++++++++++ .../run-action/config.json | 3 +++ 2 files changed, 27 insertions(+) create mode 100644 tests/fixtures/base-component-data-dir/run-action/config.json diff --git a/tests/BaseComponentTest.php b/tests/BaseComponentTest.php index c4694bf6..eb541a60 100644 --- a/tests/BaseComponentTest.php +++ b/tests/BaseComponentTest.php @@ -6,6 +6,7 @@ use Keboola\Component\BaseComponent; use Keboola\Component\Logger; +use Monolog\Handler\TestHandler; use PHPUnit\Framework\TestCase; use Symfony\Component\Serializer\Exception\NotEncodableValueException; @@ -42,6 +43,29 @@ public function testLoadInputStateFile(): void $this->assertEquals('value', $inputStateFile['dict']['key']); } + public function testRunAction(): void + { + $logger = new Logger(); + $handler = new TestHandler(); + $logger->setHandlers([$handler]); + putenv(sprintf( + 'KBC_DATADIR=%s', + __DIR__ . '/fixtures/base-component-data-dir/run-action' + )); + $baseComponent = new class ($logger) extends BaseComponent + { + public function run(): void + { + echo 'Shitty output'; + $this->getLogger()->alert('Log message from run'); + } + }; + $this->expectOutputString('Shitty output'); + $baseComponent->run(); + + $this->assertTrue($handler->hasAlert('Log message from run')); + } + public function testLoadInputStateFileEmptyThrowsException(): void { putenv(sprintf( diff --git a/tests/fixtures/base-component-data-dir/run-action/config.json b/tests/fixtures/base-component-data-dir/run-action/config.json new file mode 100644 index 00000000..d88d9944 --- /dev/null +++ b/tests/fixtures/base-component-data-dir/run-action/config.json @@ -0,0 +1,3 @@ +{ + "action": "run" +} From a4cdd46c1d7958271e1a3c07f3ee24e5659402fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Fejfar?= Date: Wed, 13 Mar 2019 17:58:16 +0100 Subject: [PATCH 3/5] Sync actions support --- README.md | 31 +++++++- example/Component.php | 12 ++- example/run.php | 2 +- src/BaseComponent.php | 60 +++++++++++++++ src/Exception/BaseComponentException.php | 23 ++++++ src/Logger.php | 68 ++++++----------- src/Logger/AsyncActionLogging.php | 10 +++ src/Logger/SyncActionLogging.php | 13 ++++ tests/BaseComponentTest.php | 76 ++++++++++++++++++- .../sync-action/config.json | 3 + 10 files changed, 246 insertions(+), 52 deletions(-) create mode 100644 src/Exception/BaseComponentException.php create mode 100644 src/Logger/AsyncActionLogging.php create mode 100644 src/Logger/SyncActionLogging.php create mode 100644 tests/fixtures/base-component-data-dir/sync-action/config.json diff --git a/README.md b/README.md index 951b59c2..e63d55a4 100644 --- a/README.md +++ b/README.md @@ -16,9 +16,10 @@ composer require keboola/php-component Create a subclass of `BaseComponent`. ```php +getConfig()->getParameters(); @@ -41,7 +42,7 @@ class Component extends \Keboola\Component\BaseComponent (new OutFileManifestOptions()) ->setTags(['tag1', 'tag2']) ); - + // write manifest for output table $this->getManifestManager()->writeTableManifest( 'data.csv', @@ -50,8 +51,17 @@ class Component extends \Keboola\Component\BaseComponent ->setDestination('out.report') ); } -} + protected function customSyncAction(): array + { + return ['result' => 'success', 'data' => ['joe', 'marry']]; + } + + protected function getSyncActions(): array + { + return ['custom' => 'customSyncAction']; + } +} ``` Use this `src/run.php` template. @@ -68,7 +78,7 @@ require __DIR__ . '/../vendor/autoload.php'; $logger = new Logger(); try { $app = new MyComponent\Component($logger); - $app->run(); + $app->execute(); exit(0); } catch (\Keboola\Component\UserException $e) { $logger->error($e->getMessage()); @@ -88,6 +98,15 @@ try { } ``` +## Sync actions support + +[Sync actions](https://developers.keboola.com/extend/common-interface/actions/) can be called directly via API. API will block and wait for the result. The correct action is selected based on the `action` key of config. `BaseComponent` class handles the selection automatically. Also it handles serialization and output of the action result - sync actions must output valid JSON. + +To implement a sync action +* add a method in your `Component` class. The naming is entirely up to you. +* override the `Component::getSyncActions()` method to return array containing your sync actions names as keys and corresponding method names from the `Component` class as values. +* return value of the method will be serialized to json + ## Customizing config ### Custom getters in config @@ -158,6 +177,10 @@ class MyComponent extends \Keboola\Component\BaseComponent If any constraint of config definition is not met a `UserException` is thrown. That means you don't need to handle the messages yourself. +## Migration from version 6 to version 7 + +The default entrypoint of component (in `index.php`) changed from `BaseComponent::run()` to `BaseComponent::execute()`. While running the component via `run` method is still supported, you need to use `execute()` if you want to take advantage of sync action support. + ## More reading For more information, please refer to the [generated docs](https://keboola.github.io/php-component/master/classes.html). See [development guide](https://developers.keboola.com/extend/component/tutorial/) for help with KBC integration. diff --git a/example/Component.php b/example/Component.php index 9f7ff458..f9acf1b9 100644 --- a/example/Component.php +++ b/example/Component.php @@ -9,7 +9,7 @@ class Component extends \Keboola\Component\BaseComponent { - public function run(): void + protected function run(): void { // get parameters $parameters = $this->getConfig()->getParameters(); @@ -41,4 +41,14 @@ public function run(): void ->setDestination('out.report') ); } + + protected function customSyncAction(): array + { + return ['result' => 'success', 'data' => ['joe', 'marry']]; + } + + protected function getSyncActions(): array + { + return ['custom' => 'customSyncAction']; + } } diff --git a/example/run.php b/example/run.php index 4421efe2..312542ee 100644 --- a/example/run.php +++ b/example/run.php @@ -9,7 +9,7 @@ $logger = new Logger(); try { $app = new MyComponent\Component($logger); - $app->run(); + $app->execute(); exit(0); } catch (\Keboola\Component\UserException $e) { $logger->error($e->getMessage()); diff --git a/src/BaseComponent.php b/src/BaseComponent.php index 57dd7aa4..95dee063 100644 --- a/src/BaseComponent.php +++ b/src/BaseComponent.php @@ -5,9 +5,14 @@ namespace Keboola\Component; use ErrorException; +use Exception; use Keboola\Component\Config\BaseConfig; use Keboola\Component\Config\BaseConfigDefinition; +use Keboola\Component\Exception\BaseComponentException; +use Keboola\Component\Logger\AsyncActionLogging; +use Keboola\Component\Logger\SyncActionLogging; use Keboola\Component\Manifest\ManifestManager; +use Monolog\Handler\NullHandler; use Psr\Log\LoggerInterface; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Filesystem\Exception\FileNotFoundException; @@ -43,6 +48,7 @@ public function __construct(LoggerInterface $logger) $this->setDataDir($dataDir); $this->loadConfig(); + $this->initializeSyncActions(); $this->loadInputState(); $this->loadManifestManager(); @@ -87,6 +93,27 @@ protected function loadConfig(): void } } + protected function initializeSyncActions(): void + { + if (array_key_exists('run', $this->getSyncActions())) { + throw BaseComponentException::runCannotBeSyncAction(); + } + foreach ($this->getSyncActions() as $method) { + if (!method_exists($this, $method)) { + throw BaseComponentException::invalidSyncAction($method); + } + } + if ($this->isSyncAction()) { + if ($this->logger instanceof SyncActionLogging) { + $this->logger->setupSyncActionLogging(); + } + } else { + if ($this->logger instanceof AsyncActionLogging) { + $this->logger->setupAsyncActionLogging(); + } + } + } + protected function loadInputState(): void { try { @@ -159,6 +186,23 @@ public function getInputState(): array return $this->inputState; } + public function execute(): void + { + if (!$this->isSyncAction()) { + $this->run(); + return; + } + + $action = $this->getConfig()->getAction(); + $syncActions = $this->getSyncActions(); + if (array_key_exists($action, $syncActions)) { + $method = $syncActions[$action]; + echo JsonHelper::encode($this->$method()); + } else { + throw BaseComponentException::invalidSyncAction($action); + } + } + /** * This is the main method for your code to run in. You have the `Config` * and `ManifestManager` ready as well as environment set up. @@ -185,4 +229,20 @@ protected function loadManifestManager(): void { $this->manifestManager = new ManifestManager($this->dataDir); } + + public function isSyncAction(): bool + { + return $this->getConfig()->getAction() !== 'run'; + } + + /** + * Whitelist method names that can be used as synchronous actions. This is a + * safeguard against executing any method of the component. + * + * Format: 'action' => 'method name' (e.g. 'getTables' => 'handleTableSyncAction') + */ + protected function getSyncActions(): array + { + return []; + } } diff --git a/src/Exception/BaseComponentException.php b/src/Exception/BaseComponentException.php new file mode 100644 index 00000000..1c9f160a --- /dev/null +++ b/src/Exception/BaseComponentException.php @@ -0,0 +1,23 @@ +setHandlers($handlers); - } - public static function getDefaultErrorHandler(): StreamHandler { $errorHandler = new StreamHandler('php://stderr'); @@ -62,6 +19,11 @@ public static function getDefaultErrorHandler(): StreamHandler return $errorHandler; } + public function __construct() + { + parent::__construct('php-component'); + } + public static function getDefaultLogHandler(): StreamHandler { $logHandler = new StreamHandler('php://stdout'); @@ -79,4 +41,22 @@ public static function getDefaultCriticalHandler(): StreamHandler $handler->setFormatter(new LineFormatter("[%datetime%] %level_name%: %message% %context% %extra%\n")); return $handler; } + + public function setupSyncActionLogging(): void + { + $this->setHandlers([]); + } + + public function setupAsyncActionLogging(): void + { + $criticalHandler = self::getDefaultCriticalHandler(); + $errorHandler = self::getDefaultErrorHandler(); + $logHandler = self::getDefaultLogHandler(); + + $this->setHandlers([ + $criticalHandler, + $errorHandler, + $logHandler, + ]); + } } diff --git a/src/Logger/AsyncActionLogging.php b/src/Logger/AsyncActionLogging.php new file mode 100644 index 00000000..706c7af2 --- /dev/null +++ b/src/Logger/AsyncActionLogging.php @@ -0,0 +1,10 @@ +assertEquals('value', $inputStateFile['dict']['key']); } + public function testSyncActions(): void + { + $logger = $this->getLogger(); + putenv(sprintf( + 'KBC_DATADIR=%s', + __DIR__ . '/fixtures/base-component-data-dir/sync-action' + )); + $baseComponent = new class ($logger) extends BaseComponent + { + public function run(): void + { + throw new Exception('Not implemented'); + } + + protected function getSyncActions(): array + { + return ['sync' => 'handleSync']; + } + + public function handleSync(): array + { + return ['status' => 'success', 'count' => 20]; + } + }; + $expectedJson = <<expectOutputString($expectedJson); + $baseComponent->execute(); + } + public function testRunAction(): void { - $logger = new Logger(); + $logger = $this->getLogger(); $handler = new TestHandler(); $logger->setHandlers([$handler]); putenv(sprintf( @@ -61,7 +98,7 @@ public function run(): void } }; $this->expectOutputString('Shitty output'); - $baseComponent->run(); + $baseComponent->execute(); $this->assertTrue($handler->hasAlert('Log message from run')); } @@ -92,4 +129,39 @@ public function testLoadInputStateFileUndefined(): void $this->assertSame([], $baseComponent->getInputState()); } + + public function testCannotSetUpInvalidSyncActions(): void + { + $logger = new Logger(); + $this->expectException(BaseComponentException::class); + $this->expectExceptionMessage('Unknown sync action "nonexistentMethod", method does not exist in class'); + new class($logger) extends BaseComponent + { + protected function getSyncActions(): array + { + return ['nonexistentMethod']; + } + }; + } + + public function testRunCannotBeSyncAction(): void + { + $logger = new Logger(); + $this->expectException(BaseComponentException::class); + $this->expectExceptionMessage('"run" cannot be a sync action'); + new class($logger) extends BaseComponent + { + protected function getSyncActions(): array + { + return ['run' => 'run']; + } + }; + } + + private function getLogger(): \Monolog\Logger + { + $logger = new \Monolog\Logger('app'); + $logger->setHandlers([new NullHandler()]); + return $logger; + } } diff --git a/tests/fixtures/base-component-data-dir/sync-action/config.json b/tests/fixtures/base-component-data-dir/sync-action/config.json new file mode 100644 index 00000000..de6616ba --- /dev/null +++ b/tests/fixtures/base-component-data-dir/sync-action/config.json @@ -0,0 +1,3 @@ +{ + "action": "sync" +} From 3b3881d19554fa7018e1c8a4582452a3ebd6daff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Fejfar?= Date: Fri, 15 Mar 2019 18:43:25 +0100 Subject: [PATCH 4/5] Ensure run is not public to make sure BC break is noticable --- src/BaseComponent.php | 15 ++++++++++++++- src/Exception/BaseComponentException.php | 5 +++++ tests/BaseComponentTest.php | 21 +++++++++++++++++++-- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/BaseComponent.php b/src/BaseComponent.php index 95dee063..c725de93 100644 --- a/src/BaseComponent.php +++ b/src/BaseComponent.php @@ -14,6 +14,8 @@ use Keboola\Component\Manifest\ManifestManager; use Monolog\Handler\NullHandler; use Psr\Log\LoggerInterface; +use Reflection; +use ReflectionClass; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Filesystem\Exception\FileNotFoundException; use function error_reporting; @@ -53,6 +55,8 @@ public function __construct(LoggerInterface $logger) $this->loadManifestManager(); + $this->checkRunMethodNotPublic(); + $this->logger->debug('Component initialization completed'); } @@ -123,6 +127,15 @@ protected function loadInputState(): void } } + private function checkRunMethodNotPublic(): void + { + $reflection = new ReflectionClass(static::class); + $method = $reflection->getMethod('run'); + if ($method->isPublic()) { + throw BaseComponentException::runMethodCannotBePublic(); + } + } + protected function writeOutputStateToFile(array $state): void { JsonHelper::writeFile( @@ -207,7 +220,7 @@ public function execute(): void * This is the main method for your code to run in. You have the `Config` * and `ManifestManager` ready as well as environment set up. */ - public function run(): void + protected function run(): void { // to be implemented in subclass } diff --git a/src/Exception/BaseComponentException.php b/src/Exception/BaseComponentException.php index 1c9f160a..3bc896f0 100644 --- a/src/Exception/BaseComponentException.php +++ b/src/Exception/BaseComponentException.php @@ -20,4 +20,9 @@ public static function runCannotBeSyncAction(): self { return new self('"run" cannot be a sync action'); } + + public static function runMethodCannotBePublic(): self + { + return new self('Method "run" cannot be public since version 7'); + } } diff --git a/tests/BaseComponentTest.php b/tests/BaseComponentTest.php index 35ed7cf9..c47a96b0 100644 --- a/tests/BaseComponentTest.php +++ b/tests/BaseComponentTest.php @@ -55,7 +55,7 @@ public function testSyncActions(): void )); $baseComponent = new class ($logger) extends BaseComponent { - public function run(): void + protected function run(): void { throw new Exception('Not implemented'); } @@ -91,7 +91,7 @@ public function testRunAction(): void )); $baseComponent = new class ($logger) extends BaseComponent { - public function run(): void + protected function run(): void { echo 'Shitty output'; $this->getLogger()->alert('Log message from run'); @@ -158,8 +158,25 @@ protected function getSyncActions(): array }; } + public function testRunActionCannotBePublic(): void + { + $this->expectException(BaseComponentException::class); + $this->expectExceptionMessage('Method "run" cannot be public since version 7'); + new class($this->getLogger()) extends BaseComponent + { + public function run(): void + { + return; + } + }; + } + private function getLogger(): \Monolog\Logger { + putenv(sprintf( + 'KBC_DATADIR=%s', + __DIR__ . '/fixtures/base-component-data-dir/run-action' + )); $logger = new \Monolog\Logger('app'); $logger->setHandlers([new NullHandler()]); return $logger; From 39ee01805679cff2978f2a6af1ab903189552e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Fejfar?= Date: Mon, 18 Mar 2019 09:09:46 +0100 Subject: [PATCH 5/5] Remove stdout output from tests --- tests/BaseComponentTest.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/BaseComponentTest.php b/tests/BaseComponentTest.php index c47a96b0..da0cc3d3 100644 --- a/tests/BaseComponentTest.php +++ b/tests/BaseComponentTest.php @@ -22,8 +22,7 @@ public function testLoadInputStateFile(): void __DIR__ . '/fixtures/base-component-data-dir/state-file' )); - $logger = new Logger(); - $baseComponent = new BaseComponent($logger); + $baseComponent = new BaseComponent($this->getLogger()); $inputStateFile = $baseComponent->getInputState(); $this->assertCount(4, $inputStateFile); @@ -124,8 +123,7 @@ public function testLoadInputStateFileUndefined(): void __DIR__ . '/fixtures/base-component-data-dir/undefined-state-file' )); - $logger = new Logger(); - $baseComponent = new BaseComponent($logger); + $baseComponent = new BaseComponent($this->getLogger()); $this->assertSame([], $baseComponent->getInputState()); } @@ -146,6 +144,10 @@ protected function getSyncActions(): array public function testRunCannotBeSyncAction(): void { + putenv(sprintf( + 'KBC_DATADIR=%s', + __DIR__ . '/fixtures/base-component-data-dir/run-action' + )); $logger = new Logger(); $this->expectException(BaseComponentException::class); $this->expectExceptionMessage('"run" cannot be a sync action'); @@ -160,6 +162,10 @@ protected function getSyncActions(): array public function testRunActionCannotBePublic(): void { + putenv(sprintf( + 'KBC_DATADIR=%s', + __DIR__ . '/fixtures/base-component-data-dir/run-action' + )); $this->expectException(BaseComponentException::class); $this->expectExceptionMessage('Method "run" cannot be public since version 7'); new class($this->getLogger()) extends BaseComponent @@ -173,10 +179,6 @@ public function run(): void private function getLogger(): \Monolog\Logger { - putenv(sprintf( - 'KBC_DATADIR=%s', - __DIR__ . '/fixtures/base-component-data-dir/run-action' - )); $logger = new \Monolog\Logger('app'); $logger->setHandlers([new NullHandler()]); return $logger;