JAMES-2557 Improve MailCmdHandle MaybeSender computation

Avoid variable re-affectation and further use method extraction.


Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/7a6d572e
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/7a6d572e
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/7a6d572e

Branch: refs/heads/master
Commit: 7a6d572ea69223c6dae43cc625c76006c7c868fc
Parents: 0bdffec
Author: Benoit Tellier <[email protected]>
Authored: Fri Dec 7 09:29:06 2018 +0700
Committer: Benoit Tellier <[email protected]>
Committed: Tue Dec 11 13:59:20 2018 +0700

----------------------------------------------------------------------
 .../protocols/smtp/core/MailCmdHandler.java     | 66 ++++++++++----------
 1 file changed, 32 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/7a6d572e/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java
----------------------------------------------------------------------
diff --git 
a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java
 
b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java
index 4ba8374..9bce885 100644
--- 
a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java
+++ 
b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java
@@ -27,6 +27,7 @@ import java.util.Map;
 import java.util.StringTokenizer;
 
 import javax.inject.Inject;
+import javax.mail.internet.AddressException;
 
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
@@ -213,47 +214,44 @@ public class MailCmdHandler extends 
AbstractHookableCmdHandler<MailHook> {
                 LOGGER.info("Error parsing sender address: {}: did not start 
and end with < >", sender);
                 return SYNTAX_ERROR;
             }
-            MaybeSender senderAddress = MaybeSender.nullSender();
-
-            if (session.getConfiguration().useAddressBracketsEnforcement()
-                    || (sender.startsWith("<") && sender.endsWith(">"))) {
-                // Remove < and >
-                sender = sender.substring(1, sender.length() - 1);
+            try {
+                MaybeSender senderAddress = 
toMaybeSender(removeBrackets(session, sender));
+                // Store the senderAddress in session map
+                session.setAttachment(SMTPSession.SENDER, senderAddress, 
State.Transaction);
+            } catch (Exception pe) {
+                LOGGER.info("Error parsing sender address: {}", sender, pe);
+                return SYNTAX_ERROR_ADDRESS;
             }
+        }
+        return null;
+    }
 
-            if (sender.length() == 0) {
-                // This is the <> case. Let senderAddress == 
MaybeSender.nullSender()
-            } else {
-
-                if (!sender.contains("@")) {
-                    sender = sender
-                            + "@"
-                            + getDefaultDomain();
-                }
+    private MaybeSender toMaybeSender(String senderAsString) throws 
AddressException {
+        if (senderAsString.length() == 0) {
+            // This is the <> case.
+            return MaybeSender.nullSender();
+        }
+        if (senderAsString.equals("@")) {
+            return MaybeSender.nullSender();
+        }
+        return MaybeSender.of(new MailAddress(
+            appendDefaultDomainIfNeeded(senderAsString)));
+    }
 
-                try {
-                    senderAddress = MaybeSender.of(new MailAddress(sender));
-                } catch (Exception pe) {
-                    LOGGER.info("Error parsing sender address: {}", sender, 
pe);
-                    return SYNTAX_ERROR_ADDRESS;
-                }
-            }
-            if (isNullSender(senderAddress)) {
-                senderAddress = MaybeSender.nullSender();
-            }
-            // Store the senderAddress in session map
-            session.setAttachment(SMTPSession.SENDER, senderAddress, 
State.Transaction);
+    private String removeBrackets(SMTPSession session, String input) {
+        if (session.getConfiguration().useAddressBracketsEnforcement()
+            || (input.startsWith("<") && input.endsWith(">"))) {
+            // Remove < and >
+            return input.substring(1, input.length() - 1);
         }
-        return null;
+        return input;
     }
 
-    private boolean isNullSender(MaybeSender senderAddress) {
-        if (senderAddress.isNullSender()) {
-            return true;
+    private String appendDefaultDomainIfNeeded(String address) {
+        if (!address.contains("@")) {
+            return address + "@" + getDefaultDomain();
         }
-        boolean hasEmptyLocalPart = 
senderAddress.get().getLocalPart().length() == 0;
-        boolean hasEmptyDomainPart = 
senderAddress.get().getDomain().name().length() == 0;
-        return hasEmptyLocalPart && hasEmptyDomainPart;
+        return address;
     }
 
     @Override


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to