Use ObjectId in TracingStore where applicable

app/controllers/AuthenticationController.scala

@@ -215,7 +215,7 @@ class AuthenticationController @Inject()(
     } yield result
 
   def accessibleBySwitching(datasetId: Option[ObjectId],
-                            annotationId: Option[String],
+                            annotationId: Option[ObjectId],
                             workflowHash: Option[String]): Action[AnyContent] = sil.SecuredAction.async {
     implicit request =>
       for {

app/controllers/WKRemoteTracingStoreController.scala

@@ -82,8 +82,7 @@ class WKRemoteTracingStoreController @Inject()(tracingStoreService: TracingStore
       tracingStoreService.validateAccess(name, key) { _ =>
         val report = request.body
         for {
-          annotationId <- ObjectId.fromString(report.annotationId)
-          annotation <- annotationDAO.findOne(annotationId)
+          annotation <- annotationDAO.findOne(report.annotationId)
           _ <- ensureAnnotationNotFinished(annotation)
           _ <- annotationDAO.updateModified(annotation._id, Instant.now)
           _ = report.statistics.map(statistics => annotationService.updateStatistics(annotation._id, statistics))

app/models/annotation/AnnotationLayerPrecedence.scala

@@ -29,7 +29,7 @@ case class RedundantTracingProperties(
     zoomLevel: Double,
     userBoundingBoxes: Seq[NamedBoundingBoxProto],
     editPositionAdditionalCoordinates: Seq[AdditionalCoordinateProto],
-    userStateBoundingBoxVisibilities: Map[String, Seq[Id32WithBool]] // UserId → Seq(bboxId, bboxIsVisible)
+    userStateBoundingBoxVisibilities: Map[ObjectId, Seq[Id32WithBool]] // UserId → Seq(bboxId, bboxIsVisible)
 )
 
 trait AnnotationLayerPrecedence extends FoxImplicits {
@@ -79,7 +79,7 @@ trait AnnotationLayerPrecedence extends FoxImplicits {
       userStates: Seq[SkeletonUserStateProto],
       oldPrecedenceLayerProperties: RedundantTracingProperties): Seq[SkeletonUserStateProto] = {
     val adaptedExistingUserStates = userStates.map { userState =>
-      val userId = userState.userId
+      val userId = ObjectId(userState.userId)
       oldPrecedenceLayerProperties.userStateBoundingBoxVisibilities.get(userId) match {
         case None => userState
         case Some(precedenceBboxVisibilities) =>
@@ -88,9 +88,9 @@ trait AnnotationLayerPrecedence extends FoxImplicits {
     }
     // We also have to create new user states for the users the old precedence layer has, but the new precedence layer is missing.
     val newUserPrecedenceProperties = oldPrecedenceLayerProperties.userStateBoundingBoxVisibilities.filter(tuple =>
-      !userStates.exists(_.userId == tuple._1))
+      !userStates.exists(_.userId == tuple._1.toString))
     val newUserStates = newUserPrecedenceProperties.map {
-      case (userId: String, boundingBoxVisibilities: Seq[Id32WithBool]) =>
+      case (userId: ObjectId, boundingBoxVisibilities: Seq[Id32WithBool]) =>
         SkeletonTracingDefaults
           .emptyUserState(userId)
           .copy(
@@ -104,7 +104,7 @@ trait AnnotationLayerPrecedence extends FoxImplicits {
       userStates: Seq[VolumeUserStateProto],
       oldPrecedenceLayerProperties: RedundantTracingProperties): Seq[VolumeUserStateProto] = {
     val adaptedExistingUserStates = userStates.map { userState =>
-      val userId = userState.userId
+      val userId = ObjectId(userState.userId)
       oldPrecedenceLayerProperties.userStateBoundingBoxVisibilities.get(userId) match {
         case None => userState
         case Some(precedenceBboxVisibilities) =>
@@ -113,9 +113,9 @@ trait AnnotationLayerPrecedence extends FoxImplicits {
     }
     // We also have to create new user states for the users the old precedence layer has, but the new precedence layer is missing.
     val newUserPrecedenceProperties = oldPrecedenceLayerProperties.userStateBoundingBoxVisibilities.filter(tuple =>
-      !userStates.exists(_.userId == tuple._1))
+      !userStates.exists(_.userId == tuple._1.toString))
     val newUserStates = newUserPrecedenceProperties.map {
-      case (userId: String, boundingBoxVisibilities: Seq[Id32WithBool]) =>
+      case (userId: ObjectId, boundingBoxVisibilities: Seq[Id32WithBool]) =>
         VolumeTracingDefaults
           .emptyUserState(userId)
           .copy(
@@ -194,7 +194,7 @@ trait AnnotationLayerPrecedence extends FoxImplicits {
           s.userBoundingBoxes ++ s.userBoundingBox.map(
             com.scalableminds.webknossos.datastore.geometry.NamedBoundingBoxProto(0, None, None, None, _)),
           s.editPositionAdditionalCoordinates,
-          s.userStates.map(userState => (userState.userId, userState.boundingBoxVisibilities)).toMap
+          s.userStates.map(userState => (ObjectId(userState.userId), userState.boundingBoxVisibilities)).toMap
         )
       case Right(v) =>
         RedundantTracingProperties(
@@ -204,7 +204,7 @@ trait AnnotationLayerPrecedence extends FoxImplicits {
           v.userBoundingBoxes ++ v.userBoundingBox.map(
             com.scalableminds.webknossos.datastore.geometry.NamedBoundingBoxProto(0, None, None, None, _)),
           v.editPositionAdditionalCoordinates,
-          v.userStates.map(userState => (userState.userId, userState.boundingBoxVisibilities)).toMap
+          v.userStates.map(userState => (ObjectId(userState.userId), userState.boundingBoxVisibilities)).toMap
         )
     }
 }

app/models/annotation/AnnotationMerger.scala

@@ -74,7 +74,7 @@ class AnnotationMerger @Inject()(datasetDAO: DatasetDAO, tracingStoreService: Tr
     for {
       dataset <- datasetDAO.findOne(datasetId)
       tracingStoreClient: WKRemoteTracingStoreClient <- tracingStoreService.clientFor(dataset)
-      mergedAnnotationProto <- tracingStoreClient.mergeAnnotationsByIds(annotations.map(_.id),
+      mergedAnnotationProto <- tracingStoreClient.mergeAnnotationsByIds(annotations.map(_._id),
                                                                         annotations.map(_._user),
                                                                         newAnnotationId,
                                                                         toTemporaryStore,

app/models/annotation/AnnotationService.scala

@@ -801,7 +801,7 @@ class AnnotationService @Inject()(
       dataStore <- dataStoreDAO.findOneByName(dataset._dataStore.trim) ?~> "datastore.notFound"
       tracingStore <- tracingStoreDAO.findFirst
       annotationSource = AnnotationSource(
-        id = annotation.id,
+        id = annotation._id,
         annotationLayers = annotation.annotationLayers,
         datasetDirectoryName = dataset.directoryName,
         organizationId = organization._id,

app/models/annotation/WKRemoteTracingStoreClient.scala

@@ -172,7 +172,7 @@ class WKRemoteTracingStoreClient(
       .postEmpty()
   }
 
-  def mergeAnnotationsByIds(annotationIds: List[String],
+  def mergeAnnotationsByIds(annotationIds: List[ObjectId],
                             ownerIds: List[ObjectId],
                             newAnnotationId: ObjectId,
                             toTemporaryStore: Boolean,
@@ -183,8 +183,8 @@ class WKRemoteTracingStoreClient(
       .addQueryString("toTemporaryStore" -> toTemporaryStore.toString)
       .addQueryString("newAnnotationId" -> newAnnotationId.toString)
       .addQueryString("requestingUserId" -> requestingUserId.toString)
-      .postJsonWithProtoResponse[MergedFromIdsRequest, AnnotationProto](
-        MergedFromIdsRequest(annotationIds, ownerIds.map(_.toString)))(AnnotationProto)
+      .postJsonWithProtoResponse[MergedFromIdsRequest, AnnotationProto](MergedFromIdsRequest(annotationIds, ownerIds))(
+        AnnotationProto)
   }
 
   def mergeSkeletonTracingsByContents(newTracingId: String, tracings: SkeletonTracings): Fox[Unit] = {

app/security/AccessibleBySwitchingService.scala

@@ -30,7 +30,7 @@ class AccessibleBySwitchingService @Inject()(
 
   def getOrganizationToSwitchTo(user: User,
                                 datasetId: Option[ObjectId],
-                                annotationId: Option[String],
+                                annotationId: Option[ObjectId],
                                 workflowHash: Option[String])(implicit ctx: DBAccessContext): Fox[Organization] =
     for {
       isSuperUser <- multiUserDAO.findOne(user._multiUser).map(_.isSuperUser)
@@ -42,7 +42,7 @@ class AccessibleBySwitchingService @Inject()(
     } yield selectedOrganization
 
   private def accessibleBySwitchingForSuperUser(datasetIdOpt: Option[ObjectId],
-                                                annotationIdOpt: Option[String],
+                                                annotationIdOpt: Option[ObjectId],
                                                 workflowHashOpt: Option[String]): Fox[Organization] = {
     implicit val ctx: DBAccessContext = GlobalAccessContext
     (datasetIdOpt, annotationIdOpt, workflowHashOpt) match {
@@ -53,8 +53,7 @@ class AccessibleBySwitchingService @Inject()(
         } yield organization
       case (None, Some(annotationId), None) =>
         for {
-          annotationObjectId <- ObjectId.fromString(annotationId)
-          annotation <- annotationDAO.findOne(annotationObjectId) // Note: this does not work for compound annotations.
+          annotation <- annotationDAO.findOne(annotationId) // Note: this does not work for compound annotations.
           user <- userDAO.findOne(annotation._user)
           organization <- organizationDAO.findOne(user._organization)
         } yield organization
@@ -69,7 +68,7 @@ class AccessibleBySwitchingService @Inject()(
 
   private def accessibleBySwitchingForMultiUser(multiUserId: ObjectId,
                                                 datasetIdOpt: Option[ObjectId],
-                                                annotationIdOpt: Option[String],
+                                                annotationIdOpt: Option[ObjectId],
                                                 workflowHashOpt: Option[String]): Fox[Organization] =
     for {
       identities <- userDAO.findAllByMultiUser(multiUserId)
@@ -80,7 +79,7 @@ class AccessibleBySwitchingService @Inject()(
 
   private def canAccessDatasetOrAnnotationOrWorkflow(user: User,
                                                      datasetIdOpt: Option[ObjectId],
-                                                     annotationIdOpt: Option[String],
+                                                     annotationIdOpt: Option[ObjectId],
                                                      workflowHashOpt: Option[String]): Fox[Boolean] = {
     val ctx = AuthorizedAccessContext(user)
     (datasetIdOpt, annotationIdOpt, workflowHashOpt) match {
@@ -99,12 +98,11 @@ class AccessibleBySwitchingService @Inject()(
     foundFox.shiftBox.map(_.isDefined)
   }
 
-  private def canAccessAnnotation(user: User, ctx: DBAccessContext, annotationId: String): Fox[Boolean] = {
+  private def canAccessAnnotation(user: User, ctx: DBAccessContext, annotationId: ObjectId): Fox[Boolean] = {
     val foundFox = for {
-      annotationIdParsed <- ObjectId.fromString(annotationId)
-      annotation <- annotationDAO.findOne(annotationIdParsed)(GlobalAccessContext)
+      annotation <- annotationDAO.findOne(annotationId)(GlobalAccessContext)
       _ <- Fox.fromBool(annotation.state != Cancelled)
-      restrictions <- annotationProvider.restrictionsFor(AnnotationIdentifier(annotation.typ, annotationIdParsed))(ctx)
+      restrictions <- annotationProvider.restrictionsFor(AnnotationIdentifier(annotation.typ, annotationId))(ctx)
       _ <- restrictions.allowAccess(user)
     } yield ()
     foundFox.shiftBox.map(_.isDefined)

build.sbt

@@ -34,13 +34,10 @@ Compile / console / scalacOptions -= "-Xlint:unused"
 scapegoatIgnoredFiles := Seq(".*/Tables.scala", ".*/Routes.scala", ".*/.*mail.*template\\.scala")
 scapegoatDisabledInspections := Seq("FinalModifierOnCaseClass", "UnusedMethodParameter", "UnsafeTraversableMethods")
 
-// Allow path binding for ObjectId
-routesImport += "com.scalableminds.util.objectid.ObjectId"
-
 lazy val commonSettings = Seq(
   resolvers ++= DependencyResolvers.dependencyResolvers,
   Compile / doc / sources := Seq.empty,
-  Compile / packageDoc / publishArtifact := false
+  Compile / packageDoc / publishArtifact := false,
 )
 
 lazy val protocolBufferSettings = Seq(
@@ -92,6 +89,7 @@ lazy val webknossosDatastore = (project in file("webknossos-datastore"))
       }
       ((libs +++ subs +++ targets) ** "*.jar").classpath
     },
+    routesImport += "com.scalableminds.util.objectid.ObjectId",
     copyMessagesFilesSetting
   )
 
@@ -102,11 +100,12 @@ lazy val webknossosTracingstore = (project in file("webknossos-tracingstore"))
   .settings(
     name := "webknossos-tracingstore",
     commonSettings,
+    routesImport += "com.scalableminds.util.objectid.ObjectId",
     generateReverseRouter := false,
     BuildInfoSettings.webknossosTracingstoreBuildInfoSettings,
     libraryDependencies ++= Dependencies.webknossosTracingstoreDependencies,
     dependencyOverrides ++= Dependencies.dependencyOverrides,
-    copyMessagesFilesSetting
+    copyMessagesFilesSetting,
   )
 
 lazy val webknossos = (project in file("."))
@@ -116,6 +115,7 @@ lazy val webknossos = (project in file("."))
   .settings(
     name := "webknossos",
     commonSettings,
+    routesImport += "com.scalableminds.util.objectid.ObjectId",
     generateReverseRouter := false,
     AssetCompilation.settings,
     BuildInfoSettings.webknossosBuildInfoSettings,

conf/webknossos.latest.routes

@@ -28,7 +28,7 @@ GET           /auth/token
 DELETE        /auth/token                                                                       controllers.AuthenticationController.deleteToken()
 GET           /auth/switch                                                                      controllers.AuthenticationController.switchMultiUser(to: String)
 POST          /auth/switchOrganization/:organizationId                                          controllers.AuthenticationController.switchOrganization(organizationId: String)
-GET           /auth/accessibleBySwitching                                                       controllers.AuthenticationController.accessibleBySwitching(datasetId: Option[ObjectId], annotationId: Option[String], workflowHash: Option[String])
+GET           /auth/accessibleBySwitching                                                       controllers.AuthenticationController.accessibleBySwitching(datasetId: Option[ObjectId], annotationId: Option[ObjectId], workflowHash: Option[String])
 POST          /auth/sendInvites                                                                 controllers.AuthenticationController.sendInvites()
 POST          /auth/startResetPassword                                                          controllers.AuthenticationController.handleStartResetPassword()
 POST          /auth/changePassword                                                              controllers.AuthenticationController.changePassword()

frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts

@@ -199,6 +199,8 @@ describe("Annotation API (E2E)", () => {
           [UpdateActions.updateCameraAnnotation([2, 3, 4], null, [1, 2, 3], 2)],
         ],
         123456789,
+        null,
+        true,
       ),
       0,
     );
@@ -246,6 +248,8 @@ describe("Annotation API (E2E)", () => {
       createSaveQueueFromUpdateActions(
         [createTreesUpdateActions, [updateTreeGroupsUpdateAction]],
         123456789,
+        null,
+        true,
       ),
       0,
     );
@@ -284,7 +288,12 @@ describe("Annotation API (E2E)", () => {
 
     const updateTreeAction = UpdateActions.updateTree(trees.getOrThrow(1), tracingId);
     const [saveQueue] = addVersionNumbers(
-      createSaveQueueFromUpdateActions([createTreesUpdateActions, [updateTreeAction]], 123456789),
+      createSaveQueueFromUpdateActions(
+        [createTreesUpdateActions, [updateTreeAction]],
+        123456789,
+        null,
+        true,
+      ),
       0,
     );
 
@@ -301,6 +310,8 @@ describe("Annotation API (E2E)", () => {
       createSaveQueueFromUpdateActions(
         [[UpdateActions.updateMetadataOfAnnotation(newDescription)]],
         123456789,
+        null,
+        true,
       ),
       0,
     );

frontend/javascripts/test/e2e-setup.ts

@@ -34,6 +34,8 @@ const tokenUserFInOrgaX =
 
 let currentUserAuthToken = tokenUserA;
 
+const idUserA = "570b9f4d2a7c0e4d008da6ef";
+
 function setUserAuthToken(token: string) {
   currentUserAuthToken = token;
 }
@@ -163,4 +165,5 @@ export {
   tokenUserFInOrgaX,
   tokenUserFInOrgaY,
   setUserAuthToken,
+  idUserA,
 };

frontend/javascripts/test/helpers/saveHelpers.ts

@@ -1,12 +1,14 @@
 import type { TracingStats } from "viewer/model/accessors/annotation_accessor";
 import type { UpdateActionWithoutIsolationRequirement } from "viewer/model/sagas/update_actions";
 import type { SaveQueueEntry } from "viewer/store";
+import { idUserA } from "test/e2e-setup";
 import dummyUser from "test/fixtures/dummy_user";
 
 export function createSaveQueueFromUpdateActions(
   updateActions: UpdateActionWithoutIsolationRequirement[][],
   timestamp: number,
   stats: TracingStats | null = null,
+  useE2eAuthorId: boolean = false,
 ): SaveQueueEntry[] {
   return updateActions.map((ua) => ({
     version: -1,
@@ -15,7 +17,7 @@ export function createSaveQueueFromUpdateActions(
     actions: ua,
     info: "[]",
     transactionGroupCount: 1,
-    authorId: dummyUser.id,
+    authorId: useE2eAuthorId ? idUserA : dummyUser.id,
     transactionGroupIndex: 0,
     transactionId: "dummyRequestId",
   }));