begin on tests for SessionApi

client/src/main/java/online/util/DesktopEntropyClient.java

@@ -84,6 +84,6 @@ public String sendSyncOnDevice(MessageSender runnable)
 	@Override
 	public void checkForUpdates()
 	{
-		UpdateManager.INSTANCE.checkForUpdates(OnlineConstants.ENTROPY_VERSION_NUMBER);
+		Globals.INSTANCE.getUpdateManager().checkForUpdates(OnlineConstants.ENTROPY_VERSION_NUMBER);
 	}
 }

client/src/main/kotlin/http/HttpClient.kt

@@ -11,13 +11,22 @@ import logging.Severity
 import org.apache.http.HttpHeaders
 import utils.InjectedThings.logger
 
-class HttpClient(val baseUrl: String) {
-    val jsonObjectMapper = JsonObjectMapper()
+class HttpClient(private val baseUrl: String) {
+    private val jsonObjectMapper = JsonObjectMapper()
 
     inline fun <reified T : Any?> doCall(
         method: HttpMethod,
         route: String,
         payload: Any? = null,
+    ): ApiResponse<T> {
+        return doCall(method, route, payload, T::class.java)
+    }
+
+    fun <T> doCall(
+        method: HttpMethod,
+        route: String,
+        payload: Any? = null,
+        responseType: Class<T>?,
     ): ApiResponse<T> {
         val requestId = UUID.randomUUID()
         val requestJson = payload?.let { jsonObjectMapper.writeValue(payload) }
@@ -42,23 +51,24 @@ class HttpClient(val baseUrl: String) {
 
         try {
             val response = request.asString()
-            return handleResponse(response, requestId, route, method, requestJson)
+            return handleResponse(response, requestId, route, method, requestJson, responseType)
         } catch (e: UnirestException) {
             logUnirestError(requestId, route, method, requestJson, e)
             return CommunicationError(e)
         }
     }
 
-    inline fun <reified T : Any?> handleResponse(
+    private fun <T : Any?> handleResponse(
         response: HttpResponse<String>,
         requestId: UUID,
         route: String,
         method: HttpMethod,
-        requestJson: String?
+        requestJson: String?,
+        responseType: Class<T>?,
     ): ApiResponse<T> =
         if (response.isSuccess) {
             logResponse(Severity.INFO, requestId, route, method, requestJson, response)
-            val body = jsonObjectMapper.readValue(response.body, T::class.java)
+            val body = jsonObjectMapper.readValue(response.body, responseType)
             SuccessResponse(response.status, body)
         } else {
             val errorResponse = tryParseErrorResponse(response)
@@ -71,38 +81,34 @@ class HttpClient(val baseUrl: String) {
                 response,
                 errorResponse,
             )
-            FailureResponse(
-                response.status,
-                errorResponse?.errorCode,
-                errorResponse?.errorMessage,
-            )
+            FailureResponse(response.status, errorResponse?.errorCode, errorResponse?.errorMessage)
         }
 
-    fun tryParseErrorResponse(response: HttpResponse<String>) =
+    private fun tryParseErrorResponse(response: HttpResponse<String>) =
         try {
             jsonObjectMapper.readValue(response.body, ClientErrorResponse::class.java)
         } catch (e: Exception) {
             null
         }
 
-    fun logUnirestError(
+    private fun logUnirestError(
         requestId: UUID,
         route: String,
         method: HttpMethod,
         requestJson: String?,
-        e: UnirestException
+        e: UnirestException,
     ) {
         logger.error(
             "http.error",
             "Caught ${e.message} for $method $route",
             e,
             "requestId" to requestId,
             "requestBody" to requestJson,
-            "unirestError" to e.message
+            "unirestError" to e.message,
         )
     }
 
-    fun logResponse(
+    private fun logResponse(
         level: Severity,
         requestId: UUID,
         route: String,

client/src/main/kotlin/http/SessionApi.kt

@@ -7,16 +7,16 @@ import javax.swing.SwingUtilities
 import kong.unirest.HttpMethod
 import screen.ScreenCache
 import util.DialogUtilNew
+import util.Globals
 import util.OnlineConstants
-import util.UpdateManager.checkForUpdates
 
 class SessionApi(private val httpClient: HttpClient) {
     fun beginSession(name: String) {
         val response =
             httpClient.doCall<BeginSessionResponse>(
                 HttpMethod.POST,
                 Routes.BEGIN_SESSION,
-                BeginSessionRequest(name)
+                BeginSessionRequest(name),
             )
 
         when (response) {
@@ -47,7 +47,9 @@ class SessionApi(private val httpClient: HttpClient) {
                         )
 
                     if (ans == JOptionPane.YES_OPTION) {
-                        checkForUpdates(OnlineConstants.ENTROPY_VERSION_NUMBER)
+                        Globals.updateManager.checkForUpdates(
+                            OnlineConstants.ENTROPY_VERSION_NUMBER
+                        )
                     }
                 }
                 else -> DialogUtilNew.showError("An error occurred.\n\n${response.errorMessage}")

client/src/main/kotlin/util/Globals.kt

@@ -11,4 +11,5 @@ object Globals {
     val healthCheckApi = HealthCheckApi(httpClient)
     val devApi = DevApi(httpClient)
     val sessionApi = SessionApi(httpClient)
+    var updateManager = UpdateManager()
 }

client/src/main/kotlin/util/UpdateManager.kt

@@ -16,7 +16,7 @@ import utils.InjectedThings.logger
  *
  * https://developer.github.com/v3/repos/releases/#get-the-latest-release
  */
-object UpdateManager {
+class UpdateManager {
     fun checkForUpdates(currentVersion: String) {
         // Show this here, checking the CRC can take time
         logger.info("updateCheck", "Checking for updates - my version is $currentVersion")
@@ -75,7 +75,7 @@ object UpdateManager {
         val answer =
             DialogUtilNew.showQuestion(
                 "An update is available (${metadata.version}). Would you like to download it now?",
-                false
+                false,
             )
         return answer == JOptionPane.YES_OPTION
     }
@@ -109,7 +109,7 @@ object UpdateManager {
                 "parseError",
                 "Error parsing update response",
                 t,
-                "responseBody" to responseJson
+                "responseBody" to responseJson,
             )
             null
         }
@@ -146,7 +146,7 @@ data class UpdateMetadata(
     val version: String,
     val assetId: Long,
     val fileName: String,
-    val size: Long
+    val size: Long,
 ) {
     fun getArgs() = "$size $version $fileName $assetId"
 }

client/src/test/kotlin/http/SessionApiTest.kt

@@ -0,0 +1,79 @@
+package http
+
+import com.github.alyssaburlton.swingtest.clickNo
+import com.github.alyssaburlton.swingtest.clickYes
+import com.github.alyssaburlton.swingtest.flushEdt
+import http.Routes.BEGIN_SESSION
+import http.dto.BeginSessionRequest
+import http.dto.BeginSessionResponse
+import io.kotest.matchers.shouldBe
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.verify
+import kong.unirest.HttpMethod
+import main.kotlin.testCore.getDialogMessage
+import main.kotlin.testCore.getQuestionDialog
+import main.kotlin.testCore.verifyNotCalled
+import org.junit.jupiter.api.Test
+import testCore.AbstractTest
+import util.Globals
+import util.OnlineConstants
+
+class SessionApiTest : AbstractTest() {
+    @Test
+    fun `should POST to the correct endpoint`() {
+        val httpClient = mockk<HttpClient>(relaxed = true)
+
+        val api = SessionApi(httpClient)
+        api.beginSession("alyssa")
+
+        verify {
+            httpClient.doCall<BeginSessionResponse>(
+                HttpMethod.POST,
+                BEGIN_SESSION,
+                BeginSessionRequest("alyssa"),
+            )
+        }
+    }
+
+    @Test
+    fun `should handle a response indicating an update is required and check for updates`() {
+        Globals.updateManager = mockk(relaxed = true)
+        val httpClient = mockHttpClient(FailureResponse(422, UPDATE_REQUIRED, "oh no"))
+
+        SessionApi(httpClient).beginSession("alyssa")
+        flushEdt()
+
+        val questionDialog = getQuestionDialog()
+        questionDialog.getDialogMessage() shouldBe
+            "Your client must be updated to connect. Check for updates now?"
+
+        questionDialog.clickYes()
+        verify { Globals.updateManager.checkForUpdates(OnlineConstants.ENTROPY_VERSION_NUMBER) }
+    }
+
+    @Test
+    fun `should not check for updates if 'No' is answered`() {
+        Globals.updateManager = mockk(relaxed = true)
+        val httpClient = mockHttpClient(FailureResponse(422, UPDATE_REQUIRED, "oh no"))
+
+        SessionApi(httpClient).beginSession("alyssa")
+        flushEdt()
+
+        val questionDialog = getQuestionDialog()
+        questionDialog.getDialogMessage() shouldBe
+            "Your client must be updated to connect. Check for updates now?"
+
+        questionDialog.clickNo()
+        verifyNotCalled { Globals.updateManager.checkForUpdates(any()) }
+    }
+
+    private fun mockHttpClient(response: ApiResponse<BeginSessionResponse>): HttpClient {
+        val httpClient = mockk<HttpClient>(relaxed = true)
+        every {
+            httpClient.doCall<BeginSessionResponse>(HttpMethod.POST, BEGIN_SESSION, any())
+        } returns FailureResponse(422, UPDATE_REQUIRED, "oh no")
+
+        return httpClient
+    }
+}

client/src/test/kotlin/util/UpdateManagerTest.kt

@@ -76,7 +76,7 @@ class UpdateManagerTest : AbstractTest() {
     }
 
     private fun queryLatestReleastJsonExpectingError(repositoryUrl: String): String {
-        val result = runAsync { UpdateManager.queryLatestReleaseJson(repositoryUrl) }
+        val result = runAsync { UpdateManager().queryLatestReleaseJson(repositoryUrl) }
 
         val error = getErrorDialog()
         val errorText = error.getDialogMessage()
@@ -93,7 +93,7 @@ class UpdateManagerTest : AbstractTest() {
     @Tag("integration")
     fun `Should retrieve a valid latest asset from the remote repo`() {
         val responseJson =
-            UpdateManager.queryLatestReleaseJson(OnlineConstants.ENTROPY_REPOSITORY_URL)!!
+            UpdateManager().queryLatestReleaseJson(OnlineConstants.ENTROPY_REPOSITORY_URL)!!
 
         val version = responseJson.getString("tag_name")
         version.shouldStartWith("v")
@@ -115,7 +115,7 @@ class UpdateManagerTest : AbstractTest() {
                     ]
                 }"""
 
-        val metadata = UpdateManager.parseUpdateMetadata(JSONObject(json))!!
+        val metadata = UpdateManager().parseUpdateMetadata(JSONObject(json))!!
         metadata.version shouldBe "foo"
         metadata.assetId shouldBe 123456
         metadata.fileName shouldBe "Dartzee_v_foo.jar"
@@ -125,7 +125,7 @@ class UpdateManagerTest : AbstractTest() {
     @Test
     fun `Should log an error if no tag_name is present`() {
         val json = "{\"other_tag\":\"foo\"}"
-        val metadata = UpdateManager.parseUpdateMetadata(JSONObject(json))
+        val metadata = UpdateManager().parseUpdateMetadata(JSONObject(json))
         metadata shouldBe null
 
         val log = verifyLog("parseError", Severity.ERROR)
@@ -136,7 +136,7 @@ class UpdateManagerTest : AbstractTest() {
     @Test
     fun `Should log an error if no assets are found`() {
         val json = """{"assets":[],"tag_name":"foo"}"""
-        val metadata = UpdateManager.parseUpdateMetadata(JSONObject(json))
+        val metadata = UpdateManager().parseUpdateMetadata(JSONObject(json))
         metadata shouldBe null
 
         val log = verifyLog("parseError", Severity.ERROR)
@@ -152,10 +152,11 @@ class UpdateManagerTest : AbstractTest() {
                 OnlineConstants.ENTROPY_VERSION_NUMBER,
                 123456,
                 "EntropyLive_x_y.jar",
-                100
+                100,
             )
 
-        UpdateManager.shouldUpdate(OnlineConstants.ENTROPY_VERSION_NUMBER, metadata) shouldBe false
+        UpdateManager().shouldUpdate(OnlineConstants.ENTROPY_VERSION_NUMBER, metadata) shouldBe
+            false
         val log = verifyLog("updateResult")
         log.message shouldBe "Up to date"
     }
@@ -211,7 +212,7 @@ class UpdateManagerTest : AbstractTest() {
     private fun shouldUpdateAsync(currentVersion: String, metadata: UpdateMetadata): AtomicBoolean {
         val result = AtomicBoolean(false)
         SwingUtilities.invokeLater {
-            result.set(UpdateManager.shouldUpdate(currentVersion, metadata))
+            result.set(UpdateManager().shouldUpdate(currentVersion, metadata))
         }
 
         flushEdt()
@@ -224,7 +225,7 @@ class UpdateManagerTest : AbstractTest() {
         val updateFile = File("update.bat")
         updateFile.writeText("blah")
 
-        UpdateManager.prepareBatchFile()
+        UpdateManager().prepareBatchFile()
 
         updateFile.readText() shouldBe javaClass.getResource("/update/update.bat").readText()
         updateFile.delete()
@@ -237,7 +238,7 @@ class UpdateManagerTest : AbstractTest() {
         val error = IOException("Argh")
         every { runtime.exec(any<String>()) } throws error
 
-        runAsync { assertDoesNotExit { UpdateManager.startUpdate("foo", runtime) } }
+        runAsync { assertDoesNotExit { UpdateManager().startUpdate("foo", runtime) } }
 
         val errorDialog = getErrorDialog()
         errorDialog.getDialogMessage() shouldBe
@@ -251,6 +252,6 @@ class UpdateManagerTest : AbstractTest() {
     fun `Should exit normally if batch file succeeds`() {
         val runtime = mockk<Runtime>(relaxed = true)
 
-        assertExits(0) { UpdateManager.startUpdate("foo", runtime) }
+        assertExits(0) { UpdateManager().startUpdate("foo", runtime) }
     }
 }

test-core/src/main/kotlin/testCore/TestUtils.kt

@@ -5,6 +5,7 @@ import com.github.alyssaburlton.swingtest.findWindow
 import com.github.alyssaburlton.swingtest.flushEdt
 import io.kotest.matchers.maps.shouldContainExactly
 import io.kotest.matchers.shouldBe
+import io.mockk.verify
 import java.time.Instant
 import javax.swing.JDialog
 import javax.swing.JLabel
@@ -22,7 +23,7 @@ fun makeLogRecord(
     loggingCode: String = "log",
     message: String = "A thing happened",
     errorObject: Throwable? = null,
-    keyValuePairs: Map<String, Any?> = mapOf()
+    keyValuePairs: Map<String, Any?> = mapOf(),
 ) = LogRecord(timestamp, severity, loggingCode, message, errorObject, keyValuePairs)
 
 fun getInfoDialog() = getOptionPaneDialog("Information")
@@ -50,3 +51,7 @@ fun <T> List<T>.only(): T {
     size shouldBe 1
     return first()
 }
+
+fun verifyNotCalled(verifyBlock: io.mockk.MockKVerificationScope.() -> Unit) {
+    verify(exactly = 0) { verifyBlock() }
+}