Change the way we're tracking attachment download progress

k9mail-library/src/main/java/com/fsck/k9/mail/AttachmentProgressCallback.java

@@ -1,5 +0,0 @@
-package com.fsck.k9.mail;
-
-public interface AttachmentProgressCallback {
-    void onUpdate(int progress);
-}

k9mail-library/src/main/java/com/fsck/k9/mail/BodyFactory.java

@@ -0,0 +1,10 @@
+package com.fsck.k9.mail;
+
+
+import java.io.IOException;
+import java.io.InputStream;
+
+
+public interface BodyFactory {
+    Body createBody(String contentTransferEncoding, String contentType, InputStream inputStream) throws IOException;
+}

k9mail-library/src/main/java/com/fsck/k9/mail/DefaultBodyFactory.java

@@ -0,0 +1,43 @@
+package com.fsck.k9.mail;
+
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+import com.fsck.k9.mail.internet.BinaryTempFileBody;
+import com.fsck.k9.mail.internet.BinaryTempFileMessageBody;
+import com.fsck.k9.mail.internet.MimeUtility;
+import org.apache.commons.io.IOUtils;
+import org.apache.james.mime4j.util.MimeUtil;
+
+
+public class DefaultBodyFactory implements BodyFactory {
+    public Body createBody(String contentTransferEncoding, String contentType, InputStream inputStream)
+            throws IOException {
+
+        if (contentTransferEncoding != null) {
+            contentTransferEncoding = MimeUtility.getHeaderParameter(contentTransferEncoding, null);
+        }
+
+        final BinaryTempFileBody tempBody;
+        if (MimeUtil.isMessage(contentType)) {
+            tempBody = new BinaryTempFileMessageBody(contentTransferEncoding);
+        } else {
+            tempBody = new BinaryTempFileBody(contentTransferEncoding);
+        }
+
+        OutputStream outputStream = tempBody.getOutputStream();
+        try {
+            copyData(inputStream, outputStream);
+        } finally {
+            outputStream.close();
+        }
+
+        return tempBody;
+    }
+
+    protected void copyData(InputStream inputStream, OutputStream outputStream) throws IOException {
+        IOUtils.copy(inputStream, outputStream);
+    }
+}

k9mail-library/src/main/java/com/fsck/k9/mail/Folder.java

@@ -127,9 +127,8 @@ public abstract class Folder<T extends Message> {
     public abstract void fetch(List<T> messages, FetchProfile fp,
                                MessageRetrievalListener<T> listener) throws MessagingException;
 
-    public void fetchPart(Message message, Part part,
+    public void fetchPart(Message message, Part part, MessageRetrievalListener<Message> listener,
-                          MessageRetrievalListener<Message> listener,
+            BodyFactory bodyFactory) throws MessagingException {
-                          AttachmentProgressCallback progressCallback) throws MessagingException {
         // This is causing trouble. Disabled for now. See issue 1733
         //throw new RuntimeException("fetchPart() not implemented.");
 

k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java

@@ -19,7 +19,9 @@ import android.support.annotation.NonNull;
 
 import com.fsck.k9.mail.Address;
 import com.fsck.k9.mail.Body;
+import com.fsck.k9.mail.BodyFactory;
 import com.fsck.k9.mail.BodyPart;
+import com.fsck.k9.mail.DefaultBodyFactory;
 import com.fsck.k9.mail.Message;
 import com.fsck.k9.mail.MessagingException;
 import com.fsck.k9.mail.Multipart;
@@ -106,7 +108,7 @@ public class MimeMessage extends Message {
         // REALLY long References: headers
         parserConfig.setMaxHeaderCount(-1); // Disable the check for header count.
         MimeStreamParser parser = new MimeStreamParser(parserConfig);
-        parser.setContentHandler(new MimeMessageBuilder());
+        parser.setContentHandler(new MimeMessageBuilder(new DefaultBodyFactory()));
         if (recurse) {
             parser.setRecurse();
         }
@@ -520,8 +522,10 @@ public class MimeMessage extends Message {
 
     private class MimeMessageBuilder implements ContentHandler {
         private final LinkedList<Object> stack = new LinkedList<>();
+        private final BodyFactory bodyFactory;
 
-        public MimeMessageBuilder() {
+        public MimeMessageBuilder(BodyFactory bodyFactory) {
+            this.bodyFactory = bodyFactory;
         }
 
         private void expect(Class<?> c) {
@@ -576,7 +580,7 @@ public class MimeMessage extends Message {
         @Override
         public void body(BodyDescriptor bd, InputStream in) throws IOException, MimeException {
             expect(Part.class);
-            Body body = MimeUtility.createBody(in, bd.getTransferEncoding(), bd.getMimeType(), null);
+            Body body = bodyFactory.createBody(bd.getTransferEncoding(), bd.getMimeType(), in);
             ((Part)stack.peek()).setBody(body);
         }
 

k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeUtility.java

@@ -4,15 +4,11 @@ package com.fsck.k9.mail.internet;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.OutputStream;
 import java.util.Locale;
-import java.util.Timer;
-import java.util.TimerTask;
 import java.util.regex.Pattern;
 
 import android.support.annotation.NonNull;
 
-import com.fsck.k9.mail.AttachmentProgressCallback;
 import com.fsck.k9.mail.Body;
 import com.fsck.k9.mail.BodyPart;
 import com.fsck.k9.mail.Message;
@@ -20,8 +16,6 @@ import com.fsck.k9.mail.MessagingException;
 import com.fsck.k9.mail.Multipart;
 import com.fsck.k9.mail.Part;
 
-import org.apache.commons.io.IOUtils;
-import org.apache.commons.io.output.CountingOutputStream;
 import org.apache.james.mime4j.codec.Base64InputStream;
 import org.apache.james.mime4j.codec.QuotedPrintableInputStream;
 import org.apache.james.mime4j.util.MimeUtil;
@@ -990,44 +984,6 @@ public class MimeUtility {
         return isSameMimeType(mimeType, DEFAULT_ATTACHMENT_MIME_TYPE);
     }
 
-    public static Body createBody(InputStream in, String contentTransferEncoding, String contentType, final AttachmentProgressCallback progressCallback)
-            throws IOException {
-
-        if (contentTransferEncoding != null) {
-            contentTransferEncoding = MimeUtility.getHeaderParameter(contentTransferEncoding, null);
-        }
-
-        final BinaryTempFileBody tempBody;
-        if (MimeUtil.isMessage(contentType)) {
-            tempBody = new BinaryTempFileMessageBody(contentTransferEncoding);
-        } else {
-            tempBody = new BinaryTempFileBody(contentTransferEncoding);
-        }
-
-        OutputStream out = tempBody.getOutputStream();
-        final CountingOutputStream countingOutputStream = new CountingOutputStream(out);
-        Timer timer = null;
-        try {
-            if (progressCallback != null) {
-                timer = new Timer();
-                timer.scheduleAtFixedRate(new TimerTask() {
-                    @Override
-                    public void run() {
-                        progressCallback.onUpdate((int) countingOutputStream.getCount());
-                    }
-                }, 0, 50);
-            }
-            IOUtils.copy(in, countingOutputStream);
-        } finally {
-            if (timer != null) {
-                timer.cancel();
-            }
-            out.close();
-        }
-
-        return tempBody;
-    }
-
     /**
      * Get decoded contents of a body.
      * <p/>

k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/FetchPartCallback.java

@@ -3,42 +3,31 @@ package com.fsck.k9.mail.store.imap;
 
 import java.io.IOException;
 
-import com.fsck.k9.mail.AttachmentProgressCallback;
+import com.fsck.k9.mail.BodyFactory;
 import com.fsck.k9.mail.Part;
 import com.fsck.k9.mail.filter.FixedLengthInputStream;
 import com.fsck.k9.mail.internet.MimeHeader;
-import com.fsck.k9.mail.internet.MimeUtility;
 
 
 class FetchPartCallback implements ImapResponseCallback {
-    private Part mPart;
+    private final Part part;
-    private AttachmentProgressCallback attachmentProgressCallback;
+    private final BodyFactory bodyFactory;
 
-    FetchPartCallback(Part part) {
-        mPart = part;
-    }
 
-    FetchPartCallback(Part part, AttachmentProgressCallback attachmentProgressCallback) {
+    FetchPartCallback(Part part, BodyFactory bodyFactory) {
-        mPart = part;
+        this.part = part;
-        this.attachmentProgressCallback = attachmentProgressCallback;
+        this.bodyFactory = bodyFactory;
     }
 
     @Override
     public Object foundLiteral(ImapResponse response, FixedLengthInputStream literal) throws IOException {
-        if (response.getTag() == null &&
+        if (response.getTag() == null && ImapResponseParser.equalsIgnoreCase(response.get(1), "FETCH")) {
-                ImapResponseParser.equalsIgnoreCase(response.get(1), "FETCH")) {
             //TODO: check for correct UID
 
-            String contentTransferEncoding = mPart
+            String contentTransferEncoding = part.getHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING)[0];
-                    .getHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING)[0];
+            String contentType = part.getHeader(MimeHeader.HEADER_CONTENT_TYPE)[0];
-            String contentType = mPart
-                    .getHeader(MimeHeader.HEADER_CONTENT_TYPE)[0];
 
-            if (attachmentProgressCallback != null) {
+            return bodyFactory.createBody(contentTransferEncoding, contentType, literal);
-                return MimeUtility.createBody(literal, contentTransferEncoding, contentType, attachmentProgressCallback);
-            }
-
-            return MimeUtility.createBody(literal, contentTransferEncoding, contentType, null);
         }
         return null;
     }

k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.java

@@ -18,8 +18,8 @@ import java.util.concurrent.ConcurrentHashMap;
 
 import android.text.TextUtils;
 
-import com.fsck.k9.mail.AttachmentProgressCallback;
 import com.fsck.k9.mail.Body;
+import com.fsck.k9.mail.BodyFactory;
 import com.fsck.k9.mail.FetchProfile;
 import com.fsck.k9.mail.Flag;
 import com.fsck.k9.mail.Folder;
@@ -780,8 +780,8 @@ class ImapFolder extends Folder<ImapMessage> {
     }
 
     @Override
-    public void fetchPart(Message message, Part part, MessageRetrievalListener<Message> listener, AttachmentProgressCallback progressCallback)
+    public void fetchPart(Message message, Part part, MessageRetrievalListener<Message> listener,
-            throws MessagingException {
+            BodyFactory bodyFactory) throws MessagingException {
         checkOpen();
 
         String partId = part.getServerExtra();
@@ -801,7 +801,7 @@ class ImapFolder extends Folder<ImapMessage> {
             ImapResponse response;
             int messageNumber = 0;
 
-            ImapResponseCallback callback = new FetchPartCallback(part, progressCallback);
+            ImapResponseCallback callback = new FetchPartCallback(part, bodyFactory);
 
             do {
                 response = connection.readResponse(callback);
@@ -829,7 +829,7 @@ class ImapFolder extends Folder<ImapMessage> {
 
                     if (literal != null) {
                         if (literal instanceof Body) {
-                            // Most of the work was done in FetchAttchmentCallback.foundLiteral()
+                            // Most of the work was done in FetchAttachmentCallback.foundLiteral()
                             MimeMessageHelper.setBody(part, (Body) literal);
                         } else if (literal instanceof String) {
                             String bodyString = (String) literal;
@@ -838,8 +838,8 @@ class ImapFolder extends Folder<ImapMessage> {
                             String contentTransferEncoding =
                                     part.getHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING)[0];
                             String contentType = part.getHeader(MimeHeader.HEADER_CONTENT_TYPE)[0];
-                            MimeMessageHelper.setBody(part, MimeUtility.createBody(bodyStream, contentTransferEncoding,
+                            Body body = bodyFactory.createBody(contentTransferEncoding, contentType, bodyStream);
-                                    contentType, progressCallback));
+                            MimeMessageHelper.setBody(part, body);
                         } else {
                             // This shouldn't happen
                             throw new MessagingException("Got FETCH response with bogus parameters");

k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java

@@ -12,6 +12,7 @@ import java.util.Set;
 import java.util.TimeZone;
 
 import com.fsck.k9.mail.Body;
+import com.fsck.k9.mail.DefaultBodyFactory;
 import com.fsck.k9.mail.FetchProfile;
 import com.fsck.k9.mail.FetchProfile.Item;
 import com.fsck.k9.mail.Flag;
@@ -962,7 +963,7 @@ public class ImapFolderTest {
         Part part = createPlainTextPart("1.1");
         setupSingleFetchResponseToCallback();
 
-        folder.fetchPart(message, part, null, null);
+        folder.fetchPart(message, part, null, new DefaultBodyFactory());
 
         ArgumentCaptor<Body> bodyArgumentCaptor = ArgumentCaptor.forClass(Body.class);
         verify(part).setBody(bodyArgumentCaptor.capture());

k9mail/src/main/java/com/fsck/k9/controller/MessagingController.java

@@ -18,8 +18,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.Timer;
-import java.util.TimerTask;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArraySet;
@@ -65,12 +63,13 @@ import com.fsck.k9.controller.MessagingControllerCommands.PendingExpunge;
 import com.fsck.k9.controller.MessagingControllerCommands.PendingMarkAllAsRead;
 import com.fsck.k9.controller.MessagingControllerCommands.PendingMoveOrCopy;
 import com.fsck.k9.controller.MessagingControllerCommands.PendingSetFlag;
-import com.fsck.k9.fragment.AttachmentDownloadDialogFragment;
+import com.fsck.k9.controller.ProgressBodyFactory.ProgressListener;
 import com.fsck.k9.helper.Contacts;
 import com.fsck.k9.mail.Address;
-import com.fsck.k9.mail.AttachmentProgressCallback;
 import com.fsck.k9.mail.AuthenticationFailedException;
+import com.fsck.k9.mail.BodyFactory;
 import com.fsck.k9.mail.CertificateValidationException;
+import com.fsck.k9.mail.DefaultBodyFactory;
 import com.fsck.k9.mail.FetchProfile;
 import com.fsck.k9.mail.FetchProfile.Item;
 import com.fsck.k9.mail.Flag;
@@ -154,8 +153,6 @@ public class MessagingController {
     private MessagingListener checkMailListener = null;
     private volatile boolean stopped = false;
 
-    private MessagingListener attachmentDownloadProgressListener = null;
-
 
     public static synchronized MessagingController getInstance(Context context) {
         if (inst == null) {
@@ -265,10 +262,6 @@ public class MessagingController {
         throw new Error(e);
     }
 
-    public void addDownloadProgressListener(MessagingListener messagingListener){
-        attachmentDownloadProgressListener = messagingListener;
-    }
-
     public void addListener(MessagingListener listener) {
         listeners.add(listener);
         refreshListener(listener);
@@ -1492,8 +1485,9 @@ public class MessagingController {
         /*
          * Now download the parts we're interested in storing.
          */
+        BodyFactory bodyFactory = new DefaultBodyFactory();
         for (Part part : viewables) {
-            remoteFolder.fetchPart(message, part, null, null);
+            remoteFolder.fetchPart(message, part, null, bodyFactory);
         }
         // Store the updated message locally
         localFolder.appendMessages(Collections.singletonList(message));
@@ -2561,15 +2555,17 @@ public class MessagingController {
                     remoteFolder = remoteStore.getFolder(folderName);
                     remoteFolder.open(Folder.OPEN_MODE_RW);
 
-                    AttachmentProgressCallback attachmentProgressCallback = new AttachmentProgressCallback() {
+                    ProgressBodyFactory bodyFactory = new ProgressBodyFactory(new ProgressListener() {
                         @Override
-                        public void onUpdate(int progress) {
+                        public void updateProgress(int progress) {
-                            attachmentDownloadProgressListener.updateProgress(progress);
+                            for (MessagingListener listener : getListeners()) {
+                                listener.updateProgress(progress);
                             }
-                    };
+                        }
+                    });
 
                     Message remoteMessage = remoteFolder.getMessage(message.getUid());
-                    remoteFolder.fetchPart(remoteMessage, part, null, attachmentProgressCallback);
+                    remoteFolder.fetchPart(remoteMessage, part, null, bodyFactory);
 
                     localFolder.addPartToMessage(message, part);
 

k9mail/src/main/java/com/fsck/k9/controller/ProgressBodyFactory.java

@@ -0,0 +1,44 @@
+package com.fsck.k9.controller;
+
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.Timer;
+import java.util.TimerTask;
+
+import com.fsck.k9.mail.DefaultBodyFactory;
+import org.apache.commons.io.output.CountingOutputStream;
+
+
+class ProgressBodyFactory extends DefaultBodyFactory {
+    private final ProgressListener progressListener;
+
+
+    ProgressBodyFactory(ProgressListener progressListener) {
+        this.progressListener = progressListener;
+    }
+
+    @Override
+    protected void copyData(InputStream inputStream, OutputStream outputStream) throws IOException {
+        final CountingOutputStream countingOutputStream = new CountingOutputStream(outputStream);
+
+        Timer timer = new Timer();
+        try {
+            timer.scheduleAtFixedRate(new TimerTask() {
+                @Override
+                public void run() {
+                    progressListener.updateProgress(countingOutputStream.getCount());
+                }
+            }, 0, 50);
+
+            super.copyData(inputStream, countingOutputStream);
+        } finally {
+            timer.cancel();
+        }
+    }
+
+    interface ProgressListener {
+        void updateProgress(int progress);
+    }
+}

k9mail/src/main/java/com/fsck/k9/fragment/AttachmentDownloadDialogFragment.java

@@ -21,6 +21,7 @@ public class AttachmentDownloadDialogFragment extends DialogFragment {
 
     protected static final String ARG_SIZE = "size";
     protected static final String ARG_MESSAGE = "message";
+    private MessagingController messagingController;
 
     public static AttachmentDownloadDialogFragment newInstance(int size, String message) {
         AttachmentDownloadDialogFragment attachmentDownloadDialogFragment = new AttachmentDownloadDialogFragment();
@@ -53,7 +54,8 @@ public class AttachmentDownloadDialogFragment extends DialogFragment {
             }
         };
 
-        MessagingController.getInstance(getActivity()).addDownloadProgressListener(messagingListener);
+        messagingController = MessagingController.getInstance(getActivity());
+        messagingController.addListener(messagingListener);
 
         dialog = new ProgressDialog(getActivity());
         dialog.setMessage(message);
@@ -65,6 +67,12 @@ public class AttachmentDownloadDialogFragment extends DialogFragment {
         return dialog;
     }
 
+    @Override
+    public void onDestroyView() {
+        messagingController.removeListener(messagingListener);
+        super.onDestroyView();
+    }
+
     @Override
     public void onCancel(DialogInterface dialog) {
         Activity activity = getActivity();