Changsung Kim 2015-03-20
fixes logics splitting and sending notification emails with recipient limit value.
* Issue

When `application.notification.bymail.hideAddress` is true, a default recipient is added to `to header`.
But previous codes does not consider it so notification mails, `application.notification.bymail.recipientLimit` + 1, are sent.

* Solution

The changed codes decide amount of notification mails to be sent with `application.notification.bymail.recipientLimit` and recipients in `to header`.

Private-issue: 2206
@3781a7369da66b7cf2dece9aec488da31657b6c9
 
app/models/MailRecipient.java (added)
+++ app/models/MailRecipient.java
@@ -0,0 +1,40 @@
+/**
+ * Yobi, Project Hosting SW
+ *
+ * Copyright 2015 NAVER Corp.
+ * http://yobi.io
+ *
+ * @Author Changsung Kim
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package models;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+/**
+ * A model for mail recipients of notification mail.
+ */
+public class MailRecipient {
+    @Nonnull
+    final String email;
+    @Nullable
+    final String name;
+
+    public MailRecipient(String email, String name) {
+        this.email = email;
+        this.name = name;
+    }
+
+}
app/models/NotificationMail.java
--- app/models/NotificationMail.java
+++ app/models/NotificationMail.java
@@ -20,6 +20,7 @@
  */
 package models;
 
+import com.google.common.collect.Lists;
 import info.schleichardt.play2.mailplugin.Mailer;
 import mailbox.EmailAddressWithDetail;
 import models.enumeration.ResourceType;
@@ -238,6 +239,12 @@
             return;
         }
 
+        final int partialRecipientSize = getPartialRecipientSize(receivers);
+
+        if (partialRecipientSize <= 0) {
+            return;
+        }
+
         HashMap<String, List<User>> usersByLang = new HashMap<>();
 
         for (User receiver : receivers) {
@@ -251,24 +258,52 @@
         }
 
         for (String langCode : usersByLang.keySet()) {
-            List<User> users = usersByLang.get(langCode);
+            List<List<User>> subLists = Lists.partition(usersByLang.get(langCode), partialRecipientSize);
 
-            if (recipientLimit == RECIPIENT_NO_LIMIT) {
-                sendMail(event, users, langCode);
-            } else {
-                int usersSize = users.size();
-                int fromIndex, toIndex = 0;
-                while (toIndex < usersSize) {
-                    fromIndex = toIndex;
-                    toIndex = Math.min(toIndex + recipientLimit, usersSize);
-                    sendMail(event, users.subList(fromIndex, toIndex), langCode);
-                }
+            for (List<User> list : subLists) {
+                Set<MailRecipient> toList = getToList(list);
+                Set<MailRecipient> bccList = getBccList(list);
+                sendMail(event, toList, bccList, langCode);
             }
         }
     }
 
-    private static void sendMail(NotificationEvent event, List<User> receivers, String langCode) {
-        if (receivers.isEmpty()) {
+    private static int getPartialRecipientSize(Set<User> receivers) {
+        if (recipientLimit == RECIPIENT_NO_LIMIT) {
+            return receivers.size();
+        }
+
+        return (hideAddress) ? recipientLimit - getDefaultToList().size() : recipientLimit;
+    }
+
+    private static Set<MailRecipient> getToList(List<User> users) {
+        return (hideAddress) ? getDefaultToList() : getMailRecipientsFromUsers(users);
+    }
+
+    private static Set<MailRecipient> getBccList(List<User> users) {
+        return (hideAddress) ? getMailRecipientsFromUsers(users) : new HashSet<MailRecipient>();
+    }
+
+    private static Set<MailRecipient> getDefaultToList() {
+        Set<MailRecipient> list = new HashSet<>();
+
+        list.add(new MailRecipient(Config.getEmailFromSmtp(), Config.getSiteName()));
+
+        return list;
+    }
+
+    private static Set<MailRecipient> getMailRecipientsFromUsers(List<User> users) {
+        Set<MailRecipient> list = new HashSet<>();
+
+        for (User user : users) {
+            list.add(new MailRecipient(user.email, user.name));
+        }
+
+        return list;
+    }
+
+    private static void sendMail(NotificationEvent event, Set<MailRecipient> toList, Set<MailRecipient> bccList, String langCode) {
+        if (toList.isEmpty()) {
             return;
         }
 
@@ -284,16 +319,12 @@
                 acceptsReply = true;
             }
 
-            for (User receiver : receivers) {
-                if (hideAddress) {
-                    email.addBcc(receiver.email, receiver.name);
-                } else {
-                    email.addTo(receiver.email, receiver.name);
-                }
+            for (MailRecipient recipient : toList) {
+                email.addTo(recipient.email, recipient.name);
             }
 
-            if (email.getToAddresses().isEmpty()) {
-                email.addTo(Config.getEmailFromSmtp(), utils.Config.getSiteName());
+            for (MailRecipient recipient : bccList) {
+                email.addBcc(recipient.email, recipient.name);
             }
 
             // FIXME: gmail은 From과 To에 같은 주소가 있으면 reply-to를 무시한다.
build.sbt
--- build.sbt
+++ build.sbt
@@ -41,7 +41,8 @@
   "org.jsoup" % "jsoup" % "1.7.2",
   "com.googlecode.juniversalchardet" % "juniversalchardet" % "1.0.3",
   "org.mockito" % "mockito-all" % "1.9.0" % "test",
-  "com.github.zafarkhaja" % "java-semver" % "0.7.2"
+  "com.github.zafarkhaja" % "java-semver" % "0.7.2",
+  "com.google.guava" % "guava" % "18.0"
 )
 
 val projectSettings = Seq(
Add a comment
List