[android] Display Image Attachements: Address first round of review comments

This commit is contained in:
Torsten Grote
2018-11-19 20:32:43 -02:00
parent de8e95692a
commit 10e9fb308d
6 changed files with 28 additions and 23 deletions

View File

@@ -119,9 +119,14 @@ class AttachmentController {
is.reset(); is.reset();
size = getSizeFromBitmap(is); size = getSizeFromBitmap(is);
} }
is.close();
} catch (IOException e) { } catch (IOException e) {
logException(LOG, WARNING, e); logException(LOG, WARNING, e);
} finally {
try {
is.close();
} catch (IOException e) {
logException(LOG, WARNING, e);
}
} }
// calculate thumbnail size // calculate thumbnail size
@@ -161,7 +166,7 @@ class AttachmentController {
BitmapFactory.Options options = new BitmapFactory.Options(); BitmapFactory.Options options = new BitmapFactory.Options();
options.inJustDecodeBounds = true; options.inJustDecodeBounds = true;
BitmapFactory.decodeStream(is, null, options); BitmapFactory.decodeStream(is, null, options);
if (options.outWidth == -1 || options.outHeight == -1) if (options.outWidth < 1 || options.outHeight < 1)
return new Size(); return new Size();
return new Size(options.outWidth, options.outHeight); return new Size(options.outWidth, options.outHeight);
} }
@@ -178,9 +183,10 @@ class AttachmentController {
} }
private static class Size { private static class Size {
private int width;
private int height; private final int width;
private boolean error; private final int height;
private final boolean error;
private Size(int width, int height) { private Size(int width, int height) {
this.width = width; this.width = width;

View File

@@ -444,8 +444,12 @@ public class ConversationActivity extends BriarActivity
List<AttachmentHeader> headers) { List<AttachmentHeader> headers) {
runOnDbThread(() -> { runOnDbThread(() -> {
try { try {
displayMessageAttachments(messageId, List<Pair<AttachmentHeader, Attachment>> attachments =
attachmentController.getMessageAttachments(headers)); attachmentController.getMessageAttachments(headers);
// TODO move getting the items off to the IoExecutor
List<AttachmentItem> items =
attachmentController.getAttachmentItems(attachments);
displayMessageAttachments(messageId, items);
} catch (DbException e) { } catch (DbException e) {
logException(LOG, WARNING, e); logException(LOG, WARNING, e);
} }
@@ -453,10 +457,8 @@ public class ConversationActivity extends BriarActivity
} }
private void displayMessageAttachments(MessageId m, private void displayMessageAttachments(MessageId m,
List<Pair<AttachmentHeader, Attachment>> attachments) { List<AttachmentItem> items) {
runOnUiThreadUnlessDestroyed(() -> { runOnUiThreadUnlessDestroyed(() -> {
List<AttachmentItem> items =
attachmentController.getAttachmentItems(attachments);
attachmentController.put(m, items); attachmentController.put(m, items);
Pair<Integer, ConversationMessageItem> pair = Pair<Integer, ConversationMessageItem> pair =
adapter.getMessageItem(m); adapter.getMessageItem(m);
@@ -829,12 +831,14 @@ public class ConversationActivity extends BriarActivity
return text; return text;
} }
@Nullable
@Override @Override
public List<AttachmentItem> getAttachmentItems(MessageId m, public List<AttachmentItem> getAttachmentItems(MessageId m,
List<AttachmentHeader> headers) { List<AttachmentHeader> headers) {
List<AttachmentItem> attachments = attachmentController.get(m); List<AttachmentItem> attachments = attachmentController.get(m);
if (attachments == null) loadMessageAttachments(m, headers); if (attachments == null) {
loadMessageAttachments(m, headers);
return emptyList();
}
return attachments; return attachments;
} }
} }

View File

@@ -1,7 +1,6 @@
package org.briarproject.briar.android.conversation; package org.briarproject.briar.android.conversation;
import android.support.annotation.LayoutRes; import android.support.annotation.LayoutRes;
import android.support.annotation.Nullable;
import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
import org.briarproject.briar.api.messaging.PrivateMessageHeader; import org.briarproject.briar.api.messaging.PrivateMessageHeader;
@@ -14,16 +13,14 @@ import javax.annotation.concurrent.NotThreadSafe;
@NotNullByDefault @NotNullByDefault
class ConversationMessageItem extends ConversationItem { class ConversationMessageItem extends ConversationItem {
@Nullable
private List<AttachmentItem> attachments; private List<AttachmentItem> attachments;
ConversationMessageItem(@LayoutRes int layoutRes, PrivateMessageHeader h, ConversationMessageItem(@LayoutRes int layoutRes, PrivateMessageHeader h,
@Nullable List<AttachmentItem> attachments) { List<AttachmentItem> attachments) {
super(layoutRes, h); super(layoutRes, h);
this.attachments = attachments; this.attachments = attachments;
} }
@Nullable
List<AttachmentItem> getAttachments() { List<AttachmentItem> getAttachments() {
return attachments; return attachments;
} }

View File

@@ -21,7 +21,6 @@ import static android.os.Build.VERSION.SDK_INT;
import static android.support.v4.view.ViewCompat.LAYOUT_DIRECTION_RTL; import static android.support.v4.view.ViewCompat.LAYOUT_DIRECTION_RTL;
import static com.bumptech.glide.load.engine.DiskCacheStrategy.NONE; import static com.bumptech.glide.load.engine.DiskCacheStrategy.NONE;
import static com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions.withCrossFade; import static com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions.withCrossFade;
import static java.util.Objects.requireNonNull;
@UiThread @UiThread
@NotNullByDefault @NotNullByDefault
@@ -83,7 +82,7 @@ class ConversationMessageViewHolder extends ConversationItemViewHolder {
super.bind(conversationItem, listener); super.bind(conversationItem, listener);
ConversationMessageItem item = ConversationMessageItem item =
(ConversationMessageItem) conversationItem; (ConversationMessageItem) conversationItem;
if (item.getAttachments() == null || item.getAttachments().isEmpty()) { if (item.getAttachments().isEmpty()) {
bindTextItem(); bindTextItem();
} else { } else {
bindImageItem(item); bindImageItem(item);
@@ -101,8 +100,7 @@ class ConversationMessageViewHolder extends ConversationItemViewHolder {
private void bindImageItem(ConversationMessageItem item) { private void bindImageItem(ConversationMessageItem item) {
// TODO show more than just the first image // TODO show more than just the first image
AttachmentItem attachment = AttachmentItem attachment = item.getAttachments().get(0);
requireNonNull(item.getAttachments()).get(0);
ConstraintSet constraintSet; ConstraintSet constraintSet;
if (item.getText() == null) { if (item.getText() == null) {

View File

@@ -294,7 +294,6 @@ class ConversationVisitor implements
} }
interface AttachmentCache { interface AttachmentCache {
@Nullable
List<AttachmentItem> getAttachmentItems(MessageId m, List<AttachmentItem> getAttachmentItems(MessageId m,
List<AttachmentHeader> headers); List<AttachmentHeader> headers);
} }

View File

@@ -25,8 +25,9 @@ public final class BriarGlideModule extends AppGlideModule {
@Override @Override
public void registerComponents(Context context, Glide glide, public void registerComponents(Context context, Glide glide,
Registry registry) { Registry registry) {
BriarModelLoaderFactory factory = BriarApplication app =
new BriarModelLoaderFactory((BriarApplication) context); (BriarApplication) context.getApplicationContext();
BriarModelLoaderFactory factory = new BriarModelLoaderFactory(app);
registry.prepend(AttachmentItem.class, InputStream.class, factory); registry.prepend(AttachmentItem.class, InputStream.class, factory);
} }