Roadmap 2.0

[makasim]

• [ ] Decouple cache from filter and filter config.
• [ ] Decouple loader from filter and filter config.
• [ ] Create a facade service. move logic from filterAction to it.
• [ ] Ability to change format of cached image (TBD)
• [ ] Cleanup code, remove duplications and deprecations.
• [ ] Symfony 2.8 as minimum version
• [ ] PHP 5.5 as minimum version.
• [ ] Remove cache images with runtime config applied
• [ ] Add ability to force cache generation. Do not use controller, always return url to cached image.

[mappedinn]

What about an implementation of google cloud storage resolver like aws s3? Is it planed? @lsmith77

[makasim]

Not it is not. Feel free to contribute it .

[mappedinn]

perfect! :)

[jrattue]

@makasim Do you have any plans to remove the /resolve/ from the filter path?

If the resolve was removed in filterAction you could return the binary rather than redirecting to the image path.

[makasim]

@ralf57 it was done on purpose both resolve prefix and redirect from filterAction. I am not going to change it

[jrattue]

@makasim thanks for the speedy reply.

Only reason I was asking is when caching html response the path with "resolve" is cached, casing the request to always hit the controller until cache clears. This only happens on image generation, if image exists will cache true image path.

[makasim]

I dont see a big problem here, the filterAction returns 301 redirects and browsers are smart enough to remember the url and do not request the action again.

Also You can force cache creation once image is uploaded, or from the cli.

[Koc]

@makasim is it possible change behavior to get rid of calling isStored https://github.com/liip/LiipImagineBundle/blob/master/Imagine/Cache/CacheManager.php#L141 when generation url? Older versions of the bundle does not check file existance and it was created on demand.

Also there are a lot of issues where people want skip redirects parts. This problems have similar root.

[makasim]

What is the problem with isStored

[Koc]

file_exists call which can be omited like in older versions https://github.com/liip/LiipImagineBundle/blob/0.x/Imagine/Cache/CacheManager.php#L123 .

[makasim]

Why do you need it? I see only case where you may need it: when you change the origin image but the id is still the same, please do not do that way

[Koc]

Cache stored on remote server mounted by sshfs. Yes, it is possible use cached resolver, but simpler sollution do not check existence each the time. Currently we are rewrited resolver and it returns always true for isStored and we are hawing same urls for resolve and target files

[makasim]

sshfs is kind of special case I think it is good to solve it by overwriting the default resolver. The other way to go is to add a cache wrapper over the web one.

[mitrae]

I dont see a big problem here, the filterAction returns 301 redirects and browsers are smart enough to remember the url and do not request the action again.

page cache will be used by many people and their browsers know nothing about that redirect. so instead of requesting static files all these unique users will hit cache/resolve/* = sf controller!

[makasim]

@mitrae Warm up a cache than. There is a cli command for that. or from the code.

[mitrae]

warming cache is OK when you have small site with few images. but for shop with huge amount of pictures lazy load(like it was in 0.21.x) is more preferable.

[mitrae]

@makasim and how do you see it? add cron job that runs this cli every X minute? or add trigger to any action that could invalidate cached image? both ways add extra complexity layer.

[makasim]

When the image is uploaded, post it to the queue and create caches there.

[eugenegp]

@makasim Not clear why one resource should have two different URLs?

[mitrae]

And when pack with images uploaded and when user changes size of images and when user changes watermark and when ... Why I should spend server resources on warming stuff that may will not be requested at all.

[mitrae]

@lsmith77 do you also think that current behavior with "/resolve/" is right?

[steven-pribilinskiy]

Would be nice to hear opinions from @graundas and @helios-ag too

[lsmith77]

@mitrae yes ... any cache should understand the 301 ..

[mitrae]

@lsmith77 again, you guys did 2 different urls for same resource, let's call it R(resolve) and C(cached)

Imagine, Varnish puts to its cache html where all images have links R. This mean that all users which will receive this cached page will make some load because on 1st visit their browsers do not know about 301 redirect, so they will hit SF controller. For website with high traffic this could be a big problem.

Imagine, Varnish puts to its cache html where all images have links C. If at some reason next day image cache became invalid then all users who open cached html(with image links C) will get 404.

[lsmith77]

no properly build reverse proxy will ever cache a 301 .. instead they will cache the page to which the 301 points to (if the client chooses to follow) ..

now if the image gets invalidated (ie. the image is missing ... your 404 scenario), then this bundle should regenerated the image .. if this does not happen, then this would be a bug that needs fixing.

[mitrae]

@lsmith77 what about 2nd case, when proxy cache stores links without "resolve"? If cache contains image src "media/cache/filter/relative/path/image.jpg" and cache gets invalidated then we get 404, because controller intercepts only "media/cache/resolve/filter/*"

[KeKKai]

@lsmith77 you forgot that we are talking about images - the page which has incorrect links will be cached and it knows nothing about 301 redirects and knows nothing about invalidated images. And since requests to controller and to image are different it will fail to detect that request to image should be redirected to "resolve"...

[makasim]

Whenever you delete something on the server it affects the cache layer, and hence you have to properly invalidate\regenerate it. Whether it is an image or other stuff, it does not matter.

[makasim]

And having two urls perfectly fine from my point of view. You have resolve one and when the cache is created the server tells you that the resource was moved permanently and could be found at new location.

If you need something different you can easily extend the bundle with your approach (by creating your own controller which always returns the image, with status 200, same url always). Publish it on github. Profit!

[mitrae]

@makasim Imagine website with tons of images. You suggest me to regenerate cache for all images if user changed some settings (e.g. width/height). It is not optimal at all.

[makasim]

@mitrae Do you have a solution?

[mitrae]

@makasim in v0.21.x it worked perfect. what was the reason to change it?

[makasim]

As a rember the approach was the same it did a redirect too. The only difference is that the url resolve and cache urls were the same. Safari does not like 301 redirect to the same url. So we had to fix it. https://github.com/liip/LiipImagineBundle/issues/429. If you are not interested in safari users you can overwrite the resolve route to exactly the same as the cache one (See https://github.com/liip/LiipImagineBundle/issues/675).

https://github.com/liip/LiipImagineBundle/blob/0490ecfd7184e71b24108ba0c2a85261c14b9fc4/Imagine/Cache/Resolver/WebPathResolver.php#L29 https://github.com/liip/LiipImagineBundle/blob/0490ecfd7184e71b24108ba0c2a85261c14b9fc4/Imagine/Cache/Resolver/AwsS3Resolver.php#L105

[makasim]

And redirect to the same url (as it was in 0.x version) worked only for local resolvers and did not work for any other like amazon s3. And as for me local storage is kind of special case

[mitrae]

@makasim I see. What was the problem to return generated image in response instead of 301?

[makasim]

I see. What was the problem to return generated image in response instead of 301?

It works only for local resolver. You do not want to pass aws s3 image content through your server, don't you? Doing a 200 response for local storage and 301 for the rest is a special case which you can implement in your own app easily but it should not be here in the bundle.

[mitrae]

@makasim OK. sure, for non-local it makes sense. But it would be nice to have such behavior configurable.

[mitrae]

@makasim Btw, correct me If I wrong, it is possible to configure Amazon S3 to request original image on your site if cached image is missed. So you can always return in html imgsrc that points to amazon s3 and still have lazy load using CloudFront. Am i right?

[makasim]

@mitrae As far as I know you cannot do it with S3. But it could be possible to do with CloudFront only, properly configured .

[mitrae]

@makasim as I wrote. So one more reason to have option to use one URL and have lazy load.

[lsmith77]

ah .. so the issue is that the URLs generated by the twig handler point to the /resolve aka 301 URL? that indeed isn't good ..

[trsteel88]

I have mentioned some of these points several times. Imo having the URL's separated isn't good.

At the moment there is no route for the non resolve paths. This means that if the source of an image is included inside a WYSIWYG (for example) and it isn't using the /resolve path the image will 404 if the cache is deleted.

We had to resolve this by adding 2 additional routes to all our sites:

acme_asset.liip.resolve_runtime:
    path: /media/cache/{filter}/rc/{hash}/{path}
    defaults: { _controller: "%liip_imagine.controller.filter_runtime_action%" }
    requirements:
        filter: "(?!\bresolve\b)[A-z0-9_\-]*"
        path:   .+

acme_asset.liip.resolve:
    path: /media/cache/{filter}/{path}
    defaults: { _controller: "%liip_imagine.controller.filter_action%" }
    requirements:
        filter: '(?!\bresolve\b)[A-z0-9_\-]*'
        path:   .+

I've said it a few times, we shouldn't be hacking this bundle up just because Safari has a bug. Instead, that issue really should be addressed with Safari's developers instead so they can patch the problem. Or, this should be an option available to enable, I don't think this should be a default.

Regarding the cli to generate images, this isn't feasible on large eCommerce websites. Some sites I have worked on have thousands of images. Generate images that may not even be displayed for several hours/days/months because those specific images & filters aren't requested is just a waste of resources.

[mitrae]

@trsteel88 as one of e-commerce developer, I fully agree with you )

This behaviour must be configurable. Having "resolve" URL makes sense only when using Amazon S3 without CloudFront. So why everybody else should have problems because of this?

@makasim @lsmith77 IMO, should be an option to return 301 or return binary (solves issue with Safari) or have "resolve" url.

[makasim]

This is to configure just overwrite the route! What could be easier than that?

[mitrae]

@makasim no problem if you know about this. Please could you add information about "resolve" url into documentation? Personally for me it was surprise to see it inside cached documents.

[Kyoushu]

I second the comment made by @mitrae, having a config option called something like resolve_strategy with either the value "redirect" or "binary" would save us having to create our own controllers.

When using the redirect strategy, "/resove/..." URLs would redirect to the generated image.

When using the binary strategy, the generated image would be returned in a BinaryFileResponse (if the image doesn't already exist), but all subsequent requests would hit the static file.

[Kyoushu]

Rather than having ImagineController::filterAction() create the RedirectResponse itself, this could be handed off to the implementation of Liip\ImagineBundle\Imagine\Cache\Resolver\ResolverInterface

/**
 * @param string $path
 * @param string $filter
 * @return Symfony\Component\HttpFoundation\Response
 */
public function createResponse($path, $filter);

In the case of WebPathResolver, an additional argument could be passed to the constructor containing the value of resolve_strategy (or whatever it may be called), This would be the switch that decides if createResponse() returns a RedirectResponse or a BinaryFileResponse.

I'm not sure, however, how it could change CacheManager::generateUrl() so that /resolve is not included in the URL.

[makasim]

Resolvers should never be coupled to request\response.

[makasim]

Please, create a fork of this repository with all those valuable special cases. I'd be more than happy to add link to it here.

[lsmith77]

FYI https://github.com/symfony/symfony/pull/21820