From a34f749e9ac1d6112a42b0f4ded3ae801d8c583c Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Mon, 14 Sep 2020 22:37:22 +0530 Subject: Add tentative workaround for dialog crashes and refactor Git-related code (#1100) * Ensure we're creating dialogs on the main thread Signed-off-by: Harsh Shandilya * Remove unused operation type Signed-off-by: Harsh Shandilya * Refactor launchGitOperation to use an enum Signed-off-by: Harsh Shandilya --- .../java/com/zeapo/pwdstore/OnboardingActivity.kt | 5 +- .../java/com/zeapo/pwdstore/PasswordFragment.kt | 9 +-- .../main/java/com/zeapo/pwdstore/PasswordStore.kt | 11 ++-- .../java/com/zeapo/pwdstore/git/BaseGitActivity.kt | 69 +++++++++++----------- .../com/zeapo/pwdstore/git/GitConfigActivity.kt | 4 +- .../zeapo/pwdstore/git/GitServerConfigActivity.kt | 18 ++++-- .../zeapo/pwdstore/git/operation/GitOperation.kt | 5 -- 7 files changed, 58 insertions(+), 63 deletions(-) diff --git a/app/src/main/java/com/zeapo/pwdstore/OnboardingActivity.kt b/app/src/main/java/com/zeapo/pwdstore/OnboardingActivity.kt index 61bba1cd..45bf1d1c 100644 --- a/app/src/main/java/com/zeapo/pwdstore/OnboardingActivity.kt +++ b/app/src/main/java/com/zeapo/pwdstore/OnboardingActivity.kt @@ -17,7 +17,6 @@ import com.github.michaelbull.result.onFailure import com.github.michaelbull.result.runCatching import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.zeapo.pwdstore.databinding.ActivityOnboardingBinding -import com.zeapo.pwdstore.git.BaseGitActivity import com.zeapo.pwdstore.git.GitServerConfigActivity import com.zeapo.pwdstore.utils.PasswordRepository import com.zeapo.pwdstore.utils.PreferenceKeys @@ -92,9 +91,7 @@ class OnboardingActivity : AppCompatActivity() { */ private fun cloneToHiddenDir() { settings.edit { putBoolean(PreferenceKeys.GIT_EXTERNAL, false) } - cloneAction.launch(Intent(this, GitServerConfigActivity::class.java).apply { - putExtra(BaseGitActivity.REQUEST_ARG_OP, BaseGitActivity.REQUEST_CLONE) - }) + cloneAction.launch(GitServerConfigActivity.createCloneIntent(this)) } /** diff --git a/app/src/main/java/com/zeapo/pwdstore/PasswordFragment.kt b/app/src/main/java/com/zeapo/pwdstore/PasswordFragment.kt index 013a922d..7c1dae9b 100644 --- a/app/src/main/java/com/zeapo/pwdstore/PasswordFragment.kt +++ b/app/src/main/java/com/zeapo/pwdstore/PasswordFragment.kt @@ -5,7 +5,6 @@ package com.zeapo.pwdstore import android.content.Context -import android.content.Intent import android.content.SharedPreferences import android.os.Bundle import android.os.Parcelable @@ -90,9 +89,7 @@ class PasswordFragment : Fragment(R.layout.password_recycler_view) { } else if (!PasswordRepository.isGitRepo()) { Snackbar.make(binding.root, getString(R.string.clone_git_repo), Snackbar.LENGTH_INDEFINITE) .setAction(R.string.clone_button) { - val intent = Intent(context, GitServerConfigActivity::class.java) - intent.putExtra(BaseGitActivity.REQUEST_ARG_OP, BaseGitActivity.REQUEST_CLONE) - swipeResult.launch(intent) + swipeResult.launch(GitServerConfigActivity.createCloneIntent(requireContext())) } .show() binding.swipeRefresher.isRefreshing = false @@ -100,8 +97,8 @@ class PasswordFragment : Fragment(R.layout.password_recycler_view) { // When authentication is set to AuthMode.None then the only git operation we can // run is a pull, so automatically fallback to that. val operationId = when (GitSettings.authMode) { - AuthMode.None -> BaseGitActivity.REQUEST_PULL - else -> BaseGitActivity.REQUEST_SYNC + AuthMode.None -> BaseGitActivity.GitOp.PULL + else -> BaseGitActivity.GitOp.SYNC } requireStore().apply { lifecycleScope.launch { diff --git a/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt b/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt index 7501146a..93dfad56 100644 --- a/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt +++ b/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt @@ -272,7 +272,7 @@ class PasswordStore : BaseGitActivity() { initBefore.show() return false } - runGitOperation(REQUEST_PUSH) + runGitOperation(GitOp.PUSH) return true } R.id.git_pull -> { @@ -280,7 +280,7 @@ class PasswordStore : BaseGitActivity() { initBefore.show() return false } - runGitOperation(REQUEST_PULL) + runGitOperation(GitOp.PULL) return true } R.id.git_sync -> { @@ -288,7 +288,7 @@ class PasswordStore : BaseGitActivity() { initBefore.show() return false } - runGitOperation(REQUEST_SYNC) + runGitOperation(GitOp.SYNC) return true } R.id.refresh -> { @@ -312,10 +312,10 @@ class PasswordStore : BaseGitActivity() { searchItem.collapseActionView() } - private fun runGitOperation(operation: Int) = lifecycleScope.launch { + private fun runGitOperation(operation: GitOp) = lifecycleScope.launch { launchGitOperation(operation).fold( success = { refreshPasswordList() }, - failure = ::promptOnErrorHandler + failure = { promptOnErrorHandler(it) }, ) } @@ -520,7 +520,6 @@ class PasswordStore : BaseGitActivity() { val intent = Intent(this, SelectFolderActivity::class.java) val fileLocations = values.map { it.file.absolutePath }.toTypedArray() intent.putExtra("Files", fileLocations) - intent.putExtra(REQUEST_ARG_OP, "SELECTFOLDER") registerForActivityResult(StartActivityForResult()) { result -> val intentData = result.data ?: return@registerForActivityResult val filesToMove = requireNotNull(intentData.getStringArrayExtra("Files")) diff --git a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt index 19413efd..6a975f6c 100644 --- a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt +++ b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt @@ -6,9 +6,7 @@ package com.zeapo.pwdstore.git import androidx.appcompat.app.AppCompatActivity import androidx.core.content.edit -import com.github.ajalt.timberkt.Timber.tag import com.github.ajalt.timberkt.d -import com.github.ajalt.timberkt.e import com.github.michaelbull.result.Err import com.github.michaelbull.result.Result import com.github.michaelbull.result.andThen @@ -18,7 +16,6 @@ import com.zeapo.pwdstore.R import com.zeapo.pwdstore.git.config.GitSettings import com.zeapo.pwdstore.git.operation.BreakOutOfDetached import com.zeapo.pwdstore.git.operation.CloneOperation -import com.zeapo.pwdstore.git.operation.GitOperation import com.zeapo.pwdstore.git.operation.PullOperation import com.zeapo.pwdstore.git.operation.PushOperation import com.zeapo.pwdstore.git.operation.ResetToRemoteOperation @@ -26,6 +23,8 @@ import com.zeapo.pwdstore.git.operation.SyncOperation import com.zeapo.pwdstore.utils.PreferenceKeys import com.zeapo.pwdstore.utils.getEncryptedPrefs import com.zeapo.pwdstore.utils.sharedPrefs +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import net.schmizz.sshj.common.DisconnectReason import net.schmizz.sshj.common.SSHException import net.schmizz.sshj.userauth.UserAuthException @@ -36,30 +35,38 @@ import net.schmizz.sshj.userauth.UserAuthException */ abstract class BaseGitActivity : AppCompatActivity() { + /** + * Enum of possible Git operations than can be run through [launchGitOperation]. + */ + enum class GitOp { + BREAK_OUT_OF_DETACHED, + CLONE, + PULL, + PUSH, + RESET, + SYNC, + } + /** * Attempt to launch the requested Git operation. * @param operation The type of git operation to launch */ - suspend fun launchGitOperation(operation: Int): Result { + suspend fun launchGitOperation(operation: GitOp): Result { if (GitSettings.url == null) { return Err(IllegalStateException("Git url is not set!")) } - if (operation == REQUEST_SYNC && !GitSettings.useMultiplexing) { + if (operation == GitOp.SYNC && !GitSettings.useMultiplexing) { // If the server does not support multiple SSH channels per connection, we cannot run // a sync operation without reconnecting and thus break sync into its two parts. - return launchGitOperation(REQUEST_PULL).andThen { launchGitOperation(REQUEST_PUSH) } + return launchGitOperation(GitOp.PULL).andThen { launchGitOperation(GitOp.PUSH) } } val op = when (operation) { - REQUEST_CLONE, GitOperation.GET_SSH_KEY_FROM_CLONE -> CloneOperation(this, GitSettings.url!!) - REQUEST_PULL -> PullOperation(this) - REQUEST_PUSH -> PushOperation(this) - REQUEST_SYNC -> SyncOperation(this) - BREAK_OUT_OF_DETACHED -> BreakOutOfDetached(this) - REQUEST_RESET -> ResetToRemoteOperation(this) - else -> { - tag(TAG).e { "Operation not recognized : $operation" } - return Err(IllegalArgumentException("$operation is not a valid Git operation")) - } + GitOp.CLONE -> CloneOperation(this, GitSettings.url!!) + GitOp.PULL -> PullOperation(this) + GitOp.PUSH -> PushOperation(this) + GitOp.SYNC -> SyncOperation(this) + GitOp.BREAK_OUT_OF_DETACHED -> BreakOutOfDetached(this) + GitOp.RESET -> ResetToRemoteOperation(this) } return op.executeAfterAuthentication(GitSettings.authMode).mapError { throwable -> val err = rootCauseException(throwable) @@ -76,7 +83,7 @@ abstract class BaseGitActivity : AppCompatActivity() { finish() } - fun promptOnErrorHandler(err: Throwable, onPromptDone: () -> Unit = {}) { + suspend fun promptOnErrorHandler(err: Throwable, onPromptDone: () -> Unit = {}) { val error = rootCauseException(err) if (!isExplicitlyUserInitiatedError(error)) { getEncryptedPrefs("git_operation").edit { @@ -84,14 +91,16 @@ abstract class BaseGitActivity : AppCompatActivity() { } sharedPrefs.edit { remove(PreferenceKeys.SSH_OPENKEYSTORE_KEYID) } d(error) - MaterialAlertDialogBuilder(this).run { - setTitle(resources.getString(R.string.jgit_error_dialog_title)) - setMessage(ErrorMessages[error]) - setPositiveButton(resources.getString(R.string.dialog_ok)) { _, _ -> } - setOnDismissListener { - onPromptDone() + withContext(Dispatchers.Main) { + MaterialAlertDialogBuilder(this@BaseGitActivity).run { + setTitle(resources.getString(R.string.jgit_error_dialog_title)) + setMessage(ErrorMessages[error]) + setPositiveButton(resources.getString(R.string.dialog_ok)) { _, _ -> } + setOnDismissListener { + onPromptDone() + } + show() } - show() } } else { onPromptDone() @@ -130,16 +139,4 @@ abstract class BaseGitActivity : AppCompatActivity() { } return rootCause } - - companion object { - - const val REQUEST_ARG_OP = "OPERATION" - const val REQUEST_PULL = 101 - const val REQUEST_PUSH = 102 - const val REQUEST_CLONE = 103 - const val REQUEST_SYNC = 104 - const val BREAK_OUT_OF_DETACHED = 105 - const val REQUEST_RESET = 106 - const val TAG = "AbstractGitActivity" - } } diff --git a/app/src/main/java/com/zeapo/pwdstore/git/GitConfigActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/GitConfigActivity.kt index 49abc92d..752a6b7a 100644 --- a/app/src/main/java/com/zeapo/pwdstore/git/GitConfigActivity.kt +++ b/app/src/main/java/com/zeapo/pwdstore/git/GitConfigActivity.kt @@ -91,7 +91,7 @@ class GitConfigActivity : BaseGitActivity() { } binding.gitAbortRebase.setOnClickListener { lifecycleScope.launch { - launchGitOperation(BREAK_OUT_OF_DETACHED).fold( + launchGitOperation(GitOp.BREAK_OUT_OF_DETACHED).fold( success = { MaterialAlertDialogBuilder(this@GitConfigActivity).run { setTitle(resources.getString(R.string.git_abort_and_push_title)) @@ -115,7 +115,7 @@ class GitConfigActivity : BaseGitActivity() { } binding.gitResetToRemote.setOnClickListener { lifecycleScope.launch { - launchGitOperation(REQUEST_RESET).fold( + launchGitOperation(GitOp.RESET).fold( success = ::finishOnSuccessHandler, failure = { err -> promptOnErrorHandler(err) { diff --git a/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt index 4bbc5965..92381384 100644 --- a/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt +++ b/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt @@ -4,6 +4,8 @@ */ package com.zeapo.pwdstore.git +import android.content.Context +import android.content.Intent import android.os.Bundle import android.os.Handler import android.view.MenuItem @@ -42,7 +44,7 @@ class GitServerConfigActivity : BaseGitActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - val isClone = intent?.extras?.getInt(REQUEST_ARG_OP) ?: -1 == REQUEST_CLONE + val isClone = intent?.extras?.getBoolean("cloning") ?: false if (isClone) { binding.saveButton.text = getString(R.string.clone_button) } @@ -151,7 +153,7 @@ class GitServerConfigActivity : BaseGitActivity() { localDir.deleteRecursively() } snackbar.dismiss() - launchGitOperation(REQUEST_CLONE).fold( + launchGitOperation(GitOp.CLONE).fold( success = { setResult(RESULT_OK) finish() @@ -185,14 +187,22 @@ class GitServerConfigActivity : BaseGitActivity() { MaterialAlertDialogBuilder(this).setMessage(e.message).show() } lifecycleScope.launch { - launchGitOperation(REQUEST_CLONE).fold( + launchGitOperation(GitOp.CLONE).fold( success = { setResult(RESULT_OK) finish() }, - failure = ::promptOnErrorHandler, + failure = { promptOnErrorHandler(it) }, ) } } } + + companion object { + fun createCloneIntent(context: Context): Intent { + return Intent(context, GitServerConfigActivity::class.java).apply { + putExtra("cloning", true) + } + } + } } diff --git a/app/src/main/java/com/zeapo/pwdstore/git/operation/GitOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/operation/GitOperation.kt index 85c38c6a..d7ff7c4c 100644 --- a/app/src/main/java/com/zeapo/pwdstore/git/operation/GitOperation.kt +++ b/app/src/main/java/com/zeapo/pwdstore/git/operation/GitOperation.kt @@ -202,9 +202,4 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) { sshSessionFactory?.close() } } - - companion object { - - const val GET_SSH_KEY_FROM_CLONE = 201 - } } -- cgit v1.2.3