Skip to content

Commit

Permalink
fix: code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
sitepark-veltrup committed Nov 30, 2023
1 parent be67418 commit ac1e943
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 15 deletions.
1 change: 1 addition & 0 deletions phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.1/phpunit.xsd"
colors="true"
bootstrap="vendor/autoload.php"
displayDetailsOnTestsThatTriggerWarnings="true"
executionOrder="random"
cacheResultFile="var/cache/.phpunit.result.cache"
cacheDirectory="var/cache/.phpunit.cache">
Expand Down
7 changes: 2 additions & 5 deletions src/Exception/RootMissingException.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@
namespace Atoolo\Resource\Exception;

/**
* This exception is used when a resource is invalid. This can have the
* following reasons:
*
* - If the resource is syntactically incorrect.
* - If the resource does not contain necessary data.
* Is used by the ResourceHierarchyLoader to indicate that the requested
* data could not be determined because no root element can be resolved.
*/
class RootMissingException extends \RuntimeException
{
Expand Down
50 changes: 42 additions & 8 deletions src/Loader/SiteKitResourceHierarchyLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,14 @@ public function loadParent(string $location): ?Resource

/**
* @return Resource[]
* @throws InvalidResourceException if an encountered Resource has no
* parent but is not considered a root.
*/
public function loadPath(string $location): array
{
$resource = $this->resourceLoader->load($location);
$path = [$resource];
while (true) {
if ($this->isRoot($resource)) {
break;
}
while (!$this->isRoot($resource)) {
$parent = $this->loadPrimaryParentResource($resource);
array_unshift($path, $parent);
$resource = $parent;
Expand All @@ -75,6 +74,9 @@ public function loadChildren(string $location): array
return $children;
}

/**
* @throws InvalidResourceException if no primary parent can be found
*/
protected function loadPrimaryParentResource(
Resource $resource
): Resource {
Expand Down Expand Up @@ -114,15 +116,47 @@ protected function getPrimaryParentLocation(Resource $resource): ?string

$firstParent = null;
foreach ($parentList as $parent) {
if ($firstParent === null) {
$firstParent = $parent;
}
$firstParent ??= $parent;
$isPrimary = $parent['isPrimary'] ?? false;
if ($isPrimary) {
if ($isPrimary === true) {
if (!isset($parent['url'])) {
throw new InvalidResourceException(
$resource->getLocation(),
'primary parent in ' .
'base.trees.' . $this->treeName . '.parents ' .
'as no url'
);
}
if (!is_string($parent['url'])) {
throw new InvalidResourceException(
$resource->getLocation(),
'url of primary parent in ' .
'base.trees.' . $this->treeName . '.parents ' .
'is not a string'
);
}
return $parent['url'];
}
}

if (!isset($firstParent['url'])) {
throw new InvalidResourceException(
$resource->getLocation(),
'first parent in ' .
'base.trees.' . $this->treeName . '.parents ' .
'has no url'
);
}

if (!is_string($firstParent['url'])) {
throw new InvalidResourceException(
$resource->getLocation(),
'url of first parent in ' .
'base.trees.' . $this->treeName . '.parents ' .
'is not a string'
);
}

return $firstParent['url'];
}

Expand Down
16 changes: 15 additions & 1 deletion src/ResourceHierarchyLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,36 @@
namespace Atoolo\Resource;

/**
* The ResourceHierarchyLoader interface defines the method used to load
* The ResourceHierarchyLoader interface defines the methods used to load
* resources or nodes whose hierarchical structure is defined in the resources.
* For example, the navigation tree.
*/
interface ResourceHierarchyLoader
{
/**
* Determines the root resource via the parent links contained in the
* resource data.
*/
public function loadRoot(string $location): Resource;

/**
* Determines the parent resource via the parent links contained in the
* resource data.
*/
public function loadParent(string $location): ?Resource;

/**
* Determines the path to the root resource via the parent links contained
* in the resource data.
* The array contains the resources starting with the root resource. The
* last element of the array is the resource of the passed `$location`
* @return Resource[]
*/
public function loadPath(string $location): array;

/**
* Determines the children resources via the children links contained in the
* resource data.
* @return Resource[]
*/
public function loadChildren(string $location): array;
Expand Down
2 changes: 1 addition & 1 deletion test/Loader/SiteKitLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ protected function setUp(): void
public function testExists(): void
{
$exists = $this->loader->exists('validResource.php');
$this->assertTrue($exists, 'resource should be exists');
$this->assertTrue($exists, 'resource should exist');
}

public function testLoadValidResource(): void
Expand Down
1 change: 1 addition & 0 deletions test/Loader/SiteKitNavigationHierarchyLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Atoolo\Resource\Test\Loader;

use Atoolo\Resource\Exception\InvalidResourceException;
use Atoolo\Resource\Exception\RootMissingException;
use Atoolo\Resource\Loader\SiteKitNavigationHierarchyLoader;
use Atoolo\Resource\ResourceLoader;
Expand Down
25 changes: 25 additions & 0 deletions test/Loader/SiteKitResourceHierarchyLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,29 @@ public function testLoadParentWithoutParent(): void
$parent = $this->hierarchyLoader->loadParent('/a.php');
$this->assertNull($parent, 'parent should be null');
}

public function testLoadRootResourcePrimaryParentWithoutUrl(): void
{
$this->expectException(InvalidResourceException::class);
$this->hierarchyLoader->loadParent('/primaryParentWithoutUrl.php');
}

public function testLoadRootResourcePrimaryParentWithNonStringUrl(): void
{
$this->expectException(InvalidResourceException::class);
$this->hierarchyLoader->loadParent('/primaryParentWithNonStringUrl.php');
}

public function testLoadRootResourceFirstParentWithoutUrl(): void
{
$this->expectException(InvalidResourceException::class);
$this->hierarchyLoader->loadParent('/firstParentWithoutUrl.php');
}

public function testLoadRootResourceFirstParentWithNonStringUrl(): void
{
$this->expectException(InvalidResourceException::class);
$this->hierarchyLoader->loadParent('/firstParentWithNonStringUrl.php');
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php


return new \Atoolo\Resource\Resource(
'/primaryParentWithoutUrl.php',
'primaryParentWithoutUrl',
'primaryParentWithoutUrl',
'',
[
'base' => [
'trees' => [
'category' => [
'parents' => [
'a' => [
'url' => false
]
]
]
]
]
]
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php


return new \Atoolo\Resource\Resource(
'/primaryParentWithoutUrl.php',
'primaryParentWithoutUrl',
'primaryParentWithoutUrl',
'',
[
'base' => [
'trees' => [
'category' => [
'parents' => [
'a' => [
]
]
]
]
]
]
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php


return new \Atoolo\Resource\Resource(
'/primaryParentWithoutUrl.php',
'primaryParentWithoutUrl',
'primaryParentWithoutUrl',
'',
[
'base' => [
'trees' => [
'category' => [
'parents' => [
'a' => [
'isPrimary' => true,
'url' => false
]
]
]
]
]
]
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php


return new \Atoolo\Resource\Resource(
'/primaryParentWithoutUrl.php',
'primaryParentWithoutUrl',
'primaryParentWithoutUrl',
'',
[
'base' => [
'trees' => [
'category' => [
'parents' => [
'a' => [
'isPrimary' => true,
]
]
]
]
]
]
);

0 comments on commit ac1e943

Please sign in to comment.