add error message to failed messages. accessible via context menu

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/entities/Message.java               | 23 
src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java      | 17 
src/main/java/eu/siacs/conversations/parser/AbstractParser.java          | 14 
src/main/java/eu/siacs/conversations/parser/MessageParser.java           | 16 
src/main/java/eu/siacs/conversations/persistance/DatabaseBackend.java    |  7 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java | 15 
src/main/java/eu/siacs/conversations/ui/ConversationFragment.java        | 15 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java   | 12 
src/main/res/menu/message_context.xml                                    |  4 
src/main/res/values/strings.xml                                          |  2 
10 files changed, 98 insertions(+), 27 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/entities/Message.java 🔗

@@ -58,6 +58,7 @@ public class Message extends AbstractEntity {
 	public static final String RELATIVE_FILE_PATH = "relativeFilePath";
 	public static final String FINGERPRINT = "axolotl_fingerprint";
 	public static final String READ = "read";
+	public static final String ERROR_MESSAGE = "errorMsg";
 	public static final String ME_COMMAND = "/me ";
 
 
@@ -83,6 +84,7 @@ public class Message extends AbstractEntity {
 	private Message mNextMessage = null;
 	private Message mPreviousMessage = null;
 	private String axolotlFingerprint = null;
+	private String errorMessage = null;
 
 	private Message() {
 
@@ -109,7 +111,8 @@ public class Message extends AbstractEntity {
 				null,
 				true,
 				null,
-				false);
+				false,
+				null);
 		this.conversation = conversation;
 	}
 
@@ -118,7 +121,7 @@ public class Message extends AbstractEntity {
 					final int encryption, final int status, final int type, final boolean carbon,
 					final String remoteMsgId, final String relativeFilePath,
 					final String serverMsgId, final String fingerprint, final boolean read,
-					final String edited, final boolean oob) {
+					final String edited, final boolean oob, final String errorMessage) {
 		this.uuid = uuid;
 		this.conversationUuid = conversationUUid;
 		this.counterpart = counterpart;
@@ -136,6 +139,7 @@ public class Message extends AbstractEntity {
 		this.read = read;
 		this.edited = edited;
 		this.oob = oob;
+		this.errorMessage = errorMessage;
 	}
 
 	public static Message fromCursor(Cursor cursor) {
@@ -177,7 +181,8 @@ public class Message extends AbstractEntity {
 				cursor.getString(cursor.getColumnIndex(FINGERPRINT)),
 				cursor.getInt(cursor.getColumnIndex(READ)) > 0,
 				cursor.getString(cursor.getColumnIndex(EDITED)),
-				cursor.getInt(cursor.getColumnIndex(OOB)) > 0);
+				cursor.getInt(cursor.getColumnIndex(OOB)) > 0,
+				cursor.getString(cursor.getColumnIndex(ERROR_MESSAGE)));
 	}
 
 	public static Message createStatusMessage(Conversation conversation, String body) {
@@ -224,6 +229,7 @@ public class Message extends AbstractEntity {
 		values.put(READ,read ? 1 : 0);
 		values.put(EDITED, edited);
 		values.put(OOB, oob ? 1 : 0);
+		values.put(ERROR_MESSAGE,errorMessage);
 		return values;
 	}
 
@@ -271,6 +277,17 @@ public class Message extends AbstractEntity {
 		this.body = body;
 	}
 
+	public String getErrorMessage() {
+		return errorMessage;
+	}
+
+	public boolean setErrorMessage(String message) {
+		boolean changed = (message != null && !message.equals(errorMessage))
+				|| (message == null && errorMessage != null);
+		this.errorMessage = message;
+		return changed;
+	}
+
 	public long getTimeSent() {
 		return timeSent;
 	}

src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java 🔗

@@ -20,6 +20,7 @@ import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.entities.DownloadableFile;
 import eu.siacs.conversations.entities.Message;
 import eu.siacs.conversations.entities.Transferable;
+import eu.siacs.conversations.parser.IqParser;
 import eu.siacs.conversations.persistance.FileBackend;
 import eu.siacs.conversations.services.AbstractConnectionManager;
 import eu.siacs.conversations.services.XmppConnectionService;
@@ -86,10 +87,10 @@ public class HttpUploadConnection implements Transferable {
 		this.canceled = true;
 	}
 
-	private void fail() {
+	private void fail(String errorMessage) {
 		mHttpConnectionManager.finishUploadConnection(this);
 		message.setTransferable(null);
-		mXmppConnectionService.markMessage(message, Message.STATUS_SEND_FAILED);
+		mXmppConnectionService.markMessage(message, Message.STATUS_SEND_FAILED, errorMessage);
 		FileBackend.close(mFileInputStream);
 	}
 
@@ -111,7 +112,7 @@ public class HttpUploadConnection implements Transferable {
 			pair = AbstractConnectionManager.createInputStream(file, true);
 		} catch (FileNotFoundException e) {
 			Log.d(Config.LOGTAG,account.getJid().toBareJid()+": could not find file to upload - "+e.getMessage());
-			fail();
+			fail(e.getMessage());
 			return;
 		}
 		this.file.setExpectedSize(pair.second);
@@ -137,7 +138,7 @@ public class HttpUploadConnection implements Transferable {
 					}
 				}
 				Log.d(Config.LOGTAG,account.getJid().toString()+": invalid response to slot request "+packet);
-				fail();
+				fail(IqParser.extractErrorMessage(packet));
 			}
 		});
 		message.setTransferable(this);
@@ -206,12 +207,12 @@ public class HttpUploadConnection implements Transferable {
 							@Override
 							public void error(int errorCode, Message object) {
 								Log.d(Config.LOGTAG,"pgp encryption failed");
-								fail();
+								fail("pgp encryption failed");
 							}
 
 							@Override
 							public void userInputRequried(PendingIntent pi, Message object) {
-								fail();
+								fail("pgp encryption failed");
 							}
 						});
 					} else {
@@ -219,12 +220,12 @@ public class HttpUploadConnection implements Transferable {
 					}
 				} else {
 					Log.d(Config.LOGTAG,"http upload failed because response code was "+code);
-					fail();
+					fail("http upload failed because response code was "+code);
 				}
 			} catch (IOException e) {
 				e.printStackTrace();
 				Log.d(Config.LOGTAG,"http upload failed "+e.getMessage());
-				fail();
+				fail(e.getMessage());
 			} finally {
 				FileBackend.close(mFileInputStream);
 				FileBackend.close(os);

src/main/java/eu/siacs/conversations/parser/AbstractParser.java 🔗

@@ -92,4 +92,18 @@ public abstract class AbstractParser {
 		user.setRole(role);
 		return user;
 	}
+
+	public static String extractErrorMessage(Element packet) {
+		final Element error = packet.findChild("error");
+		if (error != null && error.getChildren().size() > 0) {
+			final String text = error.findChildContent("text");
+			if (text != null && !text.trim().isEmpty()) {
+				return text;
+			} else {
+				return error.getChildren().get(0).getName().replace("-"," ");
+			}
+		} else {
+			return null;
+		}
+	}
 }

src/main/java/eu/siacs/conversations/parser/MessageParser.java 🔗

@@ -266,19 +266,15 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
 		if (packet.getType() == MessagePacket.TYPE_ERROR) {
 			Jid from = packet.getFrom();
 			if (from != null) {
-				Element error = packet.findChild("error");
-				String text = error == null ? null : error.findChildContent("text");
-				if (text != null) {
-					Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": sending message to "+ from+ " failed - " + text);
-				} else if (error != null) {
-					Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": sending message to "+ from+ " failed - " + error);
-				}
 				Message message = mXmppConnectionService.markMessage(account,
 						from.toBareJid(),
 						packet.getId(),
-						Message.STATUS_SEND_FAILED);
-				if (message != null && message.getEncryption() == Message.ENCRYPTION_OTR) {
-					message.getConversation().endOtrIfNeeded();
+						Message.STATUS_SEND_FAILED,
+						extractErrorMessage(packet));
+				if (message != null) {
+					if (message.getEncryption() == Message.ENCRYPTION_OTR) {
+						message.getConversation().endOtrIfNeeded();
+					}
 				}
 			}
 			return true;

src/main/java/eu/siacs/conversations/persistance/DatabaseBackend.java 🔗

@@ -54,7 +54,7 @@ public class DatabaseBackend extends SQLiteOpenHelper {
 	private static DatabaseBackend instance = null;
 
 	private static final String DATABASE_NAME = "history";
-	private static final int DATABASE_VERSION = 28;
+	private static final int DATABASE_VERSION = 29;
 
 	private static String CREATE_CONTATCS_STATEMENT = "create table "
 			+ Contact.TABLENAME + "(" + Contact.ACCOUNT + " TEXT, "
@@ -181,6 +181,7 @@ public class DatabaseBackend extends SQLiteOpenHelper {
 				+ Message.EDITED + " TEXT, "
 				+ Message.READ + " NUMBER DEFAULT 1, "
 				+ Message.OOB + " INTEGER, "
+				+ Message.ERROR_MESSAGE + " TEXT,"
 				+ Message.REMOTE_MSG_ID + " TEXT, FOREIGN KEY("
 				+ Message.CONVERSATION + ") REFERENCES "
 				+ Conversation.TABLENAME + "(" + Conversation.UUID
@@ -333,6 +334,10 @@ public class DatabaseBackend extends SQLiteOpenHelper {
 		if (oldVersion < 28 && newVersion >= 28) {
 			canonicalizeJids(db);
 		}
+
+		if (oldVersion < 29 && newVersion >= 29) {
+			db.execSQL("ALTER TABLE " + Message.TABLENAME + " ADD COLUMN " + Message.ERROR_MESSAGE + " TEXT");
+		}
 	}
 
 	private void canonicalizeJids(SQLiteDatabase db) {

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java 🔗

@@ -2946,6 +2946,10 @@ public class XmppConnectionService extends Service {
 	}
 
 	public Message markMessage(final Account account, final Jid recipient, final String uuid, final int status) {
+		return markMessage(account, recipient, uuid, status, null);
+	}
+
+	public Message markMessage(final Account account, final Jid recipient, final String uuid, final int status, String errorMessage) {
 		if (uuid == null) {
 			return null;
 		}
@@ -2953,7 +2957,7 @@ public class XmppConnectionService extends Service {
 			if (conversation.getJid().toBareJid().equals(recipient) && conversation.getAccount() == account) {
 				final Message message = conversation.findSentMessageWithUuidOrRemoteId(uuid);
 				if (message != null) {
-					markMessage(message, status);
+					markMessage(message, status, errorMessage);
 				}
 				return message;
 			}
@@ -2976,11 +2980,18 @@ public class XmppConnectionService extends Service {
 	}
 
 	public void markMessage(Message message, int status) {
+		markMessage(message, status, null);
+	}
+
+
+	public void markMessage(Message message, int status, String errorMessage) {
 		if (status == Message.STATUS_SEND_FAILED
 				&& (message.getStatus() == Message.STATUS_SEND_RECEIVED || message
-				.getStatus() == Message.STATUS_SEND_DISPLAYED)) {
+				.getStatus() == Message.STATUS_SEND_DISPLAYED)
+				&& errorMessage == null) {
 			return;
 		}
+		message.setErrorMessage(errorMessage);
 		message.setStatus(status);
 		databaseBackend.updateMessage(message);
 		updateConversationUi();

src/main/java/eu/siacs/conversations/ui/ConversationFragment.java 🔗

@@ -546,6 +546,7 @@ public class ConversationFragment extends Fragment implements EditMessage.Keyboa
 			MenuItem downloadFile = menu.findItem(R.id.download_file);
 			MenuItem cancelTransmission = menu.findItem(R.id.cancel_transmission);
 			MenuItem deleteFile = menu.findItem(R.id.delete_file);
+			MenuItem showErrorMessage = menu.findItem(R.id.show_error_message);
 			if (!treatAsFile
 					&& !GeoHelper.isGeoUri(m.getBody())
 					&& m.treatAsDownloadable() != Message.Decision.MUST) {
@@ -589,6 +590,9 @@ public class ConversationFragment extends Fragment implements EditMessage.Keyboa
 					deleteFile.setTitle(activity.getString(R.string.delete_x_file, UIHelper.getFileDescriptionString(activity, m)));
 				}
 			}
+			if (m.getStatus() == Message.STATUS_SEND_FAILED && m.getErrorMessage() != null) {
+				showErrorMessage.setVisible(true);
+			}
 		}
 	}
 
@@ -625,11 +629,22 @@ public class ConversationFragment extends Fragment implements EditMessage.Keyboa
 			case R.id.delete_file:
 				deleteFile(selectedMessage);
 				return true;
+			case R.id.show_error_message:
+				showErrorMessage(selectedMessage);
+				return true;
 			default:
 				return super.onContextItemSelected(item);
 		}
 	}
 
+	private void showErrorMessage(final Message message) {
+		AlertDialog.Builder builder = new AlertDialog.Builder(activity);
+		builder.setTitle(R.string.error_message);
+		builder.setMessage(message.getErrorMessage());
+		builder.setPositiveButton(R.string.confirm,null);
+		builder.create().show();
+	}
+
 	private void shareWith(Message message) {
 		Intent shareIntent = new Intent();
 		shareIntent.setAction(Intent.ACTION_SEND);

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java 🔗

@@ -25,6 +25,7 @@ import eu.siacs.conversations.entities.Presence;
 import eu.siacs.conversations.entities.ServiceDiscoveryResult;
 import eu.siacs.conversations.entities.Transferable;
 import eu.siacs.conversations.entities.TransferablePlaceholder;
+import eu.siacs.conversations.parser.IqParser;
 import eu.siacs.conversations.persistance.FileBackend;
 import eu.siacs.conversations.services.AbstractConnectionManager;
 import eu.siacs.conversations.services.XmppConnectionService;
@@ -87,7 +88,7 @@ public class JingleConnection implements Transferable {
 		@Override
 		public void onIqPacketReceived(Account account, IqPacket packet) {
 			if (packet.getType() != IqPacket.TYPE.RESULT) {
-				fail();
+				fail(IqParser.extractErrorMessage(packet));
 			}
 		}
 	};
@@ -488,7 +489,7 @@ public class JingleConnection implements Transferable {
 						mJingleStatus = JINGLE_STATUS_INITIATED;
 						mXmppConnectionService.markMessage(message, Message.STATUS_OFFERED);
 					} else {
-						fail();
+						fail(IqParser.extractErrorMessage(packet));
 					}
 				}
 			});
@@ -846,6 +847,10 @@ public class JingleConnection implements Transferable {
 	}
 
 	private void fail() {
+		fail(null);
+	}
+
+	private void fail(String errorMessage) {
 		this.mJingleStatus = JINGLE_STATUS_FAILED;
 		this.disconnectSocks5Connections();
 		if (this.transport != null && this.transport instanceof JingleInbandTransport) {
@@ -862,7 +867,8 @@ public class JingleConnection implements Transferable {
 				this.mXmppConnectionService.updateConversationUi();
 			} else {
 				this.mXmppConnectionService.markMessage(this.message,
-						Message.STATUS_SEND_FAILED);
+						Message.STATUS_SEND_FAILED,
+						errorMessage);
 				this.message.setTransferable(null);
 			}
 		}

src/main/res/menu/message_context.xml 🔗

@@ -25,6 +25,10 @@
         android:id="@+id/copy_url"
         android:title="@string/copy_original_url"
         android:visible="false"/>
+    <item
+        android:id="@+id/show_error_message"
+        android:title="@string/show_error_message"
+        android:visible="false"/>
     <item
         android:id="@+id/send_again"
         android:title="@string/send_again"

src/main/res/values/strings.xml 🔗

@@ -694,4 +694,6 @@
 	<string name="pref_delete_omemo_identities_summary">Regenerate your OMEMO keys. All your contacts will have to verify you again. Use this only as a last resort.</string>
 	<string name="delete_selected_keys">Delete selected keys</string>
 	<string name="error_publish_avatar_offline">You need to be connected to publish your avatar.</string>
+	<string name="show_error_message">Show error message</string>
+	<string name="error_message">Error Message</string>
 </resources>