Refactoring of OpinionVoter

src/main/kotlin/application/Application.kt

@@ -42,7 +42,6 @@ import fr.dcproject.event.EventNotification
 import fr.dcproject.event.EventSubscriber
 import fr.dcproject.routes.*
 import fr.dcproject.security.voter.OpinionChoiceVoter
-import fr.dcproject.security.voter.OpinionVoter
 import fr.ktorVoter.AuthorizationVoter
 import fr.ktorVoter.VoterException
 import fr.postgresjson.migration.Migrations
@@ -92,7 +91,6 @@ fun Application.module(env: Env = PROD) {
 
     install(AuthorizationVoter) {
         voters = listOf(
-            OpinionVoter(),
             OpinionChoiceVoter()
         )
     }
@@ -177,7 +175,7 @@ fun Application.module(env: Env = PROD) {
             commentConstitution(get(), get())
             voteArticle(get(), get(), get(), get())
             voteConstitution(get(), get())
-            opinionArticle(get())
+            opinionArticle(get(), get())
             opinionChoice(get())
             definition()
         }

src/main/kotlin/application/KoinModule.kt

@@ -25,6 +25,7 @@ import fr.dcproject.messages.NotificationEmailSender
 import fr.dcproject.repository.CommentConstitutionRepository
 import fr.dcproject.security.voter.ConstitutionVoter
 import fr.dcproject.security.voter.FollowVoter
+import fr.dcproject.security.voter.OpinionVoter
 import fr.dcproject.security.voter.VoteVoter
 import fr.postgresjson.connexion.Connection
 import fr.postgresjson.connexion.Requester
@@ -129,6 +130,7 @@ val KoinModule = module {
     single { ConstitutionVoter() }
     single { VoteVoter() }
     single { FollowVoter() }
+    single { OpinionVoter() }
 
     // Elasticsearch Client
     single<RestClient> {

src/main/kotlin/component/comment/generic/Comment.kt

@@ -72,7 +72,7 @@ open class CommentParent<T : TargetI>(
 
 interface CommentParentI<T : TargetI> : CommentI, EntityDeletedAt, CommentWithTargetI<T>
 
-interface CommentWithTargetI<T : TargetI> : CommentI, TargetI, AsTarget<T>
+interface CommentWithTargetI<T : TargetI> : CommentI, TargetI, HasTarget<T>
 
 interface CommentWithParentI<T : TargetI> {
     val parent: CommentParent<T>?

src/main/kotlin/component/comment/generic/CommentVoter.kt

@@ -1,7 +1,7 @@
 package fr.dcproject.component.comment.generic
 
 import fr.dcproject.component.citizen.CitizenI
-import fr.dcproject.entity.AsTarget
+import fr.dcproject.entity.HasTarget
 import fr.dcproject.voter.Voter
 import fr.dcproject.voter.VoterResponse
 import fr.postgresjson.entity.EntityCreatedBy
@@ -23,7 +23,7 @@ class CommentVoter : Voter() {
     where S : CommentI,
           S : EntityCreatedBy<CR>,
           S : CommentWithParentI<*>,
-          S : AsTarget<*> = when {
+          S : HasTarget<*> = when {
         citizen == null -> denied("You must be connected to create user", "comment.create.notConnected")
         subject.createdBy.id != citizen.id -> denied("You cannot create a comment with other user than yours", "comment.create.wrongUser")
         subject.parent?.isDeleted() ?: false -> denied("You cannot create a comment on deleted parent", "comment.create.deletedParent")

src/main/kotlin/entity/Extra.kt

@@ -13,11 +13,11 @@ import kotlin.reflect.full.isSubclassOf
 
 interface ExtraI<T : TargetI, C : CitizenI> :
     UuidEntityI,
-    AsTarget<T>,
+    HasTarget<T>,
     EntityCreatedAt,
     EntityCreatedBy<C>
 
-interface AsTarget<T : TargetI> {
+interface HasTarget<T : TargetI> {
     val target: T
 }
 
@@ -45,7 +45,7 @@ interface TargetI : UuidEntityI {
                 t.isSubclassOf(ArticleRef::class) -> TargetName.Article.targetReference
                 t.isSubclassOf(ConstitutionRef::class) -> TargetName.Constitution.targetReference
                 t.isSubclassOf(CommentRef::class) -> TargetName.Comment.targetReference
-                t.isSubclassOf(Opinion::class) -> TargetName.Opinion.targetReference
+                t.isSubclassOf(OpinionRef::class) -> TargetName.Opinion.targetReference
                 else -> throw error("target not implemented: ${t.qualifiedName} \nImplement it or return 'reference' from SQL")
             }
         }

src/main/kotlin/entity/Follow.kt

@@ -29,7 +29,7 @@ class FollowForUpdate<T : TargetI, C : CitizenI>(
     override val target: T,
     override val createdBy: C
 ) : FollowRef(id),
-    AsTarget<T>,
+    HasTarget<T>,
     EntityCreatedBy<C> by EntityCreatedByImp<C>(createdBy)
 
 open class FollowRef(

src/main/kotlin/entity/Opinion.kt

@@ -14,8 +14,8 @@ open class Opinion<T : TargetI>(
     override val createdBy: CitizenBasic,
     override val target: T,
     val choice: OpinionChoice
-) : ExtraI<T, CitizenBasicI>,
+) : OpinionRef(id),
-    TargetRef(id),
+    ExtraI<T, CitizenBasicI>,
     EntityCreatedAt by EntityCreatedAtImp(),
     EntityCreatedBy<CitizenBasicI> by EntityCreatedByImp(createdBy) {
 
@@ -32,14 +32,15 @@ class OpinionArticle(
 
 data class OpinionForUpdate<T : TargetI>(
     override val id: UUID = UUID.randomUUID(),
-    val target: T,
+    override val target: T,
-    val choice: OpinionChoice,
+    val choice: OpinionChoiceRef,
     override val createdBy: CitizenRef
 ) : OpinionRef(id),
+    HasTarget<T>,
     EntityCreatedBy<CitizenI> by EntityCreatedByImp(createdBy)
 
 open class OpinionRef(
     override val id: UUID
-) : OpinionI
+) : OpinionI, TargetRef(id)
 
 interface OpinionI : UuidEntityI

src/main/kotlin/entity/Vote.kt

@@ -33,7 +33,7 @@ class VoteForUpdate<T : TargetI, C : CitizenI>(
     VoteForUpdateI<T, C>,
     EntityCreatedBy<C> by EntityCreatedByImp<C>(createdBy)
 
-interface VoteForUpdateI<T : TargetI, C : CitizenI> : VoteI, AsTarget<T>, EntityCreatedBy<C> {
+interface VoteForUpdateI<T : TargetI, C : CitizenI> : VoteI, HasTarget<T>, EntityCreatedBy<C> {
     override val id: UUID
     val note: Int
     override val target: T

src/main/kotlin/repository/Opinion.kt

@@ -2,8 +2,6 @@ package fr.dcproject.repository
 
 import com.fasterxml.jackson.core.type.TypeReference
 import fr.dcproject.component.article.ArticleRef
-import fr.dcproject.component.citizen.CitizenRef
-import fr.dcproject.entity.OpinionChoiceRef
 import fr.dcproject.entity.OpinionForUpdate
 import fr.dcproject.entity.TargetRef
 import fr.postgresjson.connexion.Paginated
@@ -67,9 +65,9 @@ abstract class Opinion<T : TargetRef>(requester: Requester) : OpinionChoice(requ
     /**
      * Create an Opinion on target (article,...)
      */
-    abstract fun updateOpinions(choices: List<OpinionChoiceRef>, citizen: CitizenRef, target: TargetRef): List<OpinionEntity<T>>
+    abstract fun updateOpinions(opinions: List<OpinionForUpdate<*>>): List<OpinionEntity<T>>
-    fun updateOpinions(choice: OpinionChoiceRef, citizen: CitizenRef, target: TargetRef): List<OpinionEntity<T>> =
+    fun updateOpinions(opinion: OpinionForUpdate<*>): List<OpinionEntity<T>> =
-        updateOpinions(listOf(choice), citizen, target)
+        updateOpinions(listOf(opinion))
 
     abstract fun addOpinion(opinion: OpinionForUpdate<T>): OpinionEntity<T>
 
@@ -135,14 +133,15 @@ class OpinionArticle(requester: Requester) : Opinion<ArticleRef>(requester) {
     /**
      * Update Opinions on Article (Delete old one)
      */
-    override fun updateOpinions(choices: List<OpinionChoiceRef>, citizen: CitizenRef, target: TargetRef): List<OpinionArticleEntity> {
+    override fun updateOpinions(opinions: List<OpinionForUpdate<*>>): List<OpinionArticleEntity> {
         return requester
+            /* TODO change SQL function to not use .first() and pass all createdBy and target */
             .getFunction("update_citizen_opinions_by_target_id")
             .select(
-                "choices_ids" to choices.map { it.id },
+                "choices_ids" to opinions.map { it.choice.id },
-                "citizen_id" to citizen.id,
+                "citizen_id" to opinions.first().createdBy.id,
-                "target_id" to target.id,
+                "target_id" to opinions.first().target.id,
-                "target_reference" to target.reference
+                "target_reference" to opinions.first().target.reference
             )
     }
 

src/main/kotlin/routes/OpinionArticle.kt

@@ -1,14 +1,14 @@
 package fr.dcproject.routes
 
 import fr.dcproject.component.article.ArticleForView
+import fr.dcproject.component.article.ArticleRef
 import fr.dcproject.component.auth.citizen
+import fr.dcproject.component.auth.citizenOrNull
 import fr.dcproject.component.citizen.CitizenRef
-import fr.dcproject.entity.OpinionChoiceRef
+import fr.dcproject.entity.*
-import fr.dcproject.security.voter.OpinionVoter.Action.CREATE
+import fr.dcproject.security.voter.OpinionVoter
-import fr.dcproject.security.voter.OpinionVoter.Action.VIEW
 import fr.dcproject.utils.toUUID
-import fr.ktorVoter.assertCan
+import fr.dcproject.voter.assert
-import fr.ktorVoter.assertCanAll
 import io.ktor.application.*
 import io.ktor.http.*
 import io.ktor.locations.*
@@ -41,7 +41,7 @@ object OpinionArticlePaths {
     @KtorExperimentalAPI
     class ArticleOpinion(val article: ArticleForView) {
         class Body(ids: List<String>) {
-            val ids = ids.map { OpinionChoiceRef(it.toUUID()) }
+            val ids: List<UUID> = ids.map { it.toUUID() }
         }
     }
 
@@ -51,29 +51,35 @@ object OpinionArticlePaths {
     @Location("/citizens/{citizen}/opinions")
     class CitizenOpinions(val citizen: CitizenEntity, id: List<String>) : KoinComponent {
         val id: List<UUID> = id.toUUID()
-        val opinionsEntities = get<OpinionArticleRepository>()
+        val opinionsEntities: List<Opinion<ArticleRef>> = get<OpinionArticleRepository>()
             .findCitizenOpinionsByTargets(citizen, this.id)
     }
 }
 
 @KtorExperimentalAPI
 @KtorExperimentalLocationsAPI
-fun Route.opinionArticle(repo: OpinionArticleRepository) {
+fun Route.opinionArticle(repo: OpinionArticleRepository, voter: OpinionVoter) {
     get<OpinionArticlePaths.CitizenOpinionArticleRequest> {
         val opinions = repo.findCitizenOpinions(citizen, it.page, it.limit)
         call.respond(opinions)
     }
 
     get<OpinionArticlePaths.CitizenOpinions> {
-        assertCanAll(VIEW, it.opinionsEntities)
+        voter.assert { canView(it.opinionsEntities, citizenOrNull) }
 
         call.respond(it.opinionsEntities)
     }
 
     put<OpinionArticlePaths.ArticleOpinion> {
-        call.receive<OpinionArticlePaths.ArticleOpinion.Body>().ids.let { choices ->
+        call.receive<OpinionArticlePaths.ArticleOpinion.Body>().ids.map { id ->
-            assertCan(CREATE, it.article)
+            OpinionForUpdate(
-            repo.updateOpinions(choices, citizen, it.article)
+                choice = OpinionChoiceRef(id),
+                target = it.article,
+                createdBy = citizen
+            )
+        }.let { opinions ->
+            voter.assert { canCreate(opinions, citizenOrNull) }
+            repo.updateOpinions(opinions)
         }.let {
             call.respond(HttpStatusCode.Created, it)
         }

src/main/kotlin/voter/OpinionVoter.kt

@@ -1,48 +1,35 @@
 package fr.dcproject.security.voter
 
-import fr.dcproject.component.article.ArticleAuthI
+import fr.dcproject.component.citizen.CitizenI
-import fr.dcproject.component.article.ArticleForView
+import fr.dcproject.entity.HasTarget
-import fr.dcproject.component.auth.user
+import fr.dcproject.entity.OpinionI
-import fr.dcproject.entity.Opinion
+import fr.dcproject.voter.Voter
-import fr.dcproject.voter.NoRuleDefinedException
+import fr.dcproject.voter.VoterResponse
-import fr.dcproject.voter.NoSubjectDefinedException
+import fr.postgresjson.entity.EntityCreatedBy
-import fr.ktorVoter.*
+import fr.postgresjson.entity.EntityDeletedAt
-import io.ktor.application.*
 
-class OpinionVoter : Voter<ApplicationCall> {
+class OpinionVoter : Voter() {
-    enum class Action : ActionI {
-        CREATE,
-        VIEW,
-        DELETE
-    }
 
-    override fun invoke(action: Any, context: ApplicationCall, subject: Any?): VoterResponseI {
+    fun <S> canCreate(subjects: List<S>, citizen: CitizenI?): VoterResponse where S : OpinionI, S : HasTarget<*> =
-        if (!((action is Action) &&
+        canAll(subjects) { canCreate(it, citizen) }
-            (subject is Opinion<*>? || subject is ArticleAuthI<*>))) return abstain()
 
-        val user = context.user
+    fun <S> canCreate(subject: S, citizen: CitizenI?): VoterResponse where S : OpinionI, S : HasTarget<*> {
-        if (action == Action.CREATE) {
+        val target = subject.target
-            if (user == null) return denied("You must be connected to make an opinion", "opinion.create.notConnected")
+        return when {
-            if (subject is ArticleAuthI<*> && !subject.isDeleted()) return granted()
+            citizen == null -> denied("You must be connected to make an opinion", "opinion.create.notConnected")
-            if (subject is Opinion<*> && subject.createdBy.user.id == user.id) return granted()
+            target is EntityDeletedAt && target.isDeleted() -> denied("You cannot make opinion on deleted target", "opinion.create.deletedTarget")
-
+            else -> granted()
-            throw NoSubjectDefinedException(action)
+        }
-        }
+    }
-
+
-        if (action == Action.VIEW) {
+    fun <S : OpinionI, SS : List<S>> canView(subjects: SS, citizen: CitizenI?): VoterResponse =
-            return if (subject is Opinion<*> || subject is ArticleForView) granted() else throw NoSubjectDefinedException(action)
+        canAll(subjects) { canView(it, citizen) }
-        }
+
-
+    fun <S : OpinionI> canView(subject: S, citizen: CitizenI?): VoterResponse = granted()
-        if (action == Action.DELETE) {
+
-            if (user == null) return denied("You must be connected to delete opinion", "opinion.delete.notConnected")
+    fun <S, C : CitizenI> canDelete(subject: S, citizen: CitizenI?): VoterResponse where S : EntityCreatedBy<C>, S : OpinionI = when {
-            if (subject !is Opinion<*>) throw NoSubjectDefinedException(action)
+        citizen == null -> denied("You must be connected to delete opinion", "opinion.delete.notConnected")
-            return if (subject.createdBy.user.id == user.id) granted() else denied("You can only delete your opinions", "opinion.delete.notYours")
+        subject.createdBy.id != citizen.id -> denied("You can only delete your opinions", "opinion.delete.notYours")
-        }
+        else -> granted()
-
-        if (action is Action) {
-            throw NoRuleDefinedException(action)
-        }
-
-        return abstain()
     }
 }

src/test/kotlin/steps/OpinionSteps.kt

@@ -77,6 +77,6 @@ class OpinionSteps : En, KoinTest {
             } ?: error("You must provide the 'article' parameter"),
             createdBy = get<CitizenRepository>().findByUsername(username) ?: error("Citizen not exist")
         )
-        get<OpinionRepository>().updateOpinions(opinion.choice, opinion.createdBy, opinion.target)
+        get<OpinionRepository>().updateOpinions(opinion)
     }
 }

src/test/kotlin/unit/voter/OpinionVoterTest.kt

@@ -10,18 +10,16 @@ import fr.dcproject.component.citizen.CitizenI
 import fr.dcproject.entity.Opinion
 import fr.dcproject.entity.OpinionChoice
 import fr.dcproject.security.voter.OpinionVoter
-import fr.dcproject.voter.NoSubjectDefinedException
+import fr.dcproject.voter.Vote.DENIED
+import fr.dcproject.voter.Vote.GRANTED
 import fr.ktorVoter.*
 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.*
@@ -83,89 +81,51 @@ internal class OpinionVoterTest {
     }
 
     @Test
-    fun `support opinion`(): Unit = OpinionVoter().run {
+    fun `can be view the opinion`() {
-        val p = object : ActionI {}
+        OpinionVoter()
-        mockk<ApplicationCall> {
+            .canView(opinion1, tesla)
-            every { user } returns tesla.user
+            .vote `should be` GRANTED
-        }.let {
-            this(OpinionVoter.Action.VIEW, it, opinion1).vote `should be` Vote.GRANTED
-            this(OpinionVoter.Action.VIEW, it, article1).vote `should be` Vote.GRANTED
-            this(OpinionVoter.Action.VIEW, it, einstein).vote `should be` Vote.ABSTAIN
-            this(p, it, opinion1).vote `should be` Vote.ABSTAIN
-        }
     }
 
     @Test
-    fun `can be view the opinion`(): Unit = listOf(OpinionVoter()).run {
+    fun `can be view the opinion list`() {
-        mockk<ApplicationCall> {
+        OpinionVoter()
-            every { user } returns tesla.user
+            .canView(listOf(opinion1), tesla)
-        }.let {
+            .vote `should be` GRANTED
-            can(OpinionVoter.Action.VIEW, it, opinion1) `should be` true
-        }
     }
 
     @Test
-    fun `can be not view the opinion if is null`(): Unit = listOf(OpinionVoter()).run {
+    fun `can be opinion an article`() {
-        mockk<ApplicationCall> {
+        OpinionVoter()
-            every { user } returns tesla.user
+            .canCreate(opinion1, tesla)
-        }.let {
+            .vote `should be` GRANTED
-            assertThrows<NoSubjectDefinedException> {
-                assertCan(OpinionVoter.Action.VIEW, it, null)
-            }
-        }
     }
 
     @Test
-    fun `can be view the opinion list`(): Unit = listOf(OpinionVoter()).run {
+    fun `can not be opinion if not connected`() {
-        mockk<ApplicationCall> {
+        OpinionVoter()
-            every { user } returns tesla.user
+            .canCreate(opinion1, null)
-        }.let {
+            .vote `should be` DENIED
-            canAll(OpinionVoter.Action.VIEW, it, listOf(opinion1)) `should be` true
-        }
     }
 
     @Test
-    fun `can be opinion an article`(): Unit = listOf(OpinionVoter()).run {
+    fun `can be remove opinion`() {
-        mockk<ApplicationCall> {
+        OpinionVoter()
-            every { user } returns tesla.user
+            .canDelete(opinion1, tesla)
-        }.let {
+            .vote `should be` GRANTED
-            can(OpinionVoter.Action.CREATE, it, opinion1) `should be` true
-        }
     }
 
     @Test
-    fun `can not be opinion if not connected`() = listOf(OpinionVoter()).run {
+    fun `can not be remove opinion if not connected`() {
-        mockk<ApplicationCall> {
+        OpinionVoter()
-            every { user } returns null
+            .canDelete(opinion1, null)
-        }.let {
+            .vote `should be` DENIED
-            can(OpinionVoter.Action.CREATE, it, opinion1) `should be` false
-        }
     }
 
     @Test
-    fun `can be remove opinion`(): Unit = listOf(OpinionVoter()).run {
+    fun `can not be remove opinion of other user`() {
-        mockk<ApplicationCall> {
+        OpinionVoter()
-            every { user } returns tesla.user
+            .canDelete(opinion1, einstein)
-        }.let {
+            .vote `should be` DENIED
-            can(OpinionVoter.Action.DELETE, it, opinion1) `should be` true
-        }
-    }
-
-    @Test
-    fun `can not be remove opinion if not connected`(): Unit = listOf(OpinionVoter()).run {
-        mockk<ApplicationCall> {
-            every { user } returns null
-        }.let {
-            can(OpinionVoter.Action.DELETE, it, opinion1) `should be` false
-        }
-    }
-
-    @Test
-    fun `can not be remove opinion of other user`(): Unit = listOf(OpinionVoter()).run {
-        mockk<ApplicationCall> {
-            every { user } returns einstein.user
-        }.let {
-            can(OpinionVoter.Action.DELETE, it, opinion1) `should be` false
-        }
     }
 }

src/test/resources/feature/opinion.feature

@@ -24,7 +24,6 @@ Feature: Opinion
       | id        | 9226c1a3-8091-c3fa-7d0d-c2e98c9bee7b |
       | createdBy | Isaac Newton                         |
     And I have an opinion choice "Opinion4" with ID "0f4f1721-3136-44f1-9f31-1459f3317b15"
-    And I have an opinion "Opinion4" on article "9226c1a3-8091-c3fa-7d0d-c2e98c9bee7b" created by Isaac Newton with ID "74e93e12-556b-4399-95a6-04f93a4dd66c"
     When I send a PUT request to "/articles/9226c1a3-8091-c3fa-7d0d-c2e98c9bee7b/opinions" with body:
     """
     {