From ca909f4a6bde55f76a0469f24293bb261dfd6b03 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 14 Sep 2026 15:33:32 -0700 Subject: [PATCH] Add a shared network JSON serializer that tolerates unknown keys (#934) `SYNCING-PROTOCOL.md` makes adding a field to a synced model a client-only change: the server stores content as an opaque blob, and a peer that predates the field is expected to drop it on decode. That only holds if the decode ignores unknown keys. Every `ContentNegotiation` install used a bare `json()`, which is Ktor's `DefaultJson`: `encodeDefaults` and `isLenient`, but no `ignoreUnknownKeys`. So instead of dropping the field, an older peer failed the whole sync. A 3.6.0 client hitting project data written by 3.9 got: Sync failed: Illegal input: Unexpected JSON token at offset 72: Encountered an unknown key 'language' at path: $.data `createJsonSerializer` already sets `ignoreUnknownKeys`, but it is meant for human-facing files: `prettyPrint` would put tabs and newlines in every request body, and `coerceInputValues` would silently coerce bad values on the wire. Split the two: files keep that one, machine-facing content gets `createNetworkJsonSerializer`, resolved through `NetworkJsonQualifier`. Applies to the three `ContentNegotiation` installs and the four classes that decode wire content with an injected `Json`: `ServerProjectApi` (entity payloads), `ProjectDataApi` and `ServerIdeasApi` (conflict bodies), and `GithubVersionCheckDataSource`. The file-facing injections are unchanged. This cannot reach already-shipped builds; 3.6 through 3.9.x keep failing against projects a newer client has touched. --- .../apps/hammer/base/http/JsonSerializer.kt | 22 +++- .../hammer/common/dependencyinjection/Http.kt | 10 +- .../common/dependencyinjection/mainModule.kt | 36 ++++++- .../server/NetworkJsonForwardCompatTest.kt | 102 ++++++++++++++++++ .../apps/hammer/plugins/Serialization.kt | 3 +- 5 files changed, 162 insertions(+), 11 deletions(-) create mode 100644 common/src/desktopTest/kotlin/server/NetworkJsonForwardCompatTest.kt diff --git a/base/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/base/http/JsonSerializer.kt b/base/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/base/http/JsonSerializer.kt index 82c24c1d2..a3125b868 100644 --- a/base/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/base/http/JsonSerializer.kt +++ b/base/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/base/http/JsonSerializer.kt @@ -2,7 +2,9 @@ package com.darkrockstudios.apps.hammer.base.http import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.json.Json +import org.koin.core.qualifier.named +/** For human-facing files on disk: indented, and forgiving of hand edits. */ @OptIn(ExperimentalSerializationApi::class) fun createJsonSerializer(): Json { return Json { @@ -13,4 +15,22 @@ fun createJsonSerializer(): Json { allowTrailingComma = true ignoreUnknownKeys = true } -} \ No newline at end of file +} + +/** + * For machine-facing content on the wire. Matches Ktor's `DefaultJson` plus `ignoreUnknownKeys`, + * which the sync protocol depends on: synced models gain fields without a protocol bump, so a peer + * must drop fields a newer build wrote rather than failing the decode. See `SYNCING-PROTOCOL.md`. + */ +fun createNetworkJsonSerializer(): Json { + return Json { + encodeDefaults = true + isLenient = true + allowSpecialFloatingPointValues = true + allowStructuredMapKeys = true + ignoreUnknownKeys = true + } +} + +/** Resolves [createNetworkJsonSerializer]; an unqualified `Json` is the file serializer. */ +val NetworkJsonQualifier = named("networkJson") diff --git a/common/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/common/dependencyinjection/Http.kt b/common/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/common/dependencyinjection/Http.kt index 69355ff95..8fb883232 100644 --- a/common/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/common/dependencyinjection/Http.kt +++ b/common/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/common/dependencyinjection/Http.kt @@ -18,14 +18,16 @@ import io.ktor.client.request.forms.* import io.ktor.http.* import io.ktor.serialization.kotlinx.json.* import io.ktor.util.* +import kotlinx.serialization.json.Json import okio.IOException private val GlobalSettingsKey = AttributeKey("GlobalSettings") fun createHttpClient( globalSettingsStore: GlobalSettingsStore, + networkJson: Json, ): HttpClient { - val tokenRefreshClient = createRefreshClient() + val tokenRefreshClient = createRefreshClient(networkJson) val client = HttpClient(getHttpPlatformEngine()) { install(Logging) { @@ -41,7 +43,7 @@ fun createHttpClient( } install(ContentNegotiation) { - json() + json(networkJson) } installCompression() @@ -82,7 +84,7 @@ private fun loadTokens(globalSettingsStore: GlobalSettingsStore): BearerTokens? } } -private fun createRefreshClient(): HttpClient { +private fun createRefreshClient(networkJson: Json): HttpClient { return HttpClient(getHttpPlatformEngine()) { install(Logging) { logger = NapierHttpLogger() @@ -90,7 +92,7 @@ private fun createRefreshClient(): HttpClient { } install(ContentNegotiation) { - json() + json(networkJson) } } } diff --git a/common/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/common/dependencyinjection/mainModule.kt b/common/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/common/dependencyinjection/mainModule.kt index bbc289c09..6f99b37b4 100644 --- a/common/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/common/dependencyinjection/mainModule.kt +++ b/common/src/commonMain/kotlin/com/darkrockstudios/apps/hammer/common/dependencyinjection/mainModule.kt @@ -1,7 +1,9 @@ package com.darkrockstudios.apps.hammer.common.dependencyinjection import com.darkrockstudios.apps.hammer.base.di.dispatcherModule +import com.darkrockstudios.apps.hammer.base.http.NetworkJsonQualifier import com.darkrockstudios.apps.hammer.base.http.createJsonSerializer +import com.darkrockstudios.apps.hammer.base.http.createNetworkJsonSerializer import com.darkrockstudios.apps.hammer.common.components.projecthome.ExportStoryUseCase import com.darkrockstudios.apps.hammer.common.components.projecthome.ImportStoryUseCase import com.darkrockstudios.apps.hammer.common.data.ProjectDef @@ -173,13 +175,34 @@ val mainModule = module { includes(platformModule) single() - single { create(::createHttpClient) } bind HttpClient::class + single { createHttpClient(get(), get(NetworkJsonQualifier)) } bind HttpClient::class single() - single() + single { + ServerProjectApi( + httpClient = get(), + globalSettingsStore = get(), + json = get(NetworkJsonQualifier), + strRes = get(), + ) + } single() single() - single() - single() + single { + ProjectDataApi( + httpClient = get(), + globalSettingsStore = get(), + json = get(NetworkJsonQualifier), + strRes = get(), + ) + } + single { + ServerIdeasApi( + httpClient = get(), + globalSettingsStore = get(), + json = get(NetworkJsonQualifier), + strRes = get(), + ) + } single() single() bind ServerSettingsDatasource::class @@ -188,7 +211,9 @@ val mainModule = module { // Only the protocol mismatch dialog checks GitHub, and only once the user has already // connected to a sync server. Nothing on the app-load path may use this. - single() bind VersionCheckDataSource::class + single { + GithubVersionCheckDataSource(http = get(), json = get(NetworkJsonQualifier)) + } bind VersionCheckDataSource::class single() single() bind ChangelogDatasource::class @@ -218,6 +243,7 @@ val mainModule = module { single { create(::createTomlSerializer) } bind Toml::class single { create(::createJsonSerializer) } bind Json::class + single(NetworkJsonQualifier) { createNetworkJsonSerializer() } single() single() diff --git a/common/src/desktopTest/kotlin/server/NetworkJsonForwardCompatTest.kt b/common/src/desktopTest/kotlin/server/NetworkJsonForwardCompatTest.kt new file mode 100644 index 000000000..40c25d01c --- /dev/null +++ b/common/src/desktopTest/kotlin/server/NetworkJsonForwardCompatTest.kt @@ -0,0 +1,102 @@ +package server + +import com.darkrockstudios.apps.hammer.base.ProjectId +import com.darkrockstudios.apps.hammer.base.http.createNetworkJsonSerializer +import com.darkrockstudios.apps.hammer.base.http.projectdata.ProjectDataDto +import com.darkrockstudios.apps.hammer.common.data.globalsettings.GlobalSettingsStore +import com.darkrockstudios.apps.hammer.common.data.globalsettings.ServerSettings +import com.darkrockstudios.apps.hammer.common.server.ProjectDataApi +import com.darkrockstudios.apps.hammer.common.util.DeviceLocaleResolver +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respond +import io.ktor.client.plugins.contentnegotiation.ContentNegotiation +import io.ktor.http.ContentType +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import io.ktor.http.headersOf +import io.ktor.serialization.kotlinx.json.json +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.BeforeEach +import org.koin.dsl.module +import utils.BaseTest +import utils.TestStrRes +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Synced models gain fields without a protocol bump, so a client has to drop fields a newer build + * wrote rather than failing the decode. See "Server storage is shape-agnostic" in + * `SYNCING-PROTOCOL.md`. + */ +class NetworkJsonForwardCompatTest : BaseTest() { + + private val userId = 42L + private val json = createNetworkJsonSerializer() + private lateinit var globalSettingsStore: GlobalSettingsStore + + /** A payload written by a build that knows fields this one does not. */ + private val futurePayload = """ + {"data":{"authorName":"Ada","theme":null,"wordCountGoal":null,"tags":[],"language":"nb", + "encyclopediaDictionary":true,"unknownFutureField":"whatever"},"hash":"CJjmUTfFWaIl8YePLhBLSQ"} + """.trimIndent().replace("\n", "") + + @BeforeEach + override fun setup() { + super.setup() + + globalSettingsStore = mockk(relaxed = true) + every { globalSettingsStore.serverSettings } returns ServerSettings( + ssl = false, + url = "example.com", + email = "user@example.com", + userId = userId, + bearerToken = "token", + refreshToken = "refresh", + ) + + setupKoin( + module { + single { DeviceLocaleResolver() } + } + ) + } + + @Test + fun `the network serializer drops fields it does not know`() { + val dto = json.decodeFromString(futurePayload) + + assertEquals("Ada", dto.data.authorName) + assertEquals("CJjmUTfFWaIl8YePLhBLSQ", dto.hash) + } + + @Test + fun `downloading project data written by a newer build succeeds`() = runTest { + val engine = MockEngine { + respond( + content = futurePayload, + status = HttpStatusCode.OK, + headers = headersOf(HttpHeaders.ContentType, ContentType.Application.Json.toString()), + ) + } + val client = HttpClient(engine) { + install(ContentNegotiation) { json(json) } + } + val api = ProjectDataApi(client, globalSettingsStore, json, TestStrRes()) + + val result = api.getProjectData(userId, ProjectId("project-uuid")) + + assertTrue(result.isSuccess) + assertEquals("Ada", result.getOrThrow()?.data?.authorName) + } + + /** prettyPrint would put tabs and newlines in every request body. */ + @Test + fun `the network serializer does not pretty print`() { + assertFalse(json.configuration.prettyPrint) + } +} diff --git a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/plugins/Serialization.kt b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/plugins/Serialization.kt index c08286a28..59dce8f6d 100644 --- a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/plugins/Serialization.kt +++ b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/plugins/Serialization.kt @@ -1,11 +1,12 @@ package com.darkrockstudios.apps.hammer.plugins +import com.darkrockstudios.apps.hammer.base.http.createNetworkJsonSerializer import io.ktor.serialization.kotlinx.json.* import io.ktor.server.application.* import io.ktor.server.plugins.contentnegotiation.* fun Application.configureSerialization() { install(ContentNegotiation) { - json() + json(createNetworkJsonSerializer()) } }