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

configure Swiftmailer handlers transports depending on the current conta... #66

Merged
merged 1 commit into from Feb 19, 2014

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jan 21, 2014

...iner configuration

This should fix #65. As you can't determine reliably inside the DI extension whether or not a container alias does exist, I added a compiler pass that does this work.

{
$definitions = $container->getDefinitions();

foreach ($definitions as $id => &$definition) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to use a reference here

  • it is an object, so it will already be modified in place
  • using a reference will force PHP to copy the reference to the array of definitions here, as you cannot share the same zval between non-reference variables (in the container builder) and reference variables.

Using references just to try being smarter than PHP at handling memory often ends up being worse.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right of course. Don't know why I did this.

@Koc
Copy link

Koc commented Jan 29, 2014

Would it work with multiple configured mailers?

@xabbuh
Copy link
Member Author

xabbuh commented Jan 30, 2014

Yes, it should. swiftmailer.transport is always set for the default mailer and swiftmailer.transport.real is always set if spool is enabled for it.

*/
public function process(ContainerBuilder $container)
{
foreach ($container->getDefinitions() as $id => $definition) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe if the extensoin would set a parameter with the swiftmailer handler ids in the container we could just read it out here, delete it, and loop over thoes ids instead of looping over everything. That sounds faster and more robust too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, @Seldaek. Your suggestion sounds reasonable. I think I can get my hands back on this next weekend.

@xabbuh
Copy link
Member Author

xabbuh commented Feb 18, 2014

@Seldaek I updated the implementation and added tests for the compiler pass.

@Seldaek
Copy link
Member

Seldaek commented Feb 19, 2014

Thanks, looks good now as far as I understand the problem.

Seldaek added a commit that referenced this pull request Feb 19, 2014
configure Swiftmailer handlers transports depending on the current conta...
@Seldaek Seldaek merged commit 71ad844 into symfony:master Feb 19, 2014
@xabbuh xabbuh deleted the issue-65 branch February 19, 2014 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error if spool config value not set
4 participants