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

Use ObjectId directly as parameter in routes #8285

Merged
merged 11 commits into from
Feb 5, 2025
26 changes: 11 additions & 15 deletions app/controllers/AiModelController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -86,23 +86,21 @@ class AiModelController @Inject()(
extends Controller
with FoxImplicits {

def readAiModelInfo(aiModelId: String): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
def readAiModelInfo(aiModelId: ObjectId): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
{
for {
_ <- userService.assertIsSuperUser(request.identity)
aiModelIdValidated <- ObjectId.fromString(aiModelId)
aiModel <- aiModelDAO.findOne(aiModelIdValidated) ?~> "aiModel.notFound" ~> NOT_FOUND
aiModel <- aiModelDAO.findOne(aiModelId) ?~> "aiModel.notFound" ~> NOT_FOUND
jsResult <- aiModelService.publicWrites(aiModel)
} yield Ok(jsResult)
}
}

def readAiInferenceInfo(aiInferenceId: String): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
def readAiInferenceInfo(aiInferenceId: ObjectId): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
{
for {
_ <- userService.assertIsSuperUser(request.identity)
aiInferenceIdValidated <- ObjectId.fromString(aiInferenceId)
aiInference <- aiInferenceDAO.findOne(aiInferenceIdValidated) ?~> "aiInference.notFound" ~> NOT_FOUND
aiInference <- aiInferenceDAO.findOne(aiInferenceId) ?~> "aiInference.notFound" ~> NOT_FOUND
jsResult <- aiInferenceService.publicWrites(aiInference, request.identity)
} yield Ok(jsResult)
}
Expand Down Expand Up @@ -213,15 +211,14 @@ class AiModelController @Inject()(
} yield Ok(newAiModelJs)
}

def updateAiModelInfo(aiModelId: String): Action[UpdateAiModelParameters] =
def updateAiModelInfo(aiModelId: ObjectId): Action[UpdateAiModelParameters] =
sil.SecuredAction.async(validateJson[UpdateAiModelParameters]) { implicit request =>
for {
_ <- userService.assertIsSuperUser(request.identity)
aiModelIdValidated <- ObjectId.fromString(aiModelId)
aiModel <- aiModelDAO.findOne(aiModelIdValidated) ?~> "aiModel.notFound" ~> NOT_FOUND
aiModel <- aiModelDAO.findOne(aiModelId) ?~> "aiModel.notFound" ~> NOT_FOUND
_ <- aiModelDAO.updateOne(
aiModel.copy(name = request.body.name, comment = request.body.comment, modified = Instant.now))
updatedAiModel <- aiModelDAO.findOne(aiModelIdValidated) ?~> "aiModel.notFound" ~> NOT_FOUND
updatedAiModel <- aiModelDAO.findOne(aiModelId) ?~> "aiModel.notFound" ~> NOT_FOUND
jsResult <- aiModelService.publicWrites(updatedAiModel)
} yield Ok(jsResult)
}
Expand All @@ -248,15 +245,14 @@ class AiModelController @Inject()(
} yield Ok
}

def deleteAiModel(aiModelId: String): Action[AnyContent] =
def deleteAiModel(aiModelId: ObjectId): Action[AnyContent] =
sil.SecuredAction.async { implicit request =>
for {
_ <- userService.assertIsSuperUser(request.identity)
aiModelIdValidated <- ObjectId.fromString(aiModelId)
referencesCount <- aiInferenceDAO.countForModel(aiModelIdValidated)
referencesCount <- aiInferenceDAO.countForModel(aiModelId)
_ <- bool2Fox(referencesCount == 0) ?~> "aiModel.delete.referencedByInferences"
_ <- aiModelDAO.findOne(aiModelIdValidated) ?~> "aiModel.notFound" ~> NOT_FOUND
_ <- aiModelDAO.deleteOne(aiModelIdValidated)
_ <- aiModelDAO.findOne(aiModelId) ?~> "aiModel.notFound" ~> NOT_FOUND
_ <- aiModelDAO.deleteOne(aiModelId)
} yield Ok
}

Expand Down
81 changes: 40 additions & 41 deletions app/controllers/AnnotationController.scala

Large diffs are not rendered by default.

24 changes: 11 additions & 13 deletions app/controllers/AnnotationIOController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ class AnnotationIOController @Inject()(

// NML or Zip file containing skeleton and/or volume data of this annotation. In case of Compound annotations, multiple such annotations wrapped in another zip
def download(typ: String,
id: String,
id: ObjectId,
skeletonVersion: Option[Long],
volumeVersion: Option[Long],
skipVolumeData: Option[Boolean],
Expand All @@ -359,7 +359,7 @@ class AnnotationIOController @Inject()(
for {
identifier <- AnnotationIdentifier.parse(typ, id)
volumeDataZipFormatParsed = volumeDataZipFormat.flatMap(VolumeDataZipFormat.fromString)
_ = request.identity.foreach(user => analyticsService.track(DownloadAnnotationEvent(user, id, typ)))
_ = request.identity.foreach(user => analyticsService.track(DownloadAnnotationEvent(user, id.toString, typ)))
result <- identifier.annotationType match {
case AnnotationType.View => Fox.failure("Cannot download View annotation")
case AnnotationType.CompoundProject => downloadProject(id, request.identity, skipVolumeData.getOrElse(false))
Expand All @@ -379,7 +379,7 @@ class AnnotationIOController @Inject()(
} yield result
}

def downloadWithoutType(id: String,
def downloadWithoutType(id: ObjectId,
skeletonVersion: Option[Long],
volumeVersion: Option[Long],
skipVolumeData: Option[Boolean],
Expand All @@ -396,7 +396,7 @@ class AnnotationIOController @Inject()(
} yield result
}

private def downloadExplorational(annotationId: String,
private def downloadExplorational(annotationId: ObjectId,
typ: String,
issuingUser: Option[User],
skeletonVersion: Option[Long],
Expand Down Expand Up @@ -523,15 +523,14 @@ class AnnotationIOController @Inject()(
}
}

private def downloadProject(projectId: String, userOpt: Option[User], skipVolumeData: Boolean)(
private def downloadProject(projectId: ObjectId, userOpt: Option[User], skipVolumeData: Boolean)(
implicit ctx: DBAccessContext,
m: MessagesProvider) =
for {
user <- userOpt.toFox ?~> Messages("notAllowed") ~> FORBIDDEN
projectIdValidated <- ObjectId.fromString(projectId)
project <- projectDAO.findOne(projectIdValidated) ?~> Messages("project.notFound", projectId) ~> NOT_FOUND
project <- projectDAO.findOne(projectId) ?~> Messages("project.notFound", projectId) ~> NOT_FOUND
_ <- Fox.assertTrue(userService.isTeamManagerOrAdminOf(user, project._team)) ?~> "notAllowed" ~> FORBIDDEN
annotations <- annotationDAO.findAllFinishedForProject(projectIdValidated)
annotations <- annotationDAO.findAllFinishedForProject(projectId)
zip <- annotationService.zipAnnotations(annotations,
project.name,
skipVolumeData,
Expand All @@ -541,7 +540,7 @@ class AnnotationIOController @Inject()(
Ok.sendFile(file, inline = false, fileName = _ => Some(TextUtils.normalize(project.name + "_nmls.zip")))
}

private def downloadTask(taskId: String, userOpt: Option[User], skipVolumeData: Boolean)(
private def downloadTask(taskId: ObjectId, userOpt: Option[User], skipVolumeData: Boolean)(
implicit ctx: DBAccessContext,
m: MessagesProvider) = {
def createTaskZip(task: Task): Fox[TemporaryFile] = annotationService.annotationsFor(task._id).flatMap {
Expand All @@ -553,7 +552,7 @@ class AnnotationIOController @Inject()(

for {
user <- userOpt.toFox ?~> Messages("notAllowed") ~> FORBIDDEN
task <- taskDAO.findOne(ObjectId(taskId)).toFox ?~> Messages("task.notFound") ~> NOT_FOUND
task <- taskDAO.findOne(taskId).toFox ?~> Messages("task.notFound") ~> NOT_FOUND
project <- projectDAO.findOne(task._project) ?~> Messages("project.notFound") ~> NOT_FOUND
_ <- Fox.assertTrue(userService.isTeamManagerOrAdminOf(user, project._team)) ?~> Messages("notAllowed") ~> FORBIDDEN
zip <- createTaskZip(task)
Expand All @@ -563,7 +562,7 @@ class AnnotationIOController @Inject()(
}
}

private def downloadTaskType(taskTypeId: String, userOpt: Option[User], skipVolumeData: Boolean)(
private def downloadTaskType(taskTypeId: ObjectId, userOpt: Option[User], skipVolumeData: Boolean)(
implicit ctx: DBAccessContext,
m: MessagesProvider) = {
def createTaskTypeZip(taskType: TaskType) =
Expand All @@ -582,8 +581,7 @@ class AnnotationIOController @Inject()(

for {
user <- userOpt.toFox ?~> Messages("notAllowed") ~> FORBIDDEN
taskTypeIdValidated <- ObjectId.fromString(taskTypeId) ?~> "taskType.id.invalid"
taskType <- taskTypeDAO.findOne(taskTypeIdValidated) ?~> "taskType.notFound" ~> NOT_FOUND
taskType <- taskTypeDAO.findOne(taskTypeId) ?~> "taskType.notFound" ~> NOT_FOUND
_ <- Fox.assertTrue(userService.isTeamManagerOrAdminOf(user, taskType._team)) ?~> "notAllowed" ~> FORBIDDEN
zip <- createTaskTypeZip(taskType)
} yield {
Expand Down
27 changes: 11 additions & 16 deletions app/controllers/AnnotationPrivateLinkController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,17 @@ class AnnotationPrivateLinkController @Inject()(
} yield Ok(Json.toJson(linksJsonList))
}

def listByAnnotation(annotationId: String): Action[AnyContent] =
def listByAnnotation(annotationId: ObjectId): Action[AnyContent] =
sil.SecuredAction.async { implicit request =>
for {
annotationIdValidated <- ObjectId.fromString(annotationId)
links <- annotationPrivateLinkDAO.findAllByAnnotation(annotationIdValidated)
links <- annotationPrivateLinkDAO.findAllByAnnotation(annotationId)
linksJsonList <- Fox.serialCombined(links)(annotationPrivateLinkService.publicWrites)
} yield Ok(Json.toJson(linksJsonList))
}

def get(id: String): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
def get(id: ObjectId): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
for {
idValidated <- ObjectId.fromString(id)

annotationPrivateLink <- annotationPrivateLinkDAO.findOne(idValidated)
annotationPrivateLink <- annotationPrivateLinkDAO.findOne(id)
_ <- bool2Fox(annotationPrivateLink.expirationDateTime.forall(_ > Instant.now)) ?~> "Token expired" ~> NOT_FOUND
_ <- annotationDAO.findOne(annotationPrivateLink._annotation) ?~> "annotation.notFound" ~> NOT_FOUND

Expand All @@ -100,27 +97,25 @@ class AnnotationPrivateLinkController @Inject()(
} yield Ok(js)
}

def update(id: String): Action[AnnotationPrivateLinkParams] =
def update(id: ObjectId): Action[AnnotationPrivateLinkParams] =
sil.SecuredAction.async(validateJson[AnnotationPrivateLinkParams]) { implicit request =>
val params = request.body
for {
annotationId <- ObjectId.fromString(params.annotation)
idValidated <- ObjectId.fromString(id)
aPLInfo <- annotationPrivateLinkDAO.findOne(idValidated) ?~> "annotation private link not found" ~> NOT_FOUND
aPLInfo <- annotationPrivateLinkDAO.findOne(id) ?~> "annotation private link not found" ~> NOT_FOUND
_ <- annotationDAO.assertUpdateAccess(aPLInfo._annotation) ?~> "notAllowed" ~> FORBIDDEN
_ <- annotationDAO.assertUpdateAccess(annotationId) ?~> "notAllowed" ~> FORBIDDEN
_ <- annotationPrivateLinkDAO.updateOne(idValidated, annotationId, params.expirationDateTime) ?~> "update.failed"
updated <- annotationPrivateLinkDAO.findOne(idValidated) ?~> "not Found"
_ <- annotationPrivateLinkDAO.updateOne(id, annotationId, params.expirationDateTime) ?~> "update.failed"
updated <- annotationPrivateLinkDAO.findOne(id) ?~> "not Found"
js <- annotationPrivateLinkService.publicWrites(updated) ?~> "write failed"
} yield Ok(js)
}
Comment on lines +100 to 112
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider converting annotation parameter to ObjectId in AnnotationPrivateLinkParams.

While the id parameter has been updated to ObjectId, the method still converts params.annotation from String to ObjectId. Consider updating AnnotationPrivateLinkParams to use ObjectId directly for consistency.

- annotationId <- ObjectId.fromString(params.annotation)
+ annotationId = params.annotation  // After updating AnnotationPrivateLinkParams

Committable suggestion skipped: line range outside the PR's diff.


def delete(id: String): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
def delete(id: ObjectId): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
for {
idValidated <- ObjectId.fromString(id)
aPLInfo <- annotationPrivateLinkDAO.findOne(idValidated) ?~> "notFound" ~> NOT_FOUND
aPLInfo <- annotationPrivateLinkDAO.findOne(id) ?~> "notFound" ~> NOT_FOUND
_ <- annotationDAO.assertUpdateAccess(aPLInfo._annotation) ?~> "notAllowed" ~> FORBIDDEN
_ <- annotationPrivateLinkDAO.deleteOne(idValidated) ?~> "delete failed"
_ <- annotationPrivateLinkDAO.deleteOne(id) ?~> "delete failed"
} yield JsonOk("privateLink deleted")
}
}
5 changes: 2 additions & 3 deletions app/controllers/AuthenticationController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,13 @@ class AuthenticationController @Inject()(
result <- combinedAuthenticatorService.embed(cookie, Redirect("/dashboard")) //to login the new user
} yield result

def accessibleBySwitching(datasetId: Option[String],
def accessibleBySwitching(datasetId: Option[ObjectId],
annotationId: Option[String],
workflowHash: Option[String]): Action[AnyContent] = sil.SecuredAction.async {
implicit request =>
for {
datasetIdValidated <- Fox.runOptional(datasetId)(ObjectId.fromString(_))
selectedOrganization <- authenticationService.getOrganizationToSwitchTo(request.identity,
datasetIdValidated,
datasetId,
annotationId,
workflowHash)
selectedOrganizationJs <- organizationService.publicWrites(selectedOrganization)
Expand Down
29 changes: 11 additions & 18 deletions app/controllers/ConfigurationController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,50 +35,43 @@ class ConfigurationController @Inject()(
} yield JsonOk(Messages("user.configuration.updated"))
}

def readDatasetViewConfiguration(datasetId: String, sharingToken: Option[String]): Action[List[String]] =
def readDatasetViewConfiguration(datasetId: ObjectId, sharingToken: Option[String]): Action[List[String]] =
sil.UserAwareAction.async(validateJson[List[String]]) { implicit request =>
val ctx = URLSharing.fallbackTokenAccessContext(sharingToken)
for {
datasetIdValidated <- ObjectId.fromString(datasetId)
configuration <- request.identity.toFox
.flatMap(
user =>
datasetConfigurationService.getDatasetViewConfigurationForUserAndDataset(
request.body,
user,
datasetIdValidated)(GlobalAccessContext))
.flatMap(user =>
datasetConfigurationService.getDatasetViewConfigurationForUserAndDataset(request.body, user, datasetId)(
GlobalAccessContext))
.orElse(
datasetConfigurationService.getDatasetViewConfigurationForDataset(request.body, datasetIdValidated)(ctx)
datasetConfigurationService.getDatasetViewConfigurationForDataset(request.body, datasetId)(ctx)
)
.getOrElse(Map.empty)
} yield Ok(Json.toJson(configuration))
}

def updateDatasetViewConfiguration(datasetId: String): Action[JsValue] =
def updateDatasetViewConfiguration(datasetId: ObjectId): Action[JsValue] =
sil.SecuredAction.async(parse.json(maxLength = 20480)) { implicit request =>
for {
jsConfiguration <- request.body.asOpt[JsObject] ?~> "user.configuration.dataset.invalid"
datasetIdValidated <- ObjectId.fromString(datasetId)
conf = jsConfiguration.fields.toMap
datasetConf = conf - "layers"
layerConf = conf.get("layers")
_ <- userService.updateDatasetViewConfiguration(request.identity, datasetIdValidated, datasetConf, layerConf)
_ <- userService.updateDatasetViewConfiguration(request.identity, datasetId, datasetConf, layerConf)
} yield JsonOk(Messages("user.configuration.dataset.updated"))
}

def readDatasetAdminViewConfiguration(datasetId: String): Action[AnyContent] =
def readDatasetAdminViewConfiguration(datasetId: ObjectId): Action[AnyContent] =
sil.SecuredAction.async { implicit request =>
for {
datasetIdValidated <- ObjectId.fromString(datasetId)
configuration <- datasetConfigurationService.getCompleteAdminViewConfiguration(datasetIdValidated)
configuration <- datasetConfigurationService.getCompleteAdminViewConfiguration(datasetId)
} yield Ok(Json.toJson(configuration))
}

def updateDatasetAdminViewConfiguration(datasetNameAndId: String): Action[JsValue] =
def updateDatasetAdminViewConfiguration(datasetId: ObjectId): Action[JsValue] =
sil.SecuredAction.async(parse.json(maxLength = 20480)) { implicit request =>
for {
datasetIdValidated <- ObjectId.fromString(datasetNameAndId)
dataset <- datasetDAO.findOne(datasetIdValidated)(GlobalAccessContext)
dataset <- datasetDAO.findOne(datasetId)(GlobalAccessContext)
_ <- datasetService.isEditableBy(dataset, Some(request.identity)) ?~> "notAllowed" ~> FORBIDDEN
jsObject <- request.body.asOpt[JsObject].toFox ?~> "user.configuration.dataset.invalid"
_ <- datasetConfigurationService.updateAdminViewConfigurationFor(dataset, jsObject.fields.toMap)
Expand Down
Loading