From 7358091699ff7e5d9c3c8547a65e58d4bbd8884e Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 12 Apr 2019 09:25:45 -0300 Subject: [PATCH] [android] Address first round of review comments for attachments --- .../attachment/AttachmentCreationTask.java | 19 +++++++++++-------- .../android/attachment/AttachmentCreator.java | 13 ++++++------- .../android/attachment/AttachmentManager.java | 2 +- .../view/TextAttachmentController.java | 10 ++++++++++ 4 files changed, 28 insertions(+), 16 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 3e4e64767..4bd89e3d9 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 @@ -37,7 +37,7 @@ class AttachmentCreationTask { private final List uris; private final boolean needsSize; @Nullable - private AttachmentCreator attachmentCreator; + private volatile AttachmentCreator attachmentCreator; private volatile boolean canceled = false; @@ -61,9 +61,10 @@ class AttachmentCreationTask { @IoExecutor public void storeAttachments() { for (Uri uri: uris) processUri(uri); + AttachmentCreator attachmentCreator = this.attachmentCreator; if (!canceled && attachmentCreator != null) attachmentCreator.onAttachmentCreationFinished(); - attachmentCreator = null; + this.attachmentCreator = null; } @IoExecutor @@ -71,16 +72,17 @@ class AttachmentCreationTask { if (canceled) return; try { AttachmentHeader h = storeAttachment(uri); + AttachmentCreator attachmentCreator = this.attachmentCreator; if (attachmentCreator != null) { - attachmentCreator - .onAttachmentHeaderReceived(uri, h, needsSize); + attachmentCreator.onAttachmentHeaderReceived(uri, h, needsSize); } } catch (DbException | IOException e) { logException(LOG, WARNING, e); + AttachmentCreator attachmentCreator = this.attachmentCreator; if (attachmentCreator != null) { attachmentCreator.onAttachmentError(uri, e); - canceled = true; } + canceled = true; } } @@ -90,9 +92,10 @@ class AttachmentCreationTask { long start = now(); String contentType = contentResolver.getType(uri); if (contentType == null) throw new IOException("null content type"); - if (!isValidMimeType(contentType)) - throw new UnsupportedMimeTypeException("", contentType, - uri.toString()); + if (!isValidMimeType(contentType)) { + String uriString = uri.toString(); + throw new UnsupportedMimeTypeException("", contentType, uriString); + } InputStream is = contentResolver.openInputStream(uri); if (is == null) throw new IOException(); long timestamp = System.currentTimeMillis(); 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 32f12c2fc..e03a0d7b5 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 @@ -44,7 +44,7 @@ public class AttachmentCreator { @IoExecutor private final Executor ioExecutor; private final MessagingManager messagingManager; - private final AttachmentRetriever controller; + private final AttachmentRetriever retriever; private final Map unsentItems = new ConcurrentHashMap<>(); @@ -57,12 +57,11 @@ public class AttachmentCreator { private AttachmentCreationTask task; public AttachmentCreator(Application app, @IoExecutor Executor ioExecutor, - MessagingManager messagingManager, - AttachmentRetriever controller) { + MessagingManager messagingManager, AttachmentRetriever retriever) { this.app = app; this.ioExecutor = ioExecutor; this.messagingManager = messagingManager; - this.controller = controller; + this.retriever = retriever; } @UiThread @@ -101,8 +100,8 @@ public class AttachmentCreator { boolean needsSize) { // get and cache AttachmentItem for ImagePreview try { - Attachment a = controller.getMessageAttachment(h); - AttachmentItem item = controller.getAttachmentItem(h, a, needsSize); + Attachment a = retriever.getMessageAttachment(h); + AttachmentItem item = retriever.getAttachmentItem(h, a, needsSize); if (item.hasError()) throw new IOException(); unsentItems.put(uri, item); MutableLiveData result = @@ -156,7 +155,7 @@ public class AttachmentCreator { * @param id The MessageId of the sent message. */ public void onAttachmentsSent(MessageId id) { - controller.cachePut(id, new ArrayList<>(unsentItems.values())); + retriever.cachePut(id, new ArrayList<>(unsentItems.values())); resetState(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java index 8b5709d5c..194af99ec 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java @@ -9,7 +9,7 @@ import java.util.Collection; import java.util.List; @UiThread -public interface AttachmentManager{ +public interface AttachmentManager { AttachmentResult storeAttachments(Collection uri); 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 a6434c865..b7bd4f23f 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 @@ -138,6 +138,16 @@ public class TextAttachmentController extends TextSendController return intent; } + /** + * This is called with the result Intent + * returned by the Activity started with {@link #getAttachFileIntent()}. + *

+ * This method must be called at most once per call to + * {@link AttachImageListener#onAttachImage(Intent)}. + * Normally, this is true if called from + * {@link Activity#onActivityResult(int, int, Intent)} since this is called + * at most once per call to {@link Activity#startActivityForResult(Intent, int)}. + */ public void onImageReceived(@Nullable Intent resultData) { if (resultData == null) return; if (loadingUris || !imageUris.isEmpty()) throw new AssertionError();