search

zenstruck / redirect-bundle

Store redirects for your site and keeps statistics on redirects and 404 errors.
MIT License
24 stars 8 forks source link

Extra EntityManager service kills EntityManager autowiring #15

Closed weaverryan closed 7 years ago

weaverryan commented 7 years ago

Hi buddy!

We were just upgrading to the latest version of the bundle, and suddenly our EntityManager couldn't be autowired into any services anymore! The problem is that this bundle introduces a second service, which uses this class: https://github.com/kbond/ZenstruckRedirectBundle/blob/7504b70a46ab663273a1437bbbdbe37091591a65/src/Resources/config/orm.xml#L8

I see that the purpose is to be able to control which entity manager is used. Could this be done in a simpler way with an alias? For example, don't actually create a service called zenstruck_redirect.entity_manager, simply set an alias in the container so that zenstruck_redirect.entity_manager is an alias to the correct entity manager service (e.g. doctrine.orm.default_entity_manager where default changes to be the model_manager_name config). This would avoid needing to add the additional service to the container, which causes the issue.

And to really try to tug at your heart-strings... it's even technically (though barely) more performant! The current method requires an extra call to $container->get('doctrine')->getManager() to get the service, instead of re-using the existing. Ok, this last reason is CRAP, and would save only 1ms every 1000 requests... but I'm just trying to sweeten the deal ;).

Cheers!

weaverryan commented 7 years ago

Also, I just asked Beckett, he's totally 👍 for my suggested change.

kbond commented 7 years ago

Hey Ryan!

I can't quite remember out why I did it that way... My guess is I was following the conventions from another bundle.

I'll take a look and get this fixed. Like you say, an alias seems like the best solution.

K