From 6f7ec3e401ae68d9f876f01a629d686658f3395b Mon Sep 17 00:00:00 2001 From: Danny Baumann Date: Sat, 5 Jan 2013 13:20:46 +0100 Subject: [PATCH] Incorporate review comments. --- .../fsck/k9/activity/MessageReference.java | 31 ++++++++++ .../k9/controller/MessagingController.java | 56 ++++--------------- src/com/fsck/k9/mail/Message.java | 45 +++++++++++++++ .../fsck/k9/mail/internet/MimeMessage.java | 30 ++++++++++ src/com/fsck/k9/mail/store/LocalStore.java | 51 +---------------- .../k9/service/NotificationActionService.java | 24 +------- 6 files changed, 122 insertions(+), 115 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageReference.java b/src/com/fsck/k9/activity/MessageReference.java index 493adfd33..990d99774 100644 --- a/src/com/fsck/k9/activity/MessageReference.java +++ b/src/com/fsck/k9/activity/MessageReference.java @@ -1,11 +1,17 @@ package com.fsck.k9.activity; +import android.content.Context; import android.os.Parcel; import android.os.Parcelable; import android.util.Log; + +import com.fsck.k9.Account; import com.fsck.k9.K9; +import com.fsck.k9.Preferences; import com.fsck.k9.helper.Utility; import com.fsck.k9.mail.Flag; +import com.fsck.k9.mail.Folder; +import com.fsck.k9.mail.Message; import com.fsck.k9.mail.MessagingException; import java.util.StringTokenizer; @@ -122,6 +128,31 @@ public class MessageReference implements Parcelable { '}'; } + public Message restoreToLocalMessage(Context context) { + try { + Account account = Preferences.getPreferences(context).getAccount(accountUuid); + if (account != null) { + Folder folder = account.getLocalStore().getFolder(folderName); + if (folder != null) { + Message message = folder.getMessage(uid); + if (message != null) { + return message; + } else { + Log.d(K9.LOG_TAG, "Could not restore message, uid " + uid + " is unknown."); + } + } else { + Log.d(K9.LOG_TAG, "Could not restore message, folder " + folderName + " is unknown."); + } + } else { + Log.d(K9.LOG_TAG, "Could not restore message, account " + accountUuid + " is unknown."); + } + } catch (MessagingException e) { + Log.w(K9.LOG_TAG, "Could not retrieve message for reference.", e); + } + + return null; + } + public static final Creator CREATOR = new Creator() { @Override public MessageReference createFromParcel(Parcel source) { diff --git a/src/com/fsck/k9/controller/MessagingController.java b/src/com/fsck/k9/controller/MessagingController.java index 52a439be9..6576aca93 100644 --- a/src/com/fsck/k9/controller/MessagingController.java +++ b/src/com/fsck/k9/controller/MessagingController.java @@ -200,9 +200,6 @@ public class MessagingController implements Runnable { // Key is accountUuid:folderName:messageUid , value is unimportant private ConcurrentHashMap deletedUids = new ConcurrentHashMap(); - // maximum length of the message preview text returned by getMessagePreview() - private final static int MAX_PREVIEW_LENGTH = 300; - private static class NotificationData { int unreadBeforeNotification; LinkedList messages; // newest one first @@ -4506,40 +4503,17 @@ public class MessagingController implements Runnable { } private static TextAppearanceSpan sEmphasizedSpan; - private void ensureEmphasizedSpan(Context context) { + private TextAppearanceSpan getEmphasizedSpan(Context context) { if (sEmphasizedSpan == null) { sEmphasizedSpan = new TextAppearanceSpan(context, R.style.TextAppearance_StatusBar_EventContent_Emphasized); } + return sEmphasizedSpan; } private CharSequence getMessagePreview(Context context, Message message) { CharSequence subject = getMessageSubject(context, message); - String snippet = null; - - try { - Part part = MimeUtility.findFirstPartByMimeType(message, "text/html"); - if (part != null) { - // We successfully found an HTML part; do the necessary character set decoding. - snippet = MimeUtility.getTextFromPart(part); - if (snippet != null) { - snippet = HtmlConverter.htmlToText(snippet); - } - } - if (snippet == null) { - // no HTML part -> try and get a text part. - part = MimeUtility.findFirstPartByMimeType(message, "text/plain"); - if (part != null) { - snippet = MimeUtility.getTextFromPart(part); - } - } - } catch (MessagingException e) { - Log.d(K9.LOG_TAG, "Could not extract message preview", e); - } - - if (snippet != null && snippet.length() > MAX_PREVIEW_LENGTH) { - snippet = snippet.substring(0, MAX_PREVIEW_LENGTH); - } + String snippet = message.getPreview(); if (TextUtils.isEmpty(subject)) { return snippet; @@ -4552,14 +4526,12 @@ public class MessagingController implements Runnable { preview.append('\n'); preview.append(snippet); - ensureEmphasizedSpan(context); - preview.setSpan(sEmphasizedSpan, 0, subject.length(), 0); + preview.setSpan(getEmphasizedSpan(context), 0, subject.length(), 0); return preview; } private CharSequence buildMessageSummary(Context context, CharSequence sender, CharSequence subject) { - if (sender == null) { return subject; } @@ -4569,8 +4541,7 @@ public class MessagingController implements Runnable { summary.append(" "); summary.append(subject); - ensureEmphasizedSpan(context); - summary.setSpan(sEmphasizedSpan, 0, sender.length(), 0); + summary.setSpan(getEmphasizedSpan(context), 0, sender.length(), 0); return summary; } @@ -4587,17 +4558,14 @@ public class MessagingController implements Runnable { return Build.VERSION.SDK_INT >= 16; } - private Message findNewestMessageForNotificationLocked(Account account, NotificationData data) { - if (data.messages.size() > 0) { + private Message findNewestMessageForNotificationLocked(Context context, + Account account, NotificationData data) { + if (!data.messages.isEmpty()) { return data.messages.get(0); } - if (data.droppedMessages.size() > 0) { - MessageReference ref = data.droppedMessages.get(0); - try { - return account.getLocalStore().getFolder(ref.folderName).getMessage(ref.uid); - } catch (MessagingException e) { - } + if (!data.droppedMessages.isEmpty()) { + return data.droppedMessages.get(0).restoreToLocalMessage(context); } return null; @@ -4620,7 +4588,7 @@ public class MessagingController implements Runnable { if (message == null) { /* this can happen if a message we previously notified for is read or deleted remotely */ - message = findNewestMessageForNotificationLocked(account, data); + message = findNewestMessageForNotificationLocked(context, account, data); updateSilently = true; if (message == null) { return; @@ -4676,7 +4644,7 @@ public class MessagingController implements Runnable { getMessageSender(context, account, m), getMessageSubject(context, m))); } - if (data.droppedMessages.size() > 0) { + if (!data.droppedMessages.isEmpty()) { style.setSummaryText(context.getString( R.string.notification_additional_messages, data.droppedMessages.size())); } diff --git a/src/com/fsck/k9/mail/Message.java b/src/com/fsck/k9/mail/Message.java index 3d31f0839..d34f30108 100644 --- a/src/com/fsck/k9/mail/Message.java +++ b/src/com/fsck/k9/mail/Message.java @@ -148,7 +148,52 @@ public abstract class Message implements Part, Body { public abstract String getPreview(); public abstract boolean hasAttachments(); + /* + * calculateContentPreview + * Takes a plain text message body as a string. + * Returns a message summary as a string suitable for showing in a message list + * + * A message summary should be about the first 160 characters + * of unique text written by the message sender + * Quoted text, "On $date" and so on will be stripped out. + * All newlines and whitespace will be compressed. + * + */ + public static String calculateContentPreview(String text) { + if (text == null) { + return null; + } + // Only look at the first 8k of a message when calculating + // the preview. This should avoid unnecessary + // memory usage on large messages + if (text.length() > 8192) { + text = text.substring(0, 8192); + } + + // try to remove lines of dashes in the preview + text = text.replaceAll("(?m)^----.*?$", ""); + // remove quoted text from the preview + text = text.replaceAll("(?m)^[#>].*$", ""); + // Remove a common quote header from the preview + text = text.replaceAll("(?m)^On .*wrote.?$", ""); + // Remove a more generic quote header from the preview + text = text.replaceAll("(?m)^.*\\w+:$", ""); + // Remove horizontal rules. + text = text.replaceAll("\\s*([-=_]{30,}+)\\s*", " "); + + // URLs in the preview should just be shown as "..." - They're not + // clickable and they usually overwhelm the preview + text = text.replaceAll("https?://\\S+", "..."); + // Don't show newlines in the preview + text = text.replaceAll("(\\r|\\n)+", " "); + // Collapse whitespace in the preview + text = text.replaceAll("\\s+", " "); + // Remove any whitespace at the beginning and end of the string. + text = text.trim(); + + return (text.length() <= 512) ? text : text.substring(0, 512); + } public void delete(String trashFolderName) throws MessagingException {} diff --git a/src/com/fsck/k9/mail/internet/MimeMessage.java b/src/com/fsck/k9/mail/internet/MimeMessage.java index 2777420de..004cef186 100644 --- a/src/com/fsck/k9/mail/internet/MimeMessage.java +++ b/src/com/fsck/k9/mail/internet/MimeMessage.java @@ -23,6 +23,10 @@ import org.apache.james.mime4j.stream.BodyDescriptor; import org.apache.james.mime4j.stream.Field; import org.apache.james.mime4j.stream.MimeConfig; +import android.util.Log; + +import com.fsck.k9.K9; +import com.fsck.k9.helper.HtmlConverter; import com.fsck.k9.mail.Address; import com.fsck.k9.mail.Body; import com.fsck.k9.mail.BodyPart; @@ -591,6 +595,32 @@ public class MimeMessage extends Message { } public String getPreview() { + String preview = null; + + try { + Part part = MimeUtility.findFirstPartByMimeType(this, "text/html"); + if (part != null) { + // We successfully found an HTML part; do the necessary character set decoding. + preview = MimeUtility.getTextFromPart(part); + if (preview != null) { + preview = HtmlConverter.htmlToText(preview); + } + } + if (preview == null) { + // no HTML part -> try and get a text part. + part = MimeUtility.findFirstPartByMimeType(this, "text/plain"); + if (part != null) { + preview = MimeUtility.getTextFromPart(part); + } + } + } catch (MessagingException e) { + Log.d(K9.LOG_TAG, "Could not extract message preview", e); + } + + if (preview != null) { + return calculateContentPreview(preview); + } + return ""; } diff --git a/src/com/fsck/k9/mail/store/LocalStore.java b/src/com/fsck/k9/mail/store/LocalStore.java index 6e7a6afc2..4db8edd25 100644 --- a/src/com/fsck/k9/mail/store/LocalStore.java +++ b/src/com/fsck/k9/mail/store/LocalStore.java @@ -2403,7 +2403,7 @@ public class LocalStore extends Store implements Serializable { html = HtmlConverter.convertEmoji2Img(container.html); } - String preview = calculateContentPreview(text); + String preview = Message.calculateContentPreview(text); try { ContentValues cv = new ContentValues(); @@ -2501,7 +2501,7 @@ public class LocalStore extends Store implements Serializable { String text = container.text; String html = HtmlConverter.convertEmoji2Img(container.html); - String preview = calculateContentPreview(text); + String preview = Message.calculateContentPreview(text); try { db.execSQL("UPDATE messages SET " @@ -3010,53 +3010,6 @@ public class LocalStore extends Store implements Serializable { } } - /* - * calculateContentPreview - * Takes a plain text message body as a string. - * Returns a message summary as a string suitable for showing in a message list - * - * A message summary should be about the first 160 characters - * of unique text written by the message sender - * Quoted text, "On $date" and so on will be stripped out. - * All newlines and whitespace will be compressed. - * - */ - public String calculateContentPreview(String text) { - if (text == null) { - return null; - } - - // Only look at the first 8k of a message when calculating - // the preview. This should avoid unnecessary - // memory usage on large messages - if (text.length() > 8192) { - text = text.substring(0, 8192); - } - - // try to remove lines of dashes in the preview - text = text.replaceAll("(?m)^----.*?$", ""); - // remove quoted text from the preview - text = text.replaceAll("(?m)^[#>].*$", ""); - // Remove a common quote header from the preview - text = text.replaceAll("(?m)^On .*wrote.?$", ""); - // Remove a more generic quote header from the preview - text = text.replaceAll("(?m)^.*\\w+:$", ""); - // Remove horizontal rules. - text = text.replaceAll("\\s*([-=_]{30,}+)\\s*", " "); - - // URLs in the preview should just be shown as "..." - They're not - // clickable and they usually overwhelm the preview - text = text.replaceAll("https?://\\S+", "..."); - // Don't show newlines in the preview - text = text.replaceAll("(\\r|\\n)+", " "); - // Collapse whitespace in the preview - text = text.replaceAll("\\s+", " "); - // Remove any whitespace at the beginning and end of the string. - text = text.trim(); - - return (text.length() <= 512) ? text : text.substring(0, 512); - } - @Override public boolean isInTopGroup() { return mInTopGroup; diff --git a/src/com/fsck/k9/service/NotificationActionService.java b/src/com/fsck/k9/service/NotificationActionService.java index fc93c4d8e..95971f36f 100644 --- a/src/com/fsck/k9/service/NotificationActionService.java +++ b/src/com/fsck/k9/service/NotificationActionService.java @@ -56,26 +56,6 @@ public class NotificationActionService extends CoreService { return i; } - private static Message getMessageFromRef(final Account account, MessageReference ref) { - try { - Folder folder = account.getLocalStore().getFolder(ref.folderName); - if (folder != null) { - Message message = folder.getMessage(ref.uid); - if (message != null) { - return message; - } else { - Log.w(K9.LOG_TAG, "Could not find message for notification action."); - } - } else { - Log.w(K9.LOG_TAG, "Could not find folder for notification action."); - } - } catch (MessagingException e) { - Log.w(K9.LOG_TAG, "Could not retrieve message for reference.", e); - } - - return null; - } - @Override public int startService(Intent intent, int startId) { if (K9.DEBUG) @@ -103,7 +83,7 @@ public class NotificationActionService extends CoreService { ArrayList messages = new ArrayList(); for (MessageReference ref : refs) { - Message m = getMessageFromRef(account, ref); + Message m = ref.restoreToLocalMessage(this); if (m != null) { messages.add(m); } @@ -115,7 +95,7 @@ public class NotificationActionService extends CoreService { Log.i(K9.LOG_TAG, "NotificationActionService initiating reply"); MessageReference ref = (MessageReference) intent.getParcelableExtra(EXTRA_MESSAGE); - Message message = getMessageFromRef(account, ref); + Message message = ref.restoreToLocalMessage(this); if (message != null) { Intent i = MessageCompose.getActionReplyIntent(this, account, message, false, null); i.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);