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ˆ

Author: zeapo

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

@@ -28,13 +28,13 @@
 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());
@@ -74,7 +75,7 @@ protected void onCreate(Bundle savedInstanceState) {
     }
 
     @Override
-    public void onResume(){
+    public void onResume() {
         super.onResume();
         // do not attempt to checkLocalRepository() if no storage permission: immediate crash
         if (settings.getBoolean("git_external", false)) {
@@ -373,10 +374,9 @@ private void checkLocalRepository(File localDir) {
     }
 
 
-
     @Override
     public void onBackPressed() {
-        if  ((null != plist) && plist.isNotEmpty()) {
+        if ((null != plist) && plist.isNotEmpty()) {
             plist.popBack();
         } else {
             super.onBackPressed();
@@ -438,20 +438,12 @@ public void deletePasswords(final PasswordRecyclerAdapter adapter, final Set<Int
                 .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);
-                        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.")
-                        );
+                        commit("[ANDROID PwdStore] Remove " + item + " from store.");
                         deletePasswords(adapter, selectedItems);
                     }
                 })
@@ -468,10 +460,10 @@ public void onClick(DialogInterface dialogInterface, int i) {
     public void movePasswords(ArrayList<PasswordItem> values) {
         Intent intent = new Intent(this, PgpHandler.class);
         ArrayList<String> fileLocations = new ArrayList<>();
-        for (PasswordItem passwordItem : values){
+        for (PasswordItem passwordItem : values) {
             fileLocations.add(passwordItem.getFile().getAbsolutePath());
         }
-        intent.putExtra("Files",fileLocations);
+        intent.putExtra("Files", fileLocations);
         intent.putExtra("Operation", "SELECTFOLDER");
         startActivityForResult(intent, PgpHandler.REQUEST_CODE_SELECT_FOLDER);
     }
@@ -480,7 +472,7 @@ public void movePasswords(ArrayList<PasswordItem> values) {
      * clears adapter's content and updates it with a fresh list of passwords from the root
      */
     public void updateListAdapter() {
-        if  ((null != plist)) {
+        if ((null != plist)) {
             plist.updateAdapter();
         }
     }
@@ -489,31 +481,36 @@ public void updateListAdapter() {
      * Updates the adapter with the current view of passwords
      */
     public void refreshListAdapter() {
-        if  ((null != plist)) {
+        if ((null != plist)) {
             plist.refreshAdapter();
         }
     }
 
     public void filterListAdapter(String filter) {
-        if  ((null != plist)) {
+        if ((null != plist)) {
             plist.filterAdapter(filter);
         }
     }
 
     private File getCurrentDir() {
-        if  ((null != plist)) {
+        if ((null != plist)) {
             return plist.getCurrentDir();
         }
         return PasswordRepository.getWorkTree();
     }
 
-    private void commit(String message) {
-        Git git = new Git(PasswordRepository.getRepository(new File("")));
-        GitAsyncTask tasks = new GitAsyncTask(this, false, false, CommitCommand.class);
-        tasks.execute(
-                git.add().addFilepattern("."),
-                git.commit().setMessage(message)
-        );
+    private void commit(final String message) {
+        new GitOperation(PasswordRepository.getRepositoryDirectory(activity), activity) {
+            @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,
@@ -573,31 +570,29 @@ protected void onActivityResult(int requestCode, int resultCode,
                     startActivityForResult(intent, GitActivity.REQUEST_CLONE);
                     break;
                 case PgpHandler.REQUEST_CODE_SELECT_FOLDER:
-                    Log.d("Moving","Moving passwords to "+data.getStringExtra("SELECTED_FOLDER_PATH"));
+                    Log.d("Moving", "Moving passwords to " + data.getStringExtra("SELECTED_FOLDER_PATH"));
                     Log.d("Moving", TextUtils.join(", ", data.getStringArrayListExtra("Files")));
                     File target = new File(data.getStringExtra("SELECTED_FOLDER_PATH"));
-                    if (!target.isDirectory()){
-                        Log.e("Moving","Tried moving passwords to a non-existing folder.");
+                    if (!target.isDirectory()) {
+                        Log.e("Moving", "Tried moving passwords to a non-existing folder.");
                         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")){
+                    for (String string : data.getStringArrayListExtra("Files")) {
                         File source = new File(string);
-                        if (!source.exists()){
-                            Log.e("Moving","Tried moving something that appears non-existent.");
+                        if (!source.exists()) {
+                            Log.e("Moving", "Tried moving something that appears non-existent.");
                             continue;
                         }
-                        if (!source.renameTo(new File(target.getAbsolutePath()+"/"+source.getName()))){
-                            Log.e("Moving","Something went wrong while moving.");
-                        }else{
-                            tasks.execute(
-                                    git.add().addFilepattern(source.getAbsolutePath().replace(PasswordRepository.getWorkTree() + "/", "")),
-                                    git.commit().setMessage("[ANDROID PwdStore] Moved "+string.replace(PasswordRepository.getWorkTree() + "/", "")+" to "+target.getAbsolutePath().replace(PasswordRepository.getWorkTree() + "/","")+target.getAbsolutePath()+"/"+source.getName()+".")
-                            );
+                        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 {
+                            commit("[ANDROID PwdStore] Moved "
+                                    + string.replace(PasswordRepository.getWorkTree() + "/", "")
+                                    + " 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,7 +18,8 @@ public class CloneOperation extends GitOperation {
 
     /**
      * Creates a new clone operation
-     * @param fileDir the git working tree directory
+     *
+     * @param fileDir         the git working tree directory
      * @param callingActivity the calling activity
      */
     public CloneOperation(File fileDir, Activity callingActivity) {
@@ -21,6 +28,7 @@ public CloneOperation(File fileDir, Activity callingActivity) {
 
     /**
      * Sets the command using the repository uri
+     *
      * @param uri the uri of the repository
      * @return the current object
      */
@@ -34,6 +42,7 @@ public CloneOperation setCommand(String uri) {
 
     /**
      * sets the authentication for user/pwd scheme
+     *
      * @param username the username
      * @param password the password
      * @return the current object
@@ -46,6 +55,7 @@ public CloneOperation setAuthentication(String username, String password) {
 
     /**
      * 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 CloneOperation setAuthentication(File sshKey, String username, String pas
     }
 
     @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 @@
 
 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 @@ protected void onPostExecute(String result) {
             result = "Unexpected error";
 
         if (!result.isEmpty()) {
-            new AlertDialog.Builder(activity).
-                    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();
-
+            this.operation.onTaskEnded(result);
         } else {
             if (finishOnEnd) {
                 this.activity.setResult(Activity.RESULT_OK);