From a3e2437ee145fd9ae192b2bdbaf9e5a0fbe1bc21 Mon Sep 17 00:00:00 2001 From: Marcin Procyk Date: Wed, 17 Apr 2024 09:42:52 +0200 Subject: [PATCH 1/7] add GroupServiceSpec --- .../admin/domain/service/GroupService.scala | 4 +- .../domain/service/GroupServiceSpec.scala | 72 +++++++++++++++++++ .../repo/service/KnoraGroupRepoLiveSpec.scala | 3 +- 3 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala index e213c9a561..3797ea3fe6 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala @@ -33,7 +33,7 @@ final case class GroupService( private def toGroups(knoraGroups: Chunk[KnoraGroup]): Task[Chunk[Group]] = ZIO.foreach(knoraGroups)(toGroup) - private def toGroup(knoraGroup: KnoraGroup): Task[Group] = + private[service] def toGroup(knoraGroup: KnoraGroup): Task[Group] = for { project <- knoraGroup.belongsToProject.map(projectService.findById).getOrElse(ZIO.none) } yield Group( @@ -45,7 +45,7 @@ final case class GroupService( selfjoin = knoraGroup.hasSelfJoinEnabled.value, ) - private def toKnoraGroup(group: Group): KnoraGroup = + private[service] def toKnoraGroup(group: Group): KnoraGroup = KnoraGroup( id = GroupIri.unsafeFrom(group.id), groupName = GroupName.unsafeFrom(group.name), diff --git a/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala b/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala new file mode 100644 index 0000000000..c97acd987b --- /dev/null +++ b/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala @@ -0,0 +1,72 @@ +package org.knora.webapi.slice.admin.domain.service + +import org.knora.webapi.messages.StringFormatter +import org.knora.webapi.messages.store.triplestoremessages.StringLiteralV2 +import org.knora.webapi.responders.IriService +import org.knora.webapi.slice.admin.domain.model.{ + Group, + GroupDescriptions, + GroupIri, + GroupName, + GroupSelfJoin, + GroupStatus, + KnoraGroup, +} +import org.knora.webapi.slice.admin.domain.repo.KnoraProjectRepoInMemory +import org.knora.webapi.slice.admin.repo.service.{KnoraGroupRepoInMemory, KnoraGroupRepoLive} +import org.knora.webapi.slice.ontology.repo.service.{OntologyCacheLive, OntologyRepoLive} +import org.knora.webapi.slice.resourceinfo.domain.IriConverter +import org.knora.webapi.store.cache.CacheService +import org.knora.webapi.store.triplestore.api.TriplestoreServiceInMemory +import zio.{Scope, ZIO} +import zio.test._ +import zio.test.{Spec, TestEnvironment, ZIOSpecDefault} + +object GroupServiceSpec extends ZIOSpecDefault { + private val exampleGroup = new Group( + id = "http://rdfh.ch/groups/0007/james-bond-group", + name = "James Bond Group", + descriptions = Seq(StringLiteralV2.from("James Bond Group Description", Some("en"))), + project = None, + status = true, + selfjoin = false, + ) + + private val exampleKnoraGroup = new KnoraGroup( + id = GroupIri.unsafeFrom("http://rdfh.ch/groups/0007/james-bond-group"), + groupName = GroupName.unsafeFrom("James Bond Group"), + groupDescriptions = + GroupDescriptions.unsafeFrom(Seq(StringLiteralV2.from("James Bond Group Description", Some("en")))), + status = GroupStatus.active, + belongsToProject = None, + hasSelfJoinEnabled = GroupSelfJoin.disabled, + ) + + override def spec: Spec[TestEnvironment with Scope, Any] = + suite("GroupServiceSpec")( + test("KnoraGroup should be transformed to Group") { + for { + group <- ZIO.serviceWithZIO[GroupService](_.toGroup(exampleKnoraGroup)) + } yield assertTrue(group == exampleGroup) + }, + test("Group should be transformed to KnoraGroup") { + for { + knoraGroup <- ZIO.serviceWith[GroupService](_.toKnoraGroup(exampleGroup)) + } yield assertTrue(knoraGroup == exampleKnoraGroup) + }, + ).provide( + CacheService.layer, + GroupService.layer, + IriConverter.layer, + IriService.layer, + KnoraGroupRepoInMemory.layer, + KnoraGroupService.KnoraGroupService.layer, + KnoraProjectRepoInMemory.layer, + KnoraProjectService.layer, + OntologyCacheLive.layer, + OntologyRepoLive.layer, + ProjectService.layer, + StringFormatter.test, + TriplestoreServiceInMemory.emptyLayer, + ) +} diff --git a/webapi/src/test/scala/org/knora/webapi/slice/admin/repo/service/KnoraGroupRepoLiveSpec.scala b/webapi/src/test/scala/org/knora/webapi/slice/admin/repo/service/KnoraGroupRepoLiveSpec.scala index a4a91f936b..6d661d2358 100644 --- a/webapi/src/test/scala/org/knora/webapi/slice/admin/repo/service/KnoraGroupRepoLiveSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/slice/admin/repo/service/KnoraGroupRepoLiveSpec.scala @@ -28,7 +28,8 @@ import org.knora.webapi.store.triplestore.api.TriplestoreServiceInMemory final case class KnoraGroupRepoInMemory(groups: Ref[Chunk[KnoraGroup]]) extends AbstractInMemoryCrudRepository[KnoraGroup, GroupIri](groups, _.id) with KnoraGroupRepo { - override def findByName(name: GroupName): Task[Option[KnoraGroup]] = ??? + override def findByName(name: GroupName): Task[Option[KnoraGroup]] = + groups.get.map(_.find(_.groupName == name)) } object KnoraGroupRepoInMemory { From 8dce490303c7d9bee843a08e1c83117b61c9a961 Mon Sep 17 00:00:00 2001 From: Marcin Procyk Date: Wed, 17 Apr 2024 09:53:19 +0200 Subject: [PATCH 2/7] remove redundant KnoraGroupService object --- .../admin/domain/AdminDomainModule.scala | 19 +++++++++-- .../domain/service/KnoraGroupService.scala | 9 ++--- .../AuthorizationRestServiceSpec.scala | 2 +- .../domain/service/GroupServiceSpec.scala | 33 ++++++++++--------- 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/AdminDomainModule.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/AdminDomainModule.scala index 7b9ced4418..a41f695f44 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/AdminDomainModule.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/AdminDomainModule.scala @@ -10,7 +10,7 @@ import zio.ZLayer import org.knora.webapi.config.AppConfig import org.knora.webapi.responders.IriService import org.knora.webapi.slice.admin.domain.service.GroupService -import org.knora.webapi.slice.admin.domain.service.KnoraGroupService.KnoraGroupService +import org.knora.webapi.slice.admin.domain.service.KnoraGroupService import org.knora.webapi.slice.admin.domain.service.KnoraProjectService import org.knora.webapi.slice.admin.domain.service.PasswordService import org.knora.webapi.slice.admin.domain.service.ProjectService @@ -24,10 +24,23 @@ import org.knora.webapi.store.triplestore.api.TriplestoreService object AdminDomainModule { type Dependencies = - AppConfig & AdminRepoModule.Provided & CacheService & IriService & OntologyRepo & PredicateObjectMapper & TriplestoreService + AppConfig & + AdminRepoModule.Provided & + CacheService & + IriService & + OntologyRepo & + PredicateObjectMapper & + TriplestoreService type Provided = - AdministrativePermissionService & GroupService & KnoraGroupService & KnoraProjectService & KnoraUserService & MaintenanceService & PasswordService & ProjectService + AdministrativePermissionService & + GroupService & + KnoraGroupService & + KnoraProjectService & + KnoraUserService & + MaintenanceService & + PasswordService & + ProjectService val layer = ZLayer.makeSome[Dependencies, Provided]( AdministrativePermissionService.layer, diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala index ff6694e958..b393a8e908 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala @@ -50,10 +50,7 @@ case class KnoraGroupService( def updateGroup(groupToUpdate: KnoraGroup, request: GroupUpdateRequest): Task[KnoraGroup] = for { - _ <- request.name match { - case Some(value) => ensureGroupNameIsUnique(value) - case None => ZIO.unit - } + _ <- ZIO.foreachDiscard(request.name)(ensureGroupNameIsUnique) updatedGroup <- knoraGroupRepo.save( @@ -73,7 +70,5 @@ case class KnoraGroupService( } object KnoraGroupService { - object KnoraGroupService { - val layer = ZLayer.derive[KnoraGroupService] - } + val layer = ZLayer.derive[KnoraGroupService] } diff --git a/webapi/src/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala b/webapi/src/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala index 7564095e82..3df935ca22 100644 --- a/webapi/src/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala @@ -19,7 +19,7 @@ import org.knora.webapi.responders.IriService import org.knora.webapi.slice.admin.domain.model.User import org.knora.webapi.slice.admin.domain.repo.KnoraProjectRepoInMemory import org.knora.webapi.slice.admin.domain.service.KnoraGroupRepo -import org.knora.webapi.slice.admin.domain.service.KnoraGroupService.KnoraGroupService +import org.knora.webapi.slice.admin.domain.service.KnoraGroupService import org.knora.webapi.slice.admin.domain.service.KnoraProjectRepo.builtIn.SystemProject import org.knora.webapi.slice.admin.domain.service.KnoraProjectService import org.knora.webapi.slice.admin.repo.service.KnoraGroupRepoInMemory diff --git a/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala b/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala index c97acd987b..63c82be9f5 100644 --- a/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala @@ -1,26 +1,29 @@ package org.knora.webapi.slice.admin.domain.service +import zio.Scope +import zio.ZIO +import zio.test.Spec +import zio.test.TestEnvironment +import zio.test.ZIOSpecDefault +import zio.test._ + import org.knora.webapi.messages.StringFormatter import org.knora.webapi.messages.store.triplestoremessages.StringLiteralV2 import org.knora.webapi.responders.IriService -import org.knora.webapi.slice.admin.domain.model.{ - Group, - GroupDescriptions, - GroupIri, - GroupName, - GroupSelfJoin, - GroupStatus, - KnoraGroup, -} +import org.knora.webapi.slice.admin.domain.model.Group +import org.knora.webapi.slice.admin.domain.model.GroupDescriptions +import org.knora.webapi.slice.admin.domain.model.GroupIri +import org.knora.webapi.slice.admin.domain.model.GroupName +import org.knora.webapi.slice.admin.domain.model.GroupSelfJoin +import org.knora.webapi.slice.admin.domain.model.GroupStatus +import org.knora.webapi.slice.admin.domain.model.KnoraGroup import org.knora.webapi.slice.admin.domain.repo.KnoraProjectRepoInMemory -import org.knora.webapi.slice.admin.repo.service.{KnoraGroupRepoInMemory, KnoraGroupRepoLive} -import org.knora.webapi.slice.ontology.repo.service.{OntologyCacheLive, OntologyRepoLive} +import org.knora.webapi.slice.admin.repo.service.KnoraGroupRepoInMemory +import org.knora.webapi.slice.ontology.repo.service.OntologyCacheLive +import org.knora.webapi.slice.ontology.repo.service.OntologyRepoLive import org.knora.webapi.slice.resourceinfo.domain.IriConverter import org.knora.webapi.store.cache.CacheService import org.knora.webapi.store.triplestore.api.TriplestoreServiceInMemory -import zio.{Scope, ZIO} -import zio.test._ -import zio.test.{Spec, TestEnvironment, ZIOSpecDefault} object GroupServiceSpec extends ZIOSpecDefault { private val exampleGroup = new Group( @@ -60,7 +63,7 @@ object GroupServiceSpec extends ZIOSpecDefault { IriConverter.layer, IriService.layer, KnoraGroupRepoInMemory.layer, - KnoraGroupService.KnoraGroupService.layer, + KnoraGroupService.layer, KnoraProjectRepoInMemory.layer, KnoraProjectService.layer, OntologyCacheLive.layer, From 88055c806c1ec7a89c374289d71f0f6cf2952100 Mon Sep 17 00:00:00 2001 From: Marcin Procyk Date: Wed, 17 Apr 2024 10:10:34 +0200 Subject: [PATCH 3/7] add missing header --- .../webapi/slice/admin/domain/service/GroupServiceSpec.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala b/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala index 63c82be9f5..17c292ccde 100644 --- a/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala @@ -1,3 +1,8 @@ +/* + * Copyright © 2021 - 2024 Swiss National Data and Service Center for the Humanities and/or DaSCH Service Platform contributors. + * SPDX-License-Identifier: Apache-2.0 + */ + package org.knora.webapi.slice.admin.domain.service import zio.Scope From a10c08663b7584cefe42732e377663b6ea32c8db Mon Sep 17 00:00:00 2001 From: Marcin Procyk Date: Wed, 17 Apr 2024 10:59:57 +0200 Subject: [PATCH 4/7] move status update method from responcer to admin services --- .../webapi/responders/admin/GroupsResponderADMSpec.scala | 4 ++-- .../webapi/slice/admin/api/service/GroupRestService.scala | 8 +++++--- .../webapi/slice/admin/domain/service/GroupService.scala | 3 +++ .../slice/admin/domain/service/KnoraGroupService.scala | 4 ++++ 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala b/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala index 36de1ed587..e35902a537 100644 --- a/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala +++ b/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala @@ -154,7 +154,7 @@ class GroupsResponderADMSpec extends CoreSpec { group.selfjoin should equal(false) } - "return 'NotFound' if a not-existing group IRI is submitted during update" in { + "return 'NotFoundException' if a not-existing group IRI is submitted during update" in { val groupIri = "http://rdfh.ch/groups/0000/notexisting" val exit = UnsafeZioRun.run( groupRestService( @@ -204,7 +204,7 @@ class GroupsResponderADMSpec extends CoreSpec { ) } - "return 'BadRequest' if nothing would be changed during the update" in { + "return 'BadRequestException' if nothing would be changed during the update" in { val exit = UnsafeZioRun.run( groupRestService( _.putGroup( diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/GroupRestService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/GroupRestService.scala index f616cc58b4..68aba0eb26 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/GroupRestService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/GroupRestService.scala @@ -75,9 +75,11 @@ final case class GroupRestService( def putGroupStatus(iri: GroupIri, request: GroupStatusUpdateRequest, user: User): Task[GroupGetResponseADM] = for { - _ <- auth.ensureSystemAdminOrProjectAdminOfGroup(user, iri) - uuid <- Random.nextUUID - internal <- responder.updateGroupStatus(iri, request, uuid) + _ <- auth.ensureSystemAdminOrProjectAdminOfGroup(user, iri) + groupToUpdate <- groupService + .findById(iri) + .someOrFail(NotFoundException(s"Group <${iri.value}> not found.")) + internal <- groupService.updateGroupStatus(groupToUpdate, request.status).map(GroupGetResponseADM.apply) external <- format.toExternalADM(internal) } yield external diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala index 3797ea3fe6..76c78890b6 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala @@ -60,6 +60,9 @@ final case class GroupService( def updateGroup(groupToUpdate: Group, request: GroupUpdateRequest): Task[Group] = knoraGroupService.updateGroup(toKnoraGroup(groupToUpdate), request).flatMap(toGroup) + + def updateGroupStatus(groupToUpdate: Group, status: GroupStatus): Task[Group] = + knoraGroupService.updateGroupStatus(toKnoraGroup(groupToUpdate), status).flatMap(toGroup) } object GroupService { diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala index b393a8e908..eba213a9d8 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala @@ -16,6 +16,7 @@ import org.knora.webapi.slice.admin.api.GroupsRequests.GroupCreateRequest import org.knora.webapi.slice.admin.api.GroupsRequests.GroupUpdateRequest import org.knora.webapi.slice.admin.domain.model.GroupIri import org.knora.webapi.slice.admin.domain.model.GroupName +import org.knora.webapi.slice.admin.domain.model.GroupStatus import org.knora.webapi.slice.admin.domain.model.KnoraGroup import org.knora.webapi.slice.admin.domain.model.KnoraProject @@ -63,6 +64,9 @@ case class KnoraGroupService( ) } yield updatedGroup + def updateGroupStatus(groupToUpdate: KnoraGroup, status: GroupStatus): Task[KnoraGroup] = + knoraGroupRepo.save(groupToUpdate.copy(status = status)) + private def ensureGroupNameIsUnique(name: GroupName) = ZIO.whenZIO(knoraGroupRepo.existsByName(name)) { ZIO.fail(DuplicateValueException(s"Group with name: '${name.value}' already exists.")) From 6983fb24bad13e395226a5cfa7f6a083deee88b5 Mon Sep 17 00:00:00 2001 From: Marcin Procyk Date: Thu, 18 Apr 2024 08:52:14 +0200 Subject: [PATCH 5/7] remove updating status from responder & implement findByGroupMembership in user repo --- .../admin/GroupsResponderADMSpec.scala | 8 +++---- .../responders/admin/GroupsResponderADM.scala | 24 ------------------- .../domain/service/KnoraGroupService.scala | 11 ++++++++- .../admin/domain/service/KnoraUserRepo.scala | 2 ++ .../domain/service/KnoraUserService.scala | 14 +++++++++++ .../repo/service/KnoraUserRepoLive.scala | 5 ++++ .../AuthorizationRestServiceSpec.scala | 20 ++++++++++------ .../domain/service/GroupServiceSpec.scala | 6 +++++ 8 files changed, 53 insertions(+), 37 deletions(-) diff --git a/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala b/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala index e35902a537..3c9615566e 100644 --- a/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala +++ b/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala @@ -7,8 +7,6 @@ package org.knora.webapi.responders.admin import zio._ -import java.util.UUID - import dsp.errors._ import org.knora.webapi._ import org.knora.webapi.messages.admin.responder.usersmessages._ @@ -245,11 +243,11 @@ class GroupsResponderADMSpec extends CoreSpec { group.members.size shouldBe 2 val statusChangeResponse = UnsafeZioRun.runOrThrow( - ZIO.serviceWithZIO[GroupsResponderADM]( - _.updateGroupStatus( + groupRestService( + _.putGroupStatus( GroupIri.unsafeFrom(imagesReviewerGroup.id), GroupStatusUpdateRequest(GroupStatus.inactive), - UUID.randomUUID(), + rootUser, ), ), ) diff --git a/webapi/src/main/scala/org/knora/webapi/responders/admin/GroupsResponderADM.scala b/webapi/src/main/scala/org/knora/webapi/responders/admin/GroupsResponderADM.scala index 42a8d569ae..282c650ed8 100644 --- a/webapi/src/main/scala/org/knora/webapi/responders/admin/GroupsResponderADM.scala +++ b/webapi/src/main/scala/org/knora/webapi/responders/admin/GroupsResponderADM.scala @@ -28,7 +28,6 @@ import org.knora.webapi.messages.util.KnoraSystemInstances import org.knora.webapi.responders.IriLocker import org.knora.webapi.responders.IriService import org.knora.webapi.responders.Responder -import org.knora.webapi.slice.admin.api.GroupsRequests.GroupStatusUpdateRequest import org.knora.webapi.slice.admin.api.GroupsRequests.GroupUpdateRequest import org.knora.webapi.slice.admin.domain.model import org.knora.webapi.slice.admin.domain.model.Group @@ -181,29 +180,6 @@ final case class GroupsResponderADM( def groupMembersGetRequest(iri: GroupIri, user: User): Task[GroupMembersGetResponseADM] = groupMembersGetADM(iri.value, user).map(GroupMembersGetResponseADM.apply) - /** - * Change group's status. - * - * @param groupIri the IRI of the group we want to change. - * @param request the change request. - * @param apiRequestID the unique request ID. - * @return a [[GroupGetResponseADM]]. - */ - def updateGroupStatus( - groupIri: GroupIri, - request: GroupStatusUpdateRequest, - apiRequestID: UUID, - ): Task[GroupGetResponseADM] = { - val task = for { - // update group status - updated <- updateGroupHelper(groupIri, GroupUpdateRequest(None, None, Some(request.status), None)) - - // remove all members from group if status is false - result <- removeGroupMembersIfNecessary(updated.group) - } yield result - IriLocker.runWithIriLock(apiRequestID, groupIri.value, task) - } - def deleteGroup( iri: GroupIri, apiRequestID: UUID, diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala index eba213a9d8..bc739401c5 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala @@ -10,6 +10,7 @@ import zio.Task import zio.ZIO import zio.ZLayer +import dsp.errors.BadRequestException import dsp.errors.DuplicateValueException import org.knora.webapi.responders.IriService import org.knora.webapi.slice.admin.api.GroupsRequests.GroupCreateRequest @@ -22,6 +23,7 @@ import org.knora.webapi.slice.admin.domain.model.KnoraProject case class KnoraGroupService( knoraGroupRepo: KnoraGroupRepo, + knoraUserService: KnoraUserService, iriService: IriService, ) { @@ -65,7 +67,14 @@ case class KnoraGroupService( } yield updatedGroup def updateGroupStatus(groupToUpdate: KnoraGroup, status: GroupStatus): Task[KnoraGroup] = - knoraGroupRepo.save(groupToUpdate.copy(status = status)) + for { + group <- knoraGroupRepo.save(groupToUpdate.copy(status = status)) + _ <- ZIO.unless(group.status.value)(knoraUserService.findByGroupMembership(group.id).flatMap { members => + ZIO.foreachDiscard(members)(user => + knoraUserService.removeUserFromKnoraGroup(user, group).mapError(BadRequestException.apply), + ) + }) + } yield group private def ensureGroupNameIsUnique(name: GroupName) = ZIO.whenZIO(knoraGroupRepo.existsByName(name)) { diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserRepo.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserRepo.scala index 770c863c96..0a538a3e0c 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserRepo.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserRepo.scala @@ -50,6 +50,8 @@ trait KnoraUserRepo extends Repository[KnoraUser, UserIri] { def findByProjectAdminMembership(projectIri: ProjectIri): Task[Chunk[KnoraUser]] + def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]] + /** * Saves a given user. Use the returned instance for further operations as the save operation might have changed the entity instance completely. * diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala index 09ec72835a..10bbba01f2 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala @@ -22,6 +22,7 @@ import org.knora.webapi.slice.admin.domain.model.FamilyName import org.knora.webapi.slice.admin.domain.model.GivenName import org.knora.webapi.slice.admin.domain.model.Group import org.knora.webapi.slice.admin.domain.model.GroupIri +import org.knora.webapi.slice.admin.domain.model.KnoraGroup import org.knora.webapi.slice.admin.domain.model.KnoraProject import org.knora.webapi.slice.admin.domain.model.KnoraProject.ProjectIri import org.knora.webapi.slice.admin.domain.model.KnoraUser @@ -51,6 +52,9 @@ case class KnoraUserService( def findByProjectAdminMembership(project: KnoraProject): Task[Chunk[KnoraUser]] = userRepo.findByProjectAdminMembership(project.id) + def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]] = + userRepo.findByGroupMembership(groupIri) + def updateSystemAdminStatus(knoraUser: KnoraUser, status: SystemAdmin): Task[KnoraUser] = updateUser(knoraUser, UserChangeRequest(systemAdmin = Some(status))) @@ -142,6 +146,16 @@ case class KnoraUserService( user <- updateUser(user, UserChangeRequest(groups = Some(user.isInGroup.filterNot(_ == group.groupIri)))).orDie } yield user + def removeUserFromKnoraGroup(user: User, group: KnoraGroup): IO[UserServiceError, KnoraUser] = + userRepo.findById(user.userIri).someOrFailException.orDie.flatMap(removeUserFromKnoraGroup(_, group)) + + def removeUserFromKnoraGroup(user: KnoraUser, group: KnoraGroup): IO[UserServiceError, KnoraUser] = for { + _ <- ZIO + .fail(UserServiceError(s"User ${user.id.value} is not member of group ${group.id.value}.")) + .when(!user.isInGroup.contains(group.id)) + user <- updateUser(user, UserChangeRequest(groups = Some(user.isInGroup.filterNot(_ == group.id)))).orDie + } yield user + def addUserToProject(user: KnoraUser, project: Project): IO[UserServiceError, KnoraUser] = for { _ <- ZIO .fail(UserServiceError(s"User ${user.id.value} is already member of project ${project.projectIri.value}.")) diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala index dd41883e0f..033b14eff5 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala @@ -67,6 +67,11 @@ final case class KnoraUserRepoLive( findAllByTriplePattern(_.has(isInProject, Rdf.iri(projectIri.value))) .map(_ ++ KnoraUserRepo.builtIn.findAllBy(_.isInProject.contains(projectIri))) + override def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]] = { + findAllByTriplePattern(_.has(isInGroup, Rdf.iri(groupIri.value))) + .map(_ ++ KnoraUserRepo.builtIn.findAllBy(_.isInGroup.contains(groupIri))) + } + override def findByEmail(mail: Email): Task[Option[KnoraUser]] = findOneByTriplePattern(_.has(email, Rdf.literalOf(mail.value))) .map(_.orElse(KnoraUserRepo.builtIn.findOneBy(_.email == mail))) diff --git a/webapi/src/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala b/webapi/src/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala index 3df935ca22..d6c91c79ca 100644 --- a/webapi/src/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala @@ -22,7 +22,10 @@ import org.knora.webapi.slice.admin.domain.service.KnoraGroupRepo import org.knora.webapi.slice.admin.domain.service.KnoraGroupService import org.knora.webapi.slice.admin.domain.service.KnoraProjectRepo.builtIn.SystemProject import org.knora.webapi.slice.admin.domain.service.KnoraProjectService +import org.knora.webapi.slice.admin.domain.service.KnoraUserService +import org.knora.webapi.slice.admin.domain.service.PasswordService import org.knora.webapi.slice.admin.repo.service.KnoraGroupRepoInMemory +import org.knora.webapi.slice.admin.repo.service.KnoraUserRepoLive import org.knora.webapi.slice.common.api.AuthorizationRestService import org.knora.webapi.slice.resourceinfo.domain.IriConverter import org.knora.webapi.store.cache.CacheService @@ -131,16 +134,19 @@ object AuthorizationRestServiceSpec extends ZIOSpecDefault { }, ), ).provide( + AppConfig.layer, AuthorizationRestService.layer, CacheService.layer, - KnoraProjectService.layer, - KnoraProjectRepoInMemory.layer, - KnoraGroupService.layer, - KnoraGroupRepoInMemory.layer, - IriService.layer, IriConverter.layer, - TriplestoreServiceLive.layer, + IriService.layer, + KnoraGroupRepoInMemory.layer, + KnoraGroupService.layer, + KnoraProjectRepoInMemory.layer, + KnoraProjectService.layer, + KnoraUserRepoLive.layer, + KnoraUserService.layer, + PasswordService.layer, StringFormatter.live, - AppConfig.layer, + TriplestoreServiceLive.layer, ) } diff --git a/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala b/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala index 17c292ccde..4898e1aedf 100644 --- a/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala +++ b/webapi/src/test/scala/org/knora/webapi/slice/admin/domain/service/GroupServiceSpec.scala @@ -12,6 +12,7 @@ import zio.test.TestEnvironment import zio.test.ZIOSpecDefault import zio.test._ +import org.knora.webapi.config.AppConfig import org.knora.webapi.messages.StringFormatter import org.knora.webapi.messages.store.triplestoremessages.StringLiteralV2 import org.knora.webapi.responders.IriService @@ -24,6 +25,7 @@ import org.knora.webapi.slice.admin.domain.model.GroupStatus import org.knora.webapi.slice.admin.domain.model.KnoraGroup import org.knora.webapi.slice.admin.domain.repo.KnoraProjectRepoInMemory import org.knora.webapi.slice.admin.repo.service.KnoraGroupRepoInMemory +import org.knora.webapi.slice.admin.repo.service.KnoraUserRepoLive import org.knora.webapi.slice.ontology.repo.service.OntologyCacheLive import org.knora.webapi.slice.ontology.repo.service.OntologyRepoLive import org.knora.webapi.slice.resourceinfo.domain.IriConverter @@ -63,6 +65,7 @@ object GroupServiceSpec extends ZIOSpecDefault { } yield assertTrue(knoraGroup == exampleKnoraGroup) }, ).provide( + AppConfig.layer, CacheService.layer, GroupService.layer, IriConverter.layer, @@ -71,8 +74,11 @@ object GroupServiceSpec extends ZIOSpecDefault { KnoraGroupService.layer, KnoraProjectRepoInMemory.layer, KnoraProjectService.layer, + KnoraUserRepoLive.layer, + KnoraUserService.layer, OntologyCacheLive.layer, OntologyRepoLive.layer, + PasswordService.layer, ProjectService.layer, StringFormatter.test, TriplestoreServiceInMemory.emptyLayer, From c466262a4ec253433e25742aed2fbb04a389e35b Mon Sep 17 00:00:00 2001 From: Marcin Procyk Date: Thu, 18 Apr 2024 09:15:36 +0200 Subject: [PATCH 6/7] fmt & change negation of ZIO.when to ZIO.unless --- .../slice/admin/domain/service/KnoraUserService.scala | 10 +++++----- .../slice/admin/repo/service/KnoraUserRepoLive.scala | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala index 10bbba01f2..7f5ea9375f 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala @@ -142,7 +142,7 @@ case class KnoraUserService( def removeUserFromGroup(user: KnoraUser, group: Group): IO[UserServiceError, KnoraUser] = for { _ <- ZIO .fail(UserServiceError(s"User ${user.id.value} is not member of group ${group.groupIri.value}.")) - .when(!user.isInGroup.contains(group.groupIri)) + .unless(user.isInGroup.contains(group.groupIri)) user <- updateUser(user, UserChangeRequest(groups = Some(user.isInGroup.filterNot(_ == group.groupIri)))).orDie } yield user @@ -152,7 +152,7 @@ case class KnoraUserService( def removeUserFromKnoraGroup(user: KnoraUser, group: KnoraGroup): IO[UserServiceError, KnoraUser] = for { _ <- ZIO .fail(UserServiceError(s"User ${user.id.value} is not member of group ${group.id.value}.")) - .when(!user.isInGroup.contains(group.id)) + .unless(user.isInGroup.contains(group.id)) user <- updateUser(user, UserChangeRequest(groups = Some(user.isInGroup.filterNot(_ == group.id)))).orDie } yield user @@ -177,7 +177,7 @@ case class KnoraUserService( ): IO[UserServiceError, KnoraUser] = for { _ <- ZIO .fail(UserServiceError(s"User ${user.id.value} is not member of project ${project.projectIri.value}.")) - .when(!user.isInProject.contains(project.projectIri)) + .unless(user.isInProject.contains(project.projectIri)) projectIri = project.projectIri newIsInProject = user.isInProject.filterNot(_ == projectIri) newIsInProjectAdminGroup = user.isInProjectAdminGroup.filterNot(_ == projectIri) @@ -208,7 +208,7 @@ case class KnoraUserService( val msg = s"User ${user.id.value} is not a member of project ${project.projectIri.value}. A user needs to be a member of the project to be added as project admin." UserServiceError(msg) - }.when(!user.isInProject.contains(project.projectIri)) + }.unless(user.isInProject.contains(project.projectIri)) theChange = UserChangeRequest(projectsAdmin = Some(user.isInProjectAdminGroup :+ project.projectIri)) user <- updateUser(user, theChange).orDie } yield user @@ -227,7 +227,7 @@ case class KnoraUserService( ): IO[UserServiceError, KnoraUser] = for { _ <- ZIO .fail(UserServiceError(s"User ${user.id.value} is not admin member of project ${project.projectIri.value}.")) - .when(!user.isInProjectAdminGroup.contains(project.projectIri)) + .unless(user.isInProjectAdminGroup.contains(project.projectIri)) theChange = UserChangeRequest(projectsAdmin = Some(user.isInProjectAdminGroup.filterNot(_ == project.projectIri))) user <- updateUser(user, theChange).orDie } yield user diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala index 033b14eff5..fb966d6a07 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala @@ -67,10 +67,9 @@ final case class KnoraUserRepoLive( findAllByTriplePattern(_.has(isInProject, Rdf.iri(projectIri.value))) .map(_ ++ KnoraUserRepo.builtIn.findAllBy(_.isInProject.contains(projectIri))) - override def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]] = { + override def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]] = findAllByTriplePattern(_.has(isInGroup, Rdf.iri(groupIri.value))) .map(_ ++ KnoraUserRepo.builtIn.findAllBy(_.isInGroup.contains(groupIri))) - } override def findByEmail(mail: Email): Task[Option[KnoraUser]] = findOneByTriplePattern(_.has(email, Rdf.literalOf(mail.value))) From d1d97fc1a9aeaba6118f0c7be088207085d24ff5 Mon Sep 17 00:00:00 2001 From: Marcin Procyk Date: Fri, 19 Apr 2024 09:21:08 +0200 Subject: [PATCH 7/7] review feedback --- .../admin/domain/service/KnoraGroupService.scala | 5 +---- .../admin/domain/service/KnoraUserService.scala | 12 ++---------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala index bc739401c5..da3c8ebd1f 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala @@ -10,7 +10,6 @@ import zio.Task import zio.ZIO import zio.ZLayer -import dsp.errors.BadRequestException import dsp.errors.DuplicateValueException import org.knora.webapi.responders.IriService import org.knora.webapi.slice.admin.api.GroupsRequests.GroupCreateRequest @@ -70,9 +69,7 @@ case class KnoraGroupService( for { group <- knoraGroupRepo.save(groupToUpdate.copy(status = status)) _ <- ZIO.unless(group.status.value)(knoraUserService.findByGroupMembership(group.id).flatMap { members => - ZIO.foreachDiscard(members)(user => - knoraUserService.removeUserFromKnoraGroup(user, group).mapError(BadRequestException.apply), - ) + ZIO.foreachDiscard(members)(user => knoraUserService.removeUserFromKnoraGroup(user, group.id)) }) } yield group diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala index 7f5ea9375f..48cb15fb87 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala @@ -22,7 +22,6 @@ import org.knora.webapi.slice.admin.domain.model.FamilyName import org.knora.webapi.slice.admin.domain.model.GivenName import org.knora.webapi.slice.admin.domain.model.Group import org.knora.webapi.slice.admin.domain.model.GroupIri -import org.knora.webapi.slice.admin.domain.model.KnoraGroup import org.knora.webapi.slice.admin.domain.model.KnoraProject import org.knora.webapi.slice.admin.domain.model.KnoraProject.ProjectIri import org.knora.webapi.slice.admin.domain.model.KnoraUser @@ -146,15 +145,8 @@ case class KnoraUserService( user <- updateUser(user, UserChangeRequest(groups = Some(user.isInGroup.filterNot(_ == group.groupIri)))).orDie } yield user - def removeUserFromKnoraGroup(user: User, group: KnoraGroup): IO[UserServiceError, KnoraUser] = - userRepo.findById(user.userIri).someOrFailException.orDie.flatMap(removeUserFromKnoraGroup(_, group)) - - def removeUserFromKnoraGroup(user: KnoraUser, group: KnoraGroup): IO[UserServiceError, KnoraUser] = for { - _ <- ZIO - .fail(UserServiceError(s"User ${user.id.value} is not member of group ${group.id.value}.")) - .unless(user.isInGroup.contains(group.id)) - user <- updateUser(user, UserChangeRequest(groups = Some(user.isInGroup.filterNot(_ == group.id)))).orDie - } yield user + def removeUserFromKnoraGroup(user: KnoraUser, groupIri: GroupIri): UIO[KnoraUser] = + userRepo.save(user.copy(isInGroup = user.isInGroup.filterNot(_ == groupIri))).orDie def addUserToProject(user: KnoraUser, project: Project): IO[UserServiceError, KnoraUser] = for { _ <- ZIO