Handle jgit errors (#243)

* initial work on the git error handling

* remove throws exception and handle the jsch one correctly

* move the commit task into its own operation

* get rid of the interface and rely on the abstract class GitOperation

* add error message to the pull command

* add error message to the push command

* add error message to the sync operationˆ

app/src/main/java/com/zeapo/pwdstore/PasswordStore.java

@@ -28,13 +28,13 @@ import android.widget.TextView;
 import com.zeapo.pwdstore.crypto.PgpHandler;
 import com.zeapo.pwdstore.git.GitActivity;
 import com.zeapo.pwdstore.git.GitAsyncTask;
+import com.zeapo.pwdstore.git.GitOperation;
 import com.zeapo.pwdstore.pwgen.PRNGFixes;
 import com.zeapo.pwdstore.utils.PasswordItem;
 import com.zeapo.pwdstore.utils.PasswordRecyclerAdapter;
 import com.zeapo.pwdstore.utils.PasswordRepository;
 
 import org.apache.commons.io.FileUtils;
-import org.eclipse.jgit.api.CommitCommand;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.lib.Repository;
 
@@ -57,6 +57,7 @@ public class PasswordStore extends AppCompatActivity {
     private final static int HOME = 403;
 
     private final static int REQUEST_EXTERNAL_STORAGE = 50;
+
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         settings = PreferenceManager.getDefaultSharedPreferences(this.getApplicationContext());
@@ -373,7 +374,6 @@ public class PasswordStore extends AppCompatActivity {
     }
 
 
-
     @Override
     public void onBackPressed() {
         if ((null != plist) && plist.isNotEmpty()) {
@@ -438,20 +438,12 @@ public class PasswordStore extends AppCompatActivity {
                 .setPositiveButton(this.getResources().getString(R.string.dialog_yes), new DialogInterface.OnClickListener() {
                     @Override
                     public void onClick(DialogInterface dialogInterface, int i) {
-                        String path = item.getFile().getAbsolutePath();
                         item.getFile().delete();
                         adapter.remove(position);
                         it.remove();
                         adapter.updateSelectedItems(position, selectedItems);
 
-                        setResult(RESULT_CANCELED);
+                        commit("[ANDROID PwdStore] Remove " + item + " from store.");
-                        Repository repo = PasswordRepository.getRepository(PasswordRepository.getRepositoryDirectory(activity));
-                        Git git = new Git(repo);
-                        GitAsyncTask tasks = new GitAsyncTask(activity, false, true, CommitCommand.class);
-                        tasks.execute(
-                                git.rm().addFilepattern(path.replace(PasswordRepository.getWorkTree() + "/", "")),
-                                git.commit().setMessage("[ANDROID PwdStore] Remove " + item + " from store.")
-                        );
                         deletePasswords(adapter, selectedItems);
                     }
                 })
@@ -507,14 +499,19 @@ public class PasswordStore extends AppCompatActivity {
         return PasswordRepository.getWorkTree();
     }
 
-    private void commit(String message) {
+    private void commit(final String message) {
-        Git git = new Git(PasswordRepository.getRepository(new File("")));
+        new GitOperation(PasswordRepository.getRepositoryDirectory(activity), activity) {
-        GitAsyncTask tasks = new GitAsyncTask(this, false, false, CommitCommand.class);
+            @Override
+            public void execute() {
+                Git git = new Git(this.repository);
+                GitAsyncTask tasks = new GitAsyncTask(activity, false, true, this);
                 tasks.execute(
                         git.add().addFilepattern("."),
                         git.commit().setMessage(message)
                 );
             }
+        };
+    }
 
     protected void onActivityResult(int requestCode, int resultCode,
                                     Intent data) {
@@ -581,10 +578,6 @@ public class PasswordStore extends AppCompatActivity {
                         break;
                     }
 
-                    Repository repo = PasswordRepository.getRepository(PasswordRepository.getRepositoryDirectory(activity));
-                    Git git = new Git(repo);
-                    GitAsyncTask tasks = new GitAsyncTask(activity, false, true, CommitCommand.class);
-
                     for (String string : data.getStringArrayListExtra("Files")) {
                         File source = new File(string);
                         if (!source.exists()) {
@@ -592,12 +585,14 @@ public class PasswordStore extends AppCompatActivity {
                             continue;
                         }
                         if (!source.renameTo(new File(target.getAbsolutePath() + "/" + source.getName()))) {
+                            // TODO this should show a warning to the user
                             Log.e("Moving", "Something went wrong while moving.");
                         } else {
-                            tasks.execute(
+                            commit("[ANDROID PwdStore] Moved "
-                                    git.add().addFilepattern(source.getAbsolutePath().replace(PasswordRepository.getWorkTree() + "/", "")),
+                                    + string.replace(PasswordRepository.getWorkTree() + "/", "")
-                                    git.commit().setMessage("[ANDROID PwdStore] Moved "+string.replace(PasswordRepository.getWorkTree() + "/", "")+" to "+target.getAbsolutePath().replace(PasswordRepository.getWorkTree() + "/","")+target.getAbsolutePath()+"/"+source.getName()+".")
+                                    + " to "
-                            );
+                                    + target.getAbsolutePath().replace(PasswordRepository.getWorkTree() + "/", "")
+                                    + target.getAbsolutePath() + "/" + source.getName() + ".");
                         }
                     }
                     updateListAdapter();

app/src/main/java/com/zeapo/pwdstore/git/CloneOperation.java

@@ -1,7 +1,13 @@
 package com.zeapo.pwdstore.git;
 
 import android.app.Activity;
+import android.app.AlertDialog;
+import android.content.DialogInterface;
 
+import com.zeapo.pwdstore.R;
+import com.zeapo.pwdstore.utils.PasswordRepository;
+
+import org.apache.commons.io.FileUtils;
 import org.eclipse.jgit.api.CloneCommand;
 import org.eclipse.jgit.api.Git;
 
@@ -12,6 +18,7 @@ public class CloneOperation extends GitOperation {
 
     /**
      * Creates a new clone operation
+     *
      * @param fileDir         the git working tree directory
      * @param callingActivity the calling activity
      */
@@ -21,6 +28,7 @@ public class CloneOperation extends GitOperation {
 
     /**
      * Sets the command using the repository uri
+     *
      * @param uri the uri of the repository
      * @return the current object
      */
@@ -34,6 +42,7 @@ public class CloneOperation extends GitOperation {
 
     /**
      * sets the authentication for user/pwd scheme
+     *
      * @param username the username
      * @param password the password
      * @return the current object
@@ -46,6 +55,7 @@ public class CloneOperation extends GitOperation {
 
     /**
      * sets the authentication for the ssh-key scheme
+     *
      * @param sshKey     the ssh-key file
      * @param username   the username
      * @param passphrase the passphrase
@@ -58,10 +68,31 @@ public class CloneOperation extends GitOperation {
     }
 
     @Override
-    public void execute() throws Exception  {
+    public void execute() {
         if (this.provider != null) {
             ((CloneCommand) this.command).setCredentialsProvider(this.provider);
         }
-        new GitAsyncTask(callingActivity, true, false, CloneCommand.class).execute(this.command);
+        new GitAsyncTask(callingActivity, true, false, this).execute(this.command);
+    }
+
+    @Override
+    public void onTaskEnded(String result) {
+        new AlertDialog.Builder(callingActivity).
+                setTitle(callingActivity.getResources().getString(R.string.jgit_error_dialog_title)).
+                setMessage("Error occured during the clone operation, "
+                        + callingActivity.getResources().getString(R.string.jgit_error_dialog_text)
+                        + result
+                        + "\nPlease check the FAQ for possible reasons why this error might occur.").
+                setPositiveButton(callingActivity.getResources().getString(R.string.dialog_ok), new DialogInterface.OnClickListener() {
+                    @Override
+                    public void onClick(DialogInterface dialogInterface, int i) {
+                        // if we were unable to finish the job
+                        try {
+                            FileUtils.deleteDirectory(PasswordRepository.getWorkTree());
+                        } catch (Exception e) {
+                            e.printStackTrace();
+                        }
+                    }
+                }).show();
     }
 }

app/src/main/java/com/zeapo/pwdstore/git/GitAsyncTask.java

@@ -2,16 +2,11 @@ package com.zeapo.pwdstore.git;
 
 import android.app.Activity;
 import android.app.ProgressDialog;
-import android.content.DialogInterface;
 import android.os.AsyncTask;
-import android.support.v7.app.AlertDialog;
 
 import com.zeapo.pwdstore.PasswordStore;
 import com.zeapo.pwdstore.R;
-import com.zeapo.pwdstore.utils.PasswordRepository;
 
-import org.apache.commons.io.FileUtils;
-import org.eclipse.jgit.api.CloneCommand;
 import org.eclipse.jgit.api.GitCommand;
 
 
@@ -20,9 +15,9 @@ public class GitAsyncTask extends AsyncTask<GitCommand, Integer, String> {
     private boolean finishOnEnd;
     private boolean refreshListOnEnd;
     private ProgressDialog dialog;
-    private Class operation;
+    private GitOperation operation;
 
-    public GitAsyncTask(Activity activity, boolean finishOnEnd, boolean refreshListOnEnd, Class operation) {
+    public GitAsyncTask(Activity activity, boolean finishOnEnd, boolean refreshListOnEnd, GitOperation operation) {
         this.activity = activity;
         this.finishOnEnd = finishOnEnd;
         this.refreshListOnEnd = refreshListOnEnd;
@@ -63,26 +58,7 @@ public class GitAsyncTask extends AsyncTask<GitCommand, Integer, String> {
             result = "Unexpected error";
 
         if (!result.isEmpty()) {
-            new AlertDialog.Builder(activity).
+            this.operation.onTaskEnded(result);
-                    setTitle(activity.getResources().getString(R.string.jgit_error_dialog_title)).
-                    setMessage(activity.getResources().getString(R.string.jgit_error_dialog_text) + result).
-                    setPositiveButton(activity.getResources().getString(R.string.dialog_ok), new DialogInterface.OnClickListener() {
-                        @Override
-                        public void onClick(DialogInterface dialogInterface, int i) {
-                            if (operation.equals(CloneCommand.class)) {
-                                // if we were unable to finish the job
-                                try {
-                                    FileUtils.deleteDirectory(PasswordRepository.getWorkTree());
-                                } catch (Exception e) {
-                                    e.printStackTrace();
-                                }
-                            } else {
-                                activity.setResult(Activity.RESULT_CANCELED);
-                                activity.finish();
-                            }
-                        }
-                    }).show();
-
         } else {
             if (finishOnEnd) {
                 this.activity.setResult(Activity.RESULT_OK);

app/src/main/java/com/zeapo/pwdstore/git/GitOperation.java

@@ -10,6 +10,7 @@ import android.widget.EditText;
 import android.widget.LinearLayout;
 
 import com.jcraft.jsch.JSch;
+import com.jcraft.jsch.JSchException;
 import com.jcraft.jsch.KeyPair;
 import com.zeapo.pwdstore.R;
 import com.zeapo.pwdstore.UserPreference;
@@ -75,10 +76,8 @@ public abstract class GitOperation {
 
     /**
      * Executes the GitCommand in an async task
-     *
-     * @throws Exception
      */
-    public abstract void execute() throws Exception;
+    public abstract void execute();
 
     /**
      * Executes the GitCommand in an async task after creating the authentication
@@ -86,9 +85,8 @@ public abstract class GitOperation {
      * @param connectionMode the server-connection mode
      * @param username       the username
      * @param sshKey         the ssh-key file
-     * @throws Exception
      */
-    public void executeAfterAuthentication(final String connectionMode, final String username, @Nullable final File sshKey) throws Exception {
+    public void executeAfterAuthentication(final String connectionMode, final String username, @Nullable final File sshKey) {
         executeAfterAuthentication(connectionMode, username, sshKey, false);
     }
 
@@ -99,9 +97,8 @@ public abstract class GitOperation {
      * @param username       the username
      * @param sshKey         the ssh-key file
      * @param showError      show the passphrase edit text in red
-     * @throws Exception
      */
-    private void executeAfterAuthentication(final String connectionMode, final String username, @Nullable final File sshKey, final boolean showError) throws Exception {
+    private void executeAfterAuthentication(final String connectionMode, final String username, @Nullable final File sshKey, final boolean showError) {
         if (connectionMode.equalsIgnoreCase("ssh-key")) {
             if (sshKey == null || !sshKey.exists()) {
                 new AlertDialog.Builder(callingActivity)
@@ -152,7 +149,9 @@ public abstract class GitOperation {
                     passphrase.setError("Wrong passphrase");
                 }
                 JSch jsch = new JSch();
+                try {
                     final KeyPair keyPair = KeyPair.load(jsch, callingActivity.getFilesDir() + "/.ssh_key");
+
                     if (keyPair.isEncrypted()) {
                         new AlertDialog.Builder(callingActivity)
                                 .setTitle(callingActivity.getResources().getString(R.string.passphrase_dialog_title))
@@ -160,7 +159,6 @@ public abstract class GitOperation {
                                 .setView(passphrase)
                                 .setPositiveButton(callingActivity.getResources().getString(R.string.dialog_ok), new DialogInterface.OnClickListener() {
                                     public void onClick(DialogInterface dialog, int whichButton) {
-                                    try {
                                         if (keyPair.decrypt(passphrase.getText().toString())) {
                                             // Authenticate using the ssh-key and then execute the command
                                             setAuthentication(sshKey, username, passphrase.getText().toString()).execute();
@@ -168,10 +166,6 @@ public abstract class GitOperation {
                                             // call back the method
                                             executeAfterAuthentication(connectionMode, username, sshKey, true);
                                         }
-                                    } catch (Exception e) {
-                                        e.printStackTrace();
-                                    }
-
                                     }
                                 }).setNegativeButton(callingActivity.getResources().getString(R.string.dialog_cancel), new DialogInterface.OnClickListener() {
                             public void onClick(DialogInterface dialog, int whichButton) {
@@ -181,6 +175,17 @@ public abstract class GitOperation {
                     } else {
                         setAuthentication(sshKey, username, "").execute();
                     }
+                } catch (JSchException e) {
+                    new AlertDialog.Builder(callingActivity)
+                            .setTitle("Unable to open the ssh-key")
+                            .setMessage("Please check that it was imported.")
+                            .setPositiveButton("Ok", new DialogInterface.OnClickListener() {
+                                @Override
+                                public void onClick(DialogInterface dialogInterface, int i) {
+
+                                }
+                            }).show();
+                }
             }
         } else {
             final EditText password = new EditText(callingActivity);
@@ -195,11 +200,7 @@ public abstract class GitOperation {
                     .setPositiveButton(callingActivity.getResources().getString(R.string.dialog_ok), new DialogInterface.OnClickListener() {
                         public void onClick(DialogInterface dialog, int whichButton) {
                             // authenticate using the user/pwd and then execute the command
-                            try {
                             setAuthentication(username, password.getText().toString()).execute();
-                            } catch (Exception e) {
-                                e.printStackTrace();
-                            }
 
                         }
                     }).setNegativeButton(callingActivity.getResources().getString(R.string.dialog_cancel), new DialogInterface.OnClickListener() {
@@ -209,4 +210,20 @@ public abstract class GitOperation {
             }).show();
         }
     }
+
+    public void onTaskEnded(String result) {
+        new AlertDialog.Builder(callingActivity).
+                setTitle(callingActivity.getResources().getString(R.string.jgit_error_dialog_title)).
+                setMessage("Error occurred during a Git operation, "
+                        + callingActivity.getResources().getString(R.string.jgit_error_dialog_text)
+                        + result
+                        + "\nPlease check the FAQ for possible reasons why this error might occur.").
+                setPositiveButton(callingActivity.getResources().getString(R.string.dialog_ok), new DialogInterface.OnClickListener() {
+                    @Override
+                    public void onClick(DialogInterface dialogInterface, int i) {
+                        callingActivity.setResult(Activity.RESULT_CANCELED);
+                        callingActivity.finish();
+                    }
+                }).show();
+    }
 }

app/src/main/java/com/zeapo/pwdstore/git/PullOperation.java

@@ -1,6 +1,10 @@
 package com.zeapo.pwdstore.git;
 
 import android.app.Activity;
+import android.app.AlertDialog;
+import android.content.DialogInterface;
+
+import com.zeapo.pwdstore.R;
 
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.PullCommand;
@@ -32,10 +36,26 @@ public class PullOperation extends GitOperation {
     }
 
     @Override
-    public void execute() throws Exception  {
+    public void execute() {
         if (this.provider != null) {
             ((PullCommand) this.command).setCredentialsProvider(this.provider);
         }
-        new GitAsyncTask(callingActivity, true, false, PullCommand.class).execute(this.command);
+        new GitAsyncTask(callingActivity, true, false, this).execute(this.command);
+    }
+
+    @Override
+    public void onTaskEnded(String result) {
+        new AlertDialog.Builder(callingActivity).
+                setTitle(callingActivity.getResources().getString(R.string.jgit_error_dialog_title)).
+                setMessage("Error occured during the pull operation, "
+                        + callingActivity.getResources().getString(R.string.jgit_error_dialog_text)
+                        + result
+                        + "\nPlease check the FAQ for possible reasons why this error might occur.").
+                setPositiveButton(callingActivity.getResources().getString(R.string.dialog_ok), new DialogInterface.OnClickListener() {
+                    @Override
+                    public void onClick(DialogInterface dialogInterface, int i) {
+                        callingActivity.finish();
+                    }
+                }).show();
     }
 }

app/src/main/java/com/zeapo/pwdstore/git/PushOperation.java

@@ -1,6 +1,10 @@
 package com.zeapo.pwdstore.git;
 
 import android.app.Activity;
+import android.app.AlertDialog;
+import android.content.DialogInterface;
+
+import com.zeapo.pwdstore.R;
 
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.PushCommand;
@@ -32,10 +36,27 @@ public class PushOperation extends GitOperation {
     }
 
     @Override
-    public void execute() throws Exception  {
+    public void execute() {
         if (this.provider != null) {
             ((PushCommand) this.command).setCredentialsProvider(this.provider);
         }
-        new GitAsyncTask(callingActivity, true, false, PushCommand.class).execute(this.command);
+        new GitAsyncTask(callingActivity, true, false, this).execute(this.command);
+    }
+
+    @Override
+    public void onTaskEnded(String result) {
+        // TODO handle the "Nothing to push" case
+        new AlertDialog.Builder(callingActivity).
+                setTitle(callingActivity.getResources().getString(R.string.jgit_error_dialog_title)).
+                setMessage("Error occured during the push operation, "
+                        + callingActivity.getResources().getString(R.string.jgit_error_dialog_text)
+                        + result
+                        + "\nPlease check the FAQ for possible reasons why this error might occur.").
+                setPositiveButton(callingActivity.getResources().getString(R.string.dialog_ok), new DialogInterface.OnClickListener() {
+                    @Override
+                    public void onClick(DialogInterface dialogInterface, int i) {
+                        callingActivity.finish();
+                    }
+                }).show();
     }
 }

app/src/main/java/com/zeapo/pwdstore/git/SyncOperation.java

@@ -1,6 +1,10 @@
 package com.zeapo.pwdstore.git;
 
 import android.app.Activity;
+import android.app.AlertDialog;
+import android.content.DialogInterface;
+
+import com.zeapo.pwdstore.R;
 
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.PullCommand;
@@ -39,12 +43,27 @@ public class SyncOperation extends GitOperation {
     }
 
     @Override
-    public void execute() throws Exception  {
+    public void execute() {
-
         if (this.provider != null) {
             this.pullCommand.setCredentialsProvider(this.provider);
             this.pushCommand.setCredentialsProvider(this.provider);
         }
-        new GitAsyncTask(callingActivity, true, false, PullCommand.class).execute(this.pullCommand, this.pushCommand);
+        new GitAsyncTask(callingActivity, true, false, this).execute(this.pullCommand, this.pushCommand);
+    }
+
+    @Override
+    public void onTaskEnded(String result) {
+        new AlertDialog.Builder(callingActivity).
+                setTitle(callingActivity.getResources().getString(R.string.jgit_error_dialog_title)).
+                setMessage("Error occured during the sync operation, "
+                        + callingActivity.getResources().getString(R.string.jgit_error_dialog_text)
+                        + result
+                        + "\nPlease check the FAQ for possible reasons why this error might occur.").
+                setPositiveButton(callingActivity.getResources().getString(R.string.dialog_ok), new DialogInterface.OnClickListener() {
+                    @Override
+                    public void onClick(DialogInterface dialogInterface, int i) {
+                        callingActivity.finish();
+                    }
+                }).show();
     }
 }

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

@@ -41,7 +41,7 @@
 
     <!-- Git Async Task -->
     <string name="running_dialog_text">Running command...</string>
-    <string name="jgit_error_dialog_title">Internal exception occurred</string>
+    <string name="jgit_error_dialog_title">An error occurred during a Git operation</string>
     <string name="jgit_error_dialog_text">Message from jgit: \n</string>
 
     <!-- Git Handler -->