Url improvement (#156)

* Save url with string

* throw malformed http url exception if the url is not valid

* catch malformed http url exception

app/src/main/java/io/homeassistant/companion/android/onboarding/manual/ManualSetupPresenterImpl.kt

@@ -1,9 +1,8 @@
 package io.homeassistant.companion.android.onboarding.manual
 
 import android.util.Log
+import io.homeassistant.companion.android.domain.MalformedHttpUrlException
 import io.homeassistant.companion.android.domain.authentication.AuthenticationUseCase
-import java.net.MalformedURLException
-import java.net.URL
 import javax.inject.Inject
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -22,18 +21,15 @@ class ManualSetupPresenterImpl @Inject constructor(
 
     private val mainScope: CoroutineScope = CoroutineScope(Dispatchers.Main + Job())
 
-    override fun onClickOk(urlString: String) {
+    override fun onClickOk(url: String) {
-        val url: URL
-        try {
-            url = URL(urlString)
-        } catch (e: MalformedURLException) {
-            Log.e(TAG, "Unable to parse url", e)
-            view.displayUrlError()
-            return
-        }
-
         mainScope.launch {
-            authenticationUseCase.saveUrl(URL(url.protocol, url.host, url.port, ""))
+            try {
+                authenticationUseCase.saveUrl(url)
+            } catch (e: MalformedHttpUrlException) {
+                Log.e(TAG, "Unable to parse url", e)
+                view.displayUrlError()
+                return@launch
+            }
             view.urlSaved()
         }
     }

app/src/test/java/io/homeassistant/companion/android/onboarding/manual/ManualSetupPresenterImplSpec.kt

@@ -1,11 +1,13 @@
 package io.homeassistant.companion.android.onboarding.manual
 
+import io.homeassistant.companion.android.domain.MalformedHttpUrlException
 import io.homeassistant.companion.android.domain.authentication.AuthenticationUseCase
-import io.mockk.Called
+import io.mockk.coEvery
 import io.mockk.coVerifyAll
+import io.mockk.just
 import io.mockk.mockk
+import io.mockk.runs
 import io.mockk.verify
-import java.net.URL
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.test.resetMain
 import kotlinx.coroutines.test.setMain
@@ -30,10 +32,11 @@ object ManualSetupPresenterImplSpec : Spek({
         describe("on click ok with valid url") {
             beforeEachTest {
                 presenter.onClickOk("https://demo.home-assistant.io:8123/lovelace/default_view?home_assistant=1&true=false")
+                coEvery { authenticationUseCase.saveUrl("https://demo.home-assistant.io:8123/lovelace/default_view?home_assistant=1&true=false") } just runs
             }
 
             it("should save the url") {
-                coVerifyAll { authenticationUseCase.saveUrl(URL("https://demo.home-assistant.io:8123")) }
+                coVerifyAll { authenticationUseCase.saveUrl("https://demo.home-assistant.io:8123/lovelace/default_view?home_assistant=1&true=false") }
             }
 
             it("should notify the listener") {
@@ -43,16 +46,14 @@ object ManualSetupPresenterImplSpec : Spek({
 
         describe("on click with invalid url") {
             beforeEachTest {
+                coEvery { authenticationUseCase.saveUrl("home assistant") } throws MalformedHttpUrlException()
                 presenter.onClickOk("home assistant")
             }
 
-            it("should not save the url") {
+            it("should save the url") {
-                coVerifyAll { authenticationUseCase wasNot Called }
+                coVerifyAll { authenticationUseCase.saveUrl("home assistant") }
             }
 
-            it("shouldn't notify the listener") {
-                verify(exactly = 0) { view.urlSaved() }
-            }
             it("should display url error") {
                 verify { view.displayUrlError() }
             }

data/src/main/java/io/homeassistant/companion/android/data/authentication/AuthenticationRepositoryImpl.kt

@@ -2,11 +2,13 @@ package io.homeassistant.companion.android.data.authentication
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import io.homeassistant.companion.android.data.LocalStorage
+import io.homeassistant.companion.android.domain.MalformedHttpUrlException
 import io.homeassistant.companion.android.domain.authentication.AuthenticationRepository
 import io.homeassistant.companion.android.domain.authentication.SessionState
 import java.net.URL
 import javax.inject.Inject
 import javax.inject.Named
+import okhttp3.HttpUrl
 import okhttp3.HttpUrl.Companion.toHttpUrl
 import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
 import org.threeten.bp.Instant
@@ -24,8 +26,21 @@ class AuthenticationRepositoryImpl @Inject constructor(
         private const val PREF_TOKEN_TYPE = "token_type"
     }
 
-    override suspend fun saveUrl(url: URL) {
+    override suspend fun saveUrl(url: String) {
-        localStorage.putString(PREF_URL, url.toString())
+        val trimUrl = try {
+            val httpUrl = url.toHttpUrl()
+            HttpUrl.Builder()
+                .scheme(httpUrl.scheme)
+                .host(httpUrl.host)
+                .port(httpUrl.port)
+                .toString()
+        } catch (e: IllegalArgumentException) {
+            throw MalformedHttpUrlException(
+                e.message
+            )
+        }
+
+        localStorage.putString(PREF_URL, trimUrl)
     }
 
     override suspend fun registerAuthorizationCode(authorizationCode: String) {

data/src/test/java/io/homeassistant/companion/android/data/authentication/AuthenticationRepositoryImplSpec.kt

@@ -1,6 +1,7 @@
 package io.homeassistant.companion.android.data.authentication
 
 import io.homeassistant.companion.android.data.LocalStorage
+import io.homeassistant.companion.android.domain.MalformedHttpUrlException
 import io.homeassistant.companion.android.domain.authentication.SessionState
 import io.mockk.Called
 import io.mockk.coEvery
@@ -35,13 +36,53 @@ object AuthenticationRepositoryImplSpec : Spek({
             )
         }
 
-        describe("saveUrl") {
+        mapOf(
-            beforeEachTest {
+            "https://demo.home-assistant.io:8123/lovelace/0/default_view?home_assistant=1&true=false" to "https://demo.home-assistant.io:8123/",
-                runBlocking { repository.saveUrl(URL("https://demo.home-assistant.io/")) }
+            "https://demo.home-assistant.io/lovelace/0/default_view?home_assistant=1&true=false" to "https://demo.home-assistant.io/",
-            }
+            "https://demo.home-assistant.io" to "https://demo.home-assistant.io/",
+            "https://192.168.1.1:8123/lovelace/0" to "https://192.168.1.1:8123/",
+            "https://192.168.1.1:8123" to "https://192.168.1.1:8123/",
+            "https://192.168.1.1" to "https://192.168.1.1/"
+        ).forEach {
+            describe("save valid url \"${it.key}\"") {
+                beforeEachTest {
+                    runBlocking { repository.saveUrl(it.key) }
+                }
 
-            it("should save url") {
+                it("should save url") {
-                coVerifyAll { localStorage.putString("url", "https://demo.home-assistant.io/") }
+                    coVerifyAll { localStorage.putString("url", it.value) }
+                }
+            }
+        }
+
+        listOf(
+            "",
+            "home assistant",
+            "http://192.168..132:8123",
+            "http://....:8123",
+            "http://......",
+            "ftp://192.168.1.1"
+        ).forEach {
+            describe("save invalid url \"$it\"") {
+                lateinit var throwable: Throwable
+                beforeEachTest {
+                    runBlocking {
+                        try {
+                            repository.saveUrl(it)
+                        } catch (e: Exception) {
+                            throwable = e
+                        }
+                    }
+                }
+
+                it("shouldn't save url") {
+                    coVerify(exactly = 0) {
+                        localStorage.putString("url", "https://demo.home-assistant.io:8123")
+                    }
+                }
+                it("should throw an exception") {
+                    assertThat(throwable).isInstanceOf(MalformedHttpUrlException::class.java)
+                }
             }
         }
 

domain/src/main/java/io/homeassistant/companion/android/domain/MalformedHttpUrlException.kt

@@ -0,0 +1,8 @@
+package io.homeassistant.companion.android.domain
+
+import java.net.MalformedURLException
+
+class MalformedHttpUrlException : MalformedURLException {
+    constructor() : super()
+    constructor(message: String?) : super(message)
+}

domain/src/main/java/io/homeassistant/companion/android/domain/authentication/AuthenticationRepository.kt

@@ -4,7 +4,7 @@ import java.net.URL
 
 interface AuthenticationRepository {
 
-    suspend fun saveUrl(url: URL)
+    suspend fun saveUrl(url: String)
 
     suspend fun registerAuthorizationCode(authorizationCode: String)
 

domain/src/main/java/io/homeassistant/companion/android/domain/authentication/AuthenticationUseCase.kt

@@ -4,7 +4,7 @@ import java.net.URL
 
 interface AuthenticationUseCase {
 
-    suspend fun saveUrl(url: URL)
+    suspend fun saveUrl(url: String)
 
     suspend fun registerAuthorizationCode(authorizationCode: String)
 

domain/src/main/java/io/homeassistant/companion/android/domain/authentication/AuthenticationUseCaseImpl.kt

@@ -7,7 +7,7 @@ class AuthenticationUseCaseImpl @Inject constructor(
     private val authenticationRepository: AuthenticationRepository
 ) : AuthenticationUseCase {
 
-    override suspend fun saveUrl(url: URL) {
+    override suspend fun saveUrl(url: String) {
         authenticationRepository.saveUrl(url)
     }
 

domain/src/test/java/io/homeassistant/companion/android/domain/authentication/AuthenticationUseCaseImplSpec.kt

@@ -17,11 +17,11 @@ object AuthenticationUseCaseImplSpec : Spek({
 
         describe("save url") {
             beforeEachTest {
-                runBlocking { useCase.saveUrl(URL("https://demo.home-assistant.io/")) }
+                runBlocking { useCase.saveUrl("https://demo.home-assistant.io/") }
             }
 
             it("should call the repository") {
-                coVerify { authenticationRepository.saveUrl(URL("https://demo.home-assistant.io/")) }
+                coVerify { authenticationRepository.saveUrl("https://demo.home-assistant.io/") }
             }
         }