Refactor openpgp-ktx to leverage coroutines (#1398)

* openpgp-ktx: leverage coroutines for async IPC

* Unwind nested dispatchers

* Fix name shadowing warning

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

app/src/main/java/dev/msfjarvis/aps/ui/crypto/DecryptActivity.kt

@@ -165,9 +165,9 @@ class DecryptActivity : BasePgpActivity(), OpenPgpServiceConnection.OnBound {
     val inputStream = File(fullPath).inputStream()
     val outputStream = ByteArrayOutputStream()
 
-    lifecycleScope.launch(Dispatchers.IO) {
+    lifecycleScope.launch(Dispatchers.Main) {
-      api?.executeApiAsync(data, inputStream, outputStream) { result ->
+      val result = withContext(Dispatchers.IO) { checkNotNull(api).executeApi(data, inputStream, outputStream) }
-        when (result?.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) {
+      when (result.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) {
         OpenPgpApi.RESULT_CODE_SUCCESS -> {
           startAutoDismissTimer()
           runCatching {
@@ -188,11 +188,8 @@ class DecryptActivity : BasePgpActivity(), OpenPgpServiceConnection.OnBound {
             }
 
             if (entry.hasTotp()) {
-                launch(Dispatchers.IO) {
+              launch {
-                  withContext(Dispatchers.Main) {
+                items.add(FieldItem.createOtpField(entry.totp.value))
-                    val code = entry.totp.value
-                    items.add(FieldItem.createOtpField(code))
-                  }
                 entry.totp.collect { code -> withContext(Dispatchers.Main) { adapter.updateOTPCode(code) } }
               }
             }
@@ -217,4 +214,3 @@ class DecryptActivity : BasePgpActivity(), OpenPgpServiceConnection.OnBound {
     }
   }
 }
-}

app/src/main/java/dev/msfjarvis/aps/ui/crypto/GetKeyIdsActivity.kt

@@ -15,6 +15,7 @@ import com.github.michaelbull.result.onFailure
 import com.github.michaelbull.result.runCatching
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import me.msfjarvis.openpgpktx.util.OpenPgpApi
 import me.msfjarvis.openpgpktx.util.OpenPgpUtils
 import org.openintents.openpgp.IOpenPgpService2
@@ -48,9 +49,9 @@ class GetKeyIdsActivity : BasePgpActivity() {
   /** Get the Key ids from OpenKeychain */
   private fun getKeyIds(data: Intent = Intent()) {
     data.action = OpenPgpApi.ACTION_GET_KEY_IDS
-    lifecycleScope.launch(Dispatchers.IO) {
+    lifecycleScope.launch(Dispatchers.Main) {
-      api?.executeApiAsync(data, null, null) { result ->
+      val result = withContext(Dispatchers.IO) { checkNotNull(api).executeApi(data, null, null) }
-        when (result?.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) {
+      when (result.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) {
         OpenPgpApi.RESULT_CODE_SUCCESS -> {
           runCatching {
             val ids =
@@ -71,4 +72,3 @@ class GetKeyIdsActivity : BasePgpActivity() {
     }
   }
 }
-}

app/src/main/java/dev/msfjarvis/aps/ui/crypto/PasswordCreationActivity.kt

@@ -394,9 +394,10 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
           else -> "$fullPath/$editName.gpg"
         }
 
-      lifecycleScope.launch(Dispatchers.IO) {
+      lifecycleScope.launch(Dispatchers.Main) {
-        api?.executeApiAsync(encryptionIntent, inputStream, outputStream) { result ->
+        val result =
-          when (result?.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) {
+          withContext(Dispatchers.IO) { checkNotNull(api).executeApi(encryptionIntent, inputStream, outputStream) }
+        when (result.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) {
           OpenPgpApi.RESULT_CODE_SUCCESS -> {
             runCatching {
               val file = File(path)
@@ -406,15 +407,15 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
               // doesn't already exist to prevent an accidental overwrite.
               if ((!editing || (editing && suggestedName != file.nameWithoutExtension)) && file.exists()) {
                 snackbar(message = getString(R.string.password_creation_duplicate_error))
-                  return@executeApiAsync
+                return@runCatching
               }
 
               if (!file.isInsideRepository()) {
                 snackbar(message = getString(R.string.message_error_destination_outside_repo))
-                  return@executeApiAsync
+                return@runCatching
               }
 
-                file.outputStream().use { it.write(outputStream.toByteArray()) }
+              withContext(Dispatchers.IO) { file.outputStream().use { it.write(outputStream.toByteArray()) } }
 
               // associate the new password name with the last name's timestamp in
               // history
@@ -451,7 +452,7 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
                     .setCancelable(false)
                     .setPositiveButton(android.R.string.ok) { _, _ -> finish() }
                     .show()
-                    return@executeApiAsync
+                  return@runCatching
                 }
               }
 
@@ -488,7 +489,6 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
       }
     }
   }
-  }
 
   companion object {
 

autofill-parser/CHANGELOG.md

@@ -12,6 +12,8 @@ All notable changes to this project will be documented in this file.
 
 - The library now requires Kotlin 1.5.0 configured with `kotlinOptions.languageVersion = "1.5"`.
 
+- The synchronous and callback based APIs in `OpenPgpApi` have been removed in favor of a singular coroutines-based entrypoint.
+
 ### Fixed
 
 - Fix build warning from undeclared unsigned type use.

openpgp-ktx/api/openpgp-ktx.api

@@ -88,8 +88,7 @@ public final class me/msfjarvis/openpgpktx/util/OpenPgpApi {
 	public static final field RESULT_SIGNATURE_MICALG Ljava/lang/String;
 	public static final field SERVICE_INTENT_2 Ljava/lang/String;
 	public fun <init> (Landroid/content/Context;Lorg/openintents/openpgp/IOpenPgpService2;)V
-	public final fun executeApi (Landroid/content/Intent;Ljava/io/InputStream;Ljava/io/OutputStream;)Landroid/content/Intent;
+	public final fun executeApi (Landroid/content/Intent;Ljava/io/InputStream;Ljava/io/OutputStream;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
-	public final fun executeApiAsync (Landroid/content/Intent;Ljava/io/InputStream;Ljava/io/OutputStream;Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 }
 
 public final class me/msfjarvis/openpgpktx/util/OpenPgpApi$Companion {

openpgp-ktx/gradle.properties

@@ -3,7 +3,7 @@
 # SPDX-License-Identifier: Apache-2.0
 #
 
-VERSION_NAME=3.0.0
+VERSION_NAME=4.0.0-SNAPSHOT
 POM_ARTIFACT_ID=openpgp-ktx
 POM_NAME=openpgp-ktx
 POM_DESCRIPTION=Reimplementation of OpenKeychain's integration library in Kotlin

openpgp-ktx/src/main/java/me/msfjarvis/openpgpktx/util/OpenPgpApi.kt

@@ -2,7 +2,7 @@
  * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved.
  * SPDX-License-Identifier: Apache-2.0
  */
-@file:Suppress("Unused")
+@file:Suppress("BlockingMethodInNonBlockingContext", "Unused")
 
 package me.msfjarvis.openpgpktx.util
 
@@ -14,8 +14,6 @@ import java.io.IOException
 import java.io.InputStream
 import java.io.OutputStream
 import java.util.concurrent.atomic.AtomicInteger
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.withContext
 import org.openintents.openpgp.IOpenPgpService2
 import org.openintents.openpgp.OpenPgpError
 
@@ -23,17 +21,7 @@ public class OpenPgpApi(private val context: Context, private val service: IOpen
 
   private val pipeIdGen: AtomicInteger = AtomicInteger()
 
-  public suspend fun executeApiAsync(
+  public suspend fun executeApi(data: Intent, inputStream: InputStream?, outputStream: OutputStream?): Intent {
-    data: Intent?,
-    inputStream: InputStream?,
-    outputStream: OutputStream?,
-    callback: (intent: Intent?) -> Unit
-  ) {
-    val result = executeApi(data, inputStream, outputStream)
-    withContext(Dispatchers.Main) { callback.invoke(result) }
-  }
-
-  public fun executeApi(data: Intent?, inputStream: InputStream?, outputStream: OutputStream?): Intent? {
     var input: ParcelFileDescriptor? = null
     return try {
       if (inputStream != null) {
@@ -57,37 +45,38 @@ public class OpenPgpApi(private val context: Context, private val service: IOpen
     }
   }
 
-  /** InputStream and OutputStreams are always closed after operating on them! */
+  private suspend fun executeApi(
-  private fun executeApi(data: Intent?, input: ParcelFileDescriptor?, os: OutputStream?): Intent? {
+    data: Intent,
+    inputFd: ParcelFileDescriptor?,
+    outputStream: OutputStream?,
+  ): Intent {
     var output: ParcelFileDescriptor? = null
     return try {
       // always send version from client
-      data?.putExtra(EXTRA_API_VERSION, API_VERSION)
+      data.putExtra(EXTRA_API_VERSION, API_VERSION)
       val result: Intent
-      var pumpThread: Thread? = null
       var outputPipeId = 0
-      if (os != null) {
+      if (outputStream != null) {
         outputPipeId = pipeIdGen.incrementAndGet()
         output = service.createOutputPipe(outputPipeId)
-        pumpThread = ParcelFileDescriptorUtil.pipeTo(os, output)
       }
       // blocks until result is ready
-      result = service.execute(data, input, outputPipeId)
+      result = service.execute(data, inputFd, outputPipeId)
       // set class loader to current context to allow unparcelling
       // of OpenPgpError and OpenPgpSignatureResult
       // http://stackoverflow.com/a/3806769
       result.setExtrasClassLoader(context.classLoader)
-      // wait for ALL data being pumped from remote side
+      if (outputStream != null) {
-      pumpThread?.join()
+        ParcelFileDescriptorUtil.pipeTo(outputStream, output)
+      }
       result
-    } catch (e: Exception) {
+    } catch (e: Throwable) {
       Log.e(TAG, "Exception in executeApi call", e)
       val result = Intent()
       result.putExtra(RESULT_CODE, RESULT_CODE_ERROR)
       result.putExtra(RESULT_ERROR, OpenPgpError(OpenPgpError.CLIENT_SIDE_ERROR, e.message))
       result
     } finally {
-      // close() is required to halt the TransferThread
       if (output != null) {
         try {
           output.close()

openpgp-ktx/src/main/java/me/msfjarvis/openpgpktx/util/ParcelFileDescriptorUtil.kt

@@ -2,6 +2,8 @@
  * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved.
  * SPDX-License-Identifier: Apache-2.0
  */
+@file:Suppress("BlockingMethodInNonBlockingContext")
+
 package me.msfjarvis.openpgpktx.util
 
 import android.os.ParcelFileDescriptor
@@ -11,30 +13,27 @@ import android.util.Log
 import java.io.IOException
 import java.io.InputStream
 import java.io.OutputStream
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 
 internal object ParcelFileDescriptorUtil {
 
   private const val TAG = "PFDUtils"
 
-  @Throws(IOException::class)
+  internal suspend fun pipeFrom(inputStream: InputStream): ParcelFileDescriptor {
-  internal fun pipeFrom(inputStream: InputStream): ParcelFileDescriptor {
     val pipe = ParcelFileDescriptor.createPipe()
     val readSide = pipe[0]
     val writeSide = pipe[1]
-    TransferThread(inputStream, AutoCloseOutputStream(writeSide)).start()
+    transferStreams(inputStream, AutoCloseOutputStream(writeSide))
     return readSide
   }
 
-  @Throws(IOException::class)
+  internal suspend fun pipeTo(outputStream: OutputStream, output: ParcelFileDescriptor?) {
-  internal fun pipeTo(outputStream: OutputStream, output: ParcelFileDescriptor?): TransferThread {
+    transferStreams(AutoCloseInputStream(output), outputStream)
-    val t = TransferThread(AutoCloseInputStream(output), outputStream)
-    t.start()
-    return t
   }
 
-  internal class TransferThread(val `in`: InputStream, private val out: OutputStream) : Thread("IPC Transfer Thread") {
+  private suspend fun transferStreams(`in`: InputStream, `out`: OutputStream) {
-
+    withContext(Dispatchers.IO) {
-    override fun run() {
       val buf = ByteArray(4096)
       var len: Int
       try {
@@ -52,9 +51,5 @@ internal object ParcelFileDescriptorUtil {
         } catch (ignored: IOException) {}
       }
     }
-
-    init {
-      isDaemon = true
-    }
   }
 }