Added Local Extension Support With Refactoring and Tests #82
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Detailed Description Of Changes :
src/main/java/com/hubspot/smtp/client/SmtpSession.java :
Line : 102 : I have my doubts over making this
localExtensionsList
Global and whether it should be kept as a volatile variable or not. This global variable is introduced to keep changes at bare minimum so that we don't have to pass it down to mail method. Comments are welcome for editsLines : 243-254 : General acceptor send method with one additional param for
localExtensionsList : List<String>
here we set the earlier declared global variable.
Lines : 309-327 :
sendInternal()
method has been refactored to make it a bit simpler . I checked the flow and found the only difference in the sendAs7Bit vs sendAs8BitMime to be finally in themailCommand
vsmailCommandWith8BitMime
. Rest remains the same. So I merged both the methods mailCommand and mailCommandWith8BitMime to one as mailCommand which reduced the two intermediate methods sendAs8BitMime and sendAs7Bit to one common method calling mailCommand. So now in case we are sending 7 bit / 8 bit appropriate extensions can be handled by mail command method based on checks used earlier and we just need to encode the content if required.7/8bit checks in sendInternal Method () : Assume content to be 8 Bit . If ehlo Response does not support 8 bit then enforce 7 bit conversion. else if it supports then send it through and 8BitMime extension will be introduced by mailCommand method.
Rest all contents are 7 bit and can be handled by default way and we wont introduce 8BitMime extension.
NOTE : this removed
content.get8bitCharacterProportion() == 0
too as we assume rest all are 7 bit except when we send 8 bit content.Lines : 386-390 : removal of the two intermediate methods :
sendAs7Bit
andsendAs8BitMime
to form a common method sendMessageLines : 415-427 : Reduced
mailCommand
andmailCommandWith8BitMime
to one mailCommand method. The check being done to determine 8Bit i.eehloResponse.isSupported(Extension.EIGHT_BIT_MIME)
has been moved here to check whether we need extensionBODY=8BITMIME
or not. RestSMTPUTF
8 checks remain the same i.e applySMTPUTF8
iffSMTPUTF8
is supported by ehlo response and all chars in from and recipients are not all ascii ( implies that there is one or more non ascii character anywhere) using conditionehloResponse.isSupported(Extension.SMTPUTF8) && !(isAllAscii(from) && isAllAscii(recipients))
src/test/java/com/hubspot/smtp/IntegrationTest.java
Lines : 368-402 : Standard test to see if everything is working as intended. Introduced new test method :
itCanSendAnEmailWithLocalExtensions()
Added dummy extensions in localExtensionsList and sent via our new public send method which supports local extensions.
assert if everything works fine.
Had to disable chunking extension as it was not working for me with chunking enabled.