[Issue-178] SMTPConnection clean up

[gaol]

Motivation: Fixes #178

Conformance:

Your commits should be signed and you should have signed the Eclipse Contributor Agreement as explained in https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md Please also make sure you adhere to the code style guidelines: https://github.com/vert-x3/wiki/wiki/Vert.x-code-style-guidelines

This PR tries to:

• Changed the Handler<String> commandReplyHandler in SMTPConnection to Handler<AsyncResult<String>> commandReplyHandler, errors during connection initiation and mail transaction will be handled by this handler as well.
• Renamed the errorHandler to exceptionHandler and leave it to be used on any NetSocket exceptions only.
• Removing closing state, which can be checked by closeHandler != null instead
• commandReplyHandler will always be set/get in the connection's context so we don't need to care about multi-threads situations.

It does not affect how MailClient is used.

[vietj]

can you rebase this because now it has merge conflicts thanks @gaol

[gaol]

Just back from PTO, will look into this tomorrow :)

[gaol]

@vietj Done

[gaol]

@vietj I would like this merged before cleaning deprecated callbacks in this component, WDYT ?

[vietj]

@gaol sorry for the miss, can you rebase this branch so we can merge it soon ?

[gaol]

@vietj sure, done :)

[gaol]

@vietj there is a PR #208 which has conflicts with this, I think we can merge this one so that #208 can go on.

[Ladicek]

So this PR does 2 things:

• improves the SMTPConnection class
• replaces callbacks with futures on a few places

I believe the attempt to futurize the client in this PR is not very successful. Only a few places are converted, and often in a suboptimal way. My changes (originally in #208, which I closed now to finish this first) are more thorough and complete.

For that reason, I believe we should first merge the SMTPConnection improvements on their own, and take care of futurization afterwards. I took this PR, removed all changes to files outside of SMTPConnection, and only did minimal updates to the SMTPConnection users to make things work. This is present in a branch here: https://github.com/Ladicek/vertx-mail-client/commits/cleanup-smtp-connection It can be easily seen that SMTPConnection class is almost exactly the same as in this PR (only a few tiny changes). WDYT about merging that branch first?

Then, we can move forward with futurizing. I have a preliminary change in another branch: https://github.com/Ladicek/vertx-mail-client/commits/futurization in case anyone wants to take a look. This branch is mostly equivalent to what I originally had in #208, but based on the new SMTPConnection internals.

[gaol]

@Ladicek sounds good to me, thanks.

[Ladicek]

@gaol Do you want to open a PR with that commit, or maybe force-push it into this PR?

[gaol]

@Ladicek sure, I am closing this one, as suppressed by #209 :)