diff --git a/src/main/kotlin/fr/dcproject/application/Converters.kt b/src/main/kotlin/fr/dcproject/application/Converters.kt index a88ba3b..3226ffd 100644 --- a/src/main/kotlin/fr/dcproject/application/Converters.kt +++ b/src/main/kotlin/fr/dcproject/application/Converters.kt @@ -1,6 +1,10 @@ package fr.dcproject.application +import fr.dcproject.application.http.BadRequestException +import fr.dcproject.application.http.HttpErrorBadRequest +import fr.dcproject.application.http.HttpErrorBadRequest.InvalidParam import io.ktor.features.DataConversion +import io.ktor.http.HttpStatusCode import io.ktor.util.KtorExperimentalAPI import org.koin.core.context.GlobalContext import org.koin.core.parameter.ParametersDefinition @@ -8,6 +12,7 @@ import org.koin.core.qualifier.Qualifier import java.util.UUID private typealias ConverterDeclaration = DataConversion.Configuration.() -> Unit + private inline fun DataConversion.Configuration.get( qualifier: Qualifier? = null, noinline parameters: ParametersDefinition? = null @@ -17,7 +22,21 @@ private inline fun DataConversion.Configuration.get( val converters: ConverterDeclaration = { convert { decode { values, _ -> - values.singleOrNull()?.let { UUID.fromString(it) } + try { + values.singleOrNull()?.let { UUID.fromString(it) } + } catch (e: Throwable) { + throw BadRequestException( + HttpErrorBadRequest( + HttpStatusCode.BadRequest, + invalidParams = listOf( + InvalidParam( + "ID", + "ID must be UUID" + ) + ) + ) + ) + } } encode { value -> diff --git a/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt b/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt index 76aa5bf..2d4f1f2 100644 --- a/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt +++ b/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt @@ -6,6 +6,7 @@ import fr.dcproject.component.auth.ForbiddenException import fr.dcproject.component.auth.user import io.ktor.application.call import io.ktor.features.NotFoundException +import io.ktor.features.ParameterConversionException import io.ktor.features.StatusPages import io.ktor.http.HttpStatusCode import io.ktor.response.respond @@ -19,10 +20,6 @@ class HttpError( val detail: String? = null, ) { val statusCode: Int = statusCode.value - data class InvalidParam( - val name: String, - val reason: String - ) } fun statusPagesInstallation(): StatusPages.Configuration.() -> Unit = { @@ -80,4 +77,12 @@ fun statusPagesInstallation(): StatusPages.Configuration.() -> Unit = { exception { e -> call.respond(HttpStatusCode.BadRequest, e.httpError) } + exception { e -> + val parent = e.cause + if (parent is BadRequestException) { + call.respond(HttpStatusCode.BadRequest, parent.httpError) + } else { + throw e + } + } } diff --git a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticleVersions.kt b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticleVersions.kt index d9deb0d..83092b3 100644 --- a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticleVersions.kt +++ b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticleVersions.kt @@ -1,42 +1,69 @@ package fr.dcproject.component.article.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert +import fr.dcproject.common.utils.toUUID +import fr.dcproject.common.validation.isUuid import fr.dcproject.component.article.ArticleAccessControl import fr.dcproject.component.article.database.ArticleForListing -import fr.dcproject.component.article.database.ArticleRef import fr.dcproject.component.article.database.ArticleRepository import fr.dcproject.component.auth.citizenOrNull +import fr.dcproject.routes.PaginatedRequest +import fr.dcproject.routes.PaginatedRequestI import fr.postgresjson.repository.RepositoryI +import io.konform.validation.Validation +import io.konform.validation.jsonschema.enum +import io.konform.validation.jsonschema.maximum +import io.konform.validation.jsonschema.minimum import io.ktor.application.call import io.ktor.locations.KtorExperimentalLocationsAPI import io.ktor.locations.Location import io.ktor.locations.get import io.ktor.response.respond import io.ktor.routing.Route -import java.util.UUID @KtorExperimentalLocationsAPI object FindArticleVersions { @Location("/articles/{article}/versions") class ArticleVersionsRequest( - article: UUID, + val article: String, page: Int = 1, limit: Int = 50, val sort: String? = null, val direction: RepositoryI.Direction? = null, val search: String? = null - ) { - val page: Int = if (page < 1) 1 else page - val limit: Int = if (limit > 50) 50 else if (limit < 1) 1 else limit - val article = ArticleRef(article) + ) : PaginatedRequestI by PaginatedRequest(page, limit) { + fun validate() = Validation { + ArticleVersionsRequest::page { + minimum(1) + maximum(100) + } + ArticleVersionsRequest::limit { + minimum(1) + maximum(50) + } + ArticleVersionsRequest::sort ifPresent { + enum( + "title", + "createdAt", + "vote", + "popularity", + ) + } + ArticleVersionsRequest::article { + isUuid() + } + }.validate(this) } private fun ArticleRepository.findVersions(request: ArticleVersionsRequest) = - findVersionsById(request.page, request.limit, request.article.id) + findVersionsById(request.page, request.limit, request.article.toUUID()) fun Route.findArticleVersions(repo: ArticleRepository, ac: ArticleAccessControl) { get { + it.validate().badRequestIfNotValid() + repo.findVersions(it) .apply { ac.assert { canView(result, citizenOrNull) } } .run { diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 7ce9bab..1013e31 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -211,6 +211,12 @@ paths: format: uuid name: type: string + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' /login: post: diff --git a/src/test/kotlin/integration/Article routes.kt b/src/test/kotlin/integration/Article routes.kt index 84e6be2..ce3e2c8 100644 --- a/src/test/kotlin/integration/Article routes.kt +++ b/src/test/kotlin/integration/Article routes.kt @@ -18,7 +18,7 @@ import integration.steps.then.`And the response should contain` import integration.steps.then.`And the response should not be null` import integration.steps.then.`And the response should not contain` import integration.steps.then.`Then the response should be` -import integration.steps.then.`whish contains` +import integration.steps.then.`which contains` import integration.steps.then.and import io.ktor.http.HttpStatusCode.Companion.BadRequest import io.ktor.http.HttpStatusCode.Companion.Forbidden @@ -46,6 +46,7 @@ class `Article routes` : BaseTest() { } @Test + @Tag("Validation") fun `I cannot get article list`() { withIntegrationApplication { `Given I have articles`(3) @@ -64,8 +65,8 @@ class `Article routes` : BaseTest() { `Given I have article created by workgroup`("2bccd5a7-9082-4b31-88f8-e25d70b22b12") `When I send a GET request`("/articles?workgroup=2bccd5a7-9082-4b31-88f8-e25d70b22b12") `Then the response should be` OK and { `And the response should not be null`() - `And have property`("$.total") `whish contains` 1 - `And have property`("$.result[0]workgroup.name") `whish contains` "Les papy" + `And have property`("$.total") `which contains` 1 + `And have property`("$.result[0]workgroup.name") `which contains` "Les papy" } } } @@ -76,7 +77,7 @@ class `Article routes` : BaseTest() { `Given I have article`(id = "65cda9f3-8991-4420-8d41-1da9da72c9bb") `When I send a GET request`("/articles/65cda9f3-8991-4420-8d41-1da9da72c9bb") `Then the response should be` OK and { `And the response should not be null`() - `And have property`("$.id") `whish contains` "65cda9f3-8991-4420-8d41-1da9da72c9bb" + `And have property`("$.id") `which contains` "65cda9f3-8991-4420-8d41-1da9da72c9bb" } } } @@ -85,10 +86,36 @@ class `Article routes` : BaseTest() { fun `I can get versions of article by the id`() { withIntegrationApplication { `Given I have article`(id = "13e6091c-8fed-4600-b079-a97a6b7a9800") - `When I send a GET request`("/articles/13e6091c-8fed-4600-b079-a97a6b7a9800/versions") `Then the response should be` OK and { + `When I send a GET request`("/articles/13e6091c-8fed-4600-b079-a97a6b7a9800/versions?page=1&limit=10&sort=title") `Then the response should be` OK and { `And the response should not be null`() - `And have property`("$.total") `whish contains` 1 - `And have property`("$.result[0].id") `whish contains` "13e6091c-8fed-4600-b079-a97a6b7a9800" + `And have property`("$.total") `which contains` 1 + `And have property`("$.result[0].id") `which contains` "13e6091c-8fed-4600-b079-a97a6b7a9800" + } + } + } + + @Test + @Tag("Validation") + fun `I cannot get versions of article by the id with wrong id`() { + withIntegrationApplication { + `Given I have article`(id = "13e6091c-8fed-4600-b079-a97a6b7a9800") + `When I send a GET request`("/articles/abcd/versions") `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".article") + `And the response should contain`("$.invalidParams[0].reason", "must be UUID") + } + } + } + + @Test + @Tag("Validation") + fun `I cannot get versions of article by the id with wrong request`() { + withIntegrationApplication { + `Given I have article`(id = "13e6091c-8fed-4600-b079-a97a6b7a9800") + `When I send a GET request`("/articles/13e6091c-8fed-4600-b079-a97a6b7a9800/versions?page=1&limit=10&sort=wrong") `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".sort") + `And the response should contain pattern`("$.invalidParams[0].reason", "must be one of: ('[^']+'(, )?)+") } } } @@ -115,7 +142,7 @@ class `Article routes` : BaseTest() { ) } `Then the response should be` OK and { `And the response should not be null`() - `And have property`("$.versionId") `whish contains` "09c418b6-63ba-448b-b38b-502b41cd500e" + `And have property`("$.versionId") `which contains` "09c418b6-63ba-448b-b38b-502b41cd500e" } } } diff --git a/src/test/kotlin/integration/Citizen routes.kt b/src/test/kotlin/integration/Citizen routes.kt index 602212b..a045682 100644 --- a/src/test/kotlin/integration/Citizen routes.kt +++ b/src/test/kotlin/integration/Citizen routes.kt @@ -9,7 +9,7 @@ import integration.steps.given.`authenticated as` import integration.steps.then.`And have property` import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` -import integration.steps.then.`whish contains` +import integration.steps.then.`which contains` import integration.steps.then.and import io.ktor.http.HttpStatusCode.Companion.BadRequest import io.ktor.http.HttpStatusCode.Companion.Created @@ -42,7 +42,7 @@ class `Citizen routes` : BaseTest() { `authenticated as`("Linus", "Pauling") } `Then the response should be` OK and { `And the response should not be null`() - `And have property`("$.id") `whish contains` "47a05c0f-7329-46c3-a7d0-325db37e9114" + `And have property`("$.id") `which contains` "47a05c0f-7329-46c3-a7d0-325db37e9114" } } } @@ -55,7 +55,7 @@ class `Citizen routes` : BaseTest() { `authenticated as`("Henri", "Becquerel") } `Then the response should be` OK and { `And the response should not be null`() - `And have property`("$.id") `whish contains` "47356809-c8ef-4649-8b99-1c5cb9886d38" + `And have property`("$.id") `which contains` "47356809-c8ef-4649-8b99-1c5cb9886d38" } } } diff --git a/src/test/kotlin/integration/Constitution routes.kt b/src/test/kotlin/integration/Constitution routes.kt index 01de75b..2f208dd 100644 --- a/src/test/kotlin/integration/Constitution routes.kt +++ b/src/test/kotlin/integration/Constitution routes.kt @@ -11,7 +11,7 @@ import integration.steps.given.`authenticated as` import integration.steps.then.`And have property` import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` -import integration.steps.then.`whish contains` +import integration.steps.then.`which contains` import integration.steps.then.and import io.ktor.http.HttpStatusCode.Companion.BadRequest import io.ktor.http.HttpStatusCode.Companion.Created @@ -41,7 +41,7 @@ class `Constitution routes` : BaseTest() { `Given I have constitution`("0321c8d1-4ce3-4763-b5f4-a92611d280b4") `When I send a GET request`("/constitutions/0321c8d1-4ce3-4763-b5f4-a92611d280b4") `Then the response should be` OK and { `And the response should not be null`() - `And have property`("$.id") `whish contains` "0321c8d1-4ce3-4763-b5f4-a92611d280b4" + `And have property`("$.id") `which contains` "0321c8d1-4ce3-4763-b5f4-a92611d280b4" } } } @@ -82,8 +82,8 @@ class `Constitution routes` : BaseTest() { ) } `Then the response should be` Created and { `And the response should not be null`() - `And have property`("$.versionId") `whish contains` "15814bb6-8d90-4c6a-a456-c3939a8ec75e" - `And have property`("$.title") `whish contains` "Hello world!" + `And have property`("$.versionId") `which contains` "15814bb6-8d90-4c6a-a456-c3939a8ec75e" + `And have property`("$.title") `which contains` "Hello world!" } } } diff --git a/src/test/kotlin/integration/steps/then/request.kt b/src/test/kotlin/integration/steps/then/request.kt index cc5156d..cdea9e5 100644 --- a/src/test/kotlin/integration/steps/then/request.kt +++ b/src/test/kotlin/integration/steps/then/request.kt @@ -46,7 +46,7 @@ infix fun TestApplicationResponse.`And have property`(path: String): Pair.`whish contains`(expected: Any): Pair = this.apply { +infix fun Pair.`which contains`(expected: Any): Pair = this.apply { second `should be equal to` expected }