From d6840e8064b421feabba6d6d657027ff099b2440 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sun, 17 Jan 2021 23:32:43 +0100 Subject: [PATCH] Refactoring of VoteVoter --- src/main/kotlin/application/Application.kt | 5 +- src/main/kotlin/application/KoinModule.kt | 2 + src/main/kotlin/routes/VoteArticle.kt | 17 ++- src/main/kotlin/routes/VoteConstitution.kt | 9 +- src/main/kotlin/voter/VoteVoter.kt | 54 +++------- src/test/kotlin/unit/voter/VoteVoterTest.kt | 114 +++++--------------- 6 files changed, 57 insertions(+), 144 deletions(-) diff --git a/src/main/kotlin/application/Application.kt b/src/main/kotlin/application/Application.kt index f297790..12b8303 100644 --- a/src/main/kotlin/application/Application.kt +++ b/src/main/kotlin/application/Application.kt @@ -96,7 +96,6 @@ fun Application.module(env: Env = PROD) { install(AuthorizationVoter) { voters = listOf( - VoteVoter(), FollowVoter(), OpinionVoter(), OpinionChoiceVoter() @@ -211,8 +210,8 @@ fun Application.module(env: Env = PROD) { followArticle(get()) followConstitution(get()) commentConstitution(get(), get()) - voteArticle(get(), get(), get()) - voteConstitution(get()) + voteArticle(get(), get(), get(), get()) + voteConstitution(get(), get()) opinionArticle(get()) opinionChoice(get()) definition() diff --git a/src/main/kotlin/application/KoinModule.kt b/src/main/kotlin/application/KoinModule.kt index 495d867..ff9c8dc 100644 --- a/src/main/kotlin/application/KoinModule.kt +++ b/src/main/kotlin/application/KoinModule.kt @@ -24,6 +24,7 @@ import fr.dcproject.messages.Mailer import fr.dcproject.messages.NotificationEmailSender import fr.dcproject.repository.CommentConstitutionRepository import fr.dcproject.security.voter.ConstitutionVoter +import fr.dcproject.security.voter.VoteVoter import fr.postgresjson.connexion.Connection import fr.postgresjson.connexion.Requester import fr.postgresjson.migration.Migrations @@ -125,6 +126,7 @@ val KoinModule = module { single { CommentVoter() } single { WorkgroupVoter() } single { ConstitutionVoter() } + single { VoteVoter() } // Elasticsearch Client single { diff --git a/src/main/kotlin/routes/VoteArticle.kt b/src/main/kotlin/routes/VoteArticle.kt index 2ad9c4d..4828932 100644 --- a/src/main/kotlin/routes/VoteArticle.kt +++ b/src/main/kotlin/routes/VoteArticle.kt @@ -2,17 +2,16 @@ package fr.dcproject.routes import fr.dcproject.component.article.ArticleForView import fr.dcproject.component.auth.citizen +import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.citizen.Citizen import fr.dcproject.component.comment.generic.CommentRepository import fr.dcproject.entity.VoteForUpdate import fr.dcproject.repository.VoteComment import fr.dcproject.routes.VoteArticlePaths.ArticleVoteRequest import fr.dcproject.routes.VoteArticlePaths.CommentVoteRequest -import fr.dcproject.security.voter.VoteVoter.Action.CREATE -import fr.dcproject.security.voter.VoteVoter.Action.VIEW +import fr.dcproject.security.voter.VoteVoter import fr.dcproject.utils.toUUID -import fr.ktorVoter.assertCan -import fr.ktorVoter.assertCanAll +import fr.dcproject.voter.assert import io.ktor.application.* import io.ktor.http.* import io.ktor.locations.* @@ -49,7 +48,7 @@ object VoteArticlePaths { } @KtorExperimentalLocationsAPI -fun Route.voteArticle(repo: VoteArticleRepository, voteCommentRepo: VoteComment, commentRepo: CommentRepository) { +fun Route.voteArticle(repo: VoteArticleRepository, voteCommentRepo: VoteComment, commentRepo: CommentRepository, voter: VoteVoter) { put { val content = call.receive() val vote = VoteForUpdate( @@ -57,7 +56,7 @@ fun Route.voteArticle(repo: VoteArticleRepository, voteCommentRepo: VoteComment, note = content.note, createdBy = this.citizen ) - assertCan(CREATE, vote) + voter.assert { canCreate(vote, citizenOrNull) } val votes = repo.vote(vote) call.respond(HttpStatusCode.Created, votes) } @@ -70,14 +69,14 @@ fun Route.voteArticle(repo: VoteArticleRepository, voteCommentRepo: VoteComment, note = content.note, createdBy = this.citizen ) - assertCan(CREATE, vote) + voter.assert { canCreate(vote, citizenOrNull) } val votes = voteCommentRepo.vote(vote) call.respond(HttpStatusCode.Created, votes) } get { val votes = repo.findByCitizen(it.citizen, it.page, it.limit) - assertCanAll(VIEW, votes.result) + voter.assert { canView(votes.result, citizenOrNull) } call.respond(votes) } @@ -85,7 +84,7 @@ fun Route.voteArticle(repo: VoteArticleRepository, voteCommentRepo: VoteComment, get { val votes = repo.findCitizenVotesByTargets(it.citizen, it.id) if (votes.isNotEmpty()) { - assertCanAll(VIEW, votes) + voter.assert { canView(votes, citizenOrNull) } } call.respond(votes) } diff --git a/src/main/kotlin/routes/VoteConstitution.kt b/src/main/kotlin/routes/VoteConstitution.kt index 4b2230a..fb12df4 100644 --- a/src/main/kotlin/routes/VoteConstitution.kt +++ b/src/main/kotlin/routes/VoteConstitution.kt @@ -1,11 +1,12 @@ package fr.dcproject.routes import fr.dcproject.component.auth.citizen +import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.citizen.Citizen import fr.dcproject.entity.VoteForUpdate import fr.dcproject.routes.VoteConstitutionPaths.ConstitutionVoteRequest.Content -import fr.dcproject.security.voter.VoteVoter.Action.CREATE -import fr.ktorVoter.assertCan +import fr.dcproject.security.voter.VoteVoter +import fr.dcproject.voter.assert import io.ktor.application.* import io.ktor.http.* import io.ktor.locations.* @@ -27,7 +28,7 @@ object VoteConstitutionPaths { } @KtorExperimentalLocationsAPI -fun Route.voteConstitution(repo: VoteConstitutionRepository) { +fun Route.voteConstitution(repo: VoteConstitutionRepository, voter: VoteVoter) { put { val content = call.receive() val vote = VoteForUpdate( @@ -35,7 +36,7 @@ fun Route.voteConstitution(repo: VoteConstitutionRepository) { note = content.note, createdBy = this.citizen ) - assertCan(CREATE, vote) + voter.assert { canCreate(vote, citizenOrNull) } repo.vote(vote) call.respond(HttpStatusCode.Created) } diff --git a/src/main/kotlin/voter/VoteVoter.kt b/src/main/kotlin/voter/VoteVoter.kt index 0ddf25f..bc0903a 100644 --- a/src/main/kotlin/voter/VoteVoter.kt +++ b/src/main/kotlin/voter/VoteVoter.kt @@ -1,50 +1,26 @@ package fr.dcproject.security.voter -import fr.dcproject.component.auth.citizenOrNull +import fr.dcproject.component.citizen.CitizenI +import fr.dcproject.entity.TargetI import fr.dcproject.entity.VoteForUpdateI -import fr.dcproject.entity.VoteI -import fr.dcproject.voter.NoSubjectDefinedException -import fr.ktorVoter.* +import fr.dcproject.voter.Voter +import fr.dcproject.voter.VoterResponse import fr.postgresjson.entity.EntityDeletedAt -import io.ktor.application.* import fr.dcproject.entity.Vote as VoteEntity -class VoteVoter : Voter { - enum class Action : ActionI { - CREATE, - VIEW +class VoteVoter : Voter() { + fun canCreate(subject: VoteForUpdateI, citizen: CitizenI?): VoterResponse where S : EntityDeletedAt, S : TargetI = when { + citizen == null -> denied("You must be connected for vote", "vote.create.connected") + subject.target.isDeleted() -> denied("You cannot vote on deleted target", "vote.create.isDeleted") + else -> granted() } - override fun invoke(action: Any, context: ApplicationCall, subject: Any?): VoterResponseI { - if ((action is Action && subject == null)) throw NoSubjectDefinedException(action) - if (!(action is Action && subject is VoteI)) return abstain() + fun > canView(subjects: List, citizen: CitizenI?): VoterResponse = + canAll(subjects) { canView(it, citizen) } - val citizen = context.citizenOrNull ?: return denied("You must be connected for vote", "vote.connected") - - if (action == Action.CREATE) { - if (subject !is VoteForUpdateI<*, *>) throw NoSubjectDefinedException(action) - subject.target.let { - if (it is EntityDeletedAt) { - if (it.isDeleted()) return denied("You cannot vote on deleted target", "vote.create.isDeleted") - } else { - throw NoSubjectDefinedException(action) - } - } - return granted() - } - - if (action == Action.VIEW) { - if (subject is VoteEntity<*>) { - return if (subject.createdBy.id != citizen.id) { - denied("You can view only your votes", "vote.view") - } else { - granted() - } - } else { - throw NoSubjectDefinedException(action) - } - } - - return abstain() + fun canView(subject: VoteEntity<*>, citizen: CitizenI?): VoterResponse = when { + citizen == null -> denied("You must be connected for view your votes", "vote.view.connected") + subject.createdBy.id != citizen.id -> denied("You can only display your votes", "vote.view.onlyYours") + else -> granted() } } diff --git a/src/test/kotlin/unit/voter/VoteVoterTest.kt b/src/test/kotlin/unit/voter/VoteVoterTest.kt index cfe3a48..e284141 100644 --- a/src/test/kotlin/unit/voter/VoteVoterTest.kt +++ b/src/test/kotlin/unit/voter/VoteVoterTest.kt @@ -4,28 +4,21 @@ import fr.dcproject.component.article.ArticleForView import fr.dcproject.component.article.ArticleRef import fr.dcproject.component.auth.User import fr.dcproject.component.auth.UserI -import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.citizen.Citizen import fr.dcproject.component.citizen.CitizenBasic import fr.dcproject.component.citizen.CitizenCart import fr.dcproject.component.citizen.CitizenI import fr.dcproject.entity.VoteForUpdate import fr.dcproject.security.voter.VoteVoter -import fr.dcproject.voter.NoSubjectDefinedException -import fr.ktorVoter.ActionI -import fr.ktorVoter.Vote -import fr.ktorVoter.can -import fr.ktorVoter.canAll +import fr.dcproject.voter.Vote.DENIED +import fr.dcproject.voter.Vote.GRANTED import io.ktor.application.* -import io.mockk.every -import io.mockk.mockk import io.mockk.mockkStatic import org.amshove.kluent.`should be` import org.joda.time.DateTime import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test import org.junit.jupiter.api.TestInstance -import org.junit.jupiter.api.assertThrows import org.junit.jupiter.api.parallel.Execution import org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT import java.util.* @@ -120,101 +113,44 @@ internal class VoteVoterTest { } @Test - fun `support vote`(): Unit = VoteVoter().run { - val p = object : ActionI {} - mockk { - every { citizenOrNull } returns tesla - }.let { - this(VoteVoter.Action.VIEW, it, vote1).vote `should be` Vote.GRANTED - this(VoteVoter.Action.VIEW, it, article1).vote `should be` Vote.ABSTAIN - this(p, it, vote1).vote `should be` Vote.ABSTAIN - } + fun `can be view your the vote`() { + VoteVoter() + .canView(vote1, tesla) + .vote `should be` GRANTED } @Test - fun `can be view your the vote`(): Unit = listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns tesla - }.let { - can(VoteVoter.Action.VIEW, it, vote1) `should be` true - } + fun `can not be view vote of other`() { + VoteVoter() + .canView(vote1, einstein) + .vote `should be` DENIED } @Test - fun `can not be view vote of other`(): Unit = listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns einstein - }.let { - can(VoteVoter.Action.VIEW, it, vote1) `should be` false - } - } - - @Test - fun `can be not view the vote if is null`(): Unit = listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns tesla - }.let { - assertThrows { - can(VoteVoter.Action.VIEW, it, null) - } - } - } - - @Test - fun `can be view your votes list`(): Unit = listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns tesla - }.let { - canAll(VoteVoter.Action.VIEW, it, listOf(vote1)) `should be` true - } + fun `can be view your votes list`() { + VoteVoter() + .canView(listOf(vote1), tesla) + .vote `should be` GRANTED } @Test fun `can be vote an article`() { - listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns tesla - }.let { - can(VoteVoter.Action.CREATE, it, voteForUpdate) `should be` true - } - } + VoteVoter() + .canCreate(voteForUpdate, tesla) + .vote `should be` GRANTED } @Test - fun `can not be vote if not connected`(): Unit = listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns null - }.let { - can(VoteVoter.Action.CREATE, it, voteForUpdate) `should be` false - } + fun `can not be vote if not connected`() { + VoteVoter() + .canCreate(voteForUpdate, null) + .vote `should be` DENIED } @Test - fun `can not be vote an article if article is deleted`(): Unit = listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns tesla - }.let { - can(VoteVoter.Action.CREATE, it, voteOnDeleted) `should be` false - } - } - - @Test - fun `can not be vote an article if article have no user`(): Unit = listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns tesla - }.let { - assertThrows { - can(VoteVoter.Action.CREATE, it, voteWithoutTargetUser) - } - } - } - - @Test - fun `can not be comment an article if article is deleted`(): Unit = listOf(VoteVoter()).run { - mockk { - every { citizenOrNull } returns tesla - }.let { - can(VoteVoter.Action.CREATE, it, voteOnDeleted) `should be` false - } + fun `can not be vote an article if article is deleted`() { + VoteVoter() + .canCreate(voteOnDeleted, tesla) + .vote `should be` DENIED } } \ No newline at end of file