diff --git a/CHANGELOG.md b/CHANGELOG.md index be4b667..6bd6cdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed * The `maxOutputs()` method is now also available and working on `Group` steps. +* Improved warning messages for step validations that are happening before running a crawler. ## [1.10.0] - 2024-08-05 ### Added diff --git a/src/Steps/BaseStep.php b/src/Steps/BaseStep.php index cd8c1b9..1487540 100644 --- a/src/Steps/BaseStep.php +++ b/src/Steps/BaseStep.php @@ -307,13 +307,11 @@ public function validateBeforeRun(BaseStep|array $previousStepOrInitialInputs): 'instead of keep()', ); } elseif ($outputType === StepOutputType::Mixed) { - $stepClassName = get_class($this); - $this->logger?->warning( - 'The ' . $stepClassName . ' step potentially yields scalar value outputs (= single ' . - 'string/int/bool/float with no key like in an associative array or object). If it does (yield a ' . - 'scalar value output), it can not keep that output value, because it needs a key for that. ' . - 'To avoid this, define a key for scalar outputs by using the keepAs() method.', + $this->getPreValidationRunMessageStartWithStepClassName() . ' potentially yields scalar value ' . + 'outputs (= single string/int/bool/float with no key like in an associative array or object). ' . + 'If it does (yield a scalar value output), it can not keep that output value, because it needs ' . + 'a key for that. To avoid this, define a key for scalar outputs by using the keepAs() method.', ); } } @@ -332,13 +330,12 @@ public function validateBeforeRun(BaseStep|array $previousStepOrInitialInputs): 'array or object). Please define a key for the input data to keep, by using keepAs() instead.', ); } elseif ($previousStepOutputType === StepOutputType::Mixed) { - $stepClassName = get_class($this); - $this->logger?->warning( - 'The step before the ' . $stepClassName . ' step, potentially yields scalar value outputs ' . - '(= single string/int/bool/float with no key like in an associative array or object). If it does ' . - '(yield a scalar value output) the next step can not keep it by using keepFromInput(). To avoid ' . - 'this, define a key for scalar inputs by using the keepInputAs() method.', + $this->getPreValidationRunMessageStartWithStepClassName($previousStepOrInitialInputs) . + ' potentially yields scalar value outputs (= single string/int/bool/float with no key like in ' . + 'an associative array or object). If it does (yield a scalar value output) the next step can not ' . + 'keep it by using keepFromInput(). To avoid this, define a key for scalar inputs by using the ' . + 'keepInputAs() method.', ); } } @@ -440,6 +437,51 @@ protected function validateFirstStepBeforeRun(array $initialInputs): void } } + protected function getPreValidationRunMessageStartWithStepClassName(?BaseStep $step = null): string + { + $stepClassName = $this->getStepClassName($step); + + if ($stepClassName) { + return 'The ' . $stepClassName . ' step'; + } else { + $stepClassName = $this->getParentStepClassName($step); + + if ( + $stepClassName && + $stepClassName !== 'Crwlr\\Crawler\\Steps\\Step' && + $stepClassName !== 'Crwlr\\Crawler\\Steps\\BaseStep' + ) { + return 'An anonymous class step, that is extending the ' . $stepClassName . ' step'; + } else { + return 'An anonymous class step'; + } + } + } + + protected function getStepClassName(?BaseStep $step = null): ?string + { + $stepClassName = get_class($step ?? $this); + + if (str_contains($stepClassName, '@anonymous')) { + return null; + } + + return $stepClassName; + } + + protected function getParentStepClassName(?BaseStep $step = null): ?string + { + $parents = class_parents($step ?? $this); + + $firstLevelParent = reset($parents); + + if ($firstLevelParent && is_string($firstLevelParent) && !str_contains($firstLevelParent, '@anonymous')) { + return $firstLevelParent; + } + + return null; + } + protected function getInputKeyToUse(Input $input): ?Input { if ($this->useInputKey !== null) { diff --git a/tests/Steps/BaseStepTest.php b/tests/Steps/BaseStepTest.php index d09ac2d..ce01eb1 100644 --- a/tests/Steps/BaseStepTest.php +++ b/tests/Steps/BaseStepTest.php @@ -15,6 +15,7 @@ use Crwlr\Crawler\Steps\Loading\Http; use Crwlr\Crawler\Steps\Step; use Crwlr\Crawler\Steps\StepOutputType; +use Exception; use Generator; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; @@ -235,6 +236,74 @@ protected function invoke(mixed $input): Generator }, ); +test( + 'the warning message, when output type is mixed and keep() was used but not keepAs() with an anonymous step ' . + 'class, extending a step that isn\'t one of the abstract classes Step or BaseStep, contains the parent step ' . + 'class', + function () { + class ParentStepClass extends Step + { + protected function invoke(mixed $input): Generator + { + yield $input; + } + } + + $step = new class extends ParentStepClass {}; + + $step->addLogger(new CliLogger())->keep()->validateBeforeRun(Http::get()); + + expect($this->getActualOutputForAssertion()) + ->toContain( + 'An anonymous class step, that is extending the tests\\Steps\\ParentStepClass step potentially ' . + 'yields scalar value outputs', + ); + }, +); + +test( + 'the warning message, when output type is mixed and keep() was used but not keepAs() with an anonymous step ' . + 'class, extending one of the abstract classes Step or BaseStep, only mentions that it is an anonymous step class', + function (string $extendClass) { + $step = null; + + if ($extendClass === Step::class) { + $step = new class extends Step { + protected function invoke(mixed $input): Generator + { + yield $input; + } + }; + } elseif ($extendClass === BaseStep::class) { + $step = new class extends BaseStep { + protected function invoke(mixed $input): Generator + { + yield $input; + } + + public function invokeStep(Input $input): Generator + { + yield from $this->invoke($input); + } + }; + } + + if ($step === null) { + throw new Exception('Invalid $extendClass parameter'); + } + + $step->addLogger(new CliLogger())->keep()->validateBeforeRun(Http::get()); + + expect($this->getActualOutputForAssertion()) + ->toContain( + 'An anonymous class step potentially yields scalar value outputs', + ); + }, +)->with([ + [Step::class], + [BaseStep::class], +]); + it('does not throw an exception or log a warning when output type is scalar and keepAs() was called', function () { helper_getInputReturningStep()->addLogger(new CliLogger())->keepAs('foo')->validateBeforeRun(Http::get()); @@ -313,6 +382,94 @@ public function outputType(): StepOutputType ->toContain('the next step can not keep it by using keepFromInput()'); }); +test( + 'the warning message, when keepFromInput() was called and previous step yields mixed outputs with an anonymous ' . + 'step class, extending a step that isn\'t one of the abstract classes Step or BaseStep, contains the parent step ' . + 'class', + function () { + class ParentStepClassTwo extends Step + { + protected function invoke(mixed $input): Generator + { + yield 'yo'; + } + + public function outputType(): StepOutputType + { + return StepOutputType::Mixed; + } + } + + $stepWithMixedOutputType = new class extends ParentStepClassTwo {}; + + Http::get() + ->keepFromInput() + ->addLogger(new CliLogger()) + ->validateBeforeRun($stepWithMixedOutputType); + + expect($this->getActualOutputForAssertion()) + ->toContain( + 'An anonymous class step, that is extending the tests\\Steps\\ParentStepClassTwo step potentially ' . + 'yields scalar value outputs', + ); + }, +); + +test( + 'the warning message, when keepFromInput() was called and previous step yields mixed outputs with an anonymous ' . + 'step class, extending one of the abstract classes Step or BaseStep, only mentions that it is an anonymous step ' . + 'class', + function (string $extendClass) { + $stepWithMixedOutputType = null; + + if ($extendClass === Step::class) { + $stepWithMixedOutputType = new class extends Step { + protected function invoke(mixed $input): Generator + { + yield 'yo'; + } + + public function outputType(): StepOutputType + { + return StepOutputType::Mixed; + } + }; + } elseif ($extendClass === BaseStep::class) { + $stepWithMixedOutputType = new class extends BaseStep { + protected function invoke(mixed $input): Generator + { + yield 'yo'; + } + + public function outputType(): StepOutputType + { + return StepOutputType::Mixed; + } + + public function invokeStep(Input $input): Generator + { + yield from $this->invoke($input); + } + }; + } + + if ($stepWithMixedOutputType === null) { + throw new Exception('Invalid $extendClass parameter'); + } + + Http::get() + ->keepFromInput() + ->addLogger(new CliLogger()) + ->validateBeforeRun($stepWithMixedOutputType); + + expect($this->getActualOutputForAssertion()) + ->toContain('An anonymous class step potentially yields scalar value outputs'); + }, +)->with([ + [Step::class], + [BaseStep::class], +]); + /* ----------------------------- keep() ----------------------------- */ it('adds all from array output to the keep array in the output object, when keep() is called', function () {