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 <me@msfjarvis.dev>

* Remove unused operation type

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>

* Refactor launchGitOperation to use an enum

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
This commit is contained in:
Harsh Shandilya 2020-09-14 22:37:22 +05:30 committed by GitHub
parent eef809760c
commit a34f749e9a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 58 additions and 63 deletions

View file

@ -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))
}
/**

View file

@ -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 {

View file

@ -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"))

View file

@ -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<Unit, Throwable> {
suspend fun launchGitOperation(operation: GitOp): Result<Unit, Throwable> {
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,7 +91,8 @@ abstract class BaseGitActivity : AppCompatActivity() {
}
sharedPrefs.edit { remove(PreferenceKeys.SSH_OPENKEYSTORE_KEYID) }
d(error)
MaterialAlertDialogBuilder(this).run {
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)) { _, _ -> }
@ -93,6 +101,7 @@ abstract class BaseGitActivity : AppCompatActivity() {
}
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"
}
}

View file

@ -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) {

View file

@ -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)
}
}
}
}

View file

@ -202,9 +202,4 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
sshSessionFactory?.close()
}
}
companion object {
const val GET_SSH_KEY_FROM_CLONE = 201
}
}