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

Refactor exception handling of Binary\Loader\ChainLoader class #1434

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
36 changes: 8 additions & 28 deletions src/Binary/Loader/ChainLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,19 @@

namespace Liip\ImagineBundle\Binary\Loader;

use Liip\ImagineBundle\Exception\Binary\Loader\ChainAttemptNotLoadableException;
use Liip\ImagineBundle\Exception\Binary\Loader\ChainNotLoadableException;
use Liip\ImagineBundle\Exception\Binary\Loader\NotLoadableException;

class ChainLoader implements LoaderInterface
{
/**
* @var LoaderInterface[]
* @var array<string, LoaderInterface>
*/
private array $loaders;

/**
* @param LoaderInterface[] $loaders
* @param array<string, LoaderInterface> $loaders
*/
public function __construct(array $loaders)
{
Expand All @@ -37,36 +39,14 @@ public function find($path)
{
$exceptions = [];

foreach ($this->loaders as $loader) {
foreach ($this->loaders as $configName => $loader) {
try {
return $loader->find($path);
} catch (\Exception $e) {
$exceptions[$e->getMessage()] = $loader;
} catch (NotLoadableException $loaderException) {
$exceptions[] = new ChainAttemptNotLoadableException($configName, $loader, $loaderException);
}
}

throw new NotLoadableException(self::getLoaderExceptionMessage($path, $exceptions, $this->loaders));
}

/**
* @param array<string, LoaderInterface> $exceptions
* @param array<string, LoaderInterface> $loaders
*/
private static function getLoaderExceptionMessage(string $path, array $exceptions, array $loaders): string
{
$loaderMessages = array_map(static function (string $name, LoaderInterface $loader) {
return sprintf('%s=[%s]', (new \ReflectionObject($loader))->getShortName(), $name);
}, array_keys($loaders), $loaders);

$exceptionMessages = array_map(static function (string $message, LoaderInterface $loader) {
return sprintf('%s=[%s]', (new \ReflectionObject($loader))->getShortName(), $message);
}, array_keys($exceptions), $exceptions);

return vsprintf('Source image not resolvable "%s" using "%s" %d loaders (internal exceptions: %s).', [
$path,
implode(', ', $loaderMessages),
\count($loaders),
implode(', ', $exceptionMessages),
]);
throw new ChainNotLoadableException($path, ...$exceptions);
}
}
53 changes: 53 additions & 0 deletions src/Exception/Binary/Loader/ChainAttemptNotLoadableException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Exception\Binary\Loader;

use Liip\ImagineBundle\Binary\Loader\LoaderInterface;

final class ChainAttemptNotLoadableException extends NotLoadableException
{
private string $configName;
private LoaderInterface $loaderInst;

public function __construct(string $configName, LoaderInterface $loaderInst, NotLoadableException $loaderException)
{
$this->configName = $configName;
$this->loaderInst = $loaderInst;

parent::__construct($this->compileFailureText(), 0, $loaderException);
}

public function getLoaderConfigName(): string
{
return $this->configName;
}

public function getLoaderObjectInst(): LoaderInterface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah naming things... i would call the thing just $loader and this method getLoader. i prefer concise names. that it is an instantiated object is defined by the return type declaration.

{
return $this->loaderInst;
}

public function getLoaderObjectName(): string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if i understand correctly, this is the class name without the namespace. how about calling the method getLoaderClassName?

{
return (new \ReflectionObject($this->getLoaderObjectInst()))->getShortName();
}

public function getLoaderPriorError(): string
{
return $this->getPrevious()->getMessage();
}

private function compileFailureText(): string
{
return sprintf('%s=[%s]', $this->getLoaderObjectName(), $this->getLoaderConfigName());
}
}
46 changes: 46 additions & 0 deletions src/Exception/Binary/Loader/ChainNotLoadableException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php

/*
* This file is part of the `liip/LiipImagineBundle` project.
*
* (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*/

namespace Liip\ImagineBundle\Exception\Binary\Loader;

final class ChainNotLoadableException extends NotLoadableException
{
public function __construct(string $path, ChainAttemptNotLoadableException ...$exceptions)
{
parent::__construct(self::compileExceptionMessage($path, ...$exceptions));
}

private static function compileExceptionMessage(string $path, ChainAttemptNotLoadableException ...$exceptions): string
{
return vsprintf('Source image not resolvable "%s" using "%s" %d loaders (internal exceptions: %s).', [
$path, self::compileLoaderConfigMaps(...$exceptions), \count($exceptions), self::compileLoaderErrorsList(...$exceptions),
]);
}

private static function compileLoaderConfigMaps(ChainAttemptNotLoadableException ...$exceptions): string
{
return self::implodeArrayMappedExceptions(static function (ChainAttemptNotLoadableException $e): string {
return $e->getMessage();
}, ...$exceptions);
}

private static function compileLoaderErrorsList(ChainAttemptNotLoadableException ...$exceptions): string
{
return self::implodeArrayMappedExceptions(static function (ChainAttemptNotLoadableException $e): string {
return sprintf('%s=[%s]', $e->getLoaderObjectName(), $e->getLoaderPriorError());
}, ...$exceptions);
}

private static function implodeArrayMappedExceptions(\Closure $mapper, ChainAttemptNotLoadableException ...$exceptions): string
{
return implode(', ', array_map($mapper, $exceptions));
}
}
2 changes: 2 additions & 0 deletions tests/Binary/Loader/ChainLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

/**
* @covers \Liip\ImagineBundle\Binary\Loader\ChainLoader
* @covers \Liip\ImagineBundle\Exception\Binary\Loader\ChainAttemptNotLoadableException
* @covers \Liip\ImagineBundle\Exception\Binary\Loader\ChainNotLoadableException
*/
class ChainLoaderTest extends AbstractTest
{
Expand Down