LiipImagineBundle icon indicating copy to clipboard operation
LiipImagineBundle copied to clipboard

Set Twig mode to lazy makes it no longer work

Open bastien70 opened this issue 2 years ago • 10 comments

Preconditions

Symfony 6.0 PHP 8.1 LiipImagineBundle 2.7.4

Steps to reproduce

Remember, in this issue #1442 , I talked about a depreciation to be corrected We agreed that we should therefore, in the liip_imagine configuration file, pass the variable twig.mode to lazy

Expected result

The depreciation had disappeared.

Actual result

However, something I hadn't noticed was that by switching to lazy twig_mode, well LiipImagineBundle just doesn't work anymore. I use LiipImagine with FlySystem, AWS S3, as well as cache support for flysystem (https://github.com/Lustmored/flysystem-v2-simple-cache-adapter).

But I noticed that now liip_imagine no longer generates the resized images as it should Similarly, if the images have already been created by LiipImagine, and they are displayed well in legacy mode, then if we switch to lazy mode, they will no longer be displayed.

So there is something wrong with this mod I guess

Question

So how can we continue to use legacy mode instead of lazy mode in the future ?

bastien70 avatar Jan 19 '22 10:01 bastien70

that sounds worrying. the lazy twig mode should still work the same when the twig functions are actually used. the only difference is that less things are initialized upfront, and things only initialized when the twig functions are actually used. have you seen anything why the lazy mode does not work? is the liip imagine system no longer called when using the twig functions? does it no longer generate urls?

this has been released a moment ago, so either not many people use the twig integration, or this is not a general problem but somehow related to your setup. i am not sure which.

deprecation means you can still use it until this problem is solved.

dbu avatar Jan 20 '22 07:01 dbu

From what I've seen, the URLs seem to be generated fine. That is to say that the URL where the resized image is supposed to be is established, but the image in reality will not be there because LiipImagine will not have created it upstream.

I will try in the afternoon to make a minimalist github repository with vichUploader and LiipImagine to focus on the problem

bastien70 avatar Jan 20 '22 07:01 bastien70

Okay I did it. When using lazy twig mode, liipImagine is searching in my local project instead of my AWS S3. For local storage, it will work in legacy and lazy mode without worries.

On the other hand, if we use a cloud such as AWS S3, then in legacy mode, the path will be correct, but in lazy mode, it will automatically take the base path of the project and not that of AWS

bastien70 avatar Jan 20 '22 14:01 bastien70

hm, that sounds weird. the lazy mode gets the cache manager injected just the same as the legacy mode

can you try to debug these service definitions, try to see where it goes wrong?

there are differences between legacy (https://github.com/liip/LiipImagineBundle/blob/2.x/Templating/Helper/FilterHelper.php resp https://github.com/liip/LiipImagineBundle/blob/2.x/Templating/FilterTrait.php) and the new https://github.com/liip/LiipImagineBundle/blob/2.x/Templating/LazyFilterRuntime.php in that the new one handles the asset version. but that should not lead to using a different storage system.

dbu avatar Jan 25 '22 13:01 dbu

I'm facing a similar problem. I use VichUploaderBundle to store my images on AWS S3 (using Gaufrette. After updating LiipImagineBundle to version 2.7.6, I changed twig.mode to lazy but all my filtered images are now broken. Switching back to legacy fix the problem.

After digging in FilterTrait and LazyFilterRuntime, i noticed a difference in the filter and filterCache methods of the FilterTrait. Both of them modify the path to parse the url and keep only the path : parse_url($path, PHP_URL_PATH).

As I store my image on AWS using VichUploader, path look like this:

https://aws.storage/bucket/folder/image.jpg

In legacy mode, the url generated when I apply a filter on this image is:

https://my.app/media/cache/resolve/my_filter/bucket/folder/image.jpg

I set a loader in LiipImagine config as:

loaders:
    default:
      stream:
        wrapper: 'https://aws.storage/'

In lazy mode, the url generated when I apply a filter on the same image is:

https://my.app/media/cache/resolve/my_filter/https://aws.storage/bucket/folder/image.jpg

And with the wrapper, the loader actually make a call on:

https://aws.storage/https://aws.storage/bucket/folder/image.jpg

And this image doesn't exists.

picoss avatar Feb 09 '22 10:02 picoss

thank you for the examples. there is a bunch of code changes in #1397, i wonder if thats where things broke.

i guess the LazyFilterRuntime is doing something a bit different than the legacy filter. but not sure if its specifically a problem with the stream wrapper or something more general.

i don't have much time to dig into this at the moment. so if one of you has time to locate the problem, for example in a PR with a test that triggers the problem, that would help

dbu avatar Feb 09 '22 11:02 dbu

Hi, I have same issue.

In order to know my configuration :

  • I have a custom cache resolver (extends of WebPathResolver)
  • I have a custom PostProcessor who generates webp images

I have notive issue with mode lazy only when my env is in prod. When i switch to dev it's work with mode lazy. In both env it's works with legacy mode too.

I don't know if this can help you...

Fredxd avatar Jun 22 '22 13:06 Fredxd

are you able to reproduce the problem with a new test case in Tests/Templating/LazyFilterRuntimeTest.php - that would help a lot to debug the problem.

dbu avatar Jul 28 '22 06:07 dbu

is this still a problem? is somebody able to reproduce it with the LazyFilterRuntimeTest?

dbu avatar Sep 26 '22 14:09 dbu

It is still a problem.

If the filter method in FilterTrait is called with an absolute URL like https://www.example.org/images/dogs.jpg, the URL gets parsed and the getBrowserPath method will be called with /images/dogs.jpg

See https://github.com/liip/LiipImagineBundle/blob/d7f6a4f87fee51b6c42eabc7646f393420f38f5a/Templating/FilterTrait.php#L46

If the filter method in LazyFilterRuntime is called with an absolute URL like https://www.example.org/images/dogs.jpg, the URL isn't parse and the getBrowserPath method will be called with https://www.example.org/images/dogs.jpg, because the parse_url in cleanPath was dropped in

https://github.com/liip/LiipImagineBundle/commit/13a32e4f32aae62f6c8fc86928ad5dbf0aea1966#diff-60bd152832efc2c3de6906c94dd05ed0438bb9a4fc09a519e918c1f88f770934L56

The testInvokeFilterMethod in LazyFilterRuntimeTest should probably be extended by a 3rd parameter to test if the CacheManager is called with the properly cleaned path

like `public function provideImageNames(): iterable { yield 'regular' => ['image' => 'cats.jpeg', 'callingimage' => 'cats.jpeg', 'urlimage' => 'cats.jpeg']; yield 'whitespace' => ['image' => 'white cat.jpeg', 'callingimage' => 'white cat.jpeg', 'urlimage' => 'white%20cat.jpeg']; yield 'plus' => ['image' => 'cat+plus.jpeg', 'callingimage' => 'cat+plus.jpeg', 'urlimage' => 'cat%2Bplus.jpeg']; yield 'questionmark' => ['image' => 'cat?question.jpeg', 'callingimage' => 'cat?question.jpeg', 'urlimage' => 'cat%3Fquestion.jpeg']; yield 'hash' => ['image' => 'cat#hash.jpeg', 'callingimage' => 'cat#hash.jpeg', 'urlimage' => 'cat%23hash.jpeg']; yield 'cleaned' => ['image' => 'https://www.example.org/images/dogs.jpg', 'callingimage' => '/images/dogs.jpg', 'urlimage' => '/images/dogs.jpg']; }

/**
 * @dataProvider provideImageNames
 */
public function testInvokeFilterMethod($image, $callingimage, $urlimage): void
{
    $this->manager
        ->expects($this->once())
        ->method('getBrowserPath')
        ->with($callingimage, self::FILTER)
        ->willReturn($urlimage);

    $actualPath = $this->runtime->filter($image, self::FILTER);

    $this->assertSame($urlimage, $actualPath);
}`

atesca09 avatar Nov 17 '22 09:11 atesca09