Repository: andregasser/bigbone (kotlin)

Issue:
Fallback URL for streaming API seems malformed
Hi,

First thank you very much for providing this library. I'm working on a social wall project for Mastodon and bigbone is a huge help.

During a try to implement an end to end test with Mastodon in a container i stumbled across the fallback URL for the streaming API. The fallback was triggered since I tried the `LIMITED_FEREDATION_MODE` in the Mastodon .env file. After enabling that, the `/api/v2/instance` endpoint is only accessable after authentication and so the fallback in your [MastodonClient](https://github.com/andregasser/bigbone/blob/de0dbc7718dff43ea1a807474640524cd3e3df4a/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt#L1041C1-L1041C87) is used:

`
streamingUrl = getStreamingApiUrl(fallbackUrl = scheme + instanceName)
`

While debugging it became clear, that the constructed URL is missing the seperator between the scheme and the instanceName. E.g. if my container with mastodon is reachable via https as `mymastodoninstance` it is `httpsmymastodoninstance`. But it should be `https://mymastodoninstance`.

I'm not adapted to Kotlin yet so I can't provide a unit test at the moment. But maybe in the future.
Do you need any other information to verify this?

Kind regards
seism0saurus

Pull request description:
# Description

A user of the library mentioned in #530 that the streaming fallback URL is malformed in that it doesn’t contain `://`, e.g. `httpsmastodon.social` instead of `https://mastodon.social`.

This PR ensures that the URL follows URL specifications by using `HttpUrl` to create a URL.
To not always run the builder, I’ve put it into a lambda that is only called if the fallback URL is actually necessary.

An easier way would be to just write `$scheme://$instanceName` but I decided against it so that OKHTTP `HttpUrl` is used on both ends, ensuring compatibility.

Closes #530 

# Type of Change

- Bug fix

# Breaking Changes

- None

# How Has This Been Tested?

I’ve added three new unit tests to test the main success case and the two fallback cases.

# Mandatory Checklist

- [x] I ran `gradle check` and there were no errors reported
- [x] I have performed a self-review of my code
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] All tests pass locally with my changes
- [x] I have added KDoc documentation to all public methods

Patch:
diff --git a/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt b/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt
index 9dd991be2..fba624dcd 100644
--- a/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt
+++ b/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt
@@ -94,7 +94,7 @@ import javax.net.ssl.X509TrustManager
  */
 class MastodonClient private constructor(
     private val instanceName: String,
-    private val streamingUrl: String,
+    internal val streamingUrl: String,
     private val client: OkHttpClient,
     private val accessToken: String? = null,
     private val debug: Boolean = false,
@@ -939,11 +939,11 @@ class MastodonClient private constructor(
             this.debug = true
         }
 
-        private fun getStreamingApiUrl(fallbackUrl: String): String {
+        private fun getStreamingApiUrl(fallbackUrl: () -> String): String {
             executeInstanceRequest().use { response: Response ->
-                if (!response.isSuccessful) return fallbackUrl
+                if (!response.isSuccessful) return fallbackUrl()
 
-                val streamingUrl: String? = response.body?.string()?.let { responseBody: String ->
+                val streamingUrl: String? = response.body.string().let { responseBody: String ->
                     val rawJsonObject = JSON_SERIALIZER
                         .parseToJsonElement(responseBody)
                         .jsonObject
@@ -956,7 +956,7 @@ class MastodonClient private constructor(
                         ?.replace("wss:", "https:")
                 }
 
-                return streamingUrl ?: fallbackUrl
+                return streamingUrl ?: fallbackUrl()
             }
         }
 
@@ -1038,7 +1038,11 @@ class MastodonClient private constructor(
                 instanceVersion = getInstanceVersion(),
                 scheme = scheme,
                 port = port,
-                streamingUrl = getStreamingApiUrl(fallbackUrl = scheme + instanceName)
+                streamingUrl = getStreamingApiUrl(
+                    fallbackUrl = {
+                        HttpUrl.Builder().scheme(scheme).host(instanceName).toString()
+                    }
+                )
             )
         }
     }


Test patch:
diff --git a/bigbone/src/test/kotlin/social/bigbone/MastodonClientTest.kt b/bigbone/src/test/kotlin/social/bigbone/MastodonClientTest.kt
index dfea5aa8f..d0010969d 100644
--- a/bigbone/src/test/kotlin/social/bigbone/MastodonClientTest.kt
+++ b/bigbone/src/test/kotlin/social/bigbone/MastodonClientTest.kt
@@ -8,6 +8,7 @@ import okhttp3.MediaType.Companion.toMediaType
 import okhttp3.Response
 import okhttp3.ResponseBody.Companion.toResponseBody
 import org.amshove.kluent.invoking
+import org.amshove.kluent.shouldBeEqualTo
 import org.amshove.kluent.shouldThrow
 import org.amshove.kluent.withMessage
 import org.junit.jupiter.api.Test
@@ -27,7 +28,13 @@ class MastodonClientTest {
         val clientBuilder = spyk(MastodonClient.Builder(serverUrl)) {
             // Mock internal NodeInfoClient so that we don't open the site in unit testing
             mockkObject(NodeInfoClient)
-            every { NodeInfoClient.retrieveServerInfo(host = serverUrl, scheme = scheme, port = port) } throws ServerInfoRetrievalException(
+            every {
+                NodeInfoClient.retrieveServerInfo(
+                    host = serverUrl,
+                    scheme = scheme,
+                    port = port
+                )
+            } throws ServerInfoRetrievalException(
                 "just for testing",
                 null
             )
@@ -131,4 +138,104 @@ class MastodonClientTest {
 
         invoking(clientBuilder::build) shouldThrow UnknownHostException::class
     }
+
+    @Test
+    fun `Given streaming URL in instance response, when building MastodonClient, then use that streaming URL`() {
+        val serverUrl = "mastodon.example"
+        val expectedStreamingUrl = "wss://streaming.example.com"
+        val clientBuilder = spyk(MastodonClient.Builder(serverUrl)) {
+            // Mock internal NodeInfoClient so that we don't open the site in unit testing
+            mockkObject(NodeInfoClient)
+            every { NodeInfoClient.retrieveServerInfo(host = serverUrl) } returns Server(
+                schemaVersion = "2.0",
+                software = Server.Software(name = "mastodon", version = "4.0.0")
+            )
+
+            val jsonResponse = """
+                {
+                  "domain": "mastodon.example",
+                  "title": "Mastodon Example",
+                  "version": "4.3.5",
+                  "source_url": "https://github.com/mastodon/mastodon",
+                  "description": "A Mastodon instance for testing",
+                  "configuration": {
+                    "urls": {
+                      "streaming": "$expectedStreamingUrl"
+                    }
+                  }
+                }
+            """.trimIndent()
+
+            val responseMock = mockk<Response> {
+                every { body } answers { jsonResponse.toResponseBody("application/json".toMediaType()) }
+                every { isSuccessful } answers { true }
+                every { close() } returns Unit
+            }
+            every { executeInstanceRequest() } answers { responseMock }
+        }
+
+        val mastodonClient = clientBuilder.build()
+        mastodonClient.streamingUrl shouldBeEqualTo expectedStreamingUrl.replace("wss", "https")
+    }
+
+    @Test
+    fun `Given no streaming URL in instance response, when building MastodonClient, then use fallback streaming url`() {
+        val serverUrl = "mastodon.example"
+        val clientBuilder = spyk(MastodonClient.Builder(serverUrl)) {
+            // Mock internal NodeInfoClient so that we don't open the site in unit testing
+            mockkObject(NodeInfoClient)
+            every { NodeInfoClient.retrieveServerInfo(host = serverUrl) } returns Server(
+                schemaVersion = "2.0",
+                software = Server.Software(name = "mastodon", version = "4.0.0")
+            )
+
+            val jsonResponse = """
+                {
+                  "domain": "mastodon.example",
+                  "title": "Mastodon Example",
+                  "version": "4.3.5",
+                  "source_url": "https://github.com/mastodon/mastodon",
+                  "description": "A Mastodon instance for testing",
+                  "configuration": {
+                    "urls": { }
+                  }
+                }
+            """.trimIndent()
+
+            val responseMock = mockk<Response> {
+                every { body } answers { jsonResponse.toResponseBody("application/json".toMediaType()) }
+                every { isSuccessful } answers { true }
+                every { close() } returns Unit
+            }
+            every { executeInstanceRequest() } answers { responseMock }
+        }
+
+        val mastodonClient = clientBuilder.build()
+        mastodonClient.streamingUrl shouldBeEqualTo "https://$serverUrl/"
+    }
+
+    @Test
+    fun `Given valid server response and no response to instance request, when building server, then use fallback streaming url`() {
+        val serverUrl = "mastodon.example"
+        val clientBuilder = spyk(MastodonClient.Builder(serverUrl)) {
+            // Mock internal NodeInfoClient so that we don't open the site in unit testing
+            mockkObject(NodeInfoClient)
+            every { NodeInfoClient.retrieveServerInfo(host = serverUrl) } returns Server(
+                schemaVersion = "2.0",
+                software = Server.Software(name = "mastodon", version = "4.3.5")
+            )
+
+            val responseMock = mockk<Response> {
+                every { body } answers {
+                    "{\"error\": \"Instance info not found\"}".toResponseBody("application/json".toMediaType())
+                }
+                every { isSuccessful } answers { false }
+                every { close() } returns Unit
+            }
+            every { executeInstanceRequest() } answers { responseMock }
+        }
+
+        val mastodonClient = clientBuilder.build()
+        mastodonClient.streamingUrl shouldBeEqualTo "https://$serverUrl/"
+    }
 }