diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53ff699..78cb6fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,8 +8,8 @@ jobs: matrix: project: [core, client, server] steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 with: distribution: zulu java-version: 11.0.18 diff --git a/client/src/main/java/online/screen/EntropyLobby.java b/client/src/main/java/online/screen/EntropyLobby.java index 5937e6c..a1595aa 100644 --- a/client/src/main/java/online/screen/EntropyLobby.java +++ b/client/src/main/java/online/screen/EntropyLobby.java @@ -6,8 +6,6 @@ import http.dto.RoomSummary; import object.RoomTable; import online.util.HeartbeatRunnable; -import online.util.XmlBuilderClient; -import org.w3c.dom.Document; import screen.MainScreen; import screen.ScreenCache; import util.*; @@ -26,7 +24,6 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import static achievement.AchievementUtilKt.getAchievementsEarned; import static util.Images.ICON_ONLINE; public class EntropyLobby extends JFrame @@ -373,12 +370,7 @@ public void exit(boolean forceClose) if (!forceClose) { //Send a disconnect message. - Document disconnectRequest = XmlBuilderClient.factoryDisconnectRequest(username); - String messageString = XmlUtil.getStringFromDocument(disconnectRequest); - - MessageSenderParams params = new MessageSenderParams(messageString, 0, 5); - params.setExpectResponse(false); - MessageUtil.sendMessage(params, true); + ClientGlobals.sessionApi.finishSession(); } ScreenCache.get(MainScreen.class).maximise(); diff --git a/client/src/main/java/online/util/XmlBuilderClient.java b/client/src/main/java/online/util/XmlBuilderClient.java index 3bc2226..b80c84b 100644 --- a/client/src/main/java/online/util/XmlBuilderClient.java +++ b/client/src/main/java/online/util/XmlBuilderClient.java @@ -15,16 +15,6 @@ public static Document factoryHeartbeat(String username) return XmlUtil.factorySimpleMessage(username, ROOT_TAG_HEARTBEAT); } - public static Document factoryDisconnectRequest(String username) - { - Document document = XmlUtil.factoryNewDocument(); - Element rootElement = document.createElement(ROOT_TAG_DISCONNECT_REQUEST); - rootElement.setAttribute("Username", username); - - document.appendChild(rootElement); - return document; - } - public static Document factoryNewChatXml(String roomId, String username, String colour, String message) { Document document = XmlUtil.factoryNewDocument(); diff --git a/client/src/main/java/util/MessageSender.java b/client/src/main/java/util/MessageSender.java index 09571df..b35907a 100644 --- a/client/src/main/java/util/MessageSender.java +++ b/client/src/main/java/util/MessageSender.java @@ -202,7 +202,6 @@ private boolean isResponseIgnored(String xmlStr) String name = rootElement.getTagName(); - return name.equals(XmlConstants.ROOT_TAG_NEW_CHAT) - || name.equals(XmlConstants.ROOT_TAG_DISCONNECT_REQUEST); + return name.equals(XmlConstants.ROOT_TAG_NEW_CHAT); } } diff --git a/client/src/main/kotlin/http/ApiResponse.kt b/client/src/main/kotlin/http/ApiResponse.kt index 547ceff..7aeda2e 100644 --- a/client/src/main/kotlin/http/ApiResponse.kt +++ b/client/src/main/kotlin/http/ApiResponse.kt @@ -2,12 +2,13 @@ package http import kong.unirest.UnirestException -interface ApiResponse +sealed interface ApiResponse data class SuccessResponse(val statusCode: Int, val body: T) : ApiResponse data class FailureResponse( val statusCode: Int, + val body: String, val errorCode: ClientErrorCode?, val errorMessage: String? ) : ApiResponse diff --git a/client/src/main/kotlin/http/HttpClient.kt b/client/src/main/kotlin/http/HttpClient.kt index 068bcc8..f6fed5b 100644 --- a/client/src/main/kotlin/http/HttpClient.kt +++ b/client/src/main/kotlin/http/HttpClient.kt @@ -1,24 +1,24 @@ package http import ch.qos.logback.classic.Level +import com.fasterxml.jackson.core.JsonProcessingException import http.dto.ClientErrorResponse import java.util.* import kong.unirest.HttpMethod import kong.unirest.HttpRequest import kong.unirest.HttpRequestWithBody import kong.unirest.HttpResponse -import kong.unirest.JsonObjectMapper import kong.unirest.Unirest import kong.unirest.UnirestException import logging.KEY_REQUEST_ID import org.apache.http.HttpHeaders +import utils.CoreGlobals.jsonMapper import utils.CoreGlobals.logger class HttpClient(private val baseUrl: String) { var sessionId: UUID? = null - private val jsonObjectMapper = JsonObjectMapper() - inline fun doCall( + inline fun doCall( method: HttpMethod, route: String, payload: Any? = null, @@ -26,14 +26,14 @@ class HttpClient(private val baseUrl: String) { return doCall(method, route, payload, T::class.java) } - fun doCall( + fun doCall( method: HttpMethod, route: String, payload: Any? = null, - responseType: Class?, + responseType: Class, ): ApiResponse { val requestId = UUID.randomUUID() - val requestJson = payload?.let { jsonObjectMapper.writeValue(payload) } + val requestJson = payload?.let { jsonMapper.writeValueAsString(payload) } logger.info( "http.request", @@ -65,27 +65,52 @@ class HttpClient(private val baseUrl: String) { private fun HttpRequest<*>.addSessionId() = sessionId?.let { id -> header(CustomHeader.SESSION_ID, id.toString()) } ?: this - private fun handleResponse( + private fun handleResponse( response: HttpResponse, requestId: UUID, route: String, method: HttpMethod, requestJson: String?, - responseType: Class?, + responseType: Class, ): ApiResponse = if (response.isSuccess) { logResponse(Level.INFO, requestId, route, method, requestJson, response) - val body = jsonObjectMapper.readValue(response.body, responseType) - SuccessResponse(response.status, body) + try { + val body = parseBody(response, responseType) + SuccessResponse(response.status, body) + } catch (e: JsonProcessingException) { + logger.error( + "responseParseError", + "Failed to parse response", + e, + KEY_REQUEST_ID to requestId, + "requestBody" to requestJson, + "responseCode" to response.status, + "responseBody" to response.body?.toString(), + ) + FailureResponse(response.status, response.body, JSON_PARSE_ERROR, e.message) + } } else { val errorResponse = tryParseErrorResponse(response) logResponse(Level.ERROR, requestId, route, method, requestJson, response, errorResponse) - FailureResponse(response.status, errorResponse?.errorCode, errorResponse?.errorMessage) + FailureResponse( + response.status, + response.body, + errorResponse?.errorCode, + errorResponse?.errorMessage + ) + } + + private fun parseBody(response: HttpResponse, responseType: Class): T = + if (responseType == Unit::class.java) { + Unit as T + } else { + jsonMapper.readValue(response.body, responseType) } private fun tryParseErrorResponse(response: HttpResponse) = try { - jsonObjectMapper.readValue(response.body, ClientErrorResponse::class.java) + jsonMapper.readValue(response.body, ClientErrorResponse::class.java) } catch (e: Exception) { null } diff --git a/client/src/main/kotlin/http/SessionApi.kt b/client/src/main/kotlin/http/SessionApi.kt index 46b5959..22b7051 100644 --- a/client/src/main/kotlin/http/SessionApi.kt +++ b/client/src/main/kotlin/http/SessionApi.kt @@ -8,6 +8,7 @@ import javax.swing.JOptionPane import javax.swing.SwingUtilities import kong.unirest.HttpMethod import online.screen.EntropyLobby +import screen.MainScreen import screen.ScreenCache import util.ClientGlobals import util.DialogUtilNew @@ -40,6 +41,11 @@ class SessionApi(private val httpClient: HttpClient) { ) } + fun finishSession() { + httpClient.doCall(HttpMethod.POST, Routes.FINISH_SESSION) + ClientGlobals.httpClient.sessionId = null + } + private fun handleConnectSuccess(response: BeginSessionResponse) { ClientGlobals.httpClient.sessionId = response.sessionId @@ -48,6 +54,8 @@ class SessionApi(private val httpClient: HttpClient) { lobby.setLocationRelativeTo(null) lobby.isVisible = true lobby.init(response.lobby) + + ScreenCache.get().minimise() } private fun handleBeginSessionFailure(response: FailureResponse<*>) = diff --git a/client/src/main/kotlin/util/ClientGlobals.kt b/client/src/main/kotlin/util/ClientGlobals.kt index c3066e8..f6e75d4 100644 --- a/client/src/main/kotlin/util/ClientGlobals.kt +++ b/client/src/main/kotlin/util/ClientGlobals.kt @@ -17,7 +17,7 @@ object ClientGlobals { val consoleAppender = LoggingConsoleAppender(loggingConsole) val healthCheckApi = HealthCheckApi(httpClient) val devApi = DevApi(httpClient) - var sessionApi = SessionApi(httpClient) + @JvmField var sessionApi = SessionApi(httpClient) var updateManager = UpdateManager() val webSocketReceiver = WebSocketReceiver() @JvmField var achievementStore: AbstractSettingStore = DefaultSettingStore("achievements") diff --git a/client/src/main/kotlin/util/UpdateManager.kt b/client/src/main/kotlin/util/UpdateManager.kt index 5127810..437de17 100644 --- a/client/src/main/kotlin/util/UpdateManager.kt +++ b/client/src/main/kotlin/util/UpdateManager.kt @@ -1,13 +1,16 @@ package util import bean.LinkLabel +import http.CommunicationError +import http.FailureResponse +import http.HttpClient +import http.SuccessResponse import java.awt.BorderLayout import java.io.File import javax.swing.JLabel import javax.swing.JOptionPane import javax.swing.JPanel -import kong.unirest.Unirest -import kong.unirest.json.JSONObject +import kong.unirest.HttpMethod import kotlin.system.exitProcess import utils.CoreGlobals.logger @@ -21,44 +24,50 @@ class UpdateManager { // Show this here, checking the CRC can take time logger.info("updateCheck", "Checking for updates - my version is $currentVersion") - val jsonResponse = queryLatestReleaseJson(OnlineConstants.ENTROPY_REPOSITORY_URL) - jsonResponse ?: return + val metadata = queryLatestRelease(OnlineConstants.ENTROPY_REPOSITORY_URL) + metadata ?: return - val metadata = parseUpdateMetadata(jsonResponse) - if (metadata == null || !shouldUpdate(currentVersion, metadata)) { + if (!shouldUpdate(currentVersion, metadata)) { return } - startUpdate(metadata.getArgs(), Runtime.getRuntime()) + startUpdate(metadata.toScriptArgs(), Runtime.getRuntime()) } - fun queryLatestReleaseJson(repositoryUrl: String): JSONObject? { + fun queryLatestRelease(repositoryUrl: String): UpdateMetadata? { try { DialogUtilNew.showLoadingDialog("Checking for updates...") - val response = Unirest.get("$repositoryUrl/releases/latest").asJson() - if (response.status != 200) { - logger.error( - "updateError", - "Received non-success HTTP status: ${response.status} - ${response.statusText}", - "responseBody" to response.body, - ) - DialogUtilNew.showError("Failed to check for updates (unable to connect).") - return null + val client = HttpClient(repositoryUrl) + val result = client.doCall(HttpMethod.GET, "/releases/latest") + return when (result) { + is CommunicationError -> { + logger.error( + "updateError", + "Caught ${result.unirestException} checking for updates", + result.unirestException + ) + DialogUtilNew.showError("Failed to check for updates (unable to connect).") + null + } + is FailureResponse -> { + logger.error( + "updateError", + "Received non-success HTTP status: ${result.statusCode} - ${result.body}", + "responseBody" to result.body + ) + DialogUtilNew.showError("Failed to check for updates (unexpected error).") + null + } + is SuccessResponse -> result.body } - - return response.body.`object` - } catch (t: Throwable) { - logger.error("updateError", "Caught $t checking for updates", t) - DialogUtilNew.showError("Failed to check for updates (unable to connect).") - return null } finally { DialogUtilNew.dismissLoadingDialog() } } fun shouldUpdate(currentVersion: String, metadata: UpdateMetadata): Boolean { - val newVersion = metadata.version + val newVersion = metadata.tag_name if (newVersion == currentVersion) { logger.info("updateResult", "Up to date") return false @@ -74,7 +83,7 @@ class UpdateManager { val answer = DialogUtilNew.showQuestion( - "An update is available (${metadata.version}). Would you like to download it now?", + "An update is available (${metadata.tag_name}). Would you like to download it now?", false, ) return answer == JOptionPane.YES_OPTION @@ -94,27 +103,6 @@ class UpdateManager { DialogUtilNew.showCustomMessage(panel) } - fun parseUpdateMetadata(responseJson: JSONObject): UpdateMetadata? { - return try { - val remoteVersion = responseJson.getString("tag_name") - val assets = responseJson.getJSONArray("assets") - val asset = assets.getJSONObject(0) - - val assetId = asset.getLong("id") - val fileName = asset.getString("name") - val size = asset.getLong("size") - UpdateMetadata(remoteVersion, assetId, fileName, size) - } catch (t: Throwable) { - logger.error( - "parseError", - "Error parsing update response", - t, - "responseBody" to responseJson, - ) - null - } - } - fun startUpdate(args: String, runtime: Runtime) { prepareBatchFile() @@ -141,12 +129,3 @@ class UpdateManager { updateFile.writeText(updateScript) } } - -data class UpdateMetadata( - val version: String, - val assetId: Long, - val fileName: String, - val size: Long, -) { - fun getArgs() = "$size $version $fileName $assetId" -} diff --git a/client/src/main/kotlin/util/UpdateMetadata.kt b/client/src/main/kotlin/util/UpdateMetadata.kt new file mode 100644 index 0000000..fd4e249 --- /dev/null +++ b/client/src/main/kotlin/util/UpdateMetadata.kt @@ -0,0 +1,10 @@ +package util + +data class ReleaseAsset(val id: Long, val name: String, val size: Long) + +data class UpdateMetadata(val tag_name: String, val assets: List) { + fun toScriptArgs(): String { + val asset = assets.first() + return "${asset.size} ${tag_name} ${asset.name} ${asset.id}" + } +} diff --git a/client/src/test/kotlin/http/HttpClientTest.kt b/client/src/test/kotlin/http/HttpClientTest.kt index 1c0d04a..4adbad9 100644 --- a/client/src/test/kotlin/http/HttpClientTest.kt +++ b/client/src/test/kotlin/http/HttpClientTest.kt @@ -8,6 +8,7 @@ import io.kotest.matchers.maps.shouldContainKeys import io.kotest.matchers.shouldBe import io.kotest.matchers.string.shouldContain import io.kotest.matchers.string.shouldNotBeEmpty +import io.kotest.matchers.types.shouldBeInstanceOf import java.util.UUID import kong.unirest.HttpMethod import kong.unirest.HttpStatus @@ -34,7 +35,7 @@ class HttpClientTest : AbstractTest() { val (client, server) = setUpWebServer() server.enqueue(MockResponse().setResponseCode(HttpStatus.OK)) - client.doCall(HttpMethod.GET, "/test-endpoint") + client.doCall(HttpMethod.GET, "/test-endpoint") val request = server.takeRequest() val requestId = request.getHeader("X-Request-ID") @@ -56,7 +57,7 @@ class HttpClientTest : AbstractTest() { val sessionId = UUID.randomUUID() client.sessionId = sessionId - client.doCall(HttpMethod.GET, "/test-endpoint") + client.doCall(HttpMethod.GET, "/test-endpoint") val request = server.takeRequest() val sessionIdFromHeader = request.getHeader(CustomHeader.SESSION_ID) @@ -90,10 +91,12 @@ class HttpClientTest : AbstractTest() { @Test fun `GET with generic error response`() { val (client, server) = setUpWebServer() - server.enqueue(MockResponse().setResponseCode(HttpStatus.NOT_FOUND)) + server.enqueue( + MockResponse().setResponseCode(HttpStatus.NOT_FOUND).setBody("I looked everywhere") + ) val response = client.doCall(HttpMethod.GET, "/test-endpoint") - response shouldBe FailureResponse(HttpStatus.NOT_FOUND, null, null) + response shouldBe FailureResponse(HttpStatus.NOT_FOUND, "I looked everywhere", null, null) val responseLog = verifyLog("http.response", Level.ERROR) responseLog.message shouldBe "Received 404 for GET /test-endpoint" @@ -113,7 +116,12 @@ class HttpClientTest : AbstractTest() { val response = client.doCall(HttpMethod.GET, "/test-endpoint") response shouldBe - FailureResponse(HttpStatus.CONFLICT, ClientErrorCode("oh.dear"), "a bid already exists") + FailureResponse( + HttpStatus.CONFLICT, + """{"errorCode":"oh.dear","errorMessage":"a bid already exists"}""", + ClientErrorCode("oh.dear"), + "a bid already exists" + ) val responseLog = verifyLog("http.response", Level.ERROR) responseLog.message shouldBe "Received 409 for GET /test-endpoint" @@ -143,7 +151,7 @@ class HttpClientTest : AbstractTest() { server.enqueue(successResponse) val response = client.doCall(HttpMethod.GET, "/test-endpoint") - response shouldBe SuccessResponse(200, null) + response shouldBe SuccessResponse(200, Unit) server.requestCount shouldBe 2 } @@ -157,7 +165,7 @@ class HttpClientTest : AbstractTest() { server.enqueue(MockResponse().setResponseCode(HttpStatus.NO_CONTENT)) val response = client.doCall(HttpMethod.POST, "/test-endpoint", request) - response shouldBe SuccessResponse(HttpStatus.NO_CONTENT, null) + response shouldBe SuccessResponse(HttpStatus.NO_CONTENT, Unit) val capturedRequest = server.takeRequest() capturedRequest.method shouldBe "POST" @@ -177,6 +185,24 @@ class HttpClientTest : AbstractTest() { responseLog.findLogField("responseBody") shouldBe "" } + @Test + fun `Should handle JSON parse errors`() { + val (client, server) = setUpWebServer() + val responseBody = """{ + "fieldOne": "foo", + }""" + + server.enqueue(MockResponse().setBody(responseBody)) + + val response = client.doCall(HttpMethod.GET, "/test-endpoint") + response.shouldBeInstanceOf>() + response.statusCode shouldBe 200 + response.body shouldBe responseBody + response.errorCode shouldBe JSON_PARSE_ERROR + + verifyLog("responseParseError", Level.ERROR) + } + @Test fun `Should handle additional fields`() { val (client, server) = setUpWebServer() diff --git a/client/src/test/kotlin/http/SessionApiTest.kt b/client/src/test/kotlin/http/SessionApiTest.kt index 84e641e..f0d25e9 100644 --- a/client/src/test/kotlin/http/SessionApiTest.kt +++ b/client/src/test/kotlin/http/SessionApiTest.kt @@ -47,7 +47,7 @@ class SessionApiTest : AbstractClientTest() { @Test fun `should handle a response indicating an update is required and check for updates`() { ClientGlobals.updateManager = mockk(relaxed = true) - val httpClient = mockHttpClient(FailureResponse(422, UPDATE_REQUIRED, "oh no")) + val httpClient = mockHttpClient(FailureResponse(422, "", UPDATE_REQUIRED, "oh no")) SessionApi(httpClient).beginSession("alyssa") flushEdt() @@ -65,7 +65,7 @@ class SessionApiTest : AbstractClientTest() { @Test fun `should not check for updates if 'No' is answered`() { ClientGlobals.updateManager = mockk(relaxed = true) - val httpClient = mockHttpClient(FailureResponse(422, UPDATE_REQUIRED, "oh no")) + val httpClient = mockHttpClient(FailureResponse(422, "", UPDATE_REQUIRED, "oh no")) SessionApi(httpClient).beginSession("alyssa") flushEdt() @@ -81,7 +81,9 @@ class SessionApiTest : AbstractClientTest() { @Test fun `should show an error for an unexpected error`() { val httpClient = - mockHttpClient(FailureResponse(422, ClientErrorCode("bad"), "Internal Server Error")) + mockHttpClient( + FailureResponse(422, "", ClientErrorCode("bad"), "Internal Server Error") + ) SessionApi(httpClient).beginSession("alyssa") flushEdt() @@ -140,6 +142,18 @@ class SessionApiTest : AbstractClientTest() { } } + @Test + fun `should send a finish session request and clear the session id`() { + ClientGlobals.httpClient.sessionId = UUID.randomUUID() + + val httpClient = mockk(relaxed = true) + SessionApi(httpClient).finishSession() + + verify { httpClient.doCall(HttpMethod.POST, Routes.FINISH_SESSION) } + + ClientGlobals.httpClient.sessionId shouldBe null + } + private fun mockHttpClient(response: ApiResponse): HttpClient { val httpClient = mockk(relaxed = true) every { diff --git a/client/src/test/kotlin/util/UpdateManagerTest.kt b/client/src/test/kotlin/util/UpdateManagerTest.kt index f88e32d..2c59da7 100644 --- a/client/src/test/kotlin/util/UpdateManagerTest.kt +++ b/client/src/test/kotlin/util/UpdateManagerTest.kt @@ -22,10 +22,10 @@ import java.util.concurrent.atomic.AtomicBoolean import javax.swing.SwingUtilities import kong.unirest.Unirest import kong.unirest.UnirestException -import kong.unirest.json.JSONException -import kong.unirest.json.JSONObject import logging.errorObject import logging.findLogField +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test @@ -43,32 +43,34 @@ class UpdateManagerTest : AbstractTest() { @BeforeEach fun beforeEach() { Unirest.config().reset() - Unirest.config().connectTimeout(2000) Unirest.config().socketTimeout(2000) } /** Communication */ @Test - @Tag("integration") fun `Should log out an unexpected HTTP response, along with the full JSON payload`() { - val errorMessage = - queryLatestReleastJsonExpectingError("https://api.github.com/repos/alyssaburlton/foo") - errorMessage shouldBe "Failed to check for updates (unable to connect)." + val server = MockWebServer() + server.start() + server.enqueue(MockResponse().setResponseCode(404).setBody("Not Found")) + + val errorMessage = queryLatestReleastJsonExpectingError(server.url("root").toString()) + errorMessage shouldBe "Failed to check for updates (unexpected error)." val log = verifyLog("updateError", Level.ERROR) - log.message shouldBe "Received non-success HTTP status: 404 - Not Found" - log.findLogField("responseBody").toString() shouldContain """"message":"Not Found"""" + log.message shouldContain "Received non-success HTTP status: 404" + log.findLogField("responseBody").toString() shouldContain "Not Found" findWindow()!!.shouldNotBeVisible() } @Test - @Tag("integration") fun `Should catch and log any exceptions communicating over HTTPS`() { - Unirest.config().connectTimeout(100) Unirest.config().socketTimeout(100) - val errorMessage = queryLatestReleastJsonExpectingError("https://ww.blargh.zcss.w") + val server = MockWebServer() + server.start() + + val errorMessage = queryLatestReleastJsonExpectingError(server.url("root").toString()) errorMessage shouldBe "Failed to check for updates (unable to connect)." val errorLog = verifyLog("updateError", Level.ERROR) @@ -78,7 +80,7 @@ class UpdateManagerTest : AbstractTest() { } private fun queryLatestReleastJsonExpectingError(repositoryUrl: String): String { - val result = runAsync { UpdateManager().queryLatestReleaseJson(repositoryUrl) } + val result = runAsync { UpdateManager().queryLatestRelease(repositoryUrl) } val error = getErrorDialog() val errorText = error.getDialogMessage() @@ -95,11 +97,10 @@ 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().queryLatestRelease(OnlineConstants.ENTROPY_REPOSITORY_URL)!! - val version = responseJson.getString("tag_name") - version.shouldStartWith("v") - responseJson.getJSONArray("assets").length() shouldBeGreaterThan 0 + responseJson.tag_name.shouldStartWith("v") + responseJson.assets.size shouldBeGreaterThan 0 } /** Parsing */ @@ -117,45 +118,35 @@ class UpdateManagerTest : AbstractTest() { ] }""" - val metadata = UpdateManager().parseUpdateMetadata(JSONObject(json))!! - metadata.version shouldBe "foo" - metadata.assetId shouldBe 123456 - metadata.fileName shouldBe "Dartzee_v_foo.jar" - metadata.size shouldBe 1 + val server = MockWebServer() + server.start() + server.enqueue(MockResponse().setBody(json)) + + val expectedMetadata = + UpdateMetadata("foo", listOf(ReleaseAsset(123456L, "Dartzee_v_foo.jar", 1))) + + val metadata = UpdateManager().queryLatestRelease(server.url("root").toString()) + metadata shouldBe expectedMetadata } @Test fun `Should log an error if no tag_name is present`() { val json = "{\"other_tag\":\"foo\"}" - val metadata = UpdateManager().parseUpdateMetadata(JSONObject(json)) - metadata shouldBe null + val server = MockWebServer() + server.start() + server.enqueue(MockResponse().setBody(json)) - val log = verifyLog("parseError", Level.ERROR) - log.errorObject().shouldBeInstanceOf() - log.findLogField("responseBody").toString() shouldBe json - } + val errorMessage = queryLatestReleastJsonExpectingError(server.url("root").toString()) + errorMessage shouldBe "Failed to check for updates (unexpected error)." - @Test - fun `Should log an error if no assets are found`() { - val json = """{"assets":[],"tag_name":"foo"}""" - val metadata = UpdateManager().parseUpdateMetadata(JSONObject(json)) - metadata shouldBe null - - val log = verifyLog("parseError", Level.ERROR) - log.errorObject().shouldBeInstanceOf() - log.findLogField("responseBody").toString() shouldBe json + verifyLog("responseParseError", Level.ERROR) } /** Should update? */ @Test fun `Should not proceed with the update if the versions match`() { - val metadata = - UpdateMetadata( - OnlineConstants.ENTROPY_VERSION_NUMBER, - 123456, - "EntropyLive_x_y.jar", - 100, - ) + val asset = ReleaseAsset(123456, "EntropyLive_x_y.jar", 100) + val metadata = UpdateMetadata(OnlineConstants.ENTROPY_VERSION_NUMBER, listOf(asset)) UpdateManager().shouldUpdate(OnlineConstants.ENTROPY_VERSION_NUMBER, metadata) shouldBe false @@ -167,7 +158,8 @@ class UpdateManagerTest : AbstractTest() { fun `Should show an info and not proceed to auto update if OS is not windows`() { ClientUtil.operatingSystem = "foo" - val metadata = UpdateMetadata("v100", 123456, "Dartzee_x_y.jar", 100) + val asset = ReleaseAsset(123456, "EntropyLive_x_y.jar", 100) + val metadata = UpdateMetadata("v100", listOf(asset)) shouldUpdateAsync(OnlineConstants.ENTROPY_VERSION_NUMBER, metadata).get() shouldBe false val log = verifyLog("updateAvailable") @@ -183,7 +175,8 @@ class UpdateManagerTest : AbstractTest() { fun `Should not proceed with the update if user selects 'No'`() { ClientUtil.operatingSystem = "windows" - val metadata = UpdateMetadata("foo", 123456, "Dartzee_x_y.jar", 100) + val asset = ReleaseAsset(123456, "EntropyLive_x_y.jar", 100) + val metadata = UpdateMetadata("foo", listOf(asset)) val result = shouldUpdateAsync("bar", metadata) val question = getQuestionDialog() @@ -199,7 +192,8 @@ class UpdateManagerTest : AbstractTest() { fun `Should proceed with the update if user selects 'Yes'`() { ClientUtil.operatingSystem = "windows" - val metadata = UpdateMetadata("foo", 123456, "Dartzee_x_y.jar", 100) + val asset = ReleaseAsset(123456, "EntropyLive_x_y.jar", 100) + val metadata = UpdateMetadata("foo", listOf(asset)) val result = shouldUpdateAsync("bar", metadata) val question = getQuestionDialog() diff --git a/core/src/main/java/util/XmlConstants.java b/core/src/main/java/util/XmlConstants.java index 074eeca..9a882ca 100644 --- a/core/src/main/java/util/XmlConstants.java +++ b/core/src/main/java/util/XmlConstants.java @@ -7,7 +7,6 @@ public interface XmlConstants { //Client public static final String ROOT_TAG_HEARTBEAT = "Heartbeat"; - public static final String ROOT_TAG_DISCONNECT_REQUEST = "DisconnectRequest"; public static final String ROOT_TAG_NEW_CHAT = "NewChat"; public static final String ROOT_TAG_ROOM_JOIN_REQUEST = "RoomJoinRequest"; public static final String ROOT_TAG_CLOSE_ROOM_REQUEST = "CloseRoomRequest"; @@ -41,6 +40,4 @@ public interface XmlConstants public static final String RESPONSE_TAG_ACKNOWLEDGEMENT = "Acknowledgement"; public static final String RESPONSE_TAG_STACK_TRACE = "StackTrace"; public static final String RESPONSE_TAG_SOCKET_TIME_OUT = "SocketTimeOut"; - - public static final String REMOVAL_REASON_FAILED_USERNAME_CHECK = "There has been an authentication error."; } diff --git a/core/src/main/kotlin/http/ClientErrorCodes.kt b/core/src/main/kotlin/http/ClientErrorCodes.kt index 2057b10..97da92a 100644 --- a/core/src/main/kotlin/http/ClientErrorCodes.kt +++ b/core/src/main/kotlin/http/ClientErrorCodes.kt @@ -6,3 +6,4 @@ val INVALID_API_VERSION = ClientErrorCode("invalidApiVersion") val EMPTY_NAME = ClientErrorCode("emptyName") val INVALID_ACHIEVEMENT_COUNT = ClientErrorCode("invalidAchievementCount") val INVALID_SESSION = ClientErrorCode("invalidSession") +val JSON_PARSE_ERROR = ClientErrorCode("jsonParseError") diff --git a/core/src/main/kotlin/http/Routes.kt b/core/src/main/kotlin/http/Routes.kt index 1059f8a..12263cf 100644 --- a/core/src/main/kotlin/http/Routes.kt +++ b/core/src/main/kotlin/http/Routes.kt @@ -5,4 +5,5 @@ object Routes { const val DEV_COMMAND = "/dev-command" const val BEGIN_SESSION = "/begin-session" const val ACHIEVEMENT_COUNT = "/achievement-count" + const val FINISH_SESSION = "/finish-session" } diff --git a/core/src/main/kotlin/utils/CoreGlobals.kt b/core/src/main/kotlin/utils/CoreGlobals.kt index 742467b..610a97b 100644 --- a/core/src/main/kotlin/utils/CoreGlobals.kt +++ b/core/src/main/kotlin/utils/CoreGlobals.kt @@ -1,6 +1,8 @@ package utils import ch.qos.logback.classic.Logger as LogbackLogger +import com.fasterxml.jackson.databind.DeserializationFeature +import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.json.JsonMapper import com.fasterxml.jackson.module.kotlin.registerKotlinModule import java.time.Clock @@ -11,5 +13,8 @@ object CoreGlobals { val slf4jLogger: LogbackLogger = LoggerFactory.getLogger("entropy") as LogbackLogger @JvmField var logger: Logger = Logger(slf4jLogger) var clock: Clock = Clock.systemUTC() - val jsonMapper = JsonMapper().also { it.registerKotlinModule() } + val jsonMapper: ObjectMapper = + JsonMapper() + .registerKotlinModule() + .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) } diff --git a/server/src/main/java/server/EntropyServer.java b/server/src/main/java/server/EntropyServer.java index 78cacec..aeddc02 100644 --- a/server/src/main/java/server/EntropyServer.java +++ b/server/src/main/java/server/EntropyServer.java @@ -8,7 +8,6 @@ import object.Room; import object.ServerRunnable; import object.ServerThread; -import org.w3c.dom.Document; import util.*; import java.util.ArrayList; @@ -43,7 +42,6 @@ private void onStart() { Debug.append("Starting permanent threads"); - startInactiveCheckRunnable(); startListenerThreads(); Debug.appendBanner("Server is ready - accepting connections"); @@ -111,13 +109,6 @@ private void registerDefaultRooms() { Debug.append("Finished creating " + hmRoomByName.size() + " rooms"); } - private void startInactiveCheckRunnable() { - InactiveCheckRunnable runnable = new InactiveCheckRunnable(this); - - ServerThread inactiveCheckThread = new ServerThread(runnable, "InactiveCheck"); - inactiveCheckThread.start(); - } - private void startListenerThreads() { try { ServerThread listenerThread = new ServerThread(new MessageListener(this, SERVER_PORT_NUMBER)); @@ -132,37 +123,7 @@ public void executeInWorkerPool(ServerRunnable runnable) { ServerGlobals.workerPool.executeServerRunnable(runnable); } - public void removeFromUsersOnline(UserConnection usc) { - //Null these out so we don't try to send any more notifications - usc.destroyNotificationSockets(); - - //Need to remove them from rooms too - String username = usc.getName(); - if (username != null) { - ColourGenerator.freeUpColour(usc.getColour()); - - List rooms = getRooms(); - for (int i = 0; i < rooms.size(); i++) { - Room room = rooms.get(i); - room.removeFromObservers(username); - room.removePlayer(username, false); - } - - Debug.append(username + " has disconnected"); - } - - //Now remove the user connection. - ServerGlobals.INSTANCE.getUscStore().remove(usc.getIpAddress()); - if (ServerGlobals.INSTANCE.getUscStore().getAll().size() == 0 - && username != null) { - resetLobby(); - return; - } - - ServerGlobals.lobbyService.lobbyChanged(); - } - - private void resetLobby() { + public void resetLobby() { int countRemoved = 0; List rooms = getRooms(); @@ -178,10 +139,10 @@ private void resetLobby() { //Log out if we've actually removed some rooms if (countRemoved > 0) { - Debug.append("Removed " + countRemoved + " excess rooms"); + logger.info("roomsRemoved", "Removed " + countRemoved + " excess rooms"); } - Debug.append("Cleared lobby messages"); + logger.info("clearedMessages", "Cleared lobby messages"); lobbyMessages.clear(); } diff --git a/server/src/main/java/server/InactiveCheckRunnable.java b/server/src/main/java/server/InactiveCheckRunnable.java deleted file mode 100644 index dc58368..0000000 --- a/server/src/main/java/server/InactiveCheckRunnable.java +++ /dev/null @@ -1,86 +0,0 @@ -package server; - -import java.util.List; - -import auth.UserConnection; -import object.ServerRunnable; -import util.Debug; -import util.ServerGlobals; - -import static utils.CoreGlobals.logger; - -public class InactiveCheckRunnable implements ServerRunnable -{ - private static int SLEEP_TIME_MILLIS = 5 * 1000; //10 seconds - private static int KICK_OFF_TIME_MILLIS = 60 * 1000; //60 seconds - - private EntropyServer server; - private String statusText = ""; - - public InactiveCheckRunnable(EntropyServer server) - { - this.server = server; - } - - @Override - public void run() - { - while (true) - { - try - { - runInactiveCheck(); - } - catch (Throwable t) - { - Debug.stackTrace(t); - } - } - } - - private void runInactiveCheck() throws InterruptedException - { - List userConnections = ServerGlobals.INSTANCE.getUscStore().getAll(); - int size = userConnections.size(); - - statusText = "Running for " + size + " uscs"; - - for (int i=size-1; i>=0; i--) - { - UserConnection usc = userConnections.get(i); - String username = usc.getName(); - long lastActiveMillis = usc.getLastActive(); - long currentMillis = System.currentTimeMillis(); - long timeSinceLastActive = currentMillis - lastActiveMillis; - - if (timeSinceLastActive >= KICK_OFF_TIME_MILLIS) - { - server.removeFromUsersOnline(usc); - - if (username != null) - { - logger.info("removedInactiveUser", "Removed " + username + " due to inactivity."); - } - else - { - logger.info("removedInactiveUser", "Removed unused connection for IP " + usc.getIpAddress()); - } - } - } - - statusText = "Sleeping between checks"; - Thread.sleep(SLEEP_TIME_MILLIS); - } - - @Override - public String getDetails() - { - return statusText; - } - - @Override - public UserConnection getUserConnection() - { - return null; - } -} diff --git a/server/src/main/java/server/MessageHandlerRunnable.java b/server/src/main/java/server/MessageHandlerRunnable.java index b446c98..da6e54d 100644 --- a/server/src/main/java/server/MessageHandlerRunnable.java +++ b/server/src/main/java/server/MessageHandlerRunnable.java @@ -209,28 +209,10 @@ private Document getResponseForMessage() throws Throwable String id = root.getAttribute("RoomId"); String username = root.getAttribute("Username"); String name = root.getNodeName(); - - String usernameForThisConnection = usc.getName(); - if (usernameForThisConnection == null - || !usernameForThisConnection.equals(username)) - { - Debug.stackTrace("Failed username check for IP " + ipAddress + ": client passed up " + username - + " but connected as " + usernameForThisConnection); - Debug.appendWithoutDate("Message passed up: " + messageStr); - server.removeFromUsersOnline(usc); - - return XmlBuilderServer.getKickOffResponse(usernameForThisConnection, REMOVAL_REASON_FAILED_USERNAME_CHECK); - } - usc.setLastActiveNow(); - if (name.equals(ROOT_TAG_DISCONNECT_REQUEST)) - { - server.removeFromUsersOnline(usc); - return null; - } - else if (name.equals(ROOT_TAG_NEW_CHAT)) + if (name.equals(ROOT_TAG_NEW_CHAT)) { String newMessage = root.getAttribute("MessageText"); String colour = root.getAttribute("Colour"); diff --git a/server/src/main/kotlin/routes/session/SessionController.kt b/server/src/main/kotlin/routes/session/SessionController.kt index 64a5d17..f269c94 100644 --- a/server/src/main/kotlin/routes/session/SessionController.kt +++ b/server/src/main/kotlin/routes/session/SessionController.kt @@ -10,14 +10,13 @@ import io.ktor.server.request.* import io.ktor.server.response.* import io.ktor.server.routing.* import routes.requiresSession -import util.ServerGlobals +import util.ServerGlobals.sessionService class SessionController { - private val sessionService = SessionService(ServerGlobals.sessionStore, ServerGlobals.uscStore) - fun installRoutes(application: Application) { application.routing { post(Routes.BEGIN_SESSION) { beginSession(call) } + post(Routes.FINISH_SESSION) { finishSession(call) } post(Routes.ACHIEVEMENT_COUNT) { updateAchievementCount(call) } } } @@ -29,6 +28,12 @@ class SessionController { call.respond(response) } + private suspend fun finishSession(call: ApplicationCall) = + requiresSession(call) { session -> + sessionService.finishSession(session) + call.respond(HttpStatusCode.NoContent) + } + private suspend fun updateAchievementCount(call: ApplicationCall) = requiresSession(call) { session -> val request = call.receive() diff --git a/server/src/main/kotlin/routes/session/SessionService.kt b/server/src/main/kotlin/routes/session/SessionService.kt index 716f0ce..7118892 100644 --- a/server/src/main/kotlin/routes/session/SessionService.kt +++ b/server/src/main/kotlin/routes/session/SessionService.kt @@ -10,11 +10,13 @@ import http.dto.BeginSessionRequest import http.dto.BeginSessionResponse import io.ktor.http.* import java.util.* +import `object`.Room import routes.ClientException import store.Store import util.OnlineConstants import util.ServerGlobals import utils.Achievement +import utils.CoreGlobals.logger class SessionService( private val sessionStore: Store, @@ -72,6 +74,28 @@ class SessionService( else nameToCheck } + fun finishSession(session: Session) { + val usc = uscStore.get(session.ip) + usc.destroyNotificationSockets() + + val rooms: List = ServerGlobals.server.getRooms() + rooms.forEach { room -> + room.removeFromObservers(usc.name) + room.removePlayer(usc.name, false) + } + + uscStore.remove(session.ip) + sessionStore.remove(session.id) + + logger.info("finishSession", "Session ended for ${usc.name}") + + if (sessionStore.count() == 0) { + ServerGlobals.server.resetLobby() + } + + ServerGlobals.lobbyService.lobbyChanged() + } + fun updateAchievementCount(session: Session, achievementCount: Int) { validateAchievementCount(achievementCount) diff --git a/server/src/main/kotlin/store/Store.kt b/server/src/main/kotlin/store/Store.kt index 82d7f62..87f5fc7 100644 --- a/server/src/main/kotlin/store/Store.kt +++ b/server/src/main/kotlin/store/Store.kt @@ -15,6 +15,8 @@ interface Store> { fun putAll(vararg items: T) + fun count(): Int + fun update(key: K, updater: (T) -> T) { val current = get(key) put(updater(current)) @@ -41,4 +43,6 @@ abstract class MemoryStore> : Store { override fun putAll(vararg items: T) { items.forEach(::put) } + + override fun count() = map.size } diff --git a/server/src/main/kotlin/util/ServerGlobals.kt b/server/src/main/kotlin/util/ServerGlobals.kt index 04efe96..2925181 100644 --- a/server/src/main/kotlin/util/ServerGlobals.kt +++ b/server/src/main/kotlin/util/ServerGlobals.kt @@ -5,6 +5,7 @@ import java.util.* import java.util.concurrent.ArrayBlockingQueue import java.util.concurrent.TimeUnit import routes.lobby.LobbyService +import routes.session.SessionService import server.EntropyServer import store.MemoryUserConnectionStore import store.SessionStore @@ -32,5 +33,6 @@ object ServerGlobals { val server: EntropyServer = EntropyServer() - @JvmField val lobbyService: LobbyService = LobbyService(server, sessionStore, uscStore) + @JvmField var lobbyService: LobbyService = LobbyService(server, sessionStore, uscStore) + @JvmField var sessionService = SessionService(sessionStore, uscStore) } diff --git a/server/src/test/kotlin/routes/session/SessionControllerTest.kt b/server/src/test/kotlin/routes/session/SessionControllerTest.kt index 11eef3a..4261cda 100644 --- a/server/src/test/kotlin/routes/session/SessionControllerTest.kt +++ b/server/src/test/kotlin/routes/session/SessionControllerTest.kt @@ -14,7 +14,9 @@ import testCore.shouldMatchJson import util.ApplicationTest import util.OnlineConstants import util.ServerGlobals.sessionStore +import util.ServerGlobals.uscStore import util.makeSession +import util.makeUserConnection class SessionControllerTest : ApplicationTest() { @Test @@ -36,6 +38,27 @@ class SessionControllerTest : ApplicationTest() { .trimIndent() } + @Test + fun `Should reject a finish session call without a session`() = testApplication { + val response = client.post(Routes.FINISH_SESSION, ::buildFinishSessionRequest) + response.status shouldBe HttpStatusCode.Unauthorized + } + + @Test + fun `Should be able to finish a session`() = testApplication { + val session = makeSession(achievementCount = 4) + val usc = makeUserConnection(session) + sessionStore.put(session) + uscStore.put(usc) + + val response = + client.post(Routes.FINISH_SESSION) { buildFinishSessionRequest(this, session.id) } + response.status shouldBe HttpStatusCode.NoContent + + sessionStore.count() shouldBe 0 + uscStore.count() shouldBe 0 + } + @Test fun `Should reject an update achievement call without a session`() = testApplication { val response = client.post(Routes.ACHIEVEMENT_COUNT, ::buildAchievementUpdateRequest) @@ -57,6 +80,10 @@ class SessionControllerTest : ApplicationTest() { updatedSession.achievementCount shouldBe 8 } + private fun buildFinishSessionRequest(builder: HttpRequestBuilder, sessionId: UUID? = null) { + sessionId?.let { builder.header(CustomHeader.SESSION_ID, sessionId) } + } + private fun buildAchievementUpdateRequest( builder: HttpRequestBuilder, sessionId: UUID? = null, diff --git a/server/src/test/kotlin/routes/session/SessionServiceTest.kt b/server/src/test/kotlin/routes/session/SessionServiceTest.kt index aa306c1..884c514 100644 --- a/server/src/test/kotlin/routes/session/SessionServiceTest.kt +++ b/server/src/test/kotlin/routes/session/SessionServiceTest.kt @@ -7,6 +7,7 @@ import http.UPDATE_REQUIRED import http.dto.BeginSessionRequest import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.collections.shouldBeEmpty +import io.kotest.matchers.collections.shouldContainExactly import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder import io.kotest.matchers.shouldBe import io.ktor.http.* @@ -19,6 +20,7 @@ import testCore.AbstractTest import testCore.only import util.OnlineConstants import util.makeSession +import util.makeUserConnection import utils.Achievement class SessionServiceTest : AbstractTest() { @@ -151,6 +153,23 @@ class SessionServiceTest : AbstractTest() { store.get(session.id).achievementCount shouldBe session.achievementCount } + @Test + fun `Should support finishing a session`() { + val sessionA = makeSession(ip = "1.2.3.4") + val uscA = makeUserConnection(sessionA) + + val sessionB = makeSession(ip = "5.6.7.8") + val uscB = makeUserConnection(sessionB) + val (service, sessionStore, uscStore) = makeService() + sessionStore.putAll(sessionA, sessionB) + uscStore.putAll(uscA, uscB) + + service.finishSession(sessionA) + + sessionStore.getAll().shouldContainExactly(sessionB) + uscStore.getAll().shouldContainExactly(uscB) + } + @Test fun `Should support updating achievement count`() { val session = makeSession(achievementCount = 5) diff --git a/server/src/test/kotlin/store/AbstractStoreTest.kt b/server/src/test/kotlin/store/AbstractStoreTest.kt index c2821ee..572e0de 100644 --- a/server/src/test/kotlin/store/AbstractStoreTest.kt +++ b/server/src/test/kotlin/store/AbstractStoreTest.kt @@ -82,4 +82,17 @@ abstract class AbstractStoreTest> { store.find(makeIdA()) shouldBe makeItemA() store.find(makeIdB()) shouldBe null } + + @Test + fun `Should be able to count items`() { + val store = makeStore() + store.count() shouldBe 0 + + store.put(makeItemA()) + store.put(makeItemB()) + store.count() shouldBe 2 + + store.remove(makeIdA()) + store.count() shouldBe 1 + } } diff --git a/server/src/test/kotlin/util/ApplicationTest.kt b/server/src/test/kotlin/util/ApplicationTest.kt index dc0c99f..dc812ed 100644 --- a/server/src/test/kotlin/util/ApplicationTest.kt +++ b/server/src/test/kotlin/util/ApplicationTest.kt @@ -1,15 +1,21 @@ package util import org.junit.jupiter.api.BeforeEach +import routes.lobby.LobbyService +import routes.session.SessionService import store.MemoryUserConnectionStore import store.SessionStore import testCore.AbstractTest +import util.ServerGlobals.sessionStore +import util.ServerGlobals.uscStore /** Clear down stores between tests. Can be replaced with proper DI once legacy code is gone */ abstract class ApplicationTest : AbstractTest() { @BeforeEach fun beforeEach() { - ServerGlobals.sessionStore = SessionStore() - ServerGlobals.uscStore = MemoryUserConnectionStore() + sessionStore = SessionStore() + uscStore = MemoryUserConnectionStore() + ServerGlobals.lobbyService = LobbyService(ServerGlobals.server, sessionStore, uscStore) + ServerGlobals.sessionService = SessionService(sessionStore, uscStore) } } diff --git a/server/src/test/kotlin/util/TestFactory.kt b/server/src/test/kotlin/util/TestFactory.kt index 99d78f7..171f8d1 100644 --- a/server/src/test/kotlin/util/TestFactory.kt +++ b/server/src/test/kotlin/util/TestFactory.kt @@ -1,6 +1,7 @@ package util import auth.Session +import auth.UserConnection import game.GameMode import game.GameSettings import java.util.* @@ -10,9 +11,11 @@ fun makeSession( name: String = "Alyssa", ip: String = "1.2.3.4", achievementCount: Int = 4, - apiVersion: Int = OnlineConstants.API_VERSION + apiVersion: Int = OnlineConstants.API_VERSION, ) = Session(id, name, ip, achievementCount, apiVersion) +fun makeUserConnection(session: Session) = UserConnection(session.ip, session.name) + fun makeGameSettings( mode: GameMode = GameMode.Entropy, jokerQuantity: Int = 0, @@ -21,7 +24,7 @@ fun makeGameSettings( includeStars: Boolean = false, negativeJacks: Boolean = false, cardReveal: Boolean = false, - illegalAllowed: Boolean = true + illegalAllowed: Boolean = true, ) = GameSettings( mode, @@ -31,5 +34,5 @@ fun makeGameSettings( includeStars, negativeJacks, cardReveal, - illegalAllowed + illegalAllowed, )