Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract service helper to base command class #686

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
2e68402
Create ValidatingCommand.php
caendesilva Nov 23, 2022
959d2f8
Add class PHPDoc
caendesilva Nov 23, 2022
8e74930
Move method PublicationService::askWithValidation() to ValidatingCommand
caendesilva Nov 23, 2022
4c921df
Apply fixes from StyleCI
StyleCIBot Nov 23, 2022
2195fdc
Convert static method to instance method
caendesilva Nov 23, 2022
564908e
Publication commands extend ValidatingCommand
caendesilva Nov 23, 2022
da752d2
Use $this-> instead of calling base class method statically
caendesilva Nov 23, 2022
64d9fa8
Use $this instead of $command
caendesilva Nov 23, 2022
6a9988e
Remove redundant self-referencing parameter $command
caendesilva Nov 23, 2022
4c0a6e0
Use $this-> instead of self:: for non-static method
caendesilva Nov 23, 2022
aba7a70
Create ValidatingCommandTest.php
caendesilva Nov 23, 2022
699bf98
Apply fixes from StyleCI
StyleCIBot Nov 23, 2022
b0e9158
Fix constant access call
caendesilva Nov 23, 2022
eeb0138
Merge branch 'extract-service-helper-to-base-command-class' of github…
caendesilva Nov 23, 2022
c6c290b
Type hint Arrayable contract for greater compatibility
caendesilva Nov 23, 2022
6c384fe
Apply fixes from StyleCI
StyleCIBot Nov 23, 2022
171fde6
Start sketching mocks for test
caendesilva Nov 23, 2022
7caa60f
Default should be null
caendesilva Nov 23, 2022
165e39e
Remove redundant argument
caendesilva Nov 23, 2022
43f3251
Finalize the test method
caendesilva Nov 23, 2022
0207462
Differentiate input from default
caendesilva Nov 23, 2022
632209d
Apply fixes from StyleCI
StyleCIBot Nov 23, 2022
7b04217
Specify test method name
caendesilva Nov 23, 2022
2e8c93c
Test retried validation
caendesilva Nov 23, 2022
27b90c0
Test validation timeout
caendesilva Nov 23, 2022
48e0acb
Document safeguard
caendesilva Nov 23, 2022
2983bf6
Increase retry count to further not disturb normal usage
caendesilva Nov 23, 2022
2f9df0f
Refactor to handle recursion safeguard within the method call instead…
caendesilva Nov 23, 2022
54d022e
Document parameter variable
caendesilva Nov 23, 2022
3dbdd4c
Use simpler wording for code documentation
caendesilva Nov 23, 2022
5af712e
Rename protected internal constant to differentiate it from variable
caendesilva Nov 23, 2022
65596bf
Fix missed default argument in refactor
caendesilva Nov 23, 2022
eeacc60
Apply fixes from StyleCI
StyleCIBot Nov 23, 2022
50fe4b0
Rename parameter $message to $question to match base implementation
caendesilva Nov 23, 2022
42a32a6
Use the Validator facade as it is mockable
caendesilva Nov 23, 2022
99fae35
Run assertions on the validator
caendesilva Nov 23, 2022
a44ea9a
Apply fixes from StyleCI
StyleCIBot Nov 23, 2022
56590f4
Extract method
caendesilva Nov 23, 2022
ef8754e
Decouple test class
caendesilva Nov 23, 2022
d11ee4d
Rename test class
caendesilva Nov 23, 2022
443830a
Inline helper method
caendesilva Nov 23, 2022
41415e6
Remove extra newlines
caendesilva Nov 23, 2022
ba2c362
Remove unnecessary boilerplate
caendesilva Nov 23, 2022
03a04ac
Test output was not sent
caendesilva Nov 23, 2022
a48f7a8
Assert rules are passed along
caendesilva Nov 23, 2022
fabdb6c
Apply fixes from StyleCI
StyleCIBot Nov 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Exception;
use Hyde\Console\Commands\Interfaces\CommandHandleInterface;
use Hyde\Console\Concerns\ValidatingCommand;
use Hyde\Framework\Actions\CreatesNewPublicationFile;
use Hyde\Framework\Features\Publications\PublicationService;
use Illuminate\Support\Str;
Expand All @@ -18,7 +19,7 @@
*
* @see \Hyde\Framework\Testing\Feature\Commands\MakePublicationCommandTest
*/
class MakePublicationCommand extends Command implements CommandHandleInterface
class MakePublicationCommand extends ValidatingCommand implements CommandHandleInterface
{
/** @var string */
protected $signature = 'make:publication
Expand Down Expand Up @@ -46,7 +47,7 @@ public function handle(): int
$offset++;
$this->line(" $offset: $pubType->name");
}
$selected = (int) PublicationService::askWithValidation($this, 'selected', "Publication type (1-$offset)", ['required', 'integer', "between:1,$offset"]);
$selected = (int) $this->askWithValidation('selected', "Publication type (1-$offset)", ['required', 'integer', "between:1,$offset"]);
$pubType = $pubTypes->{$pubTypes->keys()[$selected - 1]};
}

Expand All @@ -65,8 +66,7 @@ public function handle(): int
// Useful for debugging
//$this->output->writeln("xxx " . $exception->getTraceAsString());
$this->output->writeln("<bg=red;fg=white>$msg</>");
$overwrite = PublicationService::askWithValidation(
$this,
$overwrite = $this->askWithValidation(
'overwrite',
'Do you wish to overwrite the existing file (y/n)',
['required', 'string', 'in:y,n'],
Expand Down Expand Up @@ -128,7 +128,7 @@ protected function captureFieldInput(object $field, Collection $mediaFiles): str
$offset = $index + 1;
$this->output->writeln(" $offset: $file");
}
$selected = PublicationService::askWithValidation($this, $field->name, $field->name, ['required', 'integer', "between:1,$offset"]);
$selected = $this->askWithValidation($field->name, $field->name, ['required', 'integer', "between:1,$offset"]);
$file = $mediaFiles->{$selected - 1};

return '_media/'.Str::of($file)->after('media/')->toString();
Expand All @@ -153,7 +153,7 @@ protected function captureFieldInput(object $field, Collection $mediaFiles): str
}
}

return PublicationService::askWithValidation($this, $field->name, $field->name, $fieldRules);
return $this->askWithValidation($field->name, $field->name, $fieldRules);
}

protected function getValidationRulesPerType(): Collection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Exception;
use Hyde\Console\Commands\Interfaces\CommandHandleInterface;
use Hyde\Console\Concerns\ValidatingCommand;
use Hyde\Framework\Actions\CreatesNewPublicationType;
use Hyde\Framework\Features\Publications\PublicationService;
use InvalidArgumentException;
Expand All @@ -17,7 +18,7 @@
*
* @see \Hyde\Framework\Testing\Feature\Commands\MakePublicationTypeCommandTest
*/
class MakePublicationTypeCommand extends Command implements CommandHandleInterface
class MakePublicationTypeCommand extends ValidatingCommand implements CommandHandleInterface
{
/** @var string */
protected $signature = 'make:publicationType
Expand All @@ -32,7 +33,7 @@ public function handle(): int

$title = $this->argument('title');
if (! $title) {
$title = trim(PublicationService::askWithValidation($this, 'name', 'Publication type name', ['required', 'string']));
$title = trim($this->askWithValidation('name', 'Publication type name', ['required', 'string']));
$dirname = PublicationService::formatNameForStorage($title);
if (file_exists($dirname)) {
throw new InvalidArgumentException("Storage path [$dirname] already exists");
Expand All @@ -48,27 +49,25 @@ public function handle(): int
$offset = $k + 1;
$this->line(" $offset: $v[name]");
}
$selected = (int) PublicationService::askWithValidation($this, 'selected', "Sort field (0-$offset)", ['required', 'integer', "between:0,$offset"], 0);
$selected = (int) $this->askWithValidation('selected', "Sort field (0-$offset)", ['required', 'integer', "between:0,$offset"], 0);
$sortField = $selected ? $fields[$selected - 1]['name'] : '__createdAt';

$this->output->writeln('<bg=magenta;fg=white>Choose the default sort direction:</>');
$this->line(' 1 - Ascending (oldest items first if sorting by dateCreated)');
$this->line(' 2 - Descending (newest items first if sorting by dateCreated)');
$selected = (int) PublicationService::askWithValidation($this, 'selected', 'Sort field (1-2)', ['required', 'integer', 'between:1,2'], 2);
$selected = (int) $this->askWithValidation('selected', 'Sort field (1-2)', ['required', 'integer', 'between:1,2'], 2);
$sortDirection = match ($selected) {
1 => 'ASC',
2 => 'DESC',
};

$pageSize = (int) PublicationService::askWithValidation(
$this,
$pageSize = (int) $this->askWithValidation(
'pageSize',
'Enter the pageSize (0 for no limit)',
['required', 'integer', 'between:0,100'],
25
);
$prevNextLinks = (bool) PublicationService::askWithValidation(
$this,
$prevNextLinks = (bool) $this->askWithValidation(
'prevNextLinks',
'Generate previous/next links in detail view (y/n)',
['required', 'string', 'in:y,n'],
Expand All @@ -82,7 +81,7 @@ public function handle(): int
$this->line(" $offset: $v->name");
}
}
$selected = (int) PublicationService::askWithValidation($this, 'selected', "Canonical field (1-$offset)", ['required', 'integer', "between:1,$offset"], 1);
$selected = (int) $this->askWithValidation('selected', "Canonical field (1-$offset)", ['required', 'integer', "between:1,$offset"], 1);
$canonicalField = $fields[$selected - 1]['name'];

try {
Expand All @@ -109,7 +108,7 @@ protected function captureFieldsDefinitions(): Collection
$this->output->writeln("<bg=cyan;fg=white>Field #$count:</>");

$field = Collection::create();
$field->name = PublicationService::askWithValidation($this, 'name', 'Field name', ['required']);
$field->name = $this->askWithValidation('name', 'Field name', ['required']);
$this->line('Field type:');
$this->line(' 1 - String');
$this->line(' 2 - Boolean ');
Expand All @@ -120,18 +119,18 @@ protected function captureFieldsDefinitions(): Collection
$this->line(' 7 - Array');
$this->line(' 8 - Text');
$this->line(' 9 - Local Image');
$type = (int) PublicationService::askWithValidation($this, 'type', 'Field type (1-9)', ['required', 'integer', 'between:1,9'], 1);
$type = (int) $this->askWithValidation('type', 'Field type (1-9)', ['required', 'integer', 'between:1,9'], 1);
do {
// TODO This should only be done for types that can have length restrictions right?
$field->min = PublicationService::askWithValidation($this, 'min', 'Min value (for strings, this refers to string length)', ['required', 'string'], 0);
$field->max = PublicationService::askWithValidation($this, 'max', 'Max value (for strings, this refers to string length)', ['required', 'string'], 0);
$field->min = $this->askWithValidation('min', 'Min value (for strings, this refers to string length)', ['required', 'string'], 0);
$field->max = $this->askWithValidation('max', 'Max value (for strings, this refers to string length)', ['required', 'string'], 0);
$lengthsValid = true;
if ($field->max < $field->min) {
$lengthsValid = false;
$this->output->warning('Field length [max] must be [>=] than [min]');
}
} while (! $lengthsValid);
$addAnother = PublicationService::askWithValidation($this, 'addAnother', 'Add another field (y/n)', ['required', 'string', 'in:y,n'], 'y');
$addAnother = $this->askWithValidation('addAnother', 'Add another field (y/n)', ['required', 'string', 'in:y,n'], 'y');

// map field choice to actual field type
$field->type = match ($type) {
Expand Down
66 changes: 66 additions & 0 deletions packages/framework/src/Console/Concerns/ValidatingCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

declare(strict_types=1);

namespace Hyde\Console\Concerns;

use Illuminate\Contracts\Support\Arrayable;
use Illuminate\Support\Facades\Validator;
use LaravelZero\Framework\Commands\Command;
use RuntimeException;
use function ucfirst;

/**
* An extended Command class that provides validation methods.
*
* @see \Hyde\Framework\Testing\Feature\ValidatingCommandTest
*/
class ValidatingCommand extends Command
{
/** @var int How many times can the validation loop run? Guards against infinite loops. */
protected final const MAX_RETRIES = 30;

/**
* Ask for a CLI input value until we pass validation rules.
*
* @param string $name
* @param string $question
* @param \Illuminate\Contracts\Support\Arrayable|array $rules
* @param mixed|null $default
* @param int $retryCount How many times has the question been asked?
* @return mixed
*
* @throws RuntimeException
*/
public function askWithValidation(
string $name,
string $question,
Arrayable|array $rules = [],
mixed $default = null,
int $retryCount = 0
): mixed {
if ($rules instanceof Arrayable) {
$rules = $rules->toArray();
}

$answer = $this->ask(ucfirst($question), $default);
$validator = Validator::make([$name => $answer], [$name => $rules]);

if ($validator->passes()) {
return $answer;
}

foreach ($validator->errors()->all() as $error) {
$this->error($error);
}

$retryCount++;

if ($retryCount >= self::MAX_RETRIES) {
// Prevent infinite loops that may happen, for example when testing. The retry count is high enough to not affect normal usage.
throw new RuntimeException(sprintf("Too many validation errors trying to validate '$name' with rules: [%s]", implode(', ', $rules)));
}

return $this->askWithValidation($name, $question, $rules, null, $retryCount);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,68 +8,16 @@
use Hyde\Framework\Features\Publications\Models\PublicationType;
use Hyde\Hyde;
use Hyde\Pages\PublicationPage;
use Illuminate\Contracts\Validation\Factory as ValidationFactory;
use Illuminate\Support\Str;
use LaravelZero\Framework\Commands\Command;
use Rgasch\Collection\Collection;
use RuntimeException;
use function Safe\file_get_contents;
use Spatie\YamlFrontMatter\YamlFrontMatter;
use function ucfirst;

/**
* @see \Hyde\Framework\Testing\Feature\Services\PublicationServiceTest
*/
class PublicationService
{
/** @var int How many times can the validation loop run? Guards against infinite loops. */
protected final const RETRY_COUNT = 10;

/**
* Ask for a CLI input value until we pass validation rules.
*
* @param \LaravelZero\Framework\Commands\Command $command
* @param string $name
* @param string $message
* @param \Rgasch\Collection\Collection|array $rules
* @param mixed|null $default
* @param bool $isBeingRetried
* @return mixed
*
* @throws RuntimeException
*/
public static function askWithValidation(Command $command, string $name, string $message, Collection|array $rules = [], mixed $default = null, bool $isBeingRetried = false): mixed
{
static $tries = 0;
if (! $isBeingRetried) {
$tries = 0;
}

if ($rules instanceof Collection) {
$rules = $rules->toArray();
}

$answer = $command->ask(ucfirst($message), $default);
$factory = app(ValidationFactory::class);
$validator = $factory->make([$name => $answer], [$name => $rules]);

if ($validator->passes()) {
return $answer;
}

foreach ($validator->errors()->all() as $error) {
$command->error($error);
}

$tries++;

if ($tries >= self::RETRY_COUNT) {
throw new RuntimeException(sprintf("Too many validation errors trying to validate '$name' with rules: [%s]", implode(', ', $rules)));
}

return self::askWithValidation($command, $name, $message, $rules, isBeingRetried: true);
}

/**
* Format the publication type name to a suitable representation for file storage.
*/
Expand Down
110 changes: 110 additions & 0 deletions packages/framework/tests/Feature/ValidatingCommandTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
<?php

declare(strict_types=1);

namespace Hyde\Framework\Testing\Feature;

use Hyde\Console\Concerns\ValidatingCommand;
use Hyde\Testing\TestCase;
use Illuminate\Console\OutputStyle;
use Illuminate\Support\Facades\Validator;
use Mockery;
use RuntimeException;
use function str_starts_with;

/**
* @covers \Hyde\Console\Concerns\ValidatingCommand
*/
class ValidatingCommandTest extends TestCase
{
public function testAskWithValidationCapturesInput()
{
$command = new ValidationTestCommand();

$output = Mockery::mock(OutputStyle::class);

$output->shouldReceive('ask')->once()->withArgs(function (string $question) {
return $question === 'What is your name?';
})->andReturn('Jane Doe');

$output->shouldReceive('writeln')->once()->withArgs(function (string $message) {
return $message === 'Hello Jane Doe!';
});

$command->setOutput($output);
$command->handle();
}

public function testAskWithValidationRetries()
{
$command = new ValidationTestCommand();
$output = Mockery::mock(OutputStyle::class);

$output->shouldReceive('ask')->times(2)->withArgs(function (string $question) {
return $question === 'What is your name?';
})->andReturn('', 'Jane Doe');

$output->shouldReceive('writeln')->times(1)->withArgs(function (string $message) {
return $message === '<error>validation.required</error>';
});

$output->shouldReceive('writeln')->once()->withArgs(function (string $message) {
return $message === 'Hello Jane Doe!';
});

$command->setOutput($output);
$command->handle();
}

public function testAskWithValidationRetriesTooManyTimes()
{
$command = new ValidationTestCommand();
$output = Mockery::mock(OutputStyle::class);

$output->shouldReceive('ask')->times(30)->withArgs(function (string $question) {
return $question === 'What is your name?';
})->andReturn('');

$output->shouldReceive('writeln')->times(30)->withArgs(function (string $message) {
return $message === '<error>validation.required</error>';
});

$this->expectException(RuntimeException::class);
$this->expectExceptionMessage("Too many validation errors trying to validate 'name' with rules: [required]");

$command->setOutput($output);
$command->handle();

$output->shouldNotReceive('writeln')->once()->withArgs(function (string $message) {
return str_starts_with($message, 'Hello');
});
}

public function testValidationIsCalled()
{
$command = new ValidationTestCommand();
$output = Mockery::mock(OutputStyle::class);

$output->shouldReceive('ask')->once()->andReturn('Jane Doe');
$output->shouldReceive('writeln')->once();

$validator = Validator::spy();
$validator->shouldReceive('make')->once()->withArgs(function (array $data, array $rules) {
return $data === ['name' => 'Jane Doe']
&& $rules === ['name' => ['required']];
})->andReturn($validator);
$validator->shouldReceive('passes')->once()->andReturn(true);

$command->setOutput($output);
$command->handle();
}
}

class ValidationTestCommand extends ValidatingCommand
{
public function handle()
{
$name = $this->askWithValidation('name', 'What is your name?', ['required'], 'John Doe');
$this->output->writeln("Hello $name!");
}
}