search

dustin10 / VichUploaderBundle

A simple Symfony bundle to ease file uploads with ORM entities and ODM documents.
MIT License
1.85k stars 519 forks source link

Fix sf5.4 compat #1240 #1243

Closed tobias-93 closed 2 years ago

tobias-93 commented 2 years ago

This should fix the compatibility issue of the File class when using this bundle with Symfony 5.4 (#1240). I had to dive quite deep to find this, so I can imagine the fixes don't seem logical. The issue originated in changes in the DoctrineBridge: https://github.com/symfony/symfony/commit/136c4a314c007092bd675eead8f78060b90948ff. It started using the Bundle path from the kernel instead of the path where the Bundle.php is present (https://github.com/symfony/symfony/commit/136c4a314c007092bd675eead8f78060b90948ff#diff-0f981a325d8c9eea80a258c36237861d1d00b0fb62fb04f2488bf2165776c94eR90). Usually those are the same, but for this bundle it was overwritten in src/VichUploaderBundle.php to use the parent directory of that file. Then the autodetection of the AbstractDoctrineExtension could not find our Entity directory and (wrongly) assumed no configuration was necessary. So, by fixing the path of the bundle, this issue was fixed.

Another issue appeared then, when Twig could no longer find the templates. For bundles these need to be placed in Resources/views, instead of the ../templates as used in this bundle. So, after moving this issue was resolved. @garak you changed the bundle structure in #1058 and introduced the change of the Bundle path, was there another reason or issue you were doing this?

At last, Doctrine autoconfiguration wrongly assumed this bundle to be using attributes because it searches for something like @Entity or @ORM\Entity. Since our only 'Entity' is only an Embeddable entity, it was not found, leading to Doctrine complaining that it is not a valid entity. I added the attributes for this entity, which should not matter for PHP 7 as they are interpreted as comments (I did not verify that). After that our application worked fine again. Since we don't use all features of the bundle, it might still be issues persist, so I encourage you to test this thoroughly.

tobias-93 commented 2 years ago

@garak The issues are related, so without both fixes the bundle will not work with SF 5.4... Doesn't make sense to me to split those as neither will provide a working bundle, I already split the non-related changes in separate comments.

garak commented 2 years ago

@garak The issues are related, so without both fixes the bundle will not work with SF 5.4... Doesn't make sense to me to split those as neither will provide a working bundle, I already split the non-related changes in separate comments.

I have to disagree on this. The Doctrine issue is confirmed (and it has a "real" open issue, already confirmed). The other one about twig is only speculative.

garak commented 2 years ago

For a reference: you can read on the docs that the current directory structure is fine, so the problem with Twig is likely due to a misconfinguration.

tobias-93 commented 2 years ago

@garak The issues are related, so without both fixes the bundle will not work with SF 5.4... Doesn't make sense to me to split those as neither will provide a working bundle, I already split the non-related changes in separate comments.

I have to disagree on this. The Doctrine issue is confirmed (and it has a "real" open issue, already confirmed). The other one about twig is only speculative.

There is nothing speculative on this, if the getPath method in the VichUploaderBundle class is removed (which is necessary to solve the Doctrine issue, see first part of my explanation), Twig can no longer find the templates (see second part of my explanation). So these are not separate issues.

For a reference: you can read on the docs that the current directory structure is fine, so the problem with Twig is likely due to a misconfinguration.

Be that as it may, all bundles we use with templates use the Resources/views directory (sometimes inside the src directory). That is probably for a reason.

garak commented 2 years ago

Be that as it may, all bundles we use with templates use the Resources/views directory (sometimes inside the src directory). That is probably for a reason.

The reason is called "legacy". By the way, I'm using this bundle with Symfony 5.4 without any problems (I don't use the embedded object)

tobias-93 commented 2 years ago

@garak I propose a different approach in #1244, let me know if you like that one better.