From 3a18ef0554c7beacab53cddeed29c11c18e38024 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sat, 3 Apr 2021 00:38:56 +0200 Subject: [PATCH 01/46] Improve articles request test --- src/test/kotlin/integration/Article routes.kt | 20 ++++++++++++++----- .../kotlin/integration/steps/given/Article.kt | 17 +++++++++++----- .../kotlin/integration/steps/given/Citizen.kt | 7 ++++--- .../kotlin/integration/steps/when/request.kt | 4 ++-- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/test/kotlin/integration/Article routes.kt b/src/test/kotlin/integration/Article routes.kt index 0ed6685..83e7792 100644 --- a/src/test/kotlin/integration/Article routes.kt +++ b/src/test/kotlin/integration/Article routes.kt @@ -1,5 +1,7 @@ package integration +import fr.dcproject.common.utils.toUUID +import integration.steps.`when`.Validate import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` import integration.steps.`when`.`with body` @@ -18,6 +20,7 @@ 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.and +import io.ktor.http.HttpStatusCode.Companion.BadRequest import io.ktor.http.HttpStatusCode.Companion.Forbidden import io.ktor.http.HttpStatusCode.Companion.OK import org.junit.jupiter.api.Tag @@ -32,17 +35,24 @@ class `Article routes` : BaseTest() { fun `I can get article list`() { withIntegrationApplication { `Given I have articles`(3) - `When I send a GET request`("/articles") `Then the response should be` OK and { + `Given I have article`(createdBy = "ddb17f17-e8ab-4ada-bdf7-bfd6b0f1b5ed".toUUID()) + `When I send a GET request`("/articles?page=1&limit=10&sort=title&createdBy=ddb17f17-e8ab-4ada-bdf7-bfd6b0f1b5ed") `Then the response should be` OK and { `And the response should not be null`() `And the response should contain pattern`("$.result[0].createdBy.name.firstName", "firstName.+") - `And the response should contain pattern`("$.result[1].createdBy.name.firstName", "firstName.+") - `And the response should contain pattern`("$.result[2].createdBy.name.firstName", "firstName.+") - `And the response should not contain`("$.result[3]") - `And the response should contain list`("$.result", 3) + `And the response should not contain`("$.result[1]") + `And the response should contain list`("$.result", 1) } } } + @Test + fun `I cannot get article list`() { + withIntegrationApplication { + `Given I have articles`(3) + `When I send a GET request`("/articles?page=1&limit=10&sort=title&createdBy=hello", Validate.ALL - Validate.REQUEST_PARAM) `Then the response should be` BadRequest + } + } + @Test fun `I can get articles filtered by workgroup`() { withIntegrationApplication { diff --git a/src/test/kotlin/integration/steps/given/Article.kt b/src/test/kotlin/integration/steps/given/Article.kt index a9d978d..b32f431 100644 --- a/src/test/kotlin/integration/steps/given/Article.kt +++ b/src/test/kotlin/integration/steps/given/Article.kt @@ -6,6 +6,7 @@ import fr.dcproject.component.article.database.ArticleForUpdate import fr.dcproject.component.article.database.ArticleForView import fr.dcproject.component.article.database.ArticleRepository import fr.dcproject.component.citizen.database.CitizenI.Name +import fr.dcproject.component.citizen.database.CitizenRef import fr.dcproject.component.workgroup.database.WorkgroupRef import io.ktor.server.testing.TestApplicationEngine import org.koin.core.context.GlobalContext @@ -16,7 +17,15 @@ fun TestApplicationEngine.`Given I have article`( workgroup: WorkgroupRef? = null, createdBy: Name? = null ) { - createArticle(id?.toUUID(), workgroup, createdBy) + createArticle(id?.toUUID(), workgroup, createCitizen(name = createdBy)) +} + +fun TestApplicationEngine.`Given I have article`( + id: String? = null, + workgroup: WorkgroupRef? = null, + createdBy: UUID +) { + createArticle(id?.toUUID(), workgroup, createCitizen(id = createdBy)) } fun TestApplicationEngine.`Given I have articles`( @@ -35,18 +44,16 @@ fun TestApplicationEngine.`Given I have article created by workgroup`( fun createArticle( id: UUID? = null, workgroup: WorkgroupRef? = null, - createdBy: Name? = null + createdBy: CitizenRef = createCitizen() ): ArticleForView { val articleRepository: ArticleRepository by lazy { GlobalContext.get().koin.get() } - val citizen = createCitizen(createdBy) - val article = ArticleForUpdate( id = id ?: UUID.randomUUID(), title = LoremIpsum().getTitle(3), content = LoremIpsum().getParagraphs(1, 2), description = LoremIpsum().getParagraphs(1, 2), - createdBy = citizen, + createdBy = createdBy, workgroup = workgroup, versionId = UUID.randomUUID() ) diff --git a/src/test/kotlin/integration/steps/given/Citizen.kt b/src/test/kotlin/integration/steps/given/Citizen.kt index 9d81861..951491c 100644 --- a/src/test/kotlin/integration/steps/given/Citizen.kt +++ b/src/test/kotlin/integration/steps/given/Citizen.kt @@ -36,16 +36,17 @@ fun TestApplicationEngine.`Given I have citizen`( return repo.insertWithUser(citizen)?.also { callback(it) } } -fun createCitizen(createdBy: CitizenI.Name? = null): Citizen { +fun createCitizen(name: CitizenI.Name? = null, id: UUID = UUID.randomUUID()): Citizen { val citizenRepository: CitizenRepository by lazy { GlobalContext.get().koin.get() } - return if (createdBy != null) { - citizenRepository.findByName(createdBy) ?: error("Citizen not exist") + return if (name != null) { + citizenRepository.findByName(name) ?: error("Citizen not exist") } else { val first = "firstName" + UUID.randomUUID().toString() val last = "lastName" + UUID.randomUUID().toString() val username = ("username" + UUID.randomUUID().toString()) CitizenForCreate( + id = id, birthday = DateTime.now(), name = CitizenI.Name( first, diff --git a/src/test/kotlin/integration/steps/when/request.kt b/src/test/kotlin/integration/steps/when/request.kt index b09e19b..a64a448 100644 --- a/src/test/kotlin/integration/steps/when/request.kt +++ b/src/test/kotlin/integration/steps/when/request.kt @@ -40,7 +40,7 @@ fun TestApplicationCall.valid(validate: BitMaskI): TestApplicationCall { return this } -fun TestApplicationEngine.`When I send a GET request`(uri: String? = null, validate: Validate = Validate.ALL, setup: (TestApplicationRequest.() -> Unit)? = null): TestApplicationCall { +fun TestApplicationEngine.`When I send a GET request`(uri: String? = null, validate: BitMaskI = Validate.ALL, setup: (TestApplicationRequest.() -> Unit)? = null): TestApplicationCall { return handleRequest(true) { method = HttpMethod.Get if (uri != null) { @@ -74,7 +74,7 @@ fun TestApplicationEngine.`When I send a PUT request`(uri: String? = null, valid }.valid(validate) } -fun TestApplicationEngine.`When I send a DELETE request`(uri: String? = null, validate: Validate = Validate.ALL, setup: (TestApplicationRequest.() -> String?)? = null): TestApplicationCall { +fun TestApplicationEngine.`When I send a DELETE request`(uri: String? = null, validate: BitMaskI = Validate.ALL, setup: (TestApplicationRequest.() -> String?)? = null): TestApplicationCall { return handleRequest(true) { method = HttpMethod.Delete if (uri != null) { From a300e275d414ccb91179519953cb430558c0491e Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sat, 3 Apr 2021 00:52:52 +0200 Subject: [PATCH 02/46] Valid FindArticles request with Konform --- build.gradle.kts | 6 ++- .../fr/dcproject/common/validation/Uuid.kt | 14 +++++++ .../component/article/routes/FindArticles.kt | 38 ++++++++++++++++++- src/main/resources/openapi.yaml | 2 + 4 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 src/main/kotlin/fr/dcproject/common/validation/Uuid.kt diff --git a/build.gradle.kts b/build.gradle.kts index 6b07044..2757de5 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -327,8 +327,9 @@ dependencyCheck { repositories { mavenLocal() jcenter() - maven { url = uri("https://kotlin.bintray.com/ktor") } - maven { url = uri("https://jitpack.io") } + maven("https://kotlin.bintray.com/ktor") + maven("https://jitpack.io") + maven("https://dl.bintray.com/konform-kt/konform") } dependencies { @@ -359,6 +360,7 @@ dependencies { implementation("org.elasticsearch.client:elasticsearch-rest-client:6.7.1") implementation("com.jayway.jsonpath:json-path:2.5.0") implementation("com.avast.gradle:gradle-docker-compose-plugin:0.14.0") + implementation("io.konform:konform-jvm:0.2.0") testImplementation("io.ktor:ktor-server-tests:$ktorVersion") testImplementation("io.ktor:ktor-client-mock:$ktorVersion") diff --git a/src/main/kotlin/fr/dcproject/common/validation/Uuid.kt b/src/main/kotlin/fr/dcproject/common/validation/Uuid.kt new file mode 100644 index 0000000..93708cd --- /dev/null +++ b/src/main/kotlin/fr/dcproject/common/validation/Uuid.kt @@ -0,0 +1,14 @@ +package fr.dcproject.common.validation + +import io.konform.validation.ValidationBuilder +import java.util.UUID + +fun ValidationBuilder.isUuid() = + addConstraint("must be UUID") { + try { + UUID.fromString(it) + true + } catch (exception: IllegalArgumentException) { + false + } + } diff --git a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt index b0b5abd..b90f269 100644 --- a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt +++ b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt @@ -2,6 +2,7 @@ package fr.dcproject.component.article.routes import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert +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.ArticleRepository @@ -10,7 +11,12 @@ import fr.dcproject.routes.PaginatedRequest import fr.dcproject.routes.PaginatedRequestI import fr.postgresjson.connexion.Paginated 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.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI import io.ktor.locations.Location import io.ktor.locations.get @@ -28,7 +34,32 @@ object FindArticles { val search: String? = null, val createdBy: String? = null, val workgroup: String? = null - ) : PaginatedRequestI by PaginatedRequest(page, limit) + ) : PaginatedRequestI by PaginatedRequest(page, limit) { + fun validate() = Validation { + ArticlesRequest::page { + minimum(1) + maximum(100) + } + ArticlesRequest::limit { + minimum(1) + maximum(50) + } + ArticlesRequest::sort ifPresent { + enum( + "title", + "createdAt", + "vote", + "popularity", + ) + } + ArticlesRequest::createdBy ifPresent { + isUuid() + } + ArticlesRequest::workgroup ifPresent { + isUuid() + } + }.validate(this) + } private fun ArticleRepository.findArticles(request: ArticlesRequest): Paginated { return find( @@ -43,6 +74,11 @@ object FindArticles { fun Route.findArticles(repo: ArticleRepository, ac: ArticleAccessControl) { get { + if (it.validate().errors.size > 0) { + call.respond(HttpStatusCode.BadRequest) + return@get + } + repo.findArticles(it) .apply { ac.assert { canView(result, citizenOrNull) } } .let { diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index b204c7e..386e9e7 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -41,6 +41,8 @@ paths: maxItems: 50 items: $ref: '#/components/schemas/ArticleListingResponse' + 400: + $ref: '#/components/responses/400' post: security: - JWTAuth: [] From 395d64a44a683c916c871235dec1dfff53a8fc4b Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sat, 3 Apr 2021 00:54:17 +0200 Subject: [PATCH 03/46] create testArticle gradle task --- build.gradle.kts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index 2757de5..47f3fd0 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -320,6 +320,13 @@ tasks.named("testComposeUp").configure { } } +tasks.register("testArticles", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("article") + } +} + dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) } From ab418ae300fd818416079884ad77cc93ad151601 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Mon, 5 Apr 2021 00:48:11 +0200 Subject: [PATCH 04/46] Add openapi response of error 400 --- src/main/resources/openapi.yaml | 41 ++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 386e9e7..e453251 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -42,7 +42,11 @@ paths: items: $ref: '#/components/schemas/ArticleListingResponse' 400: - $ref: '#/components/responses/400' + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' post: security: - JWTAuth: [] @@ -2211,6 +2215,41 @@ components: - REPORTER example: MASTER + 400: + description: noting + required: + - title + - invalidParams + additionalProperties: false + properties: + statusCode: + type: integer + type: + type: string + nullable: true + title: + type: string + detail: + type: string + nullable: true + cause: + type: string + nullable: true + stackTrace: + type: string + nullable: true + invalidParams: + type: array + items: + required: + - name + - reason + properties: + name: + type: string + reason: + type: string + securitySchemes: JWTAuth: type: http From 3faf2e5f0d6de9149809017e0cf735d4c5bd06e0 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Mon, 5 Apr 2021 00:48:58 +0200 Subject: [PATCH 05/46] Add function to respond on BadRequest --- .../application/http/HttpStatusError.kt | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt b/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt index 215add9..319c0ee 100644 --- a/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt +++ b/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt @@ -4,11 +4,14 @@ import com.github.jasync.sql.db.postgresql.exceptions.GenericDatabaseException import fr.dcproject.common.security.AccessDeniedException import fr.dcproject.component.auth.ForbiddenException import fr.dcproject.component.auth.user +import io.konform.validation.ValidationResult +import io.ktor.application.ApplicationCall import io.ktor.application.call import io.ktor.features.NotFoundException import io.ktor.features.StatusPages import io.ktor.http.HttpStatusCode import io.ktor.response.respond +import io.ktor.util.pipeline.PipelineContext import java.util.concurrent.CompletionException class HttpError( @@ -27,6 +30,30 @@ class HttpError( ) } +fun ValidationResult<*>.toOutput(): HttpError { + return HttpError( + HttpStatusCode.BadRequest, + invalidParams = this.errors.map { + HttpError.InvalidParam( + it.dataPath, + it.message + ) + } + ) +} + +suspend fun PipelineContext<*, ApplicationCall>.respondIfNotValid(validationResult: ValidationResult<*>): HttpError? { + if (validationResult.errors.size > 0) { + val out = validationResult.toOutput() + this.call.respond( + HttpStatusCode.BadRequest, + out + ) + return out + } + return null +} + fun statusPagesInstallation(): StatusPages.Configuration.() -> Unit = { exception { e -> val parent = e.cause?.cause From b5fc3d25bbe57cc577bdb37261bad523994ebce1 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Mon, 5 Apr 2021 00:49:40 +0200 Subject: [PATCH 06/46] Improve Article validation & test on BadRequest --- .../fr/dcproject/component/article/routes/FindArticles.kt | 7 ++----- src/test/kotlin/integration/Article routes.kt | 5 ++++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt index b90f269..c1df6d6 100644 --- a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt +++ b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.article.routes +import fr.dcproject.application.http.respondIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.common.validation.isUuid @@ -16,7 +17,6 @@ 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.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI import io.ktor.locations.Location import io.ktor.locations.get @@ -74,10 +74,7 @@ object FindArticles { fun Route.findArticles(repo: ArticleRepository, ac: ArticleAccessControl) { get { - if (it.validate().errors.size > 0) { - call.respond(HttpStatusCode.BadRequest) - return@get - } + respondIfNotValid(it.validate())?.apply { return@get } repo.findArticles(it) .apply { ac.assert { canView(result, citizenOrNull) } } diff --git a/src/test/kotlin/integration/Article routes.kt b/src/test/kotlin/integration/Article routes.kt index 83e7792..84e6be2 100644 --- a/src/test/kotlin/integration/Article routes.kt +++ b/src/test/kotlin/integration/Article routes.kt @@ -49,7 +49,10 @@ class `Article routes` : BaseTest() { fun `I cannot get article list`() { withIntegrationApplication { `Given I have articles`(3) - `When I send a GET request`("/articles?page=1&limit=10&sort=title&createdBy=hello", Validate.ALL - Validate.REQUEST_PARAM) `Then the response should be` BadRequest + `When I send a GET request`("/articles?page=1&limit=10&sort=title&createdBy=hello", Validate.ALL - Validate.REQUEST_PARAM) `Then the response should be` BadRequest and { + `And the response should contain`("$.invalidParams[*].name", ".createdBy") + `And the response should contain`("$.invalidParams[*].reason", "must be UUID") + } } } From 2ef9f65f2c4a7dcf772cacde2f7505ee1518e47f Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Mon, 5 Apr 2021 01:09:58 +0200 Subject: [PATCH 07/46] Clean BadRequest response --- .../application/http/HttpErrorBadRequest.kt | 35 +++++++++++++++++++ .../application/http/HttpStatusError.kt | 32 ++--------------- .../component/article/routes/FindArticles.kt | 4 +-- src/main/resources/openapi.yaml | 24 ++++++------- 4 files changed, 52 insertions(+), 43 deletions(-) create mode 100644 src/main/kotlin/fr/dcproject/application/http/HttpErrorBadRequest.kt diff --git a/src/main/kotlin/fr/dcproject/application/http/HttpErrorBadRequest.kt b/src/main/kotlin/fr/dcproject/application/http/HttpErrorBadRequest.kt new file mode 100644 index 0000000..88b4ae5 --- /dev/null +++ b/src/main/kotlin/fr/dcproject/application/http/HttpErrorBadRequest.kt @@ -0,0 +1,35 @@ +package fr.dcproject.application.http + +import fr.dcproject.application.http.HttpErrorBadRequest.InvalidParam +import io.konform.validation.ValidationResult +import io.ktor.http.HttpStatusCode + +class BadRequestException(val httpError: HttpErrorBadRequest) : Exception() + +class HttpErrorBadRequest( + statusCode: HttpStatusCode, + val title: String = statusCode.description, + val invalidParams: List, +) { + val statusCode: Int = statusCode.value + data class InvalidParam( + val name: String, + val reason: String + ) +} + +fun ValidationResult<*>.toOutput() = HttpErrorBadRequest( + HttpStatusCode.BadRequest, + invalidParams = this.errors.map { + InvalidParam( + it.dataPath, + it.message + ) + } +) + +fun ValidationResult<*>.badRequestIfNotValid() { + if (errors.size > 0) { + throw BadRequestException(toOutput()) + } +} diff --git a/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt b/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt index 319c0ee..76aa5bf 100644 --- a/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt +++ b/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt @@ -4,14 +4,11 @@ import com.github.jasync.sql.db.postgresql.exceptions.GenericDatabaseException import fr.dcproject.common.security.AccessDeniedException import fr.dcproject.component.auth.ForbiddenException import fr.dcproject.component.auth.user -import io.konform.validation.ValidationResult -import io.ktor.application.ApplicationCall import io.ktor.application.call import io.ktor.features.NotFoundException import io.ktor.features.StatusPages import io.ktor.http.HttpStatusCode import io.ktor.response.respond -import io.ktor.util.pipeline.PipelineContext import java.util.concurrent.CompletionException class HttpError( @@ -20,8 +17,6 @@ class HttpError( val type: String? = null, val title: String = cause?.message ?: statusCode.description, val detail: String? = null, - val invalidParams: List? = null, - val stackTrace: String? = cause?.stackTraceToString() ) { val statusCode: Int = statusCode.value data class InvalidParam( @@ -30,30 +25,6 @@ class HttpError( ) } -fun ValidationResult<*>.toOutput(): HttpError { - return HttpError( - HttpStatusCode.BadRequest, - invalidParams = this.errors.map { - HttpError.InvalidParam( - it.dataPath, - it.message - ) - } - ) -} - -suspend fun PipelineContext<*, ApplicationCall>.respondIfNotValid(validationResult: ValidationResult<*>): HttpError? { - if (validationResult.errors.size > 0) { - val out = validationResult.toOutput() - this.call.respond( - HttpStatusCode.BadRequest, - out - ) - return out - } - return null -} - fun statusPagesInstallation(): StatusPages.Configuration.() -> Unit = { exception { e -> val parent = e.cause?.cause @@ -106,4 +77,7 @@ fun statusPagesInstallation(): StatusPages.Configuration.() -> Unit = { call.respond(HttpStatusCode.Forbidden, it) } } + exception { e -> + call.respond(HttpStatusCode.BadRequest, e.httpError) + } } diff --git a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt index c1df6d6..2d8bd9a 100644 --- a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt +++ b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt @@ -1,6 +1,6 @@ package fr.dcproject.component.article.routes -import fr.dcproject.application.http.respondIfNotValid +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.common.validation.isUuid @@ -74,7 +74,7 @@ object FindArticles { fun Route.findArticles(repo: ArticleRepository, ac: ArticleAccessControl) { get { - respondIfNotValid(it.validate())?.apply { return@get } + it.validate().badRequestIfNotValid() repo.findArticles(it) .apply { ac.assert { canView(result, citizenOrNull) } } diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index e453251..7ce9bab 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -134,6 +134,12 @@ paths: tags: - article operationId: getArticle + parameters: + - $ref: '#/components/parameters/page' + - $ref: '#/components/parameters/limit' + - $ref: '#/components/parameters/sort' + - $ref: '#/components/parameters/direction' + - $ref: '#/components/parameters/search' responses: 200: description: The Article objects @@ -149,6 +155,12 @@ paths: tags: - article operationId: getArticleVersions + parameters: + - $ref: '#/components/parameters/page' + - $ref: '#/components/parameters/limit' + - $ref: '#/components/parameters/sort' + - $ref: '#/components/parameters/direction' + - $ref: '#/components/parameters/search' responses: 200: description: The versions of Article @@ -2224,20 +2236,8 @@ components: properties: statusCode: type: integer - type: - type: string - nullable: true title: type: string - detail: - type: string - nullable: true - cause: - type: string - nullable: true - stackTrace: - type: string - nullable: true invalidParams: type: array items: From 61a7091736c3363a27f5eab374ff97e7b3508904 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Tue, 6 Apr 2021 00:05:27 +0200 Subject: [PATCH 08/46] Add validation on route Article versions --- .../fr/dcproject/application/Converters.kt | 21 ++++++++- .../application/http/HttpStatusError.kt | 13 ++++-- .../article/routes/FindArticleVersions.kt | 43 +++++++++++++++---- src/main/resources/openapi.yaml | 6 +++ src/test/kotlin/integration/Article routes.kt | 43 +++++++++++++++---- src/test/kotlin/integration/Citizen routes.kt | 6 +-- .../kotlin/integration/Constitution routes.kt | 8 ++-- .../kotlin/integration/steps/then/request.kt | 2 +- 8 files changed, 113 insertions(+), 29 deletions(-) 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 } From fe11384ad284c834aa5acd55828a57dd3773ac6f Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Tue, 6 Apr 2021 23:04:02 +0200 Subject: [PATCH 09/46] Add validation on route GetOneArticle --- .../fr/dcproject/application/Converters.kt | 2 +- src/main/resources/openapi.yaml | 6 ++++++ src/test/kotlin/integration/Article routes.kt | 19 ++++++++++++++++--- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/application/Converters.kt b/src/main/kotlin/fr/dcproject/application/Converters.kt index 3226ffd..4a67f2e 100644 --- a/src/main/kotlin/fr/dcproject/application/Converters.kt +++ b/src/main/kotlin/fr/dcproject/application/Converters.kt @@ -31,7 +31,7 @@ val converters: ConverterDeclaration = { invalidParams = listOf( InvalidParam( "ID", - "ID must be UUID" + "must be UUID" ) ) ) diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 1013e31..4706131 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -147,6 +147,12 @@ paths: application/json: schema: $ref: '#/components/schemas/ArticleResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' /articles/{article}/versions: parameters: - $ref: '#/components/parameters/article' diff --git a/src/test/kotlin/integration/Article routes.kt b/src/test/kotlin/integration/Article routes.kt index ce3e2c8..5b12bf0 100644 --- a/src/test/kotlin/integration/Article routes.kt +++ b/src/test/kotlin/integration/Article routes.kt @@ -46,7 +46,7 @@ class `Article routes` : BaseTest() { } @Test - @Tag("Validation") + @Tag("BadRequest") fun `I cannot get article list`() { withIntegrationApplication { `Given I have articles`(3) @@ -82,6 +82,19 @@ class `Article routes` : BaseTest() { } } + @Test + @Tag("BadRequest") + fun `I cannot get article by id with wrong id format`() { + withIntegrationApplication { + `Given I have article`(id = "65cda9f3-8991-4420-8d41-1da9da72c9bb") + `When I send a GET request`("/articles/abcd") `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", "ID") + `And the response should contain`("$.invalidParams[0].reason", "must be UUID") + } + } + } + @Test fun `I can get versions of article by the id`() { withIntegrationApplication { @@ -95,7 +108,7 @@ class `Article routes` : BaseTest() { } @Test - @Tag("Validation") + @Tag("BadRequest") fun `I cannot get versions of article by the id with wrong id`() { withIntegrationApplication { `Given I have article`(id = "13e6091c-8fed-4600-b079-a97a6b7a9800") @@ -108,7 +121,7 @@ class `Article routes` : BaseTest() { } @Test - @Tag("Validation") + @Tag("BadRequest") fun `I cannot get versions of article by the id with wrong request`() { withIntegrationApplication { `Given I have article`(id = "13e6091c-8fed-4600-b079-a97a6b7a9800") From e26710898e3012969c6ca216c0132a09ce510498 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Tue, 6 Apr 2021 23:35:36 +0200 Subject: [PATCH 10/46] add example on openapi 400 error --- src/main/resources/openapi.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 4706131..4d8d410 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -2248,8 +2248,10 @@ components: properties: statusCode: type: integer + example: 400 title: type: string + example: Bad Request invalidParams: type: array items: @@ -2259,8 +2261,10 @@ components: properties: name: type: string + example: '.title' reason: type: string + example: 'Cannot be null' securitySchemes: JWTAuth: From e4745e71c271d1fa8a607f384c3eaf2de9e72c41 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Wed, 7 Apr 2021 20:53:21 +0200 Subject: [PATCH 11/46] Add validation on route UpsertArticle --- .../component/article/routes/FindArticles.kt | 1 - .../component/article/routes/UpsertArticle.kt | 28 ++++++++++++- src/main/resources/openapi.yaml | 11 +++++ src/test/kotlin/integration/Article routes.kt | 40 +++++++++++++++++-- .../kotlin/integration/steps/when/request.kt | 1 + 5 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt index 2d8bd9a..44d930d 100644 --- a/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt +++ b/src/main/kotlin/fr/dcproject/component/article/routes/FindArticles.kt @@ -38,7 +38,6 @@ object FindArticles { fun validate() = Validation { ArticlesRequest::page { minimum(1) - maximum(100) } ArticlesRequest::limit { minimum(1) diff --git a/src/main/kotlin/fr/dcproject/component/article/routes/UpsertArticle.kt b/src/main/kotlin/fr/dcproject/component/article/routes/UpsertArticle.kt index 20beaef..57fce8a 100644 --- a/src/main/kotlin/fr/dcproject/component/article/routes/UpsertArticle.kt +++ b/src/main/kotlin/fr/dcproject/component/article/routes/UpsertArticle.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.article.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest import fr.dcproject.component.article.ArticleAccessControl @@ -12,6 +13,11 @@ import fr.dcproject.component.auth.mustBeAuth import fr.dcproject.component.notification.ArticleUpdateNotification import fr.dcproject.component.notification.Publisher import fr.dcproject.component.workgroup.database.WorkgroupRef +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxItems +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minItems +import io.konform.validation.jsonschema.minLength import io.ktor.application.ApplicationCall import io.ktor.application.call import io.ktor.locations.KtorExperimentalLocationsAPI @@ -35,11 +41,31 @@ object UpsertArticle { val draft: Boolean = false, val versionId: UUID, val workgroup: WorkgroupRef? = null, - ) + ) { + fun validate() = Validation { + Input::title { + minLength(5) + maxLength(80) + } + Input::content { + minLength(50) + maxLength(6000) + } + Input::description { + minLength(50) + maxLength(6000) + } + Input::tags { + minItems(0) + maxItems(15) + } + }.validate(this) + } } fun Route.upsertArticle(repo: ArticleRepository, publisher: Publisher, ac: ArticleAccessControl) { suspend fun ApplicationCall.convertRequestToEntity(): ArticleForUpdate = receiveOrBadRequest().run { + validate().badRequestIfNotValid() ArticleForUpdate( id = id ?: UUID.randomUUID(), title = title, diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 4d8d410..17d5216 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -71,16 +71,21 @@ paths: Limit power of press content: type: string + minLength: 50 + maxLength: 6000 example: Lorem upsum... description: type: string + minLength: 50 + maxLength: 6000 example: I think is the bether choice tags: type: array items: type: string + maxItems: 15 default: [ ] example: [ power, press ] anonymous: @@ -112,6 +117,12 @@ paths: format: uuid versionNumber: type: integer + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' 403: diff --git a/src/test/kotlin/integration/Article routes.kt b/src/test/kotlin/integration/Article routes.kt index 5b12bf0..fa0cc82 100644 --- a/src/test/kotlin/integration/Article routes.kt +++ b/src/test/kotlin/integration/Article routes.kt @@ -145,8 +145,8 @@ class `Article routes` : BaseTest() { "versionId": "09c418b6-63ba-448b-b38b-502b41cd500e", "title": "title2", "anonymous": false, - "content": "content2", - "description": "description2", + "content": "Sed malesuada ante et sem congue, scelerisque feugiat lorem viverra.", + "description": "Sed vulputate, ligula id porta posuere, sapien lorem mattis arcu, sit amet luctus erat orci sed tellus.", "tags": [ "green" ] @@ -161,6 +161,7 @@ class `Article routes` : BaseTest() { } @Test + @Tag("Forbidden") fun `I cannot create an article if I'm not connected`() { withIntegrationApplication { `When I send a POST request`("/articles") { @@ -170,8 +171,8 @@ class `Article routes` : BaseTest() { "versionId": "e3c7ce42-241c-4caf-9a59-aba4e466440e", "title": "title2", "anonymous": false, - "content": "content2", - "description": "description2", + "content": "Sed malesuada ante et sem congue, scelerisque feugiat lorem viverra.", + "description": "Sed vulputate, ligula id porta posuere, sapien lorem mattis arcu, sit amet luctus erat orci sed tellus.", "tags": [ "green" ] @@ -185,4 +186,35 @@ class `Article routes` : BaseTest() { } } } + + @Test + @Tag("BadRequest") + fun `I cannot create an article with wrong request`() { + withIntegrationApplication { + `Given I have citizen`("John", "Doe") + `When I send a POST request`("/articles", Validate.NONE) { + `authenticated as`("John", "Doe") + `with body`( + """ + { + "versionId": "09c418b6-63ba-448b-b38b-502b41cd500e", + "title": "title2", + "anonymous": false, + "content": "content2", + "description": "description2", + "tags": [ + "green" + ] + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".content") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 50 characters") + `And the response should contain`("$.invalidParams[1].name", ".description") + `And the response should contain`("$.invalidParams[1].reason", "must have at least 50 characters") + } + } + } } diff --git a/src/test/kotlin/integration/steps/when/request.kt b/src/test/kotlin/integration/steps/when/request.kt index a64a448..ca428c8 100644 --- a/src/test/kotlin/integration/steps/when/request.kt +++ b/src/test/kotlin/integration/steps/when/request.kt @@ -14,6 +14,7 @@ import io.ktor.server.testing.TestApplicationRequest import io.ktor.server.testing.setBody enum class Validate(override val bit: Long) : BitMaskI { + NONE(0), REQUEST_BODY(1), REQUEST_PARAM(2), REQUEST_HEADER(4), From 708d241a263a2b243c0de55c5cd1f525c0c634c6 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Wed, 7 Apr 2021 20:53:51 +0200 Subject: [PATCH 12/46] Add tags on tests --- src/test/kotlin/integration/Citizen routes.kt | 1 + src/test/kotlin/integration/Constitution routes.kt | 1 + 2 files changed, 2 insertions(+) diff --git a/src/test/kotlin/integration/Citizen routes.kt b/src/test/kotlin/integration/Citizen routes.kt index a045682..dfd2d26 100644 --- a/src/test/kotlin/integration/Citizen routes.kt +++ b/src/test/kotlin/integration/Citizen routes.kt @@ -79,6 +79,7 @@ class `Citizen routes` : BaseTest() { } @Test + @Tag("BadRequest") fun `I cannot change my password if request is bad formatted`() { withIntegrationApplication { `Given I have citizen`("Louis", "Breguet", id = "6cf2a19d-d15d-4ee5-b2a9-907afd26b525") diff --git a/src/test/kotlin/integration/Constitution routes.kt b/src/test/kotlin/integration/Constitution routes.kt index 2f208dd..75ce11d 100644 --- a/src/test/kotlin/integration/Constitution routes.kt +++ b/src/test/kotlin/integration/Constitution routes.kt @@ -89,6 +89,7 @@ class `Constitution routes` : BaseTest() { } @Test + @Tag("BadRequest") fun `I cannot create an constitution if bad request`() { withIntegrationApplication { `Given I have citizen`("Henri", "Poincaré") From 6aa3ddb28d9c500448190a01e1c77805de64a29f Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 8 Apr 2021 01:55:10 +0200 Subject: [PATCH 13/46] Add Password validation --- .../dcproject/common/validation/Password.kt | 22 +++++++++ src/test/kotlin/unit/Password Validation.kt | 45 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 src/main/kotlin/fr/dcproject/common/validation/Password.kt create mode 100644 src/test/kotlin/unit/Password Validation.kt diff --git a/src/main/kotlin/fr/dcproject/common/validation/Password.kt b/src/main/kotlin/fr/dcproject/common/validation/Password.kt new file mode 100644 index 0000000..3e6b0cb --- /dev/null +++ b/src/main/kotlin/fr/dcproject/common/validation/Password.kt @@ -0,0 +1,22 @@ +package fr.dcproject.common.validation + +import io.konform.validation.ValidationBuilder + +fun ValidationBuilder.passwordScore(minScore: Int) = + addConstraint("is not enough strong. Use Upper case, Lower case and special characters or juste use more characters.") { value -> + value.passwordScore() >= minScore + } + +fun String.passwordScore(): Int { + var score: Int = length + val alphaNum = ('a'..'z').toList() + ('A'..'Z').toList() + ('0'..'9').toList() + val specialCount = (length - toList().intersect(alphaNum).size) + score += specialCount.let { if (it > 3) 3 else it } + + val hasAlphaLower = toList().intersect(('a'..'z').toList()).size.let { if (it > 2) 2 else it } + val hasAlphaUpper = toList().intersect(('A'..'Z').toList()).size.let { if (it > 2) 2 else it } + val hasNum = toList().intersect(('0'..'9').toList()).size.let { if (it > 2) 2 else it } + score += (hasAlphaLower + hasAlphaUpper + hasNum - 2) * 2 + + return score +} \ No newline at end of file diff --git a/src/test/kotlin/unit/Password Validation.kt b/src/test/kotlin/unit/Password Validation.kt new file mode 100644 index 0000000..e39e500 --- /dev/null +++ b/src/test/kotlin/unit/Password Validation.kt @@ -0,0 +1,45 @@ +package unit + +import fr.dcproject.common.validation.passwordScore +import io.konform.validation.Invalid +import io.konform.validation.Valid +import io.konform.validation.Validation +import org.amshove.kluent.`should be equal to` +import org.amshove.kluent.`should be instance of` +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Tags +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.parallel.Execution +import org.junit.jupiter.api.parallel.ExecutionMode + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@Execution(ExecutionMode.CONCURRENT) +@Tags(Tag("validation"), Tag("unit")) +internal class `Password Validation` { + @Test + fun password() { + "1234567890".passwordScore() `should be equal to` 10 + "1234567A".passwordScore() `should be equal to` 10 + "1234Aa".passwordScore() `should be equal to` 10 + "12Aab".passwordScore() `should be equal to` 11 + "1234Aa".passwordScore() `should be equal to` 10 + "12abCD-+".passwordScore() `should be equal to` 18 + "Abcde12!".passwordScore() `should be equal to` 15 + "Hello world".passwordScore() `should be equal to` 16 + } + + @Test + fun passwordScore() { + Validation { + ObjectToValid::password { + this.passwordScore(10) + } + }.run { + validate(ObjectToValid("1234567890")) `should be instance of` Valid::class + validate(ObjectToValid("12345678")) `should be instance of` Invalid::class + } + } + + class ObjectToValid(val password: String) +} From 33a8cdb169d527eeb651d354e2cbbf28d1a039ca Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 8 Apr 2021 01:57:34 +0200 Subject: [PATCH 14/46] Add email validation --- .../fr/dcproject/common/validation/Email.kt | 6 ++++ src/test/kotlin/unit/Email Validation.kt | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 src/main/kotlin/fr/dcproject/common/validation/Email.kt create mode 100644 src/test/kotlin/unit/Email Validation.kt diff --git a/src/main/kotlin/fr/dcproject/common/validation/Email.kt b/src/main/kotlin/fr/dcproject/common/validation/Email.kt new file mode 100644 index 0000000..32842bb --- /dev/null +++ b/src/main/kotlin/fr/dcproject/common/validation/Email.kt @@ -0,0 +1,6 @@ +package fr.dcproject.common.validation + +import io.konform.validation.ValidationBuilder +import io.konform.validation.jsonschema.pattern + +fun ValidationBuilder.email() = pattern(""".+@.+\..+""") diff --git a/src/test/kotlin/unit/Email Validation.kt b/src/test/kotlin/unit/Email Validation.kt new file mode 100644 index 0000000..a7754ff --- /dev/null +++ b/src/test/kotlin/unit/Email Validation.kt @@ -0,0 +1,32 @@ +package unit + +import fr.dcproject.common.validation.email +import io.konform.validation.Invalid +import io.konform.validation.Valid +import io.konform.validation.Validation +import org.amshove.kluent.`should be instance of` +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Tags +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.api.parallel.Execution +import org.junit.jupiter.api.parallel.ExecutionMode + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@Execution(ExecutionMode.CONCURRENT) +@Tags(Tag("validation"), Tag("unit")) +internal class `Email Validation` { + @Test + fun passwordScore() { + Validation { + ObjectToValid::email { + email() + } + }.run { + validate(ObjectToValid("abc@123.com")) `should be instance of` Valid::class + validate(ObjectToValid("abc123.com")) `should be instance of` Invalid::class + } + } + + class ObjectToValid(val email: String) +} From 9511331cd22b13bd0cc5df38d72aebf1d6898ea4 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 8 Apr 2021 02:07:49 +0200 Subject: [PATCH 15/46] Add validation on route Register --- .../dcproject/common/validation/Password.kt | 2 +- .../component/auth/routes/Register.kt | 40 ++++++++++++++++++- src/main/resources/openapi.yaml | 2 +- .../kotlin/integration/Register routes.kt | 2 +- 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/common/validation/Password.kt b/src/main/kotlin/fr/dcproject/common/validation/Password.kt index 3e6b0cb..b08bb7d 100644 --- a/src/main/kotlin/fr/dcproject/common/validation/Password.kt +++ b/src/main/kotlin/fr/dcproject/common/validation/Password.kt @@ -19,4 +19,4 @@ fun String.passwordScore(): Int { score += (hasAlphaLower + hasAlphaUpper + hasNum - 2) * 2 return score -} \ No newline at end of file +} diff --git a/src/main/kotlin/fr/dcproject/component/auth/routes/Register.kt b/src/main/kotlin/fr/dcproject/component/auth/routes/Register.kt index 08d1d67..0daa29a 100644 --- a/src/main/kotlin/fr/dcproject/component/auth/routes/Register.kt +++ b/src/main/kotlin/fr/dcproject/component/auth/routes/Register.kt @@ -1,7 +1,10 @@ package fr.dcproject.component.auth.routes import com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.utils.receiveOrBadRequest +import fr.dcproject.common.validation.email +import fr.dcproject.common.validation.passwordScore import fr.dcproject.component.auth.database.UserForCreate import fr.dcproject.component.auth.database.UserI import fr.dcproject.component.auth.jwt.makeToken @@ -9,6 +12,9 @@ import fr.dcproject.component.auth.routes.Register.RegisterRequest.Input import fr.dcproject.component.citizen.database.CitizenForCreate import fr.dcproject.component.citizen.database.CitizenI import fr.dcproject.component.citizen.database.CitizenRepository +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength import io.ktor.application.call import io.ktor.features.BadRequestException import io.ktor.http.ContentType @@ -43,6 +49,35 @@ object Register { val username: String, val password: String ) + + fun validate() = Validation { + Input::name { + Name::firstName { + minLength(2) + maxLength(50) + } + Name::lastName { + minLength(2) + maxLength(50) + } + Name::civility ifPresent { + minLength(1) + maxLength(10) + } + } + Input::user { + User::username { + minLength(7) + maxLength(30) + } + User::password { + passwordScore(15) + } + } + Input::email { + email() + } + }.validate(this) } } @@ -62,7 +97,10 @@ object Register { post { try { - val citizen = call.receiveOrBadRequest().toCitizen() + val citizen = call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } + .toCitizen() + citizenRepo.insertWithUser(citizen)?.user?.makeToken()?.let { token -> if (call.request.accept() == ContentType.Application.Json.toString()) { call.respond( diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 17d5216..f1deaca 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -351,7 +351,7 @@ paths: content: application/json: schema: - description: sdf + $ref: '#/components/schemas/400' /auth/passwordless: post: summary: Send a connexion link by email diff --git a/src/test/kotlin/integration/Register routes.kt b/src/test/kotlin/integration/Register routes.kt index 52c4ae5..510d0b2 100644 --- a/src/test/kotlin/integration/Register routes.kt +++ b/src/test/kotlin/integration/Register routes.kt @@ -29,7 +29,7 @@ class `Register routes` : BaseTest() { "birthday": "2001-01-01", "user":{ "username": "george-junior", - "password": "azerty" + "password": "Azerty123!" }, "email": "george-junior@gmail.com" } From 1ec1c59c8cce23a5183d9dbdb6e1aaeb75f3cd4c Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 8 Apr 2021 02:36:20 +0200 Subject: [PATCH 16/46] remove useless log --- src/main/kotlin/fr/dcproject/common/utils/Elastic.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/common/utils/Elastic.kt b/src/main/kotlin/fr/dcproject/common/utils/Elastic.kt index b5f98b7..ed71814 100644 --- a/src/main/kotlin/fr/dcproject/common/utils/Elastic.kt +++ b/src/main/kotlin/fr/dcproject/common/utils/Elastic.kt @@ -4,7 +4,6 @@ import com.jayway.jsonpath.JsonPath import com.jayway.jsonpath.PathNotFoundException import org.apache.http.util.EntityUtils import org.elasticsearch.client.Response -import org.slf4j.LoggerFactory fun Response.contentToString(): String { return EntityUtils.toString(this.entity) @@ -22,8 +21,6 @@ fun String.getJsonField(jsonPath: String): Int? { return try { JsonPath.read(this, jsonPath) } catch (e: PathNotFoundException) { - LoggerFactory.getLogger("fr.dcproject.utils.getJsonField") - .warn("No value for Json path ${JsonPath.compile(jsonPath).path}") null } } From eb399392c932d4983037694cc200fff7082e252f Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 8 Apr 2021 03:03:04 +0200 Subject: [PATCH 17/46] remove parallel run for tests --- build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 47f3fd0..1c304e8 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -197,7 +197,7 @@ val sourcesJar by tasks.registering(Jar::class) { tasks.test { useJUnit() useJUnitPlatform() - systemProperty("junit.jupiter.execution.parallel.enabled", true) + // systemProperty("junit.jupiter.execution.parallel.enabled", true) dependsOn(testSql) finalizedBy(tasks.jacocoTestReport) // report is always generated after tests run } From 9d3eeeb04b26250e3063836f76fef393600344e7 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 8 Apr 2021 17:55:05 +0200 Subject: [PATCH 18/46] Add validation on route ChangePasswordCitizenRequest --- .../fr/dcproject/common/validation/Password.kt | 2 +- .../component/citizen/routes/ChangeMyPassword.kt | 14 ++++++++++++-- src/main/resources/openapi.yaml | 4 ++++ src/test/kotlin/integration/Citizen routes.kt | 4 ++-- src/test/kotlin/integration/Login routes.kt | 2 +- src/test/kotlin/integration/steps/given/Citizen.kt | 4 ++-- .../kotlin/integration/steps/given/Workgroup.kt | 2 +- 7 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/common/validation/Password.kt b/src/main/kotlin/fr/dcproject/common/validation/Password.kt index b08bb7d..718d60b 100644 --- a/src/main/kotlin/fr/dcproject/common/validation/Password.kt +++ b/src/main/kotlin/fr/dcproject/common/validation/Password.kt @@ -10,7 +10,7 @@ fun ValidationBuilder.passwordScore(minScore: Int) = fun String.passwordScore(): Int { var score: Int = length val alphaNum = ('a'..'z').toList() + ('A'..'Z').toList() + ('0'..'9').toList() - val specialCount = (length - toList().intersect(alphaNum).size) + val specialCount = length - toList().intersect(alphaNum).size score += specialCount.let { if (it > 3) 3 else it } val hasAlphaLower = toList().intersect(('a'..'z').toList()).size.let { if (it > 2) 2 else it } diff --git a/src/main/kotlin/fr/dcproject/component/citizen/routes/ChangeMyPassword.kt b/src/main/kotlin/fr/dcproject/component/citizen/routes/ChangeMyPassword.kt index 6a360de..cca3586 100644 --- a/src/main/kotlin/fr/dcproject/component/citizen/routes/ChangeMyPassword.kt +++ b/src/main/kotlin/fr/dcproject/component/citizen/routes/ChangeMyPassword.kt @@ -1,7 +1,9 @@ package fr.dcproject.component.citizen.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest +import fr.dcproject.common.validation.passwordScore import fr.dcproject.component.auth.citizen import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.auth.database.UserRepository @@ -9,6 +11,7 @@ import fr.dcproject.component.auth.database.UserWithPassword import fr.dcproject.component.auth.mustBeAuth import fr.dcproject.component.citizen.CitizenAccessControl import fr.dcproject.component.citizen.database.CitizenRef +import io.konform.validation.Validation import io.ktor.application.call import io.ktor.auth.UserPasswordCredential import io.ktor.features.BadRequestException @@ -25,14 +28,21 @@ object ChangeMyPassword { @Location("/citizens/{citizen}/password/change") class ChangePasswordCitizenRequest(citizen: UUID) { val citizen = CitizenRef(citizen) - data class Input(val oldPassword: String, val newPassword: String) + data class Input(val oldPassword: String, val newPassword: String) { + fun validate() = Validation { + Input::newPassword { + passwordScore(15) + } + }.validate(this) + } } fun Route.changeMyPassword(ac: CitizenAccessControl, userRepository: UserRepository) { put { mustBeAuth() - ac.assert { canChangePassword(it.citizen, citizenOrNull) } val content = call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } + ac.assert { canChangePassword(it.citizen, citizenOrNull) } userRepository.findByCredentials(UserPasswordCredential(citizen.user.username, content.oldPassword)) ?: throw BadRequestException("Bad Password") userRepository.changePassword( UserWithPassword( diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index f1deaca..5800f17 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -484,6 +484,10 @@ paths: description: Password changed 400: description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' 404: diff --git a/src/test/kotlin/integration/Citizen routes.kt b/src/test/kotlin/integration/Citizen routes.kt index dfd2d26..62f9843 100644 --- a/src/test/kotlin/integration/Citizen routes.kt +++ b/src/test/kotlin/integration/Citizen routes.kt @@ -69,8 +69,8 @@ class `Citizen routes` : BaseTest() { `with body`( """ { - "oldPassword": "azerty", - "newPassword": "qwerty" + "oldPassword": "Azerty123!", + "newPassword": "Qwerty123!" } """ ) diff --git a/src/test/kotlin/integration/Login routes.kt b/src/test/kotlin/integration/Login routes.kt index 7336f9b..131b278 100644 --- a/src/test/kotlin/integration/Login routes.kt +++ b/src/test/kotlin/integration/Login routes.kt @@ -27,7 +27,7 @@ class `Login routes` : BaseTest() { """ { "username": "niels-bohr", - "password": "azerty" + "password": "Azerty123!" } """ ) diff --git a/src/test/kotlin/integration/steps/given/Citizen.kt b/src/test/kotlin/integration/steps/given/Citizen.kt index 951491c..730ae70 100644 --- a/src/test/kotlin/integration/steps/given/Citizen.kt +++ b/src/test/kotlin/integration/steps/given/Citizen.kt @@ -23,7 +23,7 @@ fun TestApplicationEngine.`Given I have citizen`( val user = UserForCreate( id = id.toUUID(), username = "$firstName-$lastName".toLowerCase(), - password = "azerty", + password = "Azerty123!", ) val citizen = CitizenForCreate( id = id.toUUID(), @@ -53,7 +53,7 @@ fun createCitizen(name: CitizenI.Name? = null, id: UUID = UUID.randomUUID()): Ci last ), email = "$first@fakeemail.com", - user = UserForCreate(username = username, password = "azerty") + user = UserForCreate(username = username, password = "Azerty123!") ).let { citizenRepository.insertWithUser(it) ?: error("Unable to create User") } diff --git a/src/test/kotlin/integration/steps/given/Workgroup.kt b/src/test/kotlin/integration/steps/given/Workgroup.kt index 33548c7..5871439 100644 --- a/src/test/kotlin/integration/steps/given/Workgroup.kt +++ b/src/test/kotlin/integration/steps/given/Workgroup.kt @@ -65,7 +65,7 @@ private fun createWorkgroup( .toLowerCase().replace(' ', '-') val user = UserForCreate( username = username, - password = "azerty", + password = "Azerty123!", ) CitizenForCreate( name = creatorName, From e473e62068d835e31234c5184eb7301a86c8209c Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 8 Apr 2021 18:08:48 +0200 Subject: [PATCH 19/46] remove CodeFactor & Codacy --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index e29db2d..488c3a8 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,5 @@ # DC Project -[![CodeFactor](https://www.codefactor.io/repository/github/flecomte/dc-project/badge?s=869dc426625a253a07bea95f9380e23fdb048b94)](https://www.codefactor.io/repository/github/flecomte/dc-project) -[![Codacy Badge](https://app.codacy.com/project/badge/Grade/0ec4fe63370148ca956974f90f8d55be)](https://www.codacy.com/gh/flecomte/dc-project/dashboard?utm_source=github.com&utm_medium=referral&utm_content=flecomte/dc-project&utm_campaign=Badge_Grade) [![Maintainability Rating](https://sonarcloud.io/api/project_badges/measure?project=dc-project&metric=sqale_rating)](https://sonarcloud.io/dashboard?id=dc-project) [![Tests](https://github.com/flecomte/dc-project/actions/workflows/tests.yml/badge.svg)](https://github.com/flecomte/dc-project/actions/workflows/tests.yml) From 13cdaaf01ab9c6b1ca354a1adefa28c6b7b07925 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 8 Apr 2021 22:25:43 +0200 Subject: [PATCH 20/46] Add validation on route FindCitizens --- build.gradle.kts | 6 +++++ .../component/citizen/routes/FindCitizens.kt | 24 ++++++++++++++++++- src/main/resources/openapi.yaml | 19 ++++++++++++++- .../sql/functions/article/find_articles.sql | 4 ++-- .../sql/functions/citizen/find_citizens.sql | 4 ++-- src/test/kotlin/integration/Citizen routes.kt | 15 +++++++++++- 6 files changed, 65 insertions(+), 7 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 1c304e8..36f4c7e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -326,6 +326,12 @@ tasks.register("testArticles", Test::class) { includeTags("article") } } +tasks.register("testCitizens", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("citizen") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/component/citizen/routes/FindCitizens.kt b/src/main/kotlin/fr/dcproject/component/citizen/routes/FindCitizens.kt index 6ca46f2..872c090 100644 --- a/src/main/kotlin/fr/dcproject/component/citizen/routes/FindCitizens.kt +++ b/src/main/kotlin/fr/dcproject/component/citizen/routes/FindCitizens.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.citizen.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.component.auth.citizenOrNull @@ -10,6 +11,10 @@ import fr.dcproject.component.citizen.database.CitizenRepository 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 @@ -27,11 +32,28 @@ object FindCitizens { val sort: String? = null, val direction: RepositoryI.Direction? = null, val search: String? = null - ) : PaginatedRequestI by PaginatedRequest(page, limit) + ) : PaginatedRequestI by PaginatedRequest(page, limit) { + fun validate() = Validation { + CitizensRequest::page { + minimum(1) + } + CitizensRequest::limit { + minimum(1) + maximum(50) + } + CitizensRequest::sort ifPresent { + enum( + "title", + "createdAt", + ) + } + }.validate(this) + } fun Route.findCitizen(ac: CitizenAccessControl, repo: CitizenRepository) { get { mustBeAuth() + it.validate().badRequestIfNotValid() val citizens = repo.find(it.page, it.limit, it.sort, it.direction, it.search) ac.assert { canView(citizens.result, citizenOrNull) } call.respond( diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 5800f17..af39955 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -395,7 +395,7 @@ paths: parameters: - $ref: '#/components/parameters/page' - $ref: '#/components/parameters/limit' - - $ref: '#/components/parameters/sort' + - $ref: '#/components/parameters/citizenSort' - $ref: '#/components/parameters/direction' - $ref: '#/components/parameters/search' responses: @@ -412,6 +412,12 @@ paths: type: array items: $ref: '#/components/schemas/CitizenListResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' /citizens/current: @@ -1418,6 +1424,17 @@ components: - createdAt - vote - popularity + citizenSort: + name: sort + in: query + description: The sort field name + example: createdAt + required: false + schema: + type: string + enum: + - title + - createdAt workgroupSort: name: sort in: query diff --git a/src/main/resources/sql/functions/article/find_articles.sql b/src/main/resources/sql/functions/article/find_articles.sql index 75164b2..ece47ee 100644 --- a/src/main/resources/sql/functions/article/find_articles.sql +++ b/src/main/resources/sql/functions/article/find_articles.sql @@ -45,7 +45,7 @@ begin case direction when 'asc' then case sort when 'title' then a.title - when 'created_at' then a.created_at::text + when 'createdAt' then a.created_at::text when 'vote' then ca.score::text when 'popularity' then ca.total::text else null @@ -54,7 +54,7 @@ begin case direction when 'desc' then case sort when 'title' then a.title - when 'created_at' then a.created_at::text + when 'createdAt' then a.created_at::text when 'vote' then ca.score::text when 'popularity' then ca.total::text end diff --git a/src/main/resources/sql/functions/citizen/find_citizens.sql b/src/main/resources/sql/functions/citizen/find_citizens.sql index c5f5a0f..a37a667 100644 --- a/src/main/resources/sql/functions/citizen/find_citizens.sql +++ b/src/main/resources/sql/functions/citizen/find_citizens.sql @@ -23,14 +23,14 @@ begin case direction when 'asc' then case sort when 'name' then (z.name->'first_name')::text - when 'created_at' then z.created_at::text + when 'createdAt' then z.created_at::text else null end end, case direction when 'desc' then case sort when 'name' then (z.name->'first_name')::text - when 'created_at' then z.created_at::text + when 'createdAt' then z.created_at::text end end desc, diff --git a/src/test/kotlin/integration/Citizen routes.kt b/src/test/kotlin/integration/Citizen routes.kt index 62f9843..95c445e 100644 --- a/src/test/kotlin/integration/Citizen routes.kt +++ b/src/test/kotlin/integration/Citizen routes.kt @@ -26,7 +26,7 @@ class `Citizen routes` : BaseTest() { fun `I can get Citizens information`() { withIntegrationApplication { `Given I have citizen`("Jean", "Perrin", id = "5267a5c6-af42-4a02-aa2b-6b71d2e43973") - `When I send a GET request`("/citizens") { + `When I send a GET request`("/citizens?page=1&limit=5&sort=createdAt") { `authenticated as`("Jean", "Perrin") } `Then the response should be` OK and { `And the response should not be null`() @@ -34,6 +34,19 @@ class `Citizen routes` : BaseTest() { } } + @Test + @Tag("BadRequest") + fun `I cannot get Citizens information with wrong request`() { + withIntegrationApplication { + `Given I have citizen`("Jean", "Perrin", id = "5267a5c6-af42-4a02-aa2b-6b71d2e43973") + `When I send a GET request`("/citizens?page=1&limit=5&sort=created_at", Validate.ALL - Validate.REQUEST_PARAM) { + `authenticated as`("Jean", "Perrin") + } `Then the response should be` BadRequest and { + `And the response should not be null`() + } + } + } + @Test fun `I can get specific Citizen information`() { withIntegrationApplication { From a07b19a3cb97b85f6a022fe86bdb6225b8fa40bd Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 9 Apr 2021 00:58:35 +0200 Subject: [PATCH 21/46] Add validation on route CreateCommentArticle --- build.gradle.kts | 6 +++ .../article/routes/CreateCommentArticle.kt | 44 ++++++++++++------- src/main/resources/openapi.yaml | 6 +++ .../integration/Comment articles routes.kt | 30 ++++++++++++- 4 files changed, 68 insertions(+), 18 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 36f4c7e..2f95015 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -332,6 +332,12 @@ tasks.register("testCitizens", Test::class) { includeTags("citizen") } } +tasks.register("testComments", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("comment") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/component/comment/article/routes/CreateCommentArticle.kt b/src/main/kotlin/fr/dcproject/component/comment/article/routes/CreateCommentArticle.kt index fd10799..493f9aa 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/article/routes/CreateCommentArticle.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/article/routes/CreateCommentArticle.kt @@ -1,6 +1,6 @@ package fr.dcproject.component.comment.article.routes -import fr.dcproject.common.response.toOutput +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest import fr.dcproject.component.article.database.ArticleRef @@ -12,6 +12,9 @@ import fr.dcproject.component.comment.article.routes.CreateCommentArticle.PostAr import fr.dcproject.component.comment.generic.CommentAccessControl import fr.dcproject.component.comment.generic.database.CommentForUpdate import fr.dcproject.component.comment.toOutput +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -26,27 +29,36 @@ object CreateCommentArticle { @Location("/articles/{article}/comments") class PostArticleCommentRequest(article: UUID) { val article = ArticleRef(article) - class Input(val content: String) + class Input(val content: String) { + fun validate() = Validation { + Input::content { + minLength(20) + maxLength(6000) + } + }.validate(this) + } } fun Route.createCommentArticle(repo: CommentArticleRepository, ac: CommentAccessControl) { post { mustBeAuth() - call.receiveOrBadRequest().run { - CommentForUpdate( - target = it.article, - createdBy = citizen, - content = content - ) - }.let { comment -> - ac.assert { canCreate(comment, citizenOrNull) } - repo.comment(comment) + call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } + .run { + CommentForUpdate( + target = it.article, + createdBy = citizen, + content = content + ) + }.let { comment -> + ac.assert { canCreate(comment, citizenOrNull) } + repo.comment(comment) - call.respond( - HttpStatusCode.Created, - comment.toOutput() - ) - } + call.respond( + HttpStatusCode.Created, + comment.toOutput() + ) + } } } } diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index af39955..810cb48 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -563,6 +563,12 @@ paths: application/json: schema: $ref: '#/components/schemas/CommentResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' /comments/{comment}: diff --git a/src/test/kotlin/integration/Comment articles routes.kt b/src/test/kotlin/integration/Comment articles routes.kt index 6c7c00e..0962e86 100644 --- a/src/test/kotlin/integration/Comment articles routes.kt +++ b/src/test/kotlin/integration/Comment articles routes.kt @@ -1,6 +1,8 @@ package integration import fr.dcproject.component.citizen.database.CitizenI.Name +import integration.steps.`when`.Validate.ALL +import integration.steps.`when`.Validate.REQUEST_BODY import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` import integration.steps.`when`.`When I send a PUT request` @@ -13,6 +15,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.`Then the response should be` import integration.steps.then.and +import io.ktor.http.HttpStatusCode.Companion.BadRequest import io.ktor.http.HttpStatusCode.Companion.Created import io.ktor.http.HttpStatusCode.Companion.OK import org.junit.jupiter.api.Tag @@ -33,14 +36,37 @@ class `Comment articles routes` : BaseTest() { `with body`( """ { - "content": "Hello mister" + "content": "Hello mister MARABOUTCHA" } """ ) } `Then the response should be` Created and { `And the response should not be null`() `And the response should contain`("$.target.id", "aa16c635-28da-46f0-9a89-934eef88c7ca") - `And the response should contain`("$.content", "Hello mister") + `And the response should contain`("$.content", "Hello mister MARABOUTCHA") + } + } + } + + @Test + @Tag("BadRequest") + fun `I cannot comment article with bad request`() { + withIntegrationApplication { + `Given I have citizen`("Michael", "Faraday") + `Given I have article`(id = "aa16c635-28da-46f0-9a89-934eef88c7ca") + `When I send a POST request`("/articles/aa16c635-28da-46f0-9a89-934eef88c7ca/comments", ALL - REQUEST_BODY) { + `authenticated as`("Michael", "Faraday") + `with body`( + """ + { + "content": "To small content" + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".content") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 20 characters") } } } From fb7b07340ae5677f6e3cb40902e9ecd984a62ff8 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 9 Apr 2021 01:09:09 +0200 Subject: [PATCH 22/46] Improve test of password validation --- src/test/kotlin/unit/Password Validation.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/kotlin/unit/Password Validation.kt b/src/test/kotlin/unit/Password Validation.kt index e39e500..242d0de 100644 --- a/src/test/kotlin/unit/Password Validation.kt +++ b/src/test/kotlin/unit/Password Validation.kt @@ -27,6 +27,7 @@ internal class `Password Validation` { "12abCD-+".passwordScore() `should be equal to` 18 "Abcde12!".passwordScore() `should be equal to` 15 "Hello world".passwordScore() `should be equal to` 16 + "hello WORLD".passwordScore() `should be equal to` 17 } @Test From 875d0bfffac5cd6cbdd1973677344f32726a4688 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 9 Apr 2021 01:40:22 +0200 Subject: [PATCH 23/46] Add test 404 for GetArticle route --- .../application/http/HttpStatusError.kt | 4 +--- src/main/resources/openapi.yaml | 23 ++++++++++++++++++- src/test/kotlin/integration/Article routes.kt | 12 ++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt b/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt index 2d4f1f2..d515f24 100644 --- a/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt +++ b/src/main/kotlin/fr/dcproject/application/http/HttpStatusError.kt @@ -14,10 +14,8 @@ import java.util.concurrent.CompletionException class HttpError( statusCode: HttpStatusCode, - val cause: Throwable? = null, - val type: String? = null, + cause: Throwable? = null, val title: String = cause?.message ?: statusCode.description, - val detail: String? = null, ) { val statusCode: Int = statusCode.value } diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 810cb48..6df61e5 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -164,6 +164,13 @@ paths: application/json: schema: $ref: '#/components/schemas/400' + 404: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/404' + /articles/{article}/versions: parameters: - $ref: '#/components/parameters/article' @@ -2278,7 +2285,7 @@ components: example: MASTER 400: - description: noting + description: Bad Request required: - title - invalidParams @@ -2304,6 +2311,20 @@ components: type: string example: 'Cannot be null' + 404: + description: Not Found + required: + - title + - statusCode + additionalProperties: false + properties: + statusCode: + type: integer + example: 404 + title: + type: string + example: Bad Request + securitySchemes: JWTAuth: type: http diff --git a/src/test/kotlin/integration/Article routes.kt b/src/test/kotlin/integration/Article routes.kt index fa0cc82..96e6d60 100644 --- a/src/test/kotlin/integration/Article routes.kt +++ b/src/test/kotlin/integration/Article routes.kt @@ -22,6 +22,7 @@ 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 +import io.ktor.http.HttpStatusCode.Companion.NotFound import io.ktor.http.HttpStatusCode.Companion.OK import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Tags @@ -82,6 +83,17 @@ class `Article routes` : BaseTest() { } } + @Test + fun `I cannot get article with id doesn't exist`() { + withIntegrationApplication { + `When I send a GET request`("/articles/635fe2e8-2dbc-4c80-b306-101d38a4ab23") `Then the response should be` NotFound and { + `And the response should not be null`() + `And the response should contain`("$.title", "Article 635fe2e8-2dbc-4c80-b306-101d38a4ab23 not found") + `And the response should contain`("$.statusCode", 404) + } + } + } + @Test @Tag("BadRequest") fun `I cannot get article by id with wrong id format`() { From f5c1aa29e89a4fb5311403dbbba992b64a5bea6e Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 9 Apr 2021 18:06:32 +0200 Subject: [PATCH 24/46] Add validation on route GetArticleComments --- .../database/CommentArticleRepository.kt | 15 ++--------- .../article/routes/GetArticleComments.kt | 26 +++++++++++++++++-- .../database/CommentConstitutionRepository.kt | 5 ++-- .../generic/database/CommentRepository.kt | 7 +++-- src/main/resources/openapi.yaml | 6 +++++ .../comment/find_comments_by_target.sql | 2 +- .../integration/Comment articles routes.kt | 20 +++++++++++++- 7 files changed, 57 insertions(+), 24 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/comment/article/database/CommentArticleRepository.kt b/src/main/kotlin/fr/dcproject/component/comment/article/database/CommentArticleRepository.kt index 4c7f1a4..a659e87 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/article/database/CommentArticleRepository.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/article/database/CommentArticleRepository.kt @@ -41,7 +41,7 @@ class CommentArticleRepository(requester: Requester) : CommentRepositoryAbs> { return requester .getFunction("find_comments_by_target") @@ -49,18 +49,7 @@ class CommentArticleRepository(requester: Requester) : CommentRepositoryAbs> } - - enum class Sort(val sql: String) { - CREATED_AT("created_at"), - VOTES("votes"); - - companion object { - fun fromString(string: String): Sort? { - return values().firstOrNull { it.sql == string } - } - } - } } diff --git a/src/main/kotlin/fr/dcproject/component/comment/article/routes/GetArticleComments.kt b/src/main/kotlin/fr/dcproject/component/comment/article/routes/GetArticleComments.kt index 896cad6..3ec2e88 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/article/routes/GetArticleComments.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/article/routes/GetArticleComments.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.comment.article.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.component.article.database.ArticleRef @@ -9,6 +10,10 @@ import fr.dcproject.component.comment.generic.CommentAccessControl import fr.dcproject.component.comment.toOutput import fr.dcproject.routes.PaginatedRequest import fr.dcproject.routes.PaginatedRequestI +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.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -26,14 +31,31 @@ object GetArticleComments { page: Int = 1, limit: Int = 50, val search: String? = null, - sort: String = CommentArticleRepository.Sort.CREATED_AT.sql + val sort: String = "createdAt" ) : PaginatedRequestI by PaginatedRequest(page, limit) { val article = ArticleRef(article) - val sort: CommentArticleRepository.Sort = CommentArticleRepository.Sort.fromString(sort) ?: CommentArticleRepository.Sort.CREATED_AT + + fun validate() = Validation { + ArticleCommentsRequest::page { + minimum(1) + } + ArticleCommentsRequest::limit { + minimum(1) + maximum(50) + } + ArticleCommentsRequest::sort ifPresent { + enum( + "votes", + "createdAt", + ) + } + }.validate(this) } fun Route.getArticleComments(repo: CommentArticleRepository, ac: CommentAccessControl) { get { + it.validate().badRequestIfNotValid() + val comments = repo.findByTarget(it.article, it.page, it.limit, it.sort) if (comments.result.isNotEmpty()) { ac.assert { canView(comments.result, citizenOrNull) } diff --git a/src/main/kotlin/fr/dcproject/component/comment/constitution/database/CommentConstitutionRepository.kt b/src/main/kotlin/fr/dcproject/component/comment/constitution/database/CommentConstitutionRepository.kt index e0d03fa..2b48c0c 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/constitution/database/CommentConstitutionRepository.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/constitution/database/CommentConstitutionRepository.kt @@ -5,7 +5,6 @@ import fr.dcproject.common.entity.TargetI import fr.dcproject.component.citizen.database.CitizenCreator import fr.dcproject.component.citizen.database.CitizenCreatorI import fr.dcproject.component.citizen.database.CitizenI -import fr.dcproject.component.comment.article.database.CommentArticleRepository import fr.dcproject.component.comment.generic.database.CommentForView import fr.dcproject.component.comment.generic.database.CommentRepositoryAbs import fr.dcproject.component.constitution.database.ConstitutionRef @@ -41,7 +40,7 @@ class CommentConstitutionRepository(requester: Requester) : CommentRepositoryAbs target: EntityI, page: Int, limit: Int, - sort: CommentArticleRepository.Sort + sort: String ): Paginated> { return requester.run { getFunction("find_comments_by_target") @@ -49,7 +48,7 @@ class CommentConstitutionRepository(requester: Requester) : CommentRepositoryAbs page, limit, "target_id" to target.id, - "sort" to sort.sql + "sort" to sort ) as Paginated> } diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt index 8cf12c7..6443819 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt @@ -6,7 +6,6 @@ import fr.dcproject.common.entity.TargetRef import fr.dcproject.component.citizen.database.CitizenCreator import fr.dcproject.component.citizen.database.CitizenCreatorI import fr.dcproject.component.citizen.database.CitizenI -import fr.dcproject.component.comment.article.database.CommentArticleRepository import fr.postgresjson.connexion.Paginated import fr.postgresjson.connexion.Requester import fr.postgresjson.repository.RepositoryI @@ -49,7 +48,7 @@ abstract class CommentRepositoryAbs(override var requester: Request target: EntityI, page: Int = 1, limit: Int = 50, - sort: CommentArticleRepository.Sort = CommentArticleRepository.Sort.CREATED_AT + sort: String = "createdAt" ): Paginated> { return findByTarget(target.id, page, limit, sort) } @@ -58,7 +57,7 @@ abstract class CommentRepositoryAbs(override var requester: Request targetId: UUID, page: Int = 1, limit: Int = 50, - sort: CommentArticleRepository.Sort = CommentArticleRepository.Sort.CREATED_AT + sort: String = "createdAt" ): Paginated> { return requester.run { getFunction("find_comments_by_target") @@ -66,7 +65,7 @@ abstract class CommentRepositoryAbs(override var requester: Request page, limit, "target_id" to targetId, - "sort" to sort.sql + "sort" to sort ) as Paginated> } diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 6df61e5..ab15282 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -544,6 +544,12 @@ paths: type: array items: $ref: '#/components/schemas/CommentResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' post: security: - JWTAuth: [ ] diff --git a/src/main/resources/sql/functions/comment/find_comments_by_target.sql b/src/main/resources/sql/functions/comment/find_comments_by_target.sql index 8a78bdc..a35fd9c 100644 --- a/src/main/resources/sql/functions/comment/find_comments_by_target.sql +++ b/src/main/resources/sql/functions/comment/find_comments_by_target.sql @@ -26,7 +26,7 @@ begin else null end desc, case sort - when 'created_at' then com.created_at::text + when 'createdAt' then com.created_at::text else null end desc, com.created_at desc diff --git a/src/test/kotlin/integration/Comment articles routes.kt b/src/test/kotlin/integration/Comment articles routes.kt index 0962e86..deebc10 100644 --- a/src/test/kotlin/integration/Comment articles routes.kt +++ b/src/test/kotlin/integration/Comment articles routes.kt @@ -3,6 +3,7 @@ package integration import fr.dcproject.component.citizen.database.CitizenI.Name import integration.steps.`when`.Validate.ALL import integration.steps.`when`.Validate.REQUEST_BODY +import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` import integration.steps.`when`.`When I send a PUT request` @@ -78,7 +79,7 @@ class `Comment articles routes` : BaseTest() { `Given I have citizen`("Enrico", "Fermi") `Given I have article`(id = "6166c078-ca97-4366-b0aa-2a5cd558c78a") `Given I have comment on article`(article = "6166c078-ca97-4366-b0aa-2a5cd558c78a", createdBy = Name("Enrico", "Fermi")) - `When I send a GET request`("/articles/6166c078-ca97-4366-b0aa-2a5cd558c78a/comments") { + `When I send a GET request`("/articles/6166c078-ca97-4366-b0aa-2a5cd558c78a/comments?page=1&limit=40&sort=votes") { `authenticated as`("Enrico", "Fermi") } `Then the response should be` OK and { `And the response should not be null`() @@ -87,6 +88,23 @@ class `Comment articles routes` : BaseTest() { } } + @Test + @Tag("BadRequest") + fun `I cannot get all comment on article with wrong parameters`() { + withIntegrationApplication { + `Given I have citizen`("Enrico", "Fermi") + `Given I have article`(id = "6166c078-ca97-4366-b0aa-2a5cd558c78a") + `Given I have comment on article`(article = "6166c078-ca97-4366-b0aa-2a5cd558c78a", createdBy = Name("Enrico", "Fermi")) + `When I send a GET request`("/articles/6166c078-ca97-4366-b0aa-2a5cd558c78a/comments?page=1&limit=40&sort=wrong", ALL - REQUEST_PARAM) { + `authenticated as`("Enrico", "Fermi") + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[*].name", ".sort") + `And the response should contain`("$.invalidParams[*].reason", "must be one of: 'votes', 'createdAt'") + } + } + } + /* TODO add votes */ @Test fun `I can get all comment on article sorted by votes`() { From 34513e25b62606907f16a73aac55a7df45b0384f Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 9 Apr 2021 18:39:03 +0200 Subject: [PATCH 25/46] Add validation on route CreateConstitutionComment & GetConstitutionCommentRequest --- .../routes/CreateConstitutionComment.kt | 44 ++++++++----- .../routes/GetConstitutionComment.kt | 33 +++++++++- src/main/resources/openapi.yaml | 43 +++++++++++-- .../Comment constitutions routes.kt | 63 ++++++++++++++++++- 4 files changed, 162 insertions(+), 21 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/comment/constitution/routes/CreateConstitutionComment.kt b/src/main/kotlin/fr/dcproject/component/comment/constitution/routes/CreateConstitutionComment.kt index cf368e4..0b700f0 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/constitution/routes/CreateConstitutionComment.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/constitution/routes/CreateConstitutionComment.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.comment.constitution.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest @@ -12,6 +13,9 @@ import fr.dcproject.component.comment.generic.CommentAccessControl import fr.dcproject.component.comment.generic.database.CommentForUpdate import fr.dcproject.component.comment.toOutput import fr.dcproject.component.constitution.database.ConstitutionRef +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -26,27 +30,37 @@ object CreateConstitutionComment { @Location("/constitutions/{constitution}/comments") class CreateConstitutionCommentRequest(constitution: UUID) { val constitution = ConstitutionRef(constitution) - class Input(val content: String) + class Input(val content: String) { + fun validate() = Validation { + Input::content { + minLength(20) + maxLength(6000) + } + }.validate(this) + } } fun Route.createConstitutionComment(repo: CommentConstitutionRepository, ac: CommentAccessControl) { post { mustBeAuth() - call.receiveOrBadRequest().run { - CommentForUpdate( - target = it.constitution, - createdBy = citizen, - content = content - ) - }.let { comment -> - ac.assert { canCreate(comment, citizenOrNull) } - repo.comment(comment) - call.respond( - HttpStatusCode.Created, - comment.toOutput() - ) - } + call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } + .run { + CommentForUpdate( + target = it.constitution, + createdBy = citizen, + content = content + ) + }.let { comment -> + ac.assert { canCreate(comment, citizenOrNull) } + repo.comment(comment) + + call.respond( + HttpStatusCode.Created, + comment.toOutput() + ) + } } } } diff --git a/src/main/kotlin/fr/dcproject/component/comment/constitution/routes/GetConstitutionComment.kt b/src/main/kotlin/fr/dcproject/component/comment/constitution/routes/GetConstitutionComment.kt index 40ae7ef..47d417e 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/constitution/routes/GetConstitutionComment.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/constitution/routes/GetConstitutionComment.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.comment.constitution.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.component.auth.citizenOrNull @@ -7,6 +8,12 @@ import fr.dcproject.component.comment.constitution.database.CommentConstitutionR import fr.dcproject.component.comment.generic.CommentAccessControl import fr.dcproject.component.comment.toOutput import fr.dcproject.component.constitution.database.ConstitutionRef +import fr.dcproject.routes.PaginatedRequest +import fr.dcproject.routes.PaginatedRequestI +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.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -19,12 +26,36 @@ import java.util.UUID @KtorExperimentalLocationsAPI object GetConstitutionComment { @Location("/constitutions/{constitution}/comments") - class GetConstitutionCommentRequest(constitution: UUID) { + class GetConstitutionCommentRequest( + constitution: UUID, + page: Int = 1, + limit: Int = 50, + val search: String? = null, + val sort: String = "createdAt" + ) : PaginatedRequestI by PaginatedRequest(page, limit) { val constitution = ConstitutionRef(constitution) + + fun validate() = Validation { + GetConstitutionCommentRequest::page { + minimum(1) + } + GetConstitutionCommentRequest::limit { + minimum(1) + maximum(50) + } + GetConstitutionCommentRequest::sort ifPresent { + enum( + "votes", + "createdAt", + ) + } + }.validate(this) } fun Route.getConstitutionComment(repo: CommentConstitutionRepository, ac: CommentAccessControl) { get { + it.validate().badRequestIfNotValid() + val comments = repo.findByTarget(it.constitution) ac.assert { canView(comments.result, citizenOrNull) } call.respond( diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index ab15282..c0a4348 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -522,13 +522,13 @@ paths: in: query required: false example: - - created_at + - createdAt - votes schema: type: string - default: created_at + default: createdAt enum: - - created_at + - createdAt - votes responses: 200: @@ -707,13 +707,42 @@ paths: tags: - comment - constitution + parameters: + - $ref: '#/components/parameters/page' + - $ref: '#/components/parameters/limit' + - $ref: '#/components/parameters/search' + - name: sort + in: query + required: false + example: + - createdAt + - votes + schema: + type: string + default: createdAt + enum: + - createdAt + - votes responses: 200: description: Return Comment and children content: application/json: schema: - $ref: '#/components/schemas/CommentResponse' + allOf: + - $ref: '#/components/schemas/Paginated' + - type: object + properties: + result: + type: array + items: + $ref: '#/components/schemas/CommentResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' post: security: - JWTAuth: [] @@ -739,6 +768,12 @@ paths: application/json: schema: $ref: '#/components/schemas/CommentResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' diff --git a/src/test/kotlin/integration/Comment constitutions routes.kt b/src/test/kotlin/integration/Comment constitutions routes.kt index a999082..523a0be 100644 --- a/src/test/kotlin/integration/Comment constitutions routes.kt +++ b/src/test/kotlin/integration/Comment constitutions routes.kt @@ -1,6 +1,9 @@ package integration import fr.dcproject.component.citizen.database.CitizenI.Name +import integration.steps.`when`.Validate +import integration.steps.`when`.Validate.ALL +import integration.steps.`when`.Validate.REQUEST_BODY import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` import integration.steps.`when`.`with body` @@ -13,6 +16,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.`Then the response should be` import integration.steps.then.and +import io.ktor.http.HttpStatusCode.Companion.BadRequest import io.ktor.http.HttpStatusCode.Companion.Created import io.ktor.http.HttpStatusCode.Companion.OK import org.junit.jupiter.api.Tag @@ -33,12 +37,69 @@ class `Comment constitutions routes` : BaseTest() { `with body`( """ { - "content": "Hello mister" + "content": "Hello mister MARABOUTCHA" } """ ) } `Then the response should be` Created and { `And the response should not be null`() + `And the response should contain`("$.target.id", "1707c287-a472-4a62-89f2-9e85030e915c") + `And the response should contain`("$.content", "Hello mister MARABOUTCHA") + } + } + } + + @Test + @Tag("BadRequest") + fun `I cannot comment constitution with bad request`() { + withIntegrationApplication { + `Given I have citizen`("Nicolas", "Copernic") + `Given I have constitution`(id = "aa16c635-28da-46f0-9a89-934eef88c7ca") + `When I send a POST request`("/constitutions/aa16c635-28da-46f0-9a89-934eef88c7ca/comments", ALL - REQUEST_BODY) { + `authenticated as`("Nicolas", "Copernic") + `with body`( + """ + { + "content": "To small content" + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".content") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 20 characters") + } + } + } + + @Test + fun `I can get all comment on constitution`() { + withIntegrationApplication { + `Given I have citizen`("Enrico", "Fermi") + `Given I have constitution`(id = "6166c078-ca97-4366-b0aa-2a5cd558c78a") + `Given I have comment on constitution`(constitution = "6166c078-ca97-4366-b0aa-2a5cd558c78a", createdBy = Name("Enrico", "Fermi")) + `When I send a GET request`("/constitutions/6166c078-ca97-4366-b0aa-2a5cd558c78a/comments?page=1&limit=40&sort=votes") { + `authenticated as`("Enrico", "Fermi") + } `Then the response should be` OK and { + `And the response should not be null`() + `And the response should contain`("$.result[0].target.id", "6166c078-ca97-4366-b0aa-2a5cd558c78a") + } + } + } + + @Test + @Tag("BadRequest") + fun `I cannot get all comment on constitution with wrong parameters`() { + withIntegrationApplication { + `Given I have citizen`("Enrico", "Fermi") + `Given I have constitution`(id = "6166c078-ca97-4366-b0aa-2a5cd558c78a") + `Given I have comment on constitution`(constitution = "6166c078-ca97-4366-b0aa-2a5cd558c78a", createdBy = Name("Enrico", "Fermi")) + `When I send a GET request`("/constitutions/6166c078-ca97-4366-b0aa-2a5cd558c78a/comments?page=1&limit=40&sort=wrong", ALL - Validate.REQUEST_PARAM) { + `authenticated as`("Enrico", "Fermi") + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[*].name", ".sort") + `And the response should contain`("$.invalidParams[*].reason", "must be one of: 'votes', 'createdAt'") } } } From 27e405c585386656ce87e1041fd7d7b048a40c0f Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 9 Apr 2021 18:43:59 +0200 Subject: [PATCH 26/46] Move tests --- .../integration/Comment articles routes.kt | 42 ---------------- src/test/kotlin/integration/Comment routes.kt | 48 +++++++++++++++++++ 2 files changed, 48 insertions(+), 42 deletions(-) diff --git a/src/test/kotlin/integration/Comment articles routes.kt b/src/test/kotlin/integration/Comment articles routes.kt index deebc10..517b04c 100644 --- a/src/test/kotlin/integration/Comment articles routes.kt +++ b/src/test/kotlin/integration/Comment articles routes.kt @@ -6,7 +6,6 @@ import integration.steps.`when`.Validate.REQUEST_BODY import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` -import integration.steps.`when`.`When I send a PUT request` import integration.steps.`when`.`with body` import integration.steps.given.`Given I have article` import integration.steps.given.`Given I have citizen` @@ -137,45 +136,4 @@ class `Comment articles routes` : BaseTest() { } } } - - @Test - fun `I can edit comment`() { - withIntegrationApplication { - `Given I have citizen`("Hubert", "Reeves") - `Given I have article`(id = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1") - `Given I have comment on article`(article = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1", createdBy = Name("Hubert", "Reeves"), id = "fd30d20f-656c-42c6-8955-f61c04537464") - `When I send a PUT request`("/comments/fd30d20f-656c-42c6-8955-f61c04537464") { - `authenticated as`("Hubert", "Reeves") - `with body`( - """ - { - "content": "Hello boy" - } - """ - ) - } `Then the response should be` OK and { - `And the response should not be null`() - `And the response should contain`("$.content", "Hello boy") - } - } - } - - @Test - fun `I can get comment by its ID`() { - withIntegrationApplication { - `Given I have citizen`("Alfred", "Kastler") - `Given I have article`(id = "3897465b-19d2-43a0-86ea-1e29dbb11ec9") - `Given I have comment on article`( - article = "3897465b-19d2-43a0-86ea-1e29dbb11ec9", - createdBy = Name("Alfred", "Kastler"), - id = "edd296a8-fc7a-4717-a2bb-9f035ceca3c2", - content = "Hello boy" - ) - `When I send a GET request`("/comments/edd296a8-fc7a-4717-a2bb-9f035ceca3c2") { - } `Then the response should be` OK and { - `And the response should not be null`() - `And the response should contain`("$.content", "Hello boy") - } - } - } } diff --git a/src/test/kotlin/integration/Comment routes.kt b/src/test/kotlin/integration/Comment routes.kt index 1a1143e..15ff4cf 100644 --- a/src/test/kotlin/integration/Comment routes.kt +++ b/src/test/kotlin/integration/Comment routes.kt @@ -1,10 +1,14 @@ package integration +import fr.dcproject.component.citizen.database.CitizenI import integration.steps.`when`.`When I send a GET request` +import integration.steps.`when`.`When I send a PUT request` +import integration.steps.`when`.`with body` import integration.steps.given.`Given I have article` import integration.steps.given.`Given I have citizen` import integration.steps.given.`Given I have comment on article` import integration.steps.given.`authenticated as` +import integration.steps.then.`And the response should contain` import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` import integration.steps.then.and @@ -30,4 +34,48 @@ class `Comment routes` : BaseTest() { } } } + + @Test + fun `I can edit comment`() { + withIntegrationApplication { + `Given I have citizen`("Hubert", "Reeves") + `Given I have article`(id = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1") + `Given I have comment on article`(article = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1", createdBy = CitizenI.Name( + "Hubert", + "Reeves" + ), id = "fd30d20f-656c-42c6-8955-f61c04537464") + `When I send a PUT request`("/comments/fd30d20f-656c-42c6-8955-f61c04537464") { + `authenticated as`("Hubert", "Reeves") + `with body`( + """ + { + "content": "Hello boy" + } + """ + ) + } `Then the response should be` OK and { + `And the response should not be null`() + `And the response should contain`("$.content", "Hello boy") + } + } + } + + @Test + fun `I can get comment by its ID`() { + withIntegrationApplication { + `Given I have citizen`("Alfred", "Kastler") + `Given I have article`(id = "3897465b-19d2-43a0-86ea-1e29dbb11ec9") + `Given I have comment on article`( + article = "3897465b-19d2-43a0-86ea-1e29dbb11ec9", + createdBy = CitizenI.Name("Alfred", "Kastler"), + id = "edd296a8-fc7a-4717-a2bb-9f035ceca3c2", + content = "Hello boy" + ) + `When I send a GET request`("/comments/edd296a8-fc7a-4717-a2bb-9f035ceca3c2") { + } `Then the response should be` OK and { + `And the response should not be null`() + `And the response should contain`("$.content", "Hello boy") + } + } + } } From 8223dd21bbd6141280a18a04e9efdc2bd2f87810 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sat, 10 Apr 2021 01:16:09 +0200 Subject: [PATCH 27/46] Add validation on route CreateComments & EditComment rename POST /comments/{comment}/children method edit and create comment of repository return edited/created comment --- .../comment/generic/database/Comment.kt | 6 +- .../generic/database/CommentRepository.kt | 42 ++++----- .../comment/generic/routes/CreateComment.kt | 63 +++++++++++++ .../generic/routes/CreateCommentChildren.kt | 47 ---------- .../comment/generic/routes/EditComment.kt | 44 ++++++--- .../comment/generic/routes/install.kt | 2 +- src/main/resources/openapi.yaml | 42 ++++++++- .../sql/functions/comment/comment.sql | 5 +- .../sql/functions/comment/edit_comment.sql | 4 +- src/test/kotlin/integration/Comment routes.kt | 94 +++++++++++++++++-- .../kotlin/integration/steps/given/Comment.kt | 65 ++++++++++--- 11 files changed, 307 insertions(+), 107 deletions(-) create mode 100644 src/main/kotlin/fr/dcproject/component/comment/generic/routes/CreateComment.kt delete mode 100644 src/main/kotlin/fr/dcproject/component/comment/generic/routes/CreateCommentChildren.kt diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/database/Comment.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/database/Comment.kt index e18cdf6..fdb95f0 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/generic/database/Comment.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/generic/database/Comment.kt @@ -63,12 +63,14 @@ open class CommentForUpdate( constructor( createdBy: C, parent: CommentParent, - content: String + content: String, + id: UUID? = null, ) : this( createdBy = createdBy, parent = parent, target = parent.target, - content = content + content = content, + id = id ?: UUID.randomUUID(), ) } diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt index 6443819..b5fdf8a 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt @@ -58,35 +58,29 @@ abstract class CommentRepositoryAbs(override var requester: Request page: Int = 1, limit: Int = 50, sort: String = "createdAt" - ): Paginated> { - return requester.run { - getFunction("find_comments_by_target") - .select>( - page, - limit, - "target_id" to targetId, - "sort" to sort - ) - as Paginated> - } - } + ): Paginated> = requester + .getFunction("find_comments_by_target") + .select>( + page, + limit, + "target_id" to targetId, + "sort" to sort + ) as Paginated> - fun comment(comment: CommentForUpdate) { - requester - .getFunction("comment") - .sendQuery( - "reference" to comment.target.reference, - "resource" to comment - ) - } + fun comment(comment: CommentForUpdate): CommentForView = requester + .getFunction("comment") + .selectOne( + "reference" to comment.target.reference, + "resource" to comment + )!! - fun edit(comment: CommentForUpdate) { - requester + fun edit(comment: CommentForUpdate): CommentForView { + return requester .getFunction("edit_comment") - .sendQuery( + .selectOne( "id" to comment.id, "content" to comment.content - ) + )!! } } diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/CreateComment.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/CreateComment.kt new file mode 100644 index 0000000..6158794 --- /dev/null +++ b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/CreateComment.kt @@ -0,0 +1,63 @@ +package fr.dcproject.component.comment.generic.routes + +import fr.dcproject.application.http.badRequestIfNotValid +import fr.dcproject.common.security.assert +import fr.dcproject.common.utils.receiveOrBadRequest +import fr.dcproject.component.auth.citizen +import fr.dcproject.component.auth.citizenOrNull +import fr.dcproject.component.auth.mustBeAuth +import fr.dcproject.component.comment.generic.CommentAccessControl +import fr.dcproject.component.comment.generic.database.CommentForUpdate +import fr.dcproject.component.comment.generic.database.CommentRef +import fr.dcproject.component.comment.generic.database.CommentRepository +import fr.dcproject.component.comment.toOutput +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength +import io.ktor.application.call +import io.ktor.features.NotFoundException +import io.ktor.http.HttpStatusCode +import io.ktor.locations.KtorExperimentalLocationsAPI +import io.ktor.locations.Location +import io.ktor.locations.post +import io.ktor.response.respond +import io.ktor.routing.Route +import java.util.UUID + +@KtorExperimentalLocationsAPI +object CreateComment { + @Location("/comments/{comment}") + class CreateCommentRequest(comment: UUID) { + val comment = CommentRef(comment) + class Input(val content: String) { + fun validate() = Validation { + Input::content { + minLength(20) + maxLength(6000) + } + }.validate(this) + } + } + + fun Route.createCommentChildren(repo: CommentRepository, ac: CommentAccessControl) { + post { + mustBeAuth() + + call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } + .run { + val parent = repo.findById(it.comment.id) ?: throw NotFoundException("Comment not found") + CommentForUpdate( + content = content, + createdBy = citizen, + target = parent.target, + parent = parent, + ) + }.let { newComment -> + ac.assert { canCreate(newComment, citizenOrNull) } + repo.comment(newComment) + call.respond(HttpStatusCode.Created, newComment.toOutput()) + } + } + } +} diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/CreateCommentChildren.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/CreateCommentChildren.kt deleted file mode 100644 index b95d2cd..0000000 --- a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/CreateCommentChildren.kt +++ /dev/null @@ -1,47 +0,0 @@ -package fr.dcproject.component.comment.generic.routes - -import fr.dcproject.common.security.assert -import fr.dcproject.common.utils.receiveOrBadRequest -import fr.dcproject.component.auth.citizen -import fr.dcproject.component.auth.citizenOrNull -import fr.dcproject.component.auth.mustBeAuth -import fr.dcproject.component.comment.generic.CommentAccessControl -import fr.dcproject.component.comment.generic.database.CommentForUpdate -import fr.dcproject.component.comment.generic.database.CommentRef -import fr.dcproject.component.comment.generic.database.CommentRepository -import fr.dcproject.component.comment.toOutput -import io.ktor.application.call -import io.ktor.features.NotFoundException -import io.ktor.http.HttpStatusCode -import io.ktor.locations.KtorExperimentalLocationsAPI -import io.ktor.locations.Location -import io.ktor.locations.post -import io.ktor.response.respond -import io.ktor.routing.Route -import java.util.UUID - -@KtorExperimentalLocationsAPI -object CreateCommentChildren { - @Location("/comments/{comment}/children") - class CreateCommentChildrenRequest(comment: UUID) { - val comment = CommentRef(comment) - class Input(val content: String) - } - - fun Route.createCommentChildren(repo: CommentRepository, ac: CommentAccessControl) { - post { - mustBeAuth() - val parent = repo.findById(it.comment.id) ?: throw NotFoundException("Comment not found") - val newComment = CommentForUpdate( - content = call.receiveOrBadRequest().content, - createdBy = citizen, - parent = parent - ) - - ac.assert { canCreate(newComment, citizenOrNull) } - repo.comment(newComment) - - call.respond(HttpStatusCode.Created, newComment.toOutput()) - } - } -} diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/EditComment.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/EditComment.kt index 7aa20c0..b4e1522 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/EditComment.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/EditComment.kt @@ -1,14 +1,18 @@ package fr.dcproject.component.comment.generic.routes -import fr.dcproject.common.response.toOutput +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.auth.mustBeAuth import fr.dcproject.component.comment.generic.CommentAccessControl +import fr.dcproject.component.comment.generic.database.CommentForUpdate import fr.dcproject.component.comment.generic.database.CommentRef import fr.dcproject.component.comment.generic.database.CommentRepository import fr.dcproject.component.comment.toOutput +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength import io.ktor.application.call import io.ktor.features.NotFoundException import io.ktor.http.HttpStatusCode @@ -24,22 +28,40 @@ object EditComment { @Location("/comments/{comment}") class EditCommentRequest(comment: UUID) { val comment = CommentRef(comment) - class Input(val content: String) + class Input(val content: String) { + fun validate() = Validation { + Input::content { + minLength(20) + maxLength(6000) + } + }.validate(this) + } } fun Route.editComment(repo: CommentRepository, ac: CommentAccessControl) { put { mustBeAuth() - val comment = repo.findById(it.comment.id) ?: throw NotFoundException("Comment not found") - ac.assert { canUpdate(comment, citizenOrNull) } + val commentOld = repo.findById(it.comment.id) ?: throw NotFoundException("Comment not found") + ac.assert { canUpdate(commentOld, citizenOrNull) } - comment.content = call.receiveOrBadRequest().content - repo.edit(comment) - - call.respond( - HttpStatusCode.OK, - comment.toOutput() - ) + call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } + .run { + CommentForUpdate( + id = commentOld.id, + createdBy = commentOld.createdBy, + target = commentOld.target, + parent = commentOld.parent, + content = content, + ) + } + .let { repo.edit(it) } + .let { + call.respond( + HttpStatusCode.OK, + it.toOutput() + ) + } } } } diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/install.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/install.kt index 8ea983c..3ca3711 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/install.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/install.kt @@ -1,6 +1,6 @@ package fr.dcproject.component.comment.generic.routes -import fr.dcproject.component.comment.generic.routes.CreateCommentChildren.createCommentChildren +import fr.dcproject.component.comment.generic.routes.CreateComment.createCommentChildren import fr.dcproject.component.comment.generic.routes.EditComment.editComment import fr.dcproject.component.comment.generic.routes.GetCommentChildren.getChildrenComments import fr.dcproject.component.comment.generic.routes.GetOneComment.getOneComment diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index c0a4348..326f39b 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -598,6 +598,40 @@ paths: application/json: schema: $ref: '#/components/schemas/CommentResponse' + post: + security: + - JWTAuth: [] + summary: create comment + tags: + - comment + requestBody: + content: + application/json: + schema: + required: + - content + properties: + content: + type: string + example: + Lorem ipsum dolor sit amet, consectetur adipiscing elit. + responses: + 201: + description: Return updated comment + content: + application/json: + schema: + $ref: '#/components/schemas/CommentResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' + 401: + $ref: '#/components/responses/401' + 404: + description: No comment found put: security: - JWTAuth: [] @@ -614,7 +648,7 @@ paths: content: type: string example: - Lorem ipsum... + Lorem ipsum dolor sit amet, consectetur adipiscing elit. responses: 200: description: Return updated comment @@ -622,6 +656,12 @@ paths: application/json: schema: $ref: '#/components/schemas/CommentResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' /comments/{comment}/children: diff --git a/src/main/resources/sql/functions/comment/comment.sql b/src/main/resources/sql/functions/comment/comment.sql index 803515b..b4228e2 100644 --- a/src/main/resources/sql/functions/comment/comment.sql +++ b/src/main/resources/sql/functions/comment/comment.sql @@ -1,4 +1,4 @@ -create or replace function comment(reference regclass, resource json, out _id uuid) +create or replace function comment(reference regclass, inout resource json) language plpgsql as $$ declare @@ -17,7 +17,8 @@ begin else raise exception 'comment with target as "%", is not implemented', reference::text; end if; - _id = _new_id; + + select find_comment_by_id(_new_id) into resource; end; $$; diff --git a/src/main/resources/sql/functions/comment/edit_comment.sql b/src/main/resources/sql/functions/comment/edit_comment.sql index f68039e..8485f89 100644 --- a/src/main/resources/sql/functions/comment/edit_comment.sql +++ b/src/main/resources/sql/functions/comment/edit_comment.sql @@ -1,9 +1,11 @@ -create or replace function edit_comment(_id uuid, _content text) returns void +create or replace function edit_comment(_id uuid, _content text, out resource json) language plpgsql as $$ begin update comment c set "content" = _content where c.id = _id; + + select find_comment_by_id(_id) into resource; end; $$; diff --git a/src/test/kotlin/integration/Comment routes.kt b/src/test/kotlin/integration/Comment routes.kt index 15ff4cf..a57acde 100644 --- a/src/test/kotlin/integration/Comment routes.kt +++ b/src/test/kotlin/integration/Comment routes.kt @@ -2,16 +2,20 @@ package integration import fr.dcproject.component.citizen.database.CitizenI import integration.steps.`when`.`When I send a GET request` +import integration.steps.`when`.`When I send a POST request` import integration.steps.`when`.`When I send a PUT request` import integration.steps.`when`.`with body` import integration.steps.given.`Given I have article` import integration.steps.given.`Given I have citizen` import integration.steps.given.`Given I have comment on article` +import integration.steps.given.`Given I have comment on comment` import integration.steps.given.`authenticated as` import integration.steps.then.`And the response should contain` import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` import integration.steps.then.and +import io.ktor.http.HttpStatusCode.Companion.BadRequest +import io.ktor.http.HttpStatusCode.Companion.Created import io.ktor.http.HttpStatusCode.Companion.OK import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Tags @@ -35,27 +39,105 @@ class `Comment routes` : BaseTest() { } } + @Test + fun `I can create comment`() { + withIntegrationApplication { + `Given I have citizen`("Hubert", "Reeves") + `Given I have comment on comment`(id = "49933147-fc0f-4e5c-aa8d-f77fa0d88fa6") + `When I send a POST request`("/comments/49933147-fc0f-4e5c-aa8d-f77fa0d88fa6") { + `authenticated as`("Hubert", "Reeves") + `with body`( + """ + { + "content": "Lorem ipsum dolor sit amet, consectetur adipiscing elit." + } + """ + ) + } `Then the response should be` Created and { + `And the response should not be null`() + `And the response should contain`("$.content", "Lorem ipsum dolor sit amet, consectetur adipiscing elit.") + } + } + } + + @Test + @Tag("BadRequest") + fun `I cannot create comment with bad request`() { + withIntegrationApplication { + `Given I have citizen`("Hubert", "Reeves") + `Given I have comment on comment`(id = "49933147-fc0f-4e5c-aa8d-f77fa0d88fa6") + `When I send a POST request`("/comments/49933147-fc0f-4e5c-aa8d-f77fa0d88fa6") { + `authenticated as`("Hubert", "Reeves") + `with body`( + """ + { + "content": "small content" + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".content") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 20 characters") + } + } + } + @Test fun `I can edit comment`() { withIntegrationApplication { `Given I have citizen`("Hubert", "Reeves") `Given I have article`(id = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1") - `Given I have comment on article`(article = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1", createdBy = CitizenI.Name( - "Hubert", - "Reeves" - ), id = "fd30d20f-656c-42c6-8955-f61c04537464") + `Given I have comment on article`( + article = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1", + createdBy = CitizenI.Name( + "Hubert", + "Reeves" + ), + id = "fd30d20f-656c-42c6-8955-f61c04537464" + ) `When I send a PUT request`("/comments/fd30d20f-656c-42c6-8955-f61c04537464") { `authenticated as`("Hubert", "Reeves") `with body`( """ { - "content": "Hello boy" + "content": "Lorem ipsum dolor sit amet, consectetur adipiscing elit." } """ ) } `Then the response should be` OK and { `And the response should not be null`() - `And the response should contain`("$.content", "Hello boy") + `And the response should contain`("$.content", "Lorem ipsum dolor sit amet, consectetur adipiscing elit.") + } + } + } + + @Test + fun `I cannot edit comment with bad request`() { + withIntegrationApplication { + `Given I have citizen`("Hubert", "Reeves") + `Given I have article`(id = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1") + `Given I have comment on article`( + article = "bb05e4a3-55a1-4088-85e7-8d8c23be29b1", + createdBy = CitizenI.Name( + "Hubert", + "Reeves" + ), + id = "fd30d20f-656c-42c6-8955-f61c04537464" + ) + `When I send a PUT request`("/comments/fd30d20f-656c-42c6-8955-f61c04537464") { + `authenticated as`("Hubert", "Reeves") + `with body`( + """ + { + "content": "small content" + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".content") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 20 characters") } } } diff --git a/src/test/kotlin/integration/steps/given/Comment.kt b/src/test/kotlin/integration/steps/given/Comment.kt index 20689b5..6dee80e 100644 --- a/src/test/kotlin/integration/steps/given/Comment.kt +++ b/src/test/kotlin/integration/steps/given/Comment.kt @@ -2,11 +2,16 @@ package integration.steps.given import com.thedeanda.lorem.LoremIpsum import fr.dcproject.common.entity.TargetI +import fr.dcproject.common.entity.TargetRef import fr.dcproject.common.utils.toUUID import fr.dcproject.component.article.database.ArticleRef import fr.dcproject.component.article.database.ArticleRepository +import fr.dcproject.component.citizen.database.CitizenCreator import fr.dcproject.component.citizen.database.CitizenI.Name import fr.dcproject.component.comment.generic.database.CommentForUpdate +import fr.dcproject.component.comment.generic.database.CommentForView +import fr.dcproject.component.comment.generic.database.CommentI +import fr.dcproject.component.comment.generic.database.CommentRef import fr.dcproject.component.comment.generic.database.CommentRepository import fr.dcproject.component.constitution.database.ConstitutionRef import fr.dcproject.component.constitution.database.ConstitutionRepository @@ -32,14 +37,14 @@ fun TestApplicationEngine.`Given I have comments on article`( } } -fun createComment( +fun createComment( id: UUID? = null, - article: ArticleRef? = null, + article: A? = null, createdBy: Name? = null, content: String? = null -) { +): CommentForView { val articleRepository: ArticleRepository by lazy { GlobalContext.get().koin.get() } - createCommentOnTarget( + return createCommentOnTarget( id, article?.id?.let { articleRepository.findById(article.id) } ?: createArticle(article?.id), createdBy, @@ -56,14 +61,14 @@ fun TestApplicationEngine.`Given I have comment on constitution`( createComment(id?.toUUID(), ConstitutionRef(constitution?.toUUID()), createdBy, content) } -fun createComment( +fun createComment( id: UUID? = null, - constitution: ConstitutionRef? = null, + constitution: C? = null, createdBy: Name? = null, content: String? = null -) { +): CommentForView { val constitutionRepository: ConstitutionRepository by lazy { GlobalContext.get().koin.get() } - createCommentOnTarget( + return createCommentOnTarget( id, constitution?.id?.let { constitutionRepository.findById(constitution.id) } ?: createConstitution(constitution?.id), createdBy, @@ -71,12 +76,12 @@ fun createComment( ) } -fun createCommentOnTarget( +fun createCommentOnTarget( id: UUID? = null, - target: TargetI, + target: T, createdBy: Name? = null, content: String? = null -) { +): CommentForView { val commentRepository: CommentRepository by lazy { GlobalContext.get().koin.get() } val creator = createCitizen(createdBy) val comment = CommentForUpdate( @@ -85,5 +90,41 @@ fun createCommentOnTarget( target = target, content = content ?: LoremIpsum().getParagraphs(1, 3) ) - commentRepository.comment(comment) + return commentRepository.comment(comment) +} + +fun TestApplicationEngine.`Given I have comment on comment`( + id: String? = null, + parent: String? = null, + createdBy: Name? = null, + content: String? = null, +): CommentForView { + return createCommentOnComment( + id?.toUUID() ?: UUID.randomUUID(), + parent?.run { CommentRef(toUUID()) }, + createdBy, + content, + ) +} + +fun createCommentOnComment( + id: UUID? = null, + parent: CommentI? = createComment(), + createdBy: Name? = null, + content: String? = null +): CommentForView { + val creator = createCitizen(createdBy) + val commentRepository: CommentRepository by lazy { GlobalContext.get().koin.get() } + val parentComment = if (parent == null) { + createComment() + } else { + commentRepository.findById(parent.id) ?: error("Parent of comment not found") + } + val comment = CommentForUpdate( + id = id ?: UUID.randomUUID(), + createdBy = creator, + content = content ?: LoremIpsum().getParagraphs(1, 3), + parent = parentComment, + ) + return commentRepository.comment(comment) } From 0c8bcbd634731e630104bff6791005ee31e50b52 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sat, 10 Apr 2021 21:34:51 +0200 Subject: [PATCH 28/46] Add limit on content field (comment request) --- src/main/resources/openapi.yaml | 12 ++++++++++-- src/test/kotlin/integration/Comment routes.kt | 6 ++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 326f39b..ced4124 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -567,8 +567,10 @@ paths: properties: content: type: string + minLength: 20 + maxLength: 6000 example: - Lorem ipsum... + Lorem ipsum dolor sit amet, consectetur adipiscing elit.Lorem ipsum... responses: 201: description: Return created Comment @@ -613,6 +615,8 @@ paths: properties: content: type: string + minLength: 20 + maxLength: 6000 example: Lorem ipsum dolor sit amet, consectetur adipiscing elit. responses: @@ -647,6 +651,8 @@ paths: properties: content: type: string + minLength: 20 + maxLength: 6000 example: Lorem ipsum dolor sit amet, consectetur adipiscing elit. responses: @@ -799,8 +805,10 @@ paths: properties: content: type: string + minLength: 20 + maxLength: 6000 example: - Lorem ipsum... + Lorem ipsum dolor sit amet, consectetur adipiscing elit. responses: 201: description: Return created comment diff --git a/src/test/kotlin/integration/Comment routes.kt b/src/test/kotlin/integration/Comment routes.kt index a57acde..ad7cea7 100644 --- a/src/test/kotlin/integration/Comment routes.kt +++ b/src/test/kotlin/integration/Comment routes.kt @@ -1,6 +1,8 @@ package integration import fr.dcproject.component.citizen.database.CitizenI +import integration.steps.`when`.Validate.ALL +import integration.steps.`when`.Validate.REQUEST_BODY import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` import integration.steps.`when`.`When I send a PUT request` @@ -66,7 +68,7 @@ class `Comment routes` : BaseTest() { withIntegrationApplication { `Given I have citizen`("Hubert", "Reeves") `Given I have comment on comment`(id = "49933147-fc0f-4e5c-aa8d-f77fa0d88fa6") - `When I send a POST request`("/comments/49933147-fc0f-4e5c-aa8d-f77fa0d88fa6") { + `When I send a POST request`("/comments/49933147-fc0f-4e5c-aa8d-f77fa0d88fa6", ALL - REQUEST_BODY) { `authenticated as`("Hubert", "Reeves") `with body`( """ @@ -125,7 +127,7 @@ class `Comment routes` : BaseTest() { ), id = "fd30d20f-656c-42c6-8955-f61c04537464" ) - `When I send a PUT request`("/comments/fd30d20f-656c-42c6-8955-f61c04537464") { + `When I send a PUT request`("/comments/fd30d20f-656c-42c6-8955-f61c04537464", ALL - REQUEST_BODY) { `authenticated as`("Hubert", "Reeves") `with body`( """ From 6a5e00bb4d714873c6196313472f8ef9770a4169 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sat, 10 Apr 2021 23:45:16 +0200 Subject: [PATCH 29/46] Add validation on Constitution routes --- build.gradle.kts | 6 ++ .../common/request/PaginatedRequest.kt | 9 +-- .../constitution/routes/CreateConstitution.kt | 20 ++++++- .../constitution/routes/FindConstitutions.kt | 24 +++++++- src/main/resources/openapi.yaml | 16 ++++- .../constitution/find_constitutions.sql | 4 +- .../kotlin/integration/Constitution routes.kt | 58 +++++++++++++++++-- 7 files changed, 120 insertions(+), 17 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 2f95015..bfddad0 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -338,6 +338,12 @@ tasks.register("testComments", Test::class) { includeTags("comment") } } +tasks.register("testConstitutions", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("constitution") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/common/request/PaginatedRequest.kt b/src/main/kotlin/fr/dcproject/common/request/PaginatedRequest.kt index 063c961..26a307a 100644 --- a/src/main/kotlin/fr/dcproject/common/request/PaginatedRequest.kt +++ b/src/main/kotlin/fr/dcproject/common/request/PaginatedRequest.kt @@ -6,9 +6,6 @@ interface PaginatedRequestI { } open class PaginatedRequest( - page: Int = 1, - limit: Int = 50 -) : PaginatedRequestI { - override val page: Int = if (page < 1) 1 else page - override val limit: Int = if (limit > 50) 50 else if (limit < 1) 1 else limit -} + override val page: Int = 1, + override val limit: Int = 50 +) : PaginatedRequestI diff --git a/src/main/kotlin/fr/dcproject/component/constitution/routes/CreateConstitution.kt b/src/main/kotlin/fr/dcproject/component/constitution/routes/CreateConstitution.kt index dfc69d0..cb4e11f 100644 --- a/src/main/kotlin/fr/dcproject/component/constitution/routes/CreateConstitution.kt +++ b/src/main/kotlin/fr/dcproject/component/constitution/routes/CreateConstitution.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.constitution.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest @@ -15,6 +16,9 @@ import fr.dcproject.component.constitution.database.ConstitutionForUpdate.TitleF import fr.dcproject.component.constitution.database.ConstitutionRepository import fr.dcproject.component.constitution.routes.CreateConstitution.PostConstitutionRequest.Input import fr.dcproject.component.constitution.routes.CreateConstitution.PostConstitutionRequest.Input.Title +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -36,7 +40,6 @@ object CreateConstitution { val draft: Boolean = false, val versionId: UUID = UUID.randomUUID() ) { - class Title( val id: UUID = UUID.randomUUID(), val name: String, @@ -44,10 +47,25 @@ object CreateConstitution { ) { class ArticleRef(val id: UUID) } + + fun validate() = Validation { + Input::title { + minLength(10) + maxLength(80) + } + Input::titles onEach { + Title::name { + minLength(10) + maxLength(80) + } + } + }.validate(this) } } private fun getNewConstitution(input: Input, citizen: Citizen) = input.run { + validate().badRequestIfNotValid() + ConstitutionForUpdate>( id = UUID.randomUUID(), title = title, diff --git a/src/main/kotlin/fr/dcproject/component/constitution/routes/FindConstitutions.kt b/src/main/kotlin/fr/dcproject/component/constitution/routes/FindConstitutions.kt index f7184f2..c570753 100644 --- a/src/main/kotlin/fr/dcproject/component/constitution/routes/FindConstitutions.kt +++ b/src/main/kotlin/fr/dcproject/component/constitution/routes/FindConstitutions.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.constitution.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.component.auth.citizenOrNull @@ -8,6 +9,10 @@ import fr.dcproject.component.constitution.database.ConstitutionRepository 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.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -27,10 +32,27 @@ object FindConstitutions { val sort: String? = null, val direction: RepositoryI.Direction? = null, val search: String? = null - ) : PaginatedRequestI by PaginatedRequest(page, limit) + ) : PaginatedRequestI by PaginatedRequest(page, limit) { + fun validate() = Validation { + FindConstitutionsRequest::page { + minimum(1) + } + FindConstitutionsRequest::limit { + minimum(1) + maximum(50) + } + FindConstitutionsRequest::sort ifPresent { + enum( + "title", + "createdAt", + ) + } + }.validate(this) + } fun Route.findConstitutions(repo: ConstitutionRepository, ac: ConstitutionAccessControl) { get { + it.validate().badRequestIfNotValid() val constitutions = repo.find(it.page, it.limit, it.sort, it.direction, it.search) ac.assert { canView(constitutions.result, citizenOrNull) } call.respond( diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index ced4124..0dbd143 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -851,6 +851,12 @@ paths: type: array items: $ref: '#/components/schemas/ConstitutionListingResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' post: security: - JWTAuth: [ ] @@ -875,7 +881,11 @@ paths: 401: $ref: '#/components/responses/401' 400: - $ref: '#/components/responses/400' + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' /constitutions/{constitution}: parameters: - $ref: '#/components/parameters/constitution' @@ -2019,6 +2029,8 @@ components: $ref: '#/components/schemas/UUID' title: type: string + minLength: 10 + maxLength: 80 example: Constitution for the liberty titles: @@ -2034,6 +2046,8 @@ components: $ref: '#/components/schemas/UUID' name: type: string + minLength: 10 + maxLength: 80 example: The liberties articles: diff --git a/src/main/resources/sql/functions/constitution/find_constitutions.sql b/src/main/resources/sql/functions/constitution/find_constitutions.sql index 1dae8c0..9f2be1c 100644 --- a/src/main/resources/sql/functions/constitution/find_constitutions.sql +++ b/src/main/resources/sql/functions/constitution/find_constitutions.sql @@ -22,14 +22,14 @@ begin case direction when 'asc' then case sort when 'title' then c.title - when 'created_at' then c.created_at::text + when 'createdAt' then c.created_at::text else null end end, case direction when 'desc' then case sort when 'title' then c.title - when 'created_at' then c.created_at::text + when 'createdAt' then c.created_at::text end end desc, diff --git a/src/test/kotlin/integration/Constitution routes.kt b/src/test/kotlin/integration/Constitution routes.kt index 75ce11d..271ca52 100644 --- a/src/test/kotlin/integration/Constitution routes.kt +++ b/src/test/kotlin/integration/Constitution routes.kt @@ -1,6 +1,8 @@ package integration -import integration.steps.`when`.Validate +import integration.steps.`when`.Validate.ALL +import integration.steps.`when`.Validate.REQUEST_BODY +import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` import integration.steps.`when`.`with body` @@ -9,6 +11,7 @@ import integration.steps.given.`Given I have constitution` import integration.steps.given.`Given I have constitutions` import integration.steps.given.`authenticated as` import integration.steps.then.`And have property` +import integration.steps.then.`And the response should contain` import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` import integration.steps.then.`which contains` @@ -28,12 +31,25 @@ class `Constitution routes` : BaseTest() { fun `I can get constitution list`() { withIntegrationApplication { `Given I have constitutions`(3) - `When I send a GET request`("/constitutions") `Then the response should be` OK and { + `When I send a GET request`("/constitutions?page=1&limit=10&sort=title&direction=desc") `Then the response should be` OK and { `And the response should not be null`() } } } + @Test + @Tag("BadRequest") + fun `I cannot get constitution list with wrong request`() { + withIntegrationApplication { + `Given I have constitutions`(3) + `When I send a GET request`("/constitutions?page=1&limit=5000&sort=title&direction=desc", ALL - REQUEST_PARAM) `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".limit") + `And the response should contain`("$.invalidParams[0].reason", "must be at most '50'") + } + } + } + @Test fun `I can get constitution by ID`() { withIntegrationApplication { @@ -70,11 +86,11 @@ class `Constitution routes` : BaseTest() { """ { "versionId":"15814bb6-8d90-4c6a-a456-c3939a8ec75e", - "title":"Hello world!", + "title":"Cras sit amet sapien mattis nulla rutrum blandit.", "anonymous":true, "titles":[ { - "name":"plop" + "name":"Cras sit amet sapien mattis nulla rutrum blandit." } ] } @@ -83,7 +99,7 @@ class `Constitution routes` : BaseTest() { } `Then the response should be` Created and { `And the response should not be null`() `And have property`("$.versionId") `which contains` "15814bb6-8d90-4c6a-a456-c3939a8ec75e" - `And have property`("$.title") `which contains` "Hello world!" + `And have property`("$.title") `which contains` "Cras sit amet sapien mattis nulla rutrum blandit." } } } @@ -93,7 +109,7 @@ class `Constitution routes` : BaseTest() { fun `I cannot create an constitution if bad request`() { withIntegrationApplication { `Given I have citizen`("Henri", "Poincaré") - `When I send a POST request`("/constitutions", Validate.ALL - Validate.REQUEST_BODY) { + `When I send a POST request`("/constitutions", ALL - REQUEST_BODY) { `authenticated as`("Henri", "Poincaré") `with body`( """ @@ -113,4 +129,34 @@ class `Constitution routes` : BaseTest() { } `Then the response should be` BadRequest } } + + @Test + @Tag("BadRequest") + fun `I cannot create an constitution if request is not valid`() { + withIntegrationApplication { + `Given I have citizen`("Henri", "Poincaré") + `When I send a POST request`("/constitutions", ALL - REQUEST_BODY) { + `authenticated as`("Henri", "Poincaré") + `with body`( + """ + { + "versionId":"15814bb6-8d90-4c6a-a456-c3939a8ec75e", + "title":"too small", + "anonymous":true, + "titles":[ + { + "name":"too small" + } + ] + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should contain`("$.invalidParams[0].name", ".title") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 10 characters") + `And the response should contain`("$.invalidParams[1].name", ".titles[0].name") + `And the response should contain`("$.invalidParams[1].reason", "must have at least 10 characters") + } + } + } } From 50b4cf181674ffa9322d184a0e433f4e0475f4ab Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Mon, 12 Apr 2021 01:03:57 +0200 Subject: [PATCH 30/46] Add testFollow task --- build.gradle.kts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index bfddad0..c227605 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -344,6 +344,12 @@ tasks.register("testConstitutions", Test::class) { includeTags("constitution") } } +tasks.register("testFollows", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("follow") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) From 39c665b7a90b74ef1af7ca3b11af3d62204cc8b4 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Wed, 14 Apr 2021 01:41:49 +0200 Subject: [PATCH 31/46] Add Test for Notification routes Add @JsonSubTypes on Notification return all creator on request find_follows_article_by_target Add testNotifications task --- build.gradle.kts | 6 ++ .../component/notification/Notification.kt | 6 ++ .../notification/NotificationsPush.kt | 16 ++-- .../follow/find_follows_article_by_target.sql | 2 +- .../functional/NotificationsPushTest.kt | 6 +- .../kotlin/integration/Notification routes.kt | 78 +++++++++++++++++++ .../kotlin/integration/steps/given/Auth.kt | 21 +++++ .../kotlin/integration/steps/given/Follow.kt | 5 +- src/test/resources/sql/follow.sql | 3 + 9 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 src/test/kotlin/integration/Notification routes.kt diff --git a/build.gradle.kts b/build.gradle.kts index c227605..1d811c4 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -350,6 +350,12 @@ tasks.register("testFollows", Test::class) { includeTags("follow") } } +tasks.register("testNotifications", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("notification") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/component/notification/Notification.kt b/src/main/kotlin/fr/dcproject/component/notification/Notification.kt index ff8871c..57e4320 100644 --- a/src/main/kotlin/fr/dcproject/component/notification/Notification.kt +++ b/src/main/kotlin/fr/dcproject/component/notification/Notification.kt @@ -1,5 +1,7 @@ package fr.dcproject.component.notification +import com.fasterxml.jackson.annotation.JsonSubTypes +import com.fasterxml.jackson.annotation.JsonTypeInfo import com.fasterxml.jackson.databind.DeserializationFeature import com.fasterxml.jackson.databind.PropertyNamingStrategies import com.fasterxml.jackson.databind.SerializationFeature @@ -12,6 +14,10 @@ import fr.dcproject.component.article.database.ArticleForView import org.joda.time.DateTime import java.util.concurrent.atomic.AtomicInteger +@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "type", visible = true) +@JsonSubTypes( + JsonSubTypes.Type(value = ArticleUpdateNotification::class, name = "article") +) open class Notification( val type: String, val createdAt: DateTime = DateTime.now() diff --git a/src/main/kotlin/fr/dcproject/component/notification/NotificationsPush.kt b/src/main/kotlin/fr/dcproject/component/notification/NotificationsPush.kt index 08e6e29..445cca1 100644 --- a/src/main/kotlin/fr/dcproject/component/notification/NotificationsPush.kt +++ b/src/main/kotlin/fr/dcproject/component/notification/NotificationsPush.kt @@ -28,12 +28,12 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import org.slf4j.LoggerFactory -class NotificationsPush private constructor( +class NotificationsPush( private val redis: RedisAsyncCommands, private val redisConnectionPubSub: StatefulRedisPubSubConnection, citizen: CitizenI, incoming: Flow, - onRecieve: suspend (Notification) -> Unit, + onReceive: suspend (Notification) -> Unit, ) { class Builder(val redisClient: RedisClient) { private val redisConnection = redisClient.connect() ?: error("Unable to connect to redis") @@ -43,8 +43,8 @@ class NotificationsPush private constructor( fun build( citizen: CitizenI, incoming: Flow, - onRecieve: suspend (Notification) -> Unit, - ): NotificationsPush = NotificationsPush(redis, redisConnectionPubSub, citizen, incoming, onRecieve) + onReceive: suspend (Notification) -> Unit, + ): NotificationsPush = NotificationsPush(redis, redisConnectionPubSub, citizen, incoming, onReceive) @ExperimentalCoroutinesApi fun build(ws: DefaultWebSocketServerSession): NotificationsPush { @@ -69,7 +69,7 @@ class NotificationsPush private constructor( override fun message(pattern: String?, channel: String?, message: String?) { runBlocking { getNotifications().collect { - onRecieve(it) + onReceive(it) } } } @@ -85,10 +85,12 @@ class NotificationsPush private constructor( /* Get old notification and sent it to websocket */ runBlocking { - getNotifications().collect { onRecieve(it) } + getNotifications().collect { + onReceive(it) + } } - /* Lisen redis event, and sent the new notification into websocket */ + /* Listen redis event, and sent the new notification into websocket */ redisConnectionPubSub.run { addListener(listener) diff --git a/src/main/resources/sql/functions/follow/find_follows_article_by_target.sql b/src/main/resources/sql/functions/follow/find_follows_article_by_target.sql index fb08b76..6baded1 100644 --- a/src/main/resources/sql/functions/follow/find_follows_article_by_target.sql +++ b/src/main/resources/sql/functions/follow/find_follows_article_by_target.sql @@ -21,7 +21,7 @@ begin f.created_at, f.target_reference, json_build_object('id', f.target_id) as target, - json_build_object('id', f.created_by_id) as created_by + find_citizen_by_id_with_user(f.created_by_id) as created_by from follow_article as f join article a on f.target_id = a.id where a.version_id = _version_id diff --git a/src/test/kotlin/functional/NotificationsPushTest.kt b/src/test/kotlin/functional/NotificationsPushTest.kt index cedbaf6..7464c6b 100644 --- a/src/test/kotlin/functional/NotificationsPushTest.kt +++ b/src/test/kotlin/functional/NotificationsPushTest.kt @@ -30,7 +30,7 @@ internal class NotificationsPushTest { @BeforeAll @JvmStatic fun before() { - val config: Configuration = Configuration("application-test.conf") + val config = Configuration("application-test.conf") RedisClient.create(config.redis).connect().sync().flushall() /* Purge rabbit notification queues */ @@ -45,7 +45,7 @@ internal class NotificationsPushTest { @Test fun `Notification from redis is well catch and return`() = runBlocking { - val config: Configuration = Configuration("application-test.conf") + val config = Configuration("application-test.conf") /* Redis client for test */ val redisClientTest = RedisClient.create(config.redis) @@ -74,7 +74,7 @@ internal class NotificationsPushTest { } val notifAfterSubscribe = ArticleUpdateNotification(article) - /* init event for emulate incomint message from websocket */ + /* init event for emulate incoming message from websocket */ val event = MutableSharedFlow() val incomingFlow = event.asSharedFlow() diff --git a/src/test/kotlin/integration/Notification routes.kt b/src/test/kotlin/integration/Notification routes.kt new file mode 100644 index 0000000..d50b298 --- /dev/null +++ b/src/test/kotlin/integration/Notification routes.kt @@ -0,0 +1,78 @@ +package integration + +import fr.dcproject.common.utils.toUUID +import fr.dcproject.component.article.database.ArticleForView +import fr.dcproject.component.auth.database.UserCreator +import fr.dcproject.component.citizen.database.CitizenCreator +import fr.dcproject.component.citizen.database.CitizenI.Name +import fr.dcproject.component.notification.ArticleUpdateNotification +import fr.dcproject.component.notification.Notification +import fr.dcproject.component.notification.Publisher +import integration.steps.given.`Given I have article` +import integration.steps.given.`Given I have citizen` +import integration.steps.given.`Given I have follow on article` +import integration.steps.given.`authenticated in url as` +import io.ktor.http.cio.websocket.Frame +import io.ktor.http.cio.websocket.readText +import kotlinx.coroutines.launch +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Tags +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import org.koin.test.get +import kotlin.test.assertEquals + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@Tags(Tag("integration"), Tag("notification")) +class `Notification routes` : BaseTest() { + @Test + fun `I can send notification`() { + withIntegrationApplication { + `Given I have citizen`("John", "Doe", id = "1a34191a-9cde-45ba-8ac1-230138a102d3") + `Given I have article`(id = "a06cbfb7-3094-4d64-aaa1-7486c0c292f4", createdBy = Name(firstName = "John", lastName = "Doe")) + `Given I have follow on article`("John", "Doe", article = "a06cbfb7-3094-4d64-aaa1-7486c0c292f4") + val notification = ArticleUpdateNotification( + ArticleForView( + id = "a06cbfb7-3094-4d64-aaa1-7486c0c292f4".toUUID(), + title = "MyTitle", + content = "myContent", + description = "myDescription", + createdBy = CitizenCreator( + id = "1a34191a-9cde-45ba-8ac1-230138a102d3".toUUID(), + name = Name(firstName = "John", lastName = "Doe"), + email = "john-doe@plop.com", + user = UserCreator(username = "john-doe"), + ) + ) + ) + val publisher = get() + launch { + publisher + .publish(notification) + .await() + } + + Thread.sleep(1000) + + handleWebSocketConversation( + "/notifications", + { + `authenticated in url as`("John", "Doe") + } + ) { incoming, outgoing -> + incoming.receive().let { + when (it) { + is Frame.Text -> Notification.fromString(it.readText()).let { notif -> + assertEquals( + "a06cbfb7-3094-4d64-aaa1-7486c0c292f4", + notif.target.id.toString() + ) + outgoing.send(it) + } + else -> error(it.toString()) + } + } + } + } + } +} diff --git a/src/test/kotlin/integration/steps/given/Auth.kt b/src/test/kotlin/integration/steps/given/Auth.kt index f8e1b8b..6c6fb71 100644 --- a/src/test/kotlin/integration/steps/given/Auth.kt +++ b/src/test/kotlin/integration/steps/given/Auth.kt @@ -3,6 +3,7 @@ package integration.steps.given import com.auth0.jwt.JWT import fr.dcproject.component.auth.jwt.JwtConfig import fr.dcproject.component.citizen.database.Citizen +import fr.dcproject.component.citizen.database.CitizenI import fr.dcproject.component.citizen.database.CitizenRepository import io.ktor.http.HttpHeaders import io.ktor.server.testing.TestApplicationRequest @@ -25,3 +26,23 @@ fun TestApplicationRequest.`authenticated as`( return citizen } +fun TestApplicationRequest.`authenticated in url as`( + firstName: String, + lastName: String, +): Citizen { + val repo: CitizenRepository by lazy { GlobalContext.get().koin.get() } + val citizen = repo.findByName(CitizenI.Name(firstName, lastName)) ?: error("Citizen not exist with name $firstName $lastName") + val algorithm = GlobalContext.get().koin.get().algorithm + val jwtAsString: String = JWT.create() + .withIssuer("dc-project.fr") + .withClaim("id", citizen.user.id.toString()) + .sign(algorithm) + + uri += when (uri.contains('?')) { + true -> '&' + false -> '?' + } + uri += "token=$jwtAsString" + + return citizen +} diff --git a/src/test/kotlin/integration/steps/given/Follow.kt b/src/test/kotlin/integration/steps/given/Follow.kt index 084f09d..3e9191f 100644 --- a/src/test/kotlin/integration/steps/given/Follow.kt +++ b/src/test/kotlin/integration/steps/given/Follow.kt @@ -3,6 +3,7 @@ package integration.steps.given import fr.dcproject.common.utils.toUUID import fr.dcproject.component.article.database.ArticleRef import fr.dcproject.component.citizen.database.Citizen +import fr.dcproject.component.citizen.database.CitizenI import fr.dcproject.component.citizen.database.CitizenRef import fr.dcproject.component.citizen.database.CitizenRepository import fr.dcproject.component.constitution.database.ConstitutionRef @@ -30,7 +31,7 @@ fun TestApplicationEngine.`Given I have follow on article`( article: String, ) { val citizenRepository: CitizenRepository by lazy { GlobalContext.get().koin.get() } - val citizen = citizenRepository.findByUsername("$firstName-$lastName".toLowerCase()) ?: error("Citizen not exist") + val citizen = citizenRepository.findByName(CitizenI.Name(firstName, lastName)) ?: error("Citizen not exist") createFollow(citizen, ArticleRef(article.toUUID())) } @@ -40,7 +41,7 @@ fun TestApplicationEngine.`Given I have follow on constitution`( constitution: String, ) { val citizenRepository: CitizenRepository by lazy { GlobalContext.get().koin.get() } - val citizen = citizenRepository.findByUsername("$firstName-$lastName".toLowerCase()) ?: error("Citizen not exist") + val citizen = citizenRepository.findByName(CitizenI.Name(firstName, lastName)) ?: error("Citizen not exist") createFollow(citizen, ArticleRef(constitution.toUUID())) } diff --git a/src/test/resources/sql/follow.sql b/src/test/resources/sql/follow.sql index 58c27bd..0d98afc 100644 --- a/src/test/resources/sql/follow.sql +++ b/src/test/resources/sql/follow.sql @@ -29,6 +29,9 @@ begin assert (select following = true from find_follow(first_article_id, _citizen_id, 'article')), '(v1) find_follow must return the following'; assert (select following = true from find_follow(first_article_updated_id, _citizen_id, 'article')), '(v2) find_follow must return the following'; + assert (select f.total = 1 from find_follows_article_by_target(first_article_id) as f), 'find_follows_article_by_target must return 1 follow'; + assert (select (f.resource#>>'{0, created_by, id}')::uuid = _citizen_id from find_follows_article_by_target(first_article_id) as f), 'find_follows_article_by_target must return follows with creator'; + perform unfollow('article'::regclass, first_article_id, _citizen_id); assert (select count(*) = 0 from follow), 'follow must be deleted after unfollow, event if article is on other version'; From 13253e4af1464ae9ae6ec120b6c8242a2ebe53eb Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 00:25:06 +0200 Subject: [PATCH 32/46] Add validation on route GetMyOpinionsArticle --- build.gradle.kts | 6 ++++ .../opinion/routes/GetMyOpinionsArticle.kt | 15 ++++++++++ src/main/resources/openapi.yaml | 10 +++++++ src/test/kotlin/integration/Opinion routes.kt | 28 ++++++++++++++++++- 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 1d811c4..283e4c8 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -356,6 +356,12 @@ tasks.register("testNotifications", Test::class) { includeTags("notification") } } +tasks.register("testOpinions", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("opinion") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/component/opinion/routes/GetMyOpinionsArticle.kt b/src/main/kotlin/fr/dcproject/component/opinion/routes/GetMyOpinionsArticle.kt index 7679ace..fe28926 100644 --- a/src/main/kotlin/fr/dcproject/component/opinion/routes/GetMyOpinionsArticle.kt +++ b/src/main/kotlin/fr/dcproject/component/opinion/routes/GetMyOpinionsArticle.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.opinion.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.entity.TargetRef import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert @@ -12,6 +13,9 @@ import fr.dcproject.component.opinion.database.Opinion import fr.dcproject.routes.PaginatedRequest import fr.dcproject.routes.PaginatedRequestI import fr.postgresjson.connexion.Paginated +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maximum +import io.konform.validation.jsonschema.minimum import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -34,11 +38,22 @@ object GetMyOpinionsArticle { limit: Int = 50 ) : PaginatedRequestI by PaginatedRequest(page, limit) { val citizen = CitizenRef(citizen) + fun validate() = Validation { + CitizenOpinionsArticleRequest::page { + minimum(1) + } + CitizenOpinionsArticleRequest::limit { + minimum(1) + maximum(50) + } + }.validate(this) } fun Route.getMyOpinionsArticle(repo: OpinionArticleRepository, ac: OpinionAccessControl) { get { mustBeAuth() + it.validate().badRequestIfNotValid() + val opinions: Paginated> = repo.findCitizenOpinions(citizen, it.page, it.limit) ac.assert { canView(opinions.result, citizenOrNull) } call.respond( diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 0dbd143..e4a1fd1 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -1118,6 +1118,9 @@ paths: tags: - opinion - citizen + parameters: + - $ref: '#/components/parameters/page' + - $ref: '#/components/parameters/limit' responses: 200: description: Opinions @@ -1132,6 +1135,13 @@ paths: type: array items: $ref: '#/components/schemas/Opinion' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' + /articles/{article}/opinions: parameters: - $ref: '#/components/parameters/article' diff --git a/src/test/kotlin/integration/Opinion routes.kt b/src/test/kotlin/integration/Opinion routes.kt index 4613372..14e8d13 100644 --- a/src/test/kotlin/integration/Opinion routes.kt +++ b/src/test/kotlin/integration/Opinion routes.kt @@ -1,6 +1,8 @@ package integration import fr.dcproject.component.citizen.database.CitizenI.Name +import integration.steps.`when`.Validate.ALL +import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a PUT request` import integration.steps.`when`.`with body` @@ -11,8 +13,10 @@ import integration.steps.given.`Given I have opinion on article` import integration.steps.given.`authenticated as` import integration.steps.then.`And the response should contain list` import integration.steps.then.`And the response should contain` +import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` import integration.steps.then.and +import io.ktor.http.HttpStatusCode import io.ktor.http.HttpStatusCode.Companion.Created import io.ktor.http.HttpStatusCode.Companion.OK import org.junit.jupiter.api.Tag @@ -133,7 +137,7 @@ class `Opinion routes` : BaseTest() { article = "8651b530-ac1b-4214-a784-706781371074", Name("Albert", "Einstein") ) - `When I send a GET request`("/citizens/c1542096-3431-432d-8e35-9dc071d4c818/opinions/articles") { + `When I send a GET request`("/citizens/c1542096-3431-432d-8e35-9dc071d4c818/opinions/articles?page=1&limit=10") { `authenticated as`("Albert", "Einstein") } `Then the response should be` OK and { `And the response should contain`("$.result[0].name", "Opinion9") @@ -141,4 +145,26 @@ class `Opinion routes` : BaseTest() { } } } + + @Test + @Tags(Tag("article"), Tag("BadRequest")) + fun `I cannot get all my opinion of one article with wrong request`() { + withIntegrationApplication { + `Given I have citizen`("Albert", "Einstein", id = "c1542096-3431-432d-8e35-9dc071d4c818") + `Given I have an opinion choice`("Opinion9") + `Given I have article`("8651b530-ac1b-4214-a784-706781371074") + `Given I have opinion on article`( + "Opinion9", + article = "8651b530-ac1b-4214-a784-706781371074", + Name("Albert", "Einstein") + ) + `When I send a GET request`("/citizens/c1542096-3431-432d-8e35-9dc071d4c818/opinions/articles?page=1&limit=60", ALL - REQUEST_PARAM) { + `authenticated as`("Albert", "Einstein") + } `Then the response should be` HttpStatusCode.BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".limit") + `And the response should contain`("$.invalidParams[0].reason", "must be at most '50'") + } + } + } } From 496cf50d88fc4a63bbc08153cecc188aef3291c1 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 01:27:48 +0200 Subject: [PATCH 33/46] Add validation on route GetCitizenVotesOnArticle --- build.gradle.kts | 6 +++++ .../vote/routes/GetCitizenVotesOnArticle.kt | 15 ++++++++++++ src/main/resources/openapi.yaml | 9 +++++++ src/test/kotlin/integration/Vote routes.kt | 24 ++++++++++++++++++- 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 283e4c8..3aff791 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -362,6 +362,12 @@ tasks.register("testOpinions", Test::class) { includeTags("opinion") } } +tasks.register("testVotes", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("vote") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/component/vote/routes/GetCitizenVotesOnArticle.kt b/src/main/kotlin/fr/dcproject/component/vote/routes/GetCitizenVotesOnArticle.kt index d8b5654..102300b 100644 --- a/src/main/kotlin/fr/dcproject/component/vote/routes/GetCitizenVotesOnArticle.kt +++ b/src/main/kotlin/fr/dcproject/component/vote/routes/GetCitizenVotesOnArticle.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.vote.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.component.auth.citizenOrNull @@ -9,6 +10,9 @@ import fr.dcproject.component.vote.VoteAccessControl import fr.dcproject.component.vote.database.VoteArticleRepository import fr.dcproject.routes.PaginatedRequest import fr.dcproject.routes.PaginatedRequestI +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maximum +import io.konform.validation.jsonschema.minimum import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -28,11 +32,22 @@ object GetCitizenVotesOnArticle { val search: String? = null ) : PaginatedRequestI by PaginatedRequest(page, limit) { val citizen = CitizenRef(citizen) + fun validate() = Validation { + CitizenVoteArticleRequest::page { + minimum(1) + } + CitizenVoteArticleRequest::limit { + minimum(1) + maximum(50) + } + }.validate(this) } fun Route.getCitizenVotesOnArticle(repo: VoteArticleRepository, ac: VoteAccessControl) { get { mustBeAuth() + it.validate().badRequestIfNotValid() + val votes = repo.findByCitizen(it.citizen, it.page, it.limit) ac.assert { canView(votes.result, citizenOrNull) } diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index e4a1fd1..76b7217 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -1265,6 +1265,9 @@ paths: - vote - article - citizen + parameters: + - $ref: '#/components/parameters/page' + - $ref: '#/components/parameters/limit' responses: 200: description: Votes @@ -1279,6 +1282,12 @@ paths: type: array items: $ref: '#/components/schemas/VoteResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' /articles/{article}/vote: diff --git a/src/test/kotlin/integration/Vote routes.kt b/src/test/kotlin/integration/Vote routes.kt index ec734b8..6d15ce2 100644 --- a/src/test/kotlin/integration/Vote routes.kt +++ b/src/test/kotlin/integration/Vote routes.kt @@ -1,6 +1,9 @@ package integration import fr.dcproject.component.citizen.database.CitizenI.Name +import integration.steps.`when`.Validate +import integration.steps.`when`.Validate.ALL +import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a PUT request` import integration.steps.`when`.`with body` @@ -12,8 +15,10 @@ import integration.steps.given.`Given I have vote +1 on article` import integration.steps.given.`Given I have vote -1 on article` import integration.steps.given.`authenticated as` import integration.steps.then.`And the response should contain` +import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` import integration.steps.then.and +import io.ktor.http.HttpStatusCode import io.ktor.http.HttpStatusCode.Companion.Created import io.ktor.http.HttpStatusCode.Companion.OK import org.junit.jupiter.api.Tag @@ -66,7 +71,7 @@ class `Vote routes` : BaseTest() { `Given I have citizen`("Carl", "Gauss", id = "c044823d-e778-4256-9016-b1334bf933d3") `Given I have article`("7c9286db-470d-448c-aab1-3f0b072213b1") `Given I have vote +1 on article`("7c9286db-470d-448c-aab1-3f0b072213b1", Name("Carl", "Gauss")) - `When I send a GET request`("/citizens/c044823d-e778-4256-9016-b1334bf933d3/votes/articles") { + `When I send a GET request`("/citizens/c044823d-e778-4256-9016-b1334bf933d3/votes/articles?page=1&limit=50") { `authenticated as`("Carl", "Gauss") } `Then the response should be` OK and { `And the response should contain`("$.currentPage", 1) @@ -77,6 +82,23 @@ class `Vote routes` : BaseTest() { } } + @Test + @Tag("BadRequest") + fun `I cannot get votes of current citizen with wrong request`() { + withIntegrationApplication { + `Given I have citizen`("Carl", "Gauss", id = "c044823d-e778-4256-9016-b1334bf933d3") + `Given I have article`("7c9286db-470d-448c-aab1-3f0b072213b1") + `Given I have vote +1 on article`("7c9286db-470d-448c-aab1-3f0b072213b1", Name("Carl", "Gauss")) + `When I send a GET request`("/citizens/c044823d-e778-4256-9016-b1334bf933d3/votes/articles?page=1&limit=60", ALL - REQUEST_PARAM) { + `authenticated as`("Carl", "Gauss") + } `Then the response should be` HttpStatusCode.BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".limit") + `And the response should contain`("$.invalidParams[0].reason", "must be at most '50'") + } + } + } + @Test fun `I can get votes of current citizen by target ids`() { withIntegrationApplication { From 0588f88f9acbac64b653e60866862ab01d53afa1 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 02:20:57 +0200 Subject: [PATCH 34/46] Add validation on route PutVoteOnArticle --- .../component/vote/routes/PutVoteOnArticle.kt | 15 ++++- src/main/resources/openapi.yaml | 6 ++ src/test/kotlin/integration/Vote routes.kt | 65 +++++++++++++++---- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnArticle.kt b/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnArticle.kt index 8f45229..0b169e0 100644 --- a/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnArticle.kt +++ b/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnArticle.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.vote.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest import fr.dcproject.component.article.database.ArticleRef @@ -10,6 +11,9 @@ import fr.dcproject.component.auth.mustBeAuth import fr.dcproject.component.vote.VoteAccessControl import fr.dcproject.component.vote.database.VoteArticleRepository import fr.dcproject.component.vote.database.VoteForUpdate +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maximum +import io.konform.validation.jsonschema.minimum import io.ktor.application.call import io.ktor.features.NotFoundException import io.ktor.http.HttpStatusCode @@ -25,13 +29,22 @@ object PutVoteOnArticle { @Location("/articles/{article}/vote") class ArticleVoteRequest(article: UUID) { val article = ArticleRef(article) - data class Input(var note: Int) + data class Input(var note: Int) { + fun validate() = Validation { + Input::note { + minimum(-1) + maximum(1) + } + }.validate(this) + } } fun Route.putVoteOnArticle(repo: VoteArticleRepository, ac: VoteAccessControl, articleRepo: ArticleRepository) { put { mustBeAuth() + val input = call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } val article = articleRepo.findById(it.article.id) ?: throw NotFoundException("Article ${it.article.id} not found") val vote = VoteForUpdate( target = article, diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 76b7217..2c3293d 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -1312,6 +1312,12 @@ paths: application/json: schema: $ref: '#/components/schemas/VoteAggregation' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' diff --git a/src/test/kotlin/integration/Vote routes.kt b/src/test/kotlin/integration/Vote routes.kt index 6d15ce2..ef65a0b 100644 --- a/src/test/kotlin/integration/Vote routes.kt +++ b/src/test/kotlin/integration/Vote routes.kt @@ -1,8 +1,8 @@ package integration import fr.dcproject.component.citizen.database.CitizenI.Name -import integration.steps.`when`.Validate import integration.steps.`when`.Validate.ALL +import integration.steps.`when`.Validate.REQUEST_BODY import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a PUT request` @@ -18,32 +18,73 @@ import integration.steps.then.`And the response should contain` import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` import integration.steps.then.and -import io.ktor.http.HttpStatusCode +import io.ktor.http.HttpStatusCode.Companion.BadRequest import io.ktor.http.HttpStatusCode.Companion.Created import io.ktor.http.HttpStatusCode.Companion.OK +import org.junit.jupiter.api.DynamicTest import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Tags import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestFactory import org.junit.jupiter.api.TestInstance @TestInstance(TestInstance.Lifecycle.PER_CLASS) @Tags(Tag("integration"), Tag("vote")) class `Vote routes` : BaseTest() { - @Test - fun `I can vote article`() { + @TestFactory + fun `I can vote article`(): List { withIntegrationApplication { `Given I have citizen`("Thalès", "Milet") `Given I have article`(id = "835c5101-ca39-4038-a4e6-da6ee62ca6d5") - `When I send a PUT request`("/articles/835c5101-ca39-4038-a4e6-da6ee62ca6d5/vote") { - `authenticated as`("Thalès", "Milet") - `with body`( - """ + } + return (-1..1).map { note -> + DynamicTest.dynamicTest("""I can vote article with note "$note"""") { + withIntegrationApplication { + `When I send a PUT request`("/articles/835c5101-ca39-4038-a4e6-da6ee62ca6d5/vote") { + `authenticated as`("Thalès", "Milet") + `with body`( + """ + { + "note": $note + } + """ + ) + } `Then the response should be` Created + } + } + } + } + + @TestFactory + @Tag("BadRequest") + fun `I cannot vote article with wrong request`(): List { + withIntegrationApplication { + `Given I have citizen`("Thalès", "Milet") + `Given I have article`(id = "835c5101-ca39-4038-a4e6-da6ee62ca6d5") + } + + return listOf(-10, -2, +2, +10).map { note -> + DynamicTest.dynamicTest("""I can vote article with note "$note"""") { + withIntegrationApplication { + `When I send a PUT request`( + "/articles/835c5101-ca39-4038-a4e6-da6ee62ca6d5/vote", + ALL - REQUEST_BODY + ) { + `authenticated as`("Thalès", "Milet") + `with body`( + """ { - "note": 1 + "note": $note } """ - ) - } `Then the response should be` Created + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".note") + `And the response should contain`("$.invalidParams[0].reason", if (note > 0) "must be at most '1'" else "must be at least '-1'") + } + } + } } } @@ -91,7 +132,7 @@ class `Vote routes` : BaseTest() { `Given I have vote +1 on article`("7c9286db-470d-448c-aab1-3f0b072213b1", Name("Carl", "Gauss")) `When I send a GET request`("/citizens/c044823d-e778-4256-9016-b1334bf933d3/votes/articles?page=1&limit=60", ALL - REQUEST_PARAM) { `authenticated as`("Carl", "Gauss") - } `Then the response should be` HttpStatusCode.BadRequest and { + } `Then the response should be` BadRequest and { `And the response should not be null`() `And the response should contain`("$.invalidParams[0].name", ".limit") `And the response should contain`("$.invalidParams[0].reason", "must be at most '50'") From 367f59ee18b5e3f240ba3849dc6ad18c0e4597cf Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 02:33:46 +0200 Subject: [PATCH 35/46] Add validation on route PutVoteOnComment --- .../component/vote/routes/PutVoteOnComment.kt | 27 +++++++++++--- src/main/resources/openapi.yaml | 6 ++++ src/test/kotlin/integration/Vote routes.kt | 35 +++++++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnComment.kt b/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnComment.kt index 8884047..cf65882 100644 --- a/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnComment.kt +++ b/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnComment.kt @@ -1,15 +1,21 @@ package fr.dcproject.component.vote.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest import fr.dcproject.component.auth.citizen import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.auth.mustBeAuth +import fr.dcproject.component.comment.generic.database.CommentRef import fr.dcproject.component.comment.generic.database.CommentRepository import fr.dcproject.component.vote.VoteAccessControl import fr.dcproject.component.vote.database.VoteCommentRepository import fr.dcproject.component.vote.database.VoteForUpdate +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maximum +import io.konform.validation.jsonschema.minimum import io.ktor.application.call +import io.ktor.features.NotFoundException import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI import io.ktor.locations.Location @@ -21,18 +27,29 @@ import java.util.UUID @KtorExperimentalLocationsAPI object PutVoteOnComment { @Location("/comments/{comment}/vote") - class CommentVoteRequest(val comment: UUID) { - data class Content(var note: Int) + class CommentVoteRequest(comment: UUID) { + val comment = CommentRef(comment) + data class Input(var note: Int) { + fun validate() = Validation { + Input::note { + minimum(-1) + maximum(1) + } + }.validate(this) + } } fun Route.putVoteOnComment(voteCommentRepo: VoteCommentRepository, commentRepo: CommentRepository, ac: VoteAccessControl) { put { mustBeAuth() - val comment = commentRepo.findById(it.comment)!! - val content = call.receiveOrBadRequest() + + val comment = commentRepo.findById(it.comment.id) ?: throw NotFoundException("Comment ${it.comment.id} not found") + val input = call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } + val vote = VoteForUpdate( target = comment, - note = content.note, + note = input.note, createdBy = this.citizen ) ac.assert { canCreate(vote, citizenOrNull) } diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 2c3293d..76b8846 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -1252,6 +1252,12 @@ paths: application/json: schema: $ref: '#/components/schemas/VoteAggregation' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' 401: $ref: '#/components/responses/401' /citizens/{citizen}/votes/articles: diff --git a/src/test/kotlin/integration/Vote routes.kt b/src/test/kotlin/integration/Vote routes.kt index ef65a0b..efd54bc 100644 --- a/src/test/kotlin/integration/Vote routes.kt +++ b/src/test/kotlin/integration/Vote routes.kt @@ -181,4 +181,39 @@ class `Vote routes` : BaseTest() { } } } + + @TestFactory + @Tag("BadRequest") + fun `I cannot vote comment with wrong request`(): List { + withIntegrationApplication { + `Given I have citizen`("Antoine", "Lavoisier") + `Given I have article`(id = "835c5101-ca39-4038-a4e6-da6ee62ca6d5") + `Given I have comment on article`( + createdBy = Name("Antoine", "Lavoisier"), + article = "835c5101-ca39-4038-a4e6-da6ee62ca6d5", + id = "e793eccc-456b-4450-a292-46d592229b74", + ) + } + + return listOf(-10, -2, +2, +10).map { note -> + DynamicTest.dynamicTest("""I can vote comment with note "$note"""") { + withIntegrationApplication { + `When I send a PUT request`("/comments/e793eccc-456b-4450-a292-46d592229b74/vote", ALL - REQUEST_BODY) { + `authenticated as`("Antoine", "Lavoisier") + `with body`( + """ + { + "note": $note + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".note") + `And the response should contain`("$.invalidParams[0].reason", if (note > 0) "must be at most '1'" else "must be at least '-1'") + } + } + } + } + } } From 11188668566da551a4a18582ea32a5117d3e8971 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 02:42:29 +0200 Subject: [PATCH 36/46] Add validation on route PutVoteOnConstitution --- .../vote/routes/PutVoteOnConstitution.kt | 18 +++++- src/main/resources/openapi.yaml | 6 ++ src/test/kotlin/integration/Vote routes.kt | 61 +++++++++++++++---- 3 files changed, 71 insertions(+), 14 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnConstitution.kt b/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnConstitution.kt index 03c30ec..0cb6a8d 100644 --- a/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnConstitution.kt +++ b/src/main/kotlin/fr/dcproject/component/vote/routes/PutVoteOnConstitution.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.vote.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest import fr.dcproject.component.auth.citizen @@ -11,6 +12,9 @@ import fr.dcproject.component.vote.VoteAccessControl import fr.dcproject.component.vote.database.VoteConstitutionRepository import fr.dcproject.component.vote.database.VoteForUpdate import fr.dcproject.component.vote.routes.PutVoteOnConstitution.ConstitutionVoteRequest.Input +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maximum +import io.konform.validation.jsonschema.minimum import io.ktor.application.call import io.ktor.features.NotFoundException import io.ktor.http.HttpStatusCode @@ -26,17 +30,25 @@ object PutVoteOnConstitution { @Location("/constitutions/{constitution}/vote") class ConstitutionVoteRequest(constitution: UUID) { val constitution = ConstitutionRef(constitution) - data class Input(var note: Int) + data class Input(var note: Int) { + fun validate() = Validation { + Input::note { + minimum(-1) + maximum(1) + } + }.validate(this) + } } fun Route.voteConstitution(repo: VoteConstitutionRepository, ac: VoteAccessControl, constitutionRepo: ConstitutionRepository) { put { mustBeAuth() val constitution = constitutionRepo.findById(it.constitution.id) ?: throw NotFoundException("Unable to find constitution ${it.constitution.id}") - val content = call.receiveOrBadRequest() + val input = call.receiveOrBadRequest() + .apply { validate().badRequestIfNotValid() } val vote = VoteForUpdate( target = constitution, - note = content.note, + note = input.note, createdBy = this.citizen ) ac.assert { canCreate(vote, citizenOrNull) } diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 76b8846..0abb426 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -1194,6 +1194,12 @@ paths: responses: 201: description: Return only http status 201 on success + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' /citizens/{citizen}/votes: parameters: - $ref: '#/components/parameters/citizen' diff --git a/src/test/kotlin/integration/Vote routes.kt b/src/test/kotlin/integration/Vote routes.kt index efd54bc..a01aaf7 100644 --- a/src/test/kotlin/integration/Vote routes.kt +++ b/src/test/kotlin/integration/Vote routes.kt @@ -88,21 +88,60 @@ class `Vote routes` : BaseTest() { } } - @Test - fun `I can vote constitution`() { + @TestFactory + fun `I can vote constitution`(): List { withIntegrationApplication { `Given I have citizen`("Gregor", "Mendel") `Given I have constitution`(id = "76e79c89-efc1-492d-9e8f-dc9717363a11") - `When I send a PUT request`("/constitutions/76e79c89-efc1-492d-9e8f-dc9717363a11/vote") { - `authenticated as`("Gregor", "Mendel") - `with body`( - """ - { - "note": 1 + } + return (-1..1).map { note -> + DynamicTest.dynamicTest("""I can vote constitution with note "$note"""") { + withIntegrationApplication { + `When I send a PUT request`("/constitutions/76e79c89-efc1-492d-9e8f-dc9717363a11/vote") { + `authenticated as`("Gregor", "Mendel") + `with body`( + """ + { + "note": $note + }² + """ + ) + } `Then the response should be` Created + } + } + } + } + + @TestFactory + @Tag("BadRequest") + fun `I cannot vote constitution with wrong request`(): List { + withIntegrationApplication { + `Given I have citizen`("Gregor", "Mendel") + `Given I have constitution`(id = "76e79c89-efc1-492d-9e8f-dc9717363a11") + } + + return listOf(-10, -2, +2, +10).map { note -> + DynamicTest.dynamicTest("""I can vote constitution with note "$note"""") { + withIntegrationApplication { + `When I send a PUT request`( + "/constitutions/76e79c89-efc1-492d-9e8f-dc9717363a11/vote", + ALL - REQUEST_BODY + ) { + `authenticated as`("Gregor", "Mendel") + `with body`( + """ + { + "note": $note + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".note") + `And the response should contain`("$.invalidParams[0].reason", if (note > 0) "must be at most '1'" else "must be at least '-1'") } - """ - ) - } `Then the response should be` Created + } + } } } From 87175eb8eab87fe27ef7d7663d71e82a92854a2d Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 02:53:32 +0200 Subject: [PATCH 37/46] clean route comment --- .../comment/generic/database/CommentRepository.kt | 2 +- .../component/comment/generic/routes/GetCommentChildren.kt | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt index b5fdf8a..8a86a7b 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/generic/database/CommentRepository.kt @@ -21,7 +21,7 @@ abstract class CommentRepositoryAbs(override var requester: Request ): Paginated> open fun findByParent( - parent: CommentForView, + parent: CommentI, page: Int = 1, limit: Int = 50 ): Paginated> { diff --git a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/GetCommentChildren.kt b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/GetCommentChildren.kt index 52d1c1e..a478f0d 100644 --- a/src/main/kotlin/fr/dcproject/component/comment/generic/routes/GetCommentChildren.kt +++ b/src/main/kotlin/fr/dcproject/component/comment/generic/routes/GetCommentChildren.kt @@ -4,6 +4,7 @@ import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.comment.generic.CommentAccessControl +import fr.dcproject.component.comment.generic.database.CommentRef import fr.dcproject.component.comment.generic.database.CommentRepository import fr.dcproject.component.comment.toOutput import fr.dcproject.routes.PaginatedRequest @@ -21,11 +22,13 @@ import java.util.UUID object GetCommentChildren { @Location("/comments/{comment}/children") class CommentChildrenRequest( - val comment: UUID, + comment: UUID, page: Int = 1, limit: Int = 50, val search: String? = null - ) : PaginatedRequestI by PaginatedRequest(page, limit) + ) : PaginatedRequestI by PaginatedRequest(page, limit) { + val comment = CommentRef(comment) + } fun Route.getChildrenComments(repo: CommentRepository, ac: CommentAccessControl) { get { From 596b7ff0c97e294863d35c0f02ba05011a3e3b0d Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 20:52:34 +0200 Subject: [PATCH 38/46] Extract Workgroup Member test into external file --- .../integration/Workgroup Members routes.kt | 118 ++++++++++++++++++ .../kotlin/integration/Workgroup routes.kt | 93 -------------- 2 files changed, 118 insertions(+), 93 deletions(-) create mode 100644 src/test/kotlin/integration/Workgroup Members routes.kt diff --git a/src/test/kotlin/integration/Workgroup Members routes.kt b/src/test/kotlin/integration/Workgroup Members routes.kt new file mode 100644 index 0000000..49034b0 --- /dev/null +++ b/src/test/kotlin/integration/Workgroup Members routes.kt @@ -0,0 +1,118 @@ +package integration + +import fr.dcproject.component.citizen.database.CitizenI.Name +import integration.steps.`when`.`When I send a DELETE request` +import integration.steps.`when`.`When I send a POST request` +import integration.steps.`when`.`When I send a PUT request` +import integration.steps.`when`.`with body` +import integration.steps.given.`Given I have citizen` +import integration.steps.given.`Given I have workgroup` +import integration.steps.given.`With members` +import integration.steps.given.`authenticated as` +import integration.steps.then.`And the response should contain list` +import integration.steps.then.`And the response should contain` +import integration.steps.then.`Then the response should be` +import integration.steps.then.and +import io.ktor.http.HttpStatusCode.Companion.Created +import io.ktor.http.HttpStatusCode.Companion.OK +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Tags +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@Tags(Tag("integration"), Tag("workgroup"), Tag("workgroupMember")) +class `Workgroup Members routes` : BaseTest() { + @Test + fun `I can add member to workgroup`() { + withIntegrationApplication { + `Given I have citizen`("Blaise", "Pascal") + `Given I have citizen`("Roger", "Penrose", id = "6d883fe7-5fc0-4a50-8858-72230673eba4") + `Given I have citizen`("Alessandro", "Volta", id = "b5bac515-45d4-4aeb-9b6d-2627a0bbc419") + `Given I have workgroup`("b0ea1922-3bc6-44e2-aa7c-40158998cfbb", createdBy = Name("Blaise", "Pascal")) + `When I send a POST request`("/workgroups/b0ea1922-3bc6-44e2-aa7c-40158998cfbb/members") { + `authenticated as`("Blaise", "Pascal") + `with body`( + """ + [ + { + "citizen": {"id":"6d883fe7-5fc0-4a50-8858-72230673eba4"}, + "roles": ["MASTER"] + }, + { + "citizen": {"id":"b5bac515-45d4-4aeb-9b6d-2627a0bbc419"}, + "roles": ["MASTER"] + } + ] + """ + ) + } `Then the response should be` Created + } + } + + @Test + fun `I can remove member to workgroup`() { + withIntegrationApplication { + `Given I have citizen`("Heinrich", "Hertz", id = "94f92424-c257-4582-907c-98564a8c4ac9") + `Given I have citizen`("William", "Thomson", id = "87909ba3-2069-431c-9924-219fd8411cf2") + `Given I have citizen`("Paul", "Dirac", id = "1baf48bb-02bc-4d8f-ac86-33335354f5e7") + `Given I have workgroup`("b6c975df-dd44-4e99-adc1-f605746b0e11", createdBy = Name("Heinrich", "Hertz")) { + `With members`( + Name("William", "Thomson"), + Name("Paul", "Dirac"), + ) + } + `When I send a DELETE request`("/workgroups/b6c975df-dd44-4e99-adc1-f605746b0e11/members") { + `authenticated as`("Heinrich", "Hertz") + """ + [ + { + "citizen": {"id":"87909ba3-2069-431c-9924-219fd8411cf2"} + } + ] + """ + } `Then the response should be` OK and { + `And the response should contain list`("$", 2) + `And the response should contain`("$.[0]citizen.id", "94f92424-c257-4582-907c-98564a8c4ac9") + `And the response should contain`("$.[1]citizen.id", "1baf48bb-02bc-4d8f-ac86-33335354f5e7") + } + } + } + + @Test + fun `I can update members on workgroup`() { + withIntegrationApplication { + `Given I have citizen`("Leon", "Foucault") + `Given I have citizen`("Sadi", "Carnot", id = "be3b0926-8628-4426-804a-75188a6eb315") + `Given I have citizen`("Joseph", "Fourier", id = "b49e20c1-8393-45d6-a6a0-3fa5c71cbdc1") + `Given I have citizen`("Georg", "Ohm") + `Given I have workgroup`("784fe6bc-7635-4ae2-b080-3a4743b998bf", createdBy = Name("Leon", "Foucault")) { + `With members`( + Name("Sadi", "Carnot"), + Name("Joseph", "Fourier"), + ) + } + `When I send a PUT request`("/workgroups/784fe6bc-7635-4ae2-b080-3a4743b998bf/members") { + `authenticated as`("Leon", "Foucault") + `with body`( + """ + [ + { + "citizen": {"id":"be3b0926-8628-4426-804a-75188a6eb315"}, + "roles": ["MASTER"] + }, + { + "citizen": {"id":"b49e20c1-8393-45d6-a6a0-3fa5c71cbdc1"}, + "roles": ["MASTER"] + } + ] + """ + ) + } `Then the response should be` OK and { + `And the response should contain list`("$", 2) + `And the response should contain`("$.[0]citizen.id", "be3b0926-8628-4426-804a-75188a6eb315") + `And the response should contain`("$.[1]citizen.id", "b49e20c1-8393-45d6-a6a0-3fa5c71cbdc1") + } + } + } +} diff --git a/src/test/kotlin/integration/Workgroup routes.kt b/src/test/kotlin/integration/Workgroup routes.kt index 32b69d7..fda1f5e 100644 --- a/src/test/kotlin/integration/Workgroup routes.kt +++ b/src/test/kotlin/integration/Workgroup routes.kt @@ -165,97 +165,4 @@ class `Workgroup routes` : BaseTest() { } } } - - @Test - fun `I can add member to workgroup`() { - withIntegrationApplication { - `Given I have citizen`("Blaise", "Pascal") - `Given I have citizen`("Roger", "Penrose", id = "6d883fe7-5fc0-4a50-8858-72230673eba4") - `Given I have citizen`("Alessandro", "Volta", id = "b5bac515-45d4-4aeb-9b6d-2627a0bbc419") - `Given I have workgroup`("b0ea1922-3bc6-44e2-aa7c-40158998cfbb", createdBy = Name("Blaise", "Pascal")) - `When I send a POST request`("/workgroups/b0ea1922-3bc6-44e2-aa7c-40158998cfbb/members") { - `authenticated as`("Blaise", "Pascal") - `with body`( - """ - [ - { - "citizen": {"id":"6d883fe7-5fc0-4a50-8858-72230673eba4"}, - "roles": ["MASTER"] - }, - { - "citizen": {"id":"b5bac515-45d4-4aeb-9b6d-2627a0bbc419"}, - "roles": ["MASTER"] - } - ] - """ - ) - } `Then the response should be` Created - } - } - - @Test - fun `I can remove member to workgroup`() { - withIntegrationApplication { - `Given I have citizen`("Heinrich", "Hertz", id = "94f92424-c257-4582-907c-98564a8c4ac9") - `Given I have citizen`("William", "Thomson", id = "87909ba3-2069-431c-9924-219fd8411cf2") - `Given I have citizen`("Paul", "Dirac", id = "1baf48bb-02bc-4d8f-ac86-33335354f5e7") - `Given I have workgroup`("b6c975df-dd44-4e99-adc1-f605746b0e11", createdBy = Name("Heinrich", "Hertz")) { - `With members`( - Name("William", "Thomson"), - Name("Paul", "Dirac"), - ) - } - `When I send a DELETE request`("/workgroups/b6c975df-dd44-4e99-adc1-f605746b0e11/members") { - `authenticated as`("Heinrich", "Hertz") - """ - [ - { - "citizen": {"id":"87909ba3-2069-431c-9924-219fd8411cf2"} - } - ] - """ - } `Then the response should be` OK and { - `And the response should contain list`("$", 2) - `And the response should contain`("$.[0]citizen.id", "94f92424-c257-4582-907c-98564a8c4ac9") - `And the response should contain`("$.[1]citizen.id", "1baf48bb-02bc-4d8f-ac86-33335354f5e7") - } - } - } - - @Test - fun `I can update members on workgroup`() { - withIntegrationApplication { - `Given I have citizen`("Leon", "Foucault") - `Given I have citizen`("Sadi", "Carnot", id = "be3b0926-8628-4426-804a-75188a6eb315") - `Given I have citizen`("Joseph", "Fourier", id = "b49e20c1-8393-45d6-a6a0-3fa5c71cbdc1") - `Given I have citizen`("Georg", "Ohm") - `Given I have workgroup`("784fe6bc-7635-4ae2-b080-3a4743b998bf", createdBy = Name("Leon", "Foucault")) { - `With members`( - Name("Sadi", "Carnot"), - Name("Joseph", "Fourier"), - ) - } - `When I send a PUT request`("/workgroups/784fe6bc-7635-4ae2-b080-3a4743b998bf/members") { - `authenticated as`("Leon", "Foucault") - `with body`( - """ - [ - { - "citizen": {"id":"be3b0926-8628-4426-804a-75188a6eb315"}, - "roles": ["MASTER"] - }, - { - "citizen": {"id":"b49e20c1-8393-45d6-a6a0-3fa5c71cbdc1"}, - "roles": ["MASTER"] - } - ] - """ - ) - } `Then the response should be` OK and { - `And the response should contain list`("$", 2) - `And the response should contain`("$.[0]citizen.id", "be3b0926-8628-4426-804a-75188a6eb315") - `And the response should contain`("$.[1]citizen.id", "b49e20c1-8393-45d6-a6a0-3fa5c71cbdc1") - } - } - } } From 518b59e9aa17d6bed1af5834338f41ee8b37311e Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 21:20:15 +0200 Subject: [PATCH 39/46] Add validation on route GetWorkgroups --- build.gradle.kts | 6 +++ .../workgroup/routes/GetWorkgroups.kt | 49 ++++++++++++++----- src/main/resources/openapi.yaml | 6 +++ .../functions/workgroup/find_workgroups.sql | 6 +-- .../kotlin/integration/Workgroup routes.kt | 19 ++++++- .../kotlin/integration/steps/when/request.kt | 3 ++ 6 files changed, 73 insertions(+), 16 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 3aff791..c293ef2 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -368,6 +368,12 @@ tasks.register("testVotes", Test::class) { includeTags("vote") } } +tasks.register("testWorkgroup", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("workgroup") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/component/workgroup/routes/GetWorkgroups.kt b/src/main/kotlin/fr/dcproject/component/workgroup/routes/GetWorkgroups.kt index 40f79ba..be8a606 100644 --- a/src/main/kotlin/fr/dcproject/component/workgroup/routes/GetWorkgroups.kt +++ b/src/main/kotlin/fr/dcproject/component/workgroup/routes/GetWorkgroups.kt @@ -1,12 +1,20 @@ package fr.dcproject.component.workgroup.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.auth.citizenOrNull import fr.dcproject.component.workgroup.WorkgroupAccessControl import fr.dcproject.component.workgroup.database.WorkgroupRepository +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.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -27,23 +35,40 @@ object GetWorkgroups { val search: String? = null, val createdBy: String? = null, members: List? = null - ) { - val page: Int = if (page < 1) 1 else page - val limit: Int = if (limit > 50) 50 else if (limit < 1) 1 else limit + ) : PaginatedRequestI by PaginatedRequest(page, limit) { val members: List? = members?.toUUID() + fun validate() = Validation { + WorkgroupsRequest::page { + minimum(1) + } + WorkgroupsRequest::limit { + minimum(1) + maximum(50) + } + WorkgroupsRequest::sort ifPresent { + enum( + "name", + "createdAt", + ) + } + WorkgroupsRequest::createdBy ifPresent { + isUuid() + } + }.validate(this) } fun Route.getWorkgroups(repo: WorkgroupRepository, ac: WorkgroupAccessControl) { get { - val workgroups = - repo.find( - it.page, - it.limit, - it.sort, - it.direction, - it.search, - WorkgroupRepository.Filter(createdById = it.createdBy, members = it.members) - ) + it.validate().badRequestIfNotValid() + + val workgroups = repo.find( + it.page, + it.limit, + it.sort, + it.direction, + it.search, + WorkgroupRepository.Filter(createdById = it.createdBy, members = it.members) + ) ac.assert { canView(workgroups.result, citizenOrNull) } call.respond( HttpStatusCode.OK, diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 0abb426..352de91 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -1359,6 +1359,12 @@ paths: type: array items: $ref: '#/components/schemas/WorkgroupListing' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' post: summary: Create new Workgroup security: diff --git a/src/main/resources/sql/functions/workgroup/find_workgroups.sql b/src/main/resources/sql/functions/workgroup/find_workgroups.sql index 8a1d86b..3600ed2 100644 --- a/src/main/resources/sql/functions/workgroup/find_workgroups.sql +++ b/src/main/resources/sql/functions/workgroup/find_workgroups.sql @@ -2,7 +2,7 @@ create or replace function find_workgroups( _search text default null, _filter json default '{}', direction text default 'desc', - sort text default 'created_at', + sort text default 'createdAt', "limit" int default 50, "offset" int default 0, out resource json, @@ -41,14 +41,14 @@ begin case direction when 'asc' then case sort when 'name' then w.name - when 'created_at' then w.created_at::text + when 'createdAt' then w.created_at::text else null end end, case direction when 'desc' then case sort when 'name' then w.name - when 'created_at' then w.created_at::text + when 'createdAt' then w.created_at::text end end desc, diff --git a/src/test/kotlin/integration/Workgroup routes.kt b/src/test/kotlin/integration/Workgroup routes.kt index fda1f5e..febebf2 100644 --- a/src/test/kotlin/integration/Workgroup routes.kt +++ b/src/test/kotlin/integration/Workgroup routes.kt @@ -1,6 +1,7 @@ package integration import fr.dcproject.component.citizen.database.CitizenI.Name +import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a DELETE request` import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` @@ -15,8 +16,10 @@ import integration.steps.then.`And have property` import integration.steps.then.`And the response should be null` import integration.steps.then.`And the response should contain list` import integration.steps.then.`And the response should contain` +import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` import integration.steps.then.and +import io.ktor.http.HttpStatusCode.Companion.BadRequest import io.ktor.http.HttpStatusCode.Companion.Created import io.ktor.http.HttpStatusCode.Companion.NoContent import io.ktor.http.HttpStatusCode.Companion.NotFound @@ -157,7 +160,7 @@ class `Workgroup routes` : BaseTest() { withIntegrationApplication { `Given I have citizen`("Max", "Planck") `Given I have workgroup`("3fd8edb6-c4b4-4c94-bc75-ddd9b290d32c") - `When I send a GET request`("/workgroups") { + `When I send a GET request`("/workgroups?page=1&limit=10&sort=createdAt") { `authenticated as`("Max", "Planck") `with no content`() } `Then the response should be` OK and { @@ -165,4 +168,18 @@ class `Workgroup routes` : BaseTest() { } } } + + @Test + @Tag("BadRequest") + fun `I cannot get workgroups list with wrong request`() { + withIntegrationApplication { + `Given I have workgroup`("3fd8edb6-c4b4-4c94-bc75-ddd9b290d32c") + `When I send a GET request`("/workgroups?sort=plop", -REQUEST_PARAM) { + } `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`("$.invalidParams[0].reason", "must be one of: 'name', 'createdAt'") + } + } + } } diff --git a/src/test/kotlin/integration/steps/when/request.kt b/src/test/kotlin/integration/steps/when/request.kt index ca428c8..fe71e72 100644 --- a/src/test/kotlin/integration/steps/when/request.kt +++ b/src/test/kotlin/integration/steps/when/request.kt @@ -1,5 +1,6 @@ package integration.steps.`when` +import fr.dcproject.common.BitMask import fr.dcproject.common.BitMaskI import integration.steps.then.`And the schema parameters must be valid` import integration.steps.then.`And the schema request body must be valid` @@ -23,6 +24,8 @@ enum class Validate(override val bit: Long) : BitMaskI { RESPONSE_HEADER(16), RESPONSE(8 + 16), ALL((1 + 2 + 4) + (8 + 16)); + + operator fun unaryMinus(): BitMaskI = ALL - BitMask(this.bit) } fun TestApplicationCall.valid(validate: BitMaskI): TestApplicationCall { From 3f392eece67baf1ddc77c5b1a770127a45bbb275 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Thu, 15 Apr 2021 23:45:47 +0200 Subject: [PATCH 40/46] Add validation on route EditWorkgroup --- .../fr/dcproject/common/validation/Url.kt | 15 +++++++ .../workgroup/routes/EditWorkgroup.kt | 23 +++++++++- src/main/resources/openapi.yaml | 6 +++ .../kotlin/integration/Workgroup routes.kt | 44 +++++++++++++++++-- 4 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 src/main/kotlin/fr/dcproject/common/validation/Url.kt diff --git a/src/main/kotlin/fr/dcproject/common/validation/Url.kt b/src/main/kotlin/fr/dcproject/common/validation/Url.kt new file mode 100644 index 0000000..e7e2cc6 --- /dev/null +++ b/src/main/kotlin/fr/dcproject/common/validation/Url.kt @@ -0,0 +1,15 @@ +package fr.dcproject.common.validation + +import io.konform.validation.ValidationBuilder +import java.net.MalformedURLException +import java.net.URL + +fun ValidationBuilder.isUrl() = + addConstraint("is not url") { + try { + val url = URL(it) + true + } catch (e: MalformedURLException) { + false + } + } diff --git a/src/main/kotlin/fr/dcproject/component/workgroup/routes/EditWorkgroup.kt b/src/main/kotlin/fr/dcproject/component/workgroup/routes/EditWorkgroup.kt index 4d96298..b116b7d 100644 --- a/src/main/kotlin/fr/dcproject/component/workgroup/routes/EditWorkgroup.kt +++ b/src/main/kotlin/fr/dcproject/component/workgroup/routes/EditWorkgroup.kt @@ -1,13 +1,18 @@ package fr.dcproject.component.workgroup.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest +import fr.dcproject.common.validation.isUrl import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.auth.mustBeAuth import fr.dcproject.component.workgroup.WorkgroupAccessControl import fr.dcproject.component.workgroup.database.WorkgroupForUpdate import fr.dcproject.component.workgroup.database.WorkgroupRepository import fr.dcproject.component.workgroup.routes.EditWorkgroup.PutWorkgroupRequest.Input +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -27,7 +32,22 @@ object EditWorkgroup { val description: String?, val logo: String?, val anonymous: Boolean? - ) + ) { + fun validate() = Validation { + Input::name ifPresent { + minLength(5) + maxLength(80) + } + Input::description ifPresent { + minLength(50) + maxLength(6000) + } + Input::logo ifPresent { + isUrl() + maxLength(2048) + } + }.validate(this) + } } fun Route.editWorkgroup(repo: WorkgroupRepository, ac: WorkgroupAccessControl) { @@ -35,6 +55,7 @@ object EditWorkgroup { mustBeAuth() repo.findById(it.workgroupId)?.let { old -> call.receiveOrBadRequest().run { + validate().badRequestIfNotValid() WorkgroupForUpdate( id = old.id, name = name ?: old.name, diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index 352de91..de03709 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -1451,6 +1451,12 @@ paths: application/json: schema: $ref: '#/components/schemas/Workgroup' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' delete: summary: Delete one workgroup security: diff --git a/src/test/kotlin/integration/Workgroup routes.kt b/src/test/kotlin/integration/Workgroup routes.kt index febebf2..c6182c4 100644 --- a/src/test/kotlin/integration/Workgroup routes.kt +++ b/src/test/kotlin/integration/Workgroup routes.kt @@ -1,6 +1,7 @@ package integration import fr.dcproject.component.citizen.database.CitizenI.Name +import integration.steps.`when`.Validate.REQUEST_BODY import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a DELETE request` import integration.steps.`when`.`When I send a GET request` @@ -112,14 +113,15 @@ class `Workgroup routes` : BaseTest() { """ { "name":"La ratatouille", - "description":"Une petite souris" + "description":"Une petite souris avec un chapeau et qui aime la cuisine", + "logo": "http://sdf@exemple.com/sdfsd?sdf=sss" } """ ) } `Then the response should be` OK and { `And the response should contain`("$.id", "aa875a24-0050-4252-9130-d37391714e26") `And the response should contain`("$.name", "La ratatouille") - `And the response should contain`("$.description", "Une petite souris") + `And the response should contain`("$.description", "Une petite souris avec un chapeau et qui aime la cuisine") `And have property`("$.members") `And the response should contain list`("$.members", 3) @@ -132,7 +134,43 @@ class `Workgroup routes` : BaseTest() { } `Then the response should be` OK and { `And the response should contain`("$.id", "aa875a24-0050-4252-9130-d37391714e26") `And the response should contain`("$.name", "La ratatouille") - `And the response should contain`("$.description", "Une petite souris") + `And the response should contain`("$.description", "Une petite souris avec un chapeau et qui aime la cuisine") + } + } + } + + @Test + @Tag("BadRequest") + fun `I cannot edit a workgroup with bad request`() { + withIntegrationApplication { + `Given I have citizen`("John", "Wheeler") + `Given I have citizen`("Heinrich", "Hertz", id = "94f92424-c257-4582-907c-98564a8c4ac9") + `Given I have citizen`("William", "Thomson", id = "87909ba3-2069-431c-9924-219fd8411cf2") + `Given I have workgroup`("aa875a24-0050-4252-9130-d37391714e26", createdBy = Name("John", "Wheeler")) { + `With members`( + Name("Heinrich", "Hertz"), + Name("William", "Thomson"), + ) + } + `When I send a PUT request`("/workgroups/aa875a24-0050-4252-9130-d37391714e26", -REQUEST_BODY) { + `authenticated as`("John", "Wheeler") + `with body`( + """ + { + "name":"sm", + "description":"small2", + "logo": "ws://sdfs.sdok" + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".name") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 5 characters") + `And the response should contain`("$.invalidParams[1].name", ".description") + `And the response should contain`("$.invalidParams[1].reason", "must have at least 50 characters") + `And the response should contain`("$.invalidParams[2].name", ".logo") + `And the response should contain`("$.invalidParams[2].reason", "is not url") } } } From 994e266b526c1b9c9334f7ec410d684435bcf383 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 16 Apr 2021 00:08:57 +0200 Subject: [PATCH 41/46] Add validation on route CreateWorkgroup --- .../workgroup/routes/CreateWorkgroup.kt | 25 ++++++++++++-- src/main/resources/openapi.yaml | 6 ++++ .../kotlin/integration/Workgroup routes.kt | 34 +++++++++++++++++-- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/component/workgroup/routes/CreateWorkgroup.kt b/src/main/kotlin/fr/dcproject/component/workgroup/routes/CreateWorkgroup.kt index 4c7f2b1..fcec099 100644 --- a/src/main/kotlin/fr/dcproject/component/workgroup/routes/CreateWorkgroup.kt +++ b/src/main/kotlin/fr/dcproject/component/workgroup/routes/CreateWorkgroup.kt @@ -1,8 +1,9 @@ package fr.dcproject.component.workgroup.routes -import fr.dcproject.common.response.toOutput +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest +import fr.dcproject.common.validation.isUrl import fr.dcproject.component.auth.citizen import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.auth.mustBeAuth @@ -10,6 +11,9 @@ import fr.dcproject.component.workgroup.WorkgroupAccessControl import fr.dcproject.component.workgroup.database.WorkgroupForUpdate import fr.dcproject.component.workgroup.database.WorkgroupRepository import fr.dcproject.component.workgroup.routes.CreateWorkgroup.PostWorkgroupRequest.Input +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -29,13 +33,30 @@ object CreateWorkgroup { val description: String, val logo: String?, val anonymous: Boolean? - ) + ) { + fun validate() = Validation { + Input::name { + minLength(5) + maxLength(80) + } + Input::description { + minLength(50) + maxLength(6000) + } + Input::logo ifPresent { + isUrl() + maxLength(2048) + } + }.validate(this) + } } fun Route.createWorkgroup(repo: WorkgroupRepository, ac: WorkgroupAccessControl) { post { mustBeAuth() call.receiveOrBadRequest().run { + validate().badRequestIfNotValid() + WorkgroupForUpdate( id ?: UUID.randomUUID(), name, diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index de03709..6afa21e 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -1403,6 +1403,12 @@ paths: application/json: schema: $ref: '#/components/schemas/Workgroup' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' /workgroups/{workgroup}: parameters: - $ref: '#/components/parameters/workgroup' diff --git a/src/test/kotlin/integration/Workgroup routes.kt b/src/test/kotlin/integration/Workgroup routes.kt index c6182c4..6a7adfb 100644 --- a/src/test/kotlin/integration/Workgroup routes.kt +++ b/src/test/kotlin/integration/Workgroup routes.kt @@ -77,7 +77,7 @@ class `Workgroup routes` : BaseTest() { { "id":"f496d86d-6654-4068-91ff-90e1dbcc5f38", "name":"Les Bouffons", - "description":"La vie est belle", + "description":"Pellentesque eleifend malesuada aliquam. Maecenas et urna quis nunc lacinia scelerisque.", "anonymous":false } """ @@ -85,7 +85,7 @@ class `Workgroup routes` : BaseTest() { } `Then the response should be` Created and { `And the response should contain`("$.id", "f496d86d-6654-4068-91ff-90e1dbcc5f38") `And the response should contain`("$.name", "Les Bouffons") - `And the response should contain`("$.description", "La vie est belle") + `And the response should contain`("$.description", "Pellentesque eleifend malesuada aliquam. Maecenas et urna quis nunc lacinia scelerisque.") `And the response should contain`("$.anonymous", false) } @@ -95,6 +95,36 @@ class `Workgroup routes` : BaseTest() { } } + @Test + @Tag("BadRequest") + fun `I cannot create a workgroup with wrong request`() { + withIntegrationApplication { + `Given I have citizen`("Werner", "Heisenberg") + `When I send a POST request`("/workgroups") { + `authenticated as`("Werner", "Heisenberg") + `with body`( + """ + { + "id":"f496d86d-6654-4068-91ff-90e1dbcc5f38", + "name":"sm", + "description":"small", + "anonymous":false, + "logo": "www.plop.com" + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".name") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 5 characters") + `And the response should contain`("$.invalidParams[1].name", ".description") + `And the response should contain`("$.invalidParams[1].reason", "must have at least 50 characters") + `And the response should contain`("$.invalidParams[2].name", ".logo") + `And the response should contain`("$.invalidParams[2].reason", "is not url") + } + } + } + @Test fun `I can edit a workgroup`() { withIntegrationApplication { From 543f3fb9bbaeaa5b2456949ac3d6d31aa44d8570 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 16 Apr 2021 02:30:58 +0200 Subject: [PATCH 42/46] Add retry for viewsTest --- build.gradle.kts | 6 +++ .../kotlin/fr/dcproject/common/utils/retry.kt | 41 +++++++++++++++++++ src/test/kotlin/functional/ViewTest.kt | 20 +++++---- 3 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 src/main/kotlin/fr/dcproject/common/utils/retry.kt diff --git a/build.gradle.kts b/build.gradle.kts index c293ef2..9364b49 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -374,6 +374,12 @@ tasks.register("testWorkgroup", Test::class) { includeTags("workgroup") } } +tasks.register("testViews", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("view") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/common/utils/retry.kt b/src/main/kotlin/fr/dcproject/common/utils/retry.kt new file mode 100644 index 0000000..2ccbb93 --- /dev/null +++ b/src/main/kotlin/fr/dcproject/common/utils/retry.kt @@ -0,0 +1,41 @@ +package fr.dcproject.common.utils + +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import kotlin.time.Duration +import kotlin.time.ExperimentalTime + +@ExperimentalTime +fun retry(numOfRetries: Int, duration: Duration = Duration.ZERO, block: (RetryContext) -> T): T { + val logger: Logger = LoggerFactory.getLogger("fr.dcproject.utils.retry") + var throwable: Throwable? = null + for (attempt in 1..numOfRetries) { + val context = RetryContext() + try { + val output = block(context) + if (context.hasStop()) { + break + } + return output + } catch (e: Throwable) { + throwable = e + logger.debug("Failed attempt $attempt / $numOfRetries. Wait ${duration.inSeconds} seconds") + Thread.sleep(duration.inMilliseconds.toLong()) + } finally { + if (context.hasStop()) { + break + } + } + } + throw throwable!! +} + +class RetryContext() { + var stoped = false + + fun stop() { + stoped = true + } + + fun hasStop(): Boolean = stoped +} diff --git a/src/test/kotlin/functional/ViewTest.kt b/src/test/kotlin/functional/ViewTest.kt index d8a74d7..5e8df91 100644 --- a/src/test/kotlin/functional/ViewTest.kt +++ b/src/test/kotlin/functional/ViewTest.kt @@ -2,6 +2,7 @@ package functional import fr.dcproject.application.Env.TEST import fr.dcproject.application.module +import fr.dcproject.common.utils.retry import fr.dcproject.component.article.database.ArticleForView import fr.dcproject.component.article.database.ArticleViewRepository import fr.dcproject.component.auth.database.UserCreator @@ -20,6 +21,8 @@ import org.junit.jupiter.api.TestInstance import org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS import org.koin.ktor.ext.get import java.util.UUID +import kotlin.time.ExperimentalTime +import kotlin.time.seconds @KtorExperimentalLocationsAPI @KtorExperimentalAPI @@ -27,6 +30,7 @@ import java.util.UUID @TestInstance(PER_CLASS) @Tags(Tag("functional"), Tag("view")) class ViewTest { + @ExperimentalTime @Test fun `test View Article`() { val article = ArticleForView( @@ -75,15 +79,15 @@ class ViewTest { article ) - /* Sleep because ES is not sync ! */ - Thread.sleep(1000) + /* Retry because ES is not sync ! */ + retry(10, 0.3.seconds) { + /* Get view */ + val afterView = viewRepository.getViewsCount(article) - /* Get view */ - val afterView = viewRepository.getViewsCount(article) - - /* Check if view has increment */ - afterView.total `should be equal to` startView.total + 4 - afterView.unique `should be equal to` startView.unique + 3 + /* Check if view has increment */ + afterView.total `should be equal to` startView.total + 4 + afterView.unique `should be equal to` startView.unique + 3 + } } } } From 242bf9c9b3291a086a6e0867ba7dddb8b2533722 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 16 Apr 2021 02:45:13 +0200 Subject: [PATCH 43/46] Fix commentSqlTest --- src/test/resources/sql/comment.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/resources/sql/comment.sql b/src/test/resources/sql/comment.sql index 5903c63..95dbb20 100644 --- a/src/test/resources/sql/comment.sql +++ b/src/test/resources/sql/comment.sql @@ -22,7 +22,7 @@ begin select "comment"( reference => 'article'::regclass, resource => _comment - ) into _comment_id; + )->>'id' into _comment_id; assert (select count(*) = 1 from "comment"), 'comment must be inserted, "' || (select count(*) from "comment") || '" exist'; assert (select com.content = 'Ho my god !' from "comment" com), 'the content of comment must be "Ho my god !" instead of "' || (select com.content from "comment" as com) || '"'; @@ -67,7 +67,7 @@ begin select "comment"( reference => 'article'::regclass, resource => _comment - ) into _comment_id_response; + )->>'id' into _comment_id_response; _comment = json_build_object( @@ -80,7 +80,7 @@ begin select "comment"( reference => 'article'::regclass, resource => _comment - ) into _comment_id_response2; + )->>'id' into _comment_id_response2; assert (select count(*) = 3 from "comment"), 'response must be inserted'; assert (select com.parents_ids @> ARRAY[_comment_id] from "comment" com where id = _comment_id_response), 'parents_ids not contain "' || _comment_id::text || '" ' || (select com.parents_ids::text[] from "comment" com where id = _comment_id_response); assert (select com.parents_ids @> ARRAY[_comment_id_response] from "comment" com where id = _comment_id_response2), 'parents_ids not contain "' || _comment_id_response::text || '" ' || (select com.parents_ids::text[] from "comment" com where id = _comment_id_response2); From e474a40068682e22dddbdc6b484c22679ec0c3f2 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 16 Apr 2021 03:00:37 +0200 Subject: [PATCH 44/46] Change tests task and CI --- .github/workflows/tests.yml | 21 ++++++++++++++++++++- build.gradle.kts | 26 ++++++++++---------------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 1eb8c3d..0a89778 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -69,6 +69,13 @@ jobs: with: name: Build path: build + + - name: Composer Up + uses: eskatos/gradle-command-action@v1 + with: + gradle-version: 6.8 + arguments: testSqlComposeUp + - name: TestSql uses: eskatos/gradle-command-action@v1 with: @@ -81,19 +88,29 @@ jobs: steps: - uses: actions/checkout@v2 + - name: Set up JDK 11 uses: actions/setup-java@v1 with: java-version: 11 + - uses: actions/download-artifact@v2 with: name: Build path: build + + - name: Composer Up + uses: eskatos/gradle-command-action@v1 + with: + gradle-version: 6.8 + arguments: testComposeUp + - name: Test uses: eskatos/gradle-command-action@v1 with: gradle-version: 6.8 - arguments: test -x testSql + arguments: test + - name: Coverage uses: eskatos/gradle-command-action@v1 with: @@ -101,12 +118,14 @@ jobs: arguments: coveralls env: COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }} + - name: Cache SonarCloud packages uses: actions/cache@v1 with: path: ~/.sonar/cache key: ${{ runner.os }}-sonar restore-keys: ${{ runner.os }}-sonar + - name: Build and analyze env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any diff --git a/build.gradle.kts b/build.gradle.kts index 9364b49..bee0958 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -94,7 +94,7 @@ val migration by tasks.registering { } val migrationTest by tasks.registering { - group = "verification" + group = "tests" dependsOn(tasks.named("testComposeUp")) finalizedBy(tasks.named("testComposeDown")) doLast { @@ -118,11 +118,9 @@ val migrationTest by tasks.registering { } val testSql by tasks.registering { - group = "verification" + group = "tests" dependsOn(tasks.named("processResources")) dependsOn(tasks.named("processTestResources")) - dependsOn(tasks.named("testSqlComposeUp")) - finalizedBy(tasks.named("testSqlComposeDown")) doLast { val config = ConfigFactory.parseFile(file("$buildDir/resources/test/application-test.conf")).resolve() @@ -197,8 +195,7 @@ val sourcesJar by tasks.registering(Jar::class) { tasks.test { useJUnit() useJUnitPlatform() - // systemProperty("junit.jupiter.execution.parallel.enabled", true) - dependsOn(testSql) + systemProperty("junit.jupiter.execution.parallel.enabled", true) finalizedBy(tasks.jacocoTestReport) // report is always generated after tests run } @@ -206,13 +203,6 @@ coveralls { sourceDirs.add("src/main/kotlin") } -tasks.register("testAll") { - group = "verification" - dependsOn(testSql) - dependsOn(tasks.test) - dependsOn(tasks.ktlintCheck) -} - apply(plugin = "docker-compose") dockerCompose { projectName = "dc-project" @@ -228,14 +218,12 @@ dockerCompose { useComposeFiles = listOf("docker-compose-test.yml") startedServices = listOf("db", "elasticsearch") stopContainers = false - isRequiredBy(project.tasks.named("testSql")) } createNested("test").apply { projectName = "dc-project_test" useComposeFiles = listOf("docker-compose-test.yml") stopContainers = false - isRequiredBy(project.tasks.test) } } @@ -320,6 +308,12 @@ tasks.named("testComposeUp").configure { } } +tasks.register("testWithDependencies", Test::class) { + group = "tests" + dependsOn(tasks.named("testComposeUp")) + dependsOn(tasks.ktlintCheck) + dependsOn(testSql) +} tasks.register("testArticles", Test::class) { group = "tests" useJUnitPlatform { @@ -368,7 +362,7 @@ tasks.register("testVotes", Test::class) { includeTags("vote") } } -tasks.register("testWorkgroup", Test::class) { +tasks.register("testWorkgroups", Test::class) { group = "tests" useJUnitPlatform { includeTags("workgroup") From 620cd73fec5b293db9ab86c400a5273f59ad547c Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Fri, 16 Apr 2021 03:01:24 +0200 Subject: [PATCH 45/46] Change intellij tasks --- .../runConfigurations/Build_without_test.xml | 2 +- ... => Sonarqube__Send_without_run_test_.xml} | 2 +- .../Test_With_Dependencies.xml | 23 +++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) rename .idea/runConfigurations/{Sonarqube_without_test.xml => Sonarqube__Send_without_run_test_.xml} (87%) create mode 100644 .idea/runConfigurations/Test_With_Dependencies.xml diff --git a/.idea/runConfigurations/Build_without_test.xml b/.idea/runConfigurations/Build_without_test.xml index 0898a5a..92be20b 100644 --- a/.idea/runConfigurations/Build_without_test.xml +++ b/.idea/runConfigurations/Build_without_test.xml @@ -4,7 +4,7 @@ diff --git a/.idea/runConfigurations/Sonarqube_without_test.xml b/.idea/runConfigurations/Sonarqube__Send_without_run_test_.xml similarity index 87% rename from .idea/runConfigurations/Sonarqube_without_test.xml rename to .idea/runConfigurations/Sonarqube__Send_without_run_test_.xml index 1774008..b7e8407 100644 --- a/.idea/runConfigurations/Sonarqube_without_test.xml +++ b/.idea/runConfigurations/Sonarqube__Send_without_run_test_.xml @@ -1,5 +1,5 @@ - +