Repository: andregasser/bigbone
Pull request #512 (2024-12-16 19:28:49)

refactor: pass scheme and port to server info url
# Description

Modifies the retrieve server info method of the `NodeInfoClient` to use the same scheme and port configuration from the calling `MastodonClient` builder.  This is to allow the customization of the link for testing with mocking frameworks, such as WireMock.  It also makes sense to use the same configuration in case a Mastodon instance is running on a non-standard SSL port.

# Type of Change

- Bug fix

# Breaking Changes

- There are no breaking changes introduced by this change

# How Has This Been Tested?

- Unit tests have been modified to validate that the existing tests still pass

# 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

# Optional checks

- [ ] In case you worked on a new feature: Did you also implement the reactive endpoint (bigbone-rx)?
- [ ] In case you added new `*Methods` classes: Did you also reference it in the `MastodonClient` main class?
- [ ] Did you also update the documentation in the `/docs` folder (e.g. API Coverage page)?


PattaFeuFeu: Thank you for your contribution @jdpgrailsdev! I will try to give this PR a review in the course of this week.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/andregasser/bigbone/pull/512?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser) Report
Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 47.75%. Comparing base [(`8b34209`)](https://app.codecov.io/gh/andregasser/bigbone/commit/8b342097307afb6bd2e4fcc48a9490bd12f2cfab?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser) to head [(`aaaeb7a`)](https://app.codecov.io/gh/andregasser/bigbone/commit/aaaeb7a4e61e73ef0b5281e9d6b7dc35e2e59e5d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser).
> Report is 61 commits behind head on master.

| [Files with missing lines](https://app.codecov.io/gh/andregasser/bigbone/pull/512?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser) | Patch % | Lines |
|---|---|---|
| [...n/kotlin/social/bigbone/nodeinfo/NodeInfoClient.kt](https://app.codecov.io/gh/andregasser/bigbone/pull/512?src=pr&el=tree&filepath=bigbone%2Fsrc%2Fmain%2Fkotlin%2Fsocial%2Fbigbone%2Fnodeinfo%2FNodeInfoClient.kt&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser#diff-YmlnYm9uZS9zcmMvbWFpbi9rb3RsaW4vc29jaWFsL2JpZ2JvbmUvbm9kZWluZm8vTm9kZUluZm9DbGllbnQua3Q=) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/andregasser/bigbone/pull/512?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser) |

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff              @@
##             master     #512      +/-   ##
============================================
- Coverage     49.99%   47.75%   -2.24%     
+ Complexity      774      773       -1     
============================================
  Files           176      174       -2     
  Lines          5107     5059      -48     
  Branches        327      268      -59     
============================================
- Hits           2553     2416     -137     
- Misses         2275     2476     +201     
+ Partials        279      167     -112     
```

| [Files with missing lines](https://app.codecov.io/gh/andregasser/bigbone/pull/512?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser) | Coverage Δ | |
|---|---|---|
| [...e/src/main/kotlin/social/bigbone/MastodonClient.kt](https://app.codecov.io/gh/andregasser/bigbone/pull/512?src=pr&el=tree&filepath=bigbone%2Fsrc%2Fmain%2Fkotlin%2Fsocial%2Fbigbone%2FMastodonClient.kt&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser#diff-YmlnYm9uZS9zcmMvbWFpbi9rb3RsaW4vc29jaWFsL2JpZ2JvbmUvTWFzdG9kb25DbGllbnQua3Q=) | `14.52% <100.00%> (-21.18%)` | :arrow_down: |
| [...n/kotlin/social/bigbone/nodeinfo/NodeInfoClient.kt](https://app.codecov.io/gh/andregasser/bigbone/pull/512?src=pr&el=tree&filepath=bigbone%2Fsrc%2Fmain%2Fkotlin%2Fsocial%2Fbigbone%2Fnodeinfo%2FNodeInfoClient.kt&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser#diff-YmlnYm9uZS9zcmMvbWFpbi9rb3RsaW4vc29jaWFsL2JpZ2JvbmUvbm9kZWluZm8vTm9kZUluZm9DbGllbnQua3Q=) | `30.30% <50.00%> (ø)` | |

... and [142 files with indirect coverage changes](https://app.codecov.io/gh/andregasser/bigbone/pull/512/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andr%C3%A9+Gasser)

</details>
PattaFeuFeu: Code coverage being decreased seems to be an issue here and won’t allow me to merge. 

Either @andregasser needs to use his elevated privileges to merge this anyway, or you @jdpgrailsdev would need to modify tests to test the behaviour changes here. I would generally prefer the latter.
jdpgrailsdev: > Code coverage being decreased seems to be an issue here and won’t allow me to merge. 
> 
> Either @andregasser needs to use his elevated privileges to merge this anyway, or you @jdpgrailsdev would need to modify tests to test the behaviour changes here. I would generally prefer the latter.

@PattaFeuFeu Happy to try to add more tests.  I'm not sure why the code covalent decreased -- I modified the existing tests to cover the change.  When I looked at the coverage report, it was not obvious why it decreased.  Any help would be appreciated!
jdpgrailsdev: @PattaFeuFeu I've added some more tests, so hopefully that pleases the coverage gods.
jdpgrailsdev: @PattaFeuFeu It looks like the coverage report is happy now, but the action status is still showing as failed.  I couldn't find a way to rerun the action to get it to update.  Let me know if there is something that I can do to get the check status to update.

Patch:
diff --git a/bigbone/build.gradle b/bigbone/build.gradle
index d5fbeb1..0937d53 100644
--- a/bigbone/build.gradle
+++ b/bigbone/build.gradle
@@ -18,4 +18,5 @@ dependencies {
     testImplementation(libs.kotlin.reflect)
     testImplementation libs.mockk
     testImplementation libs.mockk.dsl
+    testImplementation libs.okhttp.mockwebserver
 }
diff --git a/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt b/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt
index 993f055..c61ddad 100644
--- a/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt
+++ b/bigbone/src/main/kotlin/social/bigbone/MastodonClient.kt
@@ -959,7 +959,7 @@ class MastodonClient private constructor(
         @Throws(BigBoneClientInstantiationException::class)
         private fun getInstanceVersion(): String {
             val serverVersion: String? = NodeInfoClient
-                .retrieveServerInfo(instanceName)
+                .retrieveServerInfo(host = instanceName, scheme = scheme, port = port)
                 ?.software
                 ?.takeIf { it.name == "mastodon" }
                 ?.version
diff --git a/bigbone/src/main/kotlin/social/bigbone/nodeinfo/NodeInfoClient.kt b/bigbone/src/main/kotlin/social/bigbone/nodeinfo/NodeInfoClient.kt
index 32989ea..50f9eda 100644
--- a/bigbone/src/main/kotlin/social/bigbone/nodeinfo/NodeInfoClient.kt
+++ b/bigbone/src/main/kotlin/social/bigbone/nodeinfo/NodeInfoClient.kt
@@ -22,13 +22,16 @@ object NodeInfoClient {
     /**
      * Retrieve server information.
      * @param host hostname of the server to retrieve information from
+     * @param scheme The URL scheme
+     * @param port The URL port
      * @return server information, including the name and version of the software running on this server
      * @throws ServerInfoRetrievalException if the request for a server info to [host] was unsuccessful
      */
-    fun retrieveServerInfo(host: String): Server? {
+    @JvmOverloads
+    fun retrieveServerInfo(host: String, scheme: String = "https", port: Int = 443): Server? {
         CLIENT.newCall(
             Request.Builder()
-                .url(getServerInfoUrl(host))
+                .url(getServerInfoUrl(host = host, scheme = scheme, port = port))
                 .get()
                 .build()
         ).execute().use { response: Response ->
@@ -46,14 +49,16 @@ object NodeInfoClient {
     /**
      * Get the URL to retrieve server information from.
      * @param host the hostname of the server to request information from
+     * @param scheme The URL scheme
+     * @param port The URL port
      * @return String containing the URL holding server information
      * @throws ServerInfoUrlRetrievalException if we could not call the [host] or if the [NodeInfo] was empty
      */
     @Throws(ServerInfoUrlRetrievalException::class)
-    private fun getServerInfoUrl(host: String): String {
+    private fun getServerInfoUrl(host: String, scheme: String, port: Int): String {
         CLIENT.newCall(
             Request.Builder()
-                .url("https://$host/.well-known/nodeinfo")
+                .url("$scheme://$host:$port/.well-known/nodeinfo")
                 .get()
                 .build()
         ).execute().use { response: Response ->


Test patch:
diff --git a/bigbone/src/test/kotlin/social/bigbone/MastodonClientTest.kt b/bigbone/src/test/kotlin/social/bigbone/MastodonClientTest.kt
index f061de6..dfea5aa 100644
--- a/bigbone/src/test/kotlin/social/bigbone/MastodonClientTest.kt
+++ b/bigbone/src/test/kotlin/social/bigbone/MastodonClientTest.kt
@@ -22,10 +22,39 @@ class MastodonClientTest {
     @Test
     fun `Given response body without instance version, when building MastodonClient, then fail with InstanceVersionRetrievalException`() {
         val serverUrl = "foo.bar"
+        val scheme = "http"
+        val port = 443
         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(serverUrl) } throws ServerInfoRetrievalException(
+            every { NodeInfoClient.retrieveServerInfo(host = serverUrl, scheme = scheme, port = port) } throws ServerInfoRetrievalException(
+                "just for testing",
+                null
+            )
+
+            val responseMock = mockk<Response> {
+                val invalidResponseBody = "{ \"foo\": \"bar\" }"
+                every { body } answers { invalidResponseBody.toResponseBody("application/json".toMediaType()) }
+                every { isSuccessful } answers { true }
+                every { close() } returns Unit
+            }
+            every { executeInstanceRequest() } answers { responseMock }
+        }
+            .withHttpsDisabled()
+            .withPort(port)
+
+        invoking(clientBuilder::build)
+            .shouldThrow(ServerInfoRetrievalException::class)
+            .withMessage("just for testing")
+    }
+
+    @Test
+    fun `Given response body without instance version, when building MastodonClient with defaults, then fail with InstanceVersionRetrievalException`() {
+        val serverUrl = "foo.bar"
+        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) } throws ServerInfoRetrievalException(
                 "just for testing",
                 null
             )
@@ -46,11 +75,39 @@ class MastodonClientTest {
 
     @Test
     fun `Given a server that doesn't run Mastodon, when building MastodonClient, then fail with InstanceVersionRetrievalException`() {
+        val serverUrl = "diasp.eu"
+        val scheme = "http"
+        val port = 443
+        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) } returns Server(
+                schemaVersion = "2.0",
+                software = Server.Software(name = "diaspora", version = "0.7.18.2-p84e7e411")
+            )
+
+            val responseMock = mockk<Response> {
+                every { code } answers { 404 }
+                every { message } answers { "Not Found" }
+                every { isSuccessful } answers { false }
+                every { close() } returns Unit
+            }
+            every { executeInstanceRequest() } answers { responseMock }
+        }
+            .withHttpsDisabled()
+            .withPort(port)
+        invoking(clientBuilder::build)
+            .shouldThrow(InstanceVersionRetrievalException::class)
+            .withMessage("Server $serverUrl doesn't appear to run Mastodon")
+    }
+
+    @Test
+    fun `Given a server that doesn't run Mastodon, when building MastodonClient with defaults, then fail with InstanceVersionRetrievalException`() {
         val serverUrl = "diasp.eu"
         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(serverUrl) } returns Server(
+            every { NodeInfoClient.retrieveServerInfo(host = serverUrl) } returns Server(
                 schemaVersion = "2.0",
                 software = Server.Software(name = "diaspora", version = "0.7.18.2-p84e7e411")
             )
diff --git a/bigbone/src/test/kotlin/social/bigbone/nodeinfo/entity/NodeInfoTest.kt b/bigbone/src/test/kotlin/social/bigbone/nodeinfo/entity/NodeInfoTest.kt
index b2c4f5f..958a41a 100644
--- a/bigbone/src/test/kotlin/social/bigbone/nodeinfo/entity/NodeInfoTest.kt
+++ b/bigbone/src/test/kotlin/social/bigbone/nodeinfo/entity/NodeInfoTest.kt
@@ -1,8 +1,16 @@
 package social.bigbone.nodeinfo.entity
 
+import okhttp3.mockwebserver.MockResponse
+import okhttp3.mockwebserver.MockWebServer
 import org.amshove.kluent.shouldBeEqualTo
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertNotNull
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
 import social.bigbone.JSON_SERIALIZER
+import social.bigbone.api.exception.ServerInfoRetrievalException
+import social.bigbone.api.exception.ServerInfoUrlRetrievalException
+import social.bigbone.nodeinfo.NodeInfoClient
 import social.bigbone.testtool.AssetsUtil
 
 class NodeInfoTest {
@@ -21,4 +29,79 @@ class NodeInfoTest {
         link.rel shouldBeEqualTo "http://nodeinfo.diaspora.software/ns/schema/2.0"
         link.href shouldBeEqualTo "https://example.org/nodeinfo/2.0"
     }
+
+    @Test
+    fun retrieveServerInfoWithOverloads() {
+        val mockWebServer = MockWebServer().apply {
+            start()
+        }
+
+        try {
+            val schemaVersion = "1.0.0"
+            val softwareName = "name"
+            val softwareVersion = "1.0.0"
+            val nodeInfoResponseBody = "{\"links\":[{\"href\":\"${mockWebServer.url("")}\",\"rel\":\"http://nodeinfo.diaspora.software/ns/schema/2.0\"}]}"
+            val serverResponseBody = "{\"version\":\"$schemaVersion\", \"software\":{ \"name\":\"$softwareName\", \"version\":\"$softwareVersion\"}}"
+
+            mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody(nodeInfoResponseBody))
+            mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody(serverResponseBody))
+
+            val server = NodeInfoClient.retrieveServerInfo(host = mockWebServer.url("").toUrl().host, scheme = "http", port = mockWebServer.port)
+            assertNotNull(server)
+            assertNotNull(server?.software)
+            assertEquals(schemaVersion, server?.schemaVersion)
+            assertEquals(softwareName, server?.software?.name)
+            assertEquals(softwareVersion, server?.software?.version)
+        } finally {
+            mockWebServer.shutdown()
+        }
+    }
+
+    @Test
+    fun retrieveServerInfoWithOverloadsUnsuccessfulNodeInfoResponse() {
+        val mockWebServer = MockWebServer().apply {
+            start()
+        }
+
+        try {
+            val schemaVersion = "1.0.0"
+            val softwareName = "name"
+            val softwareVersion = "1.0.0"
+            val nodeInfoResponseBody = "{\"links\":[{\"href\":\"${mockWebServer.url("")}\",\"rel\":\"http://nodeinfo.diaspora.software/ns/schema/2.0\"}]}"
+            val serverResponseBody = "{\"version\":\"$schemaVersion\", \"software\":{ \"name\":\"$softwareName\", \"version\":\"$softwareVersion\"}}"
+
+            mockWebServer.enqueue(MockResponse().setResponseCode(400).setBody(nodeInfoResponseBody))
+            mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody(serverResponseBody))
+
+            assertThrows<ServerInfoUrlRetrievalException> {
+                NodeInfoClient.retrieveServerInfo(host = mockWebServer.url("").toUrl().host, scheme = "http", port = mockWebServer.port)
+            }
+        } finally {
+            mockWebServer.shutdown()
+        }
+    }
+
+    @Test
+    fun retrieveServerInfoWithOverloadsUnsuccessfulServerInfoResponse() {
+        val mockWebServer = MockWebServer().apply {
+            start()
+        }
+
+        try {
+            val schemaVersion = "1.0.0"
+            val softwareName = "name"
+            val softwareVersion = "1.0.0"
+            val nodeInfoResponseBody = "{\"links\":[{\"href\":\"${mockWebServer.url("")}\",\"rel\":\"http://nodeinfo.diaspora.software/ns/schema/2.0\"}]}"
+            val serverResponseBody = "{\"version\":\"$schemaVersion\", \"software\":{ \"name\":\"$softwareName\", \"version\":\"$softwareVersion\"}}"
+
+            mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody(nodeInfoResponseBody))
+            mockWebServer.enqueue(MockResponse().setResponseCode(400).setBody(serverResponseBody))
+
+            assertThrows<ServerInfoRetrievalException> {
+                NodeInfoClient.retrieveServerInfo(host = mockWebServer.url("").toUrl().host, scheme = "http", port = mockWebServer.port)
+            }
+        } finally {
+            mockWebServer.shutdown()
+        }
+    }
 }