Code review changes

app/src/androidTest/java/tech/relaycorp/ping/test/IntentsRule.kt

@@ -6,10 +6,15 @@ import org.junit.runner.Description
 import org.junit.runners.model.Statement
 
 class IntentsRule : TestRule {
+
     override fun apply(base: Statement, description: Description?) =
         object : Statement() {
             override fun evaluate() {
-                Intents.init()
+                try {
+                    Intents.init()
+                } catch (_: IllegalStateException) {
+                    // Occasionally `Intents.init()` might be called twice before `Intents.release()` is called
+                }
                 base.evaluate()
                 Intents.release()
             }

app/src/androidTest/java/tech/relaycorp/ping/ui/peers/AddPublicPeerActivityTest.kt

@@ -7,8 +7,6 @@ import android.net.Uri
 import androidx.test.espresso.intent.Intents.intending
 import androidx.test.espresso.intent.matcher.IntentMatchers.hasAction
 import androidx.test.ext.junit.runners.AndroidJUnit4
-import com.schibsted.spain.barista.assertion.BaristaEnabledAssertions.assertDisabled
-import com.schibsted.spain.barista.assertion.BaristaEnabledAssertions.assertEnabled
 import com.schibsted.spain.barista.assertion.BaristaVisibilityAssertions.assertDisplayed
 import com.schibsted.spain.barista.interaction.BaristaClickInteractions.clickOn
 import com.schibsted.spain.barista.interaction.BaristaEditTextInteractions.writeTo
@@ -40,8 +38,6 @@ class AddPublicPeerActivityTest {
         clickOn(R.id.addPeer)
         clickOn(R.string.peer_public)
 
-        assertDisabled(R.id.save)
-
         val address = "ping.awala.services"
         writeTo(R.id.addressEdit, address)
 
@@ -56,34 +52,22 @@ class AddPublicPeerActivityTest {
             )
         clickOn(R.string.peer_certificate_button)
 
-        assertEnabled(R.id.save)
         clickOn(R.id.save)
 
         waitForCurrentActivityToBe(PeersActivity::class)
         assertDisplayed(address)
     }
 
     @Test
-    fun addPublicPeerFailure() {
+    fun addPublicPeerMissingCertificate() {
         testRule.start()
         clickOn(R.id.addPeer)
         clickOn(R.string.peer_public)
 
         val address = "ping.awala.services"
         writeTo(R.id.addressEdit, address)
 
-        intending(hasAction(Intent.ACTION_OPEN_DOCUMENT))
-            .respondWith(
-                Instrumentation.ActivityResult(
-                    Activity.RESULT_OK,
-                    Intent().setData(
-                        Uri.parse("android.resource://invalid")
-                    )
-                )
-            )
-        clickOn(R.string.peer_certificate_button)
-
         clickOn(R.id.save)
-        assertDisplayed(R.string.peer_add_error)
+        assertDisplayed(R.string.peer_add_missing_certificate)
     }
 }

app/src/main/AndroidManifest.xml

@@ -33,6 +33,7 @@
             android:label="@string/peer" />
         <activity
             android:name=".ui.peers.AddPublicPeerActivity"
-            android:label="@string/peer_public" />
+            android:label="@string/peer_public"
+            android:windowSoftInputMode="adjustResize" />
     </application>
 </manifest>

app/src/main/java/tech/relaycorp/ping/data/ReadFile.kt

@@ -2,6 +2,7 @@ package tech.relaycorp.ping.data
 
 import android.content.Context
 import android.net.Uri
+import android.provider.OpenableColumns
 import javax.inject.Inject
 
 class ReadFile
@@ -13,4 +14,17 @@ class ReadFile
         context.contentResolver.openInputStream(uri)?.use {
             it.readBytes()
         } ?: ByteArray(0)
+
+    fun getFileName(uri: Uri): String {
+        if (uri.scheme.equals("content")) {
+            context.contentResolver
+                .query(uri, null, null, null, null)
+                ?.use { cursor ->
+                    if (cursor.moveToFirst()) {
+                        return cursor.getString(cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME))
+                    }
+                }
+        }
+        return uri.path?.split("/")?.lastOrNull() ?: ""
+    }
 }

app/src/main/java/tech/relaycorp/ping/domain/AddPublicPeer.kt

@@ -1,5 +1,6 @@
 package tech.relaycorp.ping.domain
 
+import tech.relaycorp.awaladroid.endpoint.InvalidThirdPartyEndpoint
 import tech.relaycorp.awaladroid.endpoint.PublicThirdPartyEndpoint
 import tech.relaycorp.ping.data.database.dao.PublicPeerDao
 import tech.relaycorp.ping.data.database.entity.PublicPeerEntity
@@ -10,8 +11,13 @@ class AddPublicPeer
     private val publicPeerDao: PublicPeerDao
 ) {
 
+    @Throws(InvalidIdentityCertificate::class)
     suspend fun add(address: String, identity: ByteArray): PublicThirdPartyEndpoint {
-        val endpoint = PublicThirdPartyEndpoint.import(address, identity)
+        val endpoint = try {
+            PublicThirdPartyEndpoint.import(address, identity)
+        } catch (e: InvalidThirdPartyEndpoint) {
+            throw InvalidIdentityCertificate(e)
+        }
         publicPeerDao.save(
             PublicPeerEntity(
                 privateAddress = endpoint.privateAddress,
@@ -21,3 +27,5 @@ class AddPublicPeer
         return endpoint
     }
 }
+
+class InvalidIdentityCertificate(cause: Throwable) : Exception(cause)

app/src/main/java/tech/relaycorp/ping/ui/peers/AddPublicPeerActivity.kt

@@ -51,30 +51,21 @@ class AddPublicPeerActivity : BaseActivity() {
         }
 
         viewModel
-            .hasCertificate()
+            .certificate()
             .onEach {
-                certificateButton.isVisible = !it
-                certificateName.isVisible = it
-                certificateClear.isVisible = it
-            }
-            .launchIn(lifecycleScope)
-
-        viewModel
-            .saveEnabled()
-            .onEach {
-                toolbar.menu.findItem(R.id.save).isEnabled = it
+                val hasCertificate = it.isPresent
+                certificateButton.isVisible = !hasCertificate
+                certificateName.isVisible = hasCertificate
+                certificateName.text = if (hasCertificate) {
+                    it.get().ifBlank { getString(R.string.peer_certificate_picked) }
+                } else ""
+                certificateClear.isVisible = hasCertificate
             }
             .launchIn(lifecycleScope)
 
         viewModel
             .errors()
-            .onEach {
-                messageManager.showError(
-                    when (it) {
-                        AddPublicPeerViewModel.Error.Save -> R.string.peer_add_error
-                    }
-                )
-            }
+            .onEach(this::showError)
             .launchIn(lifecycleScope)
 
         viewModel
@@ -100,6 +91,21 @@ class AddPublicPeerActivity : BaseActivity() {
         )
     }
 
+    private fun showError(error: AddPublicPeerViewModel.Error) {
+        messageManager.showError(
+            when (error) {
+                AddPublicPeerViewModel.Error.InvalidAddress ->
+                    R.string.peer_add_invalid_address
+                AddPublicPeerViewModel.Error.MissingCertificate ->
+                    R.string.peer_add_missing_certificate
+                AddPublicPeerViewModel.Error.InvalidCertificate ->
+                    R.string.peer_add_invalid_certificate
+                AddPublicPeerViewModel.Error.GenericSave ->
+                    R.string.peer_add_error
+            }
+        )
+    }
+
     companion object {
         private const val PICK_CERTIFICATE = 11
 

app/src/main/java/tech/relaycorp/ping/ui/peers/AddPublicPeerViewModel.kt

@@ -4,8 +4,10 @@ import android.net.Uri
 import kotlinx.coroutines.flow.*
 import tech.relaycorp.gateway.ui.BaseViewModel
 import tech.relaycorp.ping.common.PublishFlow
+import tech.relaycorp.ping.common.element
 import tech.relaycorp.ping.data.ReadFile
 import tech.relaycorp.ping.domain.AddPublicPeer
+import tech.relaycorp.ping.domain.InvalidIdentityCertificate
 import tech.relaycorp.ping.ui.common.Click
 import tech.relaycorp.ping.ui.common.Finish
 import tech.relaycorp.ping.ui.common.clicked
@@ -21,9 +23,9 @@ class AddPublicPeerViewModel
 
     // Inputs
 
-    private val alias = MutableStateFlow("")
+    private val address = MutableStateFlow("")
     fun aliasChanged(value: String) {
-        alias.value = value
+        address.value = value
     }
 
     private val certificatePicks = MutableStateFlow(Optional.empty<Uri>())
@@ -43,11 +45,8 @@ class AddPublicPeerViewModel
 
     // Outputs
 
-    private val _hasCertificate = MutableStateFlow(false)
-    fun hasCertificate(): Flow<Boolean> = _hasCertificate.asStateFlow()
-
-    private val _saveEnabled = MutableStateFlow(false)
-    fun saveEnabled(): Flow<Boolean> = _saveEnabled.asStateFlow()
+    private val _certificate = MutableStateFlow(Optional.empty<String>())
+    fun certificate(): Flow<Optional<String>> = _certificate.asStateFlow()
 
     private val _errors = PublishFlow<Error>()
     fun errors(): Flow<Error> = _errors.asFlow()
@@ -56,50 +55,69 @@ class AddPublicPeerViewModel
     fun finish(): Flow<Finish> = _finish.asFlow()
 
     init {
-        val form = alias.combine(certificatePicks, ::Form)
-
-        form
-            .onEach { _saveEnabled.value = it.isValid() }
-            .launchIn(backgroundScope)
+        val form = address.combine(certificatePicks, ::Form)
 
         certificatePicks
-            .onEach { _hasCertificate.value = it.isPresent }
+            .element()
+            .onEach { _certificate.value = Optional.of(readFile.getFileName(it)) }
             .launchIn(backgroundScope)
 
         removeCertificateClicks
             .asFlow()
             .onEach {
                 certificatePicks.value = Optional.empty()
-                _hasCertificate.value = false
+                _certificate.value = Optional.empty()
             }
             .launchIn(backgroundScope)
 
         saveClicks
             .asFlow()
             .flatMapLatest { form.take(1) }
-            .filter { it.isValid() }
-            .onEach { f ->
-                val cert = readFile.read(f.certificateUri.get())
-                addPublicPeer.add(f.alias, cert)
-                _finish.finish()
+            .map { f ->
+                if (f.isValid()) {
+                    val cert = readFile.read(f.certificateUri.get())
+                    addPublicPeer.add(f.address, cert)
+                    _finish.finish()
+                } else {
+                    _errors.send(
+                        when {
+                            !f.isAddressValid() -> Error.InvalidAddress
+                            !f.isCertificatePresent() -> Error.MissingCertificate
+                            else -> Error.GenericSave
+                        }
+                    )
+                }
             }
-            .catch {
-                _errors.send(Error.Save)
+            .catch { exception ->
+                _errors.send(
+                    when (exception) {
+                        is InvalidIdentityCertificate -> Error.InvalidCertificate
+                        else -> Error.GenericSave
+                    }
+                )
+                emit(Unit)
             }
             .launchIn(backgroundScope)
     }
 
     enum class Error {
-        Save
+        InvalidAddress, MissingCertificate, InvalidCertificate, GenericSave
     }
 
     data class Form(
-        val alias: String,
+        val address: String,
         val certificateUri: Optional<Uri>
     ) {
-        fun isValid() =
-            alias.isNotBlank()
-                    && alias.matches(Regex("(\\w+\\.)+\\w+"))
-                    && certificateUri.isPresent
+        fun isAddressValid() =
+            address.isNotBlank() && address.matches(PUBLIC_ADDRESS_REGEX)
+
+        fun isCertificatePresent() = certificateUri.isPresent
+
+        fun isValid() = isAddressValid() && isCertificatePresent()
+    }
+
+    companion object {
+        internal val PUBLIC_ADDRESS_REGEX =
+            Regex("([a-zA-Z0-9][a-zA-Z0-9-]{1,61}[a-zA-Z0-9]\\.)+[a-zA-Z]{2,}")
     }
 }

app/src/main/res/layout/activity_add_public_peer.xml

@@ -64,9 +64,9 @@
                 android:layout_width="0dp"
                 android:layout_height="wrap_content"
                 android:layout_weight="1"
-                android:text="@string/peer_certificate_added"
                 android:textColor="?colorPrimary"
                 android:visibility="gone"
+                tools:text="file.der"
                 tools:visibility="visible" />
 
             <com.google.android.material.button.MaterialButton

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

@@ -9,6 +9,7 @@
     <color name="gray_dark">#666666</color>
     <color name="black">#27302B</color>
     <color name="red">#F44336</color>
+    <color name="gray">#8027302B</color>
 
     <color name="ic_launcher_background">@color/white</color>
 </resources>

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

@@ -30,7 +30,10 @@
     <string name="peer_address_hint">Type an address</string>
     <string name="peer_certificate_label">Identity Certificate *</string>
     <string name="peer_certificate_button">Upload certificate</string>
-    <string name="peer_certificate_added">Certificate picked</string>
+    <string name="peer_certificate_picked">Certificate picked</string>
     <string name="peer_add_error">Error saving new peer</string>
+    <string name="peer_add_invalid_address">Invalid Address</string>
+    <string name="peer_add_missing_certificate">Missing Identity Certificate</string>
+    <string name="peer_add_invalid_certificate">Invalid Identity Certificate</string>
 
 </resources>