-
Notifications
You must be signed in to change notification settings - Fork 379
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
[Draft] Clean interfaces for the imagine processes #1590
base: refactor
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<?php | ||
|
||
namespace Liip\ImagineBundle\Domain; | ||
|
||
interface ImageReference | ||
{ | ||
public function getContent(): string; | ||
|
||
public function getMimeType(): ?string; | ||
|
||
public function getFormat(): ?string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?php | ||
|
||
namespace Liip\ImagineBundle\Domain; | ||
|
||
interface ImageReferenceFile extends ImageReference | ||
{ | ||
public function getPath(): string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<?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\Domain\Imagine\Filter; | ||
|
||
use Imagine\Image\ImageInterface; | ||
|
||
interface FilterExecutor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better name for Liip\ImagineBundle\Imagine\Filter\Loader\LoaderInterface - functionality does not need to be changed. |
||
{ | ||
/** | ||
* Loads and applies a filter on the given image. | ||
*/ | ||
public function applyTo(ImageInterface $image, array $options = []): ImageInterface; | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,53 @@ | ||||||
<?php | ||||||
|
||||||
namespace Liip\ImagineBundle\Domain; | ||||||
|
||||||
/** | ||||||
* Main entry point into the imagine system. | ||||||
* | ||||||
* The transformer takes an image id, does all necessary transformations (and potentially caching) and gives the URL to the result image. | ||||||
*/ | ||||||
interface ImagineTransformer | ||||||
{ | ||||||
/** | ||||||
* Determine the URL to the transformed image. | ||||||
* | ||||||
* For performance reasons, this method SHOULD cache the resulting images and skip transformation when it already exists. | ||||||
* | ||||||
* Calling the URL must result in the image being returned. | ||||||
* If possible, the image should only be generated on the fly when the URL is called. | ||||||
* | ||||||
* @param string $sourceImageId Identifier for the image, for example a file system path | ||||||
* @param string $stackName The stack name as configured | ||||||
* @param string $targetFormat Output format to generate | ||||||
* | ||||||
* @return string URL to where the image is available | ||||||
* | ||||||
* @throws ImageNotFoundException | ||||||
* @throws StackNotFoundException | ||||||
* @throws FormatNotSupportedException The requested image format can not be generated by the system (The transformer does not know about browser capabilities) | ||||||
* @throws TransformingException Something went wrong while transforming the image | ||||||
*/ | ||||||
public function transformToUrl(string $sourceImageId, string $stackName, string $targetFormat): string; | ||||||
Check failure on line 31 in src/Domain/ImagineTransformer.php GitHub Actions / phpstan
|
||||||
|
||||||
/** | ||||||
* Force generating the transformed image. | ||||||
* | ||||||
* If the result image is already cached, it will be regenerated and overwritten. | ||||||
* | ||||||
* There is no cache lifetime defined. The application is expected to use active invalidation if the source image changes. | ||||||
* | ||||||
* @param string $sourceImageId Identifier for the image, for example a file system path | ||||||
* @param string[] $stackNames | ||||||
* @param string[] $targetFormats | ||||||
*/ | ||||||
public function warmupCache(string $sourceImageId, array $stackNames, array $targetFormats): void; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i realized we said that transformToUrl should ideally not be slow and not necessarily generates the image right away but can chose to only generate it on demand. and when that is the case, we need this separate method to force warming up the cache. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as per discussion in #1602: there is also a use case to make sure an image exists without forcing regeneration if the image already exists. we could add a flag to this method to control regeneration:
Suggested change
on the other hand, that use case would like to get the image, so maybe a separate method |
||||||
|
||||||
/** | ||||||
* Remove cached images for the specified source image. | ||||||
* | ||||||
* @param string $sourceImageId Identifier for the image, for example a file system path | ||||||
* @param string[] $stackNames Limit invalidation to the specified stacks. If empty, all stacks are invalidated. | ||||||
*/ | ||||||
public function invalidateCache(string $sourceImageId, array $stackNames = []): void; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not strictly about transforming, but imho makes sense to have on the main interaction point with the system. |
||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
<?php | ||
|
||
namespace Liip\ImagineBundle\Domain; | ||
|
||
use Liip\ImagineBundle\Domain\Stack\TransformerStackExecutor; | ||
use Liip\ImagineBundle\Domain\Storage\ImageLoader; | ||
use Liip\ImagineBundle\Domain\Storage\ImageStore; | ||
|
||
/** | ||
* Main entry point into the imagine system. | ||
* | ||
* The transformer takes an image id, does all necessary transformations (and potentially caching) and gives the URL to the result image. | ||
*/ | ||
final class LiipImagineTransformer implements ImagineTransformer | ||
{ | ||
public function __construct( | ||
private ImageLoader $sourceImageLoader, | ||
private TransformerStackExecutor $transformerStackExecutor, | ||
private ImageStore $imageStore, | ||
) {} | ||
|
||
public function transformToUrl(string $sourceImageId, string $stackName, string $targetFormat): string | ||
{ | ||
if ($this->imageStore->supportsOnDemandCreation() | ||
|| $this->imageStore->exists($sourceImageId, $stackName, $targetFormat) | ||
) { | ||
return $this->imageStore->getUrl($sourceImageId, $stackName, $targetFormat); | ||
} | ||
|
||
$this->warmupCache($sourceImageId, [$stackName], [$targetFormat]); | ||
|
||
return $this->imageStore->getUrl($sourceImageId, $stackName, $targetFormat); | ||
} | ||
|
||
public function warmupCache(string $sourceImageId, array $stackNames, array $targetFormats): void | ||
{ | ||
if (0 === count($stackNames)) { | ||
throw new \Exception('TODO: implement determining all stack names'); | ||
} | ||
$sourceImage = $this->sourceImageLoader->loadImage($sourceImageId); | ||
foreach ($stackNames as $stackName) { | ||
foreach ($targetFormats as $targetFormat) { | ||
// TODO: if we would separate stack executor creation and execution, we could build the stack only once and apply it for each target format | ||
$transformedImage = $this->transformerStackExecutor->apply($stackName, $sourceImage); | ||
$this->imageStore->store($transformedImage, $sourceImageId, $stackName, $targetFormat); | ||
} | ||
} | ||
} | ||
|
||
public function invalidateCache(string $sourceImageId, array $stackNames = []): void | ||
{ | ||
$this->imageStore->delete($sourceImageId, $stackNames); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?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\Domain\PostProcessor; | ||
|
||
use Liip\ImagineBundle\Domain\ImageReference; | ||
|
||
/** | ||
* Post processors do additional work on the resulting image after filters have been applied. | ||
* | ||
* @author Konstantin Tjuterev <[email protected]> | ||
*/ | ||
interface PostProcessor | ||
{ | ||
/** | ||
* Allows processing a BinaryInterface, with run-time options, so PostProcessors remain stateless. | ||
* | ||
* @param array<mixed> $options Operation-specific options | ||
*/ | ||
public function process(ImageReference $binary, array $options = []): ImageReference; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<?php | ||
|
||
namespace Liip\ImagineBundle\Domain\Stack; | ||
|
||
use Liip\ImagineBundle\Domain\ImageReference; | ||
use Liip\ImagineBundle\Imagine\Filter\PostProcessor\PostProcessorInterface; | ||
|
||
class LiipTransformerStack implements TransformerStack | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: either finish this or remove it. |
||
{ | ||
/** | ||
* @param FilterInterface[] $filters # used to be Filter\Loader\LoaderInterface | ||
* @param PostProcessorInterface[] $postProcessors | ||
*/ | ||
public function __construct( | ||
private array $filters, | ||
Check failure on line 15 in src/Domain/Stack/LiipTransformerStack.php GitHub Actions / phpstan
|
||
private array $postProcessors, | ||
) {} | ||
|
||
public function applyTo(ImageReference $image): ImageReference | ||
{ | ||
foreach ($this->filters as $filter) { | ||
$image = $filter->applyTo($image); | ||
} | ||
|
||
foreach ($this->postProcessors as $postProcessor) { | ||
$image = $postProcessor->process($image); | ||
} | ||
|
||
return $image; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i renamed this to ImageReference as its the reference to an image rather than an actual image. i noticed when i got confused in LiipTransformerStackExecutor where we also have the imagine ImageInterface which holds the actual image data and has filters applied to it.