From 9abe32ab4b4179ec242383f49c92005863a50193 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 14 Jun 2019 15:30:34 +0100 Subject: [PATCH] Refactor attachment code to reduce mutable state. --- .../attachment/AttachmentCreationTask.java | 71 ++++--- .../android/attachment/AttachmentCreator.java | 177 ++++++------------ .../attachment/AttachmentItemResult.java | 12 +- .../android/attachment/AttachmentResult.java | 9 +- .../attachment/AttachmentRetriever.java | 11 +- .../conversation/ConversationActivity.java | 35 ++-- .../conversation/ConversationViewModel.java | 28 ++- .../view/TextAttachmentController.java | 42 +++-- 8 files changed, 186 insertions(+), 199 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java index fee272a74..7f54f0742 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java @@ -1,22 +1,28 @@ package org.briarproject.briar.android.attachment; +import android.arch.lifecycle.LiveData; +import android.arch.lifecycle.MutableLiveData; import android.content.ContentResolver; import android.net.Uri; -import android.support.annotation.Nullable; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.GroupId; +import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; import org.jsoup.UnsupportedMimeTypeException; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.logging.Logger; +import javax.annotation.Nullable; + import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.IoUtils.tryToClose; @@ -33,56 +39,64 @@ class AttachmentCreationTask { private final MessagingManager messagingManager; private final ContentResolver contentResolver; + private final AttachmentRetriever retriever; private final GroupId groupId; private final Collection uris; private final boolean needsSize; - @Nullable - private volatile AttachmentCreator attachmentCreator; + private final MutableLiveData result; private volatile boolean canceled = false; AttachmentCreationTask(MessagingManager messagingManager, - ContentResolver contentResolver, - AttachmentCreator attachmentCreator, GroupId groupId, - Collection uris, boolean needsSize) { + ContentResolver contentResolver, AttachmentRetriever retriever, + GroupId groupId, Collection uris, boolean needsSize) { this.messagingManager = messagingManager; this.contentResolver = contentResolver; + this.retriever = retriever; this.groupId = groupId; this.uris = uris; this.needsSize = needsSize; - this.attachmentCreator = attachmentCreator; + result = new MutableLiveData<>(); } - public void cancel() { + LiveData getResult() { + return result; + } + + void cancel() { canceled = true; - attachmentCreator = null; } @IoExecutor void storeAttachments() { - for (Uri uri: uris) processUri(uri); - AttachmentCreator attachmentCreator = this.attachmentCreator; - if (!canceled && attachmentCreator != null) - attachmentCreator.onAttachmentCreationFinished(); - this.attachmentCreator = null; + List results = new ArrayList<>(); + for (Uri uri : uris) { + if (canceled) break; + results.add(processUri(uri)); + result.postValue(new AttachmentResult(new ArrayList<>(results), + false, false)); + } + result.postValue(new AttachmentResult(new ArrayList<>(results), true, + !canceled)); } @IoExecutor - private void processUri(Uri uri) { - if (canceled) return; + private AttachmentItemResult processUri(Uri uri) { + AttachmentHeader header = null; try { - AttachmentHeader h = storeAttachment(uri); - AttachmentCreator attachmentCreator = this.attachmentCreator; - if (attachmentCreator != null) { - attachmentCreator.onAttachmentHeaderReceived(uri, h, needsSize); - } + header = storeAttachment(uri); + Attachment a = retriever.getMessageAttachment(header); + AttachmentItem item = + retriever.getAttachmentItem(header, a, needsSize); + if (item.hasError()) throw new IOException(); + retriever.cachePut(item); + return new AttachmentItemResult(uri, item); } catch (DbException | IOException e) { logException(LOG, WARNING, e); - AttachmentCreator attachmentCreator = this.attachmentCreator; - if (attachmentCreator != null) { - attachmentCreator.onAttachmentError(uri, e); - } + // If the attachment was already stored, delete it + tryToRemove(header); canceled = true; + return new AttachmentItemResult(uri, e); } } @@ -113,4 +127,11 @@ class AttachmentCreationTask { return false; } + private void tryToRemove(@Nullable AttachmentHeader h) { + try { + if (h != null) messagingManager.removeAttachment(h); + } catch (DbException e1) { + logException(LOG, WARNING, e1); + } + } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java index ea7ed11b1..244ffb7fd 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -3,7 +3,7 @@ package org.briarproject.briar.android.attachment; import android.app.Application; import android.arch.lifecycle.LiveData; -import android.arch.lifecycle.MutableLiveData; +import android.arch.lifecycle.Observer; import android.net.Uri; import android.support.annotation.Nullable; import android.support.annotation.UiThread; @@ -12,27 +12,20 @@ import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.GroupId; -import org.briarproject.bramble.api.sync.MessageId; -import org.briarproject.briar.R; -import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; -import org.briarproject.briar.api.messaging.FileTooBigException; import org.briarproject.briar.api.messaging.MessagingManager; -import org.jsoup.UnsupportedMimeTypeException; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.logging.Logger; +import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.LogUtils.logException; -import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; -import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE; @NotNullByDefault public class AttachmentCreator { @@ -45,12 +38,6 @@ public class AttachmentCreator { private final MessagingManager messagingManager; private final AttachmentRetriever retriever; - private final CopyOnWriteArrayList uris = new CopyOnWriteArrayList<>(); - private final CopyOnWriteArrayList itemResults = - new CopyOnWriteArrayList<>(); - - private final MutableLiveData result = - new MutableLiveData<>(); @Nullable private AttachmentCreationTask task; @@ -62,20 +49,19 @@ public class AttachmentCreator { this.retriever = retriever; } + /** + * Starts a background task to create attachments from the given URIs and + * returns a LiveData to monitor the progress of the task. + */ @UiThread - public LiveData storeAttachments( - LiveData groupId, Collection newUris) { - if (task != null || !uris.isEmpty()) - throw new IllegalStateException(); - uris.addAll(newUris); - observeForeverOnce(groupId, id -> { - if (id == null) throw new IllegalStateException(); - boolean needsSize = uris.size() == 1; - task = new AttachmentCreationTask(messagingManager, - app.getContentResolver(), this, id, uris, needsSize); - ioExecutor.execute(() -> task.storeAttachments()); - }); - return result; + public LiveData storeAttachments(GroupId groupId, + Collection uris) { + if (task != null) throw new IllegalStateException(); + boolean needsSize = uris.size() == 1; + task = new AttachmentCreationTask(messagingManager, + app.getContentResolver(), retriever, groupId, uris, needsSize); + ioExecutor.execute(() -> task.storeAttachments()); + return task.getResult(); } /** @@ -85,112 +71,70 @@ public class AttachmentCreator { */ @UiThread public LiveData getLiveAttachments() { - if (task == null || uris.isEmpty()) - throw new IllegalStateException(); + if (task == null) throw new IllegalStateException(); // A task is already running. It will update the result LiveData. // So nothing more to do here. - return result; - } - - @IoExecutor - void onAttachmentHeaderReceived(Uri uri, AttachmentHeader h, - boolean needsSize) { - // get and cache AttachmentItem for ImagePreview - try { - Attachment a = retriever.getMessageAttachment(h); - AttachmentItem item = retriever.getAttachmentItem(h, a, needsSize); - if (item.hasError()) throw new IOException(); - AttachmentItemResult itemResult = - new AttachmentItemResult(uri, item); - itemResults.add(itemResult); - result.postValue(getResult(false)); - } catch (IOException | DbException e) { - logException(LOG, WARNING, e); - onAttachmentError(uri, e); - } - } - - @IoExecutor - void onAttachmentError(Uri uri, Exception e) { - // get error message - String errorMsg; - if (e instanceof UnsupportedMimeTypeException) { - String mimeType = ((UnsupportedMimeTypeException) e).getMimeType(); - errorMsg = app.getString( - R.string.image_attach_error_invalid_mime_type, mimeType); - } else if (e instanceof FileTooBigException) { - int mb = MAX_IMAGE_SIZE / 1024 / 1024; - errorMsg = app.getString(R.string.image_attach_error_too_big, mb); - } else { - errorMsg = null; // generic error - } - AttachmentItemResult itemResult = - new AttachmentItemResult(uri, errorMsg); - itemResults.add(itemResult); - result.postValue(getResult(false)); - // expect to receive a cancel from the UI - } - - @IoExecutor - void onAttachmentCreationFinished() { - result.postValue(getResult(true)); + return task.getResult(); } + /** + * Returns the headers of any attachments created by + * {@link #storeAttachments(GroupId, Collection)}. + */ @UiThread public List getAttachmentHeadersForSending() { - List headers = new ArrayList<>(itemResults.size()); - for (AttachmentItemResult itemResult : itemResults) { - // check if we are trying to send attachment items with errors - if (itemResult.getItem() == null) throw new IllegalStateException(); - headers.add(itemResult.getItem().getHeader()); + if (task == null) return emptyList(); + AttachmentResult result = task.getResult().getValue(); + if (result == null) return emptyList(); + List headers = new ArrayList<>(); + for (AttachmentItemResult itemResult : result.getItemResults()) { + AttachmentItem item = itemResult.getItem(); + if (item != null) headers.add(item.getHeader()); } return headers; } /** - * Marks the attachments as sent and adds the items to the cache for display - * - * @param id The MessageId of the sent message. + * Informs the AttachmentCreator that the attachments created by + * {@link #storeAttachments(GroupId, Collection)} will be sent. */ @UiThread - public void onAttachmentsSent(MessageId id) { - List items = new ArrayList<>(itemResults.size()); - for (AttachmentItemResult itemResult : itemResults) { - // check if we are trying to send attachment items with errors - if (itemResult.getItem() == null) throw new IllegalStateException(); - items.add(itemResult.getItem()); - } - retriever.cachePut(id, items); - resetState(); + public void onAttachmentsSent() { + task = null; } /** - * Needs to be called when created attachments will not be sent anymore. + * Cancels the task started by + * {@link #storeAttachments(GroupId, Collection)} and deletes any + * created attachments, unless {@link #onAttachmentsSent()} has + * been called. */ @UiThread public void cancel() { - if (task == null) throw new AssertionError(); + if (task == null) return; // Already sent or cancelled task.cancel(); - deleteUnsentAttachments(); - resetState(); - } - - @UiThread - private void resetState() { + // Observe the task until it finishes (which may already have + // happened) and delete any created attachments + LiveData taskResult = task.getResult(); + taskResult.observeForever(new Observer() { + @Override + public void onChanged(@Nullable AttachmentResult result) { + requireNonNull(result); + if (result.isFinished()) { + deleteUnsentAttachments(result.getItemResults()); + taskResult.removeObserver(this); + } + } + }); task = null; - uris.clear(); - itemResults.clear(); - result.setValue(null); } - @UiThread - public void deleteUnsentAttachments() { - // Make a copy for the IoExecutor as we clear the itemResults soon + private void deleteUnsentAttachments( + Collection itemResults) { List headers = new ArrayList<>(itemResults.size()); for (AttachmentItemResult itemResult : itemResults) { - // check if we are trying to send attachment items with errors - if (itemResult.getItem() != null) - headers.add(itemResult.getItem().getHeader()); + AttachmentItem item = itemResult.getItem(); + if (item != null) headers.add(item.getHeader()); } ioExecutor.execute(() -> { for (AttachmentHeader header : headers) { @@ -202,17 +146,4 @@ public class AttachmentCreator { } }); } - - private AttachmentResult getResult(boolean finished) { - // Make a copy of the list, - // because our copy will continue to change in the background. - // (As it's a CopyOnWriteArrayList, - // the code that receives the result can safely do simple things - // like iterating over the list, - // but anything that involves calling more than one list method - // is still unsafe.) - Collection items = new ArrayList<>(itemResults); - return new AttachmentResult(items, finished); - } - } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java index 1254a851d..ad9f1597e 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java @@ -15,18 +15,18 @@ public class AttachmentItemResult { @Nullable private final AttachmentItem item; @Nullable - private final String errorMsg; + private final Exception exception; AttachmentItemResult(Uri uri, AttachmentItem item) { this.uri = uri; this.item = item; - this.errorMsg = null; + this.exception = null; } - AttachmentItemResult(Uri uri, @Nullable String errorMsg) { + AttachmentItemResult(Uri uri, Exception exception) { this.uri = uri; this.item = null; - this.errorMsg = errorMsg; + this.exception = exception; } public Uri getUri() { @@ -43,8 +43,8 @@ public class AttachmentItemResult { } @Nullable - public String getErrorMsg() { - return errorMsg; + public Exception getException() { + return exception; } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java index 776d2ab59..61ef44def 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java @@ -12,11 +12,13 @@ public class AttachmentResult { private final Collection itemResults; private final boolean finished; + private final boolean success; - public AttachmentResult(Collection itemResults, - boolean finished) { + AttachmentResult(Collection itemResults, + boolean finished, boolean success) { this.itemResults = itemResults; this.finished = finished; + this.success = success; } public Collection getItemResults() { @@ -27,4 +29,7 @@ public class AttachmentResult { return finished; } + public boolean isSuccess() { + return success; + } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java index a8da12a2a..14cd322cb 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java @@ -56,7 +56,7 @@ public class AttachmentRetriever { private final int minWidth, maxWidth; private final int minHeight, maxHeight; - private final Map> attachmentCache = + private final Map attachmentCache = new ConcurrentHashMap<>(); @VisibleForTesting @@ -94,13 +94,13 @@ public class AttachmentRetriever { }); } - public void cachePut(MessageId messageId, List attachments) { - attachmentCache.put(messageId, attachments); + public void cachePut(AttachmentItem item) { + attachmentCache.put(item.getMessageId(), item); } @Nullable - public List cacheGet(MessageId messageId) { - return attachmentCache.get(messageId); + public AttachmentItem cacheGet(MessageId attachmentId) { + return attachmentCache.get(attachmentId); } @DatabaseExecutor @@ -117,7 +117,6 @@ public class AttachmentRetriever { return attachments; } - @DatabaseExecutor Attachment getMessageAttachment(AttachmentHeader h) throws DbException { return messagingManager.getAttachment(h.getMessageId()); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java index 52cc1a19a..9235d579c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java @@ -448,14 +448,16 @@ public class ConversationActivity extends BriarActivity } // If the message has a single image, load its size - for multiple // images we use a grid so the size is fixed - if (h.getAttachmentHeaders().size() == 1) { - List items = attachmentRetriever.cacheGet(id); - if (items == null) { + List headers = h.getAttachmentHeaders(); + if (headers.size() == 1) { + MessageId attachmentId = headers.get(0).getMessageId(); + AttachmentItem item = attachmentRetriever.cacheGet(attachmentId); + if (item == null) { LOG.info("Eagerly loading image size for latest message"); - items = attachmentRetriever.getAttachmentItems( - attachmentRetriever.getMessageAttachments( - h.getAttachmentHeaders())); - attachmentRetriever.cachePut(id, items); + item = attachmentRetriever.getAttachmentItems( + attachmentRetriever.getMessageAttachments(headers)) + .get(0); + attachmentRetriever.cachePut(item); } } } @@ -553,7 +555,9 @@ public class ConversationActivity extends BriarActivity private void displayMessageAttachments(MessageId m, List items) { runOnUiThreadUnlessDestroyed(() -> { - attachmentRetriever.cachePut(m, items); + for (AttachmentItem item : items) { + attachmentRetriever.cachePut(item); + } Pair pair = adapter.getMessageItem(m); if (pair != null) { @@ -905,12 +909,17 @@ public class ConversationActivity extends BriarActivity @Override public List getAttachmentItems(MessageId m, List headers) { - List attachments = attachmentRetriever.cacheGet(m); - if (attachments == null) { - loadMessageAttachments(m, headers); - return emptyList(); + List items = new ArrayList<>(headers.size()); + for (AttachmentHeader header : headers) { + AttachmentItem item = + attachmentRetriever.cacheGet(header.getMessageId()); + if (item == null) { + loadMessageAttachments(m, headers); + return emptyList(); + } + items.add(item); } - return attachments; + return items; } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index fc565d3a2..cfe94606b 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -4,6 +4,7 @@ import android.app.Application; import android.arch.lifecycle.AndroidViewModel; import android.arch.lifecycle.LiveData; import android.arch.lifecycle.MutableLiveData; +import android.arch.lifecycle.Observer; import android.arch.lifecycle.Transformations; import android.net.Uri; import android.support.annotation.Nullable; @@ -124,7 +125,7 @@ public class ConversationViewModel extends AndroidViewModel @Override protected void onCleared() { super.onCleared(); - attachmentCreator.deleteUnsentAttachments(); + attachmentCreator.cancel(); } /** @@ -200,12 +201,23 @@ public class ConversationViewModel extends AndroidViewModel @UiThread public LiveData storeAttachments(Collection uris, boolean restart) { - if (restart) { - return attachmentCreator.getLiveAttachments(); - } else { - // messagingGroupId is loaded with the contact - return attachmentCreator.storeAttachments(messagingGroupId, uris); - } + MutableLiveData delegate = new MutableLiveData<>(); + // messagingGroupId is loaded with the contact + observeForeverOnce(messagingGroupId, groupId -> { + requireNonNull(groupId); + LiveData result; + if (restart) result = attachmentCreator.getLiveAttachments(); + else result = attachmentCreator.storeAttachments(groupId, uris); + result.observeForever(new Observer() { + @Override + public void onChanged(@Nullable AttachmentResult value) { + requireNonNull(value); + if (value.isFinished()) result.removeObserver(this); + delegate.setValue(value); + } + }); + }); + return delegate; } @Override @@ -294,7 +306,7 @@ public class ConversationViewModel extends AndroidViewModel } private void storeMessage(PrivateMessage m) { - attachmentCreator.onAttachmentsSent(m.getMessage().getId()); + attachmentCreator.onAttachmentsSent(); dbExecutor.execute(() -> { try { long start = now(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java index 87cb9fb24..e7af004f6 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java @@ -22,6 +22,8 @@ import org.briarproject.briar.android.attachment.AttachmentItemResult; import org.briarproject.briar.android.attachment.AttachmentManager; import org.briarproject.briar.android.attachment.AttachmentResult; import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener; +import org.briarproject.briar.api.messaging.FileTooBigException; +import org.jsoup.UnsupportedMimeTypeException; import java.util.ArrayList; import java.util.Collection; @@ -41,9 +43,11 @@ import static android.support.v4.content.ContextCompat.getColor; import static android.support.v4.view.AbsSavedState.EMPTY_STATE; import static android.view.View.GONE; import static android.widget.Toast.LENGTH_LONG; +import static java.util.Objects.requireNonNull; import static org.briarproject.briar.android.util.UiUtils.resolveColorAttribute; import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES; import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_ATTACHMENTS_PER_MESSAGE; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE; import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_DISMISSED; import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_FINISHED; @@ -186,18 +190,16 @@ public class TextAttachmentController extends TextSendController result.observe(attachmentListener, new Observer() { @Override public void onChanged(@Nullable AttachmentResult attachmentResult) { - if (attachmentResult == null) { - // The fresh LiveData was deliberately set to null. - // This means that we can stop observing it. + requireNonNull(attachmentResult); + boolean finished = attachmentResult.isFinished(); + boolean success = attachmentResult.isSuccess(); + if (finished) { result.removeObserver(this); - } else { - boolean noError = onNewAttachmentItemResults( - attachmentResult.getItemResults()); - if (noError && attachmentResult.isFinished()) { - onAllAttachmentsCreated(); - result.removeObserver(this); - } + if (!success) return; } + boolean noError = onNewAttachmentItemResults( + attachmentResult.getItemResults()); + if (noError && success) onAllAttachmentsCreated(); } }); } @@ -207,7 +209,7 @@ public class TextAttachmentController extends TextSendController if (!loadingUris) throw new AssertionError(); for (AttachmentItemResult result : itemResults) { if (result.hasError()) { - onError(result.getErrorMsg()); + onError(requireNonNull(result.getException())); return false; } else { imagePreview.loadPreviewImage(result); @@ -253,12 +255,20 @@ public class TextAttachmentController extends TextSendController } @UiThread - private void onError(@Nullable String errorMsg) { - if (errorMsg == null) { - errorMsg = imagePreview.getContext() - .getString(R.string.image_attach_error); + private void onError(Exception e) { + String errorMsg; + Context ctx = imagePreview.getContext(); + if (e instanceof UnsupportedMimeTypeException) { + String mimeType = ((UnsupportedMimeTypeException) e).getMimeType(); + errorMsg = ctx.getString( + R.string.image_attach_error_invalid_mime_type, mimeType); + } else if (e instanceof FileTooBigException) { + int mb = MAX_IMAGE_SIZE / 1024 / 1024; + errorMsg = ctx.getString(R.string.image_attach_error_too_big, mb); + } else { + errorMsg = ctx.getString(R.string.image_attach_error); } - Toast.makeText(textInput.getContext(), errorMsg, LENGTH_LONG).show(); + Toast.makeText(ctx, errorMsg, LENGTH_LONG).show(); onCancel(); }