Skip to content

Commit c3a9d55

Browse files
author
Oleksandr Dzhychko
authored
Merge pull request #966 from modelix/fix/MODELIX-998
fix(model-server): do not use blocking network calls
2 parents 5ee00d2 + 4bf4325 commit c3a9d55

File tree

4 files changed

+292
-51
lines changed

4 files changed

+292
-51
lines changed

‎model-server/src/main/kotlin/org/modelix/model/server/handlers/HttpException.kt‎

Lines changed: 77 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,22 +65,6 @@ open class HttpException(problem: Problem, cause: Throwable? = null) : RuntimeEx
6565
)
6666
}
6767

68-
/**
69-
* Indicates a bad request from the client with missing or invalid information provided.
70-
*
71-
* @param details the detailed message to expose to the caller
72-
* @param typeSuffix A detailed type making this bad request instance uniquely identifiable. [PROBLEM_NAMESPACE] will
73-
* automatically be prepended. This field should be written in kebab-case.
74-
* @param cause The causing exception for the bad request or null if none.
75-
*/
76-
class BadRequestException(details: String, typeSuffix: String, cause: Throwable? = null) : HttpException(
77-
HttpStatusCode.BadRequest,
78-
title = "Bad request",
79-
details = details,
80-
type = problemType(typeSuffix),
81-
cause = cause,
82-
)
83-
8468
/**
8569
* A [HttpException] indicating that a branch was not found in a repository.
8670
*
@@ -105,7 +89,7 @@ class BranchNotFoundException(branch: String, repositoryId: String, cause: Throw
10589
/**
10690
* A [HttpException] indicating that a version inside a repository or branch was not found.
10791
*
108-
* @param versionHash has of the missing version
92+
* @param versionHash hash of the missing version
10993
* @param cause The causing exception for the bad request or null if none.
11094
*/
11195
class VersionNotFoundException(versionHash: String, cause: Throwable? = null) :
@@ -116,3 +100,79 @@ class VersionNotFoundException(versionHash: String, cause: Throwable? = null) :
116100
type = "/problems/version-not-found",
117101
cause = cause,
118102
)
103+
104+
/**
105+
* An [HttpException] indicating that a provided repository name is not valid.
106+
*
107+
* @param invalidRepositoryId the invalid repository ID
108+
* @param cause The causing exception for the bad request or null if none.
109+
*/
110+
class InvalidRepositoryIdException(invalidRepositoryId: String, cause: Throwable? = null) :
111+
HttpException(
112+
HttpStatusCode.BadRequest,
113+
title = "Invalid repository ID.",
114+
details = "Repository ID `$invalidRepositoryId` is not valid.",
115+
type = "/problems/invalid-repository-id",
116+
cause = cause,
117+
)
118+
119+
/**
120+
* An [HttpException] indicating that an object key was uploaded without the corresponding value.
121+
*
122+
* @param objectKey the uploaded object key
123+
*/
124+
class ObjectKeyWithoutObjectValueException(objectKey: String) :
125+
HttpException(
126+
HttpStatusCode.BadRequest,
127+
title = "Uploaded object key without object value.",
128+
details = "Uploaded object key `$objectKey` without object value.",
129+
type = "/problems/object-key-without-object-value",
130+
)
131+
132+
/**
133+
* An [HttpException] indicating that an invalid object key was uploaded.
134+
*
135+
* @param objectKey the uploaded object key
136+
*/
137+
class InvalidObjectKeyException(objectKey: String) :
138+
HttpException(
139+
HttpStatusCode.BadRequest,
140+
title = "Uploaded invalid object key.",
141+
details = "Uploaded invalid object key `$objectKey`.",
142+
type = "/problems/invalid-object-key",
143+
)
144+
145+
/**
146+
* An [HttpException] indicating that an object key and value were uploaded that do not match.
147+
*
148+
* @param uploadedObjectKey the uploaded object key
149+
* @param expectedObjectKey the expected object key based on the object value
150+
* @param objectValue the uploaded object value
151+
*/
152+
class MismatchingObjectKeyAndValueException(
153+
uploadedObjectKey: String,
154+
expectedObjectKey: String,
155+
objectValue: String,
156+
) :
157+
HttpException(
158+
HttpStatusCode.BadRequest,
159+
title = "Uploaded mismatching object key and value.",
160+
details = "Uploaded object key `$uploadedObjectKey` does not match expected object key `$expectedObjectKey` " +
161+
"for object value `$objectValue`.",
162+
type = "/problems/mismatching-object-and-value",
163+
)
164+
165+
/**
166+
* A [HttpException] indicating that an object value was not found.
167+
*
168+
* @param objectHash hash of the missing object
169+
* @param cause The causing exception for the bad request or null if none.
170+
*/
171+
class ObjectValueNotFoundException(objectHash: String, cause: Throwable? = null) :
172+
HttpException(
173+
HttpStatusCode.NotFound,
174+
title = "Object value not found.",
175+
details = "Object value with hash `$objectHash` does not exist.",
176+
type = "/problems/object-value-not-found",
177+
cause = cause,
178+
)

‎model-server/src/main/kotlin/org/modelix/model/server/handlers/ModelReplicationServer.kt‎

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import io.ktor.server.application.ApplicationCall
2121
import io.ktor.server.application.call
2222
import io.ktor.server.request.acceptItems
2323
import io.ktor.server.request.receive
24-
import io.ktor.server.request.receiveStream
24+
import io.ktor.server.request.receiveChannel
2525
import io.ktor.server.response.respond
2626
import io.ktor.server.response.respondBytesWriter
2727
import io.ktor.server.response.respondText
@@ -31,6 +31,7 @@ import io.ktor.util.cio.use
3131
import io.ktor.util.pipeline.PipelineContext
3232
import io.ktor.utils.io.ByteWriteChannel
3333
import io.ktor.utils.io.close
34+
import io.ktor.utils.io.readUTF8Line
3435
import io.ktor.utils.io.writeStringUtf8
3536
import kotlinx.coroutines.Dispatchers
3637
import kotlinx.coroutines.flow.Flow
@@ -153,7 +154,7 @@ class ModelReplicationServer(
153154
val repositoryId = try {
154155
RepositoryId(repository)
155156
} catch (e: IllegalArgumentException) {
156-
throw BadRequestException("Invalid repository name", "invalid-request", cause = e)
157+
throw InvalidRepositoryIdException(repository, e)
157158
}
158159

159160
checkPermission(ModelServerPermissionSchema.repository(repositoryId).branch(branch).delete)
@@ -239,17 +240,27 @@ class ModelReplicationServer(
239240

240241
override suspend fun PipelineContext<Unit, ApplicationCall>.postRepositoryObjectsGetAll(repository: String) {
241242
checkPermission(ModelServerPermissionSchema.repository(repository).objects.read)
242-
runWithRepository(repository) {
243-
val keys = call.receiveStream().bufferedReader().use { reader ->
244-
reader.lineSequence().toHashSet()
245-
}
246-
val objects = withContext(Dispatchers.IO) { modelClient.store.getAll(keys) }.checkValuesNotNull {
247-
"Object not found: $it"
248-
}
249-
call.respondTextWriter(contentType = ImmutableObjectsStream.CONTENT_TYPE) {
250-
ImmutableObjectsStream.encode(this, objects)
243+
val channel = call.receiveChannel()
244+
val keys = hashSetOf<String>()
245+
while (true) {
246+
val line = channel.readUTF8Line() ?: break
247+
keys.add(line)
248+
}
249+
250+
val objects = runWithRepository(repository) {
251+
withContext(Dispatchers.IO) {
252+
modelClient.store.getAll(keys)
251253
}
252254
}
255+
256+
for (entry in objects) {
257+
if (entry.value == null) { throw ObjectValueNotFoundException(entry.key) }
258+
}
259+
@Suppress("UNCHECKED_CAST")
260+
objects as Map<String, String>
261+
call.respondTextWriter(contentType = ImmutableObjectsStream.CONTENT_TYPE) {
262+
ImmutableObjectsStream.encode(this, objects)
263+
}
253264
}
254265

255266
override suspend fun PipelineContext<Unit, ApplicationCall>.pollRepositoryBranchHash(
@@ -336,30 +347,27 @@ class ModelReplicationServer(
336347

337348
override suspend fun PipelineContext<Unit, ApplicationCall>.putRepositoryObjects(repository: String) {
338349
checkPermission(ModelServerPermissionSchema.repository(parameter("repository")).objects.add)
339-
runWithRepository(repository) {
340-
val writtenEntries = withContext(Dispatchers.IO) {
341-
val entries = call.receiveStream().bufferedReader().use { reader ->
342-
reader.lineSequence().windowed(2, 2).map {
343-
val key = it[0]
344-
val value = it[1]
345-
346-
require(HashUtil.isSha256(key)) {
347-
"This API cannot be used to store other entries than serialized objects." +
348-
" The key is expected to be a SHA256 hash over the value: $key -> $value"
349-
}
350-
val expectedKey = HashUtil.sha256(value)
351-
require(expectedKey == key) { "Hash mismatch. Expected $expectedKey, but $key was provided. Value: $value" }
352-
353-
key to value
354-
}.toMap()
355-
}
356350

357-
storeClient.putAll(entries, true)
351+
val channel = call.receiveChannel()
352+
// Hash map can be used. Server does not expect entries in any order.
353+
val entries = hashMapOf<String, String>()
354+
355+
while (true) {
356+
val key = channel.readUTF8Line() ?: break
358357

359-
entries.size
358+
if (!HashUtil.isSha256(key)) {
359+
throw InvalidObjectKeyException(key)
360360
}
361-
call.respondText("$writtenEntries objects received")
361+
362+
val value = channel.readUTF8Line() ?: throw ObjectKeyWithoutObjectValueException(key)
363+
364+
val expectedKey = HashUtil.sha256(value)
365+
if (expectedKey != key) { throw MismatchingObjectKeyAndValueException(key, expectedKey, value) }
366+
entries[key] = value
362367
}
368+
369+
runWithRepository(repository) { withContext(Dispatchers.IO) { storeClient.putAll(entries, true) } }
370+
call.respondText("${entries.size} objects received")
363371
}
364372

365373
@Deprecated("deprecated flag is set in the OpenAPI specification")

‎model-server/src/test/kotlin/org/modelix/model/server/handlers/HttpExceptionTest.kt‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package org.modelix.model.server.handlers
2+
3+
import io.kotest.matchers.shouldBe
4+
import io.ktor.http.HttpStatusCode
5+
import kotlin.test.Test
6+
7+
class HttpExceptionTest {
8+
9+
@Test
10+
fun `test problem of InvalidRepositoryIdException`() {
11+
val problem = InvalidRepositoryIdException("invalid ID").problem
12+
13+
val expectedProblem = Problem(
14+
type = "/problems/invalid-repository-id",
15+
title = "Invalid repository ID.",
16+
status = HttpStatusCode.BadRequest.value,
17+
detail = "Repository ID `invalid ID` is not valid.",
18+
)
19+
problem shouldBe expectedProblem
20+
}
21+
22+
@Test
23+
fun `test problem of ObjectKeyWithoutObjectValueException`() {
24+
val problem = ObjectKeyWithoutObjectValueException("someKey").problem
25+
26+
val expectedProblem = Problem(
27+
type = "/problems/object-key-without-object-value",
28+
title = "Uploaded object key without object value.",
29+
status = HttpStatusCode.BadRequest.value,
30+
detail = "Uploaded object key `someKey` without object value.",
31+
)
32+
problem shouldBe expectedProblem
33+
}
34+
35+
@Test
36+
fun `test problem of InvalidObjectKeyException`() {
37+
val problem = InvalidObjectKeyException("someKey").problem
38+
39+
val expectedProblem = Problem(
40+
type = "/problems/invalid-object-key",
41+
title = "Uploaded invalid object key.",
42+
status = HttpStatusCode.BadRequest.value,
43+
detail = "Uploaded invalid object key `someKey`.",
44+
)
45+
problem shouldBe expectedProblem
46+
}
47+
48+
@Test
49+
fun `test problem of MismatchingObjectKeyAndValueException`() {
50+
val problem = MismatchingObjectKeyAndValueException(
51+
"wrongObjectKey",
52+
"correctObjectKey",
53+
"objectValue",
54+
).problem
55+
56+
val expectedDetailMsg = "Uploaded object key `wrongObjectKey` does not match " +
57+
"expected object key `correctObjectKey` for object value `objectValue`."
58+
val expectedProblem = Problem(
59+
type = "/problems/mismatching-object-and-value",
60+
title = "Uploaded mismatching object key and value.",
61+
status = HttpStatusCode.BadRequest.value,
62+
detail = expectedDetailMsg,
63+
)
64+
problem shouldBe expectedProblem
65+
}
66+
67+
@Test
68+
fun `test problem of ObjectValueNotFoundException`() {
69+
val problem = ObjectValueNotFoundException("objectValue").problem
70+
71+
val expectedProblem = Problem(
72+
type = "/problems/object-value-not-found",
73+
title = "Object value not found.",
74+
status = HttpStatusCode.NotFound.value,
75+
detail = "Object value with hash `objectValue` does not exist.",
76+
)
77+
problem shouldBe expectedProblem
78+
}
79+
}

0 commit comments

Comments
 (0)