Skip to content

Commit 4bf4325

Browse files
author
Oleksandr Dzhychko
committed
fix(model-server): respond with Problem JSON and reasonable status code for selected endpoints
This commit is only concerned with cleaning-up error responses for: * POST "/v2/repositories/{repository}/objects/getAll" * PUT "/v2/repositories/{repository}/objects"
1 parent eb38620 commit 4bf4325

File tree

4 files changed

+259
-34
lines changed

4 files changed

+259
-34
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: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ class ModelReplicationServer(
154154
val repositoryId = try {
155155
RepositoryId(repository)
156156
} catch (e: IllegalArgumentException) {
157-
throw BadRequestException("Invalid repository name", "invalid-request", cause = e)
157+
throw InvalidRepositoryIdException(repository, e)
158158
}
159159

160160
checkPermission(ModelServerPermissionSchema.repository(repositoryId).branch(branch).delete)
@@ -254,9 +254,7 @@ class ModelReplicationServer(
254254
}
255255

256256
for (entry in objects) {
257-
if (entry.value == null) {
258-
throw IllegalStateException("Object not found: ${entry.value}")
259-
}
257+
if (entry.value == null) { throw ObjectValueNotFoundException(entry.key) }
260258
}
261259
@Suppress("UNCHECKED_CAST")
262260
objects as Map<String, String>
@@ -356,21 +354,15 @@ class ModelReplicationServer(
356354

357355
while (true) {
358356
val key = channel.readUTF8Line() ?: break
359-
val value = channel.readUTF8Line()!!
360357

361358
if (!HashUtil.isSha256(key)) {
362-
throw IllegalStateException(
363-
"This API cannot be used to store other entries than serialized objects." +
364-
" The key is expected to be a SHA256 hash over the value: $key -> $value",
365-
)
359+
throw InvalidObjectKeyException(key)
366360
}
367361

362+
val value = channel.readUTF8Line() ?: throw ObjectKeyWithoutObjectValueException(key)
363+
368364
val expectedKey = HashUtil.sha256(value)
369-
if (expectedKey != key) {
370-
throw IllegalStateException(
371-
"Hash mismatch. Expected $expectedKey, but $key was provided. Value: $value",
372-
)
373-
}
365+
if (expectedKey != key) { throw MismatchingObjectKeyAndValueException(key, expectedKey, value) }
374366
entries[key] = value
375367
}
376368

‎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+
}

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

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import io.ktor.client.request.accept
2929
import io.ktor.client.request.delete
3030
import io.ktor.client.request.get
3131
import io.ktor.client.request.post
32+
import io.ktor.client.request.put
33+
import io.ktor.client.request.setBody
3234
import io.ktor.client.statement.bodyAsText
3335
import io.ktor.http.ContentType
3436
import io.ktor.http.HttpHeaders
@@ -182,13 +184,17 @@ class ModelReplicationServerTest {
182184
@Test
183185
fun `responds with 400 when deleting from an invalid repository ID`() {
184186
runWithTestModelServer { _, _ ->
187+
val client = createClient { install(ContentNegotiation) { json() } }
188+
185189
val response = client.delete {
186190
url {
187191
appendPathSegments("v2", "repositories", "invalid with spaces", "branches", "master")
188192
}
189193
}
190-
assertEquals(HttpStatusCode.BadRequest, response.status)
191-
assertContains(response.bodyAsText(), "Invalid repository name")
194+
val problem = response.body<Problem>()
195+
196+
response shouldHaveStatus HttpStatusCode.BadRequest
197+
problem.type shouldBe "/problems/invalid-repository-id"
192198
}
193199
}
194200

@@ -370,7 +376,7 @@ class ModelReplicationServerTest {
370376
),
371377
) { _, _ ->
372378
val client = createClient {
373-
install(io.ktor.client.plugins.contentnegotiation.ContentNegotiation) { json() }
379+
install(ContentNegotiation) { json() }
374380
}
375381
val response = client.get {
376382
url {
@@ -440,4 +446,92 @@ class ModelReplicationServerTest {
440446
)
441447
}
442448
}
449+
450+
@Test
451+
fun `putRepositoryObjects detects missing values`() = runWithTestModelServer { _, _ ->
452+
val client = createClient { install(ContentNegotiation) { json() } }
453+
val modelClient = ModelClientV2.builder().url("v2").client(client).build().also { it.init() }
454+
val repositoryId = RepositoryId("repo1")
455+
modelClient.initRepository(repositoryId)
456+
val invalidRequestBody = "7-m7S*flQz_CNc9F3ipSQ5Iz7SZJSyn2r0_1PPVvh8yA"
457+
458+
val response = client.put {
459+
url {
460+
takeFrom("v2")
461+
appendPathSegments("repositories", repositoryId.id, "objects")
462+
}
463+
contentType(ContentType.Text.Plain)
464+
setBody(invalidRequestBody)
465+
}
466+
val problem = response.body<Problem>()
467+
468+
response shouldHaveStatus HttpStatusCode.BadRequest
469+
problem.type shouldBe "/problems/object-key-without-object-value"
470+
}
471+
472+
@Test
473+
fun `putRepositoryObjects reports invalid key`() = runWithTestModelServer { _, _ ->
474+
val client = createClient { install(ContentNegotiation) { json() } }
475+
val modelClient = ModelClientV2.builder().url("v2").client(client).build().also { it.init() }
476+
val repositoryId = RepositoryId("repo1")
477+
modelClient.initRepository(repositoryId)
478+
val invalidRequestBody = "not-a-valid-key"
479+
480+
val response = client.put {
481+
url {
482+
takeFrom("v2")
483+
appendPathSegments("repositories", repositoryId.id, "objects")
484+
}
485+
contentType(ContentType.Text.Plain)
486+
setBody(invalidRequestBody)
487+
}
488+
val problem = response.body<Problem>()
489+
490+
response shouldHaveStatus HttpStatusCode.BadRequest
491+
problem.type shouldBe "/problems/invalid-object-key"
492+
}
493+
494+
@Test
495+
fun `putRepositoryObjects detects mismatch between object key and object value`() = runWithTestModelServer { _, _ ->
496+
val client = createClient { install(ContentNegotiation) { json() } }
497+
val modelClient = ModelClientV2.builder().url("v2").client(client).build().also { it.init() }
498+
val repositoryId = RepositoryId("repo1")
499+
modelClient.initRepository(repositoryId)
500+
val invalidRequestBody = "7-m7S*flQz_CNc9F3ipSQ5Iz7SZJSyn2r0_1PPVvh8yA\ninvalidObjectValue"
501+
502+
val response = client.put {
503+
url {
504+
takeFrom("v2")
505+
appendPathSegments("repositories", repositoryId.id, "objects")
506+
}
507+
contentType(ContentType.Text.Plain)
508+
setBody(invalidRequestBody)
509+
}
510+
val problem = response.body<Problem>()
511+
512+
response shouldHaveStatus HttpStatusCode.BadRequest
513+
problem.type shouldBe "/problems/mismatching-object-and-value"
514+
}
515+
516+
@Test
517+
fun `postRepositoryObjectsGetAll fails for missing object value value`() = runWithTestModelServer { _, _ ->
518+
val client = createClient { install(ContentNegotiation) { json() } }
519+
val modelClient = ModelClientV2.builder().url("v2").client(client).build().also { it.init() }
520+
val repositoryId = RepositoryId("repo1")
521+
modelClient.initRepository(repositoryId)
522+
val invalidRequestBody = "7-m7S*flQz_CNc9F3ipSQ5Iz7SZJSyn2r0_1PPVvh8yA"
523+
524+
val response = client.post {
525+
url {
526+
takeFrom("v2")
527+
appendPathSegments("repositories", repositoryId.id, "objects", "getAll")
528+
}
529+
contentType(ContentType.Text.Plain)
530+
setBody(invalidRequestBody)
531+
}
532+
val problem = response.body<Problem>()
533+
534+
response shouldHaveStatus HttpStatusCode.NotFound
535+
problem.type shouldBe "/problems/object-value-not-found"
536+
}
443537
}

0 commit comments

Comments
 (0)