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

Improve speed of user list query #7466

Merged
merged 18 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released

### Changed
- Improved loading speed of the annotation list. [#7410](https://github.com/scalableminds/webknossos/pull/7410)
- Improved loading speed for the users list. [#7466](https://github.com/scalableminds/webknossos/pull/7466)
- Admins and Team Managers can now also download job exports for jobs of other users, if they have the link. [#7462](https://github.com/scalableminds/webknossos/pull/7462)
- Updated some dependencies of the backend code (play 2.9, sbt 1.9, minor upgrades for others) for optimized performance. [#7366](https://github.com/scalableminds/webknossos/pull/7366)
- Processing jobs can now be distributed to multiple webknossos-workers with finer-grained configurability. Compare migration guide. [#7463](https://github.com/scalableminds/webknossos/pull/7463)
Expand Down
13 changes: 8 additions & 5 deletions app/controllers/UserController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,14 @@ class UserController @Inject()(userService: UserService,
isAdmin: Option[Boolean]
): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
for {
users <- userDAO.findAllWithFilters(isEditable, isTeamManagerOrAdmin, isAdmin, request.identity)
js <- Fox.serialCombined(users.sortBy(_.lastName.toLowerCase))(u => userService.publicWrites(u, request.identity))
} yield {
Ok(Json.toJson(js))
}
(users, userCompactInfos) <- userDAO.findAllCompactWithFilters(isEditable,
isTeamManagerOrAdmin,
isAdmin,
request.identity)
zipped = users.zip(userCompactInfos)
js <- Fox.serialCombined(zipped.sortBy(_._1.lastName.toLowerCase))(u =>
userService.publicWritesCompact(u._1, u._2))
} yield Ok(Json.toJson(js))
}

private val userUpdateReader =
Expand Down
27 changes: 12 additions & 15 deletions app/models/annotation/Annotation.scala
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,6 @@ class AnnotationDAO @Inject()(sqlClient: SqlClient, annotationLayerDAO: Annotati
} yield parsed
}

private def parseObjectIdArray(objectIdArray: String): Seq[ObjectId] =
Option(objectIdArray).map(_.split(",").map(id => ObjectId(id))).getOrElse(Array[ObjectId]()).toSeq

def findAllListableExplorationals(
isFinished: Option[Boolean],
forUser: Option[ObjectId],
Expand All @@ -328,9 +325,9 @@ class AnnotationDAO @Inject()(sqlClient: SqlClient, annotationLayerDAO: Annotati
u.firstname,
u.lastname,
a.othersmayedit,
STRING_AGG(t._id, ',') AS team_ids,
STRING_AGG(t.name, ',') AS team_names,
STRING_AGG(t._organization, ',') AS team_orgs,
ARRAY_REMOVE(ARRAY_AGG(t._id), null) AS team_ids,
ARRAY_REMOVE(ARRAY_AGG(t.name), null) AS team_names,
ARRAY_REMOVE(ARRAY_AGG(t._organization), null) AS team_orgs,
a.modified,
a.statistics,
a.tags,
Expand All @@ -340,9 +337,9 @@ class AnnotationDAO @Inject()(sqlClient: SqlClient, annotationLayerDAO: Annotati
a.visibility,
a.tracingtime,
o.name,
STRING_AGG(al.tracingid, ',') AS tracing_ids,
STRING_AGG(al.name, ',') AS tracing_names,
STRING_AGG(al.typ :: varchar, ',') AS tracing_typs
ARRAY_REMOVE(ARRAY_AGG(al.tracingid), null) AS tracing_ids,
ARRAY_REMOVE(ARRAY_AGG(al.name), null) AS tracing_names,
ARRAY_REMOVE(ARRAY_AGG(al.typ :: varchar), null) AS tracing_typs
FROM webknossos.annotations as a
LEFT JOIN webknossos.users_ u
ON u._id = a._user
Expand Down Expand Up @@ -395,9 +392,9 @@ class AnnotationDAO @Inject()(sqlClient: SqlClient, annotationLayerDAO: Annotati
ownerFirstName = r._5,
ownerLastName = r._6,
othersMayEdit = r._7,
teamIds = parseObjectIdArray(r._8),
teamNames = Option(r._9).map(_.split(",")).getOrElse(Array[String]()).toSeq,
teamOrganizationIds = parseObjectIdArray(r._10),
teamIds = parseArrayLiteral(r._8).map(ObjectId(_)),
teamNames = parseArrayLiteral(r._9),
teamOrganizationIds = parseArrayLiteral(r._10).map(ObjectId(_)),
modified = r._11,
stats = Json.parse(r._12).validate[JsObject].getOrElse(Json.obj()),
tags = parseArrayLiteral(r._13).toSet,
Expand All @@ -407,9 +404,9 @@ class AnnotationDAO @Inject()(sqlClient: SqlClient, annotationLayerDAO: Annotati
visibility = AnnotationVisibility.fromString(r._17).getOrElse(AnnotationVisibility.Internal),
tracingTime = Option(r._18),
organizationName = r._19,
tracingIds = Option(r._20).map(_.split(",")).getOrElse(Array[String]()).toSeq,
annotationLayerNames = Option(r._21).map(_.split(",")).getOrElse(Array[String]()).toSeq,
annotationLayerTypes = Option(r._22).map(_.split(",")).getOrElse(Array[String]()).toSeq
tracingIds = parseArrayLiteral(r._20),
annotationLayerNames = parseArrayLiteral(r._21),
annotationLayerTypes = parseArrayLiteral(r._22)
)
}
)
Expand Down
165 changes: 144 additions & 21 deletions app/models/user/User.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import com.scalableminds.webknossos.schema.Tables._
import javax.inject.Inject
import models.team._
import play.api.libs.json._
import slick.jdbc.GetResult
import slick.jdbc.PostgresProfile.api._
import slick.jdbc.TransactionIsolation.Serializable
import slick.lifted.Rep
import utils.sql.{SQLDAO, SimpleSQLDAO, SqlClient, SqlToken}
import utils.ObjectId

import java.sql.Timestamp
import scala.concurrent.ExecutionContext

object User {
Expand Down Expand Up @@ -60,6 +62,57 @@ case class User(

}

case class UserCompactInfo(
_id: String,
_multiUserId: String,
email: String,
firstname: String,
lastname: String,
userConfiguration: String,
isAdmin: Boolean,
isOrganizationOwner: Boolean,
isDatasetManager: Boolean,
isDeactivated: Boolean,
teamIdsAsArrayLiteral: String,
teamNamesAsArrayLiteral: String,
teamManagersAsArrayLiteral: String,
experienceValuesAsArrayLiteral: String,
experienceDomainsAsArrayLiteral: String,
lastActivity: Timestamp,
organization_id: String,
organization_name: String,
novelUserExperienceInfos: String,
selectedTheme: String,
created: Timestamp,
lastTaskTypeId: Option[String],
isSuperUser: Boolean,
isEmailVerified: Boolean,
isEditable: Boolean
) {
def toUser(implicit ec: ExecutionContext): Fox[User] =
for {
userConfiguration <- Fox.box2Fox(parseAndValidateJson[JsObject](userConfiguration))
} yield {
User(
ObjectId(_id),
ObjectId(_multiUserId),
ObjectId(organization_id),
firstname,
lastname,
Instant.fromSql(lastActivity),
userConfiguration,
LoginInfo(User.default_login_provider_id, _id),
isAdmin,
isOrganizationOwner,
isDatasetManager,
isDeactivated,
isUnlisted = false,
Instant.fromSql(created),
lastTaskTypeId.map(ObjectId(_))
)
}
}

class UserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
extends SQLDAO[User, UsersRow, Users](sqlClient) {
protected val collection = Users
Expand Down Expand Up @@ -92,14 +145,18 @@ class UserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
}

override protected def readAccessQ(requestingUserId: ObjectId) =
q"""(_id in (select _user from webknossos.user_team_roles where _team in (select _team from webknossos.user_team_roles where _user = $requestingUserId and isTeamManager)))
or (_organization in (select _organization from webknossos.users_ where _id = $requestingUserId and isAdmin))
or _id = $requestingUserId"""
readAccessQWithPrefix(requestingUserId, SqlToken.raw(""))

protected def readAccessQWithPrefix(requestingUserId: ObjectId, userPrefix: SqlToken) =
q"""(${userPrefix}_id in (select _user from webknossos.user_team_roles where _team in (select _team from webknossos.user_team_roles where _user = $requestingUserId and isTeamManager)))
or (${userPrefix}_organization in (select _organization from webknossos.users_ where _id = $requestingUserId and isAdmin))
or ${userPrefix}_id = $requestingUserId"""
override protected def deleteAccessQ(requestingUserId: ObjectId) =
q"_organization in (select _organization from webknossos.users_ where _id = $requestingUserId and isAdmin)"

private def listAccessQ(requestingUserId: ObjectId) =
q"""(${readAccessQ(requestingUserId)})
private def listAccessQ(requestingUserId: ObjectId) = listAccessQWithPrefix(requestingUserId, SqlToken.raw(""))
private def listAccessQWithPrefix(requestingUserId: ObjectId, prefix: SqlToken) =
q"""(${readAccessQWithPrefix(requestingUserId, prefix)})
and
(
isUnlisted = false
Expand Down Expand Up @@ -130,48 +187,114 @@ class UserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
def buildSelectionPredicates(isEditableOpt: Option[Boolean],
isTeamManagerOrAdminOpt: Option[Boolean],
isAdminOpt: Option[Boolean],
requestingUser: User)(implicit ctx: DBAccessContext): Fox[SqlToken] =
requestingUser: User,
userPrefix: SqlToken)(implicit ctx: DBAccessContext): Fox[SqlToken] =
for {
accessQuery <- accessQueryFromAccessQ(listAccessQ)
accessQuery <- accessQueryFromAccessQWithPrefix(listAccessQWithPrefix, userPrefix)
editablePredicate = isEditableOpt match {
case Some(isEditable) =>
val usersInTeamsManagedByRequestingUser =
q"(SELECT _user FROM webknossos.user_team_roles WHERE _team IN (SELECT _team FROM webknossos.user_team_roles WHERE _user = ${requestingUser._id} AND isTeamManager)))"
if (isEditable) {
q"(_id IN $usersInTeamsManagedByRequestingUser OR (${requestingUser.isAdmin} AND _organization = ${requestingUser._organization})"
q"(${userPrefix}_id IN $usersInTeamsManagedByRequestingUser OR (${requestingUser.isAdmin} AND ${userPrefix}_organization = ${requestingUser._organization})"
} else {
q"(_id NOT IN $usersInTeamsManagedByRequestingUser AND (NOT (${requestingUser.isAdmin} AND _organization = ${requestingUser._organization}))"
q"(${userPrefix}_id NOT IN $usersInTeamsManagedByRequestingUser AND (NOT (${requestingUser.isAdmin} AND ${userPrefix}_organization = ${requestingUser._organization}))"
}
case None => q"${true}"
}
isTeamManagerOrAdminPredicate = isTeamManagerOrAdminOpt match {
case Some(isTeamManagerOrAdmin) =>
val teamManagers = q"(SELECT _user FROM webknossos.user_team_roles WHERE isTeamManager)"
if (isTeamManagerOrAdmin) {
q"_id IN $teamManagers OR isAdmin"
q"${userPrefix}_id IN $teamManagers OR ${userPrefix}isAdmin"
} else {
q"_id NOT IN $teamManagers AND NOT isAdmin"
q"${userPrefix}_id NOT IN $teamManagers AND NOT ${userPrefix}isAdmin"
}
case None => q"${true}"
}
adminPredicate = isAdminOpt.map(isAdmin => q"isAdmin = $isAdmin").getOrElse(q"${true}")
adminPredicate = isAdminOpt.map(isAdmin => q"${userPrefix}isAdmin = $isAdmin").getOrElse(q"${true}")
} yield q"""
($editablePredicate) AND
($isTeamManagerOrAdminPredicate) AND
($adminPredicate) AND
$accessQuery
"""

def findAllWithFilters(isEditable: Option[Boolean],
isTeamManagerOrAdmin: Option[Boolean],
isAdmin: Option[Boolean],
requestingUser: User)(implicit ctx: DBAccessContext): Fox[List[User]] =
// Necessary since a tuple can only have 22 elements
implicit def GetResultUserCompactInfo(implicit e0: GetResult[String],
e1: GetResult[java.sql.Timestamp],
e2: GetResult[Boolean],
e3: GetResult[Option[String]]): GetResult[UserCompactInfo] = GetResult { prs =>
import prs._
// format: off
UserCompactInfo(<<[String],<<[String],<<[String],<<[String],<<[String],<<[String],<<[Boolean],<<[Boolean],
<<[Boolean],<<[Boolean],<<[String],<<[String],<<[String],<<[String], <<[String],<<[java.sql.Timestamp],<<[String],
<<[String],<<[String],<<[String],<<[java.sql.Timestamp],<<?[String],<<[Boolean],<<[Boolean],<<[Boolean]
)
// format: on
}
def findAllCompactWithFilters(
isEditable: Option[Boolean],
isTeamManagerOrAdmin: Option[Boolean],
isAdmin: Option[Boolean],
requestingUser: User)(implicit ctx: DBAccessContext): Fox[(List[User], List[UserCompactInfo])] =
for {

selectionPredicates <- buildSelectionPredicates(isEditable, isTeamManagerOrAdmin, isAdmin, requestingUser)
r <- run(q"select $columns from $existingCollectionName where $selectionPredicates".as[UsersRow])
parsed <- parseAll(r)
} yield parsed
selectionPredicates <- buildSelectionPredicates(isEditable,
isTeamManagerOrAdmin,
isAdmin,
requestingUser,
SqlToken.raw("u."))
isEditableAttribute = q"""
(u._id IN
(SELECT _id AS editableUsers FROM webknossos.users WHERE _organization IN
(SELECT _organization FROM webknossos.users WHERE _id = ${requestingUser._id} AND isadmin)
UNION
SELECT _user AS editableUsers FROM webknossos.user_team_roles WHERE _team in
(SELECT _team FROM webknossos.user_team_roles WHERE _user = ${requestingUser._id} AND isteammanager)
)
)
OR COUNT(t._id) = 0
AS iseditable
"""
r <- run(q"""
SELECT
u._id,
m._id,
m.email,
u.firstname,
u.lastname,
u.userConfiguration,
u.isadmin,
u.isorganizationowner,
u.isdatasetmanager,
u.isdeactivated,
ARRAY_REMOVE(ARRAY_AGG(t._id), null) AS team_ids,
ARRAY_REMOVE(ARRAY_AGG(t.name), null) AS team_names,
ARRAY_REMOVE(ARRAY_AGG(utr.isteammanager :: TEXT), null) AS team_managers,
ARRAY_REMOVE(ARRAY_AGG(ux.value), null) AS experience_values,
ARRAY_REMOVE(ARRAY_AGG(ux.domain), null) AS experience_domains,
u.lastactivity,
o._id,
o.name,
m.noveluserexperienceinfos,
m.selectedtheme,
u.created,
u.lasttasktypeid,
m.issuperuser,
m.isemailverified,
$isEditableAttribute
FROM webknossos.users AS u
INNER JOIN webknossos.organizations o on o._id = u._organization
INNER JOIN webknossos.multiusers m on u._multiuser = m._id
INNER JOIN webknossos.user_team_roles utr on utr._user = u._id
INNER JOIN webknossos.teams t on t._id = utr._team
LEFT JOIN webknossos.user_experiences ux on ux._user = u._id
WHERE $selectionPredicates
GROUP BY u._id, o._id, m._id, m.email, m.noveluserexperienceinfos, m.selectedtheme, m.issuperuser, m.isemailverified
""".as[UserCompactInfo])
users <- Fox.combined(r.toList.map(_.toUser))
compactInfos = r.toList
} yield (users, compactInfos)

def findAllByTeams(teamIds: List[ObjectId])(implicit ctx: DBAccessContext): Fox[List[User]] =
if (teamIds.isEmpty) Fox.successful(List())
Expand Down
46 changes: 46 additions & 0 deletions app/models/user/UserService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import utils.{ObjectId, WkConf}

import javax.inject.Inject
import models.organization.OrganizationDAO
import net.liftweb.common.Box.tryo
import net.liftweb.common.{Box, Full}
import security.{PasswordHasher, TokenDAO}
import utils.sql.SqlEscaping

import scala.concurrent.{ExecutionContext, Future}

Expand All @@ -44,6 +46,7 @@ class UserService @Inject()(conf: WkConf,
actorSystem: ActorSystem)(implicit ec: ExecutionContext)
extends FoxImplicits
with LazyLogging
with SqlEscaping
with IdentityService[User] {

private lazy val Mailer =
Expand Down Expand Up @@ -327,6 +330,7 @@ class UserService @Inject()(conf: WkConf,
} yield isTeamManager || user.isAdminOf(_organization)

def isEditableBy(possibleEditee: User, possibleEditor: User): Fox[Boolean] =
// Note that the same logic is implemented in User/findAllCompactWithFilters in SQL
for {
otherIsTeamManagerOrAdmin <- isTeamManagerOrAdminOf(possibleEditor, possibleEditee)
teamMemberships <- teamMembershipsFor(possibleEditee._id)
Expand Down Expand Up @@ -368,6 +372,48 @@ class UserService @Inject()(conf: WkConf,
}
}

def publicWritesCompact(user: User, userCompactInfo: UserCompactInfo): Fox[JsObject] =
for {
_ <- Fox.successful(())
teamsJson = parseArrayLiteral(userCompactInfo.teamIdsAsArrayLiteral).indices.map(
idx =>
Json.obj(
"id" -> parseArrayLiteral(userCompactInfo.teamIdsAsArrayLiteral)(idx),
"name" -> parseArrayLiteral(userCompactInfo.teamNamesAsArrayLiteral)(idx),
"isTeamManager" -> parseArrayLiteral(userCompactInfo.teamManagersAsArrayLiteral)(idx).toBoolean
))
experienceJson = Json.obj(
parseArrayLiteral(userCompactInfo.experienceValuesAsArrayLiteral).zipWithIndex
.filter(valueAndIndex => tryo(valueAndIndex._1.toInt).isDefined)
.map(valueAndIndex =>
(parseArrayLiteral(userCompactInfo.experienceDomainsAsArrayLiteral)(valueAndIndex._2),
Json.toJsFieldJsValueWrapper(valueAndIndex._1.toInt))): _*)
novelUserExperienceInfos <- Json.parse(userCompactInfo.novelUserExperienceInfos).validate[JsObject]
} yield {
Json.obj(
"id" -> user._id.toString,
"email" -> userCompactInfo.email,
"firstName" -> user.firstName,
"lastName" -> user.lastName,
"isAdmin" -> user.isAdmin,
"isOrganizationOwner" -> user.isOrganizationOwner,
"isDatasetManager" -> user.isDatasetManager,
"isActive" -> !user.isDeactivated,
"teams" -> teamsJson,
"experiences" -> experienceJson,
"lastActivity" -> user.lastActivity,
"isAnonymous" -> false,
"isEditable" -> userCompactInfo.isEditable,
"organization" -> userCompactInfo.organization_name,
"novelUserExperienceInfos" -> novelUserExperienceInfos,
"selectedTheme" -> userCompactInfo.selectedTheme,
"created" -> user.created,
"lastTaskTypeId" -> user.lastTaskTypeId.map(_.toString),
"isSuperUser" -> userCompactInfo.isSuperUser,
"isEmailVerified" -> userCompactInfo.isEmailVerified,
)
}

def compactWrites(user: User): Fox[JsObject] = {
implicit val ctx: DBAccessContext = GlobalAccessContext
for {
Expand Down