Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override Auto-Inject configuration for Entity #1146

Open
fyrye opened this issue Aug 28, 2020 · 5 comments
Open

Override Auto-Inject configuration for Entity #1146

fyrye opened this issue Aug 28, 2020 · 5 comments

Comments

@fyrye
Copy link
Contributor

fyrye commented Aug 28, 2020

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
Copy link
Collaborator

garak commented Aug 29, 2020

What is the purpose of avoiding auto inject?

@fyrye
Copy link
Contributor Author

fyrye commented Aug 29, 2020

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
Copy link
Collaborator

garak commented Aug 31, 2020

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
Copy link
Contributor Author

fyrye commented Sep 2, 2020

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
Copy link
Collaborator

garak commented Sep 2, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants