Skip to content

Commit 34d67ca

Browse files
committed
Improve empty string handling and validation value exception format
⛑ In default strategy ensure that `empty string` values are converted to null in all value types 🚀 Improve validation exception text with value description: `(null)`, `(array with count X)`, `(empty string)`, string value with maximum of 30 characters.
1 parent 8ae6b2d commit 34d67ca

18 files changed

+351
-37
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ composer require wrkflow/php-get-typed-value
1616
- 🏆 **Makes PHPStan / IDE** happy due the type strict return types.
1717
- 🤹‍ **Validation:** Ensures that desired value is in correct type (without additional loop validation).
1818
- 🛠 **Transformers:** Ensures that values are in expected type.
19+
- ⛑ Converts empty string values to null (can be disabled, see transformers).
1920

2021
```php
2122
use Wrkflow\GetValue\GetValue;

docs/content/en/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ position: 1
1414
- 🏆 **Makes PHPStan / IDE** happy due the type strict return types.
1515
- 🤹‍ **Validation:** Ensures that desired value is in correct type (without additional loop validation).
1616
- 🛠 **Transformers:** Ensures that values are in expected type.
17+
- ⛑ Converts empty string values to null (can be disabled, see transformers).
1718

1819
```php
1920
use Wrkflow\GetValue\GetValue;

docs/content/en/laravel.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ $test = app(TestAction::class)->execute();
5757

5858
To change the implementation using dependency injection just bind contracts below in your framework container:
5959

60-
- `Wrkflow\GetValue\Contracts\TransformerStrategy $transformerStrategy`
60+
- `Wrkflow\GetValue\Contracts\TransformerStrategyContract $transformerStrategy`
6161
- `Wrkflow\GetValue\Contracts\ExceptionBuilderContract $exceptionBuilder`
6262

6363
*Example for Laravel:*
@@ -67,7 +67,7 @@ class MyServiceProvider extends ServiceProvider {
6767
public function register (): void {
6868
parent::register();
6969

70-
$this->app->bind(Wrkflow\GetValue\Contracts\TransformerStrategy::class, MyTransformerStrategy::class);
70+
$this->app->bind(Wrkflow\GetValue\Contracts\TransformerStrategyContract::class, MyTransformerStrategy::class);
7171
$this->app->bind(Wrkflow\GetValue\Contracts\ExceptionBuilderContract::class, MyExceptionBuilder::class);
7272
}
7373
}

src/Actions/ValidateAction.php

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,31 @@
88

99
class ValidateAction
1010
{
11+
private const MaxStringLength = 30;
12+
1113
public function __construct(
1214
private readonly ExceptionBuilderContract $exceptionBuilder,
1315
) {
1416
}
1517

1618
public function execute(array $rules, mixed $value, string $key): void
1719
{
18-
$valueForLog = (is_string($value) || is_numeric($value)) ? (string) $value : null;
20+
if ($value === '') {
21+
$valueForLog = '(empty string)';
22+
} elseif (is_string($value) || is_numeric($value)) {
23+
// Length does not need to be exact (no need for ext-mbstring)
24+
$valueForLog = (string) $value;
25+
if (strlen($valueForLog) > self::MaxStringLength) {
26+
$valueForLog = substr($valueForLog, 0, self::MaxStringLength - 3) . '...';
27+
}
28+
} elseif ($value === null) {
29+
$valueForLog = '(null)';
30+
} elseif (is_array($value)) {
31+
$valueForLog = '(array with count ' . count($value) . ')';
32+
} else {
33+
$valueForLog = null;
34+
}
35+
1936
foreach ($rules as $rule) {
2037
if ($rule->passes($value) === false) {
2138
throw $this->exceptionBuilder->validationFailed($key, $rule::class, $valueForLog);

src/Builders/ExceptionBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public function validationFailed(string $key, string $ruleClassName, ?string $va
3434
$classNameParts = explode('\\', $ruleClassName);
3535
$shortClassName = end($classNameParts);
3636

37-
$valueMessage = $value === null ? '' : (' with value ' . $value);
37+
$valueMessage = $value === null ? '' : (' with value <' . $value . '>');
3838
return new ValidationFailedException($key, $shortClassName . ' failed' . $valueMessage);
3939
}
4040

src/Contracts/TransformerStrategy.php renamed to src/Contracts/TransformerStrategyContract.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace Wrkflow\GetValue\Contracts;
66

7-
interface TransformerStrategy
7+
interface TransformerStrategyContract
88
{
99
/**
1010
* @return array<TransformerContract>

src/GetValue.php

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
use Wrkflow\GetValue\Contracts\GetValueTransformerContract;
1919
use Wrkflow\GetValue\Contracts\RuleContract;
2020
use Wrkflow\GetValue\Contracts\TransformerContract;
21-
use Wrkflow\GetValue\Contracts\TransformerStrategy;
21+
use Wrkflow\GetValue\Contracts\TransformerStrategyContract;
2222
use Wrkflow\GetValue\DataHolders\AbstractData;
2323
use Wrkflow\GetValue\DataHolders\ArrayData;
2424
use Wrkflow\GetValue\DataHolders\XMLAttributesData;
@@ -37,7 +37,7 @@ class GetValue
3737

3838
public function __construct(
3939
public readonly AbstractData $data,
40-
public readonly TransformerStrategy $transformerStrategy = new DefaultTransformerStrategy(),
40+
public readonly TransformerStrategyContract $transformerStrategy = new DefaultTransformerStrategy(),
4141
public readonly ExceptionBuilderContract $exceptionBuilder = new ExceptionBuilder(),
4242
GetValidatedValueAction $getValidatedValueAction = null,
4343
) {
@@ -255,16 +255,24 @@ public function getDateTime(string|array $key, array $rules = [], ?array $transf
255255
transformers: $transformers ?? $this->transformerStrategy->dateTime()
256256
);
257257

258-
if ($value === null || $value === '') {
259-
return null;
260-
}
261-
262258
// Transformer built the date time
263259
if ($value instanceof DateTime) {
264260
return $value;
265261
}
266262

267-
return new DateTime($value);
263+
if ($value === null) {
264+
return null;
265+
}
266+
267+
if ($value === '') {
268+
throw $this->exceptionBuilder->validationFailed(
269+
$this->data->getKey($key),
270+
StringRule::class,
271+
'(empty string)'
272+
);
273+
}
274+
275+
return new DateTime((string) $value);
268276
}
269277

270278
/**

src/GetValueFactory.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
use SimpleXMLElement;
1010
use Wrkflow\GetValue\Builders\ExceptionBuilder;
1111
use Wrkflow\GetValue\Contracts\ExceptionBuilderContract;
12-
use Wrkflow\GetValue\Contracts\TransformerStrategy;
12+
use Wrkflow\GetValue\Contracts\TransformerStrategyContract;
1313
use Wrkflow\GetValue\DataHolders\AbstractData;
1414
use Wrkflow\GetValue\DataHolders\ArrayData;
1515
use Wrkflow\GetValue\DataHolders\XMLData;
@@ -18,7 +18,7 @@
1818
class GetValueFactory
1919
{
2020
public function __construct(
21-
public readonly TransformerStrategy $transformerStrategy = new DefaultTransformerStrategy(),
21+
public readonly TransformerStrategyContract $transformerStrategy = new DefaultTransformerStrategy(),
2222
public readonly ExceptionBuilderContract $exceptionBuilder = new ExceptionBuilder(),
2323
) {
2424
}

src/Strategies/DefaultTransformerStrategy.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44

55
namespace Wrkflow\GetValue\Strategies;
66

7-
use Wrkflow\GetValue\Contracts\TransformerStrategy;
7+
use Wrkflow\GetValue\Contracts\TransformerStrategyContract;
88
use Wrkflow\GetValue\Transformers\TransformToBool;
99
use Wrkflow\GetValue\Transformers\TrimAndEmptyStringToNull;
1010

11-
class DefaultTransformerStrategy implements TransformerStrategy
11+
class DefaultTransformerStrategy implements TransformerStrategyContract
1212
{
1313
public function string(): array
1414
{
@@ -17,7 +17,7 @@ public function string(): array
1717

1818
public function int(): array
1919
{
20-
return [];
20+
return [new TrimAndEmptyStringToNull()];
2121
}
2222

2323
public function bool(): array
@@ -27,21 +27,21 @@ public function bool(): array
2727

2828
public function dateTime(): array
2929
{
30-
return [];
30+
return [new TrimAndEmptyStringToNull()];
3131
}
3232

3333
public function float(): array
3434
{
35-
return [];
35+
return [new TrimAndEmptyStringToNull()];
3636
}
3737

3838
public function array(): array
3939
{
40-
return [];
40+
return [new TrimAndEmptyStringToNull()];
4141
}
4242

4343
public function xml(): array
4444
{
45-
return [];
45+
return [new TrimAndEmptyStringToNull()];
4646
}
4747
}

src/Strategies/NoTransformerStrategy.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44

55
namespace Wrkflow\GetValue\Strategies;
66

7-
use Wrkflow\GetValue\Contracts\TransformerStrategy;
7+
use Wrkflow\GetValue\Contracts\TransformerStrategyContract;
88

9-
class NoTransformerStrategy implements TransformerStrategy
9+
class NoTransformerStrategy implements TransformerStrategyContract
1010
{
1111
public function string(): array
1212
{

tests/Actions/ValidateActionTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Wrkflow\GetValueTests\Actions;
6+
7+
use Exception;
8+
use PHPUnit\Framework\TestCase;
9+
use stdClass;
10+
use Wrkflow\GetValue\Actions\ValidateAction;
11+
use Wrkflow\GetValueTests\Builders\CustomExceptionBuilder;
12+
use Wrkflow\GetValueTests\Rules\TestRule;
13+
14+
class ValidateActionTest extends TestCase
15+
{
16+
public function data(): array
17+
{
18+
return [
19+
['', '(empty string)'],
20+
['fail', 'fail'],
21+
[
22+
'very long string that will be cut off with. This can contain max 30 characters with the dots.',
23+
'very long string that will ...',
24+
],
25+
[1, '1'],
26+
[null, '(null)'],
27+
[[], '(array with count 0)'],
28+
[[1, 2], '(array with count 2)'],
29+
[[
30+
'marco' => 1,
31+
'polo' => 2,
32+
3,
33+
], '(array with count 3)'],
34+
[new stdClass(), ''],
35+
];
36+
}
37+
38+
/**
39+
* @dataProvider data
40+
*/
41+
public function testConvertsValuesToHumanDescription(mixed $value, string $expectedValueMessage): void
42+
{
43+
$action = new ValidateAction(new CustomExceptionBuilder());
44+
try {
45+
$action->execute([new TestRule()], $value, 'test');
46+
} catch (Exception $exception) {
47+
$this->assertStringContainsString('value: <' . $expectedValueMessage . '>', $exception->getMessage());
48+
}
49+
}
50+
}

tests/Builders/CustomExceptionBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function notAnArray(string $key): Exception
2626

2727
public function validationFailed(string $key, string $ruleClassName, ?string $value): Exception
2828
{
29-
return new Exception('validationFailed: ' . $key . ' ' . $ruleClassName);
29+
return new Exception('validationFailed: ' . $key . ' ' . $ruleClassName . ' value: <' . $value . '>');
3030
}
3131

3232
public function notXML(string $key): Exception

0 commit comments

Comments
 (0)