Better error handling if attach file doesnt work (on permission denied)

Daniel Gultsch created

and unified encrypted and unencrypted file attachment

Change summary

res/values/strings.xml                                         |  5 
src/eu/siacs/conversations/crypto/OnPgpEngineResult.java       | 11 
src/eu/siacs/conversations/crypto/PgpEngine.java               | 33 +-
src/eu/siacs/conversations/persistance/FileBackend.java        | 12 
src/eu/siacs/conversations/services/XmppConnectionService.java | 48 +--
src/eu/siacs/conversations/ui/ConversationActivity.java        | 54 ++-
src/eu/siacs/conversations/ui/ConversationFragment.java        | 15 
src/eu/siacs/conversations/ui/ManageAccountActivity.java       |  1 
src/eu/siacs/conversations/ui/UiCallback.java                  |  9 
src/eu/siacs/conversations/ui/XmppActivity.java                | 26 +
10 files changed, 112 insertions(+), 102 deletions(-)

Detailed changes

res/values/strings.xml 🔗

@@ -115,4 +115,9 @@
     <string name="pref_advanced_options">Advanced Options</string>
     <string name="pref_never_send_crash">Never send crash reports</string>
     <string name="pref_never_send_crash_summary">By sending in stack traces you are helping the ongoing development of Conversations</string>
+    <string name="openpgp_error">OpenKeychain reporeted an error</string>
+    <string name="error_decrypting_file">I/O Error decrypting file</string>
+    <string name="error_copying_image_file">Error copying image file.</string>
+    <string name="accept">Accept</string>
+    <string name="error">An error has occurred</string>
 </resources>

src/eu/siacs/conversations/crypto/OnPgpEngineResult.java 🔗

@@ -1,11 +0,0 @@
-package eu.siacs.conversations.crypto;
-
-import org.openintents.openpgp.OpenPgpError;
-
-import android.app.PendingIntent;
-
-public interface OnPgpEngineResult {
-	public void success();
-	public void error(OpenPgpError openPgpError);
-	public void userInputRequried(PendingIntent pi);
-}

src/eu/siacs/conversations/crypto/PgpEngine.java 🔗

@@ -14,12 +14,13 @@ import org.openintents.openpgp.OpenPgpSignatureResult;
 import org.openintents.openpgp.util.OpenPgpApi;
 import org.openintents.openpgp.util.OpenPgpApi.IOpenPgpCallback;
 
+import eu.siacs.conversations.R;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.entities.Contact;
 import eu.siacs.conversations.entities.Message;
 import eu.siacs.conversations.services.XmppConnectionService;
+import eu.siacs.conversations.ui.UiCallback;
 import eu.siacs.conversations.xmpp.jingle.JingleFile;
-
 import android.app.PendingIntent;
 import android.content.Intent;
 import android.graphics.BitmapFactory;
@@ -34,7 +35,7 @@ public class PgpEngine {
 		this.mXmppConnectionService = service;
 	}
 
-	public void decrypt(final Message message, final OnPgpEngineResult callback) {
+	public void decrypt(final Message message, final UiCallback callback) {
 		Log.d("xmppService","decrypting message "+message.getUuid());
 		Intent params = new Intent();
 		params.setAction(OpenPgpApi.ACTION_DECRYPT_VERIFY);
@@ -59,8 +60,7 @@ public class PgpEngine {
 								.getParcelableExtra(OpenPgpApi.RESULT_INTENT));
 						return;
 					case OpenPgpApi.RESULT_CODE_ERROR:
-						callback.error((OpenPgpError) result
-								.getParcelableExtra(OpenPgpApi.RESULT_ERROR));
+						callback.error(R.string.openpgp_error);
 						return;
 					default:
 						return;
@@ -97,8 +97,7 @@ public class PgpEngine {
 									.getParcelableExtra(OpenPgpApi.RESULT_INTENT));
 							return;
 						case OpenPgpApi.RESULT_CODE_ERROR:
-							callback.error((OpenPgpError) result
-									.getParcelableExtra(OpenPgpApi.RESULT_ERROR));
+							callback.error(R.string.openpgp_error);
 							return;
 						default:
 							return;
@@ -106,15 +105,15 @@ public class PgpEngine {
 					}
 				});
 			} catch (FileNotFoundException e) {
-				callback.error(new OpenPgpError(0, "file not found: "+e.getMessage()));
+				callback.error(R.string.error_decrypting_file);
 			} catch (IOException e) {
-				callback.error(new OpenPgpError(0, "io exception: "+e.getMessage()));
+				callback.error(R.string.error_decrypting_file);
 			}
 			
 		}
 	}
 
-	public void encrypt(final Message message,final OnPgpEngineResult callback) {
+	public void encrypt(final Message message,final UiCallback callback) {
 		long[] keys = { message.getConversation().getContact().getPgpKeyId() };
 		Intent params = new Intent();
 		params.setAction(OpenPgpApi.ACTION_ENCRYPT);
@@ -146,8 +145,7 @@ public class PgpEngine {
 								.getParcelableExtra(OpenPgpApi.RESULT_INTENT));
 						break;
 					case OpenPgpApi.RESULT_CODE_ERROR:
-						callback.error((OpenPgpError) result
-								.getParcelableExtra(OpenPgpApi.RESULT_ERROR));
+						callback.error(R.string.openpgp_error);
 						break;
 					}
 				}
@@ -173,8 +171,7 @@ public class PgpEngine {
 									.getParcelableExtra(OpenPgpApi.RESULT_INTENT));
 							break;
 						case OpenPgpApi.RESULT_CODE_ERROR:
-							callback.error((OpenPgpError) result
-									.getParcelableExtra(OpenPgpApi.RESULT_ERROR));
+							callback.error(R.string.openpgp_error);
 							break;
 						}
 					}
@@ -235,7 +232,7 @@ public class PgpEngine {
 	}
 
 	public void generateSignature(final Account account, String status,
-			final OnPgpEngineResult callback) {
+			final UiCallback callback) {
 		Intent params = new Intent();
 		params.putExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true);
 		params.setAction(OpenPgpApi.ACTION_SIGN);
@@ -261,15 +258,14 @@ public class PgpEngine {
 							.getParcelableExtra(OpenPgpApi.RESULT_INTENT));
 					return;
 				case OpenPgpApi.RESULT_CODE_ERROR:
-					callback.error((OpenPgpError) result
-							.getParcelableExtra(OpenPgpApi.RESULT_ERROR));
+					callback.error(R.string.openpgp_error);
 					return;
 				}
 			}
 		});
 	}
 	
-	public void hasKey(Contact contact, final OnPgpEngineResult callback) {
+	public void hasKey(Contact contact, final UiCallback callback) {
 		Intent params = new Intent();
 		params.setAction(OpenPgpApi.ACTION_GET_KEY);
 		params.putExtra(OpenPgpApi.EXTRA_KEY_ID, contact.getPgpKeyId());
@@ -287,8 +283,7 @@ public class PgpEngine {
 							.getParcelableExtra(OpenPgpApi.RESULT_INTENT));
 					return;
 				case OpenPgpApi.RESULT_CODE_ERROR:
-					callback.error((OpenPgpError) result
-							.getParcelableExtra(OpenPgpApi.RESULT_ERROR));
+					callback.error(R.string.openpgp_error);
 					return;
 				}
 			}

src/eu/siacs/conversations/persistance/FileBackend.java 🔗

@@ -102,7 +102,7 @@ public class FileBackend {
 			boolean success = scalledBitmap.compress(
 					Bitmap.CompressFormat.WEBP, 75, os);
 			if (!success) {
-				// Log.d("xmppService", "couldnt compress");
+				return null;
 			}
 			os.flush();
 			os.close();
@@ -112,14 +112,12 @@ public class FileBackend {
 			message.setBody(""+size+","+width+","+height);
 			return file;
 		} catch (FileNotFoundException e) {
-			// TODO Auto-generated catch block
-			e.printStackTrace();
+			return null;
 		} catch (IOException e) {
-			// TODO Auto-generated catch block
-			e.printStackTrace();
+			return null;
+		} catch (SecurityException e) {
+			return null;
 		}
-
-		return null;
 	}
 
 	public Bitmap getImageFromMessage(Message message) {

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

@@ -17,8 +17,7 @@ import org.openintents.openpgp.util.OpenPgpServiceConnection;
 import net.java.otr4j.OtrException;
 import net.java.otr4j.session.Session;
 import net.java.otr4j.session.SessionStatus;
-
-import eu.siacs.conversations.crypto.OnPgpEngineResult;
+import eu.siacs.conversations.R;
 import eu.siacs.conversations.crypto.PgpEngine;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.entities.Contact;
@@ -33,6 +32,7 @@ import eu.siacs.conversations.persistance.OnPhoneContactsMerged;
 import eu.siacs.conversations.ui.OnAccountListChangedListener;
 import eu.siacs.conversations.ui.OnConversationListChangedListener;
 import eu.siacs.conversations.ui.OnRosterFetchedListener;
+import eu.siacs.conversations.ui.UiCallback;
 import eu.siacs.conversations.utils.ExceptionHelper;
 import eu.siacs.conversations.utils.MessageParser;
 import eu.siacs.conversations.utils.OnPhoneContactsLoadedListener;
@@ -446,45 +446,35 @@ public class XmppConnectionService extends Service {
 		return this.fileBackend;
 	}
 
-	public Message attachImageToConversation(final Conversation conversation,
-			final String presence, final Uri uri) {
-		final Message message = new Message(conversation, "",Message.ENCRYPTION_NONE);
-		message.setPresence(presence);
+	public Message attachImageToConversation(final Conversation conversation, final Uri uri,  final UiCallback callback) {
+		final Message message;
+		if (conversation.getNextEncryption() == Message.ENCRYPTION_PGP) {
+			message = new Message(conversation, "",Message.ENCRYPTION_DECRYPTED);
+		} else {
+			message = new Message(conversation, "", Message.ENCRYPTION_NONE);
+		}
+		message.setPresence(conversation.getNextPresence());
 		message.setType(Message.TYPE_IMAGE);
 		message.setStatus(Message.STATUS_OFFERED);
 		new Thread(new Runnable() {
 
 			@Override
 			public void run() {
-				getFileBackend().copyImageToPrivateStorage(message, uri);
-				databaseBackend.createMessage(message);
-				conversation.getMessages().add(message);
-				if (convChangedListener != null) {
-					convChangedListener.onConversationListChanged();
+				JingleFile file = getFileBackend().copyImageToPrivateStorage(message, uri);
+				if (file==null) {
+					callback.error(R.string.error_copying_image_file);
+				} else {
+					if (conversation.getNextEncryption() == Message.ENCRYPTION_PGP) {
+						getPgpEngine().encrypt(message, callback);
+					} else {
+						callback.success();
+					}
 				}
-				sendMessage(message, null);
 			}
 		}).start();
 		return message;
 	}
 	
-	public Message attachEncryptedImageToConversation(final Conversation conversation, final String presence, final Uri uri, final OnPgpEngineResult callback) {
-		Log.d(LOGTAG,"attach encrypted image");
-		final Message message = new Message(conversation, "",Message.ENCRYPTION_DECRYPTED);
-		message.setPresence(presence);
-		message.setType(Message.TYPE_IMAGE);
-		message.setStatus(Message.STATUS_OFFERED);
-		new Thread(new Runnable() {
-
-			@Override
-			public void run() {
-				getFileBackend().copyImageToPrivateStorage(message, uri);
-				getPgpEngine().encrypt(message, callback);
-			}
-		}).start();
-		return message;
-	}
-
 	protected Conversation findMuc(String name, Account account) {
 		for (Conversation conversation : this.conversations) {
 			if (conversation.getContactJid().split("/")[0].equals(name)

src/eu/siacs/conversations/ui/ConversationActivity.java 🔗

@@ -9,7 +9,6 @@ import java.util.List;
 import org.openintents.openpgp.OpenPgpError;
 
 import eu.siacs.conversations.R;
-import eu.siacs.conversations.crypto.OnPgpEngineResult;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.entities.Contact;
 import eu.siacs.conversations.entities.Conversation;
@@ -354,7 +353,7 @@ public class ConversationActivity extends XmppActivity {
 		if (conversation.getNextEncryption() == Message.ENCRYPTION_PGP) {
 			if (hasPgp()) {
 				if (conversation.getContact().getPgpKeyId()!=0) {
-					xmppConnectionService.getPgpEngine().hasKey(conversation.getContact(), new OnPgpEngineResult() {
+					xmppConnectionService.getPgpEngine().hasKey(conversation.getContact(), new UiCallback() {
 						
 						@Override
 						public void userInputRequried(PendingIntent pi) {
@@ -367,7 +366,7 @@ public class ConversationActivity extends XmppActivity {
 						}
 						
 						@Override
-						public void error(OpenPgpError openPgpError) {
+						public void error(int error) {
 							// TODO Auto-generated method stub
 							
 						}
@@ -671,10 +670,26 @@ public class ConversationActivity extends XmppActivity {
 			} else if (requestCode == REQUEST_ATTACH_FILE_DIALOG) {
 				prepareImageToast = Toast.makeText(getApplicationContext(), getText(R.string.preparing_image), Toast.LENGTH_LONG);
 				final Conversation conversation = getSelectedConversation();
-				String presence = conversation.getNextPresence();
 				if (conversation.getNextEncryption() == Message.ENCRYPTION_NONE) {
 					prepareImageToast.show();
-					xmppConnectionService.attachImageToConversation(conversation, presence, data.getData());
+					this.pendingMessage = xmppConnectionService.attachImageToConversation(conversation, data.getData(),new UiCallback() {
+						
+						@Override
+						public void userInputRequried(PendingIntent pi) {
+							
+						}
+						
+						@Override
+						public void success() {
+							sendPendingImageMessage();
+						}
+						
+						@Override
+						public void error(int error) {
+							pendingMessage = null;
+							displayErrorDialog(error);
+						}
+					});
 				} else if (conversation.getNextEncryption() == Message.ENCRYPTION_PGP) {
 					prepareImageToast.show();
 					attachPgpFile(conversation,data.getData());
@@ -696,8 +711,7 @@ public class ConversationActivity extends XmppActivity {
 	}
 	
 	private void attachPgpFile(Conversation conversation, Uri uri) {
-			String presence = conversation.getNextPresence();
-			pendingMessage = xmppConnectionService.attachEncryptedImageToConversation(conversation, presence, uri, new OnPgpEngineResult() {
+			pendingMessage = xmppConnectionService.attachImageToConversation(conversation, uri, new UiCallback() {
 				
 				@Override
 				public void userInputRequried(PendingIntent pi) {
@@ -706,20 +720,25 @@ public class ConversationActivity extends XmppActivity {
 				
 				@Override
 				public void success() {
-					pendingMessage.getConversation().getMessages().add(pendingMessage);
-					xmppConnectionService.databaseBackend.createMessage(pendingMessage);
-					xmppConnectionService.sendMessage(pendingMessage, null);
-					xmppConnectionService.updateUi(pendingMessage.getConversation(), false);
-					pendingMessage = null;
+					sendPendingImageMessage();
 				}
 				
 				@Override
-				public void error(OpenPgpError openPgpError) {
-					Log.d(LOGTAG,"pgp error"+openPgpError.getMessage());
+				public void error(int error) {
+					pendingMessage = null;
+					displayErrorDialog(error);
 				}
 			});
 	}
 
+	private void sendPendingImageMessage() {
+		pendingMessage.getConversation().getMessages().add(pendingMessage);
+		xmppConnectionService.databaseBackend.createMessage(pendingMessage);
+		xmppConnectionService.sendMessage(pendingMessage, null);
+		xmppConnectionService.updateUi(pendingMessage.getConversation(), false);
+		pendingMessage = null;
+	}
+	
 	public void updateConversationList() {
 		conversationList.clear();
 		conversationList.addAll(xmppConnectionService.getConversations());
@@ -925,7 +944,7 @@ public class ConversationActivity extends XmppActivity {
 	}
 
 	public void encryptTextMessage() {
-		xmppConnectionService.getPgpEngine().encrypt(this.pendingMessage, new OnPgpEngineResult() {
+		xmppConnectionService.getPgpEngine().encrypt(this.pendingMessage, new UiCallback() {
 
 					@Override
 					public void userInputRequried(
@@ -947,10 +966,7 @@ public class ConversationActivity extends XmppActivity {
 					}
 
 					@Override
-					public void error(
-							OpenPgpError openPgpError) {
-						// TODO Auto-generated method
-						// stub
+					public void error(int error) {
 
 					}
 				});

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

@@ -8,9 +8,7 @@ import java.util.Set;
 import org.openintents.openpgp.OpenPgpError;
 
 import net.java.otr4j.session.SessionStatus;
-
 import eu.siacs.conversations.R;
-import eu.siacs.conversations.crypto.OnPgpEngineResult;
 import eu.siacs.conversations.crypto.PgpEngine;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.entities.Contact;
@@ -560,7 +558,7 @@ public class ConversationFragment extends Fragment {
 	private void decryptMessage(final Message message) {
 		PgpEngine engine = activity.xmppConnectionService.getPgpEngine();
 		if (engine != null) {
-			engine.decrypt(message, new OnPgpEngineResult() {
+			engine.decrypt(message, new UiCallback() {
 
 				@Override
 				public void userInputRequried(PendingIntent pi) {
@@ -576,9 +574,7 @@ public class ConversationFragment extends Fragment {
 				}
 
 				@Override
-				public void error(OpenPgpError openPgpError) {
-					Log.d("xmppService",
-							"decryption error" + openPgpError.getMessage());
+				public void error(int error) {
 					message.setEncryption(Message.ENCRYPTION_DECRYPTION_FAILED);
 					// updateMessages();
 				}
@@ -680,7 +676,7 @@ public class ConversationFragment extends Fragment {
 		if (activity.hasPgp()) {
 			if (contact.getPgpKeyId() != 0) {
 				xmppService.getPgpEngine().hasKey(contact,
-						new OnPgpEngineResult() {
+						new UiCallback() {
 
 							@Override
 							public void userInputRequried(PendingIntent pi) {
@@ -695,9 +691,8 @@ public class ConversationFragment extends Fragment {
 							}
 
 							@Override
-							public void error(OpenPgpError openPgpError) {
-								Log.d("xmppService", "openpgp error"
-										+ openPgpError.getMessage());
+							public void error(int error) {
+								
 							}
 						});
 

src/eu/siacs/conversations/ui/ManageAccountActivity.java 🔗

@@ -6,7 +6,6 @@ import java.util.List;
 import org.openintents.openpgp.OpenPgpError;
 
 import eu.siacs.conversations.R;
-import eu.siacs.conversations.crypto.OnPgpEngineResult;
 import eu.siacs.conversations.crypto.PgpEngine;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.ui.EditAccount.EditAccountListener;

src/eu/siacs/conversations/ui/UiCallback.java 🔗

@@ -0,0 +1,9 @@
+package eu.siacs.conversations.ui;
+
+import android.app.PendingIntent;
+
+public interface UiCallback {
+	public void success();
+	public void error(int errorCode);
+	public void userInputRequried(PendingIntent pi);
+}

src/eu/siacs/conversations/ui/XmppActivity.java 🔗

@@ -1,9 +1,8 @@
 package eu.siacs.conversations.ui;
 
-import org.openintents.openpgp.OpenPgpError;
+import java.nio.channels.AlreadyConnectedException;
 
 import eu.siacs.conversations.R;
-import eu.siacs.conversations.crypto.OnPgpEngineResult;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.entities.Conversation;
 import eu.siacs.conversations.entities.Message;
@@ -164,7 +163,7 @@ public abstract class XmppActivity extends Activity {
 	}
 	
 	protected void announcePgp(final Account account, final Conversation conversation) {
-		xmppConnectionService.getPgpEngine().generateSignature(account, "online", new OnPgpEngineResult() {
+		xmppConnectionService.getPgpEngine().generateSignature(account, "online", new UiCallback() {
 			
 			@Override
 			public void userInputRequried(PendingIntent pi) {
@@ -185,10 +184,25 @@ public abstract class XmppActivity extends Activity {
 			}
 			
 			@Override
-			public void error(OpenPgpError openPgpError) {
-				// TODO Auto-generated method stub
-				
+			public void error(int error) {
+				displayErrorDialog(error);
+			}
+		});
+	}
+	
+	protected void displayErrorDialog(final int errorCode) {
+		runOnUiThread(new Runnable() {
+			
+			@Override
+			public void run() {
+				AlertDialog.Builder builder = new AlertDialog.Builder(XmppActivity.this);
+				builder.setIconAttribute(android.R.attr.alertDialogIcon);
+				builder.setTitle(getString(R.string.error));
+				builder.setMessage(errorCode);
+				builder.setNeutralButton(R.string.accept, null);
+				builder.create().show();
 			}
 		});
+		
 	}
 }