mirror of
https://github.com/Darkrock-Studios/hammer-editor.git
synced 2026-10-03 12:16:51 +00:00
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.
This commit is contained in:
parent
375d4553c6
commit
ca909f4a6b
5 changed files with 162 additions and 11 deletions
|
|
@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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")
|
||||
|
|
|
|||
|
|
@ -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<GlobalSettingsStore>("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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<ProtocolMismatchRepository>()
|
||||
single { create(::createHttpClient) } bind HttpClient::class
|
||||
single { createHttpClient(get(), get(NetworkJsonQualifier)) } bind HttpClient::class
|
||||
single<ServerAccountApi>()
|
||||
single<ServerProjectApi>()
|
||||
single<ServerProjectApi> {
|
||||
ServerProjectApi(
|
||||
httpClient = get(),
|
||||
globalSettingsStore = get(),
|
||||
json = get(NetworkJsonQualifier),
|
||||
strRes = get(),
|
||||
)
|
||||
}
|
||||
single<ServerProjectsApi>()
|
||||
single<WritingActivityApi>()
|
||||
single<ProjectDataApi>()
|
||||
single<ServerIdeasApi>()
|
||||
single<ProjectDataApi> {
|
||||
ProjectDataApi(
|
||||
httpClient = get(),
|
||||
globalSettingsStore = get(),
|
||||
json = get(NetworkJsonQualifier),
|
||||
strRes = get(),
|
||||
)
|
||||
}
|
||||
single<ServerIdeasApi> {
|
||||
ServerIdeasApi(
|
||||
httpClient = get(),
|
||||
globalSettingsStore = get(),
|
||||
json = get(NetworkJsonQualifier),
|
||||
strRes = get(),
|
||||
)
|
||||
}
|
||||
single<ServerAdminApi>()
|
||||
|
||||
single<ServerSettingsFilesystemDatasource>() 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<GithubVersionCheckDataSource>() bind VersionCheckDataSource::class
|
||||
single<GithubVersionCheckDataSource> {
|
||||
GithubVersionCheckDataSource(http = get(), json = get(NetworkJsonQualifier))
|
||||
} bind VersionCheckDataSource::class
|
||||
single<VersionCheckRepository>()
|
||||
|
||||
single<ResourceChangelogDatasource>() 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<ClientIdeasSynchronizer>()
|
||||
single<ClientAccountSynchronizer>()
|
||||
|
|
|
|||
|
|
@ -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<ProjectDataDto>(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)
|
||||
}
|
||||
}
|
||||
|
|
@ -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())
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue