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.

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

perf: Make GET /admin/users faster by caching projects (DEV-3311) #3062

Merged
merged 5 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,8 @@ package org.knora.webapi.core
import org.apache.pekko
import zio.*

import org.knora.webapi.config.AppConfig
import org.knora.webapi.store.cache.settings.CacheServiceSettings

object ActorSystemTest {

def layer(sys: pekko.actor.ActorSystem): ZLayer[AppConfig, Nothing, ActorSystem] =
ZLayer.scoped {
ZIO.serviceWith[AppConfig] { config =>
new ActorSystem {
override val system: pekko.actor.ActorSystem = sys
override val cacheServiceSettings: CacheServiceSettings = new CacheServiceSettings(config)
}
}
}
def layer(sys: pekko.actor.ActorSystem): ZLayer[Any, Nothing, ActorSystem] =
ZLayer.scoped(ZIO.succeed(new ActorSystem { override val system: pekko.actor.ActorSystem = sys }))
}
10 changes: 2 additions & 8 deletions webapi/src/main/scala/org/knora/webapi/core/ActorSystem.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ import zio.macros.accessible

import scala.concurrent.ExecutionContext

import org.knora.webapi.config.AppConfig
import org.knora.webapi.store.cache.settings.CacheServiceSettings

import pekko.actor

@accessible
trait ActorSystem {
val system: pekko.actor.ActorSystem
val cacheServiceSettings: CacheServiceSettings
}

object ActorSystem {
Expand All @@ -43,15 +39,13 @@ object ActorSystem {
.zipLeft(ZIO.logInfo(">>> Release Actor System <<<"))
.orDie

val layer: ZLayer[AppConfig, Nothing, ActorSystem] =
val layer: ZLayer[Any, Nothing, ActorSystem] =
ZLayer.scoped {
for {
config <- ZIO.service[AppConfig]
context <- ZIO.executor.map(_.asExecutionContext)
actorSystem <- ZIO.acquireRelease(acquire(context))(release)
} yield new ActorSystem {
override val system: pekko.actor.ActorSystem = actorSystem
override val cacheServiceSettings: CacheServiceSettings = new CacheServiceSettings(config)
override val system: pekko.actor.ActorSystem = actorSystem
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import dsp.valueobjects.Iri
import dsp.valueobjects.V2
import org.knora.webapi.*
import org.knora.webapi.config.AppConfig
import org.knora.webapi.core.MessageHandler
import org.knora.webapi.core.MessageRelay
import org.knora.webapi.messages.IriConversions.*
Expand All @@ -25,8 +24,6 @@
import org.knora.webapi.messages.admin.responder.usersmessages.UserGetByIriADM
import org.knora.webapi.messages.admin.responder.usersmessages.UserInformationTypeADM
import org.knora.webapi.messages.store.cacheservicemessages.CacheServiceFlushDB
import org.knora.webapi.messages.store.cacheservicemessages.CacheServiceGetProjectADM
import org.knora.webapi.messages.store.cacheservicemessages.CacheServicePutProjectADM
import org.knora.webapi.messages.store.triplestoremessages.*
import org.knora.webapi.messages.twirl.queries.sparql
import org.knora.webapi.messages.util.KnoraSystemInstances
Expand All @@ -43,7 +40,6 @@
import org.knora.webapi.slice.admin.domain.model.UserIri
import org.knora.webapi.slice.admin.domain.service.ProjectADMService
import org.knora.webapi.slice.common.repo.service.PredicateObjectMapper
import org.knora.webapi.store.cache.settings.CacheServiceSettings
import org.knora.webapi.store.triplestore.api.TriplestoreService
import org.knora.webapi.store.triplestore.api.TriplestoreService.Queries.Ask
import org.knora.webapi.store.triplestore.api.TriplestoreService.Queries.Construct
Expand Down Expand Up @@ -79,7 +75,7 @@
* Tries to retrieve a [[ProjectADM]] either from triplestore or cache if caching is enabled.
* If project is not found in cache but in triplestore, then project is written to cache.
*/
def getProjectFromCacheOrTriplestore(identifier: ProjectIdentifierADM): Task[Option[ProjectADM]]
def findByProjectIdentifier(identifier: ProjectIdentifierADM): Task[Option[ProjectADM]]

/**
* Gets the members of a project with the given IRI, shortname, shortcode or UUID. Returns an empty list
Expand Down Expand Up @@ -175,7 +171,6 @@
}

final case class ProjectsResponderADMLive(
private val cacheServiceSettings: CacheServiceSettings,
private val iriService: IriService,
private val messageRelay: MessageRelay,
private val permissionsResponderADM: PermissionsResponderADM,
Expand All @@ -196,7 +191,7 @@
* Receives a message extending [[ProjectsResponderRequestADM]], and returns an appropriate response message.
*/
override def handle(msg: ResponderRequest): Task[Any] = msg match {
case ProjectGetADM(identifier) => getProjectFromCacheOrTriplestore(identifier)
case ProjectGetADM(identifier) => findByProjectIdentifier(identifier)

Check warning on line 194 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L194

Added line #L194 was not covered by tests
case ProjectGetRequestADM(identifier) => getSingleProjectADMRequest(identifier)
case ProjectCreateRequestADM(createRequest, requestingUser, apiRequestID) =>
projectCreateRequestADM(createRequest, requestingUser, apiRequestID)
Expand Down Expand Up @@ -238,9 +233,10 @@
* [[NotFoundException]] When no project for the given IRI can be found.
*/
override def getSingleProjectADMRequest(id: ProjectIdentifierADM): Task[ProjectGetResponseADM] =
getProjectFromCacheOrTriplestore(id)
.flatMap(ZIO.fromOption(_))
.mapBoth(_ => NotFoundException(s"Project '${getId(id)}' not found"), ProjectGetResponseADM)
projectService
.findByProjectIdentifier(id)
.someOrFail(NotFoundException(s"Project '${getId(id)}' not found"))
.map(ProjectGetResponseADM.apply)

Check warning on line 239 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L238-L239

Added lines #L238 - L239 were not covered by tests

/**
* Gets the members of a project with the given IRI, shortname, shortcode or UUID. Returns an empty list
Expand All @@ -256,9 +252,9 @@
): Task[ProjectMembersGetResponseADM] =
for {
/* Get project and verify permissions. */
project <- getProjectFromCacheOrTriplestore(id)
.flatMap(ZIO.fromOption(_))
.orElseFail(NotFoundException(s"Project '${getId(id)}' not found."))
project <- projectService

Check warning on line 255 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L255

Added line #L255 was not covered by tests
.findByProjectIdentifier(id)
.someOrFail(NotFoundException(s"Project '${getId(id)}' not found."))

Check warning on line 257 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L257

Added line #L257 was not covered by tests
_ <- ZIO
.fail(ForbiddenException("SystemAdmin or ProjectAdmin permissions are required."))
.when {
Expand Down Expand Up @@ -317,9 +313,9 @@
): Task[ProjectAdminMembersGetResponseADM] =
for {
/* Get project and verify permissions. */
project <- getProjectFromCacheOrTriplestore(id)
.flatMap(ZIO.fromOption(_))
.orElseFail(NotFoundException(s"Project '${getId(id)}' not found."))
project <- projectService

Check warning on line 316 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L316

Added line #L316 was not covered by tests
.findByProjectIdentifier(id)
.someOrFail(NotFoundException(s"Project '${getId(id)}' not found."))

Check warning on line 318 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L318

Added line #L318 was not covered by tests
_ <- ZIO
.fail(ForbiddenException("SystemAdmin or ProjectAdmin permissions are required."))
.when {
Expand Down Expand Up @@ -499,8 +495,7 @@
for {
_ <- projectService
.findByProjectIdentifier(projectId)
.flatMap(ZIO.fromOption(_))
.orElseFail(NotFoundException(s"Project '${projectIri.value}' not found. Aborting update request."))
.someOrFail(NotFoundException(s"Project '${projectIri.value}' not found. Aborting update request."))

Check warning on line 498 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L498

Added line #L498 was not covered by tests

// we are changing the project, so lets get rid of the cached copy
_ <- messageRelay.ask[Any](CacheServiceFlushDB(KnoraSystemInstances.Users.SystemUser))
Expand All @@ -523,8 +518,7 @@
updatedProject <-
projectService
.findByProjectIdentifier(projectId)
.flatMap(ZIO.fromOption(_))
.orElseFail(UpdateNotPerformedException("Project was not updated. Please report this as a possible bug."))
.someOrFail(UpdateNotPerformedException("Project was not updated. Please report this as a possible bug."))

Check warning on line 521 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L521

Added line #L521 was not covered by tests

_ <- ZIO.logDebug(
s"updateProjectADM - projectUpdatePayload: $projectUpdatePayload / updatedProject: $updatedProject"
Expand Down Expand Up @@ -794,46 +788,8 @@
IriLocker.runWithIriLock(apiRequestID, PROJECTS_GLOBAL_LOCK_IRI, task)
}

////////////////////
// Helper Methods //
////////////////////

/**
* Tries to retrieve a [[ProjectADM]] either from triplestore or cache if caching is enabled.
* If project is not found in cache but in triplestore, then project is written to cache.
*/
override def getProjectFromCacheOrTriplestore(
identifier: ProjectIdentifierADM
): Task[Option[ProjectADM]] =
if (cacheServiceSettings.cacheServiceEnabled) {
// caching enabled
getProjectFromCache(identifier).flatMap {
case None =>
// none found in cache. getting from triplestore.
projectService.findByProjectIdentifier(identifier).flatMap {
case None =>
// also none found in triplestore. finally returning none.
logger.debug("getProjectFromCacheOrTriplestore - not found in cache and in triplestore")
ZIO.succeed(None)
case Some(project) =>
// found a project in the triplestore. need to write to cache.
logger.debug(
"getProjectFromCacheOrTriplestore - not found in cache but found in triplestore. need to write to cache."
)
// writing project to cache and afterwards returning the project found in the triplestore
messageRelay
.ask[Unit](CacheServicePutProjectADM(project))
.as(Some(project))
}
case Some(project) =>
logger.debug("getProjectFromCacheOrTriplestore - found in cache. returning project.")
ZIO.succeed(Some(project))
}
} else {
// caching disabled
logger.debug("getProjectFromCacheOrTriplestore - caching disabled. getting from triplestore.")
projectService.findByProjectIdentifier(identifier)
}
override def findByProjectIdentifier(identifier: ProjectIdentifierADM): Task[Option[ProjectADM]] =
projectService.findByProjectIdentifier(identifier)

Check warning on line 792 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L792

Added line #L792 was not covered by tests

/**
* Helper method for checking if a project identified by shortname exists.
Expand All @@ -852,26 +808,23 @@
*/
private def projectByShortcodeExists(shortcode: String): Task[Boolean] =
triplestore.query(Ask(sparql.admin.txt.checkProjectExistsByShortcode(shortcode)))

private def getProjectFromCache(identifier: ProjectIdentifierADM): Task[Option[ProjectADM]] =
messageRelay.ask[Option[ProjectADM]](CacheServiceGetProjectADM(identifier)).map(_.map(_.unescape))
}

object ProjectsResponderADMLive {
val layer: URLayer[
AppConfig & IriService & MessageRelay & PermissionsResponderADM & PredicateObjectMapper & ProjectADMService & StringFormatter & TriplestoreService,
val layer: ZLayer[
IriService & MessageRelay & PermissionsResponderADM & PredicateObjectMapper & ProjectADMService & StringFormatter & TriplestoreService,
Nothing,
ProjectsResponderADMLive
] = ZLayer.fromZIO {
for {
c <- ZIO.service[AppConfig].map(new CacheServiceSettings(_))
iris <- ZIO.service[IriService]
ps <- ZIO.service[ProjectADMService]
sf <- ZIO.service[StringFormatter]
ts <- ZIO.service[TriplestoreService]
po <- ZIO.service[PredicateObjectMapper]
mr <- ZIO.service[MessageRelay]
pr <- ZIO.service[PermissionsResponderADM]
handler <- mr.subscribe(ProjectsResponderADMLive(c, iris, mr, pr, po, ps, ts, sf))
handler <- mr.subscribe(ProjectsResponderADMLive(iris, mr, pr, po, ps, ts, sf))

Check warning on line 827 in webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/responders/admin/ProjectsResponderADM.scala#L827

Added line #L827 was not covered by tests
} yield handler
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
import org.knora.webapi.slice.admin.domain.model.RestrictedView
import org.knora.webapi.slice.ontology.domain.service.OntologyRepo
import org.knora.webapi.slice.resourceinfo.domain.InternalIri
import org.knora.webapi.store.cache.api.CacheService

final case class ProjectADMService(
private val ontologyRepo: OntologyRepo,
private val projectRepo: KnoraProjectRepo
private val projectRepo: KnoraProjectRepo,
private val cacheService: CacheService
) {

def findAll: Task[List[ProjectADM]] = projectRepo.findAll().flatMap(ZIO.foreachPar(_)(toProjectADM))
Expand All @@ -29,7 +31,14 @@
findByProjectIdentifier(ProjectIdentifierADM.from(id))

def findByProjectIdentifier(projectId: ProjectIdentifierADM): Task[Option[ProjectADM]] =
projectRepo.findById(projectId).flatMap(ZIO.foreach(_)(toProjectADM))
cacheService.getProjectADM(projectId).flatMap {
case Some(project) => ZIO.some(project)

Check warning on line 35 in webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/ProjectADMService.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/ProjectADMService.scala#L34-L35

Added lines #L34 - L35 were not covered by tests
case None =>
projectRepo.findById(projectId).flatMap(ZIO.foreach(_)(toProjectADM)).tap {
case Some(prj) => cacheService.putProjectADM(prj)
case None => ZIO.unit

Check warning on line 39 in webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/ProjectADMService.scala

View check run for this annotation

Codecov / codecov/patch

webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/ProjectADMService.scala#L37-L39

Added lines #L37 - L39 were not covered by tests
}
}

private def toProjectADM(knoraProject: KnoraProject): Task[ProjectADM] =
ZIO.attempt(
Expand Down

This file was deleted.