[WIP][1.0][FilterManager] Filter manager drop get method. Add ability to configure at runtime.

[makasim]

It is based on #284 PR so please consider reviewing and merging that one first.

To say briefly about changes done and future:

• Remove FilterManger::get method
• Extend FilterConfiguration to support runtime configuration feature.
• Use Image instance in CacheResolver::store method instead of Response.
• CacheResolver::store must not return anything.
• ...

[havvg]

Please rebase this one on develop again.

As far as I can see, this feature won't make it this way. But it will be cleaner to review once rebased.

[makasim]

@havvg done. please take into account that it is work in progress. Lots of things has to be cleaned up

[havvg]

Yeah I know, it's WIP :-)

First of all: there is nothing marked to be deprecated and we are not keeping backwards compatibility if it stays in the way. The v1.x version line is meant to clean up things caused by the bundles history. The way it's currently committed is invalid anyway, the deprecated method contains non-forward logic and is used by the default implementation (ImagineController::filterAction).

This PR currently mixes up multiple features, I'm fine with it, but it will be a long time until this gets merged.

I don't want to comment this in the code directly, as the following are more general thoughts about where to go with the whole FilterManager thing:

I want to address multiple issues present in the 0.x version line.

Any logic from the FilterManager that's related to fact resolution or defining defaults for undefined facts should be removed. It should rather end up in no undefined facts at all. Example: The is no fact about the format of the image other than the configured one, see https://github.com/formapro-forks/LiipImagineBundle/blob/b2cd329718ec8e2adbb0e4cc6730720ddc644180/Imagine/Filter/FilterManager.php#L76 This line is invalid, it assumes the path being a filename, but the path maybe anything, e.g. an id of a persisted dataset. The same applies to https://github.com/formapro-forks/LiipImagineBundle/blob/b2cd329718ec8e2adbb0e4cc6730720ddc644180/Controller/ImagineController.php#L61

The whole filter configuration and filter loader concept has to be reworked. I'm currently on it and will open a PR for this within the next days.

[makasim]

The is no fact about the format of the image other than the configured one,

good catch!

I'm currently on it and will open a PR for this within the next days.

I am looking forward to see your PR! I'll wait with any changes until than

[makasim]

@havvg What about providing a model that will be returned by data loader? This model will be reused by applyFilters method and cache resolver store. First benefit we do not relay on http request\response any more. Second is we can add format property to the model (or any other) so the applyFilter can use it as default.

[havvg]

That's exactly what I'm on :-)

[makasim]

any way I can help it?

[makasim]

@havvg any progress so far? what about such model?

<?php

namespace Liip\ImagineBundle\Imagine;

use Imagine\Image\ImageInterface;

class ImageWrapper
{
    /**
     * @var
     */
    protected $image;

    /**
     * @var string
     */
    protected $format;

    /**
     * @param ImageInterface $image
     * @param $format
     */
    public function __construct(ImageInterface $image, $format)
    {
        $this->image = $image;
        $this->format = $format;
    }

    /**
     * @return ImageInterface
     */
    public function getImage()
    {
        return $this->image;
    }

    /**
     * @return string
     */
    public function getFormat()
    {
        return $this->format;
    }

    /**
     * @throws \RuntimeException
     *
     * @return string
     */
    public function getMimeType()
    {
        static $mimeTypes = array(
            'jpeg' => 'image/jpeg',
            'jpg'  => 'image/jpeg',
            'gif'  => 'image/gif',
            'png'  => 'image/png',
            'wbmp' => 'image/vnd.wap.wbmp',
            'xbm'  => 'image/xbm',
        );

        if (!isset($mimeTypes[$this->format])) {
            throw new \RuntimeException(sprintf(
                'Unsupported format given. Only %s are supported, %s given',
                implode(", ", array_keys($mimeTypes)), $this->format
            ));
        }

        return $mimeTypes[$this->format];
    }
}

This model will be returned by data loader and required by cache resolver store method?

[makasim]

@havvg if we could agreed on a model its name and data it may contain we can split the work. So I think we should first do PR with only this single model? wdt?

[makasim]

for the reference: php-imagine/Imagine#221, php-imagine/Imagine#208

[makasim]

close in favor #309