Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: Move creation Group from responder to services (DEV-3292) #3185

Merged
merged 7 commits into from Apr 15, 2024
Merged

refactor: Move creation Group from responder to services (DEV-3292) #3185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3bd580a
move group creation to slice services
mpro7 Apr 11, 2024
782f713
remove responder method & replace in tests
mpro7 Apr 11, 2024
c263c75
fix layers
mpro7 Apr 11, 2024
9620ec8
expect external dataset in test
mpro7 Apr 12, 2024
251bba7
Merge branch 'main' into dev-3292-4
mpro7 Apr 12, 2024
8fcb122
review feedback
mpro7 Apr 15, 2024
c9f1c78
Merge branch 'main' into dev-3292-4
mpro7 Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions integration/src/test/scala/org/knora/webapi/e2e/admin/GroupsADME2ESpec.scala
View file
Open in desktop
Expand Up @@ -183,13 +183,13 @@ class GroupsADME2ESpec extends E2ESpec with GroupsADMJsonProtocol {
val response: HttpResponse = singleAwaitingRequest(request)
response.status should be(StatusCodes.OK)

val groupInfo: Group = AkkaHttpUtils.httpResponseToJson(response).fields("group").convertTo[Group]
val group: Group = AkkaHttpUtils.httpResponseToJson(response).fields("group").convertTo[Group]

groupInfo.name should be("NewGroup")
groupInfo.descriptions should be(Seq(StringLiteralV2.from("NewGroupDescription", Some("en"))))
groupInfo.project should be(Some(SharedTestDataADM.imagesProjectExternal))
groupInfo.status should be(true)
groupInfo.selfjoin should be(false)
group.name should be("NewGroup")
group.descriptions should be(Seq(StringLiteralV2.from("NewGroupDescription", Some("en"))))
group.project should be(Some(SharedTestDataADM.imagesProjectExternal))
group.status should be(true)
group.selfjoin should be(false)

clientTestDataCollector.addFile(
TestDataFileContent(
Expand All @@ -202,7 +202,7 @@ class GroupsADME2ESpec extends E2ESpec with GroupsADMJsonProtocol {
),
)

val iri = groupInfo.id
val iri = group.id
newGroupIri.set(iri)
}

Expand Down
30 changes: 15 additions & 15 deletions integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala
View file
Open in desktop
Expand Up @@ -65,8 +65,8 @@ class GroupsResponderADMSpec extends CoreSpec {

"CREATE the group and return the group's info if the supplied group name is unique" in {
val response = UnsafeZioRun.runOrThrow(
ZIO.serviceWithZIO[GroupsResponderADM](
_.createGroup(
groupRestService(
_.postGroup(
GroupCreateRequest(
id = None,
name = GroupName.unsafeFrom("NewGroup"),
Expand All @@ -83,29 +83,29 @@ class GroupsResponderADMSpec extends CoreSpec {
status = GroupStatus.active,
selfjoin = GroupSelfJoin.disabled,
),
UUID.randomUUID,
rootUser,
),
),
)

val newGroupInfo = response.group
newGroupInfo.name should equal("NewGroup")
newGroupInfo.descriptions should equal(
val group = response.group
group.name should equal("NewGroup")
group.descriptions should equal(
Seq(StringLiteralV2.from("""NewGroupDescription with "quotes" and <html tag>""", Some("en"))),
)
newGroupInfo.project should equal(Some(imagesProject))
newGroupInfo.status should equal(true)
newGroupInfo.selfjoin should equal(false)
group.project should equal(Some(imagesProjectExternal))
group.status should equal(true)
group.selfjoin should equal(false)

// store for later usage
newGroupIri.set(newGroupInfo.id)
newGroupIri.set(group.id)
}

"return a 'DuplicateValueException' if the supplied group name is not unique" in {
val groupName = GroupName.unsafeFrom("NewGroup")
val exit = UnsafeZioRun.run(
ZIO.serviceWithZIO[GroupsResponderADM](
_.createGroup(
groupRestService(
_.postGroup(
GroupCreateRequest(
id = Some(GroupIri.unsafeFrom(imagesReviewerGroup.id)),
name = groupName,
Expand All @@ -115,13 +115,13 @@ class GroupsResponderADMSpec extends CoreSpec {
status = GroupStatus.active,
selfjoin = GroupSelfJoin.disabled,
),
UUID.randomUUID,
rootUser,
),
),
)
assertFailsWithA[DuplicateValueException](
exit,
s"Group with the name '${groupName.value}' already exists",
s"Group with name: '${groupName.value}' already exists.",
)
}

Expand Down Expand Up @@ -200,7 +200,7 @@ class GroupsResponderADMSpec extends CoreSpec {
)
assertFailsWithA[BadRequestException](
exit,
s"Group with the name '${groupName.value}' already exists.",
s"Group with name: '${groupName.value}' already exists.",
)
}

Expand Down
11 changes: 11 additions & 0 deletions webapi/src/main/scala/org/knora/webapi/responders/IriService.scala
View file
Open in desktop
Expand Up @@ -15,6 +15,8 @@ import org.knora.webapi.messages.SmartIri
import org.knora.webapi.messages.StringFormatter
import org.knora.webapi.messages.StringFormatter.MAX_IRI_ATTEMPTS
import org.knora.webapi.messages.twirl.queries.sparql
import org.knora.webapi.slice.admin.domain.model.GroupIri
import org.knora.webapi.slice.admin.domain.model.KnoraProject.Shortcode
import org.knora.webapi.slice.admin.domain.model.UserIri
import org.knora.webapi.slice.resourceinfo.domain.IriConverter
import org.knora.webapi.store.triplestore.api.TriplestoreService
Expand Down Expand Up @@ -71,6 +73,15 @@ final case class IriService(
.orElseFail(BadRequestException(s"Invalid User IRI: $userIriStr"))
} yield userIri

def checkOrCreateNewGroupIri(entityIri: Option[GroupIri], shortcode: Shortcode): Task[GroupIri] =
for {
iriToSmartIri <- ZIO.foreach(entityIri.map(_.value))(iriConverter.asSmartIri)
checkedCustomIriOrNewIri <- checkOrCreateEntityIri(iriToSmartIri, GroupIri.makeNew(shortcode).value)
iri <- ZIO
.fromEither(GroupIri.from(checkedCustomIriOrNewIri))
.orElseFail(BadRequestException(s"Invalid Group IRI: $checkedCustomIriOrNewIri"))
} yield iri

/**
* Checks whether an entity with the provided custom IRI exists in the triplestore. If yes, throws an exception.
* If no custom IRI was given, creates a random unused IRI.
Expand Down
58 changes: 1 addition & 57 deletions webapi/src/main/scala/org/knora/webapi/responders/admin/GroupsResponderADM.scala
View file
Open in desktop
Expand Up @@ -15,10 +15,8 @@ import org.knora.webapi._
import org.knora.webapi.core.MessageHandler
import org.knora.webapi.core.MessageRelay
import org.knora.webapi.messages.IriConversions._
import org.knora.webapi.messages.OntologyConstants
import org.knora.webapi.messages.OntologyConstants.KnoraAdmin._
import org.knora.webapi.messages.ResponderRequest
import org.knora.webapi.messages.SmartIri
import org.knora.webapi.messages.StringFormatter
import org.knora.webapi.messages.admin.responder.groupsmessages._
import org.knora.webapi.messages.admin.responder.projectsmessages.Project
Expand All @@ -30,8 +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.AdminConstants
import org.knora.webapi.slice.admin.api.GroupsRequests.GroupCreateRequest
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
Expand Down Expand Up @@ -185,58 +181,6 @@ final case class GroupsResponderADM(
def groupMembersGetRequest(iri: GroupIri, user: User): Task[GroupMembersGetResponseADM] =
groupMembersGetADM(iri.value, user).map(GroupMembersGetResponseADM.apply)

/**
* Create a new group.
*
* @param request the create request information.
* @param apiRequestID the unique request ID.
* @return a [[GroupGetResponseADM]]
*/
def createGroup(
request: GroupCreateRequest,
apiRequestID: UUID,
): Task[GroupGetResponseADM] = {
val task = for {
nameExists <- groupByNameAndProjectExists(request.name.value, request.project.value)
_ <- ZIO
.fail(DuplicateValueException(s"Group with the name '${request.name.value}' already exists"))
.when(nameExists)

project <-
findProjectByIriOrFail(
request.project.value,
NotFoundException(s"Cannot create group inside project <${request.project}>. The project was not found."),
)

// check the custom IRI; if not given, create an unused IRI
customGroupIri: Option[SmartIri] = request.id.map(_.value).map(iri => iri.toSmartIri)
groupIri <- iriService.checkOrCreateEntityIri(customGroupIri, GroupIri.makeNew(project.getShortcode).value)

/* create the group */
createNewGroupSparqlString =
sparql.admin.txt
.createNewGroup(
AdminConstants.adminDataNamedGraph.value,
groupIri,
groupClassIri = OntologyConstants.KnoraAdmin.UserGroup,
name = request.name.value,
descriptions = request.descriptions.value,
projectIri = request.project.value,
status = request.status.value,
hasSelfJoinEnabled = request.selfjoin.value,
)

_ <- triplestore.query(Update(createNewGroupSparqlString))

/* Verify that the group was created and updated */
createdGroup <-
groupGetADM(groupIri)
.someOrFail(UpdateNotPerformedException("Group was not created. Please report this as a possible bug."))

} yield GroupGetResponseADM(createdGroup)
IriLocker.runWithIriLock(apiRequestID, "http://rdfh.ch/groups", task)
}

/**
* Change group's basic information.
*
Expand Down Expand Up @@ -324,7 +268,7 @@ final case class GroupsResponderADM(
} yield groupByNameAndProjectExists(name.value, project.id)
).getOrElse(ZIO.succeed(false))
_ <- ZIO
.fail(BadRequestException(s"Group with the name '${request.name.get.value}' already exists."))
.fail(BadRequestException(s"Group with name: '${request.name.get.value}' already exists."))
.when(groupByNameAlreadyExists)

/* Update group */
Expand Down
12 changes: 8 additions & 4 deletions webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/GroupRestService.scala
View file
Open in desktop
Expand Up @@ -17,14 +17,16 @@ 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.User
import org.knora.webapi.slice.admin.domain.service.GroupService
import org.knora.webapi.slice.admin.domain.service.KnoraProjectService
import org.knora.webapi.slice.common.api.AuthorizationRestService
import org.knora.webapi.slice.common.api.KnoraResponseRenderer

final case class GroupRestService(
auth: AuthorizationRestService,
format: KnoraResponseRenderer,
groupService: GroupService,
knoraProjectService: KnoraProjectService,
responder: GroupsResponderADM,
format: KnoraResponseRenderer,
) {

def getGroups: Task[GroupsGetResponseADM] = for {
Expand All @@ -49,9 +51,11 @@ final case class GroupRestService(

def postGroup(request: GroupCreateRequest, user: User): Task[GroupGetResponseADM] =
for {
_ <- auth.ensureSystemAdminOrProjectAdmin(user, request.project)
uuid <- Random.nextUUID
internal <- responder.createGroup(request, uuid)
_ <- auth.ensureSystemAdminOrProjectAdmin(user, request.project)
project <- knoraProjectService
.findById(request.project)
.someOrFail(NotFoundException(s"Project <${request.project}> not found."))
internal <- groupService.createGroup(request, project).map(GroupGetResponseADM.apply)
external <- format.toExternalADM(internal)
} yield external

Expand Down
5 changes: 5 additions & 0 deletions webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala
View file
Open in desktop
Expand Up @@ -8,9 +8,11 @@ package org.knora.webapi.slice.admin.domain.service
import zio.ZIO
import zio._

import org.knora.webapi.slice.admin.api.GroupsRequests.GroupCreateRequest
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

final case class GroupService(
private val knoraGroupService: KnoraGroupService,
Expand All @@ -36,6 +38,9 @@ final case class GroupService(
status = knoraGroup.status.value,
selfjoin = knoraGroup.hasSelfJoinEnabled.value,
)

def createGroup(request: GroupCreateRequest, project: KnoraProject): Task[Group] =
knoraGroupService.createGroup(request, project).flatMap(toGroup)
}

object GroupService {
Expand Down
4 changes: 4 additions & 0 deletions webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupRepo.scala
View file
Open in desktop
Expand Up @@ -19,6 +19,10 @@ import org.knora.webapi.slice.common.repo.service.Repository

trait KnoraGroupRepo extends Repository[KnoraGroup, GroupIri] {

def findByName(name: GroupName): Task[Option[KnoraGroup]]

def existsByName(name: GroupName): Task[Boolean] = findByName(name).map(_.isDefined)

/**
* Saves the user group, returns the created data. Updates not supported.
*
Expand Down
33 changes: 32 additions & 1 deletion webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala
View file
Open in desktop
Expand Up @@ -7,12 +7,22 @@ package org.knora.webapi.slice.admin.domain.service

import zio.Chunk
import zio.Task
import zio.ZIO
import zio.ZLayer

import dsp.errors.DuplicateValueException
import org.knora.webapi.responders.IriService
import org.knora.webapi.slice.admin.api.GroupsRequests.GroupCreateRequest
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.KnoraGroup
import org.knora.webapi.slice.admin.domain.model.KnoraProject

case class KnoraGroupService(knoraGroupRepo: KnoraGroupRepo) {
case class KnoraGroupService(
knoraGroupRepo: KnoraGroupRepo,
knoraProjectRepo: KnoraProjectRepo,
mpro7 marked this conversation as resolved.
Show resolved Hide resolved
iriService: IriService,
) {

def findAllRegularGroups(): Task[Chunk[KnoraGroup]] = findAll().map(_.filter(_.id.isRegularGroupIri))

Expand All @@ -21,6 +31,27 @@ case class KnoraGroupService(knoraGroupRepo: KnoraGroupRepo) {
def findById(id: GroupIri): Task[Option[KnoraGroup]] = knoraGroupRepo.findById(id)

def findByIds(ids: Seq[GroupIri]): Task[Chunk[KnoraGroup]] = knoraGroupRepo.findByIds(ids)

def createGroup(request: GroupCreateRequest, project: KnoraProject): Task[KnoraGroup] =
for {
_ <- ensureGroupNameIsUnique(request.name)
groupIri <- iriService.checkOrCreateNewGroupIri(request.id, project.shortcode)
group =
KnoraGroup(
id = groupIri,
groupName = request.name,
groupDescriptions = request.descriptions,
status = request.status,
belongsToProject = Some(project.id),
hasSelfJoinEnabled = request.selfjoin,
)
_ <- knoraGroupRepo.save(group)
} yield group
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this belong in KnoraGroupRepo? What's the difference between KnoraGroupRepo and this file? In master this file is a really thin wrapper over it, does it really serve a purpose? I see @seakayone made it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sole responsibility for repositories is to store entities and retrieve them. Maintaining constraints on certain properties of the entity which cannot be modelled by a type system, e.g. the uniqueness of a property other than the id of the entity is not the responsibility of a repository.

The admin module does not expose the repositories but only services operating on these. The methods of these services represent certain use cases and contain the business logic based on them, e.g. the creation of a group with a custom IRI.


private def ensureGroupNameIsUnique(name: GroupName) =
ZIO.whenZIO(knoraGroupRepo.existsByName(name)) {
ZIO.fail(DuplicateValueException(s"Group with name: '${name.value}' already exists."))
}
}

object KnoraGroupService {
Expand Down
4 changes: 4 additions & 0 deletions webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraGroupRepoLive.scala
View file
Open in desktop
Expand Up @@ -53,6 +53,10 @@ final case class KnoraGroupRepoLive(triplestore: TriplestoreService, mapper: Rdf
.die(new IllegalArgumentException("Update not supported for built-in groups"))
.when(KnoraGroupRepo.builtIn.findOneBy(_.id == group.id).isDefined) *>
super.save(group)

override def findByName(name: GroupName): Task[Option[KnoraGroup]] =
findOneByTriplePattern(_.has(groupName, Rdf.literalOf(name.value)))
.map(_.orElse(KnoraGroupRepo.builtIn.findOneBy(_.groupName == name)))
}

object KnoraGroupRepoLive {
Expand Down
10 changes: 10 additions & 0 deletions ...rc/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala
View file
Open in desktop
Expand Up @@ -12,7 +12,10 @@ import zio.test._

import dsp.errors.ForbiddenException
import org.knora.webapi.TestDataFactory
import org.knora.webapi.config.AppConfig
import org.knora.webapi.messages.StringFormatter
import org.knora.webapi.messages.admin.responder.permissionsmessages.PermissionsDataADM
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
Expand All @@ -21,7 +24,9 @@ import org.knora.webapi.slice.admin.domain.service.KnoraProjectRepo.builtIn.Syst
import org.knora.webapi.slice.admin.domain.service.KnoraProjectService
import org.knora.webapi.slice.admin.repo.service.KnoraGroupRepoInMemory
import org.knora.webapi.slice.common.api.AuthorizationRestService
import org.knora.webapi.slice.resourceinfo.domain.IriConverter
import org.knora.webapi.store.cache.CacheService
import org.knora.webapi.store.triplestore.impl.TriplestoreServiceLive

object AuthorizationRestServiceSpec extends ZIOSpecDefault {

Expand Down Expand Up @@ -132,5 +137,10 @@ object AuthorizationRestServiceSpec extends ZIOSpecDefault {
KnoraProjectRepoInMemory.layer,
KnoraGroupService.layer,
KnoraGroupRepoInMemory.layer,
IriService.layer,
IriConverter.layer,
TriplestoreServiceLive.layer,
StringFormatter.live,
AppConfig.layer,
)
}
5 changes: 4 additions & 1 deletion webapi/src/test/scala/org/knora/webapi/slice/admin/repo/service/KnoraGroupRepoLiveSpec.scala
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@ package org.knora.webapi.slice.admin.repo.service

import zio.Chunk
import zio.Ref
import zio.Task
import zio.ZIO
import zio.ZLayer
import zio.test.Gen
Expand All @@ -26,7 +27,9 @@ import org.knora.webapi.store.triplestore.api.TriplestoreServiceInMemory

final case class KnoraGroupRepoInMemory(groups: Ref[Chunk[KnoraGroup]])
extends AbstractInMemoryCrudRepository[KnoraGroup, GroupIri](groups, _.id)
with KnoraGroupRepo {}
with KnoraGroupRepo {
override def findByName(name: GroupName): Task[Option[KnoraGroup]] = ???
}

object KnoraGroupRepoInMemory {
val layer = ZLayer.fromZIO(Ref.make(Chunk.empty[KnoraGroup])) >>>
Expand Down