From 2f9d4c7ebb1881a5cc96114eb21b8e1f5281d152 Mon Sep 17 00:00:00 2001 From: M66B Date: Sun, 17 Feb 2019 09:18:42 +0000 Subject: [PATCH] Fixed race condition while handling sent messages --- .../java/eu/faircode/email/DaoMessage.java | 11 +- .../eu/faircode/email/EntityIdentity.java | 9 +- .../eu/faircode/email/FragmentIdentity.java | 9 - .../eu/faircode/email/FragmentQuickSetup.java | 2 - .../eu/faircode/email/ServiceSynchronize.java | 177 +++++++----------- app/src/main/res/layout/fragment_identity.xml | 21 +-- app/src/main/res/values/strings.xml | 2 - 7 files changed, 76 insertions(+), 155 deletions(-) diff --git a/app/src/main/java/eu/faircode/email/DaoMessage.java b/app/src/main/java/eu/faircode/email/DaoMessage.java index ce488850f5..8df9ac97c3 100644 --- a/app/src/main/java/eu/faircode/email/DaoMessage.java +++ b/app/src/main/java/eu/faircode/email/DaoMessage.java @@ -249,11 +249,12 @@ public interface DaoMessage { " AND NOT ui_browsed") List getUids(long folder, Long received); - @Query("SELECT * FROM message" + - " WHERE folder = :folder" + - " AND uid IS NULL" + - " AND ui_browsed") - List getSentOrphans(long folder); + @Query("SELECT message.* FROM message" + + " JOIN folder on folder.id = message.folder" + + " WHERE message.account = :account" + + " AND folder.type = '" + EntityFolder.OUTBOX + "'" + + " AND sent IS NOT NULL") + List getSentOrphans(long account); @Query("SELECT * FROM message WHERE NOT ui_snoozed IS NULL") List getSnoozed(); diff --git a/app/src/main/java/eu/faircode/email/EntityIdentity.java b/app/src/main/java/eu/faircode/email/EntityIdentity.java index b3b9bd1ffc..eaccef8b31 100644 --- a/app/src/main/java/eu/faircode/email/EntityIdentity.java +++ b/app/src/main/java/eu/faircode/email/EntityIdentity.java @@ -82,8 +82,8 @@ public class EntityIdentity { @NonNull public Boolean read_receipt = false; @NonNull - public Boolean store_sent = false; - public Long sent_folder; // obsolete + public Boolean store_sent = false; // obsolete + public Long sent_folder = null; // obsolete public Boolean tbd; public String state; public String error; @@ -122,7 +122,6 @@ public class EntityIdentity { json.put("plain_only", plain_only); json.put("delivery_receipt", delivery_receipt); json.put("read_receipt", read_receipt); - json.put("store_sent", store_sent); // not state // not error return json; @@ -165,9 +164,6 @@ public class EntityIdentity { if (json.has("read_receipt")) identity.read_receipt = json.getBoolean("read_receipt"); - if (json.has("store_sent")) - identity.store_sent = json.getBoolean("store_sent"); - return identity; } @@ -195,7 +191,6 @@ public class EntityIdentity { (this.bcc == null ? other.bcc == null : this.bcc.equals(other.bcc)) && this.delivery_receipt.equals(other.delivery_receipt) && this.read_receipt.equals(other.read_receipt) && - this.store_sent.equals(other.store_sent) && (this.tbd == null ? other.tbd == null : this.tbd.equals(other.tbd)) && (this.state == null ? other.state == null : this.state.equals(other.state)) && (this.error == null ? other.error == null : this.error.equals(other.error)) && diff --git a/app/src/main/java/eu/faircode/email/FragmentIdentity.java b/app/src/main/java/eu/faircode/email/FragmentIdentity.java index e6c168f7d0..b8d8570591 100644 --- a/app/src/main/java/eu/faircode/email/FragmentIdentity.java +++ b/app/src/main/java/eu/faircode/email/FragmentIdentity.java @@ -102,8 +102,6 @@ public class FragmentIdentity extends FragmentBase { private CheckBox cbDeliveryReceipt; private CheckBox cbReadReceipt; - private CheckBox cbStoreSent; - private Button btnSave; private ContentLoadingProgressBar pbSave; private TextView tvError; @@ -169,8 +167,6 @@ public class FragmentIdentity extends FragmentBase { cbDeliveryReceipt = view.findViewById(R.id.cbDeliveryReceipt); cbReadReceipt = view.findViewById(R.id.cbReadReceipt); - cbStoreSent = view.findViewById(R.id.cbStoreSent); - btnSave = view.findViewById(R.id.btnSave); pbSave = view.findViewById(R.id.pbSave); tvError = view.findViewById(R.id.tvError); @@ -467,7 +463,6 @@ public class FragmentIdentity extends FragmentBase { args.putBoolean("plain_only", cbPlainOnly.isChecked()); args.putBoolean("delivery_receipt", cbDeliveryReceipt.isChecked()); args.putBoolean("read_receipt", cbReadReceipt.isChecked()); - args.putBoolean("store_sent", cbStoreSent.isChecked()); args.putLong("account", account == null ? -1 : account.id); args.putInt("auth_type", auth_type); args.putString("host", etHost.getText().toString()); @@ -527,7 +522,6 @@ public class FragmentIdentity extends FragmentBase { boolean plain_only = args.getBoolean("plain_only"); boolean delivery_receipt = args.getBoolean("delivery_receipt"); boolean read_receipt = args.getBoolean("read_receipt"); - boolean store_sent = args.getBoolean("store_sent"); if (TextUtils.isEmpty(name)) throw new IllegalArgumentException(context.getString(R.string.title_no_name)); @@ -632,8 +626,6 @@ public class FragmentIdentity extends FragmentBase { identity.plain_only = plain_only; identity.delivery_receipt = delivery_receipt; identity.read_receipt = read_receipt; - identity.store_sent = store_sent; - identity.sent_folder = null; identity.error = null; identity.last_connected = last_connected; @@ -734,7 +726,6 @@ public class FragmentIdentity extends FragmentBase { cbPlainOnly.setChecked(identity == null ? false : identity.plain_only); cbDeliveryReceipt.setChecked(identity == null ? false : identity.delivery_receipt); cbReadReceipt.setChecked(identity == null ? false : identity.read_receipt); - cbStoreSent.setChecked(identity == null ? false : identity.store_sent); color = (identity == null || identity.color == null ? Color.TRANSPARENT : identity.color); diff --git a/app/src/main/java/eu/faircode/email/FragmentQuickSetup.java b/app/src/main/java/eu/faircode/email/FragmentQuickSetup.java index b0749a8b72..fb3574c6fa 100644 --- a/app/src/main/java/eu/faircode/email/FragmentQuickSetup.java +++ b/app/src/main/java/eu/faircode/email/FragmentQuickSetup.java @@ -338,8 +338,6 @@ public class FragmentQuickSetup extends FragmentBase { identity.bcc = null; identity.delivery_receipt = false; identity.read_receipt = false; - identity.store_sent = false; - identity.sent_folder = null; identity.error = null; identity.id = db.identity().insertIdentity(identity); diff --git a/app/src/main/java/eu/faircode/email/ServiceSynchronize.java b/app/src/main/java/eu/faircode/email/ServiceSynchronize.java index 0318f544b2..c3c8ec294f 100644 --- a/app/src/main/java/eu/faircode/email/ServiceSynchronize.java +++ b/app/src/main/java/eu/faircode/email/ServiceSynchronize.java @@ -839,6 +839,7 @@ public class ServiceSynchronize extends LifecycleService { wlAccount.acquire(); String type = (e.getMessageType() == StoreEvent.ALERT ? "alert" : "notice"); if (e.getMessageType() == StoreEvent.ALERT) { + Log.w(account.name + " " + type + ": " + e.getMessage()); EntityLog.log(ServiceSynchronize.this, account.name + " " + type + ": " + e.getMessage()); db.account().setAccountError(account.id, e.getMessage()); reportError(account, null, new AlertException(e.getMessage())); @@ -1550,6 +1551,7 @@ public class ServiceSynchronize extends LifecycleService { db.operation().deleteOperation(op.id); } catch (Throwable ex) { // TODO: SMTP response codes: https://www.ietf.org/rfc/rfc821.txt + Log.e(folder.name, ex); reportError(account, folder, ex); db.operation().setOperationError(op.id, Helper.formatThrowable(ex)); @@ -1977,116 +1979,69 @@ public class ServiceSynchronize extends LifecycleService { db.identity().setIdentityState(ident.id, "connected"); + // Append replied/forwarded text + String body = Helper.readText(EntityMessage.getFile(this, message.id)); + File refFile = EntityMessage.getRefFile(this, message.id); + if (refFile.exists()) + body += Helper.readText(refFile); + // Send message - Long sid = null; + Address[] to = imessage.getAllRecipients(); + itransport.sendMessage(imessage, to); + EntityLog.log(this, "Sent via " + ident.host + "/" + ident.user + + " to " + TextUtils.join(", ", to)); + try { - // Append replied/forwarded text - String body = Helper.readText(EntityMessage.getFile(this, message.id)); - File refFile = EntityMessage.getRefFile(this, message.id); - if (refFile.exists()) - body += Helper.readText(refFile); + db.beginTransaction(); - EntityFolder sent = db.folder().getFolderByType(ident.account, EntityFolder.SENT); - if (sent != null) { - long id = message.id; - long folder = message.folder; + db.message().setMessageSent(message.id, imessage.getSentDate().getTime()); + db.message().setMessageSeen(message.id, true); + db.message().setMessageUiSeen(message.id, true); + db.message().setMessageError(message.id, null); + Helper.writeText(EntityMessage.getFile(this, message.id), body); - message.id = null; - message.folder = sent.id; - message.seen = true; - message.ui_seen = true; - message.ui_hide = true; - message.ui_browsed = true; // prevent deleting on sync - message.error = null; - message.id = db.message().insertMessage(message); - Helper.writeText(EntityMessage.getFile(this, message.id), body); - - sid = message.id; - message.id = id; - message.folder = folder; - message.seen = false; - message.ui_seen = false; - message.ui_browsed = false; - message.ui_hide = false; - - EntityAttachment.copy(this, db, message.id, sid); - } - - Address[] to = imessage.getAllRecipients(); - itransport.sendMessage(imessage, to); - EntityLog.log(this, "Sent via " + ident.host + "/" + ident.user + - " to " + TextUtils.join(", ", to)); - - try { - db.beginTransaction(); - - if (sid == null) { - db.message().setMessageSent(message.id, imessage.getSentDate().getTime()); - db.message().setMessageSeen(message.id, true); - db.message().setMessageUiSeen(message.id, true); - db.message().setMessageError(message.id, null); - Helper.writeText(EntityMessage.getFile(this, message.id), body); - } else { - db.message().setMessageSent(sid, imessage.getSentDate().getTime()); - db.message().setMessageUiHide(sid, false); - db.message().deleteMessage(message.id); - - if (ident.store_sent) { - message.id = sid; - message.folder = sent.id; - EntityOperation.queue(this, db, message, EntityOperation.ADD); - } - } - - db.setTransactionSuccessful(); - } finally { - db.endTransaction(); - } - - if (refFile.exists()) - refFile.delete(); - - if (message.inreplyto != null) { - List replieds = db.message().getMessageByMsgId(message.account, message.inreplyto); - for (EntityMessage replied : replieds) - if (replied.uid != null) - EntityOperation.queue(this, db, replied, EntityOperation.ANSWERED, true); - } - - db.identity().setIdentityConnected(ident.id, new Date().getTime()); - db.identity().setIdentityError(ident.id, null); - - NotificationManager nm = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE); - nm.cancel("send", message.identity.intValue()); - - if (message.to != null) - for (Address recipient : message.to) { - String email = ((InternetAddress) recipient).getAddress(); - String name = ((InternetAddress) recipient).getPersonal(); - List contacts = db.contact().getContacts(EntityContact.TYPE_TO, email); - if (contacts.size() == 0) { - EntityContact contact = new EntityContact(); - contact.type = EntityContact.TYPE_TO; - contact.email = email; - contact.name = name; - db.contact().insertContact(contact); - Log.i("Inserted recipient contact=" + contact); - } else { - EntityContact contact = contacts.get(0); - if (name != null && !name.equals(contact.name)) { - contact.name = name; - db.contact().updateContact(contact); - Log.i("Updated recipient contact=" + contact); - } - } - } - - - } catch (Throwable ex) { - if (sid != null) - db.message().deleteMessage(sid); - throw ex; + db.setTransactionSuccessful(); + } finally { + db.endTransaction(); } + + if (refFile.exists()) + refFile.delete(); + + if (message.inreplyto != null) { + List replieds = db.message().getMessageByMsgId(message.account, message.inreplyto); + for (EntityMessage replied : replieds) + if (replied.uid != null) + EntityOperation.queue(this, db, replied, EntityOperation.ANSWERED, true); + } + + db.identity().setIdentityConnected(ident.id, new Date().getTime()); + db.identity().setIdentityError(ident.id, null); + + NotificationManager nm = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE); + nm.cancel("send", message.identity.intValue()); + + if (message.to != null) + for (Address recipient : message.to) { + String email = ((InternetAddress) recipient).getAddress(); + String name = ((InternetAddress) recipient).getPersonal(); + List contacts = db.contact().getContacts(EntityContact.TYPE_TO, email); + if (contacts.size() == 0) { + EntityContact contact = new EntityContact(); + contact.type = EntityContact.TYPE_TO; + contact.email = email; + contact.name = name; + db.contact().insertContact(contact); + Log.i("Inserted recipient contact=" + contact); + } else { + EntityContact contact = contacts.get(0); + if (name != null && !name.equals(contact.name)) { + contact.name = name; + db.contact().updateContact(contact); + Log.i("Updated recipient contact=" + contact); + } + } + } } catch (MessagingException ex) { if (ex instanceof SendFailedException) { SendFailedException sfe = (SendFailedException) ex; @@ -2532,12 +2487,13 @@ public class ServiceSynchronize extends LifecycleService { // Add local sent messages to remote sent folder if (EntityFolder.SENT.equals(folder.type)) { - List orphans = db.message().getSentOrphans(folder.id); - Log.i(folder.name + " sent orphans=" + orphans.size()); + List orphans = db.message().getSentOrphans(folder.account); + Log.i(folder.name + " sent orphans=" + orphans.size() + " account=" + folder.account); for (EntityMessage orphan : orphans) { - Log.i(folder.name + " adding orphan id=" + orphan.id); + Log.i(folder.name + " adding orphan id=" + orphan.id + " sent=" + new Date(orphan.sent)); + orphan.folder = folder.id; + db.message().updateMessage(orphan); EntityOperation.queue(this, db, orphan, EntityOperation.ADD); - db.message().setMessageUiBrowsed(orphan.id, false); // Prevent adding again } } @@ -2631,7 +2587,8 @@ public class ServiceSynchronize extends LifecycleService { " folder=" + dfolder.type + ":" + dup.folder + "/" + folder.type + ":" + folder.id + " msgid=" + dup.msgid + " thread=" + dup.thread); - if (dup.folder.equals(folder.id)) { + if (dup.folder.equals(folder.id) || + (EntityFolder.OUTBOX.equals(dfolder.type) && EntityFolder.SENT.equals(folder.type))) { String thread = helper.getThreadId(uid); Log.i(folder.name + " found as id=" + dup.id + " uid=" + dup.uid + "/" + uid + diff --git a/app/src/main/res/layout/fragment_identity.xml b/app/src/main/res/layout/fragment_identity.xml index 87122e9594..ee66bc87c8 100644 --- a/app/src/main/res/layout/fragment_identity.xml +++ b/app/src/main/res/layout/fragment_identity.xml @@ -485,25 +485,6 @@ app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@id/cbReadReceipt" /> - - - -