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

[Draft] Clean interfaces for the imagine processes #1590

Open
wants to merge 1 commit into
base: refactor
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
12 changes: 12 additions & 0 deletions src/Domain/ImageReference.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace Liip\ImagineBundle\Domain;

interface ImageReference
Copy link
Member Author

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.

{
public function getContent(): string;

public function getMimeType(): ?string;

public function getFormat(): ?string;
}
8 changes: 8 additions & 0 deletions src/Domain/ImageReferenceFile.php
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;
}
22 changes: 22 additions & 0 deletions src/Domain/Imagine/Filter/FilterExecutor.php
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
Copy link
Member Author

Choose a reason for hiding this comment

The 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;

Check failure on line 21 in src/Domain/Imagine/Filter/FilterExecutor.php

View workflow job for this annotation

GitHub Actions / phpstan

Method Liip\ImagineBundle\Domain\Imagine\Filter\FilterExecutor::applyTo() has parameter $options with no value type specified in iterable type array.
}
53 changes: 53 additions & 0 deletions src/Domain/ImagineTransformer.php
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

View workflow job for this annotation

GitHub Actions / phpstan

PHPDoc tag @throws with type Liip\ImagineBundle\Domain\FormatNotSupportedException|Liip\ImagineBundle\Domain\ImageNotFoundException|Liip\ImagineBundle\Domain\StackNotFoundException|Liip\ImagineBundle\Domain\TransformingException is not subtype of Throwable

/**
* 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;
Copy link
Member Author

@dbu dbu Jun 19, 2024

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

@dbu dbu Jul 10, 2024

Choose a reason for hiding this comment

The 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
public function warmupCache(string $sourceImageId, array $stackNames, array $targetFormats): void;
public function warmupCache(string $sourceImageId, array $stackNames, array $targetFormats, bool $regenerateExisting): void;

on the other hand, that use case would like to get the image, so maybe a separate method public function getImage(string $sourceImageId, string $stackName, string $targetFormat): ImageReference would be more suited for that use case.


/**
* 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;
Copy link
Member Author

Choose a reason for hiding this comment

The 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.

}
54 changes: 54 additions & 0 deletions src/Domain/LiipImagineTransformer.php
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);
}
}
29 changes: 29 additions & 0 deletions src/Domain/PostProcessor/PostProcessor.php
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 <kostik.lv@gmail.com>
*/
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;
}
31 changes: 31 additions & 0 deletions src/Domain/Stack/LiipTransformerStack.php
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

Check failure on line 8 in src/Domain/Stack/LiipTransformerStack.php

View workflow job for this annotation

GitHub Actions / phpstan

Class Liip\ImagineBundle\Domain\Stack\LiipTransformerStack implements unknown interface Liip\ImagineBundle\Domain\Stack\TransformerStack.
Copy link
Member Author

Choose a reason for hiding this comment

The 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

View workflow job for this annotation

GitHub Actions / phpstan

Parameter $filters of method Liip\ImagineBundle\Domain\Stack\LiipTransformerStack::__construct() has invalid type Liip\ImagineBundle\Domain\Stack\FilterInterface.

Check failure on line 15 in src/Domain/Stack/LiipTransformerStack.php

View workflow job for this annotation

GitHub Actions / phpstan

Property Liip\ImagineBundle\Domain\Stack\LiipTransformerStack::$filters has unknown class Liip\ImagineBundle\Domain\Stack\FilterInterface as its type.
private array $postProcessors,
) {}

public function applyTo(ImageReference $image): ImageReference
{
foreach ($this->filters as $filter) {
$image = $filter->applyTo($image);

Check failure on line 22 in src/Domain/Stack/LiipTransformerStack.php

View workflow job for this annotation

GitHub Actions / phpstan

Call to method applyTo() on an unknown class Liip\ImagineBundle\Domain\Stack\FilterInterface.
}

foreach ($this->postProcessors as $postProcessor) {
$image = $postProcessor->process($image);
}

return $image;
}
}
Loading
Loading