Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Add a time limit option for the SMTP connection #172

Merged
merged 7 commits into from
Jun 6, 2018
Merged

Add a time limit option for the SMTP connection #172

merged 7 commits into from
Jun 6, 2018

Conversation

MatthiasKuehneEllerhold
Copy link
Contributor

As described in the documentation, this PR will add the option for a time limit for the SMTP connection.

If you like the general direction, I'm willing to add unit tests tomorrow.

@Ocramius
Copy link
Member

Ocramius commented Mar 1, 2018

This patch requires extended testing (probably via a clock object)

@MatthiasKuehneEllerhold
Copy link
Contributor Author

MatthiasKuehneEllerhold commented Mar 2, 2018

I've added a test with the help of Reflections.

There may be one issue: this PR sets the current connection to null if the time limit was reached. This means we're relying on lazyLoadConnection() to recreate the connection. But we cant recreate the connection if it was injected through setConnection(). So (maybe / probably) we're getting a different connection if the time limit is up and the connection is recreated using lazyLoadConnection() if it was injected through setConnection() on the first mail.

Example: We're injected a mocked connection in the Unit-Tests and do not rely on lazyLoadConnection().

EDIT: Is targetting the master branch correct?

Possible solution: Implement the auto-reconnect in the Zend\Mail\Protocol\Smtp class instead.

@weierophinney weierophinney merged commit 709e342 into zendframework:master Jun 6, 2018
weierophinney added a commit that referenced this pull request Jun 6, 2018
weierophinney added a commit that referenced this pull request Jun 6, 2018
@weierophinney
Copy link
Member

Thanks, @MatthiasKuehneEllerhold!

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

Successfully merging this pull request may close these issues.

3 participants