diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index d4edf75d612..cb88a714ced 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -13,6 +13,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released ### Added - Remote datasets can now also be streamed from Google Cloud Storage URIs (`gs://`). [#6775](https://github.com/scalableminds/webknossos/pull/6775) - Remote volume datasets in the neuroglancer precomputed format can now be viewed in WEBKNOSSOS. [#6716](https://github.com/scalableminds/webknossos/pull/6716) +- If an annotation that others are allowed to edit is opened, it will now be automatically locked. This prevents conflicts when multiple users try to edit it at the same time. [#6819](https://github.com/scalableminds/webknossos/pull/6819) - Added new mesh-related menu items to the context menu when a mesh is hovered in the 3d viewport. [#](https://github.com/scalableminds/webknossos/pull/6813) - Highlight 'organization owner' in Admin>User page. [#6832](https://github.com/scalableminds/webknossos/pull/6832) - Added functions to get and set segment colors to the frontend API (`api.data.{getSegmentColor,setSegmentColor}`). [#6853](https://github.com/scalableminds/webknossos/pull/6853) diff --git a/app/WebKnossosModule.scala b/app/WebKnossosModule.scala index 54daf59a2ba..6d567cf404e 100644 --- a/app/WebKnossosModule.scala +++ b/app/WebKnossosModule.scala @@ -1,7 +1,7 @@ import com.google.inject.AbstractModule import controllers.InitialDataService import models.analytics.AnalyticsSessionService -import models.annotation.AnnotationStore +import models.annotation.{AnnotationMutexService, AnnotationStore} import models.binary.DataSetService import models.job.{JobService, WorkerLivenessService} import models.storage.UsedStorageService @@ -26,6 +26,7 @@ class WebKnossosModule extends AbstractModule { bind(classOf[UserDataSetConfigurationDAO]).asEagerSingleton() bind(classOf[UserCache]).asEagerSingleton() bind(classOf[AnnotationStore]).asEagerSingleton() + bind(classOf[AnnotationMutexService]).asEagerSingleton() bind(classOf[DataSetService]).asEagerSingleton() bind(classOf[TimeSpanService]).asEagerSingleton() bind(classOf[TempFileService]).asEagerSingleton() diff --git a/app/controllers/AnnotationController.scala b/app/controllers/AnnotationController.scala index b9dc5459e45..49150928fd8 100755 --- a/app/controllers/AnnotationController.scala +++ b/app/controllers/AnnotationController.scala @@ -22,7 +22,8 @@ import models.team.{TeamDAO, TeamService} import models.user.time._ import models.user.{User, UserDAO, UserService} import oxalis.mail.{MailchimpClient, MailchimpTag} -import oxalis.security.{URLSharing, WkEnv} +import oxalis.security.{URLSharing, UserAwareRequestLogging, WkEnv} +import oxalis.telemetry.SlackNotificationService import play.api.i18n.{Messages, MessagesProvider} import play.api.libs.json.Json.WithDefaultValues import play.api.libs.json._ @@ -54,6 +55,7 @@ class AnnotationController @Inject()( dataSetDAO: DataSetDAO, dataSetService: DataSetService, annotationService: AnnotationService, + annotationMutexService: AnnotationMutexService, userService: UserService, teamService: TeamService, projectDAO: ProjectDAO, @@ -64,10 +66,12 @@ class AnnotationController @Inject()( provider: AnnotationInformationProvider, annotationRestrictionDefaults: AnnotationRestrictionDefaults, analyticsService: AnalyticsService, + slackNotificationService: SlackNotificationService, mailchimpClient: MailchimpClient, conf: WkConf, sil: Silhouette[WkEnv])(implicit ec: ExecutionContext, bodyParsers: PlayBodyParsers) extends Controller + with UserAwareRequestLogging with FoxImplicits { implicit val timeout: Timeout = Timeout(5 seconds) @@ -559,6 +563,7 @@ class AnnotationController @Inject()( sil.SecuredAction.async { implicit request => for { annotation <- provider.provideAnnotation(typ, id, request.identity) + _ <- bool2Fox(annotation.typ == AnnotationType.Explorational || annotation.typ == AnnotationType.Task) ?~> "annotation.othersMayEdit.onlyExplorationalOrTask" _ <- bool2Fox(annotation._user == request.identity._id) ?~> "notAllowed" ~> FORBIDDEN _ <- annotationDAO.updateOthersMayEdit(annotation._id, othersMayEdit) } yield Ok(Json.toJson(othersMayEdit)) @@ -601,4 +606,20 @@ class AnnotationController @Inject()( } } yield annotationLayer.copy(tracingId = newTracingId) + @ApiOperation(hidden = true, value = "") + def tryAcquiringAnnotationMutex(id: String): Action[AnyContent] = + sil.SecuredAction.async { implicit request => + logTime(slackNotificationService.noticeSlowRequest, durationThreshold = 1 second) { + for { + idValidated <- ObjectId.fromString(id) + annotation <- provider.provideAnnotation(id, request.identity) ~> NOT_FOUND + _ <- bool2Fox(annotation.othersMayEdit) ?~> "notAllowed" ~> FORBIDDEN + restrictions <- provider.restrictionsFor(AnnotationIdentifier(annotation.typ, idValidated)) ?~> "restrictions.notFound" ~> NOT_FOUND + _ <- restrictions.allowUpdate(request.identity) ?~> "notAllowed" ~> FORBIDDEN + mutexResult <- annotationMutexService.tryAcquiringAnnotationMutex(annotation._id, request.identity._id) ?~> "annotation.mutex.failed" + resultJson <- annotationMutexService.publicWrites(mutexResult) + } yield Ok(resultJson) + } + } + } diff --git a/app/models/annotation/AnnotationMutexService.scala b/app/models/annotation/AnnotationMutexService.scala new file mode 100644 index 00000000000..50c53c48b3f --- /dev/null +++ b/app/models/annotation/AnnotationMutexService.scala @@ -0,0 +1,119 @@ +package models.annotation + +import akka.actor.ActorSystem +import com.scalableminds.util.accesscontext.GlobalAccessContext +import com.scalableminds.util.time.Instant +import com.scalableminds.util.tools.Fox +import com.scalableminds.webknossos.datastore.helpers.IntervalScheduler +import com.scalableminds.webknossos.schema.Tables.AnnotationMutexesRow +import com.typesafe.scalalogging.LazyLogging +import models.user.{UserDAO, UserService} +import net.liftweb.common.Full +import play.api.inject.ApplicationLifecycle +import play.api.libs.json.{JsObject, Json} +import utils.{ObjectId, WkConf} +import utils.sql.{SimpleSQLDAO, SqlClient} + +import javax.inject.Inject +import scala.concurrent.ExecutionContext +import scala.concurrent.duration.{DurationInt, FiniteDuration} + +case class AnnotationMutex(annotationId: ObjectId, userId: ObjectId, expiry: Instant) + +case class MutexResult(canEdit: Boolean, blockedByUser: Option[ObjectId]) + +class AnnotationMutexService @Inject()(val lifecycle: ApplicationLifecycle, + val system: ActorSystem, + wkConf: WkConf, + userDAO: UserDAO, + userService: UserService, + annotationMutexDAO: AnnotationMutexDAO) + extends IntervalScheduler + with LazyLogging { + + override protected def tickerInterval: FiniteDuration = 1 hour + + override protected def tick(): Unit = { + logger.info("Cleaning up expired annotation mutexes...") + annotationMutexDAO.deleteExpired() + () + } + + private val defaultExpiryTime = wkConf.WebKnossos.Annotation.Mutex.expiryTime + + def tryAcquiringAnnotationMutex(annotationId: ObjectId, userId: ObjectId)( + implicit ec: ExecutionContext): Fox[MutexResult] = + this.synchronized { + for { + mutexBox <- annotationMutexDAO.findOne(annotationId).futureBox + result <- mutexBox match { + case Full(mutex) => + if (mutex.userId == userId) + refresh(mutex) + else + Fox.successful(MutexResult(canEdit = false, blockedByUser = Some(mutex.userId))) + case _ => + acquire(annotationId, userId) + } + } yield result + } + + private def acquire(annotationId: ObjectId, userId: ObjectId): Fox[MutexResult] = + for { + _ <- annotationMutexDAO.upsertOne(AnnotationMutex(annotationId, userId, Instant.in(defaultExpiryTime))) + } yield MutexResult(canEdit = true, None) + + private def refresh(mutex: AnnotationMutex): Fox[MutexResult] = + for { + _ <- annotationMutexDAO.upsertOne(mutex.copy(expiry = Instant.in(defaultExpiryTime))) + } yield MutexResult(canEdit = true, None) + + def publicWrites(mutexResult: MutexResult)(implicit ec: ExecutionContext): Fox[JsObject] = + for { + userOpt <- Fox.runOptional(mutexResult.blockedByUser)(user => userDAO.findOne(user)(GlobalAccessContext)) + userJsonOpt <- Fox.runOptional(userOpt)(user => userService.compactWrites(user)) + } yield + Json.obj( + "canEdit" -> mutexResult.canEdit, + "blockedByUser" -> userJsonOpt + ) + +} + +class AnnotationMutexDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext) + extends SimpleSQLDAO(sqlClient) { + + private def parse(r: AnnotationMutexesRow): AnnotationMutex = + AnnotationMutex( + ObjectId(r._Annotation), + ObjectId(r._User), + Instant.fromSql(r.expiry) + ) + + def findOne(annotationId: ObjectId): Fox[AnnotationMutex] = + for { + rows <- run(q"""SELECT _annotation, _user, expiry + FROM webknossos.annotation_mutexes + WHERE _annotation = $annotationId + AND expiry > NOW()""".as[AnnotationMutexesRow]) + first <- rows.headOption + parsed = parse(first) + } yield parsed + + def upsertOne(annotationMutex: AnnotationMutex): Fox[Unit] = + for { + _ <- run(q"""INSERT INTO webknossos.annotation_mutexes(_annotation, _user, expiry) + VALUES(${annotationMutex.annotationId}, ${annotationMutex.userId}, ${annotationMutex.expiry}) + ON CONFLICT (_annotation) + DO UPDATE SET + _user = ${annotationMutex.userId}, + expiry = ${annotationMutex.expiry} + """.asUpdate) + } yield () + + def deleteExpired(): Fox[Unit] = + for { + _ <- run(q"DELETE FROM webknossos.annotation_mutexes WHERE expiry < NOW()".asUpdate) + } yield () + +} diff --git a/app/models/annotation/AnnotationService.scala b/app/models/annotation/AnnotationService.scala index 9d62e957e8d..07e55242b68 100755 --- a/app/models/annotation/AnnotationService.scala +++ b/app/models/annotation/AnnotationService.scala @@ -1,6 +1,5 @@ package models.annotation -import java.io.{BufferedOutputStream, File, FileOutputStream} import akka.actor.ActorSystem import akka.stream.Materializer import com.scalableminds.util.accesscontext.{AuthorizedAccessContext, DBAccessContext, GlobalAccessContext} @@ -37,8 +36,6 @@ import com.scalableminds.webknossos.tracingstore.tracings.volume.{ } import com.typesafe.scalalogging.LazyLogging import controllers.AnnotationLayerParameters - -import javax.inject.Inject import models.annotation.AnnotationState._ import models.annotation.AnnotationType.AnnotationType import models.annotation.handler.SavedTracingInformationHandler @@ -57,6 +54,8 @@ import play.api.libs.iteratee.Enumerator import play.api.libs.json.{JsNull, JsObject, JsValue, Json} import utils.ObjectId +import java.io.{BufferedOutputStream, File, FileOutputStream} +import javax.inject.Inject import scala.concurrent.{ExecutionContext, Future} case class DownloadAnnotation(skeletonTracingIdOpt: Option[String], diff --git a/app/oxalis/telemetry/SlackNotificationService.scala b/app/oxalis/telemetry/SlackNotificationService.scala index b840dd29196..777b537f121 100644 --- a/app/oxalis/telemetry/SlackNotificationService.scala +++ b/app/oxalis/telemetry/SlackNotificationService.scala @@ -48,4 +48,10 @@ class SlackNotificationService @Inject()(rpc: RPC, config: WkConf) extends LazyL title = "Task creation with base", msg = s"$numberOfTasks tasks with BaseAnnotation for TaskTypes ${taskType.mkString(", ")} have been created" ) + + def noticeSlowRequest(msg: String): Unit = + slackClient.info( + title = "Slow request", + msg = msg + ) } diff --git a/app/utils/WkConf.scala b/app/utils/WkConf.scala index 31b307d9241..f3d19843ace 100644 --- a/app/utils/WkConf.scala +++ b/app/utils/WkConf.scala @@ -40,6 +40,12 @@ class WkConf @Inject()(configuration: Configuration) extends ConfigReader with L val maxOpenPerUser: Int = get[Int]("webKnossos.tasks.maxOpenPerUser") } + object Annotation { + object Mutex { + val expiryTime: FiniteDuration = get[FiniteDuration]("webKnossos.annotation.mutex.expiryTime") + } + } + object Cache { object User { val timeout: FiniteDuration = get[FiniteDuration]("webKnossos.cache.user.timeout") diff --git a/conf/application.conf b/conf/application.conf index 2d8dd2057b3..bb0b2c27bbb 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -59,6 +59,7 @@ webKnossos { cache { user.timeout = 3 minutes } + annotation.mutex.expiryTime = 2 minutes fetchUsedStorage { rescanInterval = 24 hours # do not scan organizations whose last scan is more recent than this tickerInterval = 10 minutes # scan some organizations at each tick diff --git a/conf/evolutions/100-annotation-mutexes.sql b/conf/evolutions/100-annotation-mutexes.sql new file mode 100644 index 00000000000..a1c33af68a7 --- /dev/null +++ b/conf/evolutions/100-annotation-mutexes.sql @@ -0,0 +1,18 @@ +START TRANSACTION; + +do $$ begin ASSERT (select schemaVersion from webknossos.releaseInformation) = 99, 'Previous schema version mismatch'; end; $$ LANGUAGE plpgsql; + +CREATE TABLE webknossos.annotation_mutexes( + _annotation CHAR(24) PRIMARY KEY, + _user CHAR(24) NOT NULL, + expiry TIMESTAMP NOT NULL +); + +ALTER TABLE webknossos.annotation_mutexes + ADD CONSTRAINT annotation_ref FOREIGN KEY(_annotation) REFERENCES webknossos.annotations(_id) ON DELETE CASCADE DEFERRABLE, + ADD CONSTRAINT user_ref FOREIGN KEY(_user) REFERENCES webknossos.users(_id) ON DELETE CASCADE DEFERRABLE; + +UPDATE webknossos.releaseInformation +SET schemaVersion = 100; + +COMMIT TRANSACTION; diff --git a/conf/evolutions/reversions/100-annotation-mutexes.sql b/conf/evolutions/reversions/100-annotation-mutexes.sql new file mode 100644 index 00000000000..14a32fc462f --- /dev/null +++ b/conf/evolutions/reversions/100-annotation-mutexes.sql @@ -0,0 +1,10 @@ +START TRANSACTION; + +do $$ begin ASSERT (select schemaVersion from webknossos.releaseInformation) = 100, 'Previous schema version mismatch'; end; $$ LANGUAGE plpgsql; + +DROP TABLE webknossos.annotation_mutexes; + +UPDATE webknossos.releaseInformation +SET schemaVersion = 99; + +COMMIT TRANSACTION; diff --git a/conf/webknossos.latest.routes b/conf/webknossos.latest.routes index a8647d0b148..60fc755ba2a 100644 --- a/conf/webknossos.latest.routes +++ b/conf/webknossos.latest.routes @@ -141,6 +141,7 @@ PATCH /annotations/:id/deleteAnnotationLayer DELETE /annotations/:id controllers.AnnotationController.cancelWithoutType(id: String) POST /annotations/:id/merge/:mergedTyp/:mergedId controllers.AnnotationController.mergeWithoutType(id: String, mergedTyp: String, mergedId: String) GET /annotations/:id/download controllers.AnnotationIOController.downloadWithoutType(id: String, skeletonVersion: Option[Long], volumeVersion: Option[Long], skipVolumeData: Option[Boolean]) +POST /annotations/:id/acquireMutex controllers.AnnotationController.tryAcquiringAnnotationMutex(id: String) GET /annotations/:typ/:id/info controllers.AnnotationController.info(typ: String, id: String, timestamp: Long) PATCH /annotations/:typ/:id/makeHybrid controllers.AnnotationController.makeHybrid(typ: String, id: String, fallbackLayerName: Option[String]) diff --git a/docs/sharing.md b/docs/sharing.md index 9a2baf5e6f3..2b69d271d20 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -106,6 +106,7 @@ To change the visibility of an annotation, follow these steps: Additionally, you can control whether other users, who can see your annotation, may also edit your annotation. Use this setting to enable collaborative work within your annotation. However, note that you should coordinate the collaboration because parallel changes to an annotation are not supported. +To avoid possible conflicts in such cases the annotation will be locked to a single user at a time. In case the annotation is locked by someone else WEBKNOSSOS will tell you the name of the person currently editing so you can coordinate with this person. ### Link Sharing Annotations can be shared via a link. People, who obtain the link, must have access to the annotation according to the permissions above to view the annotation. diff --git a/frontend/javascripts/admin/admin_rest_api.ts b/frontend/javascripts/admin/admin_rest_api.ts index c42e0253fcf..b59f88baeef 100644 --- a/frontend/javascripts/admin/admin_rest_api.ts +++ b/frontend/javascripts/admin/admin_rest_api.ts @@ -65,6 +65,7 @@ import type { VoxelyticsWorkflowListing, APIPricingPlanStatus, VoxelyticsLogLine, + APIUserCompact, APIDatasetCompact, } from "types/api_flow_types"; import { APIAnnotationTypeEnum } from "types/api_flow_types"; @@ -860,6 +861,18 @@ export async function getTracingsForAnnotation( return fullAnnotationLayers; } +export async function acquireAnnotationMutex( + annotationId: string, +): Promise<{ canEdit: boolean; blockedByUser: APIUserCompact | undefined | null }> { + const { canEdit, blockedByUser } = await Request.receiveJSON( + `/api/annotations/${annotationId}/acquireMutex`, + { + method: "POST", + }, + ); + return { canEdit, blockedByUser }; +} + function extractVersion( versions: Versions, tracingId: string, diff --git a/frontend/javascripts/messages.tsx b/frontend/javascripts/messages.tsx index a6e3ef9ce9d..e58f680b893 100644 --- a/frontend/javascripts/messages.tsx +++ b/frontend/javascripts/messages.tsx @@ -259,6 +259,13 @@ instead. Only enable this option if you understand its effect. All layers will n "annotation.undoFinish.confirm": "Are you sure you want to reopen your old task?", "annotation.undoFinish.content": "If you reopen your old annotation, the current annotation will not be finished or cancelled. Instead, it will remain open and you can find it in the dashboard to continue annotating.", + "annotation.acquiringMutexFailed": _.template( + "This annotation is currently being edited by <%- userName %>. To avoid conflicts, you can only view it. If you want to edit it, please ask <%- userName %> to finish their work first.", + ), + "annotation.acquiringMutexFailed.noUser": + "This annotation is currently being edited by someone else. To avoid conflicts, you can only view it at the moment.", + "annotation.acquiringMutexSucceeded": + "This annotation is not being edited anymore and available for editing. Reload the page to see its newest version and to edit it.", "task.bulk_create_invalid": "Can not parse task specification. It includes at least one invalid task.", "task.recommended_configuration": "The author of this task suggests to use these settings:", diff --git a/frontend/javascripts/navbar.tsx b/frontend/javascripts/navbar.tsx index f166d8ba337..5b0c84a204c 100644 --- a/frontend/javascripts/navbar.tsx +++ b/frontend/javascripts/navbar.tsx @@ -1,4 +1,14 @@ -import { Avatar, Button, Badge, Tooltip, Layout, Menu, Popover, type SubMenuProps } from "antd"; +import { + Avatar, + Button, + Badge, + Tooltip, + Layout, + Menu, + Popover, + type SubMenuProps, + Tag, +} from "antd"; import { SwapOutlined, TeamOutlined, @@ -15,7 +25,7 @@ import classnames from "classnames"; import { connect } from "react-redux"; import React, { useState, useEffect } from "react"; import Toast from "libs/toast"; -import type { APIOrganization, APIUser, APIUserTheme } from "types/api_flow_types"; +import type { APIOrganization, APIUser, APIUserCompact, APIUserTheme } from "types/api_flow_types"; import { PortalTarget } from "oxalis/view/layouting/portal_utils"; import { getBuildInfo, @@ -38,6 +48,7 @@ import features from "features"; import { setThemeAction } from "oxalis/model/actions/ui_actions"; import { HelpModal } from "oxalis/view/help_modal"; import { PricingPlanEnum } from "admin/organization/pricing_plan_utils"; +import messages from "messages"; import { PricingEnforcedSpan } from "components/pricing_enforcers"; import { ItemType, MenuItemType, SubMenuType } from "antd/lib/menu/hooks/useItems"; import { MenuClickEventHandler } from "rc-menu/lib/interface"; @@ -53,6 +64,9 @@ type StateProps = { activeUser: APIUser | null | undefined; isInAnnotationView: boolean; hasOrganizations: boolean; + othersMayEdit: boolean; + allowUpdate: boolean; + blockedByUser: APIUserCompact | null | undefined; }; type Props = OwnProps & StateProps; export const navbarHeight = 48; @@ -631,7 +645,54 @@ async function getAndTrackVersion(dontTrack: boolean = false) { return version; } -function Navbar({ activeUser, isAuthenticated, isInAnnotationView, hasOrganizations }: Props) { +function AnnotationLockedByUserTag({ + blockedByUser, + activeUser, +}: { + blockedByUser: APIUserCompact | null | undefined; + activeUser: APIUser; +}) { + let content; + if (blockedByUser == null) { + content = ( + + Locked by unknown user. + + ); + } else if (blockedByUser.id === activeUser.id) { + content = ( + + Locked by you. Reload to edit. + + ); + } else { + const blockingUserName = `${blockedByUser.firstName} ${blockedByUser.lastName}`; + content = ( + + Locked by {blockingUserName} + + ); + } + return ( + + {content} + + ); +} + +function Navbar({ + activeUser, + isAuthenticated, + isInAnnotationView, + hasOrganizations, + othersMayEdit, + blockedByUser, + allowUpdate, +}: Props) { const history = useHistory(); const handleLogout = async (event: React.SyntheticEvent) => { @@ -706,6 +767,11 @@ function Navbar({ activeUser, isAuthenticated, isInAnnotationView, hasOrganizati menuItems.push(getTimeTrackingMenu(collapseAllNavItems)); } + if (othersMayEdit && !allowUpdate) { + trailingNavItems.push( + , + ); + } trailingNavItems.push(); trailingNavItems.push( ({ activeUser: state.activeUser, isInAnnotationView: state.uiInformation.isInAnnotationView, hasOrganizations: state.uiInformation.hasOrganizations, + othersMayEdit: state.tracing.othersMayEdit, + blockedByUser: state.tracing.blockedByUser, + allowUpdate: state.tracing.restrictions.allowUpdate, }); const connector = connect(mapStateToProps); diff --git a/frontend/javascripts/oxalis/default_state.ts b/frontend/javascripts/oxalis/default_state.ts index 5e5d7389f4a..5241de52285 100644 --- a/frontend/javascripts/oxalis/default_state.ts +++ b/frontend/javascripts/oxalis/default_state.ts @@ -159,6 +159,7 @@ const defaultState: OxalisState = { owner: null, contributors: [], othersMayEdit: false, + blockedByUser: null, annotationLayers: [], }, save: { diff --git a/frontend/javascripts/oxalis/model/actions/annotation_actions.ts b/frontend/javascripts/oxalis/model/actions/annotation_actions.ts index 409eddd9778..7eced8a171e 100644 --- a/frontend/javascripts/oxalis/model/actions/annotation_actions.ts +++ b/frontend/javascripts/oxalis/model/actions/annotation_actions.ts @@ -4,6 +4,7 @@ import type { APIDataLayer, APIDataset, APIMeshFile, + APIUserCompact, EditableLayerProperties, LocalMeshMetaData, MeshMetaData, @@ -26,6 +27,7 @@ type SetAnnotationVisibilityAction = ReturnType; type SetAnnotationDescriptionAction = ReturnType; type SetAnnotationAllowUpdateAction = ReturnType; +type SetBlockedByUserAction = ReturnType; type SetUserBoundingBoxesAction = ReturnType; type FinishedResizingUserBoundingBoxAction = ReturnType< typeof finishedResizingUserBoundingBoxAction @@ -63,6 +65,7 @@ export type AnnotationActionTypes = | EditAnnotationLayerAction | SetAnnotationDescriptionAction | SetAnnotationAllowUpdateAction + | SetBlockedByUserAction | UpdateRemoteMeshMetaDataAction | SetUserBoundingBoxesAction | ChangeUserBoundingBoxAction @@ -144,6 +147,12 @@ export const setAnnotationAllowUpdateAction = (allowUpdate: boolean) => allowUpdate, } as const); +export const setBlockedByUserAction = (blockedByUser: APIUserCompact | null | undefined) => + ({ + type: "SET_BLOCKED_BY_USER", + blockedByUser, + } as const); + // Strictly speaking this is no annotation action but a tracing action, as the boundingBox is saved with // the tracing, hence no ANNOTATION in the action type. export const setUserBoundingBoxesAction = (userBoundingBoxes: Array) => diff --git a/frontend/javascripts/oxalis/model/reducers/annotation_reducer.ts b/frontend/javascripts/oxalis/model/reducers/annotation_reducer.ts index 2fa5e926d30..8ff9ec5a41b 100644 --- a/frontend/javascripts/oxalis/model/reducers/annotation_reducer.ts +++ b/frontend/javascripts/oxalis/model/reducers/annotation_reducer.ts @@ -95,6 +95,13 @@ function AnnotationReducer(state: OxalisState, action: Action): OxalisState { }); } + case "SET_BLOCKED_BY_USER": { + const { blockedByUser } = action; + return updateKey(state, "tracing", { + blockedByUser, + }); + } + case "SET_USER_BOUNDING_BOXES": { return updateUserBoundingBoxes(state, action.userBoundingBoxes); } diff --git a/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts b/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts index 4454c9a977a..027e327aa9f 100644 --- a/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts +++ b/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts @@ -121,6 +121,7 @@ export function convertServerAnnotationToFrontendAnnotation(annotation: APIAnnot contributors, othersMayEdit, annotationLayers, + blockedByUser: null, }; } export function getNextTool(state: OxalisState): AnnotationTool | null { diff --git a/frontend/javascripts/oxalis/model/sagas/annotation_saga.ts b/frontend/javascripts/oxalis/model/sagas/annotation_saga.tsx similarity index 52% rename from frontend/javascripts/oxalis/model/sagas/annotation_saga.ts rename to frontend/javascripts/oxalis/model/sagas/annotation_saga.tsx index ef2755f6eec..9e74c00edd2 100644 --- a/frontend/javascripts/oxalis/model/sagas/annotation_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/annotation_saga.tsx @@ -1,14 +1,36 @@ +import React from "react"; import _ from "lodash"; import type { Action } from "oxalis/model/actions/actions"; -import type { EditAnnotationLayerAction } from "oxalis/model/actions/annotation_actions"; +import { + EditAnnotationLayerAction, + setAnnotationAllowUpdateAction, + setBlockedByUserAction, + type SetOthersMayEditForAnnotationAction, +} from "oxalis/model/actions/annotation_actions"; import type { EditableAnnotation } from "admin/admin_rest_api"; -import { editAnnotation, updateAnnotationLayer } from "admin/admin_rest_api"; +import { + editAnnotation, + updateAnnotationLayer, + acquireAnnotationMutex, +} from "admin/admin_rest_api"; import { SETTINGS_MAX_RETRY_COUNT, SETTINGS_RETRY_DELAY, } from "oxalis/model/sagas/save_saga_constants"; import type { Saga } from "oxalis/model/sagas/effect-generators"; -import { takeLatest, select, take, retry, delay } from "typed-redux-saga"; +import { + takeLatest, + select, + take, + retry, + delay, + call, + put, + fork, + takeEvery, + cancel, + cancelled, +} from "typed-redux-saga"; import { getMappingInfo } from "oxalis/model/accessors/dataset_accessor"; import { getRequestLogZoomStep } from "oxalis/model/accessors/flycam_accessor"; import { Model } from "oxalis/singletons"; @@ -16,6 +38,8 @@ import Store from "oxalis/store"; import Toast from "libs/toast"; import constants, { MappingStatusEnum } from "oxalis/constants"; import messages from "messages"; +import { APIUserCompact } from "types/api_flow_types"; +import { Button } from "antd"; /* Note that this must stay in sync with the back-end constant compare https://github.com/scalableminds/webknossos/issues/5223 */ @@ -159,4 +183,113 @@ export function* watchAnnotationAsync(): Saga { ); yield* takeLatest("EDIT_ANNOTATION_LAYER", pushAnnotationLayerUpdateAsync); } -export default [warnAboutSegmentationZoom, watchAnnotationAsync]; + +export function* acquireAnnotationMutexMaybe(): Saga { + yield* take("WK_READY"); + const allowUpdate = yield* select((state) => state.tracing.restrictions.allowUpdate); + const annotationId = yield* select((storeState) => storeState.tracing.annotationId); + if (!allowUpdate) { + return; + } + const othersMayEdit = yield* select((state) => state.tracing.othersMayEdit); + const activeUser = yield* select((state) => state.activeUser); + const acquireMutexInterval = 1000 * 60; + const RETRY_COUNT = 12; + const MUTEX_NOT_ACQUIRED_KEY = "MutexCouldNotBeAcquired"; + const MUTEX_ACQUIRED_KEY = "AnnotationMutexAcquired"; + let isInitialRequest = true; + let doesHaveMutexFromBeginning = false; + let doesHaveMutex = false; + let shallTryAcquireMutex = othersMayEdit; + + function onMutexStateChanged(canEdit: boolean, blockedByUser: APIUserCompact | null | undefined) { + if (canEdit) { + Toast.close("MutexCouldNotBeAcquired"); + if (!isInitialRequest) { + const message = ( + + {messages["annotation.acquiringMutexSucceeded"]}{" "} + + + ); + Toast.success(message, { sticky: true, key: MUTEX_ACQUIRED_KEY }); + } + } else { + Toast.close(MUTEX_ACQUIRED_KEY); + const message = + blockedByUser != null + ? messages["annotation.acquiringMutexFailed"]({ + userName: `${blockedByUser.firstName} ${blockedByUser.lastName}`, + }) + : messages["annotation.acquiringMutexFailed.noUser"]; + Toast.warning(message, { sticky: true, key: MUTEX_NOT_ACQUIRED_KEY }); + } + } + + function* tryAcquireMutexContinuously(): Saga { + while (shallTryAcquireMutex) { + if (isInitialRequest) { + yield* put(setAnnotationAllowUpdateAction(false)); + } + try { + const { canEdit, blockedByUser } = yield* retry( + RETRY_COUNT, + acquireMutexInterval / RETRY_COUNT, + acquireAnnotationMutex, + annotationId, + ); + if (isInitialRequest && canEdit) { + doesHaveMutexFromBeginning = true; + // Only set allow update to true in case the first try to get the mutex succeeded. + yield* put(setAnnotationAllowUpdateAction(true)); + } + if (!canEdit || !doesHaveMutexFromBeginning) { + // If the mutex could not be acquired anymore or the user does not have it from the beginning, set allow update to false. + doesHaveMutexFromBeginning = false; + yield* put(setAnnotationAllowUpdateAction(false)); + } + if (canEdit) { + yield* put(setBlockedByUserAction(activeUser)); + } else { + yield* put(setBlockedByUserAction(blockedByUser)); + } + if (canEdit !== doesHaveMutex || isInitialRequest) { + doesHaveMutex = canEdit; + onMutexStateChanged(canEdit, blockedByUser); + } + } catch (error) { + const wasCanceled = yield* cancelled(); + if (!wasCanceled) { + console.error("Error while trying to acquire mutex.", error); + yield* put(setBlockedByUserAction(undefined)); + yield* put(setAnnotationAllowUpdateAction(false)); + doesHaveMutexFromBeginning = false; + if (doesHaveMutex || isInitialRequest) { + onMutexStateChanged(false, null); + doesHaveMutex = false; + } + } + } + isInitialRequest = false; + yield* call(delay, acquireMutexInterval); + } + } + let runningTryAcquireMutexContinuouslySaga = yield* fork(tryAcquireMutexContinuously); + function* reactToOthersMayEditChanges({ + othersMayEdit, + }: SetOthersMayEditForAnnotationAction): Saga { + shallTryAcquireMutex = othersMayEdit; + if (shallTryAcquireMutex) { + if (runningTryAcquireMutexContinuouslySaga != null) { + yield* cancel(runningTryAcquireMutexContinuouslySaga); + } + isInitialRequest = true; + runningTryAcquireMutexContinuouslySaga = yield* fork(tryAcquireMutexContinuously); + } else { + // othersMayEdit was turned off. The user editing it should be able to edit the annotation. + yield* put(setAnnotationAllowUpdateAction(true)); + } + } + yield* takeEvery("SET_OTHERS_MAY_EDIT_FOR_ANNOTATION", reactToOthersMayEditChanges); +} +export default [warnAboutSegmentationZoom, watchAnnotationAsync, acquireAnnotationMutexMaybe]; diff --git a/frontend/javascripts/oxalis/model/sagas/save_saga.ts b/frontend/javascripts/oxalis/model/sagas/save_saga.ts index 2a9c5bc41c4..6bb2768be5b 100644 --- a/frontend/javascripts/oxalis/model/sagas/save_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/save_saga.ts @@ -1129,6 +1129,12 @@ const VERSION_POLL_INTERVAL_SINGLE_EDITOR = 30 * 1000; function* watchForSaveConflicts() { function* checkForNewVersion() { const allowSave = yield* select((state) => state.tracing.restrictions.allowSave); + const allowUpdate = yield* select((state) => state.tracing.restrictions.allowUpdate); + const othersMayEdit = yield* select((state) => state.tracing.othersMayEdit); + if (!allowUpdate && othersMayEdit) { + // The user does not have the annotation's mutex and therefore no repeated version check is needed. + return; + } const maybeSkeletonTracing = yield* select((state) => state.tracing.skeleton); const volumeTracings = yield* select((state) => state.tracing.volumes); diff --git a/frontend/javascripts/oxalis/store.ts b/frontend/javascripts/oxalis/store.ts index 6220d3cdd73..211f56d6702 100644 --- a/frontend/javascripts/oxalis/store.ts +++ b/frontend/javascripts/oxalis/store.ts @@ -24,6 +24,7 @@ import type { APIMeshFile, ServerEditableMapping, APIOrganization, + APIUserCompact, } from "types/api_flow_types"; import type { Action } from "oxalis/model/actions/actions"; import type { @@ -186,6 +187,7 @@ export type Annotation = { readonly owner: APIUserBase | null | undefined; readonly contributors: APIUserBase[]; readonly othersMayEdit: boolean; + readonly blockedByUser: APIUserCompact | null | undefined; }; type TracingBase = { readonly createdTimestamp: number; diff --git a/frontend/javascripts/oxalis/view/action-bar/tracing_actions_view.tsx b/frontend/javascripts/oxalis/view/action-bar/tracing_actions_view.tsx index f534558751e..f777314a068 100644 --- a/frontend/javascripts/oxalis/view/action-bar/tracing_actions_view.tsx +++ b/frontend/javascripts/oxalis/view/action-bar/tracing_actions_view.tsx @@ -25,7 +25,7 @@ import { } from "@ant-design/icons"; import { connect } from "react-redux"; import * as React from "react"; -import type { APIAnnotationType, APIUser } from "types/api_flow_types"; +import type { APIAnnotationType, APIUser, APIUserBase } from "types/api_flow_types"; import { APIAnnotationTypeEnum, TracingTypeEnum } from "types/api_flow_types"; import { AsyncButton, AsyncButtonProps } from "components/async_clickables"; import type { LayoutKeys } from "oxalis/view/layouting/default_layout_configs"; @@ -88,6 +88,8 @@ type StateProps = { isDownloadModalOpen: boolean; isShareModalOpen: boolean; busyBlockingInfo: BusyBlockingInfo; + annotationOwner: APIUserBase | null | undefined; + othersMayEdit: boolean; }; type Props = OwnProps & StateProps; type State = { @@ -456,7 +458,16 @@ class TracingActionsView extends React.PureComponent { activeUser, layoutMenu, busyBlockingInfo, + othersMayEdit, + annotationOwner, } = this.props; + const copyAnnotationText = + !restrictions.allowUpdate && + activeUser != null && + annotationOwner?.id === activeUser.id && + othersMayEdit + ? "Duplicate" + : "Copy To My Account"; const archiveButtonText = task ? "Finish and go to Dashboard" : "Archive"; const saveButton = restrictions.allowUpdate ? [ @@ -503,7 +514,7 @@ class TracingActionsView extends React.PureComponent { key="copy-sandbox-button" icon={} onClick={this.handleCopySandboxToAccount} - title="Copy To My Account" + title={copyAnnotationText} > Copy To My Account , @@ -527,9 +538,9 @@ class TracingActionsView extends React.PureComponent { key="copy-button" icon={} onClick={this.handleCopyToAccount} - title="Copy To My Account" + title={copyAnnotationText} > - Copy To My Account + {copyAnnotationText} , ]; const finishAndNextTaskButton = @@ -701,12 +712,14 @@ function mapStateToProps(state: OxalisState): StateProps { annotationType: state.tracing.annotationType, annotationId: state.tracing.annotationId, restrictions: state.tracing.restrictions, + annotationOwner: state.tracing.owner, task: state.task, activeUser: state.activeUser, hasTracing: state.tracing.skeleton != null || state.tracing.volumes.length > 0, isDownloadModalOpen: state.uiInformation.showDownloadModal, isShareModalOpen: state.uiInformation.showShareModal, busyBlockingInfo: state.uiInformation.busyBlockingInfo, + othersMayEdit: state.tracing.othersMayEdit, }; } diff --git a/frontend/javascripts/test/sagas/annotation_saga.spec.ts b/frontend/javascripts/test/sagas/annotation_saga.spec.ts new file mode 100644 index 00000000000..0c8a1ad7d60 --- /dev/null +++ b/frontend/javascripts/test/sagas/annotation_saga.spec.ts @@ -0,0 +1,308 @@ +import test, { ExecutionContext } from "ava"; +import _ from "lodash"; +import mockRequire from "mock-require"; +import { OxalisState } from "oxalis/store"; +import { createMockTask } from "@redux-saga/testing-utils"; +import { take, put } from "redux-saga/effects"; +import dummyUser from "test/fixtures/dummy_user"; +import defaultState from "oxalis/default_state"; +import { expectValueDeepEqual } from "test/helpers/sagaHelpers"; +import { + setAnnotationAllowUpdateAction, + setBlockedByUserAction, + setOthersMayEditForAnnotationAction, +} from "oxalis/model/actions/annotation_actions"; + +const createInitialState = (othersMayEdit: boolean, allowUpdate: boolean = true): OxalisState => ({ + ...defaultState, + activeUser: dummyUser, + tracing: { + ...defaultState.tracing, + restrictions: { + ...defaultState.tracing.restrictions, + allowUpdate, + }, + volumes: [], + othersMayEdit, + annotationId: "1234", + }, +}); + +const blockingUser = { firstName: "Sample", lastName: "User", id: "1111" }; + +mockRequire("libs/toast", { + warning: _.noop, + close: _.noop, + success: _.noop, +}); +const { wkReadyAction } = mockRequire.reRequire("oxalis/model/actions/actions"); +const { acquireAnnotationMutexMaybe } = mockRequire.reRequire("oxalis/model/sagas/annotation_saga"); + +test.serial( + "An annotation with allowUpdate = false should not try to acquire the annotation mutex.", + (t) => { + const storeState = createInitialState(false, false); + const saga = acquireAnnotationMutexMaybe(); + saga.next(); + saga.next(wkReadyAction()); + saga.next(storeState.tracing.restrictions.allowUpdate); + saga.next(storeState.tracing.annotationId); + t.deepEqual(saga.next().done, true, "The saga should terminate."); + }, +); + +function prepareTryAcquireMutexSaga(t: ExecutionContext, othersMayEdit: boolean) { + const tryAcquireMutexContinuouslyMocked = createMockTask(); + const listenForOthersMayEditMocked = createMockTask(); + const storeState = createInitialState(othersMayEdit); + const saga = acquireAnnotationMutexMaybe(); + expectValueDeepEqual(t, saga.next(), take("WK_READY")); + t.deepEqual( + saga.next(wkReadyAction()).value.type, + "SELECT", + "The saga should select the allowUpdate next.", + ); + t.deepEqual( + saga.next(storeState.tracing.restrictions.allowUpdate).value.type, + "SELECT", + "The saga should select the annotationId next.", + ); + t.deepEqual( + saga.next(storeState.tracing.annotationId).value.type, + "SELECT", + "The saga should select the othersMayEdit next.", + ); + t.deepEqual( + saga.next(storeState.tracing.othersMayEdit).value.type, + "SELECT", + "The saga should select the activeUser next.", + ); + let sagaValue = saga.next(storeState.activeUser).value; + const tryAcquireMutexContinuously = sagaValue.payload.fn(); + t.deepEqual(sagaValue.type, "FORK", "The saga should fork tryAcquireMutexContinuously."); + sagaValue = saga.next(tryAcquireMutexContinuouslyMocked).value; + const listenForOthersMayEdit = sagaValue.payload.args[1]; + t.deepEqual(sagaValue.type, "FORK", "The saga should fork listenForOthersMayEdit."); + t.deepEqual(saga.next(listenForOthersMayEditMocked).done, true, "The saga should terminate."); + return { tryAcquireMutexContinuously, listenForOthersMayEdit }; +} + +function testReacquiringMutex(t: ExecutionContext, tryAcquireMutexContinuously: any) { + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setAnnotationAllowUpdateAction(false)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next({ + canEdit: true, + blockedByUser: null, + }), + put(setAnnotationAllowUpdateAction(true)), + ); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setBlockedByUserAction(dummyUser)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); // delay is called + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next({ + canEdit: true, + blockedByUser: null, + }), + put(setBlockedByUserAction(dummyUser)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); // delay is called + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next({ + canEdit: false, + blockedByUser: blockingUser, + }), + put(setAnnotationAllowUpdateAction(false)), + ); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setBlockedByUserAction(blockingUser)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); // delay is called +} + +function testFailingReacquiringMutex(t: ExecutionContext, tryAcquireMutexContinuously: any) { + // This method test whether the saga handles the following server responses correctly: + // 1. canEdit: false, 2. canEdit: false, 3. canEdit: true, 4. canEdit: false + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setAnnotationAllowUpdateAction(false)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next({ + canEdit: false, + blockedByUser: blockingUser, + }), + put(setAnnotationAllowUpdateAction(false)), + ); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setBlockedByUserAction(blockingUser)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); // delay is called + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next({ + canEdit: false, + blockedByUser: blockingUser, + }), + put(setAnnotationAllowUpdateAction(false)), + ); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setBlockedByUserAction(blockingUser)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); // delay is called + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next({ + canEdit: true, + blockedByUser: null, + }), + put(setAnnotationAllowUpdateAction(false)), + ); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setBlockedByUserAction(dummyUser)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); // delay is called + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next({ + canEdit: false, + blockedByUser: blockingUser, + }), + put(setAnnotationAllowUpdateAction(false)), + ); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setBlockedByUserAction(blockingUser)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); // delay is called +} + +test.serial( + "An annotation with othersMayEdit = false should not try to acquire the annotation mutex.", + (t) => { + prepareTryAcquireMutexSaga(t, false); + }, +); + +test.serial( + "An annotation with othersMayEdit = true should try to acquire the annotation mutex.", + (t) => { + prepareTryAcquireMutexSaga(t, true); + }, +); + +test.serial( + "An annotation with othersMayEdit = true should retry to acquire the annotation mutex.", + (t) => { + const { tryAcquireMutexContinuously } = prepareTryAcquireMutexSaga(t, true); + testReacquiringMutex(t, tryAcquireMutexContinuously); + }, +); + +test.serial( + "An annotation with othersMayEdit = true should not be updatable when the initial mutex acquiring failed.", + (t) => { + const { tryAcquireMutexContinuously } = prepareTryAcquireMutexSaga(t, true); + testFailingReacquiringMutex(t, tryAcquireMutexContinuously); + }, +); + +test.serial( + "It should not try to acquire the annotation mutex if othersMayEdit is set to false.", + (t) => { + const { tryAcquireMutexContinuously, listenForOthersMayEdit } = prepareTryAcquireMutexSaga( + t, + true, + ); + + // testing the tryAcquireMutexContinuously saga + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setAnnotationAllowUpdateAction(false)), + ); + t.deepEqual(tryAcquireMutexContinuously.next().value.type, "CALL"); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next({ + canEdit: true, + blockedByUser: null, + }), + put(setAnnotationAllowUpdateAction(true)), + ); + expectValueDeepEqual( + t, + tryAcquireMutexContinuously.next(), + put(setBlockedByUserAction(dummyUser)), + ); + tryAcquireMutexContinuously.next(); // delay is called + const listenForOthersMayEditSaga = listenForOthersMayEdit( + setOthersMayEditForAnnotationAction(false), + ); //othersMayEdit is set to false. + expectValueDeepEqual( + t, + listenForOthersMayEditSaga.next(), + put(setAnnotationAllowUpdateAction(true)), + ); + t.deepEqual( + listenForOthersMayEditSaga.next().done, + true, + "The listenForOthersMayEdit saga should terminate.", + ); + t.deepEqual( + tryAcquireMutexContinuously.next().done, + true, + "The tryAcquireMutexContinuously saga should terminate as otherMayEdit is set to false.", + ); + }, +); + +test.serial( + "An annotation with othersMayEdit getting set to true should try to acquire the annotation mutex.", + (t) => { + const tryAcquireMutexContinuouslyMocked = createMockTask(); + const { tryAcquireMutexContinuously: cancelledTryAcquireMutexSaga, listenForOthersMayEdit } = + prepareTryAcquireMutexSaga(t, false); + const listenForOthersMayEditSaga = listenForOthersMayEdit( + setOthersMayEditForAnnotationAction(true), + ); + t.deepEqual(listenForOthersMayEditSaga.next(cancelledTryAcquireMutexSaga).value.type, "CANCEL"); + const sagaValue = listenForOthersMayEditSaga.next(cancelledTryAcquireMutexSaga).value; + const tryAcquireMutexContinuously = sagaValue.payload.fn(); + t.deepEqual(sagaValue.type, "FORK"); + t.deepEqual( + listenForOthersMayEditSaga.next(tryAcquireMutexContinuouslyMocked).done, + true, + "The listenForOthersMayEdit saga should terminate.", + ); + testReacquiringMutex(t, tryAcquireMutexContinuously); + }, +); diff --git a/package.json b/package.json index ca1cdee85b0..15bed00d741 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "url": "git://github.com/scalableminds/webknossos.git" }, "devDependencies": { + "@redux-saga/testing-utils": "^1.1.5", "@shaderfrog/glsl-parser": "^0.2.2", "@types/color-hash": "^1.0.2", "@types/cwise": "^1.0.4", diff --git a/tools/postgres/schema.sql b/tools/postgres/schema.sql index e5e510fc19c..ed2a14bbcb0 100644 --- a/tools/postgres/schema.sql +++ b/tools/postgres/schema.sql @@ -19,7 +19,7 @@ START TRANSACTION; CREATE TABLE webknossos.releaseInformation ( schemaVersion BIGINT NOT NULL ); -INSERT INTO webknossos.releaseInformation(schemaVersion) values(99); +INSERT INTO webknossos.releaseInformation(schemaVersion) values(100); COMMIT TRANSACTION; @@ -73,6 +73,12 @@ CREATE TABLE webknossos.annotation_contributors( PRIMARY KEY (_annotation, _user) ); +CREATE TABLE webknossos.annotation_mutexes( + _annotation CHAR(24) PRIMARY KEY, + _user CHAR(24) NOT NULL, + expiry TIMESTAMP NOT NULL +); + CREATE TABLE webknossos.meshes( _id CHAR(24) PRIMARY KEY, _annotation CHAR(24) NOT NULL, @@ -656,6 +662,9 @@ ALTER TABLE webknossos.annotation_sharedTeams ALTER TABLE webknossos.annotation_contributors ADD CONSTRAINT annotation_ref FOREIGN KEY(_annotation) REFERENCES webknossos.annotations(_id) ON DELETE CASCADE DEFERRABLE, ADD CONSTRAINT user_ref FOREIGN KEY(_user) REFERENCES webknossos.users(_id) ON DELETE CASCADE DEFERRABLE; +ALTER TABLE webknossos.annotation_mutexes + ADD CONSTRAINT annotation_ref FOREIGN KEY(_annotation) REFERENCES webknossos.annotations(_id) ON DELETE CASCADE DEFERRABLE, + ADD CONSTRAINT user_ref FOREIGN KEY(_user) REFERENCES webknossos.users(_id) ON DELETE CASCADE DEFERRABLE; ALTER TABLE webknossos.meshes ADD CONSTRAINT annotation_ref FOREIGN KEY(_annotation) REFERENCES webknossos.annotations(_id) DEFERRABLE; ALTER TABLE webknossos.dataSets diff --git a/util/src/main/scala/com/scalableminds/util/requestlogging/RequestLogging.scala b/util/src/main/scala/com/scalableminds/util/requestlogging/RequestLogging.scala index 73e4f002f34..50e4a5155bc 100644 --- a/util/src/main/scala/com/scalableminds/util/requestlogging/RequestLogging.scala +++ b/util/src/main/scala/com/scalableminds/util/requestlogging/RequestLogging.scala @@ -1,14 +1,13 @@ package com.scalableminds.util.requestlogging -import java.io.{PrintWriter, StringWriter} - +import com.scalableminds.util.time.Instant import com.typesafe.scalalogging.LazyLogging import play.api.http.{HttpEntity, Status} import play.api.mvc.{Request, Result} -import scala.concurrent.duration.FiniteDuration -import scala.concurrent.{ExecutionContext, Future} +import java.io.{PrintWriter, StringWriter} import scala.concurrent.duration._ +import scala.concurrent.{ExecutionContext, Future} trait AbstractRequestLogging extends LazyLogging { @@ -30,38 +29,38 @@ trait AbstractRequestLogging extends LazyLogging { case _ => "" } -} - -trait RequestLogging extends AbstractRequestLogging { - // Hint: within webKnossos itself, UserAwareRequestLogging is available, which additionally logs the requester user id - - def log(notifier: Option[String => Unit] = None)(block: => Future[Result])(implicit request: Request[_], - ec: ExecutionContext): Future[Result] = - for { - result: Result <- block - _ = logRequestFormatted(request, result, notifier) - } yield result - def logTime(notifier: String => Unit, durationThreshold: FiniteDuration = 30 seconds)( block: => Future[Result])(implicit request: Request[_], ec: ExecutionContext): Future[Result] = { - def logTimeFormatted(executionTime: Long, request: Request[_], result: Result): Unit = { - val debugString = s"Request ${request.method} ${request.uri} took ${BigDecimal(executionTime / 1e9) + def logTimeFormatted(executionTime: FiniteDuration, request: Request[_], result: Result): Unit = { + val debugString = s"Request ${request.method} ${request.uri} took ${BigDecimal(executionTime.toMillis / 1000) .setScale(2, BigDecimal.RoundingMode.HALF_UP)} seconds and was${if (result.header.status != 200) " not " else " "}successful" logger.info(debugString) notifier(debugString) } - val start = System.nanoTime() + val start = Instant.now for { result: Result <- block - executionTime = System.nanoTime() - start - _ = if (executionTime > durationThreshold.toNanos) logTimeFormatted(executionTime, request, result) + executionTime = Instant.now - start + _ = if (executionTime > durationThreshold) logTimeFormatted(executionTime, request, result) } yield result } } +trait RequestLogging extends AbstractRequestLogging { + // Hint: within webKnossos itself, UserAwareRequestLogging is available, which additionally logs the requester user id + + def log(notifier: Option[String => Unit] = None)(block: => Future[Result])(implicit request: Request[_], + ec: ExecutionContext): Future[Result] = + for { + result: Result <- block + _ = logRequestFormatted(request, result, notifier) + } yield result + +} + trait RateLimitedErrorLogging extends LazyLogging { // Allows to log errors that occur many times only once (per lifetime of the class) diff --git a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/Application.scala b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/Application.scala index 52866b5a9f9..796d63f1ed2 100644 --- a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/Application.scala +++ b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/Application.scala @@ -1,5 +1,6 @@ package com.scalableminds.webknossos.datastore.controllers +import com.scalableminds.util.time.Instant import com.scalableminds.util.tools.Fox import com.scalableminds.webknossos.datastore.services.ApplicationHealthService import com.scalableminds.webknossos.datastore.storage.DataStoreRedisStore @@ -18,11 +19,10 @@ class Application @Inject()(redisClient: DataStoreRedisStore, applicationHealthS def health: Action[AnyContent] = Action.async { implicit request => log() { for { - before <- Fox.successful(System.currentTimeMillis()) + before <- Fox.successful(Instant.now) _ <- redisClient.checkHealth _ <- Fox.bool2Fox(applicationHealthService.getRecentProblem().isEmpty) ?~> "Java Internal Errors detected" - afterChecks = System.currentTimeMillis() - _ = logger.info(s"Answering ok for Datastore health check, took ${afterChecks - before} ms") + _ = logger.info(s"Answering ok for Datastore health check, took ${Instant.now - before}") } yield Ok("Ok") } } diff --git a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/BinaryDataController.scala b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/BinaryDataController.scala index 60f58a7b8f5..a7e97bdc097 100644 --- a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/BinaryDataController.scala +++ b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/BinaryDataController.scala @@ -3,6 +3,7 @@ package com.scalableminds.webknossos.datastore.controllers import com.google.inject.Inject import com.scalableminds.util.geometry.Vec3Int import com.scalableminds.util.image.{ImageCreator, ImageCreatorParameters, JPEGWriter} +import com.scalableminds.util.time.Instant import com.scalableminds.util.tools.Fox import com.scalableminds.webknossos.datastore.DataStoreConfig import com.scalableminds.webknossos.datastore.helpers.MissingBucketHeaders @@ -21,6 +22,7 @@ import net.liftweb.util.Helpers.tryo import play.api.i18n.Messages import play.api.libs.json.Json import play.api.mvc._ +import scala.concurrent.duration.DurationInt import java.io.ByteArrayOutputStream import java.nio.{ByteBuffer, ByteOrder} @@ -57,14 +59,14 @@ class BinaryDataController @Inject()( accessTokenService.validateAccess(UserAccessRequest.readDataSources(DataSourceId(dataSetName, organizationName)), urlOrHeaderToken(token, request)) { logTime(slackNotificationService.noticeSlowRequest) { - val t = System.currentTimeMillis() + val t = Instant.now for { (dataSource, dataLayer) <- dataSourceRepository.getDataSourceAndDataLayer(organizationName, dataSetName, dataLayerName) ~> NOT_FOUND (data, indices) <- requestData(dataSource, dataLayer, request.body) - duration = System.currentTimeMillis() - t - _ = if (duration > 10000) + duration = Instant.now - t + _ = if (duration > (10 seconds)) logger.info( s"Complete data request took $duration ms.\n" + s" dataSource: $organizationName/$dataSetName\n" diff --git a/yarn.lock b/yarn.lock index f768dc40d9e..3608646763b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -785,11 +785,29 @@ resolved "https://registry.yarnpkg.com/@redux-saga/symbols/-/symbols-1.1.2.tgz#216a672a487fc256872b8034835afc22a2d0595d" integrity sha512-EfdGnF423glv3uMwLsGAtE6bg+R9MdqlHEzExnfagXPrIiuxwr3bdiAwz3gi+PsrQ3yBlaBpfGLtDG8rf3LgQQ== +"@redux-saga/symbols@^1.1.3": + version "1.1.3" + resolved "https://registry.yarnpkg.com/@redux-saga/symbols/-/symbols-1.1.3.tgz#b731d56201719e96dc887dc3ae9016e761654367" + integrity sha512-hCx6ZvU4QAEUojETnX8EVg4ubNLBFl1Lps4j2tX7o45x/2qg37m3c6v+kSp8xjDJY+2tJw4QB3j8o8dsl1FDXg== + +"@redux-saga/testing-utils@^1.1.5": + version "1.1.5" + resolved "https://registry.yarnpkg.com/@redux-saga/testing-utils/-/testing-utils-1.1.5.tgz#dee03394666e6434b022d8e16beeb25b9e2417c2" + integrity sha512-XhE/j7tcW7B5rkoXQ5yoIj6j2KI84tsPGc5DfxRJNKdb8nOBz7UPgEqrDHiOawbbWAD1sZ0f4DSG/2RUc744CA== + dependencies: + "@redux-saga/symbols" "^1.1.3" + "@redux-saga/types" "^1.2.1" + "@redux-saga/types@^1.1.0": version "1.1.0" resolved "https://registry.yarnpkg.com/@redux-saga/types/-/types-1.1.0.tgz#0e81ce56b4883b4b2a3001ebe1ab298b84237204" integrity sha512-afmTuJrylUU/0OtqzaRkbyYFFNgCF73Bvel/sw90pvGrWIZ+vyoIJqA6eMSoA6+nb443kTmulmBtC9NerXboNg== +"@redux-saga/types@^1.2.1": + version "1.2.1" + resolved "https://registry.yarnpkg.com/@redux-saga/types/-/types-1.2.1.tgz#9403f51c17cae37edf870c6bc0c81c1ece5ccef8" + integrity sha512-1dgmkh+3so0+LlBWRhGA33ua4MYr7tUOj+a9Si28vUi0IUFNbff1T3sgpeDJI/LaC75bBYnQ0A3wXjn0OrRNBA== + "@rehooks/document-title@^1.0.2": version "1.0.2" resolved "https://registry.yarnpkg.com/@rehooks/document-title/-/document-title-1.0.2.tgz#d03e8a09e09f6fd481518982b151ab0da4bc5eec"