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

Override Auto-Inject configuration for Entity #1146

Open fyrye opened 4 years ago

fyrye commented 4 years ago

Feature Request

Q A
New Feature yes
RFC yes
BC Break no

Summary

There are certain circumstances where auto-inject is not required for a specific request. For example listing of a Product versus displaying the Order or Invoice of a product.

When auto-inject is enabled, retrieving the Product association of the Order will automatically inject the image. This occurs either as a lazy loaded association ($order->getProducts()) or when eager loaded through a JOIN ORM query.

It would be nice, if there were a method to disable the auto-inject setting for an entity in specific request(s) or operation. Which could be implemented as an annotation or as a service event override.

Example usage:

class OrderController
{

      /**
        * @Vich\DisableAutoInject(class="Entity\Product", property="image")
        */
      public function viewAction(Request $request, VichUploaderService $vich, Order $order)
      {
             /* service event override - annotation method alternative above */
             $vich->disableAutoInject(Product::class, 'image');

             $em = $this->getDoctrine()->getManager();
             $products = $em->getRepository(Product::class)->findBy(['order' => $order]);
             //...
      }
}

From what I understand, the only way to circumvent the auto-inject currently would be to instruct an ORM query to select partial on the affected entity. Which would result in needing to write several use-case queries and having to maintain the queries as the object changes instead of being able to rely solely on the associations and events.

garak commented 4 years ago

What is the purpose of avoiding auto inject?

fyrye commented 4 years ago

To remove the extraneous calls when the subsequent collection of entities that have VIch auto-inject enabled is very large and the files or user-defined inject event listeners are not needed for that request.

In my case, I have a listener that converts the legacy database blob file instances to Vich during the pre-inject event. Ultimately providing the option to override the configuration setting if it is ever needed would be convenient, as opposed to the alternative approaches to accomplish the same.

garak commented 4 years ago

Can't you override vich_uploader.file_injector with your own service? Then listen to pre-inject event and avoid injecting using your conditions.

fyrye commented 4 years ago

It would be nice if it came built-in to VichUploaderBundle, so that a custom service did not need to be implemented by others or across multiple Symfony applications.

I can work on a pull request that doesn't cause a BC break to implement the changes, so that the override feature is available to everyone. The idea is to provide an intermediary from having to specify inject_on_load: false to handle one-off requests.

I consider it a bad practice to override another bundle's services with your own. If there are any changes to the base services it could lead to unforeseen issues. For example, albeit a BC break that would be easily caught and fixed in testing, if the service declaration was updated from vich_uploader.file_injector to Vich\UploaderBundle\Injector\FileInjector to align with current Symfony service declarations.

garak commented 4 years ago

I consider it a bad practice to override another bundle's services with your own.

I disagree. It's even mentioned in documentation

If there are any changes to the base services it could lead to unforeseen issues.

But this can happen with every part of a bundle you override.

Anyway, feel free to propose a PR.