Refactoring of updateOpinions (route/repo/query)

Can nw be set multiple opinion on sigle query
fix OpinionVoter on CREATE

src/main/kotlin/fr/dcproject/repository/Opinion.kt

@@ -1,8 +1,9 @@
 package fr.dcproject.repository
 
 import com.fasterxml.jackson.core.type.TypeReference
-import fr.dcproject.entity.Article
+import fr.dcproject.entity.ArticleRef
-import fr.dcproject.entity.OpinionAggregation
+import fr.dcproject.entity.CitizenRef
+import fr.dcproject.entity.OpinionChoiceRef
 import fr.dcproject.entity.TargetRef
 import fr.postgresjson.connexion.Paginated
 import fr.postgresjson.connexion.Requester
@@ -44,6 +45,16 @@ open class OpinionChoice(override val requester: Requester) : RepositoryI {
                 "id" to id
             )
 
+    /**
+     * find one opinion choices by id
+     */
+    fun findOpinionChoicesByIds(ids: List<UUID>): List<OpinionChoiceEntity> =
+        requester
+            .getFunction("find_opinion_choices_by_ids")
+            .select(
+                "ids" to ids
+            )
+
     fun upsertOpinionChoice(opinionChoice: OpinionChoiceEntity): OpinionChoiceEntity = requester
         .getFunction("upsert_opinion_choice")
         .selectOne(
@@ -51,20 +62,13 @@ open class OpinionChoice(override val requester: Requester) : RepositoryI {
         )!!
 }
 
-open class Opinion<T : TargetRef>(requester: Requester) : OpinionChoice(requester) {
+abstract class Opinion<T : TargetRef>(requester: Requester) : OpinionChoice(requester) {
     /**
      * Create an Opinion on target (article,...)
      */
-    fun opinion(opinion: OpinionEntity<T>): OpinionAggregation {
+    abstract fun updateOpinions(choices: List<OpinionChoiceRef>, citizen: CitizenRef, target: TargetRef): List<OpinionEntity<T>>
-        return requester
+    fun updateOpinions(choice: OpinionChoiceRef, citizen: CitizenRef, target: TargetRef): List<OpinionEntity<T>> =
-            .getFunction("opinion")
+        updateOpinions(listOf(choice), citizen, target)
-            .selectOne(
-                "reference" to opinion.target.reference,
-                "target_id" to opinion.target.id,
-                "opinion" to opinion.id,
-                "created_by_id" to opinion.createdBy
-            )!!
-    }
 
     /**
      * Find opinions of one citizen filtered by target ids
@@ -124,13 +128,18 @@ open class Opinion<T : TargetRef>(requester: Requester) : OpinionChoice(requeste
     }
 }
 
-class OpinionArticle(requester: Requester) : Opinion<Article>(requester) {
+class OpinionArticle(requester: Requester) : Opinion<ArticleRef>(requester) {
     /**
-     * Create an Opinion on Article
+     * Create an Opinions on Article
      */
-    fun opinion(opinion: OpinionArticleEntity): OpinionArticleEntity {
+    override fun updateOpinions(choices: List<OpinionChoiceRef>, citizen: CitizenRef, target: TargetRef): List<OpinionArticleEntity> {
         return requester
-            .getFunction("upsert_opinion")
+            .getFunction("update_citizen_opinions_by_target_id")
-            .selectOne(opinion) ?: error("query 'upsert_opinion' return null")
+            .select(
+                "choices_ids" to choices.map { it.id },
+                "citizen_id" to citizen.id,
+                "target_id" to target.id,
+                "target_reference" to target.reference
+            )
     }
 }

src/main/kotlin/fr/dcproject/routes/OpinionArticle.kt

@@ -1,19 +1,16 @@
 package fr.dcproject.routes
 
 import fr.dcproject.citizen
-import fr.dcproject.entity.Citizen
 import fr.dcproject.entity.CitizenRef
-import fr.dcproject.entity.OpinionArticle
 import fr.dcproject.entity.OpinionChoiceRef
 import fr.dcproject.entity.request.RequestBuilder
 import fr.dcproject.entity.request.getContent
-import fr.dcproject.repository.OpinionChoice
+import fr.dcproject.security.voter.OpinionVoter.Action.CREATE
 import fr.dcproject.security.voter.OpinionVoter.Action.VIEW
 import fr.dcproject.security.voter.assertCan
 import fr.dcproject.utils.toUUID
 import io.ktor.application.ApplicationCall
 import io.ktor.application.call
-import io.ktor.features.BadRequestException
 import io.ktor.http.HttpStatusCode
 import io.ktor.locations.KtorExperimentalLocationsAPI
 import io.ktor.locations.Location
@@ -47,25 +44,14 @@ object OpinionArticlePaths {
      */
     @Location("/articles/{article}/opinions")
     @KtorExperimentalAPI
-    class ArticleOpinion(val article: ArticleEntity) : RequestBuilder<OpinionArticle> {
+    class ArticleOpinion(val article: ArticleEntity) : RequestBuilder<List<OpinionChoiceRef>> {
 
-        private class Content(
+        private class Content(ids: List<String>) : KoinComponent {
-            opinionChoice: String
+            val ids = ids.map { it.toUUID() }
-        ) : KoinComponent {
-            val opinionChoice = OpinionChoiceRef(opinionChoice.toUUID())
-
-            fun create(citizen: Citizen, article: ArticleEntity): OpinionArticle {
-                return OpinionArticle(
-                    choice = get<OpinionChoice>().findOpinionChoiceById(opinionChoice.id) ?: throw BadRequestException("OpinionChoice not exist: id(${opinionChoice.id})"),
-                    target = article,
-                    createdBy = citizen
-                )
-            }
         }
 
-        override suspend fun getContent(call: ApplicationCall): OpinionArticle {
+        override suspend fun getContent(call: ApplicationCall): List<OpinionChoiceRef> =
-            return call.receive<Content>().create(call.citizen, article)
+            call.receive<Content>().ids.map { OpinionChoiceRef(it) }
-        }
     }
 
     /**
@@ -95,9 +81,9 @@ fun Route.opinionArticle(repo: OpinionArticleRepository) {
 
     put<OpinionArticlePaths.ArticleOpinion> {
         call.getContent(it)
-            .let { opinion ->
+            .let { choices ->
-                assertCan(VIEW, opinion)
+                assertCan(CREATE, it.article)
-                repo.opinion(opinion)
+                repo.updateOpinions(choices, citizen, it.article)
             }.let {
                 call.respond(HttpStatusCode.Created, it)
             }

src/main/kotlin/fr/dcproject/security/voter/OpinionVoter.kt

@@ -1,5 +1,6 @@
 package fr.dcproject.security.voter
 
+import fr.dcproject.entity.ArticleAuthI
 import fr.dcproject.entity.Opinion
 import io.ktor.application.ApplicationCall
 
@@ -12,13 +13,17 @@ class OpinionVoter : Voter {
 
     override fun supports(action: ActionI, call: ApplicationCall, subject: Any?): Boolean {
         return (action is Action)
-            .and(subject is Opinion<*>?)
+            .and(subject is Opinion<*>? || subject is ArticleAuthI<*>)
     }
 
     override fun vote(action: ActionI, call: ApplicationCall, subject: Any?): Vote {
         val user = call.user
         if (action == Action.CREATE) {
-            return if (user != null) Vote.GRANTED else Vote.DENIED
+            return if (user != null && (
+                (subject is ArticleAuthI<*> && !subject.isDeleted()) ||
+                (subject is Opinion<*> && subject.createdBy.user.id == user.id)
+            )) Vote.GRANTED
+            else Vote.DENIED
         }
 
         if (action == Action.VIEW) {

src/main/kotlin/fr/dcproject/security/voter/Voter.kt

@@ -38,14 +38,15 @@ enum class Vote {
 
 private val votersAttributeKey = AttributeKey<List<Voter>>("voters")
 
-fun ApplicationCall.assertCan(action: ActionI, subject: Any? = null) {
+fun ApplicationCall.assertCan(action: ActionI, subject: Any? = null, agreeIfNullOrEmpty: Boolean = true) {
-    if (!can(action, subject)) {
+    val isNullOrEmpty = (subject == null || (subject is Collection<*> && subject.isNullOrEmpty()))
+    if (!can(action, subject) && !agreeIfNullOrEmpty && isNullOrEmpty) {
         throw UnauthorizedException(action)
     }
 }
 
-fun PipelineContext<Unit, ApplicationCall>.assertCan(action: ActionI, subject: Any? = null) =
+fun PipelineContext<Unit, ApplicationCall>.assertCan(action: ActionI, subject: Any? = null, agreeIfNullOrEmpty: Boolean = true) =
-    context.assertCan(action, subject)
+    context.assertCan(action, subject, agreeIfNullOrEmpty)
 
 fun PipelineContext<Unit, ApplicationCall>.can(action: ActionI, subject: Any? = null) =
     context.can(action, subject)

src/main/resources/openapi.yaml

@@ -1376,10 +1376,12 @@ components:
     ArticleOpinionRequest:
       type: object
       properties:
-        opinion_choice:
+        ids:
-          type: string
+          type: array
-          format: uuid
+          items:
-          example: 6e978eb5-3c48-0def-b093-e01f43983adb
+            type: string
+            format: uuid
+            example: 6e978eb5-3c48-0def-b093-e01f43983adb
 
     OpinionChoices:
       description: Opinion Choice

src/main/resources/sql/functions/opinion/find_opinion_choices_by_ids.sql

@@ -0,0 +1,11 @@
+create or replace function find_opinion_choices_by_ids(_ids uuid[], out resource json)
+    language plpgsql as
+$$
+begin
+    select json_agg(ol) into resource
+    from opinion_choice ol
+    where (ol.deleted_at <= now()
+       or ol.deleted_at is null)
+       and ol.id = any(_ids);
+end;
+$$;

src/main/resources/sql/functions/opinion/update_citizen_opinions_by_target_id.sql

@@ -0,0 +1,31 @@
+create or replace function update_citizen_opinions_by_target_id(
+    _choices_ids uuid[],
+    _citizen_id uuid,
+    _target_id uuid,
+    _target_reference regclass,
+    out opinions json,
+    out ids_deleted uuid[]
+) language plpgsql as
+$$
+begin
+    if _target_reference = 'article'::regclass then
+        insert into opinion_on_article (created_by_id, target_id, choice_id)
+        select _citizen_id, _target_id, _choice_id
+        from unnest(_choices_ids) _choice_id
+        on conflict (created_by_id, target_id, choice_id) do nothing;
+
+        with deleted as (
+            delete from opinion_on_article o
+                where o.created_by_id = _citizen_id
+                    and o.target_id = _target_id
+                    and (not array[o.choice_id]::uuid[] <@ _choices_ids or _choices_ids = '{}'::uuid[])
+                returning id
+        )
+        select array_agg(d.id) into ids_deleted from deleted d;
+    else
+        raise exception '% no implemented for opinion', _target_reference::text;
+    end if;
+
+    select find_citizen_opinions_by_target_id(_citizen_id, _target_id) into opinions;
+end
+$$;

src/test/kotlin/feature/OpinionSteps.kt

@@ -45,15 +45,22 @@ class OpinionSteps : En, KoinTest {
         }
     }
 
-    private fun createOpinion(opinionChoiceName: String, articleId: String, firstName: String, lastName: String, id: String? = null) {
+    private fun createOpinion(
+        opinionChoiceName: String,
+        articleId: String,
+        firstName: String,
+        lastName: String,
+        id: String? = null
+    ) {
         val opinion = OpinionArticle(
             id = id?.toUUID() ?: UUID.randomUUID(),
             choice = get<OpinionChoiceRepository>().findOpinionsChoiceByName(opinionChoiceName)
                 ?: error("Opinion Choice not exist"),
             target = get<ArticleRepository>().findById(articleId.toUUID()) ?: error("Article not exist"),
-            createdBy = get<CitizenRepository>().findByUsername("$firstName-$lastName".toLowerCase().replace(' ', '-')) ?: error("Citizen not exist")
+            createdBy = get<CitizenRepository>().findByUsername("$firstName-$lastName".toLowerCase().replace(' ', '-'))
+                ?: error("Citizen not exist")
         )
-        get<OpinionRepository>().opinion(opinion)
+        get<OpinionRepository>().updateOpinions(opinion.choice, opinion.createdBy, opinion.target)
     }
 
     private fun createOpinionOnArticle(extraInfo: DataTable? = null) {
@@ -69,6 +76,6 @@ class OpinionSteps : En, KoinTest {
             } ?: error("You must provide the 'article' parameter"),
             createdBy = get<CitizenRepository>().findByUsername(username) ?: error("Citizen not exist")
         )
-        get<OpinionRepository>().opinion(opinion)
+        get<OpinionRepository>().updateOpinions(opinion.choice, opinion.createdBy, opinion.target)
     }
 }

src/test/kotlin/fr/dcproject/security/voter/OpinionVoterTest.kt

@@ -65,7 +65,8 @@ internal class OpinionVoterTest {
             every { user } returns tesla.user
         }.let {
             supports(OpinionVoter.Action.VIEW, it, opinion1) `should be` true
-            supports(OpinionVoter.Action.VIEW, it, article1) `should be` false
+            supports(OpinionVoter.Action.VIEW, it, article1) `should be` true
+            supports(OpinionVoter.Action.VIEW, it, einstein) `should be` false
             supports(p, it, opinion1) `should be` false
         }
     }

src/test/sql/opinion.sql

@@ -41,6 +41,8 @@ declare
     opinion_choice1_id uuid = uuid_generate_v4();
     opinion_choice2_id uuid = uuid_generate_v4();
     opinion2 json;
+    _opinions json;
+    _opinions_deleted_ids uuid[];
 begin
     -- insert user for context
     select insert_user(created_user) into created_user;
@@ -120,6 +122,46 @@ begin
         select (resource#>>'{0, choice, name}') = 'Opinion1' from find_citizen_opinions(_citizen_id, null, null, 1, 0)
     ), 'find_citizen_opinions must return a list of opinion with name';
 
+    -- test update_citizen_opinions_by_target_id
+    select opinions into _opinions
+    from update_citizen_opinions_by_target_id(
+            array[opinion_choice1_id]::uuid[],
+        _citizen_id,
+        (created_article->>'id')::uuid,
+        'article'
+    );
+    assert (json_array_length(_opinions) = 1), format('Opinions updated must be count of 1. instead of: %s', json_array_length(_opinions));
+    assert(select (_opinions#>>'{0, choice, id}')::uuid = opinion_choice1_id), 'opinion1 is not inserted';
+    assert(
+        select (o#>>'{0, choice, name}') = 'Opinion1'
+        from find_citizen_opinions_by_target_id(_citizen_id, (created_article->>'id')::uuid) o),
+        'The opinion must have a name';
+
+    -- test update_citizen_opinions_by_target_id with multiple ids
+    select opinions into _opinions
+    from update_citizen_opinions_by_target_id(
+            array[opinion_choice1_id, opinion_choice2_id]::uuid[],
+        _citizen_id,
+        (created_article->>'id')::uuid,
+        'article'
+    );
+    assert (json_array_length(_opinions) = 2), format('(on multi update) Opinions updated must be count of 1. instead of: %s', json_array_length(_opinions));
+    assert(select (_opinions#>>'{0, choice, id}')::uuid = opinion_choice1_id), '(on multi update) opinion1 is not inserted';
+    assert(select (_opinions#>>'{1, choice, id}')::uuid = opinion_choice2_id), '(on multi update) opinion2 is not inserted';
+    assert(
+        select (o#>>'{0, choice, name}') = 'Opinion1'
+        from find_citizen_opinions_by_target_id(_citizen_id, (created_article->>'id')::uuid) o),
+        '(on multi update) The opinion must have a name';
+
+    -- test update_citizen_opinions_by_target_id if empty
+    select opinions, ids_deleted into _opinions, _opinions_deleted_ids
+    from update_citizen_opinions_by_target_id(
+            '{}'::uuid[],
+        _citizen_id,
+        (created_article->>'id')::uuid,
+        'article'
+    );
+    assert json_array_length(_opinions) = 0;
     rollback;
     raise notice 'opinion test pass';
 end