Skip to content

Commit

Permalink
some edge-case tests and prevention
Browse files Browse the repository at this point in the history
  • Loading branch information
dakujem committed Feb 4, 2024
1 parent 0217b27 commit 5d62b60
Show file tree
Hide file tree
Showing 5 changed files with 204 additions and 10 deletions.
18 changes: 18 additions & 0 deletions src/Exceptions/CallableIssue.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

declare(strict_types=1);

namespace Dakujem\Oliva\Exceptions;

use LogicException;

/**
* A problem caused by an exception thrown while running a callable.
* This exception is provided to enable context for external exceptions.
*
* @author Andrej Rypak <[email protected]>
*/
final class CallableIssue extends LogicException implements FailsIntegrity, AcceptsDebugContext
{
use AcceptDebugContext;
}
3 changes: 3 additions & 0 deletions src/MaterializedPath/Path.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ public static function delimited(string $delimiter, callable $accessor): callabl
*/
public static function fixed(int $levelWidth, callable $accessor): callable
{
if ($levelWidth < 1) {
throw new ConfigurationIssue('The level width must be a positive integer.');
}
return function (mixed $data, mixed $inputIndex = null, ?TreeNodeContract $node = null) use (
$levelWidth,
$accessor,
Expand Down
34 changes: 25 additions & 9 deletions src/Simple/TreeWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
namespace Dakujem\Oliva\Simple;

use Dakujem\Oliva\Exceptions\AcceptsDebugContext;
use Dakujem\Oliva\Exceptions\CallableIssue;
use Dakujem\Oliva\Exceptions\ExtractorReturnValueIssue;
use Dakujem\Oliva\Exceptions\InvalidNodeFactoryReturnValue;
use Dakujem\Oliva\MovableNodeContract;
use Dakujem\Oliva\TreeNodeContract;
use Throwable;

/**
* Simple tree builder.
Expand Down Expand Up @@ -55,22 +57,36 @@ private function wrapNode(
callable $nodeFactory,
callable $childrenExtractor,
): MovableNodeContract {
// Create a node using the provided factory.
$node = $nodeFactory($data);
try {
// Create a node using the provided factory.
$node = $nodeFactory($data);

// Check for consistency.
if (!$node instanceof MovableNodeContract) {
throw (new InvalidNodeFactoryReturnValue())
->tag('node', $node)
->tag('data', $data);
// Check for consistency.
if (!$node instanceof MovableNodeContract) {
throw (new InvalidNodeFactoryReturnValue())
->tag('node', $node)
->tag('data', $data);
}

$childrenData = $childrenExtractor($data, $node);
} catch (Throwable $e) {
if (!$e instanceof AcceptsDebugContext) {
// wrap the exception so that it supports context decoration
$e = (new CallableIssue(
message: $e->getMessage(),
code: $e->getCode(),
previous: $e,
));
}
throw $e->push('nodes', $node ?? null);
}

$childrenData = $childrenExtractor($data, $node);
if (null !== $childrenData && !is_iterable($childrenData)) {
throw (new ExtractorReturnValueIssue('Children data extractor must return an iterable collection containing children data.'))
->tag('children', $childrenData)
->tag('parent', $node)
->tag('data', $data);
->tag('data', $data)
->push('nodes', $node);
}
foreach ($childrenData ?? [] as $key => $childData) {
try {
Expand Down
58 changes: 57 additions & 1 deletion tests/nodes.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ declare(strict_types=1);

namespace Dakujem\Test;

use Dakujem\Oliva\Exceptions\AcceptsDebugContext;
use Dakujem\Oliva\Exceptions\Context;
use Dakujem\Oliva\Node;
use Dakujem\Oliva\Simple\NodeBuilder;
use Dakujem\Oliva\Simple\TreeWrapper;
Expand Down Expand Up @@ -78,11 +80,65 @@ require_once __DIR__ . '/setup.php';
],
],
];
$wrapper = new TreeWrapper(fn(array $raw) => new Node($raw['data']), fn(array $raw) => $raw['children'] ?? null);
$wrapper = new TreeWrapper(
fn(array $raw) => new Node($raw['data']),
fn(array $raw): ?iterable => $raw['children'] ?? null,
);
$root = $wrapper->wrap($data);

// pre-order
Assert::same('FBADCEGIH', TreeTesterTool::flatten($root));

$faulty = $data;
$thrown = false;
try {
$faulty['children'][1]['children'][] = ['data' => 'this will cause type error inside the extractor', 'children' => 42];
$wrapper->wrap($faulty);
} catch (AcceptsDebugContext $e) {
/** @var Context $context */
$context = $e->context;
Assert::count(3, $context->bag['nodes'] ?? []);
Assert::same(
['this will cause type error inside the extractor', 'G', 'F'],
array_map(fn(Node $node) => $node->data(), $context->bag['nodes']),
);
$thrown = true;
}
Assert::true($thrown, 'The catch block has to run');

$faultyWrapper = new TreeWrapper(
fn(array $raw) => new Node($raw['data']),
function (array $raw): mixed {
if (isset($raw['children']) && !is_iterable($raw['children'] ?? null)) {
return 'fault';
}
return $raw['children'] ?? null;
},
);

$thrown = false;
try {
$faultyWrapper->wrap($faulty);
} catch (AcceptsDebugContext $e) {
/** @var Context $context */
$context = $e->context;
Assert::count(3, $context->bag['nodes'] ?? []);
Assert::same(
['this will cause type error inside the extractor', 'G', 'F'],
array_map(fn(Node $node) => $node->data(), $context->bag['nodes']),
);
// the "children" extracted by the extractor
Assert::same('fault', $context->bag['children'] ?? null);
// the data that was present
Assert::same([
'data' => 'this will cause type error inside the extractor',
'children' => 42,
], $context->bag['data'] ?? null);
// ref to the parent node
Assert::type(Node::class, $context->bag['parent'] ?? null);
$thrown = true;
}
Assert::true($thrown, 'The catch block has to run');
})();


Expand Down
101 changes: 101 additions & 0 deletions tests/path.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<?php

declare(strict_types=1);

namespace Dakujem\Test;

use Dakujem\Oliva\Exceptions\ConfigurationIssue;
use Dakujem\Oliva\Exceptions\Context;
use Dakujem\Oliva\Exceptions\InvalidTreePath;
use Dakujem\Oliva\MaterializedPath\Path;
use Dakujem\Oliva\Node;
use Tester\Assert;

require_once __DIR__ . '/setup.php';

// Test some edge cases of Path::delimited
(function () {
Assert::throws(function () {
Path::delimited('', fn() => null);
}, ConfigurationIssue::class, 'The delimiter must be a single character.');
Assert::throws(function () {
Path::delimited('42', fn() => null);
}, ConfigurationIssue::class, 'The delimiter must be a single character.');

Assert::throws(function () {
$extractor = Path::delimited('.', fn() => 42);
$extractor(null);
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');

$extractor = Path::delimited('.', fn($data) => $data);
Assert::same([], $extractor(null));
Assert::same([], $extractor(''));
Assert::same(['a string'], $extractor('a string'));
Assert::same(['a', 'string'], $extractor('a.string'));
Assert::same([' a', ' string '], $extractor(' a. string ')); // no trim implemented

Assert::throws(function () use ($extractor) {
$extractor(42);
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
Assert::throws(function () use ($extractor) {
$extractor([]);
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
Assert::throws(function () use ($extractor) {
$extractor(4.2);
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
Assert::throws(function () use ($extractor) {
$extractor(new Node(null));
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');

$extractor = Path::delimited('.', fn() => 42);
$thrown = false;
$node = new Node('empty');
try {
$extractor('foo', 'bar', $node);
} catch (InvalidTreePath $e) {
/** @var Context $context */
$context = $e->context;
Assert::type(Context::class, $context);
Assert::same([
'path' => 42,
'data' => 'foo',
'index' => 'bar',
'node' => $node,
], $context->bag);
$thrown = true;
}
Assert::true($thrown, 'The catch block has not been executed.');
})();

// Test some edge cases of Path::fixed
(function () {
Assert::throws(function () {
Path::fixed(0, fn() => null);
}, ConfigurationIssue::class, 'The level width must be a positive integer.');
Assert::throws(function () {
Path::fixed(-1, fn() => null);
}, ConfigurationIssue::class, 'The level width must be a positive integer.');

$extractor = Path::fixed(1, fn($data) => $data);
Assert::same(['a', 'b', 'c'], $extractor('abc'));
Assert::same([], $extractor(''));

$extractor = Path::fixed(2, fn($data) => $data);
Assert::same(['ab', 'cd'], $extractor('abcd'));
Assert::same([' ', 'ab', 'cd', ' '], $extractor(' abcd ')); // no trim implemented
Assert::same([], $extractor(''));

Assert::throws(function () use ($extractor) {
$extractor(42);
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
Assert::throws(function () use ($extractor) {
$extractor([]);
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
Assert::throws(function () use ($extractor) {
$extractor(4.2);
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
Assert::throws(function () use ($extractor) {
$extractor(new Node(null));
}, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
})();

0 comments on commit 5d62b60

Please sign in to comment.